-
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
Add new WrapLayout and StaggeredLayout controls #3160
Conversation
…tead use a new StaggeredItem to track the size of the element
… be part of a layout. The ItemsRepeater will handle that
…clear the items because we will need to recalculate the size of the items
…ight and number of items in each column
… In that case clear out the cache of columns
…e have or, if we have all of the items use the total height
@michael-hawker I don't see any reason we need to use a focusable element in the sample. We're not trying to show keyboard navigation, just the layout controls and the virtualization |
@skendrot I understand, but having the sample be accessible via the keyboard I think would be important still. I think if we fix that and add these two samples to their own category, we'll be all set to merge this in. :) I have a repro with just an ItemsRepeater, so I'll file a bug for @ranjeshj @anawishnoff to follow-up on WinUI. |
@skendrot I had a go at using this with Avalonia UI (github.com/avaloniaui/avalonia), it all seems to work well. However it seems if an item inside the panel changes its size, it doesnt trigger the layout to change. Do you expect this to respond to item size change? |
It seems to be caused by https://github.com/windows-toolkit/WindowsCommunityToolkit/pull/3160/files#diff-d2892572b73dd1d89100c042acdd492aR184 which means that if the item does change size its position doesnt change either. |
@skendrot assuming your tracking fix was addressing my 3rd item from above? Feature Freeze is May 14th, think you can look at @danwalmsley's position update issue and update the categories? Then we can get this merged this week? |
This one just needs the last revisit and we should be good 🚀 |
Nevermind, I can update and test later, as I remember now that 2.4 breaks the sample app styling, so I need to fix that later. |
…d reset everything after it
…e item changed. We need to recalculate layout for everything after it
@skendrot @michael-hawker nice fixes! can I ask what is the difference between the WrapLayout and |
No idea... It's brand new in 2.4 which I haven't had a chance to play with yet. |
Thanks @skendrot for the updates, I'll check this out and get this merged in. @danwalmsley I'm following up with @ranjeshj from the WinUI team to figure out what FlowLayout is and how it relates. |
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.
@skendrot thanks for all your hard work on this! These are going to be great additions to the toolkit and for ItemsRepeater! 🎉🎉🎉
(FYI, there was the XAML shadow placeholder missing for release mode of the Sample App, but I'll just fix that as a quick separate commit 71459d6 with the two lines needed.)
Fixes #3114 #3107
PR Type
What kind of change does this PR introduce?
What is the current behavior?
The StaggeredPanel and WrapPanel are not virtualized. Adding the new StaggeredLayout and WrapLayout allows them to be used with the ItemsRepeater so they can be virtualized
What is the new behavior?
See above
PR Checklist
Please check if your PR fulfills the following requirements:
Other information
Sample app changes and documentation still coming. Want to get this in here for people to start the review process.