From 191a835c6a6a4119feaed8d4678d24a3c2bee728 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Thu, 6 Apr 2023 12:04:20 +0200 Subject: [PATCH] Make container IsSelected bindings work again. Rather than using the `ISelectable` interface to communicate container selection from the `SelectingItemsControl` to the container, use the `SelectingItemsControl.IsSelected` attached property, setting it with `SetCurrentValue` so that bindings defined in a style or item container theme can override the selection. Required an extra virtual `ContainerForItemPreparedOverride` method on `ItemsControl`. --- src/Avalonia.Controls/ISelectable.cs | 13 +- src/Avalonia.Controls/ItemsControl.cs | 16 + .../Primitives/SelectingItemsControl.cs | 68 ++-- .../Primitives/SelectingItemsControlTests.cs | 7 +- .../SelectingItemsControlTests_AutoSelect.cs | 1 - .../SelectingItemsControlTests_Multiple.cs | 306 ++++++++++++++++-- 6 files changed, 346 insertions(+), 65 deletions(-) diff --git a/src/Avalonia.Controls/ISelectable.cs b/src/Avalonia.Controls/ISelectable.cs index 144adaa2f5..d0da7c4932 100644 --- a/src/Avalonia.Controls/ISelectable.cs +++ b/src/Avalonia.Controls/ISelectable.cs @@ -1,16 +1,9 @@ -using Avalonia.Controls.Primitives; - namespace Avalonia.Controls { /// - /// Interface for objects that are selectable. + /// An interface that is implemented by objects that expose their selection state via a + /// boolean property. /// - /// - /// Controls such as use this interface to indicate the - /// selected control in a list. If changing the control's property - /// should update the selection in a or equivalent, then - /// the control should raise the . - /// public interface ISelectable { /// @@ -18,4 +11,4 @@ namespace Avalonia.Controls /// bool IsSelected { get; set; } } -} \ No newline at end of file +} diff --git a/src/Avalonia.Controls/ItemsControl.cs b/src/Avalonia.Controls/ItemsControl.cs index 1b2d0b1ca6..60b0f8b193 100644 --- a/src/Avalonia.Controls/ItemsControl.cs +++ b/src/Avalonia.Controls/ItemsControl.cs @@ -424,6 +424,21 @@ namespace Avalonia.Controls } } + /// + /// Called when a container has been fully prepared to display an item. + /// + /// The container control. + /// The item being displayed. + /// The index of the item being displayed. + /// + /// This method will 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 is + /// called immediately before the event is raised. + /// + protected internal virtual void ContainerForItemPreparedOverride(Control container, object? item, int index) + { + } + /// /// Called when the index for a container changes due to an insertion or removal in the /// items collection. @@ -654,6 +669,7 @@ namespace Avalonia.Controls internal void ItemContainerPrepared(Control container, object? item, int index) { + ContainerForItemPreparedOverride(container, item, index); _childIndexChanged?.Invoke(this, new ChildIndexChangedEventArgs(container, index)); ContainerPrepared?.Invoke(this, new(container, index)); } diff --git a/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs b/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs index 310626cbb0..b924a3763d 100644 --- a/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs +++ b/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs @@ -119,9 +119,8 @@ namespace Avalonia.Controls.Primitives AvaloniaProperty.Register(nameof(IsTextSearchEnabled), false); /// - /// Event that should be raised by items that implement to - /// notify the parent that their selection state - /// has changed. + /// Event that should be raised by containers when their selection state changes to notify + /// the parent that their selection state has changed. /// public static readonly RoutedEvent IsSelectedChangedEvent = RoutedEvent.Register( @@ -496,20 +495,32 @@ namespace Avalonia.Controls.Primitives } } - /// - protected internal override void PrepareContainerForItemOverride(Control element, object? item, int index) + protected internal override void ContainerForItemPreparedOverride(Control container, object? item, int index) { - base.PrepareContainerForItemOverride(element, item, index); + base.ContainerForItemPreparedOverride(container, item, index); - if ((element as ISelectable)?.IsSelected == true) + // Once the container has been full prepared and added to the tree, any bindings from + // styles or item container themes are guaranteed to be applied. + if (!container.IsSet(IsSelectedProperty)) { - Selection.Select(index); - MarkContainerSelected(element, true); + // The IsSelected property is not set on the container: update the container + // selection based on the current selection as understood by this control. + MarkContainerSelected(container, Selection.IsSelected(index)); } else { - var selected = Selection.IsSelected(index); - MarkContainerSelected(element, selected); + // The IsSelected property is set on the container: there is a style or item + // container theme which has bound the IsSelected property. Update our selection + // based on the selection state of the container. + var containerIsSelected = GetIsSelected(container); + + if (containerIsSelected != Selection.IsSelected(index)) + { + if (containerIsSelected) + Selection.Select(index); + else + Selection.Deselect(index); + } } } @@ -531,8 +542,7 @@ namespace Avalonia.Controls.Primitives KeyboardNavigation.SetTabOnceActiveElement(panel, null); } - if (element is ISelectable) - MarkContainerSelected(element, false); + element.ClearValue(IsSelectedProperty); } /// @@ -1121,11 +1131,14 @@ namespace Avalonia.Controls.Primitives { if (!_ignoreContainerSelectionChanged && e.Source is Control control && - e.Source is ISelectable selectable && control.Parent == this && - IndexFromContainer(control) != -1) + IndexFromContainer(control) is var index && + index >= 0) { - UpdateSelection(control, selectable.IsSelected); + if (GetIsSelected(control)) + Selection.Select(index); + else + Selection.Deselect(index); } if (e.Source != this) @@ -1135,31 +1148,18 @@ namespace Avalonia.Controls.Primitives } /// - /// Sets a container's 'selected' class or . + /// Sets the on the specified container. /// /// The container. /// Whether the control is selected /// The previous selection state. - private bool MarkContainerSelected(Control container, bool selected) + private void MarkContainerSelected(Control container, bool selected) { + _ignoreContainerSelectionChanged = true; + try { - bool result; - - _ignoreContainerSelectionChanged = true; - - if (container is ISelectable selectable) - { - result = selectable.IsSelected; - selectable.IsSelected = selected; - } - else - { - result = container.Classes.Contains(":selected"); - ((IPseudoClasses)container.Classes).Set(":selected", selected); - } - - return result; + container.SetCurrentValue(IsSelectedProperty, selected); } finally { diff --git a/tests/Avalonia.Controls.UnitTests/Primitives/SelectingItemsControlTests.cs b/tests/Avalonia.Controls.UnitTests/Primitives/SelectingItemsControlTests.cs index 9574598038..db6460e8aa 100644 --- a/tests/Avalonia.Controls.UnitTests/Primitives/SelectingItemsControlTests.cs +++ b/tests/Avalonia.Controls.UnitTests/Primitives/SelectingItemsControlTests.cs @@ -2170,7 +2170,12 @@ namespace Avalonia.Controls.UnitTests.Primitives private class Item : Control, ISelectable { public string Value { get; set; } - public bool IsSelected { get; set; } + + public bool IsSelected + { + get => SelectingItemsControl.GetIsSelected(this); + set => SelectingItemsControl.SetIsSelected(this, value); + } } private class MasterViewModel : NotifyingBase diff --git a/tests/Avalonia.Controls.UnitTests/Primitives/SelectingItemsControlTests_AutoSelect.cs b/tests/Avalonia.Controls.UnitTests/Primitives/SelectingItemsControlTests_AutoSelect.cs index 71bd0933a1..3ef93c7cda 100644 --- a/tests/Avalonia.Controls.UnitTests/Primitives/SelectingItemsControlTests_AutoSelect.cs +++ b/tests/Avalonia.Controls.UnitTests/Primitives/SelectingItemsControlTests_AutoSelect.cs @@ -116,7 +116,6 @@ namespace Avalonia.Controls.UnitTests.Primitives Assert.Equal(0, target.SelectedIndex); Assert.Equal("bar", target.SelectedItem); - Assert.Equal(new[] { ":selected" }, target.Presenter.Panel.Children[0].Classes); } private static FuncControlTemplate Template() diff --git a/tests/Avalonia.Controls.UnitTests/Primitives/SelectingItemsControlTests_Multiple.cs b/tests/Avalonia.Controls.UnitTests/Primitives/SelectingItemsControlTests_Multiple.cs index 0ad8db43b2..f34d090122 100644 --- a/tests/Avalonia.Controls.UnitTests/Primitives/SelectingItemsControlTests_Multiple.cs +++ b/tests/Avalonia.Controls.UnitTests/Primitives/SelectingItemsControlTests_Multiple.cs @@ -4,6 +4,7 @@ using System.Collections.Generic; using System.Collections.ObjectModel; using System.Linq; using Avalonia.Collections; +using Avalonia.Controls.Mixins; using Avalonia.Controls.Presenters; using Avalonia.Controls.Primitives; using Avalonia.Controls.Selection; @@ -134,7 +135,7 @@ namespace Avalonia.Controls.UnitTests.Primitives target.SelectedItems.Add("foo"); bool raised = false; - target.PropertyChanged += (s, e) => + target.PropertyChanged += (s, e) => raised |= e.Property.Name == "SelectedIndex" || e.Property.Name == "SelectedItem"; @@ -152,9 +153,9 @@ namespace Avalonia.Controls.UnitTests.Primitives target.SelectedItems.Add("foo"); bool raised = false; - target.PropertyChanged += (s, e) => - raised |= e.Property.Name == "SelectedIndex" && - (int)e.OldValue! == 0 && + target.PropertyChanged += (s, e) => + raised |= e.Property.Name == "SelectedIndex" && + (int)e.OldValue! == 0 && (int)e.NewValue! == -1; target.SelectedItems.RemoveAt(0); @@ -708,14 +709,14 @@ namespace Avalonia.Controls.UnitTests.Primitives using var app = Start(); var items = new[] { - new ItemContainer(), - new ItemContainer(), + new TestContainer(), + new TestContainer(), }; var target = CreateTarget(items: items); - target.Items.Add(new ItemContainer { IsSelected = true }); - target.Items.Add(new ItemContainer { IsSelected = true }); + target.Items.Add(new TestContainer { IsSelected = true }); + target.Items.Add(new TestContainer { IsSelected = true }); Assert.Equal(2, target.SelectedIndex); Assert.Equal(target.Items[2], target.SelectedItem); @@ -867,12 +868,169 @@ namespace Avalonia.Controls.UnitTests.Primitives Assert.Equal(2, raised); } + [Fact] + public void Can_Bind_Initial_Selected_State_Via_ItemContainerTheme() + { + using var app = Start(); + var items = new ItemViewModel[] { new("Item 0", true), new("Item 1", false), new("Item 2", true) }; + var itemTheme = new ControlTheme(typeof(ContentPresenter)) + { + Setters = + { + new Setter(SelectingItemsControl.IsSelectedProperty, new Binding("IsSelected")), + } + }; + + var target = CreateTarget(itemsSource: items, itemContainerTheme: itemTheme); + + Assert.Equal(new[] { 0, 2 }, SelectedContainers(target)); + Assert.Equal(0, target.SelectedIndex); + Assert.Equal(items[0], target.SelectedItem); + Assert.Equal(new[] { 0, 2 }, target.Selection.SelectedIndexes); + Assert.Equal(new[] { items[0], items[2] }, target.Selection.SelectedItems); + } + + [Fact] + public void Can_Bind_Initial_Selected_State_Via_Style() + { + using var app = Start(); + var items = new ItemViewModel[] { new("Item 0", true), new("Item 1", false), new("Item 2", true) }; + var style = new Style(x => x.OfType()) + { + Setters = + { + new Setter(SelectingItemsControl.IsSelectedProperty, new Binding("IsSelected")), + } + }; + + var target = CreateTarget(itemsSource: items, styles: new[] { style }); + + Assert.Equal(new[] { 0, 2 }, SelectedContainers(target)); + Assert.Equal(0, target.SelectedIndex); + Assert.Equal(items[0], target.SelectedItem); + Assert.Equal(new[] { 0, 2 }, target.Selection.SelectedIndexes); + Assert.Equal(new[] { items[0], items[2] }, target.Selection.SelectedItems); + } + + [Fact] + public void Selection_State_Is_Updated_Via_IsSelected_Binding() + { + using var app = Start(); + var items = new ItemViewModel[] { new("Item 0", true), new("Item 1", false), new("Item 2", true) }; + var itemTheme = new ControlTheme(typeof(TestContainer)) + { + BasedOn = CreateTestContainerTheme(), + Setters = + { + new Setter(SelectingItemsControl.IsSelectedProperty, new Binding("IsSelected")), + } + }; + + // For the container selection state to be communicated back to the SelectingItemsControl + // we need a container which raises the SelectingItemsControl.IsSelectedChangedEvent when + // the IsSelected property changes. + var target = CreateTarget( + itemsSource: items, + itemContainerTheme: itemTheme); + + items[1].IsSelected = true; + + Assert.Equal(new[] { 0, 1, 2 }, SelectedContainers(target)); + Assert.Equal(0, target.SelectedIndex); + Assert.Equal(items[0], target.SelectedItem); + Assert.Equal(new[] { 0, 1, 2 }, target.Selection.SelectedIndexes); + Assert.Equal(new[] { items[0], items[1], items[2] }, target.Selection.SelectedItems); + + items[0].IsSelected = false; + + Assert.Equal(new[] { 1, 2 }, SelectedContainers(target)); + Assert.Equal(1, target.SelectedIndex); + Assert.Equal(items[1], target.SelectedItem); + Assert.Equal(new[] { 1, 2 }, target.Selection.SelectedIndexes); + Assert.Equal(new[] { items[1], items[2] }, target.Selection.SelectedItems); + } + + [Fact] + public void Selection_State_Is_Written_Back_To_Item_Via_IsSelected_Binding() + { + using var app = Start(); + var items = new ItemViewModel[] { new("Item 0", true), new("Item 1", false), new("Item 2", true) }; + var itemTheme = new ControlTheme(typeof(ContentPresenter)) + { + Setters = + { + new Setter(SelectingItemsControl.IsSelectedProperty, new Binding("IsSelected")), + } + }; + + var target = CreateTarget(itemsSource: items, itemContainerTheme: itemTheme); + var container0 = Assert.IsAssignableFrom(target.ContainerFromIndex(0)); + var container1 = Assert.IsAssignableFrom(target.ContainerFromIndex(1)); + + SelectingItemsControl.SetIsSelected(container1, true); + + Assert.True(items[1].IsSelected); + + SelectingItemsControl.SetIsSelected(container0, false); + + Assert.False(items[0].IsSelected); + } + + [Fact] + public void Selection_State_Change_On_Unrealized_Item_Is_Respected_With_IsSelected_Binding() + { + using var app = Start(); + var items = Enumerable.Range(0, 100).Select(x => new ItemViewModel($"Item {x}", false)).ToList(); + var itemTheme = new ControlTheme(typeof(ContentPresenter)) + { + Setters = + { + new Setter(SelectingItemsControl.IsSelectedProperty, new Binding("IsSelected")), + new Setter(Control.HeightProperty, 100.0), + } + }; + + // Create a SelectingItemsControl with a virtualizing stack panel. + var target = CreateTarget(itemsSource: items, itemContainerTheme: itemTheme, virtualizing: true); + var panel = Assert.IsType(target.ItemsPanelRoot); + var scroll = panel.FindAncestorOfType()!; + + // Scroll item 1 out of view. + scroll.Offset = new(0, 1000); + Layout(target); + + Assert.Equal(10, panel.FirstRealizedIndex); + Assert.Equal(19, panel.LastRealizedIndex); + + // Select item 1 now it's unrealized. + items[1].IsSelected = true; + + // The SelectingItemsControl does not yet know anything about the selection change. + Assert.Empty(SelectedContainers(target)); + Assert.Equal(-1, target.SelectedIndex); + Assert.Null(target.SelectedItem); + Assert.Empty(target.Selection.SelectedIndexes); + Assert.Empty(target.Selection.SelectedItems); + + // Scroll item 1 back into view. + scroll.Offset = new(0, 0); + Layout(target); + + // The item and container should be marked as selected. + Assert.True(items[1].IsSelected); + Assert.Equal(new[] { 1 }, SelectedContainers(target)); + Assert.Equal(1, target.SelectedIndex); + Assert.Equal(items[1], target.SelectedItem); + Assert.Equal(new[] { 1 }, target.Selection.SelectedIndexes); + Assert.Equal(new[] { items[1] }, target.Selection.SelectedItems); + } + private static IEnumerable SelectedContainers(SelectingItemsControl target) { Assert.NotNull(target.ItemsPanel); return target.ItemsPanelRoot!.Children - .Select(x => x.Classes.Contains(":selected") ? target.IndexFromContainer(x) : -1) + .Select(x => SelectingItemsControl.GetIsSelected(x) ? target.IndexFromContainer(x) : -1) .Where(x => x != -1); } @@ -882,9 +1040,33 @@ namespace Avalonia.Controls.UnitTests.Primitives IList? itemsSource = null, ControlTheme? itemContainerTheme = null, IDataTemplate? itemTemplate = null, - bool performLayout = true) + IEnumerable