-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
feat: Enable Grid to dynamically switch layouts #4474
base: main
Are you sure you want to change the base?
feat: Enable Grid to dynamically switch layouts #4474
Conversation
Thanks nandin-borjigin for opening a Pull Request! The reviewers will test the PR and highlight if there is any conflict or changes required. If the PR is approved we will proceed to merge the pull request 🙌 |
Microsoft.Toolkit.Uwp.SampleApp/SamplePages/GridExtensions/GridExtensionsPage.xaml
Show resolved
Hide resolved
Microsoft.Toolkit.Uwp.SampleApp/SamplePages/GridExtensions/GridExtensionsPage.xaml
Outdated
Show resolved
Hide resolved
Microsoft.Toolkit.Uwp.SampleApp/SamplePages/GridExtensions/GridExtensionsPage.xaml
Outdated
Show resolved
Hide resolved
Microsoft.Toolkit.Uwp.UI/Extensions/Grid/GridLayoutDefinition.cs
Outdated
Show resolved
Hide resolved
Microsoft.Toolkit.Uwp.SampleApp/SamplePages/GridExtensions/GridExtensionsPage.xaml
Outdated
Show resolved
Hide resolved
Microsoft.Toolkit.Uwp.UI/Extensions/Grid/GridLayoutDefinition.cs
Outdated
Show resolved
Hide resolved
Microsoft.Toolkit.Uwp.UI/Extensions/Grid/GridLayoutDefinition.cs
Outdated
Show resolved
Hide resolved
Microsoft.Toolkit.Uwp.UI/Extensions/Grid/GridLayoutDefinition.cs
Outdated
Show resolved
Hide resolved
Microsoft.Toolkit.Uwp.SampleApp/SamplePages/GridExtensions/GridExtensionsCode.bind
Outdated
Show resolved
Hide resolved
Co-authored-by: XAML-Knight <[email protected]>
This PR has been marked as "needs attention 👋" and awaiting a response from the team. |
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.
Nice implementation - like the simplicity in defining a grid layout
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.
Looks pretty good. I might add some UI tests and potential test cases, though.
/// This usage is not really invalid (it does not throw), | ||
/// but this API is not expected to be used like this. | ||
/// </example> | ||
public string AreaDefinition |
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.
Do we maybe want to rename this AreaDefinitions
(plural) to align with the others, RowDefinitions
and ColumnDefinitions
?
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.
There is only one area definition per layout definition, whereas row definitions and column definitions are multiple.
Fixes #4470 CommunityToolkit/Labs-Windows#232
Added attached properties to
Grid
element to enable dynamic layout switching.PR Type
What kind of change does this PR introduce?
Feature
What is the current behavior?
We cannot switch
Grid
layouts dynamically once it's defined.What is the new behavior?
One can define as many layouts (dictionary of string to
GridLayoutDefinition
, a newly added data structure to describe a grid layout) as desired in aGrid
element and switch among them using the newly addedGridExtensions.ActiveLayout
(string, it's the dictionary key of the corresponding layout definition) attached property associated with theGrid
element.PR Checklist
Please check if your PR fulfills the following requirements: