Browse Source

Merge pull request #11168 from AvaloniaUI/fixes/selection-clearing-on-unrealized

Fix selection being clearing when recycling containers.
Max Katz 2 years ago
parent
commit
1d03f8f653

+ 11 - 4
src/Avalonia.Controls/Generators/ItemContainerGenerator.cs

@@ -85,7 +85,7 @@ namespace Avalonia.Controls.Generators
         /// <param name="index">The index of the item to display.</param>
         /// <remarks>
         /// If <see cref="IsItemItsOwnContainer(Control)"/> 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.
         /// </remarks>
@@ -100,10 +100,11 @@ namespace Avalonia.Controls.Generators
         /// <param name="item">The item being displayed.</param>
         /// <param name="index">The index of the item being displayed.</param>
         /// <remarks>
-        /// 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
-        /// <see cref="IsItemItsOwnContainer(Control)"/>.
+        /// It must be called regardless of the result of
+        /// <see cref="IsItemItsOwnContainer(Control)"/> but if that method returned true then
+        /// must be called only a single time.
         /// </remarks>
         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 <see cref="PrepareItemContainer(Control, object, int)"/> method.
         /// </summary>
         /// <param name="container">The container control.</param>
+        /// <remarks>
+        /// 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
+        /// <see cref="IsItemItsOwnContainer"/> returned true for the item.
+        /// </remarks>
         public void ClearItemContainer(Control container) => _owner.ClearItemContainer(container);
 
         [Obsolete("Use ItemsControl.ContainerFromIndex")]

+ 20 - 8
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;

+ 35 - 0
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<TestSelectorWithContainers>(
+                itemsSource: items, 
+                virtualizing: true);
+            target.AutoScrollToSelectedItem = false;
+
+            var panel = Assert.IsType<VirtualizingStackPanel>(target.ItemsPanelRoot);
+            var scroll = panel.FindAncestorOfType<ScrollViewer>()!;
+
+            // 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),
                 },
             };
         }