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.

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

Ensure that Grid is treating star rows/columns as Auto when unconstrained #13999

Merged
merged 2 commits into from
Mar 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
53 changes: 39 additions & 14 deletions src/Core/src/Layouts/GridLayoutManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -373,14 +373,17 @@ void KnownMeasurePass()
{
var cell = _cells[n];

bool treatCellHeightAsAuto = TreatCellHeightAsAuto(cell);
bool treatCellWidthAsAuto = TreatCellWidthAsAuto(cell);

if (double.IsNaN(cell.MeasureHeight) || double.IsNaN(cell.MeasureWidth))
{
// We still have some unknown measure constraints (* rows/columns that need to have
// the Auto measurements settled before we can measure them). So mark this cell for the
// second pass, once we know the constraints.
cell.NeedsSecondPass = true;

if (!cell.IsColumnSpanAuto && !cell.IsRowSpanAuto)
if (!(treatCellHeightAsAuto || treatCellWidthAsAuto))
{
// If neither span of this cell includes _any_ Auto values, then there's no reason
// to measure it at all during this pass; we can skip it for now
Expand All @@ -393,7 +396,7 @@ void KnownMeasurePass()

var measure = MeasureCell(cell, measureWidth, measureHeight);

if (cell.IsColumnSpanAuto)
if (treatCellWidthAsAuto)
{
if (cell.ColumnSpan == 1)
{
Expand All @@ -405,7 +408,7 @@ void KnownMeasurePass()
}
}

if (cell.IsRowSpanAuto)
if (treatCellHeightAsAuto)
{
if (cell.RowSpan == 1)
{
Expand Down Expand Up @@ -971,15 +974,10 @@ void DetermineCellMeasureWidth(Cell cell)
{
UpdateKnownMeasureWidth(cell);
}
else if (cell.IsColumnSpanAuto)
else if (TreatCellWidthAsAuto(cell))
{
cell.MeasureWidth = double.PositiveInfinity;
}
else if (cell.IsColumnSpanStar && double.IsInfinity(_gridWidthConstraint))
{
// Because the Grid isn't horizontally constrained, we treat * columns as Auto
cell.MeasureWidth = double.PositiveInfinity;
}

// For all other situations, we'll have to wait until we've measured the Auto columns
}
Expand All @@ -990,17 +988,44 @@ void DetermineCellMeasureHeight(Cell cell)
{
UpdateKnownMeasureHeight(cell);
}
else if (cell.IsRowSpanAuto)
else if (TreatCellHeightAsAuto(cell))
{
cell.MeasureHeight = double.PositiveInfinity;
}
else if (cell.IsRowSpanStar && double.IsInfinity(_gridHeightConstraint))

// For all other situations, we'll have to wait until we've measured the Auto rows
}

bool TreatCellWidthAsAuto(Cell cell)
{
if (cell.IsColumnSpanAuto)
{
// Because the Grid isn't vertically constrained, we treat * rows as Auto
cell.MeasureHeight = double.PositiveInfinity;
return true;
}

// For all other situations, we'll have to wait until we've measured the Auto rows
if (double.IsInfinity(_gridWidthConstraint) && cell.IsColumnSpanStar)
{
// Because the Grid isn't horizontally constrained, we treat * columns as Auto
return true;
}

return false;
Comment on lines +1006 to +1012
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we just return? no need for the if

Suggested change
if (double.IsInfinity(_gridWidthConstraint) && cell.IsColumnSpanStar)
{
// Because the Grid isn't horizontally constrained, we treat * columns as Auto
return true;
}
return false;
// Because the Grid isn't horizontally constrained, we treat * columns as Auto
return double.IsInfinity(_gridWidthConstraint) && cell.IsColumnSpanStar;

}

bool TreatCellHeightAsAuto(Cell cell)
{
if (cell.IsRowSpanAuto)
{
return true;
}

if (double.IsInfinity(_gridHeightConstraint) && cell.IsRowSpanStar)
{
// Because the Grid isn't vertically constrained, we treat * rows as Auto
return true;
}

return false;
Comment on lines +1022 to +1028
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Samer comment.

}
}

Expand Down
49 changes: 49 additions & 0 deletions src/Core/tests/UnitTests/Layouts/GridLayoutManagerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using Microsoft.Maui.Layouts;
using Microsoft.Maui.Primitives;
using NSubstitute;
using NSubstitute.ReceivedExtensions;
using Xunit;
using static Microsoft.Maui.UnitTests.Layouts.LayoutTestHelpers;
using LayoutAlignment = Microsoft.Maui.Primitives.LayoutAlignment;
Expand Down Expand Up @@ -2378,5 +2379,53 @@ public void StarsExpandToFixedSizes()

AssertArranged(view0, new Rect(0, 0, 100, 120));
}

[Fact]
public void AutoStarColumnsRespectUnconstrainedHeight()
{
var grid = CreateGridLayout(columns: "Auto, *");

var view0 = CreateTestView(new Size(20, 20));
view0.HorizontalLayoutAlignment.Returns(LayoutAlignment.Start);
view0.VerticalLayoutAlignment.Returns(LayoutAlignment.Start);

SubstituteChildren(grid, view0);
SetLocation(grid, view0, col: 1);

_ = MeasureAndArrange(grid, widthConstraint: 200, heightConstraint: double.PositiveInfinity);

// The Grid only has one view; since we're measuring the Grid without height constraints,
// and the single view does not have an explicit height, then there should have been at least
// one measurement with an unconstrained height
view0.Received().Measure(Arg.Any<double>(), double.PositiveInfinity);

// The Auto column has no Views, so we expect it to have zero width; the single view should
// be arranged at the top left corner
AssertArranged(view0, new Rect(0, 0, 20, 20));
}

[Fact]
public void AutoStarRowsRespectUnconstrainedWidth()
{
var grid = CreateGridLayout(rows: "Auto, *");

var view0 = CreateTestView(new Size(20, 20));
view0.HorizontalLayoutAlignment.Returns(LayoutAlignment.Start);
view0.VerticalLayoutAlignment.Returns(LayoutAlignment.Start);

SubstituteChildren(grid, view0);
SetLocation(grid, view0, row: 1);

_ = MeasureAndArrange(grid, widthConstraint: double.PositiveInfinity, heightConstraint: 200);

// The Grid only has one view; since we're measuring the Grid without width constraints,
// and the single view does not have an explicit width, then there should have been at least
// one measurement with an unconstrained width
view0.Received().Measure(double.PositiveInfinity, Arg.Any<double>());

// The Auto row has no Views, so we expect it to have zero height; the single view should
// be arranged at the top left corner
AssertArranged(view0, new Rect(0, 0, 20, 20));
}
}
}