Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[net7.0] Fix edge case with Grids, *s, and unconstrained layouts #14369

Merged
merged 3 commits into from
Apr 11, 2023
Merged

[net7.0] Fix edge case with Grids, *s, and unconstrained layouts #14369

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e0765ca
Fix edge case with Grids, *s, and unconstrained layouts
hartez Mar 22, 2023
876a692
Auto-format source code
Mar 22, 2023
693dcfe
Ensure that second-pass measures of Auto rows/columns are allowed to …
hartez Mar 31, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 62 additions & 28 deletions src/Core/src/Layouts/GridLayoutManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -397,30 +397,36 @@ void KnownMeasurePass()
var measure = MeasureCell(cell, measureWidth, measureHeight);

if (treatCellWidthAsAuto)
{
if (cell.ColumnSpan == 1)
{
_columns[cell.Column].Update(measure.Width);
}
else
{
TrackSpan(new Span(cell.Column, cell.ColumnSpan, true, measure.Width));
}
}
{
if (cell.ColumnSpan == 1)
{
if (!cell.NeedsSecondPass)
{
_columns[cell.Column].Update(measure.Width);
}
}
else
{
TrackSpan(new Span(cell.Column, cell.ColumnSpan, true, measure.Width));
}
}

if (treatCellHeightAsAuto)
{
if (cell.RowSpan == 1)
{
_rows[cell.Row].Update(measure.Height);
}
else
{
TrackSpan(new Span(cell.Row, cell.RowSpan, false, measure.Height));
}
}
}
}
{
if (cell.RowSpan == 1)
{
if (!cell.NeedsSecondPass)
{
_rows[cell.Row].Update(measure.Height);
}
}
else
{
TrackSpan(new Span(cell.Row, cell.RowSpan, false, measure.Height));
}
}
}
}

void SecondMeasurePass()
{
Expand All @@ -434,14 +440,28 @@ void SecondMeasurePass()
double width = 0;
double height = 0;

for (int n = cell.Row; n < cell.Row + cell.RowSpan; n++)
if (cell.IsRowSpanAuto)
{
height += _rows[n].Size;
height = double.PositiveInfinity;
}
else
{
for (int n = cell.Row; n < cell.Row + cell.RowSpan; n++)
{
height += _rows[n].Size;
}
}

for (int n = cell.Column; n < cell.Column + cell.ColumnSpan; n++)
if (cell.IsColumnSpanAuto)
{
width += _columns[n].Size;
width = double.PositiveInfinity;
}
else
{
for (int n = cell.Column; n < cell.Column + cell.ColumnSpan; n++)
{
width += _columns[n].Size;
}
}

if (width == 0 || height == 0)
Expand All @@ -455,11 +475,19 @@ void SecondMeasurePass()
{
TrackSpan(new Span(cell.Column, cell.ColumnSpan, true, measure.Width));
}
else if (cell.ColumnSpan == 1)
{
_columns[cell.Column].Update(measure.Width);
}

if (cell.IsRowSpanStar && cell.RowSpan > 1)
{
TrackSpan(new Span(cell.Row, cell.RowSpan, false, measure.Height));
}
else if (cell.RowSpan == 1)
{
_rows[cell.Row].Update(measure.Height);
}
}
}

Expand Down Expand Up @@ -749,7 +777,13 @@ double AvailableHeight(Cell cell)

public void DecompressStars(Size targetSize)
{
if (_grid.VerticalLayoutAlignment == LayoutAlignment.Fill || Dimension.IsExplicitSet(_explicitGridHeight))
bool decompressVertical = Dimension.IsExplicitSet(_explicitGridHeight)
|| _grid.VerticalLayoutAlignment == LayoutAlignment.Fill && targetSize.Height > MeasuredGridHeight();

bool decompressHorizontal = Dimension.IsExplicitSet(_explicitGridWidth)
|| _grid.HorizontalLayoutAlignment == LayoutAlignment.Fill && targetSize.Width > MeasuredGridWidth();

if (decompressVertical)
{
// Reset the size on all star rows
ZeroOutStarSizes(_rows);
Expand All @@ -758,7 +792,7 @@ public void DecompressStars(Size targetSize)
ResolveStarRows(targetSize.Height, true);
}

if (_grid.HorizontalLayoutAlignment == LayoutAlignment.Fill || Dimension.IsExplicitSet(_explicitGridWidth))
if (decompressHorizontal)
{
// Reset the size on all star columns
ZeroOutStarSizes(_columns);
Expand Down
138 changes: 129 additions & 9 deletions src/Core/tests/UnitTests/Layouts/GridLayoutManagerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1936,7 +1936,7 @@ public void StarColumnWidthCorrectWhenFirstChildCollapsed()

[Fact("ArrangeChildren should arranged within measured size")]
[Category(GridStarSizing)]
public void ArrangeChildrenShouldArrangedWithinMeasuredSize()
public void ArrangeChildrenShouldArrangeWithinMeasuredSize()
{
var grid = CreateGridLayout(rows: "*");
grid.Width.Returns(105);
Expand All @@ -1953,7 +1953,7 @@ public void ArrangeChildrenShouldArrangedWithinMeasuredSize()
AssertArranged(view0, 0, 0, measuredSize.Width, measuredSize.Height);
}

[Theory]
[Theory, Category(GridStarSizing)]
[InlineData(LayoutAlignment.Center, true)]
[InlineData(LayoutAlignment.Center, false)]
[InlineData(LayoutAlignment.Start, true)]
Expand All @@ -1980,7 +1980,7 @@ public void GridMeasuresStarColumnToChildWidth(LayoutAlignment alignment, bool i
Assert.Equal(20, measuredSize.Width);
}

[Theory]
[Theory, Category(GridStarSizing)]
[InlineData(true, 100)]
[InlineData(false, 100)]
[InlineData(true, 15)]
Expand All @@ -2002,7 +2002,7 @@ public void FillGridArrangesStarColumnToWidthConstraint(bool implied, double con
AssertArranged(view0, new Rect(0, 0, constraint, 100));
}

[Theory]
[Theory, Category(GridStarSizing)]
[InlineData(LayoutAlignment.Center, true)]
[InlineData(LayoutAlignment.Center, false)]
[InlineData(LayoutAlignment.Start, true)]
Expand All @@ -2029,7 +2029,7 @@ public void NonFillGridArrangesStarColumnToChildWidth(LayoutAlignment alignment,
AssertArranged(view0, new Rect(0, 0, 20, 100));
}

[Theory]
[Theory, Category(GridStarSizing)]
[InlineData(LayoutAlignment.Center, true)]
[InlineData(LayoutAlignment.Center, false)]
[InlineData(LayoutAlignment.Start, true)]
Expand All @@ -2056,7 +2056,7 @@ public void GridMeasuresStarRowToChildHeight(LayoutAlignment alignment, bool imp
Assert.Equal(20, measuredSize.Height);
}

[Theory]
[Theory, Category(GridStarSizing)]
[InlineData(true, 100)]
[InlineData(false, 100)]
[InlineData(true, 15)]
Expand All @@ -2078,7 +2078,7 @@ public void FillGridArrangesStarRowToHeightConstraint(bool implied, double const
AssertArranged(view0, new Rect(0, 0, 100, constraint));
}

[Theory]
[Theory, Category(GridStarSizing)]
[InlineData(LayoutAlignment.Center, true)]
[InlineData(LayoutAlignment.Center, false)]
[InlineData(LayoutAlignment.Start, true)]
Expand All @@ -2104,7 +2104,7 @@ public void NonFillGridArrangesStarRowToChildHeight(LayoutAlignment alignment, b
AssertArranged(view0, new Rect(0, 0, 100, 20));
}

[Fact]
[Fact, Category(GridStarSizing)]
public void StarRowsResizeWhenGridExpandsToFill()
{
var grid = CreateGridLayout(rows: "*");
Expand All @@ -2130,7 +2130,7 @@ public void StarRowsResizeWhenGridExpandsToFill()
AssertArranged(view0, new Rect(0, 0, 20, 100));
}

[Fact]
[Fact, Category(GridStarSizing)]
public void StarColumnsResizeWhenGridExpandsToFill()
{
var grid = CreateGridLayout(columns: "*");
Expand Down Expand Up @@ -2427,5 +2427,125 @@ public void AutoStarRowsRespectUnconstrainedWidth()
// be arranged at the top left corner
AssertArranged(view0, new Rect(0, 0, 20, 20));
}

[Fact, Category(GridStarSizing)]
public void UnconstrainedStarRowsRetainTheirHeightsWhenArrangedAtMeasuredSize()
{
// This test accounts for the situation where a Grid has Rows marked as "*", is measured
// without height constraint, is vertically set to Fill, and is arranged at the measured height.
// Basically, a situation where the Grid is inside a vertically-oriented ScrollView or StackLayout,
// and the concept of vertical "Fill" doesn't mean anything. In that situation, the rows should
// retain their automatic sizing, rather than being evenly distributed as usual.

var grid = CreateGridLayout(rows: "*, *, *");
grid.VerticalLayoutAlignment.Returns(LayoutAlignment.Fill);

var view0 = CreateTestView(new Size(20, 20));
var view1 = CreateTestView(new Size(20, 40));
var view2 = CreateTestView(new Size(20, 60));

SubstituteChildren(grid, view0, view1, view2);

SetLocation(grid, view0, row: 0);
SetLocation(grid, view1, row: 1);
SetLocation(grid, view2, row: 2);

// Measure the Grid with no height constraint, then arrange it using the resulting size
// Unconstrained, we expect the views to total 20 + 40 + 60 = 120 height
// Since we're arranging it at that same height, there's reason for it to expand the items
// so we expect them to be arranged at the same heights
var measure = MeasureAndArrange(grid, widthConstraint: 200, heightConstraint: double.PositiveInfinity);

Assert.Equal(120, measure.Height);

AssertArranged(view0, new Rect(0, 0, 20, 20));
AssertArranged(view1, new Rect(0, 20, 20, 40));
AssertArranged(view2, new Rect(0, 60, 20, 60));
}

[Fact, Category(GridStarSizing)]
public void UnconstrainedStarColumnsRetainTheirWidthsWhenArrangedAtMeasuredSize()
{
// This test accounts for the situation where a Grid has Columns marked as "*", is measured
// without width constraint, is horizontally set to Fill, and is arranged at the measured width.
// Basically, a situation where the Grid is inside a horizontally-oriented ScrollView or StackLayout,
// and the concept of horizontal "Fill" doesn't mean anything. In that situation, the columns should
// retain their automatic sizing, rather than being evenly distributed as usual.

var grid = CreateGridLayout(columns: "*, *, *");
grid.HorizontalLayoutAlignment.Returns(LayoutAlignment.Fill);

var view0 = CreateTestView(new Size(20, 20));
var view1 = CreateTestView(new Size(40, 20));
var view2 = CreateTestView(new Size(60, 20));

SubstituteChildren(grid, view0, view1, view2);

SetLocation(grid, view0, col: 0);
SetLocation(grid, view1, col: 1);
SetLocation(grid, view2, col: 2);

// Measure the Grid with no width constraint, then arrange it using the resulting size
// Unconstrained, we expect the views to total 20 + 40 + 60 = 120 width
// Since we're arranging it at that same width, there's reason for it to expand the items
// so we expect them to be arranged at the same widths
var measure = MeasureAndArrange(grid, widthConstraint: double.PositiveInfinity, heightConstraint: 200);

Assert.Equal(120, measure.Width);

AssertArranged(view0, new Rect(0, 0, 20, 20));
AssertArranged(view1, new Rect(20, 0, 40, 20));
AssertArranged(view2, new Rect(60, 0, 60, 20));
}

// These next two tests validate cases where the Grid structure necessitates multiple
// measure passes (because a Star value intersects with multiple Auto values)
// and the items being measured may have a different Auto height/width on the second pass.

[Theory, Category(GridAutoSizing)]
[InlineData(10, 30)] // Replicating the situation from https://github.com/dotnet/maui/issues/14296
[InlineData(40, 30)] // Simulating something like an Image where the height shrinks as the width constraint gets tighter
public void AutoRowIsDominatedByTallestView(double unconstrainedHeight, double constrainedHeight)
{
var grid = CreateGridLayout(rows: "Auto", columns: "Auto, *");

var view0 = CreateTestView(new Size(20, 20));

// The view changes size depending on how wide its measurement constraints are
var view1 = CreateWidthDominatedView(100, unconstrainedHeight, new Tuple<double, double>(200, constrainedHeight));

SubstituteChildren(grid, view0, view1);

SetLocation(grid, view0, row: 0, col: 0);
SetLocation(grid, view1, row: 0, col: 1);

var measure = MeasureAndArrange(grid, widthConstraint: 200, heightConstraint: 200);

// We expect the Grid to grow to accommodate the full height of view1 at this width
Assert.Equal(constrainedHeight, measure.Height);
}

[Theory, Category(GridAutoSizing)]
[InlineData(10, 30)] // Replicating https://github.com/dotnet/maui/issues/14296 but for columns
[InlineData(50, 30)] // Simulating something like an Image where the width shrinks as the height constraint gets tighter
public void AutoColumnIsDominatedByWidestView(double unconstrainedWidth, double constrainedWidth)
{
var grid = CreateGridLayout(columns: "Auto", rows: "Auto, *");

var view0 = CreateTestView(new Size(20, 20));

// The view changes size depending on how high its measurement constraints are
var view1 = CreateHeightDominatedView(unconstrainedWidth, 100, new Tuple<double, double>(constrainedWidth, 100));

SubstituteChildren(grid, view0, view1);

SetLocation(grid, view0, row: 0, col: 0);
SetLocation(grid, view1, row: 1, col: 0);

var measure = MeasureAndArrange(grid, widthConstraint: 100, heightConstraint: 100);

// We expect the Grid to group to accommodate the full width of view1 at this height
Assert.Equal(constrainedWidth, measure.Width);
}
}
}
Loading