Windowscommunitytoolkit: Sometime AdaptiveGridView is not adaptive enough

[harvinders]

I have seen this behaviour with my app also. Please see the attachment below from the sample UWPToolkit app from the store.

[Odonno]

You should use DesiredWidth instead of ItemHeight so it will fit the entire container size/width.

[harvinders]

Are you suggesting to not set ItemHeight and only set DesiredWidth? I have done that too and still can reproduce the issue.
The example above is the sample app which has set both ItemHeight and DesiredWidth.

[skendrot]

Can you please give more details to the issue? One picture is not enough information

[Odonno]

@harvinders Yes, it works for me. Or maybe I did not understand your problem.

[harvinders]

@skendrot Adaptive grid is suppose to arrange the children so as to cover all the width, at least that is the reason why I am using this grid. From the above image you can see that this is not happening.

@Odonno Can you check it with say DesiredWidth of 100 and the width of the container say 960.
According to the code 10 columns are to be displayed. In the RecalculateLayout method only when the new ItemWidth is greater than old value by 1 does the layout is performed again.

                if (double.IsNaN(ItemWidth) || Math.Abs(newWidth - ItemWidth) > 1)
                {
                    ItemWidth = newWidth;
                }
                // I can possibly understand the rationale, as relayouts maybe expensive and hence should be minimised. However the overall UX seems to be compromised when more columns are there.

So when we increase the width of grid which changes the ItemWidth by say .9, no layout operation would be performed even through we have now more space available.

Also I am not sure if the value 5 used in the calculation below is the right value. I believe it is add to compensate for the cell spacing. From my observations the cell spacing does not look like to be 5. Is there anyway to know what GridView uses internally?

       protected virtual double CalculateItemWidth(double containerWidth)
        {
            ...
            return (containerWidth / columns) - 5;
        }

Lastly I have seen some jerky UI when I reduce the width of the container. I would do some more experiments and share the results.

Let me know if you are able to reproduce the results, otherwise I would share the complete code on GitHub by tomorrow.

        <controls:AdaptiveGridView Background="{ThemeResource ApplicationPageBackgroundThemeBrush}" 
                x:Name="Grid"
                DesiredWidth="100"
                ItemHeight="100"
                OneRowModeEnabled="False"
                SelectionMode="Single" >
            <controls:AdaptiveGridView.ItemTemplate>
                <DataTemplate>
                    <Border Background="LightGray" MinWidth="0">
                        <TextBlock Text="{Binding}" FontSize="20"></TextBlock>
                    </Border>
                </DataTemplate>
            </controls:AdaptiveGridView.ItemTemplate>
            <controls:AdaptiveGridView.Items>
                <x:String>1</x:String>
                <x:String>2</x:String>
                <x:String>3</x:String>
                <x:String>4</x:String>
                <x:String>5</x:String>
                <x:String>6</x:String>
                <x:String>7</x:String>
                <x:String>8</x:String>
                <x:String>9</x:String>
                <x:String>10</x:String>
                <x:String>11</x:String>
                <x:String>12</x:String>
                <x:String>13</x:String>
                <x:String>14</x:String>
                <x:String>15</x:String>
                <x:String>16</x:String>
                <x:String>17</x:String>
                <x:String>18</x:String>
                <x:String>19</x:String>
                <x:String>20</x:String>
                <x:String>21</x:String>
                <x:String>22</x:String>
                <x:String>23</x:String>
                <x:String>24</x:String>
                <x:String>25</x:String>
            </controls:AdaptiveGridView.Items>
        </controls:AdaptiveGridView>

[deltakosh]

Do you have a bug fix in mind?

[skendrot]

I'm not able to see any large gaps when using the current sample app

[harvinders]

@skendrot I believe it depends on the monitor resolution, mine was 1600 wide. Anyway, if you play with the attached XAML you should be able to reproduce it. I could not reproduce it on my surface pro. However if you try reducing the DesiredWidth to around 100 you should start seeing this.

@deltakosh I played a little by removing the condition if (double.IsNaN(ItemWidth) || Math.Abs(newWidth - ItemWidth) > 1) and subtracting 4.5 instead of 5 whilst calculating the ItemWidth and it seems to behave little better. I would have to do little more research on the cell spacing before I can provide a definite answer. Hoping that I can pull the source from the symbol server of the GridView.

There is another thing that I have noticed, lets say the container width is 960 and I reduce it though the mouse, I see event with the new width set to 959 and then another event with width 960. This may be the reason I am seeing the flicker.

Also, I was trying to put an image with some text over top of the image in the grid and wanted that the text to not be clipped. In short, I would try to understand and experiment with the control little more before I can propose a solution addressing my 3 concerns.

[harvinders]

@deltakosh @skendrot This is my current understanding.

• ItemsWrapGrid has in-built style with Margin of 0,0,4,4 which means that width calculation should not subtract 5 but instead read the style margin, maybe something like this

protected virtual double CalculateItemWidth(double containerWidth)
{
    ...
    var margin = ItemContainerStyle.Setters.Cast<Setter>().FirstOrDefault(s => s.Property == ItemsWrapGrid.MarginProperty);

    return (containerWidth / columns) - ((Thickness)margin.Value).Right - ((Thickness)margin.Value).Left;
}

• However, this still does not fix the issue, even after removing the conditional check to update the ItemWidth as below, the problem still exists.

private void RecalculateLayout(double containerWidth)
{
    if (containerWidth > 0)
    {
        ItemWidth = CalculateItemWidth(containerWidth);
    }
}

The following image shows the result after these changes. I have used the AlignmentGrid to show where the Items 10 should be drawn. As it can be seen Item has a little offset and that leaves little room for Item 11 to be drawn next to it.

It seems to me the ItemsWrapGrid panel is not laying out the Children properly in its MeasureOverride and ArrangeOverride. I would suggest that we look into creating a Panel with the correct implementation of these methods. I have written an experimental Panel to test my hypothesis that the issue seems to be in these methods. However, I don't have the experience to write a Panel which involves Groups.

The code works fine, but I don't know how to handle the Groups and also it does not work with the rearrange animation provided with the toolkit.

    public class ItemsAdaptiveWrapGrid: Panel
    {
        private double _calculatedDesiredWidth;
        private int _totalColumns;

        public ItemsAdaptiveWrapGrid()
        {
            CellSpacing = 4;
            DesiredMinimumWidth = Double.NaN;
            ItemHeight = 0;
        }


        public double CellSpacing
        {
            get => (double)GetValue(CellSpacingProperty);
            set => SetValue(CellSpacingProperty, value);
        }

        // Using a DependencyProperty as the backing store for CellSpacing.  This enables animation, styling, binding, etc...
        public static readonly DependencyProperty CellSpacingProperty =
            DependencyProperty.Register("CellSpacing", typeof(double), typeof(ItemsAdaptiveWrapGrid), new PropertyMetadata(0));

        public double DesiredMinimumWidth
        {
            get => (double)GetValue(DesiredMinimumWidthProperty);
            set => SetValue(DesiredMinimumWidthProperty, value);
        }

        // Using a DependencyProperty as the backing store for CellSpacing.  This enables animation, styling, binding, etc...
        public static readonly DependencyProperty DesiredMinimumWidthProperty =
            DependencyProperty.Register("DesiredMinimumWidth", typeof(double), typeof(ItemsAdaptiveWrapGrid), new PropertyMetadata(0));

        public double ItemHeight
        {
            get => (double)GetValue(ItemHeightProperty);
            set => SetValue(ItemHeightProperty, value);
        }

        // Using a DependencyProperty as the backing store for CellSpacing.  This enables animation, styling, binding, etc...
        public static readonly DependencyProperty ItemHeightProperty =
            DependencyProperty.Register("ItemHeight", typeof(double), typeof(ItemsAdaptiveWrapGrid), new PropertyMetadata(0));

        // Make the panel as big as the biggest element
        protected override Size MeasureOverride(Size availableSize)
        {
            Size maxSize = new Size(availableSize.Width, 0);

            double desiredWidth = double.IsNaN(DesiredMinimumWidth) ? availableSize.Width : DesiredMinimumWidth;
            double itemHeight = (0 == ItemHeight) ? availableSize.Height : ItemHeight;

            _totalColumns = CalculateColumns(availableSize.Width, desiredWidth, CellSpacing);
            _calculatedDesiredWidth = CalculateDesiredItemWidth(_totalColumns, availableSize.Width, CellSpacing);


            foreach (UIElement child in Children)
            {
                child.Measure(new Size(_calculatedDesiredWidth, itemHeight));
                ItemHeight = Math.Max(child.DesiredSize.Height + CellSpacing, ItemHeight);
            }

            maxSize.Height = Math.Ceiling((double)Children.Count / _totalColumns)* (ItemHeight + CellSpacing) - CellSpacing;
            return maxSize;
        }

        // Arrange the child elements to their final position
        protected override Size ArrangeOverride(Size finalSize)
        {
            Point anchorPoint = new Point();

            for (int i = 0; i < Children.Count; i++)
            {
                anchorPoint.X = (i % _totalColumns) * (_calculatedDesiredWidth + CellSpacing);
                anchorPoint.Y = Math.Floor((double) i / _totalColumns) * (ItemHeight + CellSpacing);
                Children[i].Arrange(new Rect(anchorPoint, new Size(_calculatedDesiredWidth, ItemHeight))); 
            }
            return finalSize; 
        }


        private static int CalculateColumns(double availableWidth, double desiredItemWidth, double cellSpacing)
        {
            var columns = (int)Math.Floor( (availableWidth + cellSpacing) / (desiredItemWidth + cellSpacing));
            return columns == 0 ? 1 : columns;
        }


        private static double CalculateDesiredItemWidth(int totalColumns, double availableWidth, double cellSpacing)
        {
            return (availableWidth - (totalColumns - 1) * cellSpacing)/totalColumns; 
        }
    }

[JohnnyWestlake]

@harvinders that's a difficult ask as ItemsWrapGrid has some complex virtualization and recycling logic contained with in it and UWP doesn't currently let us write "native" virtualizing panels, and certain ListView / GridView events rely on that specific panel or ItemsStackPanel being used, as well as grouping handling, etc.

Certainly we shouldn't be hardcoding the margin width in to the calculations (something I've mention before here). I never got around to testing this properly but sticking a -1 pixel margin on the ItemsPanel tended to work for me most of the time, possibly some issue with LayoutRounding.

[harvinders]

@JohnnyWestlake I think you have solved the issue, setting UseLayoutRounding = False on the AdaptiveGridView solves the problem. However, if I set the Padding on the AdaptiveGridView, the problem comes again. Should one take into account the Padding in MeasureOverride and ArrangeOverride?

[skendrot]

@harvinders I finally got around to testing with the sample xaml you provided and I was still not able to reproduce the issue

[skendrot]

Scratch that! I was able to reproduce by going to 10 items on a row, but I had to go very slow to get it to work. It's very hard to reproduce

[skendrot]

It's pretty difficult to reproduce "as-is". Very easy to reproduce with padding on the control. Closing with the latest PR #1135