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

UniformGridLayout (FlowLayout): Fix issue with scrolling up creating gaps in layout #3393

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion dev/Repeater/FlowLayoutAlgorithm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -364,7 +364,9 @@ void FlowLayoutAlgorithm::Generate(
{
// Does not fit, wrap to the previous row
const auto availableSizeMinor = Minor(availableSize);
MinorStart(currentBounds) = std::isfinite(availableSizeMinor) ? availableSizeMinor - Minor(desiredSize) : 0.0f;
// If the last available size is finite, start from end and subtract our desired size.
// Otherwise, look at the last extent and use that for positioning.
MinorStart(currentBounds) = std::isfinite(availableSizeMinor) ? availableSizeMinor - Minor(desiredSize) : MinorSize(LastExtent()) - Minor(desiredSize);
MajorStart(currentBounds) = lineOffset - Major(desiredSize) - static_cast<float>(lineSpacing);

if (lineNeedsReposition)
Expand Down
32 changes: 32 additions & 0 deletions dev/Repeater/InteractionTests/RepeaterTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,38 @@ public static void ClassInitialize(TestContext testContext)
TestEnvironment.Initialize(testContext);
}

[TestMethod]
public void VerifyUniformGridLayoutDoesNotCreateHoles()
{
using (var setup = new TestSetupHelper("ItemsRepeater Tests"))
{
// Open page
FindElement.ByName("UniformGridLayoutDemo").Click();

// Scroll down
var scrollviewer = FindElement.ByName("RepeaterScrollViewer");
var repeaterHeightButton = FindElement.ByName("GetRepeaterActualHeightButton");
repeaterHeightButton.Click();
// Get actual repeater height
var oldActualRepeaterHeight = double.Parse(FindElement.ByName("RepeaterActualHeightLabel").GetText());

InputHelper.RotateWheel(scrollviewer, -2000);
Wait.ForIdle();

// Scroll back up again, users don't scroll large offsets at once but rather small chunks in succession.
for(int i=0;i<5;i++)
{
InputHelper.RotateWheel(scrollviewer, 50);
Wait.ForIdle();
}

repeaterHeightButton.Click();
Verify.IsTrue(Math.Abs(oldActualRepeaterHeight -
double.Parse(FindElement.ByName("RepeaterActualHeightLabel").GetText())) < 1,
"Repeater heights did not match. This indicates that there are holes in layout as the repeater now needed more/less space.");
}
}

public void TestCleanup()
{
TestCleanupHelper.Cleanup();
Expand Down
1 change: 1 addition & 0 deletions dev/Repeater/TestUI/RepeaterTestUIPage.xaml
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@
<TextBlock Text="Other Samples" Style="{ThemeResource HeaderTextBlockStyle}"/>
<Button x:Name="defaultDemo" AutomationProperties.Name="Default Demo" >Default Demo</Button>
<Button x:Name="basicDemo" AutomationProperties.Name="Basic Demo">Basic Demo</Button>
<Button x:Name="uniformGridLayoutDemo" AutomationProperties.Name="UniformGridLayoutDemo">UniformGridLayout testing</Button>
</StackPanel>

</controls:LayoutPanel>
Expand Down
6 changes: 6 additions & 0 deletions dev/Repeater/TestUI/RepeaterTestUIPage.xaml.cs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,11 @@ public RepeaterTestUIPage()
Frame.NavigateWithoutAnimation(typeof(BasicDemo));
};

uniformGridLayoutDemo.Click += delegate
{
Frame.NavigateWithoutAnimation(typeof(UniformGridLayoutDemo));
};

itemsSourceDemo.Click += delegate
{
Frame.NavigateWithoutAnimation(typeof(ElementsInItemsSourcePage));
Expand Down Expand Up @@ -224,6 +229,7 @@ public RepeaterTestUIPage()
Orientation = orientation.IsOn ? Orientation.Horizontal : Orientation.Vertical,
});
};

}

private VirtualizingLayout GetStackLayout()
Expand Down
7 changes: 7 additions & 0 deletions dev/Repeater/TestUI/Repeater_TestUI.projitems
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,10 @@
<SubType>Designer</SubType>
<Generator>MSBuild:Compile</Generator>
</Page>
<Page Include="$(MSBuildThisFileDirectory)Samples\UniformGridLayoutDemo.xaml">
<SubType>Designer</SubType>
<Generator>MSBuild:Compile</Generator>
</Page>
</ItemGroup>
<ItemGroup>
<Compile Include="$(MSBuildThisFileDirectory)RepeaterTestUIPage.xaml.cs">
Expand Down Expand Up @@ -199,6 +203,9 @@
<Compile Include="$(MSBuildThisFileDirectory)Samples\LayoutSamples\NestedLayoutSamples\StoreMockData.cs" />
<Compile Include="$(MSBuildThisFileDirectory)Samples\AnimationSamples\DefaultElementAnimator.cs" />
<Compile Include="$(MSBuildThisFileDirectory)Samples\ItemsSourceSamples\ResetableCollection.cs" />
<Compile Include="$(MSBuildThisFileDirectory)Samples\UniformGridLayoutDemo.cs">
<DependentUpon>UniformGridLayoutDemo.xaml</DependentUpon>
</Compile>
</ItemGroup>
<ItemGroup>
<Content Include="$(MSBuildThisFileDirectory)Images\recipe1.png" />
Expand Down
31 changes: 31 additions & 0 deletions dev/Repeater/TestUI/Samples/UniformGridLayoutDemo.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License. See LICENSE in the project root for license information.

using Microsoft.UI.Xaml.Controls;
using System;
using System.Collections.Generic;
using System.Collections.ObjectModel;
using System.Linq;
using Windows.UI.Xaml;
using Windows.UI.Xaml.Controls;
using RecyclingElementFactory = Microsoft.UI.Xaml.Controls.RecyclingElementFactory;
using SelectTemplateEventArgs = Microsoft.UI.Xaml.Controls.SelectTemplateEventArgs;

namespace MUXControlsTestApp.Samples
{
public sealed partial class UniformGridLayoutDemo : Page
{
public IEnumerable<int> collection;

public UniformGridLayoutDemo()
{
collection = Enumerable.Range(0, 40);
this.InitializeComponent();
}

public void GetRepeaterActualHeightButtonClick(object sender, RoutedEventArgs e)
{
RepeaterActualHeightLabel.Text = UniformGridRepeater.ActualHeight.ToString();
}
}
}
40 changes: 40 additions & 0 deletions dev/Repeater/TestUI/Samples/UniformGridLayoutDemo.xaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
<!-- Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT License. See LICENSE in the project root for license information. -->
<Page
x:Class="MUXControlsTestApp.Samples.UniformGridLayoutDemo"
xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
xmlns:controls="using:Microsoft.UI.Xaml.Controls">

<Page.Resources>
<!-- The Layout specifications used: -->
<controls:UniformGridLayout x:Name="UniformGridLayout"
MinRowSpacing="8" MinColumnSpacing="8"
MaximumRowsOrColumns="4"/>

<DataTemplate x:Key="SimpleElementTemplate" x:DataType="x:String">
<Grid Background="{ThemeResource SystemControlForegroundBaseMediumLowBrush}"
Width="100"
Height="100">
<TextBlock Text="{Binding}"
FontSize="20"/>
</Grid>
</DataTemplate>
</Page.Resources>
<StackPanel Orientation="Horizontal">
<ScrollViewer HorizontalScrollBarVisibility="Auto"
HorizontalScrollMode="Auto"
IsVerticalScrollChainingEnabled="False"
AutomationProperties.Name="RepeaterScrollViewer"
MaxHeight="500">
<controls:ItemsRepeater x:Name="UniformGridRepeater"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

curious if this would be easier to see if you set vertical cache to 0

ItemsSource="{x:Bind collection}"
Layout="{StaticResource UniformGridLayout}"
ItemTemplate="{StaticResource SimpleElementTemplate}"/>
</ScrollViewer>
<StackPanel>
<Button AutomationProperties.Name="GetRepeaterActualHeightButton"
Click="GetRepeaterActualHeightButtonClick">Get actual Repeater height</Button>
<TextBlock x:Name="RepeaterActualHeightLabel" AutomationProperties.Name="RepeaterActualHeightLabel"/>
</StackPanel>
</StackPanel>
</Page>