Skip to content
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

Merged
merged 119 commits into from
May 15, 2020

Conversation

skendrot
Copy link
Contributor

@skendrot skendrot commented Mar 4, 2020

Fixes #3114 #3107

PR Type

What kind of change does this PR introduce?

  • Feature
  • Sample app changes

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:

  • Tested code with current supported SDKs
  • Pull Request has been submitted to the documentation repository instructions. Link:
  • Sample in sample app has been added / updated (for bug fixes / features)
  • Tests for the changes have been added (for bug fixes / features) (if applicable)
  • Header has been added to all new source files (run build/UpdateHeaders.bat)
  • Contains NO breaking changes

Other information

Sample app changes and documentation still coming. Want to get this in here for people to start the review process.

michael-hawker and others added 30 commits January 31, 2020 14:15
…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
… In that case clear out the cache of columns
…e have or, if we have all of the items use the total height
@skendrot
Copy link
Contributor Author

skendrot commented May 8, 2020

@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

@michael-hawker
Copy link
Member

@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.

@danwalmsley
Copy link

@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?

@danwalmsley
Copy link

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.

@michael-hawker
Copy link
Member

@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?

@Kyaa-dost
Copy link
Contributor

This one just needs the last revisit and we should be good 🚀

@ghost ghost removed the needs attention 👋 label May 13, 2020
@michael-hawker
Copy link
Member

michael-hawker commented May 14, 2020

@skendrot did we want to update to WinUI 2.4 now that it's been released?

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.

@danwalmsley
Copy link

@skendrot @michael-hawker nice fixes! can I ask what is the difference between the WrapLayout and
https://docs.microsoft.com/en-us/uwp/api/microsoft.ui.xaml.controls.flowlayout?view=winui-2.4

@skendrot
Copy link
Contributor Author

No idea... It's brand new in 2.4 which I haven't had a chance to play with yet.

@skendrot
Copy link
Contributor Author

After referencing 2.4 I'm not able to get the FlowLayout to appear. And it's not the in XAML Controls Gallery

image

image

@michael-hawker
Copy link
Member

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.

Copy link
Member

@michael-hawker michael-hawker left a 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.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
controls 🎛️ feature 💡 WinUI 💠 Related to WinUI 3 Version or when paired with External can mean requires fix in WinUI 2/3.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature] StaggeredLayout based on ItemsRepeater
8 participants