Merge pull request #11141 from AvaloniaUI/fixes/11119-carousel-control-items

Fix selection with unrealized container items

src/Avalonia.Controls/Generators/ItemContainerGenerator.cs

@@ -7,8 +7,8 @@ namespace Avalonia.Controls.Generators
     /// </summary>
     /// <remarks>
     /// When creating a container for an item from a <see cref="VirtualizingPanel"/>, the following
-    /// method order should be followed:
-    /// 
+    /// process should be followed:
+    ///
     /// - <see cref="IsItemItsOwnContainer(Control)"/> should first be called if the item is
     ///   derived from the <see cref="Control"/> class. If this method returns true then the
     ///   item itself should be used as the container.
@@ -19,9 +19,29 @@ namespace Avalonia.Controls.Generators
     /// - The container should then be added to the panel using 
     ///   <see cref="VirtualizingPanel.AddInternalChild(Control)"/>
     /// - Finally, <see cref="ItemContainerPrepared(Control, object?, int)"/> should be called.
-    /// - When the item is ready to be recycled, <see cref="ClearItemContainer(Control)"/> should
-    ///   be called if <see cref="IsItemItsOwnContainer(Control)"/> returned false.
     /// 
+    /// NOTE: If <see cref="IsItemItsOwnContainer(Control)"/> in the first step above returns true
+    /// then the above steps should be carried out a single time; the first time the item is 
+    /// displayed. Otherwise the steps should be carried out each time a new container is realized
+    /// for an item.
+    ///
+    /// When unrealizing a container, the following process should be followed:
+    /// 
+    /// - If <see cref="IsItemItsOwnContainer(Control)"/> for the item returned true then the item
+    ///   cannot be unrealized or recycled.
+    /// - Otherwise, <see cref="ClearItemContainer(Control)"/> should be called for the container
+    /// - If recycling is supported then the container should be added to a recycle pool.
+    /// - It is assumed that recyclable containers will not be removed from the panel but instead
+    ///   hidden from view using e.g. `container.IsVisible = false`.
+    ///
+    /// When recycling an unrealized container, the following process should be followed:
+    /// 
+    /// - An element should be taken from the recycle pool.
+    /// - The container should be made visible.
+    /// - <see cref="PrepareItemContainer(Control, object?, int)"/> method should be called for the
+    ///   container.
+    /// - <see cref="ItemContainerPrepared(Control, object?, int)"/> should be called.
+    ///
     /// NOTE: Although this class is similar to that found in WPF/UWP, in Avalonia this class only
     /// concerns itself with generating and clearing item containers; it does not maintain a
     /// record of the currently realized containers, that responsibility is delegated to the

src/Avalonia.Controls/VirtualizingCarouselPanel.cs

@@ -168,7 +168,13 @@ namespace Avalonia.Controls
 
         protected internal override Control? ContainerFromIndex(int index)
         {
-            return index == _realizedIndex ? _realized : null;
+            if (index < 0 || index >= Items.Count)
+                return null;
+            if (index == _realizedIndex)
+                return _realized;
+            if (Items[index] is Control c && c.GetValue(ItemIsOwnContainerProperty))
+                return c;
+            return null;
         }
 
         protected internal override IEnumerable<Control>? GetRealizedContainers()
@@ -264,7 +270,6 @@ namespace Avalonia.Controls
                 if (controlItem.IsSet(ItemIsOwnContainerProperty))
                 {
                     controlItem.IsVisible = true;
-                    generator.ItemContainerPrepared(controlItem, item, index);
                     return controlItem;
                 }
                 else if (generator.IsItemItsOwnContainer(controlItem))

src/Avalonia.Controls/VirtualizingPanel.cs

@@ -76,6 +76,11 @@ namespace Avalonia.Controls
         /// The container for the item at the specified index within the item collection, if the
         /// item is realized; otherwise, null.
         /// </returns>
+        /// <remarks>
+        /// Note for implementors: if the item at the the specified index is an ItemIsOwnContainer
+        /// item that has previously been realized, then the item should be returned even if it
+        /// currently falls outside the realized viewport.
+        /// </remarks>
         protected internal abstract Control? ContainerFromIndex(int index);
 
         /// <summary>

src/Avalonia.Controls/VirtualizingStackPanel.cs

@@ -3,7 +3,6 @@ using System.Collections.Generic;
 using System.Collections.Specialized;
 using System.Diagnostics;
 using System.Linq;
-using System.Reflection;
 using Avalonia.Controls.Primitives;
 using Avalonia.Controls.Utils;
 using Avalonia.Input;
@@ -326,7 +325,17 @@ namespace Avalonia.Controls
             return _realizedElements?.Elements.Where(x => x is not null)!;
         }
 
-        protected internal override Control? ContainerFromIndex(int index) => _realizedElements?.GetElement(index);
+        protected internal override Control? ContainerFromIndex(int index)
+        {
+            if (index < 0 || index >= Items.Count)
+                return null;
+            if (_realizedElements?.GetElement(index) is { } realized)
+                return realized;
+            if (Items[index] is Control c && c.GetValue(ItemIsOwnContainerProperty))
+                return c;
+            return null;
+        }
+
         protected internal override int IndexFromContainer(Control container) => _realizedElements?.GetIndex(container) ?? -1;
 
         protected internal override Control? ScrollIntoView(int index)
@@ -578,7 +587,6 @@ namespace Avalonia.Controls
                 if (controlItem.IsSet(ItemIsOwnContainerProperty))
                 {
                     controlItem.IsVisible = true;
-                    generator.ItemContainerPrepared(controlItem, item, index);
                     return controlItem;
                 }
                 else if (generator.IsItemItsOwnContainer(controlItem))

tests/Avalonia.Controls.UnitTests/CarouselTests.cs

@@ -261,6 +261,71 @@ namespace Avalonia.Controls.UnitTests
             }
         }
 
+        [Fact]
+        public void Can_Move_Forward_Back_Forward()
+        {
+            using var app = Start();
+            var items = new[] { "foo", "bar" };
+            var target = new Carousel
+            {
+                Template = CarouselTemplate(),
+                ItemsSource = items,
+            };
+
+            Prepare(target);
+
+            target.SelectedIndex = 1;
+            Layout(target);
+
+            Assert.Equal(1, target.SelectedIndex);
+
+            target.SelectedIndex = 0;
+            Layout(target);
+
+            Assert.Equal(0, target.SelectedIndex);
+
+            target.SelectedIndex = 1;
+            Layout(target);
+
+            Assert.Equal(1, target.SelectedIndex);
+        }
+
+        [Fact]
+        public void Can_Move_Forward_Back_Forward_With_Control_Items()
+        {
+            // Issue #11119
+            using var app = Start();
+            var items = new[] { new Canvas(), new Canvas() };
+            var target = new Carousel
+            {
+                Template = CarouselTemplate(),
+                ItemsSource = items,
+            };
+
+            Prepare(target);
+
+            target.SelectedIndex = 1;
+            Layout(target);
+
+            Assert.Equal(1, target.SelectedIndex);
+
+            target.SelectedIndex = 0;
+            Layout(target);
+
+            Assert.Equal(0, target.SelectedIndex);
+
+            target.SelectedIndex = 1;
+            target.PropertyChanged += (s, e) =>
+            {
+                if (e.Property == Carousel.SelectedIndexProperty)
+                {
+                }
+            };
+            Layout(target);
+
+            Assert.Equal(1, target.SelectedIndex);
+        }
+
         private static IDisposable Start() => UnitTestApplication.Start(TestServices.MockPlatformRenderInterface);
 
         private static void Prepare(Carousel target)

tests/Avalonia.Controls.UnitTests/Primitives/SelectingItemsControlTests_Multiple.cs

@@ -1024,6 +1024,56 @@ namespace Avalonia.Controls.UnitTests.Primitives
             Assert.Equal(new[] { 15 }, SelectedContainers(target));
         }
 
+        [Fact]
+        public void Can_Change_Selection_For_Containers_Outside_Of_Viewport()
+        {
+            // Issue #11119
+            using var app = Start();
+            var items = Enumerable.Range(0, 100).Select(x => new TestContainer 
+            { 
+                Content = $"Item {x}",
+                Height = 100,
+            }).ToList();
+
+            // Create a SelectingItemsControl with a virtualizing stack panel.
+            var target = CreateTarget(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 and 2 out of view.
+            scroll.Offset = new(0, 1000);
+            Layout(target);
+
+            Assert.Equal(10, panel.FirstRealizedIndex);
+            Assert.Equal(19, panel.LastRealizedIndex);
+
+            // Select item 2 now that items 1 and 2 are both unrealized.
+            target.SelectedIndex = 2;
+
+            // The selection should be updated.
+            Assert.Empty(SelectedContainers(target));
+            Assert.Equal(2, target.SelectedIndex);
+            Assert.Same(items[2], target.SelectedItem);
+            Assert.Equal(new[] { 2 }, target.Selection.SelectedIndexes);
+            Assert.Equal(new[] { items[2] }, target.Selection.SelectedItems);
+
+            // Scroll selected item back into view.
+            scroll.Offset = new(0, 0);
+            Layout(target);
+
+            // The selection should be preserved.
+            Assert.Equal(new[] { 2 }, SelectedContainers(target));
+            Assert.Equal(2, target.SelectedIndex);
+            Assert.Same(items[2], target.SelectedItem);
+            Assert.Equal(new[] { 2 }, target.Selection.SelectedIndexes);
+            Assert.Equal(new[] { items[2] }, target.Selection.SelectedItems);
+        }
+
         [Fact]
         public void Selection_State_Change_On_Unrealized_Item_Is_Respected_With_IsSelected_Binding()
         {
@@ -1197,7 +1247,7 @@ namespace Avalonia.Controls.UnitTests.Primitives
             {
                 Setters =
                 {
-                    new Setter(TreeView.TemplateProperty, CreateTestContainerTemplate()),
+                    new Setter(TestContainer.TemplateProperty, CreateTestContainerTemplate()),
                 },
             };
         }