Skip to content

Commit

Permalink
Fix missing Orientation check in VirtualizingStackPanel (#17135)
Browse files Browse the repository at this point in the history 
* Fix missing Orientation check in VirtualizingStackPanel > CalculateMeasureViewport

* Added UnitTests ScrollIntoView_Correctly_Scrolls_Right_To_A_Page_Of_[Smaller/Larger]_Items for the horizontal orientation

* Revert "Added UnitTests ScrollIntoView_Correctly_Scrolls_Right_To_A_Page_Of_[Smaller/Larger]_Items for the horizontal orientation"

This reverts commit 9aa7ac2.

* Added VirtualizingStackPanelTests horizontal unittests

* Changed SV orientation before settings Template

* fixed missing orientation on target

* Revert "fixed missing orientation on target"

This reverts commit 8f304d0.

* Fix missing orientation assignment.
  • Loading branch information
@dbriard
dbriard authored Oct 7, 2024
1 parent 770d31f commit 20db30d
Show file tree
Hide file tree
Showing 2 changed files with 123 additions and 31 deletions.
10 changes: 5 additions & 5 deletions src/Avalonia.Controls/VirtualizingStackPanel.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ protected override Size MeasureOverride(Size availableSize)
// We handle horizontal and vertical layouts here so X and Y are abstracted to:
// - Horizontal layouts: U = horizontal, V = vertical
// - Vertical layouts: U = vertical, V = horizontal
var viewport = CalculateMeasureViewport(items);
var viewport = CalculateMeasureViewport(orientation, items);

// If the viewport is disjunct then we can recycle everything.
if (viewport.viewportIsDisjunct)
Expand Down Expand Up @@ -465,15 +465,15 @@ protected internal override int IndexFromContainer(Control container)
return _realizedElements?.Elements ?? Array.Empty<Control>();
}

private MeasureViewport CalculateMeasureViewport(IReadOnlyList<object?> items)
private MeasureViewport CalculateMeasureViewport(Orientation orientation, IReadOnlyList<object?> items)
{
Debug.Assert(_realizedElements is not null);

var viewport = _viewport;

// Get the viewport in the orientation direction.
var viewportStart = Orientation == Orientation.Horizontal ? viewport.X : viewport.Y;
var viewportEnd = Orientation == Orientation.Horizontal ? viewport.Right : viewport.Bottom;
var viewportStart = orientation == Orientation.Horizontal ? viewport.X : viewport.Y;
var viewportEnd = orientation == Orientation.Horizontal ? viewport.Right : viewport.Bottom;

// Get or estimate the anchor element from which to start realization. If we are
// scrolling to an element, use that as the anchor element. Otherwise, estimate the
Expand All @@ -484,7 +484,7 @@ private MeasureViewport CalculateMeasureViewport(IReadOnlyList<object?> items)
if (_scrollToIndex >= 0 && _scrollToElement is not null)
{
anchorIndex = _scrollToIndex;
anchorU = _scrollToElement.Bounds.Top;
anchorU = orientation == Orientation.Horizontal ? _scrollToElement.Bounds.Left : _scrollToElement.Bounds.Top;
}
else
{
Expand Down
144 changes: 118 additions & 26 deletions tests/Avalonia.Controls.UnitTests/VirtualizingStackPanelTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,13 @@ public class VirtualizingStackPanelTests : ScopedTestBase
[!Layoutable.HeightProperty] = new Binding("Height"),
});

private static FuncDataTemplate<ItemWithWidth> CanvasWithWidthTemplate = new((_, _) =>
new Canvas
{
Height = 100,
[!Layoutable.WidthProperty] = new Binding("Width"),
});

[Fact]
public void Creates_Initial_Items()
{
Expand All @@ -45,7 +52,7 @@ public void Creates_Initial_Items()
public void Initializes_Initial_Control_Items()
{
using var app = App();
var items = Enumerable.Range(0, 100).Select(x => new Button { Width = 25, Height = 10});
var items = Enumerable.Range(0, 100).Select(x => new Button { Width = 25, Height = 10 });
var (target, scroll, itemsControl) = CreateTarget(items: items, itemTemplate: null);

Assert.Equal(1000, scroll.Extent.Height);
Expand Down Expand Up @@ -492,17 +499,17 @@ public void Resetting_Collection_To_Have_Less_Than_A_Page_Of_Items_When_Scrolled
public void NthChild_Selector_Works()
{
using var app = App();

var style = new Style(x => x.OfType<ContentPresenter>().NthChild(5, 0))
{
Setters = { new Setter(ListBoxItem.BackgroundProperty, Brushes.Red) },
};

var (target, _, _) = CreateTarget(styles: new[] { style });
var realized = target.GetRealizedContainers()!.Cast<ContentPresenter>().ToList();

Assert.Equal(10, realized.Count);

for (var i = 0; i < 10; ++i)
{
var container = realized[i];
Expand Down Expand Up @@ -537,7 +544,7 @@ public void NthChild_Selector_Works_For_ItemTemplate_Children()
var expectedBackground = (i == 4 || i == 9) ? Brushes.Red : null;

Assert.Equal(i, index);
Assert.Equal(expectedBackground, ((Canvas) container.Child!).Background);
Assert.Equal(expectedBackground, ((Canvas)container.Child!).Background);
}
}

Expand Down Expand Up @@ -590,7 +597,7 @@ public void NthLastChild_Selector_Works_For_ItemTemplate_Children()
var expectedBackground = (i == 0 || i == 5) ? Brushes.Red : null;

Assert.Equal(i, index);
Assert.Equal(expectedBackground, ((Canvas) container.Child!).Background);
Assert.Equal(expectedBackground, ((Canvas)container.Child!).Background);
}
}

Expand Down Expand Up @@ -762,7 +769,7 @@ public void Scrolling_Down_With_Larger_Element_Does_Not_Cause_Jump_And_Arrives_A
Layout(target);

Assert.True(
target.FirstRealizedIndex >= index,
target.FirstRealizedIndex >= index,
$"{target.FirstRealizedIndex} is not greater or equal to {index}");

if (scroll.Offset.Y + scroll.Viewport.Height == scroll.Extent.Height)
Expand Down Expand Up @@ -801,7 +808,7 @@ public void Scrolling_Up_To_Larger_Element_Does_Not_Cause_Jump()
index = target.FirstRealizedIndex;
}
}

[Fact]
public void Scrolling_Up_To_Smaller_Element_Does_Not_Cause_Jump()
{
Expand All @@ -828,12 +835,12 @@ public void Scrolling_Up_To_Smaller_Element_Does_Not_Cause_Jump()
Layout(target);

Assert.True(
target.FirstRealizedIndex <= index,
target.FirstRealizedIndex <= index,
$"{target.FirstRealizedIndex} is not less than {index}");
Assert.True(
index - target.FirstRealizedIndex <= 1,
$"FirstIndex changed from {index} to {target.FirstRealizedIndex}");

index = target.FirstRealizedIndex;
}
}
Expand All @@ -846,7 +853,7 @@ public void Does_Not_Throw_When_Estimating_Viewport_With_Ancestor_Margin()
var (_, _, itemsControl) = CreateUnrootedTarget<ItemsControl>();
var container = new Decorator { Margin = new Thickness(100) };
var root = new TestRoot(true, container);

root.LayoutManager.ExecuteInitialLayoutPass();

container.Child = itemsControl;
Expand Down Expand Up @@ -889,7 +896,7 @@ public void Supports_Null_Recycle_Key_When_Clearing_Items()

Assert.Null(firstItem.Parent);
Assert.Null(firstItem.VisualParent);
Assert.Empty(itemsControl.ItemsPanelRoot!.Children);
Assert.Empty(itemsControl.ItemsPanelRoot!.Children);
}

[Fact]
Expand Down Expand Up @@ -1038,7 +1045,7 @@ public void Inserting_Item_Before_Viewport_Preserves_FirstRealizedIndex()
public void Can_Bind_Item_IsVisible()
{
using var app = App();
var style = CreateIsVisibleBindingStyle();
var style = CreateIsVisibleBindingStyle();
var items = Enumerable.Range(0, 100).Select(x => new ItemWithIsVisible(x)).ToList();
var (target, scroll, itemsControl) = CreateTarget(items: items, styles: new[] { style });
var container = target.ContainerFromIndex(2)!;
Expand Down Expand Up @@ -1192,6 +1199,58 @@ public void ScrollIntoView_Correctly_Scrolls_Down_To_A_Page_Of_Larger_Items()
AssertRealizedItems(target, itemsControl, 15, 5);
}

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

// First 10 items have width of 20, next 10 have width of 10.
var items = Enumerable.Range(0, 20).Select(x => new ItemWithWidth(x, ((29 - x) / 10) * 10));
var (target, scroll, itemsControl) = CreateTarget(items: items, itemTemplate: CanvasWithWidthTemplate, orientation: Orientation.Horizontal);

// Scroll the last item into view.
target.ScrollIntoView(19);

// At the time of the scroll, the average item width 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 width of 10. This is obviously not a perfect answer, but
// it's the best we can do without knowing the actual item widths.
var container = Assert.IsType<ContentPresenter>(target.ContainerFromIndex(19));
Assert.Equal(new Rect(380, 0, 10, 100), container.Bounds);
Assert.Equal(new Size(100, 100), scroll.Viewport);
Assert.Equal(new Size(390, 100), scroll.Extent);
Assert.Equal(new Vector(290, 0), scroll.Offset);

// Items 10-19 should be visible.
AssertRealizedItems(target, itemsControl, 10, 10);
}

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

// First 10 items have width of 10, next 10 have width of 20.
var items = Enumerable.Range(0, 20).Select(x => new ItemWithWidth(x, ((x / 10) + 1) * 10));
var (target, scroll, itemsControl) = CreateTarget(items: items, itemTemplate: CanvasWithWidthTemplate, orientation: Orientation.Horizontal);

// Scroll the last item into view.
target.ScrollIntoView(19);

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

// Items 15-19 should be visible.
AssertRealizedItems(target, itemsControl, 15, 5);
}

[Fact]
public void Extent_And_Offset_Should_Be_Updated_When_Containers_Resize()
{
Expand Down Expand Up @@ -1348,21 +1407,24 @@ private static void AssertRealizedControlItems<TContainer>(
private static (VirtualizingStackPanel, ScrollViewer, ItemsControl) CreateTarget(
IEnumerable<object>? items = null,
Optional<IDataTemplate?> itemTemplate = default,
IEnumerable<Style>? styles = null)
IEnumerable<Style>? styles = null,
Orientation orientation = Orientation.Vertical)
{
return CreateTarget<ItemsControl>(
items: items,
itemTemplate: itemTemplate,
styles: styles);
items: items,
itemTemplate: itemTemplate,
styles: styles,
orientation: orientation);
}

private static (VirtualizingStackPanel, ScrollViewer, T) CreateTarget<T>(
IEnumerable<object>? items = null,
Optional<IDataTemplate?> itemTemplate = default,
IEnumerable<Style>? styles = null)
IEnumerable<Style>? styles = null,
Orientation orientation = Orientation.Vertical)
where T : ItemsControl, new()
{
var (target, scroll, itemsControl) = CreateUnrootedTarget<T>(items, itemTemplate);
var (target, scroll, itemsControl) = CreateUnrootedTarget<T>(items, itemTemplate, orientation);
var root = CreateRoot(itemsControl, styles: styles);

root.LayoutManager.ExecuteInitialLayoutPass();
Expand All @@ -1372,10 +1434,14 @@ private static (VirtualizingStackPanel, ScrollViewer, T) CreateTarget<T>(

private static (VirtualizingStackPanel, ScrollViewer, T) CreateUnrootedTarget<T>(
IEnumerable<object>? items = null,
Optional<IDataTemplate?> itemTemplate = default)
Optional<IDataTemplate?> itemTemplate = default,
Orientation orientation = Orientation.Vertical)
where T : ItemsControl, new()
{
var target = new VirtualizingStackPanel();
var target = new VirtualizingStackPanel
{
Orientation = orientation,
};

items ??= new ObservableCollection<string>(Enumerable.Range(0, 100).Select(x => $"Item {x}"));

Expand All @@ -1388,9 +1454,16 @@ private static (VirtualizingStackPanel, ScrollViewer, T) CreateUnrootedTarget<T>
{
Name = "PART_ScrollViewer",
Content = presenter,
Template = ScrollViewerTemplate(),
};

if (orientation == Orientation.Horizontal)
{
scroll.HorizontalScrollBarVisibility = ScrollBarVisibility.Auto;
scroll.VerticalScrollBarVisibility = ScrollBarVisibility.Disabled;
}

scroll.Template = ScrollViewerTemplate();

var itemsControl = new T
{
ItemsSource = items,
Expand All @@ -1403,7 +1476,7 @@ private static (VirtualizingStackPanel, ScrollViewer, T) CreateUnrootedTarget<T>
}

private static TestRoot CreateRoot(
Control? child,
Control? child,
Size? clientSize = null,
IEnumerable<Style>? styles = null)
{
Expand Down Expand Up @@ -1469,16 +1542,35 @@ public ItemWithHeight(int index, double height = 10)
Caption = $"Item {index}";
Height = height;
}

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

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

private class ItemWithWidth : NotifyingBase
{
private double _width;

public ItemWithWidth(int index, double width = 10)
{
Caption = $"Item {index}";
Width = width;
}

public string Caption { get; set; }

public double Width
{
get => _width;
set => SetField(ref _width, value);
}
}

private class ItemWithIsVisible : NotifyingBase
{
private bool _isVisible = true;
Expand Down

0 comments on commit 20db30d

Please sign in to comment.