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

Investigate: Default WrapPanel behavior Changed? #3574

Closed
michael-hawker opened this issue Nov 23, 2020 · 8 comments
Closed

Investigate: Default WrapPanel behavior Changed? #3574

michael-hawker opened this issue Nov 23, 2020 · 8 comments
Assignees
Labels
bug 🐛 An unexpected issue that highlights incorrect behavior controls 🎛️ documentation 📃 good first issue Issues identified as good for first-time contributors help wanted Issues identified as good community contribution opportunities need more info 📌
Projects
Milestone

Comments

@michael-hawker
Copy link
Member

michael-hawker commented Nov 23, 2020

Originally Reported by Spyromaniac on the UWP Community Discord Server

Originally posted by @michael-hawker in #3471 (comment)

#3471 may have changed the default behavior for WrapPanel? We had a user expecting to see Sample App behavior (6.1.x):

image

But saw their items centered in the rows instead (vs. top aligned) w/ 8.0.0-preview3 (based off our 7.0.0 code):

image

We should investigate if this default behavior has changed, and if anything document the behavior update. This should only effect scenarios of different sized items and is resolved by setting the VerticalContentAlignment property.

FYI @vgromfeld

@ghost ghost added the needs triage 🔍 label Nov 23, 2020
@ghost
Copy link

ghost commented Nov 23, 2020

Hello michael-hawker, thank you for opening an issue with us!

I have automatically added a "needs triage" label to help get things started. Our team will analyze and investigate the issue, and escalate it to the relevant team if possible. Other community members may also look into the issue and provide feedback 🙌

@michael-hawker michael-hawker added bug 🐛 An unexpected issue that highlights incorrect behavior controls 🎛️ documentation 📃 good first issue Issues identified as good for first-time contributors help wanted Issues identified as good community contribution opportunities need more info 📌 and removed needs triage 🔍 labels Nov 23, 2020
@michael-hawker michael-hawker added this to To do in Bugs 7.0 via automation Nov 23, 2020
@michael-hawker michael-hawker added this to the 7.0 milestone Nov 23, 2020
@skendrot
Copy link
Contributor

@michael-hawker This seems like a stackoverflow issue. I'm guessing the content they are using has a VerticalAlignment of center

@michael-hawker
Copy link
Member Author

@skendrot could be, it was resolved for them in chat, but I just wanted to double-check and validate that we're not accidently changing the default behavior between v6 and v7. Figured it'd be a good testing intro for someone to pick-up if they're looking for something simpler to get their feet wet with the Toolkit.

@vgromfeld
Copy link
Contributor

The default value for the VerticalAlignment is Stretch (MSDN).
This is what is changing the behavior. Now that we are using the row height to arrange all the items, the default Stretch alignment is centering them if they height is smaller than the row height.
We can add a note in the documentation.

@michael-hawker
Copy link
Member Author

@vgromfeld so it's great we have more flexibility here, but also want to make sure we can preserve the ability of the old scenario as well.

Looks like by default now the container around the item fills up the entire space (which is a bit odd here):

image

Compared to the 6.1 behavior where the reveal/container surrounded the item:

image

We basically want the ability for the container to still be able to be centered within the defined bounds?


Also to the original question posed by the developer who found the issue. It appears we center with the VerticalAlignment Stretch, but even though HorizontalAlignment default is also Stretch they're left-aligned:

image

This seems counter-intuitive that they would behave differently here?


(I also noticed the style here is duplicated above here, we should only have a single copy.)

@vgromfeld
Copy link
Contributor

Also to the original question posed by the developer who found the issue. It appears we center with the VerticalAlignment Stretch, but even though HorizontalAlignment default is also Stretch they're left-aligned:

It is not something that we're doing inside the WrapPanel but more a side effect on how the XAML layout/children elements are drawing themselves.

The change done in the WrapPanel is to always give the full row height/column width to the children. Unfortunately, we do not have any HorizontalContentAlignment/VerticalContentAlignment on the control (like Control.HorizontalContentAlignment) so each child item must be explicitly configured with the required alignment.

@michael-hawker michael-hawker self-assigned this Mar 2, 2021
@michael-hawker
Copy link
Member Author

Alright, I've validated that the boxing behavior with the item border is the same as WPF:

image

So, I think we should be fine here to leave as is and just make a note about the behavior change.

I'm still not sure how we'd get mimic the old behavior with the new code, but I'm not sure it's possible in WPF either.

Sound good @vgromfeld?

@michael-hawker michael-hawker moved this from To do to Review in progress in Bugs 7.0 Mar 4, 2021
@vgromfeld
Copy link
Contributor

Yes, sounds good 👍

This was referenced Mar 12, 2021
@ghost ghost locked as resolved and limited conversation to collaborators May 4, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug 🐛 An unexpected issue that highlights incorrect behavior controls 🎛️ documentation 📃 good first issue Issues identified as good for first-time contributors help wanted Issues identified as good community contribution opportunities need more info 📌
Projects
No open projects
Bugs 7.0
  
Done
Development

No branches or pull requests

3 participants