Add FrameworkElement.VerticalAlignment and FrameworkElement.HorizontalAlignment support to WrapPanel

[vgromfeld]

Fixes #3466

This PR adds the support of the FrameworkElement.VerticalAlignment and FrameworkElement.HorizontalAlignment to WrapPanel.

PR Type

What kind of change does this PR introduce?

• Bugfix

What is the current behavior?

The wrap panel is always top/left aligning its children.

What is the new behavior?

The controls will now use the children alignment to properly align them inside their row/column.
The Arrange code has been updated and is now a two steps process. The position and size of all the controls are stored in a new UvRect structure. Those UvRect are grouped by Row. This allows us to get the final height of the row before drawing the children so we can adjust their position.

VerticalAlignment.Top

VerticalAlignment.Center

VerticalAlignment.Bottom

VerticalAlignment.Stretch

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)
  • Icon has been created (if new sample) following the Thumbnail Style Guide and templates
• 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

[ghost]

Thanks vgromfeld 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 🙌

[skendrot]

I don't see how this solves the original issue. It does provide a work-around but the issue would still persist that if you set the VerticalAlignment on the items it wouldn't be "respected".
I don't believe we should be adding properties to a Panel that are not normally on Panels. (*ContentAlignment properties are not found on Panels)
This problem ultimately comes down to the fact we are arranging the items based on the minimum values they take up, rather than the actual height (V) of the given row, or width of the given column, based on the Orientation. This can be fixed by modifying the ArrangeOverride method to re-arrange the items if we find the V value to be different at the end than it was at the start. This does require some extra arranging. We could remove this duplicate arranging by keeping track of the heights (V) of the rows from the measure to the arrange.

[michael-hawker]

@skendrot I think you're basically saying that a better approach to basically be instead of setting this at the panel level, perform the similar breakout that @vgromfeld does in this PR, but just inspect each child's FrameworkElement alignment properties individually at each arrange call in order to place them in their 'cell'? I think this is feasible and would result in the same desired outcome, eh?

This would make things much more flexible and also remove the confusion around having the alignment properties on the panel tied to the orientation property.

I think this further shows that we almost need some base panel helpers that perform some of these common layout operations we can use across our solutions, but that's outside of scope here.

[vgromfeld]

I've removed the two custom properties and will now use the children's alignment values.
I've also updated to PR description.

[skendrot]

@michael-hawker No, we shouldn't need to inspect the child element alignment at all. What we would need to do is ensure that when we do the layout (arrange), that we give elements all of the available space afforded by the row/column.
Example:
Item 1 has a width of 100 and height of 30.
Item 2 has a width of 120 and height of 30
Item 3 has a width of 100 and height of 40
Item 4 has a width of 150 and height of 30.
All items have a VerticalAlignment of Center. With the current approach (pre PR/Bug) all items would be given the minimum height needed (30 for items 1,2, 4 and 40 for item 3). All of these items should have a height of 40 during the layout (arrange) to ensure that they are allowed to align properly. The alignment of these items happens automatically. We don't need to check the alignment of each item, we just need to ensure that each item is allowed a height of 40 during layout

[vgromfeld]

Thanks @skendrot, I've updated the PR code to give the items the row/column height/width and let them align automatically.

<controls:WrapPanel Background="{ThemeResource Brush-Grey-04}"
                    Padding="0,0,0,0"
                    VerticalSpacing="20"
                    HorizontalSpacing="20" >
  <Border Width="100" Height="100" Background="Blue"/>
  <TextBlock Text="Hello" VerticalAlignment="Center" />
  <Border Width="20" Height="20" Background="Green" VerticalAlignment="Bottom" HorizontalAlignment="Left"/>
  <Border Width="20" Height="20" Background="Yellow" VerticalAlignment="Top" HorizontalAlignment="Right" />
  <Button Content="Click me!" VerticalAlignment="Center" HorizontalAlignment="Center" />
  <Button Content="Click me!" VerticalAlignment="Stretch" />
</controls:WrapPanel>

[ghost]

Hello @michael-hawker!

Because this pull request has the auto merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

[michael-hawker]

@Sergio0694 look good now?

[Sergio0694]

@michael-hawker Yup, all my notes have been addressed now 😄

The CI failed though 👀

[michael-hawker]

@vgromfeld StyleCop issue to fix, then you should be good:

WrapPanel\WrapPanel.Data.cs(6,1): error SA1208: Using directive for 'System' should appear before directive for 'Microsoft.Toolkit.Diagnostics' [D:\a\1\s\Microsoft.Toolkit.Uwp.UI.Controls\Microsoft.Toolkit.Uwp.UI.Controls.csproj]
         WrapPanel\WrapPanel.Data.cs(7,1): error SA1208: Using directive for 'System.Collections.Generic' should appear before directive for 'Microsoft.Toolkit.Diagnostics' [D:\a\1\s\Microsoft.Toolkit.Uwp.UI.Controls\Microsoft.Toolkit.Uwp.UI.Controls.csproj]

[Sergio0694]

LGTM 🚀

[vgromfeld]

@michael-hawker, I've fixed the file.

[michael-hawker]

@azchohfi looks like the TAEF tests passed but then reported failure? Any idea what might have happened there?

[michael-hawker]

Closed and re-opened to try and re-trigger build.

[azchohfi]

@michael-hawker no idea on what happened. VSTest crashed out of nowhere. Lets see if it passes now.

outdated

[michael-hawker]

This may have changed the default behavior for WrapPanel? We had a user expecting to see Sample App behavior:

But saw their items centered in the rows instead (vs. top aligned):

Filed issue #3574