Skip to content
This repository has been archived by the owner on May 1, 2024. It is now read-only.

Commit

Permalink
Apply maximum starred column/row size for all starred columns/rows
Browse files Browse the repository at this point in the history 
Fixes #12961
Fixes #12725
Fixes #12900
  • Loading branch information
@hartez
hartez committed Dec 13, 2020
1 parent 3fdffc0 commit 66f1afe
Show file tree
Hide file tree
Showing 3 changed files with 825 additions and 632 deletions.
190 changes: 177 additions & 13 deletions Xamarin.Forms.Core.UnitTests/GridTests.cs
View file
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
using NUnit.Framework;
using NUnit.Framework.Constraints;
using System;
using System.Collections.Generic;
using System.Linq;
using System.Reflection;
using NUnit.Framework;
using NUnit.Framework.Constraints;
using Xamarin.Forms.Internals;

namespace Xamarin.Forms.Core.UnitTests
Expand Down Expand Up @@ -108,6 +108,168 @@ public void StarRowsHaveEqualHeights()
Assert.That(column0Height, Is.LessThan(gridHeight));
}

[Test]
public void StarRowsDoNotOverlapWithStackLayoutOnTop()
{
SetupStarRowOverlapTest(rowAIsOnTop: false, out VisualElement rowAControl,
out VisualElement rowBControl, out Label lastLabel);

var bottomOfRowB = rowBControl.Y + rowBControl.Height;
var bottomOfLastLabelInRowB = rowBControl.Y + lastLabel.Y + lastLabel.Height;
var topOfRowA = rowAControl.Y;

Assert.That(bottomOfRowB, Is.EqualTo(bottomOfLastLabelInRowB));

Assert.That(topOfRowA, Is.EqualTo(bottomOfRowB),
"B is on top of A, so the top of A should be the bottom of B");
}

[Test]
public void StarRowsDoNotOverlapWithStackLayoutOnBottom()
{
SetupStarRowOverlapTest(rowAIsOnTop: true, out VisualElement rowAControl,
out VisualElement rowBControl, out Label lastLabel);

var topOfRowB = rowBControl.Y;
var bottomOfRowB = rowBControl.Y + rowBControl.Height;
var bottomOfLastLabelInRowB = rowBControl.Y + lastLabel.Y + lastLabel.Height;
var bottomOfRowA = rowAControl.Y + rowAControl.Height;

Assert.That(bottomOfRowB, Is.EqualTo(bottomOfLastLabelInRowB));

Assert.That(topOfRowB, Is.EqualTo(bottomOfRowA),
"A is on top of B, so the top of B should be the bottom of A");
}

[Test]
public void StarColumnsDoNotOverlapWithStackLayoutAtStart()
{
SetupStarColumnOverlapTest(colAIsAtStart: false, out VisualElement colAControl,
out VisualElement colBControl, out Label lastLabel);

var endOfColB = colBControl.X + colBControl.Width;
var endOfLastLabelInColB = colBControl.X + lastLabel.X + lastLabel.Width;
var startOfColA = colAControl.X;

Assert.That(endOfColB, Is.EqualTo(endOfLastLabelInColB));

Assert.That(startOfColA, Is.EqualTo(endOfColB),
"B is before A, so the start of A should be the end of B");
}

[Test]
public void StarColumnsDoNotOverlapWithStackLayoutAtEnd()
{
SetupStarColumnOverlapTest(colAIsAtStart: true, out VisualElement colAControl,
out VisualElement colBControl, out Label lastLabel);

var startOfColB = colBControl.X;
var endOfColB = colBControl.X + colBControl.Width;
var endOfLastLabelInColB = colBControl.X + lastLabel.X + lastLabel.Width;
var endOfColA = colAControl.X + colAControl.Width;

Assert.That(endOfColB, Is.EqualTo(endOfLastLabelInColB));

Assert.That(endOfColA, Is.EqualTo(startOfColB),
"A is before B, so the end of A should be the start of B");
}

void SetupStarRowOverlapTest(bool rowAIsOnTop, out VisualElement rowAControl,
out VisualElement rowBControl, out Label lastLabel)
{
var grid = new Grid
{
RowSpacing = 0,
RowDefinitions = new RowDefinitionCollection
{
new RowDefinition() { Height = GridLength.Star },
new RowDefinition() { Height = GridLength.Star }
}
};

var labelSize = new Size(100, 20);

Label label;
rowAControl = label = new FixedSizeLabel(labelSize) { Text = "Hello" };

StackLayout stackLayout;
rowBControl = stackLayout = new StackLayout() { Spacing = 0, IsPlatformEnabled = true };

for (int n = 0; n < 14; n++)
{
var labelInStack = new FixedSizeLabel(labelSize) { Text = "Hello" };
stackLayout.Children.Add(labelInStack);
}

lastLabel = new FixedSizeLabel(labelSize) { Text = "Hello" };
stackLayout.Children.Add(lastLabel);

grid.Children.Add(stackLayout);
grid.Children.Add(label);

if (rowAIsOnTop)
{
Grid.SetRow(rowAControl, 0);
Grid.SetRow(rowBControl, 1);
}
else
{
Grid.SetRow(rowBControl, 0);
Grid.SetRow(rowAControl, 1);
}

var sizeRequest = grid.Measure(300, double.PositiveInfinity);
grid.Layout(new Rectangle(0, 0, sizeRequest.Request.Width, sizeRequest.Request.Height));
}

void SetupStarColumnOverlapTest(bool colAIsAtStart, out VisualElement colAControl,
out VisualElement colBControl, out Label lastLabel)
{
var grid = new Grid
{
ColumnSpacing = 0,
ColumnDefinitions = new ColumnDefinitionCollection
{
new ColumnDefinition() { Width = GridLength.Star },
new ColumnDefinition() { Width = GridLength.Star }
}
};

var labelSize = new Size(20, 100);

Label label;
colAControl = label = new FixedSizeLabel(labelSize) { Text = "Hello" };

StackLayout stackLayout;
colBControl = stackLayout = new StackLayout() { Spacing = 0, Orientation = StackOrientation.Horizontal, IsPlatformEnabled = true };

for (int n = 0; n < 14; n++)
{
var labelInStack = new FixedSizeLabel(labelSize) { Text = "Hello" };
stackLayout.Children.Add(labelInStack);
}

lastLabel = new FixedSizeLabel(labelSize) { Text = "Hello" };
stackLayout.Children.Add(lastLabel);

grid.Children.Add(stackLayout);
grid.Children.Add(label);

if (colAIsAtStart)
{
Grid.SetColumn(colAControl, 0);
Grid.SetColumn(colBControl, 1);
}
else
{
Grid.SetColumn(colBControl, 0);
Grid.SetColumn(colAControl, 1);
}

var sizeRequest = grid.Measure(double.PositiveInfinity, 300);
grid.Layout(new Rectangle(0, 0, sizeRequest.Request.Width, sizeRequest.Request.Height));
}

[Test(Description = "Columns with a Star width less than one should not cause the Grid to contract below the target width; see https://github.com/xamarin/Xamarin.Forms/issues/11742")]
public void StarWidthsLessThanOneShouldNotContractGrid()
{
Expand Down Expand Up @@ -176,8 +338,8 @@ public void ColumnsLessThanOneStarShouldBeTallerThanOneStarColumns()

var grid2 = new Grid() { ColumnSpacing = 0 };

// Because the column with the label in it is narrower in this grid (0.5* vs 1*), the label will have
// grow taller to fit the text. So we expect this grid to grow vertically to accommodate it.
// Because the column with the label in it is narrower in this grid (0.5* vs 1*), the label will have
// grow taller to fit the text. So we expect this grid to grow vertically to accommodate it.
grid2.ColumnDefinitions.Add(new ColumnDefinition() { Width = new GridLength(0.5, GridUnitType.Star) });
grid2.ColumnDefinitions.Add(new ColumnDefinition() { Width = GridLength.Star });

Expand Down Expand Up @@ -224,7 +386,7 @@ public void ContentHeightSumShouldMatchGridHeightWithAutoRows()
}

[Test]
public void UnconstrainedStarRowWithMultipleStarColumnsAllowsTextToGrow()
public void UnconstrainedStarRowWithMultipleStarColumnsAllowsTextToGrow()
{
var outerGrid = new Grid() { ColumnSpacing = 0, Padding = 0, RowSpacing = 0, IsPlatformEnabled = true };
outerGrid.RowDefinitions.Add(new RowDefinition() { Height = GridLength.Star });
Expand All @@ -235,10 +397,10 @@ public void UnconstrainedStarRowWithMultipleStarColumnsAllowsTextToGrow()
var sl = new StackLayout { Padding = 0, IsPlatformEnabled = true };

var label1 = new HeightBasedOnTextLengthLabel
{
{
Text = "The actual text here doesn't matter, just the length. The length determines the height."
};

var label2 = new ColumnTestLabel { FontSize = 13, LineBreakMode = LineBreakMode.NoWrap, Text = "Description" };

sl.Children.Add(label1);
Expand Down Expand Up @@ -293,8 +455,8 @@ public void AbsoluteColumnShouldNotBloatStarredColumns(double firstColumnWidth)

outerGrid.ColumnDefinitions.Add(new ColumnDefinition() { Width = new GridLength(firstColumnWidth, GridUnitType.Star) }); // 0.3, but 0.2 works fine
outerGrid.ColumnDefinitions.Add(new ColumnDefinition() { Width = GridLength.Star });
// This last column is the trouble spot; MeasureAndContractStarredColumns needs to account for this width

// This last column is the trouble spot; MeasureAndContractStarredColumns needs to account for this width
// when measuring and distributing the starred column space. Otherwise it reports the wrong width and every
// measure after that is wrong.
outerGrid.ColumnDefinitions.Add(new ColumnDefinition() { Width = 36 });
Expand All @@ -307,7 +469,7 @@ public void AbsoluteColumnShouldNotBloatStarredColumns(double firstColumnWidth)
innerGrid.RowDefinitions.Add(new RowDefinition() { Height = GridLength.Auto });
innerGrid.RowDefinitions.Add(new RowDefinition() { Height = GridLength.Auto });

var hugeLabel = new _12292TestLabel() { };
var hugeLabel = new _12292TestLabel() { };
var tinyLabel = new ColumnTestLabel { Text = "label1" };

innerGrid.Children.Add(hugeLabel);
Expand Down Expand Up @@ -448,13 +610,15 @@ public FixedSizeLabel(Size minimumSize, Size requestedSize)
_requestedSize = requestedSize;
}

public FixedSizeLabel(Size size) : this(size, size) { }

protected override SizeRequest OnMeasure(double widthConstraint, double heightConstraint)
{
return new SizeRequest(_requestedSize, _minimumSize);
}
}

class _12292TestLabel : Label
class _12292TestLabel : Label
{
// We need a label that simulates a fairly specific layout/measure pattern to reproduce
// the circumstances of issue 12292.
Expand Down Expand Up @@ -508,8 +672,8 @@ protected override SizeRequest OnMeasure(double widthConstraint, double heightCo

class HeightBasedOnTextLengthLabel : TestLabel
{
public double DesiredHeight(double widthConstraint)
{
public double DesiredHeight(double widthConstraint)
{
return (Text.Length / widthConstraint) * 200;
}

Expand Down
16 changes: 5 additions & 11 deletions Xamarin.Forms.Core/Grid.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ public partial class Grid : Layout<View>, IGridController, IElementConfiguration
public Grid()
{
_children = new GridElementCollection(InternalChildren, this) { Parent = this };
_platformConfigurationRegistry = new Lazy<PlatformConfigurationRegistry<Grid>>(() =>
_platformConfigurationRegistry = new Lazy<PlatformConfigurationRegistry<Grid>>(() =>
new PlatformConfigurationRegistry<Grid>(this));
}

Expand Down Expand Up @@ -162,14 +162,14 @@ internal override void ComputeConstraintForView(View view)

var result = LayoutConstraint.None;

if (_rows == null || _columns == null)
EnsureRowsColumnsInitialized();
// grab a snapshot of this grid's structure for computing the constraints
var structure = new GridStructure(this);

if (vOptions.Alignment == LayoutAlignment.Fill)
{
int row = GetRow(view);
int rowSpan = GetRowSpan(view);
List<RowDefinition> rowDefinitions = _rows;
List<RowDefinition> rowDefinitions = structure.Rows;

var canFix = true;

Expand All @@ -196,7 +196,7 @@ internal override void ComputeConstraintForView(View view)
{
int col = GetColumn(view);
int colSpan = GetColumnSpan(view);
List<ColumnDefinition> columnDefinitions = _columns;
List<ColumnDefinition> columnDefinitions = structure.Columns;

var canFix = true;

Expand Down Expand Up @@ -227,12 +227,6 @@ public void InvalidateMeasureInernalNonVirtual(InvalidationTrigger trigger)
{
InvalidateMeasureInternal(trigger);
}
internal override void InvalidateMeasureInternal(InvalidationTrigger trigger)
{
base.InvalidateMeasureInternal(trigger);
_columns = null;
_rows = null;
}

void OnDefinitionChanged(object sender, EventArgs args)
{
Expand Down
Loading

0 comments on commit 66f1afe

Please sign in to comment.