Skip to content

Commit

Permalink
fix(itemsRepeater): Fix an issue where IR might re-position first ite…
Browse files Browse the repository at this point in the history
…ms if items below are significantly higher
  • Loading branch information
dr1rrb committed Apr 29, 2024
1 parent a22090b commit 602ea10
Show file tree
Hide file tree
Showing 3 changed files with 85 additions and 17 deletions.
4 changes: 0 additions & 4 deletions src/SamplesApp/UITests.Shared/ItemExclusions.props
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,6 @@
<Compile Remove="$(MSBuildThisFileDirectory)Microsoft_UI_Xaml_Controls\PagerControlTests\**" />
<None Include="$(MSBuildThisFileDirectory)Microsoft_UI_Xaml_Controls\PagerControlTests\**" />

<Page Remove="$(MSBuildThisFileDirectory)Windows_UI_Xaml_Controls\Repeater\**" />
<Compile Remove="$(MSBuildThisFileDirectory)Windows_UI_Xaml_Controls\Repeater\**" />
<None Include="$(MSBuildThisFileDirectory)Windows_UI_Xaml_Controls\Repeater\**" />

<Page Remove="$(MSBuildThisFileDirectory)Lottie\**" />
<Compile Remove="$(MSBuildThisFileDirectory)Lottie\**" />
<None Include="$(MSBuildThisFileDirectory)Lottie\**" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -430,13 +430,82 @@ public async Task When_ClearItemsWhileUnloaded_Then_MaterializeItems()
sut.Materialized.Should().Be(0);
}

private record SUT(Border Root, ScrollViewer Scroller, ItemsRepeater Repeater, ObservableCollection<string> Source)

[TestMethod]
[RunsOnUIThread]
#if __MACOS__
[Ignore("Currently fails on macOS, part of #9282 epic")]
#endif
public async Task When_ItemSignificantlyHigher_Then_VirtualizeProperly()
{
public static SUT Create(int itemsCount = 3, Size? viewport = default)
var sut = SUT.Create(
new ObservableCollection<MyItem>
{
new (0, 200, Colors.FromARGB("#FF0000")),
new (1, 400, Colors.FromARGB("#FF8000")),
new (2, 200, Colors.FromARGB("#FFFF00")),
new (3, 5000, Colors.FromARGB("#008000")),
new (4, 100, Colors.FromARGB("#0000FF")),
new (5, 100, Colors.FromARGB("#A000C0"))
},
new DataTemplate(() => new Border
{
Width = 100,
Margin = new Thickness(10),
Child = new TextBlock().Apply(tb => tb.SetBinding(TextBlock.TextProperty, new Binding()))
}
.Apply(b => b.SetBinding(FrameworkElement.HeightProperty, new Binding{Path = nameof(MyItem.Height)}))
.Apply(b => b.SetBinding(FrameworkElement.BackgroundProperty, new Binding { Path = nameof(MyItem.Color) }))),
new Size(100, 500)
);

await sut.Load();
sut.Scroller.ViewChanged += (s, e) => Console.WriteLine($"Vertical: {sut.Scroller.VerticalOffset}");

var originalEstimatedExtent = sut.Scroller.ExtentHeight;

sut.Scroller.ChangeView(null, 800, null, disableAnimation: true); // First scroll enough to get item #3 to be materialized
await TestServices.WindowHelper.WaitForIdle();

sut.MaterializedItems.Should().Contain(sut.Source[3]); // Confirm that item has been materialized!
sut.Scroller.ExtentHeight.Should().BeGreaterThan(originalEstimatedExtent); // Confirm that the extent has increased due to item #3

var item3OriginalVerticalOffset = sut.Repeater.Children.First(elt => ReferenceEquals(elt.DataContext, sut.Source[3])).ActualOffset.Y;

sut.Scroller.ChangeView(null, 1500, null, disableAnimation: true); // Then scroll enough for first items to be DE-materialized
await TestServices.WindowHelper.WaitForIdle();

sut.MaterializedItems.Should().NotContain(sut.Source[0]); // Confirm that first items has been removed!
sut.MaterializedItems.Should().NotContain(sut.Source[1]);

var item3UpdatedVerticalOffset = sut.Repeater.Children.First(elt => ReferenceEquals(elt.DataContext, sut.Source[3])).ActualOffset.Y;

item3UpdatedVerticalOffset.Should().Be(item3OriginalVerticalOffset); // Confirm that item #3 has not been moved down
var result = await UITestHelper.ScreenShot(sut.Repeater);
ImageAssert.HasColorAt(result, 10, 10, Colors.FromARGB("#008000")); // For safety also check it's effectively the item 3 that is visible
}

private record MyItem(int Id, double Height, Color Color)
{
public string Title => $"Item {Id}";
}

#nullable enable
private static class SUT
{
public static SUT<T> Create<T>(ObservableCollection<T> source, DataTemplate? itemTemplate = null, Size? viewport = default)
{
itemTemplate ??= new DataTemplate(() => new Border
{
Width = 100,
Height = 100,
Background = new SolidColorBrush(Colors.DeepSkyBlue),
Margin = new Thickness(10),
Child = new TextBlock().Apply(tb => tb.SetBinding(TextBlock.TextProperty, new Binding()))
});

var repeater = default(ItemsRepeater);
var scroller = default(ScrollViewer);
var source = new ObservableCollection<string>(Enumerable.Range(0, itemsCount).Select(i => $"Item #{i}"));
var root = new Border
{
BorderThickness = new Thickness(5),
Expand All @@ -447,14 +516,7 @@ public static SUT Create(int itemsCount = 3, Size? viewport = default)
{
ItemsSource = source,
Layout = new StackLayout(),
ItemTemplate = new DataTemplate(() => new Border
{
Width = 100,
Height = 100,
Background = new SolidColorBrush(Colors.DeepSkyBlue),
Margin = new Thickness(10),
Child = new TextBlock().Apply(tb => tb.SetBinding(TextBlock.TextProperty, new Binding()))
})
ItemTemplate = itemTemplate
})
})
};
Expand All @@ -468,9 +530,15 @@ public static SUT Create(int itemsCount = 3, Size? viewport = default)
return new(root, scroller, repeater, source);
}

public static SUT<string> Create(int itemsCount = 3, Size? viewport = default)
=> Create(new ObservableCollection<string>(Enumerable.Range(0, itemsCount).Select(i => $"Item #{i}")), viewport: viewport);
}

private record SUT<T>(Border Root, ScrollViewer Scroller, ItemsRepeater Repeater, ObservableCollection<T> Source)
{
public int Materialized => Repeater.Children.Count(elt => elt.ActualOffset.X >= 0);

public IEnumerable<string> MaterializedItems => Repeater.Children.Where(elt => elt.ActualOffset.X >= 0).Select(elt => elt.DataContext?.ToString());
public IEnumerable<T> MaterializedItems => Repeater.Children.Where(elt => elt.ActualOffset.X >= 0).Select(elt => (T)elt.DataContext);

public async ValueTask Load()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,11 @@ Rect GetExtent(
if (firstRealized != null)
{
MUX_ASSERT(lastRealized != null);
SetMajorStart(ref extent, (float)(MajorStart(firstRealizedLayoutBounds) - firstRealizedItemIndex * averageElementSize));
var firstRealizedMajor = (float)(MajorStart(firstRealizedLayoutBounds) - firstRealizedItemIndex * averageElementSize);
// Uno workaround [BEGIN]: Make sure to not move items above the viewport. This can be the case if an items is significantly higher than previous items (will increase the average items size)
firstRealizedMajor = Math.Max(0.0f, firstRealizedMajor);
// Uno workaround [END]
SetMajorStart(ref extent, firstRealizedMajor);
var remainingItems = itemsCount - lastRealizedItemIndex - 1;
SetMajorSize(ref extent, MajorEnd(lastRealizedLayoutBounds) - MajorStart(extent) + (float)(remainingItems * averageElementSize));
}
Expand Down

0 comments on commit 602ea10

Please sign in to comment.