WrapPanel2 Component Creation #764
Conversation
|
Demo Video StretchPanel-Demo1.mp4 |
There was a problem hiding this comment.
Thanks @Avid29, great start! I just dropping a couple of quick comments.
There's a few option combinations/scenarios that I may be a bit confused on still with how stretching works, maybe it has to do when both children in a row have a specific fixed size, as it's weird to see them not always be that fixed size. Though I know the video is a little out-of-date too, so not sure if that's a factor.
Not sure if it needs to be part of the doc or some other aspect, but I think it'll be good for us to enumerate a bunch of various settings and scenarios and how we expect their behavior to react to layout and see how that matches up with our expectations on various settings and such. Then we can encode those results in the docs/examples/tests.
(This can be an ongoing process too, as we gather feedback.)
|
Fixed a bug with Equal children stretching. Before: After: |
…retching items when ItemsStretch is none.
|
Here's the new behavior for when It will override the |
|
Cool, |
Hmm. Maybe worth leaving a remark on both properties, but definitely should be a remark for |
michael-hawker
left a comment
michael-hawker
left a comment
There was a problem hiding this comment.
Some quick initial comments, but need to play a bit more with it, just had a few minutes to start digging into it.
|
|
||
| When stretched along the main axis, the child elements with star-sized GridLength values will proportionally occupy the available space. | ||
|
|
||
| When not stretched along the main axis, star-sized child elements will be the smallest size possible while maintaining proportional sizing relative to each other and ensuring that all child elements are fully visible. |
There was a problem hiding this comment.
Could be good to clarify what the default behavior is here and how it compares to the standard WrapPanel.
Also not clear to me what "stretched along the main axis" means, is that setting a specific property to a specific value? If so, we should call that out more explicitly.
| public WrapPanel2BasicSample() | ||
| { | ||
| this.InitializeComponent(); | ||
| } |
There was a problem hiding this comment.
We should start with some items in the collection by default, I was very confused about the properties not doing anything at first, not realizing the buttons were control buttons and not part of the collection.
We actually need to just make these action buttons consistent with a little docked status bar or something in the sample infrastructure, it's been on the backlog for a while, maybe I'll take a quick look into: CommunityToolkit/Tooling-Windows-Submodule#4
There was a problem hiding this comment.
| } | |
| Add5ItemsClick(null!, null!); | |
| } |
Should we just call the method for now in the constructor to have something?
| /// <summary> | ||
| /// Gets or sets the method used to fill rows without a star-sized item when <see cref="ItemsJustification"/> is in a spacing mode. | ||
| /// </summary> | ||
| public WrapPanelItemsStretch ItemsStretch |
There was a problem hiding this comment.
Hmm, I wasn't seeing these do much unless the justification was set to one of the spaced properties. May need to think about the behaviors of some of the combinations here. I need to play with this a bit more.
There was a problem hiding this comment.
Yeah, played with this again, unless I had HorizontalAlignment set to Stretch the ItemsStretch wasn't doing a lot. I wonder if we need to distinguish these scenarios a big more. Could be good to get some input from a broader set of folks. So, let's ensure we get this merged and move forward with some discussions on expected behaviors/settings combinations (there's a lot now with this!)
michael-hawker
left a comment
michael-hawker
left a comment
There was a problem hiding this comment.
Looks like I can't push to this branch (well maybe, but not how I checked it out), these were the two spots I had to fix for the third demo to work.
Fix issue with third demo, just some translational typos.
|
Been working on CommunityToolkit/Tooling-Windows-Submodule#4 to clean-up the interface a bit, I was confused at first (we should have a few items in the collection by default for the basic sample). Going to open PR with initial support for that, but we can do that independently of this PR, it's a small update later. |
michael-hawker
left a comment
michael-hawker
left a comment
There was a problem hiding this comment.
Couple small things. I think the main thing to ensure we fix is just the project name for the package, should we align it to the control name? (Or would we expect other panels to put in here, then should we make it plural now?)
Once we fix that, we can merge. Looks good! Thanks! 🦙❤️
There was a problem hiding this comment.
Should this be CommunityToolkit.WinUI.Controls.WrapPanel2.csproj for now, as it's the only control in this package?
| public WrapPanel2BasicSample() | ||
| { | ||
| this.InitializeComponent(); | ||
| } |
There was a problem hiding this comment.
| } | |
| Add5ItemsClick(null!, null!); | |
| } |
Should we just call the method for now in the constructor to have something?
| /// <summary> | ||
| /// Gets or sets the method used to fill rows without a star-sized item when <see cref="ItemsJustification"/> is in a spacing mode. | ||
| /// </summary> | ||
| public WrapPanelItemsStretch ItemsStretch |
There was a problem hiding this comment.
Yeah, played with this again, unless I had HorizontalAlignment set to Stretch the ItemsStretch wasn't doing a lot. I wonder if we need to distinguish these scenarios a big more. Could be good to get some input from a broader set of folks. So, let's ensure we get this merged and move forward with some discussions on expected behaviors/settings combinations (there's a lot now with this!)
| # WrapPanel2 | ||
|
|
||
| The WrapPanel2 is an advanced layout control that uses `GridLength` definitions to manage item sizing within a wrapping flow. It provides granular control over how items occupy space, particularly when using proportional (Star) sizing. | ||
|
|
There was a problem hiding this comment.
Could be good to add more here about how this extends/differs from the original WrapPanel.
(We should decide if one of our goals is drop-in upgrade or not, but that can come later.)
michael-hawker
left a comment
michael-hawker
left a comment
There was a problem hiding this comment.
Thanks @Avid29! 🦙❤️ Let's merge this in and fix anything else forward!
3 participants
Introduces the first draft of the
WrapPanel2.Resolves: #763 as discussed in #762.