From 2c9f286db066acfa44e7d3f938f5026a2e2ded09 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Wed, 26 Apr 2023 14:27:48 +0200 Subject: [PATCH 1/2] Added failing tests for #11119. --- .../CarouselTests.cs | 65 +++++++++++++++++++ .../SelectingItemsControlTests_Multiple.cs | 52 ++++++++++++++- 2 files changed, 116 insertions(+), 1 deletion(-) diff --git a/tests/Avalonia.Controls.UnitTests/CarouselTests.cs b/tests/Avalonia.Controls.UnitTests/CarouselTests.cs index 6624d13165..2a35787f3b 100644 --- a/tests/Avalonia.Controls.UnitTests/CarouselTests.cs +++ b/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) diff --git a/tests/Avalonia.Controls.UnitTests/Primitives/SelectingItemsControlTests_Multiple.cs b/tests/Avalonia.Controls.UnitTests/Primitives/SelectingItemsControlTests_Multiple.cs index 0817979e33..928c0c94ef 100644 --- a/tests/Avalonia.Controls.UnitTests/Primitives/SelectingItemsControlTests_Multiple.cs +++ b/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(target.ItemsPanelRoot); + var scroll = panel.FindAncestorOfType()!; + + // 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()), }, }; } From 8a354d8cb9251ff666d226000af1e5ae6d34cfd0 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Wed, 26 Apr 2023 16:54:23 +0200 Subject: [PATCH 2/2] Only prepare items that are containers once. When an `ItemsControl` returns true for `IsItemItsOwnContainer`, `ItemContainerPrepared` should only be called once the first time the container is prepared. Requires that `ContainerFromIndex` returns `ItemIsOwnContainer` items that have previously been prepared in order for `SelectingItemsControl` to update their selection correctly when outside the realized viewport. Fixes #11119 --- .../Generators/ItemContainerGenerator.cs | 28 ++++++++++++++++--- .../VirtualizingCarouselPanel.cs | 9 ++++-- src/Avalonia.Controls/VirtualizingPanel.cs | 5 ++++ .../VirtualizingStackPanel.cs | 14 ++++++++-- 4 files changed, 47 insertions(+), 9 deletions(-) diff --git a/src/Avalonia.Controls/Generators/ItemContainerGenerator.cs b/src/Avalonia.Controls/Generators/ItemContainerGenerator.cs index 57ed67b508..f2b105c901 100644 --- a/src/Avalonia.Controls/Generators/ItemContainerGenerator.cs +++ b/src/Avalonia.Controls/Generators/ItemContainerGenerator.cs @@ -7,8 +7,8 @@ namespace Avalonia.Controls.Generators /// /// /// When creating a container for an item from a , the following - /// method order should be followed: - /// + /// process should be followed: + /// /// - should first be called if the item is /// derived from the 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 /// /// - Finally, should be called. - /// - When the item is ready to be recycled, should - /// be called if returned false. /// + /// NOTE: If 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 for the item returned true then the item + /// cannot be unrealized or recycled. + /// - Otherwise, 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. + /// - method should be called for the + /// container. + /// - 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 diff --git a/src/Avalonia.Controls/VirtualizingCarouselPanel.cs b/src/Avalonia.Controls/VirtualizingCarouselPanel.cs index da0ff1eb69..28d6a83309 100644 --- a/src/Avalonia.Controls/VirtualizingCarouselPanel.cs +++ b/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? 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)) diff --git a/src/Avalonia.Controls/VirtualizingPanel.cs b/src/Avalonia.Controls/VirtualizingPanel.cs index a95d4f1ffa..ed92f30454 100644 --- a/src/Avalonia.Controls/VirtualizingPanel.cs +++ b/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. /// + /// + /// 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. + /// protected internal abstract Control? ContainerFromIndex(int index); /// diff --git a/src/Avalonia.Controls/VirtualizingStackPanel.cs b/src/Avalonia.Controls/VirtualizingStackPanel.cs index 77268c7831..67ec238ceb 100644 --- a/src/Avalonia.Controls/VirtualizingStackPanel.cs +++ b/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))