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

Make VirtualizingStackPanel better handle container size changes #16168

Merged
merged 7 commits into from
Jul 8, 2024
Merged
Show file tree
Hide file tree
Changes from 4 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
31 changes: 30 additions & 1 deletion src/Avalonia.Controls/Utils/RealizedStackElements.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System;
using System.Collections.Generic;
using Avalonia.Layout;
using Avalonia.Utilities;

namespace Avalonia.Controls.Utils
Expand Down Expand Up @@ -538,6 +539,34 @@ public void ResetForReuse()
_elements?.Clear();
_sizes?.Clear();
}
}

/// <summary>
/// Validates that <see cref="StartU"/> is still valid.
/// </summary>
/// <param name="orientation">The panel orientation.</param>
/// <remarks>
/// If the U size of any element in the realized elements has changed, then the value of
/// <see cref="StartU"/> should be considered unstable.
/// </remarks>
public void ValidateStartU(Orientation orientation)
{
if (_elements is null || _sizes is null || _startUUnstable)
return;

for (var i = 0; i < _elements.Count; ++i)
{
if (_elements[i] is not { } element)
continue;

var sizeU = orientation == Orientation.Horizontal ?
element.Bounds.Width : element.Bounds.Height;

if (sizeU != _sizes[i])
{
_startUUnstable = true;
break;
}
}
}
}
}
15 changes: 15 additions & 0 deletions src/Avalonia.Controls/VirtualizingStackPanel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,7 @@ protected override Size MeasureOverride(Size availableSize)

try
{
_realizedElements?.ValidateStartU(Orientation);
grokys marked this conversation as resolved.
Show resolved Hide resolved
_realizedElements ??= new();
_measureElements ??= new();

Expand All @@ -179,6 +180,10 @@ protected override Size MeasureOverride(Size availableSize)
(_measureElements, _realizedElements) = (_realizedElements, _measureElements);
_measureElements.ResetForReuse();

// If there is a focused element is outside the visible viewport (i.e.
// _focusedElement is non-null), ensure it's measured.
_focusedElement?.Measure(availableSize);

return CalculateDesiredSize(orientation, items.Count, viewport);
}
finally
Expand Down Expand Up @@ -215,6 +220,16 @@ protected override Size ArrangeOverride(Size finalSize)
}
}

// Ensure that the focused element is in the correct position.
if (_focusedElement is not null && _focusedIndex >= 0)
{
u = _realizedElements.GetOrEstimateElementU(_focusedIndex, ref _lastEstimatedElementSizeU);
var rect = orientation == Orientation.Horizontal ?
new Rect(u, 0, _focusedElement.DesiredSize.Width, finalSize.Height) :
new Rect(0, u, finalSize.Width, _focusedElement.DesiredSize.Height);
_focusedElement.Arrange(rect);
}

return finalSize;
}
finally
Expand Down
92 changes: 83 additions & 9 deletions tests/Avalonia.Controls.UnitTests/VirtualizingStackPanelTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1152,15 +1152,14 @@ public void ScrollIntoView_Correctly_Scrolls_Down_To_A_Page_Of_Smaller_Items()
// Scroll the last item into view.
target.ScrollIntoView(19);

// At the time of the scroll, the average item height is 20, so the requested item
// should be placed at 380 (19 * 20) which therefore results in an extent of 390 to
// accommodate the item height of 10. This is obviously not a perfect answer, but
// it's the best we can do without knowing the actual item heights.
// After scroll, the average item height is 10, so the requested item
// should be placed at 190 (19 * 10) which results in an extent of 200 to
// accommodate the item height of 10.
var container = Assert.IsType<ContentPresenter>(target.ContainerFromIndex(19));
Assert.Equal(new Rect(0, 380, 100, 10), container.Bounds);
Assert.Equal(new Rect(0, 190, 100, 10), container.Bounds);
Assert.Equal(new Size(100, 100), scroll.Viewport);
Assert.Equal(new Size(100, 390), scroll.Extent);
Assert.Equal(new Vector(0, 290), scroll.Offset);
Assert.Equal(new Size(100, 200), scroll.Extent);
Assert.Equal(new Vector(0, 100), scroll.Offset);

// Items 10-19 should be visible.
AssertRealizedItems(target, itemsControl, 10, 10);
Expand Down Expand Up @@ -1192,6 +1191,74 @@ public void ScrollIntoView_Correctly_Scrolls_Down_To_A_Page_Of_Larger_Items()
AssertRealizedItems(target, itemsControl, 15, 5);
}

[Fact]
public void Extent_And_Offset_Should_Be_Updated_When_Containers_Resize()
{
using var app = App();

// All containers start off with a height of 50 (2 containers fit in viewport).
var items = Enumerable.Range(0, 20).Select(x => new ItemWithHeight(x, 50)).ToList();
var (target, scroll, itemsControl) = CreateTarget(items: items, itemTemplate: CanvasWithHeightTemplate);

// Scroll to the 5th item (containers 4 and 5 should be visible).
target.ScrollIntoView(5);
Assert.Equal(4, target.FirstRealizedIndex);
Assert.Equal(5, target.LastRealizedIndex);

// The extent should be 500 (10 * 50) and the offset should be 200 (4 * 50).
var container = Assert.IsType<ContentPresenter>(target.ContainerFromIndex(5));
Assert.Equal(new Rect(0, 250, 100, 50), container.Bounds);
Assert.Equal(new Size(100, 100), scroll.Viewport);
Assert.Equal(new Size(100, 1000), scroll.Extent);
Assert.Equal(new Vector(0, 200), scroll.Offset);

// Update the height of all items to 25 and run a layout pass.
foreach (var item in items)
item.Height = 25;
target.UpdateLayout();

// The extent should be updated to reflect the new heights. The offset should be
// unchanged but the first realized index should be updated to 8 (200 / 25).
Assert.Equal(new Size(100, 100), scroll.Viewport);
Assert.Equal(new Size(100, 500), scroll.Extent);
Assert.Equal(new Vector(0, 200), scroll.Offset);
Assert.Equal(8, target.FirstRealizedIndex);
Assert.Equal(11, target.LastRealizedIndex);
}

[Fact]
public void Focused_Container_Is_Positioned_Correctly_when_Container_Size_Change_Causes_It_To_Be_Moved_Out_Of_Visible_Viewport()
{
using var app = App();

// All containers start off with a height of 50 (2 containers fit in viewport).
var items = Enumerable.Range(0, 20).Select(x => new ItemWithHeight(x, 50)).ToList();
var (target, scroll, itemsControl) = CreateTarget(items: items, itemTemplate: CanvasWithHeightTemplate);

// Scroll to the 5th item (containers 4 and 5 should be visible).
target.ScrollIntoView(5);
Assert.Equal(4, target.FirstRealizedIndex);
Assert.Equal(5, target.LastRealizedIndex);

// Focus the 5th item.
var container = Assert.IsType<ContentPresenter>(target.ContainerFromIndex(5));
container.Focusable = true;
container.Focus();

// Update the height of all items to 25 and run a layout pass.
foreach (var item in items)
item.Height = 25;
target.UpdateLayout();

// The focused container should now be outside the realized range.
Assert.Equal(8, target.FirstRealizedIndex);
Assert.Equal(11, target.LastRealizedIndex);

// The container should still exist and be positioned outside the visible viewport.
container = Assert.IsType<ContentPresenter>(target.ContainerFromIndex(5));
Assert.Equal(new Rect(0, 125, 100, 25), container.Bounds);
}

private static IReadOnlyList<int> GetRealizedIndexes(VirtualizingStackPanel target, ItemsControl itemsControl)
{
return target.GetRealizedElements()
Expand Down Expand Up @@ -1354,16 +1421,23 @@ private static IControlTemplate ScrollViewerTemplate()

private static IDisposable App() => UnitTestApplication.Start(TestServices.RealFocus);

private class ItemWithHeight
private class ItemWithHeight : NotifyingBase
{
private double _height;

public ItemWithHeight(int index, double height = 10)
{
Caption = $"Item {index}";
Height = height;
}

public string Caption { get; set; }
public double Height { get; set; }

public double Height
{
get => _height;
set => SetField(ref _height, value);
}
}

private class ItemWithIsVisible : NotifyingBase
Expand Down
Loading