From 5391361d4501ddde8eebeb0525fcf82e4b616754 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Thu, 27 Apr 2023 22:39:50 +0200 Subject: [PATCH 1/3] Failing test for selection w/ virtualization. --- .../SelectingItemsControlTests_Multiple.cs | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/tests/Avalonia.Controls.UnitTests/Primitives/SelectingItemsControlTests_Multiple.cs b/tests/Avalonia.Controls.UnitTests/Primitives/SelectingItemsControlTests_Multiple.cs index 928c0c94ef..daebc1e709 100644 --- a/tests/Avalonia.Controls.UnitTests/Primitives/SelectingItemsControlTests_Multiple.cs +++ b/tests/Avalonia.Controls.UnitTests/Primitives/SelectingItemsControlTests_Multiple.cs @@ -1074,6 +1074,40 @@ namespace Avalonia.Controls.UnitTests.Primitives Assert.Equal(new[] { items[2] }, target.Selection.SelectedItems); } + [Fact] + public void Selection_Is_Not_Cleared_On_Recycling_Containers() + { + using var app = Start(); + var items = Enumerable.Range(0, 100).Select(x => new ItemViewModel($"Item {x}", false)).ToList(); + + // Create a SelectingItemsControl that creates containers that raise IsSelectedChanged, + // with a virtualizing stack panel. + var target = CreateTarget( + itemsSource: items, + virtualizing: true); + target.AutoScrollToSelectedItem = false; + + var panel = Assert.IsType(target.ItemsPanelRoot); + var scroll = panel.FindAncestorOfType()!; + + // Select item 1. + target.SelectedIndex = 1; + + // Scroll item 1 out of view. + scroll.Offset = new(0, 1000); + Layout(target); + + Assert.Equal(10, panel.FirstRealizedIndex); + Assert.Equal(19, panel.LastRealizedIndex); + + // The selection should be preserved. + Assert.Empty(SelectedContainers(target)); + Assert.Equal(1, target.SelectedIndex); + Assert.Same(items[1], target.SelectedItem); + Assert.Equal(new[] { 1 }, target.Selection.SelectedIndexes); + Assert.Equal(new[] { items[1] }, target.Selection.SelectedItems); + } + [Fact] public void Selection_State_Change_On_Unrealized_Item_Is_Respected_With_IsSelected_Binding() { @@ -1248,6 +1282,7 @@ namespace Avalonia.Controls.UnitTests.Primitives Setters = { new Setter(TestContainer.TemplateProperty, CreateTestContainerTemplate()), + new Setter(TestContainer.HeightProperty, 100.0), }, }; } From 8a5a5c8d441d10699f5b696524064d0fddae3716 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Thu, 27 Apr 2023 22:51:43 +0200 Subject: [PATCH 2/3] Remove element from list before recycling. Remove the element from the realized element list before recycling it, so that `IndexFromContainer` and `ContainerFromIndex` report the container as not found during `ClearItemContainer`. Fixes selection being reset when a container is unrealized. --- .../Utils/RealizedStackElements.cs | 28 +++++++++++++------ 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/src/Avalonia.Controls/Utils/RealizedStackElements.cs b/src/Avalonia.Controls/Utils/RealizedStackElements.cs index 8dbfb2c957..11bbaa11c4 100644 --- a/src/Avalonia.Controls/Utils/RealizedStackElements.cs +++ b/src/Avalonia.Controls/Utils/RealizedStackElements.cs @@ -353,7 +353,10 @@ namespace Avalonia.Controls.Utils for (var i = start; i < end; ++i) { if (_elements[i] is Control element) + { + _elements[i] = null; recycleElement(element); + } } _elements.RemoveRange(start, end - start); @@ -389,10 +392,13 @@ namespace Avalonia.Controls.Utils if (_elements is null || _elements.Count == 0) return; - foreach (var e in _elements) + for (var i = 0; i < _elements.Count; i++) { - if (e is not null) + if (_elements[i] is Control e) + { + _elements[i] = null; recycleElement(e); + } } _startU = _firstIndex = 0; @@ -422,7 +428,10 @@ namespace Avalonia.Controls.Utils for (var i = 0; i < endIndex; ++i) { if (_elements[i] is Control e) + { + _elements[i] = null; recycleElement(e, i + FirstIndex); + } } _elements.RemoveRange(0, endIndex); @@ -453,7 +462,10 @@ namespace Avalonia.Controls.Utils for (var i = startIndex; i < count; ++i) { if (_elements[i] is Control e) + { + _elements[i] = null; recycleElement(e, i + FirstIndex); + } } _elements.RemoveRange(startIndex, _elements.Count - startIndex); @@ -470,13 +482,13 @@ namespace Avalonia.Controls.Utils if (_elements is null || _elements.Count == 0) return; - var i = FirstIndex; - - foreach (var e in _elements) + for (var i = 0; i < _elements.Count; i++) { - if (e is not null) - recycleElement(e, i); - ++i; + if (_elements[i] is Control e) + { + _elements[i] = null; + recycleElement(e, i + FirstIndex); + } } _startU = _firstIndex = 0; From 3d693beffa5d548199b555febf3af1c6e3128bde Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Thu, 27 Apr 2023 22:57:17 +0200 Subject: [PATCH 3/3] Clarify `ClearItemContainer` docs. And change usages of "should" to "must" as it's not optional. --- .../Generators/ItemContainerGenerator.cs | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/Avalonia.Controls/Generators/ItemContainerGenerator.cs b/src/Avalonia.Controls/Generators/ItemContainerGenerator.cs index f2b105c901..b2c138599e 100644 --- a/src/Avalonia.Controls/Generators/ItemContainerGenerator.cs +++ b/src/Avalonia.Controls/Generators/ItemContainerGenerator.cs @@ -85,7 +85,7 @@ namespace Avalonia.Controls.Generators /// The index of the item to display. /// /// If is true for an item, then this method - /// only needs to be called a single time, otherwise this method should be called after the + /// must only be called a single time, otherwise this method must be called after the /// container is created, and each subsequent time the container is recycled to display a /// new item. /// @@ -100,10 +100,11 @@ namespace Avalonia.Controls.Generators /// The item being displayed. /// The index of the item being displayed. /// - /// This method should be called when a container has been fully prepared and added + /// This method must be called when a container has been fully prepared and added /// to the logical and visual trees, but may be called before a layout pass has completed. - /// It should be called regardless of the result of - /// . + /// It must be called regardless of the result of + /// but if that method returned true then + /// must be called only a single time. /// public void ItemContainerPrepared(Control container, object? item, int index) => _owner.ItemContainerPrepared(container, item, index); @@ -122,6 +123,12 @@ namespace Avalonia.Controls.Generators /// Undoes the effects of the method. /// /// The container control. + /// + /// This method must be called when a container is unrealized. The container must have + /// already have been removed from the virtualizing panel's list of realized containers before + /// this method is called. This method must not be called if + /// returned true for the item. + /// public void ClearItemContainer(Control container) => _owner.ClearItemContainer(container); [Obsolete("Use ItemsControl.ContainerFromIndex")]