[Windows] Fix CollectionView broken test #15584
Conversation
jsuarezruiz
added
t/bug
Eilon
added
the
area-controls-collectionview
| var h = layout?.ItemSpacing ?? 0; | ||
| var v = layout?.ItemSpacing ?? 0; | ||
| var margin = WinUIHelpers.CreateThickness(h, v, h, v); | ||
| style.Setters.Add(new WSetter(FrameworkElement.MarginProperty, margin)); |
There was a problem hiding this comment.
Why do we only set Margin on Vertical ? what happens if i give ItemsSpacing in a Horizontal layout ?
There was a problem hiding this comment.
Need to do more tests to know exactly why, but could have sense to use in both cases. The code comes from XF https://github.com/xamarin/Xamarin.Forms/blob/4f3acdfb0bc98e3ba8e2f3eae86e88602350ee84/Xamarin.Forms.Platform.UAP/CollectionView/StructuredItemsViewRenderer.cs#L241
There was a problem hiding this comment.
It looks like the test is broken because in this PR when the methods were moved from the handler to the extension method class, the MinHeight and MinWidth property settings weren't moved along with everything else. The code that was removed just needs to be added back in, the rest of this is unnecessary.
| @@ -31,13 +38,18 @@ public static class CollectionViewExtensions | |||
| if (layout is null) | |||
| return null; | |||
|
|
|||
| var style = new WStyle(typeof(GridViewItem)); | |||
|
|
|||
| if (layout?.Orientation == ItemsLayoutOrientation.Vertical) | |||
There was a problem hiding this comment.
What's the default value here for MinHeight? why setting it fixes the issue?
|
Please also verify against #15241 |
samhouts
added
backport/suggested
| var h = layout?.ItemSpacing ?? 0; | ||
| var v = layout?.ItemSpacing ?? 0; | ||
| var margin = WinUIHelpers.CreateThickness(h, v, h, v); | ||
| style.Setters.Add(new WSetter(FrameworkElement.MarginProperty, margin)); |
There was a problem hiding this comment.
It looks like the test is broken because in this PR when the methods were moved from the handler to the extension method class, the MinHeight and MinWidth property settings weren't moved along with everything else. The code that was removed just needs to be added back in, the rest of this is unnecessary.
|
Superceded by #17261 |
Description of Change
Fix CollectionView broken test on Windows.
Also added a sample in the Gallery.
Checked the sample from 15492 too.
Issues Fixed
Fixes #15529