-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Add Min/Max Width/Height support on Grid Column/Row Definitions #29582
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
base: main
Are you sure you want to change the base?
Conversation
…erfaces Co-authored-by: mattleibow <[email protected]>
…olumns Co-authored-by: mattleibow <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is all looking great, could you please add a comprehensive set of unit tests for these new layout features.
Co-authored-by: mattleibow <[email protected]>
I've added a comprehensive set of unit tests for the new Grid Min/Max Width/Height features in commit 2638abb. The tests include:
The tests cover all edge cases including combinations of constraints, spanning columns/rows, and interactions with other sizing features. Warning Firewall rules blocked me from connecting to one or more addressesI tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
keep going little AI!
namespace Microsoft.Maui.UnitTests.Layouts | ||
{ | ||
[Category(TestCategory.Layout)] | ||
public class GridLayoutManagerMinMaxTests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you actually refactor the existing code in src/Core/tests/UnitTests/Layouts/GridLayoutManagerTests.cs to be a partial class and this one can just add new mthods.
Name this file src/Core/tests/UnitTests/Layouts/GridLayoutManagerTests.MinMax.cs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Refactored the code to make GridLayoutManagerTests
a partial class and moved the Min/Max tests to a new file called GridLayoutManagerTests.MinMax.cs
in commit 2ef0dc9.
|
||
namespace Microsoft.Maui.Controls.Core.UnitTests | ||
{ | ||
public class GridMinMaxIntegrationTests : BaseTestFixture |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you make the src/Controls/tests/Core.UnitTests/Layouts/GridLayoutTests.cs partial and then make this be the other partial.
Also, the new file should be called GridLayoutTests.MinMax.cs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Made GridLayoutTests.cs
a partial class and created GridLayoutTests.MinMax.cs
with the grid min/max integration tests from the original GridMinMaxIntegrationTests.cs
file in commit 2ef0dc9.
|
||
namespace Microsoft.Maui.Controls.Core.UnitTests | ||
{ | ||
public class ColumnDefinitionTests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make src/Controls/tests/Core.UnitTests/Layouts/GridLayoutTests.cs partial and this file shoulkd be called GridLayoutTests.ColumnDefinition.cs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Created GridLayoutTests.ColumnDefinition.cs
as part of the partial GridLayoutTests
class with the column definition tests from the original file in commit 2ef0dc9.
|
||
namespace Microsoft.Maui.Controls.Core.UnitTests | ||
{ | ||
public class RowDefinitionTests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make src/Controls/tests/Core.UnitTests/Layouts/GridLayoutTests.cs partial and this file shoulkd be called GridLayoutTests.RowDefinition.cs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Created GridLayoutTests.RowDefinition.cs
as part of the partial GridLayoutTests
class with the row definition tests from the original file in commit 2ef0dc9.
Co-authored-by: mattleibow <[email protected]>
I've refactored all the test files according to your request:
All changes completed in commit 2ef0dc9. Warning Firewall rules blocked me from connecting to one or more addressesI tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
crazy things happen here. Little AI is freaking me out |
Wow, this is awesome, well done little AI |
@copilot will you add a UITest as well? |
@@ -10,6 +10,16 @@ public sealed class ColumnDefinition : BindableObject, IDefinition, IGridColumnD | |||
public static readonly BindableProperty WidthProperty = BindableProperty.Create(nameof(Width), typeof(GridLength), typeof(ColumnDefinition), GridLength.Star, | |||
propertyChanged: (bindable, oldValue, newValue) => ((ColumnDefinition)bindable).OnSizeChanged()); | |||
|
|||
/// <summary>Bindable property for <see cref="MinWidth"/>.</summary> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not default to zero and PositiveInfinity?
@@ -1198,6 +1198,8 @@ class Definition | |||
{ | |||
readonly GridLength _gridLength; | |||
private double _size; | |||
private readonly double _minSize; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These should default to zero and Double.PositiveInfinity. The Definition constructor should also coerce each value to zero if something tries to set them less than zero.
Then you avoid the extra ">= 0" check on each one during the Size setter.
_maxSize = -1; | ||
} | ||
|
||
public Definition(GridLength gridLength, double minSize, double maxSize) : this(gridLength) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this is an internal constructor, you might be able to just modify the original constructor to add the new parameters with defaults, and not have two constructors here.
namespace Microsoft.Maui.UnitTests.Layouts | ||
{ | ||
[Category(TestCategory.Layout)] | ||
public class GridLayoutManagerMinMaxTests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did the robot dupe all these tests? Looks like there's two versions of GridLayoutMangerMinMaxTests.
Changes
This PR implements support for Minimum and Maximum Width/Height constraints on Grid Column and Row definitions, similar to the WinUI implementation:
MinWidth
andMaxWidth
properties toColumnDefinition
MinHeight
andMaxHeight
properties toRowDefinition
IGridColumnDefinition
andIGridRowDefinition
) to include these propertiesGridLayoutManager.Definition
class to enforce min/max constraints during layout calculationUsage Example
This implementation ensures that Grid rows and columns respect minimum and maximum size constraints, providing more control over responsive layouts.
Fixes #710.
Warning
Firewall rules blocked me from connecting to one or more addresses
I tried to connect to the following addresses, but was blocked by firewall rules:
cdn.fwupd.org
/usr/bin/fwupdmgr refresh
(dns block)pkgs.dev.azure.com
dotnet build src/Controls/src/Core/Controls.Core.csproj
(dns block)/home/REDACTED/work/_temp/ghcca-node/node/bin/node /home/REDACTED/work/_temp/copilot-developer-action-main/dist/index.js
(dns block)If you need me to access, download, or install something from one of these locations, you can either:
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.