[UniformGridLayout] Fix bug where controls would not know their limit…

…ations when being evaluated for layouting (#5359) * Add test code, fix bug * Refactor * Refactor code * CR feedback, refactor * CR feedback * Add non working test * Attempt to fix the test. * Fix build break * Add attribute * Make type public * Move namespace * Change namespace of type. Co-authored-by: Stephen Peters <[email protected]>
microsoft · Aug 6, 2021 · a4dcfc9 · a4dcfc9
1 parent 8b7e40a
commit a4dcfc9
Show file tree

Hide file tree

Showing 8 changed files with 186 additions and 17 deletions.
diff --git a/dev/Repeater/APITests/Common/AspectRatioRespectingControl.cs b/dev/Repeater/APITests/Common/AspectRatioRespectingControl.cs
@@ -0,0 +1,37 @@
+using System;
+using System.Collections.Generic;
+using System.Text;
+using Windows.UI.Xaml.Data;
+using Windows.Foundation;
+using Windows.UI.Xaml.Controls;
+
+namespace MUXControls.ApiTests.RepeaterTests.Common
+{
+    [Windows.UI.Xaml.Data.Bindable]
+    public class AspectRatioRespectingControl : ContentPresenter
+    {
+        protected override Size MeasureOverride(Size availableSize)
+        {
+            return TwoByOneifySize(availableSize);
+        }
+
+        /// <inheritdoc/>
+        protected override Size ArrangeOverride(Size finalSize)
+        {
+            return TwoByOneifySize(finalSize);
+        }
+
+        private Size TwoByOneifySize(Size initialSize)
+        {
+            if (Double.IsNaN(initialSize.Height) || Double.IsInfinity(initialSize.Height))
+            {
+                return new Size(initialSize.Width/ 2, initialSize.Width);
+            }
+            if (Double.IsNaN(initialSize.Width) || Double.IsInfinity(initialSize.Width))
+            {
+                return new Size(initialSize.Height / 2, initialSize.Height);
+            }
+            return new Size(Math.Min(100,initialSize.Height / 2), Math.Min(100,initialSize.Height));
+        }
+    }
+}
diff --git a/dev/Repeater/APITests/FlowLayoutTests.cs b/dev/Repeater/APITests/FlowLayoutTests.cs
@@ -1245,8 +1245,7 @@ public void ValidateUniformGridWithNoItems()
         {
             RunOnUIThread.Execute(() =>
             {
-                var repeater = new ItemsRepeater()
-                {
+                var repeater = new ItemsRepeater() {
                     ItemsSource = new List<string>(), // no items 
                     Layout = new UniformGridLayout() { Orientation = Orientation.Vertical },
                 };
@@ -1260,6 +1259,43 @@ public void ValidateUniformGridWithNoItems()
             });
         }
 
+        [TestMethod]
+        public void ValidateUniformGridWithItemsRelyingOnAspectRatio()
+        {
+            ItemsRepeater repeater = null;
+            ScrollViewer scroller = null;
+            RunOnUIThread.Execute(() =>
+            {
+                repeater = new ItemsRepeater() {
+                    ItemsSource = Enumerable.Range(0, 6), // no items 
+                    Layout = new UniformGridLayout() {
+                        Orientation = Orientation.Vertical,
+                        MaximumRowsOrColumns = 1,
+                        MinItemWidth = 100
+                    },
+                    MaxWidth = 300,
+                    ItemTemplate = GetDataTemplate("<repeaterCommon:AspectRatioRespectingControl/>")
+                };
+
+                scroller = new ScrollViewer() {
+                    Content = repeater
+                };
+
+                Content = scroller;
+                repeater.UpdateLayout();
+                Content.UpdateLayout();
+            });
+
+            IdleSynchronizer.Wait();
+
+            RunOnUIThread.Execute(() =>
+            {
+                // Assert not implemented yet
+                Verify.IsTrue(Math.Abs(300 - repeater.DesiredSize.Width) < 1, "Repeater width should be 300");
+                Verify.IsTrue(repeater.DesiredSize.Height > 1, "Repeater should have height");
+            });
+        }
+
         [TestMethod]
         public void ValidateFlowLayoutWithOneItemHasNonZeroExtent()
         {
@@ -1545,7 +1581,8 @@ private DataTemplate GetDataTemplate(string content)
         {
             return (DataTemplate)XamlReader.Load(
                        string.Format(@"<DataTemplate  
-                            xmlns='http://schemas.microsoft.com/winfx/2006/xaml/presentation'>
+                            xmlns='http://schemas.microsoft.com/winfx/2006/xaml/presentation'
+                            xmlns:repeaterCommon='using:MUXControls.ApiTests.RepeaterTests.Common'>
                            {0}
                         </DataTemplate>", content));
         }

diff --git a/dev/Repeater/APITests/Repeater_APITests.projitems b/dev/Repeater/APITests/Repeater_APITests.projitems
@@ -11,6 +11,7 @@
   </PropertyGroup>
   <ItemGroup>
     <Compile Include="$(MSBuildThisFileDirectory)AccessibilityTests.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)Common\AspectRatioRespectingControl.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Common\CollectionChangeEventArgsConverters.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Common\CustomItemsSource.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Common\CustomItemsSourceView.cs" />

diff --git a/dev/Repeater/TestUI/Samples/ItemsSourceSamples/ElementsInItemsSourcePage.xaml b/dev/Repeater/TestUI/Samples/ItemsSourceSamples/ElementsInItemsSourcePage.xaml
@@ -50,7 +50,7 @@
                             <Button>27</Button>
                             <Button>28</Button>
                             <Button>29</Button>
-                            <Button>30</Button>                            
+                            <Button>30</Button>
                         </local:UICollection>
                     </controls:ItemsRepeater.ItemsSource>
                 </controls:ItemsRepeater>

diff --git a/dev/Repeater/TestUI/Samples/UniformGridLayoutDemo.cs b/dev/Repeater/TestUI/Samples/UniformGridLayoutDemo.cs
@@ -17,6 +17,7 @@ public UniformGridLayoutDemo()
         {
             collection = Enumerable.Range(0, 40);
             this.InitializeComponent();
+            imageAspectRatioTestRepeater.ItemsSource = Enumerable.Range(0, 30);
         }
 
         public void GetRepeaterActualHeightButtonClick(object sender, RoutedEventArgs e)

diff --git a/dev/Repeater/TestUI/Samples/UniformGridLayoutDemo.xaml b/dev/Repeater/TestUI/Samples/UniformGridLayoutDemo.xaml
@@ -154,5 +154,28 @@
                 Layout="{StaticResource UniformGridLayout}"
                 ItemTemplate="{StaticResource SimpleElementTemplate}"/>
         </ScrollViewer>
+
+        <ScrollViewer VerticalScrollMode="Enabled">
+            <Grid>
+                <!--  This background image demonstrates that the height of one row in the UniformGridLayout is equal to the height of the image when it is laid out across the width of the page  -->
+                <Image
+                    VerticalAlignment="Top"
+                    Opacity="0.2"
+                    Source="/Images/recipe1.png" />
+                <controls:ItemsRepeater HorizontalAlignment="Left" VerticalAlignment="Stretch"
+                    x:Name="imageAspectRatioTestRepeater">
+                    <controls:ItemsRepeater.Layout>
+                        <controls:UniformGridLayout MaximumRowsOrColumns="4"
+                            Orientation="Horizontal"
+                            MinItemWidth="56" />
+                    </controls:ItemsRepeater.Layout>
+                    <controls:ItemsRepeater.ItemTemplate>
+                        <DataTemplate>
+                            <Image VerticalAlignment="Top" Source="/Images/recipe1.png" />
+                        </DataTemplate>
+                    </controls:ItemsRepeater.ItemTemplate>
+                </controls:ItemsRepeater>
+            </Grid>
+        </ScrollViewer>
     </StackPanel>
 </local:TestPage>
diff --git a/dev/Repeater/UniformGridLayoutState.cpp b/dev/Repeater/UniformGridLayoutState.cpp
@@ -27,7 +27,7 @@ void UniformGridLayoutState::EnsureElementSize(
     const winrt::Size availableSize,
     const winrt::VirtualizingLayoutContext& context,
     const double layoutItemWidth,
-    const double LayoutItemHeight,
+    const double layoutItemHeight,
     const winrt::UniformGridLayoutItemsStretch& stretch,
     const winrt::Orientation& orientation,
     double minRowSpacing,
@@ -44,26 +44,86 @@ void UniformGridLayoutState::EnsureElementSize(
         // If the first element is realized we don't need to get it from the context
         if (auto realizedElement = m_flowAlgorithm.GetElementIfRealized(0))
         {
-            realizedElement.Measure(availableSize);
-            SetSize(realizedElement.DesiredSize(), layoutItemWidth, LayoutItemHeight, availableSize, stretch, orientation, minRowSpacing, minColumnSpacing, maxItemsPerLine);
+            realizedElement.Measure(CalculateAvailableSize(availableSize, orientation, stretch, maxItemsPerLine, layoutItemWidth, layoutItemHeight, minRowSpacing, minColumnSpacing));
+            SetSize(realizedElement.DesiredSize(), layoutItemWidth, layoutItemHeight, availableSize, stretch, orientation, minRowSpacing, minColumnSpacing, maxItemsPerLine);
         }
         else
         {
             // Not realized by flowlayout, so do this now!
             if (const auto firstElement = context.GetOrCreateElementAt(0, winrt::ElementRealizationOptions::ForceCreate))
             {
-                firstElement.Measure(availableSize);
-                SetSize(firstElement.DesiredSize(), layoutItemWidth, LayoutItemHeight, availableSize, stretch, orientation, minRowSpacing, minColumnSpacing, maxItemsPerLine);
+                firstElement.Measure(CalculateAvailableSize(availableSize,orientation, stretch, maxItemsPerLine, layoutItemWidth, layoutItemHeight, minRowSpacing, minColumnSpacing));
+                SetSize(firstElement.DesiredSize(), layoutItemWidth, layoutItemHeight, availableSize, stretch, orientation, minRowSpacing, minColumnSpacing, maxItemsPerLine);
                 context.RecycleElement(firstElement);
             }
         }
     }
 }
 
+winrt::Size UniformGridLayoutState::CalculateAvailableSize(const winrt::Size availableSize,
+    const winrt::Orientation orientation,
+    const winrt::UniformGridLayoutItemsStretch& stretch,
+    const unsigned int maxItemsPerLine,
+    const double itemWidth,
+    const double itemHeight,
+    double minRowSpacing,
+    double minColumnSpacing)
+{
+    // Since some controls might have certain requirements when rendering (e.g. maintaining an aspect ratio),
+    // we will let elements know the actual size they will get within our layout and let them measure based on that assumption.
+    // That way we ensure that no gaps will be created within our layout because of a control deciding it doesn't need as much height (or width)
+    // for the column width (or row height) being provided.
+    if (orientation == winrt::Orientation::Horizontal)
+    {
+        if (!isnan(itemWidth))
+        {
+            double allowedColumnWidth = itemWidth;
+            if (stretch != winrt::UniformGridLayoutItemsStretch::None)
+            {
+                allowedColumnWidth += CalculateExtraPixelsInLine(maxItemsPerLine, availableSize.Width, itemWidth, minColumnSpacing);
+            }
+            return winrt::Size{ (float)allowedColumnWidth, availableSize.Height};
+        }
+    }
+    else {
+        if (!isnan(itemHeight))
+        {
+            double allowedRowHeight = itemHeight;
+            if (stretch != winrt::UniformGridLayoutItemsStretch::None)
+            {
+                allowedRowHeight += CalculateExtraPixelsInLine(maxItemsPerLine, availableSize.Height, itemHeight, minRowSpacing);
+            }
+            return winrt::Size{availableSize.Width, (float)itemHeight};
+        }
+    }
+    return availableSize;
+}
+
+double UniformGridLayoutState::CalculateExtraPixelsInLine(unsigned int maxItemsPerLine,
+    const float availableSizeMinor,
+    const double itemSizeMinor,
+    const double minorItemSpacing)
+{
+    const auto numItemsPerColumn = [](unsigned int maxItemsPerLine, const float availableSizeMinor, const double itemSizeMinor, const double minorItemSpacing){
+        const unsigned int numItemsBasedOnSize = static_cast<unsigned int>(std::max(1.0, availableSizeMinor / (itemSizeMinor + minorItemSpacing)));
+        if (numItemsBasedOnSize == 0) {
+            return maxItemsPerLine;
+        }
+        else {
+            return std::min(
+                maxItemsPerLine,
+                numItemsBasedOnSize);
+        }
+    }(maxItemsPerLine,availableSizeMinor,itemSizeMinor,minorItemSpacing);
+    const auto usedSpace = (numItemsPerColumn * (itemSizeMinor + minorItemSpacing)) - minorItemSpacing;
+    const auto remainingSpace = ((int)(availableSizeMinor - usedSpace));
+    return remainingSpace / ((int)numItemsPerColumn);
+}
+
 void UniformGridLayoutState::SetSize(
     const winrt::Size& desiredItemSize,
     const double layoutItemWidth,
-    const double LayoutItemHeight,
+    const double layoutItemHeight,
     const winrt::Size availableSize,
     const winrt::UniformGridLayoutItemsStretch& stretch,
     const winrt::Orientation& orientation,
@@ -77,7 +137,7 @@ void UniformGridLayoutState::SetSize(
     }
 
     m_effectiveItemWidth = (std::isnan(layoutItemWidth) ? desiredItemSize.Width : layoutItemWidth);
-    m_effectiveItemHeight = (std::isnan(LayoutItemHeight) ? desiredItemSize.Height : LayoutItemHeight);
+    m_effectiveItemHeight = (std::isnan(layoutItemHeight) ? desiredItemSize.Height : layoutItemHeight);
 
     const auto availableSizeMinor = orientation == winrt::Orientation::Horizontal ? availableSize.Width : availableSize.Height;
     const auto minorItemSpacing = orientation == winrt::Orientation::Vertical ? minRowSpacing : minColumnSpacing;
@@ -87,12 +147,7 @@ void UniformGridLayoutState::SetSize(
     double extraMinorPixelsForEachItem = 0.0;
     if (std::isfinite(availableSizeMinor))
     {
-        const auto numItemsPerColumn = std::min(
-            maxItemsPerLine,
-            static_cast<unsigned int>(std::max(1.0, availableSizeMinor / (itemSizeMinor + minorItemSpacing))));
-        const auto usedSpace = (numItemsPerColumn * (itemSizeMinor + minorItemSpacing)) - minorItemSpacing;
-        const auto remainingSpace = ((int)(availableSizeMinor - usedSpace));
-        extraMinorPixelsForEachItem = remainingSpace / ((int)numItemsPerColumn);
+        extraMinorPixelsForEachItem = CalculateExtraPixelsInLine(maxItemsPerLine, availableSizeMinor, itemSizeMinor, minorItemSpacing);
     }
 
     if (stretch == winrt::UniformGridLayoutItemsStretch::Fill)

diff --git a/dev/Repeater/UniformGridLayoutState.h b/dev/Repeater/UniformGridLayoutState.h
@@ -37,6 +37,21 @@ class UniformGridLayoutState :
     double m_effectiveItemWidth{ 0.0 };
     double m_effectiveItemHeight{ 0.0 };
 
+    winrt::Size CalculateAvailableSize(const winrt::Size availableSize,
+        const winrt::Orientation orientation,
+        const winrt::UniformGridLayoutItemsStretch& stretch,
+        const unsigned int maxItemsPerLine,
+        const double itemWidth,
+        const double itemHeight,
+        double minRowSpacing,
+        double minColumnSpacing);
+
+    double CalculateExtraPixelsInLine(unsigned int maxItemsPerLine,
+        const float availableSizeMinor,
+        const double itemSizeMinor,
+        const double minorItemSpacing);
+
+
     void SetSize(const winrt::Size& desiredItemSize,
         const double itemWidth,
         const double itemHeight,