From 9d16e33843bbe9c42dbe34d718776e0d5f561f0a Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Fri, 19 May 2023 11:52:50 +0200 Subject: [PATCH 1/7] Started refactoring ListBox selection. - Don't make selection follow focus - Move all keyboard interaction into `ListBox` - Move all pointer interaction into `ListBoxItem` - Added `ItemsControl.ItemsControlFromItemContaner` as in WPF --- src/Avalonia.Controls/ItemsControl.cs | 24 +++++++ src/Avalonia.Controls/ListBox.cs | 68 ++++++++++-------- src/Avalonia.Controls/ListBoxItem.cs | 63 +++++++++++++++++ .../Primitives/SelectingItemsControl.cs | 51 ++++---------- .../ListBoxTests.cs | 10 +-- .../ListBoxTests_Multiple.cs | 70 ------------------- .../ListBoxTests_Single.cs | 39 ----------- 7 files changed, 145 insertions(+), 180 deletions(-) diff --git a/src/Avalonia.Controls/ItemsControl.cs b/src/Avalonia.Controls/ItemsControl.cs index 1613bda45b..300c66d126 100644 --- a/src/Avalonia.Controls/ItemsControl.cs +++ b/src/Avalonia.Controls/ItemsControl.cs @@ -365,6 +365,30 @@ namespace Avalonia.Controls /// public IEnumerable GetRealizedContainers() => Presenter?.GetRealizedContainers() ?? Array.Empty(); + /// + /// Returns the that owns the specified container control. + /// + /// The container. + /// + /// The owning or null if the control is not an items container. + /// + public static ItemsControl? ItemsControlFromItemContaner(Control container) + { + var c = container.Parent as Control; + + while (c is not null) + { + if (c is ItemsControl itemsControl) + { + return itemsControl.IndexFromContainer(container) >= 0 ? itemsControl : null; + } + + c = c.Parent as Control; + } + + return null; + } + /// /// Creates or a container that can be used to display an item. /// diff --git a/src/Avalonia.Controls/ListBox.cs b/src/Avalonia.Controls/ListBox.cs index 2586333f20..b2cc1977ea 100644 --- a/src/Avalonia.Controls/ListBox.cs +++ b/src/Avalonia.Controls/ListBox.cs @@ -1,4 +1,8 @@ +using System; using System.Collections; +using System.Collections.Generic; +using System.Drawing; +using System.Linq; using Avalonia.Controls.Metadata; using Avalonia.Controls.Primitives; using Avalonia.Controls.Selection; @@ -47,7 +51,7 @@ namespace Avalonia.Controls /// [System.Diagnostics.CodeAnalysis.SuppressMessage("AvaloniaProperty", "AVP1010", Justification = "This property is owned by SelectingItemsControl, but protected there. ListBox changes its visibility.")] - public static readonly new StyledProperty SelectionModeProperty = + public static readonly new StyledProperty SelectionModeProperty = SelectingItemsControl.SelectionModeProperty; private IScrollable? _scroll; @@ -121,41 +125,34 @@ namespace Avalonia.Controls return NeedsContainer(item, out recycleKey); } - /// - protected override void OnGotFocus(GotFocusEventArgs e) + protected override void OnKeyDown(KeyEventArgs e) { - base.OnGotFocus(e); + var hotkeys = AvaloniaLocator.Current.GetService(); - if (e.NavigationMethod == NavigationMethod.Directional) + if (e.Key.ToNavigationDirection() is { } direction) + { + e.Handled |= MoveSelection( + direction, + WrapSelection, + e.KeyModifiers.HasAllFlags(KeyModifiers.Shift)); + } + else if (SelectionMode.HasAllFlags(SelectionMode.Multiple) && + hotkeys is not null && hotkeys.SelectAll.Any(x => x.Matches(e))) { - e.Handled = UpdateSelectionFromEventSource( + Selection.SelectAll(); + e.Handled = true; + } + else if (e.Key == Key.Space || e.Key == Key.Enter) + { + var toggle = hotkeys is not null && e.KeyModifiers.HasAllFlags(hotkeys.CommandModifiers); + e.Handled |= UpdateSelectionFromEventSource( e.Source, true, - e.KeyModifiers.HasAllFlags(KeyModifiers.Shift), - e.KeyModifiers.HasAllFlags(KeyModifiers.Control), - fromFocus: true); + e.KeyModifiers.HasFlag(KeyModifiers.Shift), + toggle); } - } - - /// - protected override void OnPointerPressed(PointerPressedEventArgs e) - { - base.OnPointerPressed(e); - if (e.Source is Visual source) - { - var point = e.GetCurrentPoint(source); - - if (point.Properties.IsLeftButtonPressed || point.Properties.IsRightButtonPressed) - { - e.Handled = UpdateSelectionFromEventSource( - e.Source, - true, - e.KeyModifiers.HasAllFlags(KeyModifiers.Shift), - e.KeyModifiers.HasAllFlags(AvaloniaLocator.Current.GetRequiredService().CommandModifiers), - point.Properties.IsRightButtonPressed); - } - } + base.OnKeyDown(e); } protected override void OnApplyTemplate(TemplateAppliedEventArgs e) @@ -163,5 +160,18 @@ namespace Avalonia.Controls base.OnApplyTemplate(e); Scroll = e.NameScope.Find("PART_ScrollViewer"); } + + internal bool UpdateSelectionFromPointerEvent(Control source, PointerEventArgs e) + { + var hotkeys = AvaloniaLocator.Current.GetService(); + var toggle = hotkeys is not null && e.KeyModifiers.HasAllFlags(hotkeys.CommandModifiers); + + return UpdateSelectionFromEventSource( + source, + true, + e.KeyModifiers.HasAllFlags(KeyModifiers.Shift), + toggle, + e.GetCurrentPoint(source).Properties.IsRightButtonPressed); + } } } diff --git a/src/Avalonia.Controls/ListBoxItem.cs b/src/Avalonia.Controls/ListBoxItem.cs index 0a6873cd59..0a959c5666 100644 --- a/src/Avalonia.Controls/ListBoxItem.cs +++ b/src/Avalonia.Controls/ListBoxItem.cs @@ -2,6 +2,8 @@ using Avalonia.Automation.Peers; using Avalonia.Controls.Metadata; using Avalonia.Controls.Mixins; using Avalonia.Controls.Primitives; +using Avalonia.Input; +using Avalonia.Platform; namespace Avalonia.Controls { @@ -17,6 +19,9 @@ namespace Avalonia.Controls public static readonly StyledProperty IsSelectedProperty = SelectingItemsControl.IsSelectedProperty.AddOwner(); + private static readonly Point s_invalidPoint = new Point(double.NaN, double.NaN); + private Point _pointerDownPoint = s_invalidPoint; + /// /// Initializes static members of the class. /// @@ -40,5 +45,63 @@ namespace Avalonia.Controls { return new ListItemAutomationPeer(this); } + + protected override void OnPointerPressed(PointerPressedEventArgs e) + { + base.OnPointerPressed(e); + + _pointerDownPoint = s_invalidPoint; + + if (e.Handled) + return; + + if (!e.Handled && ItemsControl.ItemsControlFromItemContaner(this) is ListBox owner) + { + var p = e.GetCurrentPoint(this); + + if (p.Properties.PointerUpdateKind is PointerUpdateKind.LeftButtonPressed or + PointerUpdateKind.RightButtonPressed) + { + if (p.Pointer.Type == PointerType.Mouse) + { + // If the pressed point comes from a mouse, perform the selection immediately. + e.Handled = owner.UpdateSelectionFromPointerEvent(this, e); + } + else + { + // Otherwise perform the selection when the pointer is released as to not + // interfere with gestures. + _pointerDownPoint = p.Position; + e.Handled = true; + } + } + } + } + + protected override void OnPointerReleased(PointerReleasedEventArgs e) + { + base.OnPointerReleased(e); + + if (!e.Handled && + !double.IsNaN(_pointerDownPoint.X) && + e.InitialPressMouseButton is MouseButton.Left or MouseButton.Right) + { + var point = e.GetCurrentPoint(this); + var settings = AvaloniaLocator.Current.GetService(); + var tapSize = settings?.GetTapSize(point.Pointer.Type) ?? new Size(4, 4); + var tapRect = new Rect(_pointerDownPoint, new Size()) + .Inflate(new Thickness(tapSize.Width, tapSize.Height)); + + if (tapRect.ContainsExclusive(point.Position) && + ItemsControl.ItemsControlFromItemContaner(this) is ListBox owner) + { + if (owner.UpdateSelectionFromPointerEvent(this, e)) + e.Handled = true; + } + } + + _pointerDownPoint = s_invalidPoint; + } + } } diff --git a/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs b/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs index af82a89517..59ea8c2e9d 100644 --- a/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs +++ b/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs @@ -618,38 +618,6 @@ namespace Avalonia.Controls.Primitives base.OnTextInput(e); } - /// - protected override void OnKeyDown(KeyEventArgs e) - { - base.OnKeyDown(e); - - if (!e.Handled) - { - var keymap = AvaloniaLocator.Current.GetService(); - - if (keymap is null) - return; - - bool Match(List gestures) => gestures.Any(g => g.Matches(e)); - - if (ItemCount > 0 && - Match(keymap.SelectAll) && - SelectionMode.HasAllFlags(SelectionMode.Multiple)) - { - Selection.SelectAll(); - e.Handled = true; - } - else if (e.Key == Key.Space || e.Key == Key.Enter) - { - UpdateSelectionFromEventSource( - e.Source, - true, - e.KeyModifiers.HasFlag(KeyModifiers.Shift), - e.KeyModifiers.HasFlag(KeyModifiers.Control)); - } - } - } - /// protected override void OnPropertyChanged(AvaloniaPropertyChangedEventArgs change) { @@ -729,11 +697,15 @@ namespace Avalonia.Controls.Primitives /// /// The direction to move. /// Whether to wrap when the selection reaches the first or last item. + /// Whether the range modifier is enabled (i.e. shift key). /// True if the selection was moved; otherwise false. - protected bool MoveSelection(NavigationDirection direction, bool wrap) + protected bool MoveSelection( + NavigationDirection direction, + bool wrap = false, + bool rangeModifier = false) { - var from = SelectedIndex != -1 ? ContainerFromIndex(SelectedIndex) : null; - return MoveSelection(from, direction, wrap); + var from = ContainerFromIndex(Selection.AnchorIndex); + return MoveSelection(from, direction, wrap, rangeModifier); } /// @@ -742,8 +714,13 @@ namespace Avalonia.Controls.Primitives /// The container which serves as a starting point for the movement. /// The direction to move. /// Whether to wrap when the selection reaches the first or last item. + /// Whether the range modifier is enabled (i.e. shift key). /// True if the selection was moved; otherwise false. - protected bool MoveSelection(Control? from, NavigationDirection direction, bool wrap) + protected bool MoveSelection( + Control? from, + NavigationDirection direction, + bool wrap = false, + bool rangeModifier = false) { if (Presenter?.Panel is INavigableContainer container && GetNextControl(container, direction, from, wrap) is Control next) @@ -752,7 +729,7 @@ namespace Avalonia.Controls.Primitives if (index != -1) { - SelectedIndex = index; + UpdateSelection(index, true, rangeModifier); return true; } } diff --git a/tests/Avalonia.Controls.UnitTests/ListBoxTests.cs b/tests/Avalonia.Controls.UnitTests/ListBoxTests.cs index 72f476a3b0..98e5017fa9 100644 --- a/tests/Avalonia.Controls.UnitTests/ListBoxTests.cs +++ b/tests/Avalonia.Controls.UnitTests/ListBoxTests.cs @@ -298,11 +298,11 @@ namespace Avalonia.Controls.UnitTests Assert.Equal(false, item.IsSelected); - RaisePressedEvent(target, item, MouseButton.Left); + RaisePressedEvent(item, MouseButton.Left); Assert.Equal(true, item.IsSelected); - RaisePressedEvent(target, item, MouseButton.Left); + RaisePressedEvent(item, MouseButton.Left); Assert.Equal(false, item.IsSelected); } @@ -328,9 +328,9 @@ namespace Avalonia.Controls.UnitTests } } - private void RaisePressedEvent(ListBox listBox, ListBoxItem item, MouseButton mouseButton) + private void RaisePressedEvent(ListBoxItem item, MouseButton mouseButton) { - _mouse.Click(listBox, item, mouseButton); + _mouse.Click(item, item, mouseButton); } [Fact] @@ -776,7 +776,7 @@ namespace Avalonia.Controls.UnitTests first.Focus(); - RaisePressedEvent(target, first, MouseButton.Left); + RaisePressedEvent(first, MouseButton.Left); Assert.Equal(true, first.IsSelected); RaiseKeyEvent(target, Key.Up); diff --git a/tests/Avalonia.Controls.UnitTests/ListBoxTests_Multiple.cs b/tests/Avalonia.Controls.UnitTests/ListBoxTests_Multiple.cs index e77a3529b8..1b1671a69c 100644 --- a/tests/Avalonia.Controls.UnitTests/ListBoxTests_Multiple.cs +++ b/tests/Avalonia.Controls.UnitTests/ListBoxTests_Multiple.cs @@ -17,76 +17,6 @@ namespace Avalonia.Controls.UnitTests { private MouseTestHelper _helper = new MouseTestHelper(); - [Fact] - public void Focusing_Item_With_Shift_And_Arrow_Key_Should_Add_To_Selection() - { - var target = new ListBox - { - Template = new FuncControlTemplate(CreateListBoxTemplate), - ItemsSource = new[] { "Foo", "Bar", "Baz " }, - SelectionMode = SelectionMode.Multiple - }; - - ApplyTemplate(target); - - target.SelectedItem = "Foo"; - - target.Presenter.Panel.Children[1].RaiseEvent(new GotFocusEventArgs - { - NavigationMethod = NavigationMethod.Directional, - KeyModifiers = KeyModifiers.Shift - }); - - Assert.Equal(new[] { "Foo", "Bar" }, target.SelectedItems); - } - - [Fact] - public void Focusing_Item_With_Ctrl_And_Arrow_Key_Should_Not_Add_To_Selection() - { - var target = new ListBox - { - Template = new FuncControlTemplate(CreateListBoxTemplate), - ItemsSource = new[] { "Foo", "Bar", "Baz " }, - SelectionMode = SelectionMode.Multiple - }; - - ApplyTemplate(target); - - target.SelectedItem = "Foo"; - - target.Presenter.Panel.Children[1].RaiseEvent(new GotFocusEventArgs - { - NavigationMethod = NavigationMethod.Directional, - KeyModifiers = KeyModifiers.Control - }); - - Assert.Equal(new[] { "Foo" }, target.SelectedItems); - } - - [Fact] - public void Focusing_Selected_Item_With_Ctrl_And_Arrow_Key_Should_Not_Remove_From_Selection() - { - var target = new ListBox - { - Template = new FuncControlTemplate(CreateListBoxTemplate), - ItemsSource = new[] { "Foo", "Bar", "Baz " }, - SelectionMode = SelectionMode.Multiple - }; - - ApplyTemplate(target); - - target.SelectedItems.Add("Foo"); - target.SelectedItems.Add("Bar"); - - target.Presenter.Panel.Children[0].RaiseEvent(new GotFocusEventArgs - { - NavigationMethod = NavigationMethod.Directional, - KeyModifiers = KeyModifiers.Control - }); - - Assert.Equal(new[] { "Foo", "Bar" }, target.SelectedItems); - } - [Fact] public void Shift_Selecting_From_No_Selection_Selects_From_Start() { diff --git a/tests/Avalonia.Controls.UnitTests/ListBoxTests_Single.cs b/tests/Avalonia.Controls.UnitTests/ListBoxTests_Single.cs index 5551d1c7df..ca7cc044ad 100644 --- a/tests/Avalonia.Controls.UnitTests/ListBoxTests_Single.cs +++ b/tests/Avalonia.Controls.UnitTests/ListBoxTests_Single.cs @@ -39,45 +39,6 @@ namespace Avalonia.Controls.UnitTests Assert.Equal(-1, target.SelectedIndex); } - [Fact] - public void Focusing_Item_With_Arrow_Key_Should_Select_It() - { - var target = new ListBox - { - Template = new FuncControlTemplate(CreateListBoxTemplate), - ItemsSource = new[] { "Foo", "Bar", "Baz " }, - }; - - ApplyTemplate(target); - - target.Presenter.Panel.Children[0].RaiseEvent(new GotFocusEventArgs - { - NavigationMethod = NavigationMethod.Directional, - }); - - Assert.Equal(0, target.SelectedIndex); - } - - [Fact] - public void Focusing_Item_With_Arrow_Key_And_Ctrl_Pressed_Should_Not_Select_It() - { - var target = new ListBox - { - Template = new FuncControlTemplate(CreateListBoxTemplate), - ItemsSource = new[] { "Foo", "Bar", "Baz " }, - }; - - ApplyTemplate(target); - - target.Presenter.Panel.Children[0].RaiseEvent(new GotFocusEventArgs - { - NavigationMethod = NavigationMethod.Directional, - KeyModifiers = KeyModifiers.Control - }); - - Assert.Equal(-1, target.SelectedIndex); - } - [Fact] public void Pressing_Space_On_Focused_Item_With_Ctrl_Pressed_Should_Select_It() { From 06bfa29b12c8ffbaaeac94b350d0e564bbb3934b Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Fri, 19 May 2023 13:13:47 +0200 Subject: [PATCH 2/7] Properly implement keyboard selection. --- src/Avalonia.Controls/ListBox.cs | 8 +- .../Primitives/SelectingItemsControl.cs | 22 ++- .../VirtualizingStackPanel.cs | 6 +- .../ListBoxTests.cs | 141 ++++++++++++++++++ .../ListBoxTests_Multiple.cs | 118 +++++++++++++-- 5 files changed, 274 insertions(+), 21 deletions(-) diff --git a/src/Avalonia.Controls/ListBox.cs b/src/Avalonia.Controls/ListBox.cs index b2cc1977ea..f3f0b135b8 100644 --- a/src/Avalonia.Controls/ListBox.cs +++ b/src/Avalonia.Controls/ListBox.cs @@ -128,8 +128,11 @@ namespace Avalonia.Controls protected override void OnKeyDown(KeyEventArgs e) { var hotkeys = AvaloniaLocator.Current.GetService(); + var ctrl = hotkeys is not null && e.KeyModifiers.HasAllFlags(hotkeys.CommandModifiers); - if (e.Key.ToNavigationDirection() is { } direction) + if (!ctrl && + e.Key.ToNavigationDirection() is { } direction && + direction.IsDirectional()) { e.Handled |= MoveSelection( direction, @@ -144,12 +147,11 @@ namespace Avalonia.Controls } else if (e.Key == Key.Space || e.Key == Key.Enter) { - var toggle = hotkeys is not null && e.KeyModifiers.HasAllFlags(hotkeys.CommandModifiers); e.Handled |= UpdateSelectionFromEventSource( e.Source, true, e.KeyModifiers.HasFlag(KeyModifiers.Shift), - toggle); + ctrl); } base.OnKeyDown(e); diff --git a/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs b/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs index 59ea8c2e9d..64ecb4321e 100644 --- a/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs +++ b/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs @@ -704,7 +704,8 @@ namespace Avalonia.Controls.Primitives bool wrap = false, bool rangeModifier = false) { - var from = ContainerFromIndex(Selection.AnchorIndex); + var from = GetContainerFromEventSource(FocusManager.Instance?.Current) ?? + ContainerFromIndex(Selection.AnchorIndex); return MoveSelection(from, direction, wrap, rangeModifier); } @@ -722,14 +723,29 @@ namespace Avalonia.Controls.Primitives bool wrap = false, bool rangeModifier = false) { - if (Presenter?.Panel is INavigableContainer container && - GetNextControl(container, direction, from, wrap) is Control next) + if (Presenter?.Panel is not INavigableContainer container) + return false; + + if (from is null) + { + direction = direction switch + { + NavigationDirection.Down => NavigationDirection.First, + NavigationDirection.Up => NavigationDirection.Last, + NavigationDirection.Right => NavigationDirection.First, + NavigationDirection.Left => NavigationDirection.Last, + _ => direction, + }; + } + + if (GetNextControl(container, direction, from, wrap) is Control next) { var index = IndexFromContainer(next); if (index != -1) { UpdateSelection(index, true, rangeModifier); + next.Focus(); return true; } } diff --git a/src/Avalonia.Controls/VirtualizingStackPanel.cs b/src/Avalonia.Controls/VirtualizingStackPanel.cs index 7ec1808e63..2e5113d341 100644 --- a/src/Avalonia.Controls/VirtualizingStackPanel.cs +++ b/src/Avalonia.Controls/VirtualizingStackPanel.cs @@ -265,12 +265,14 @@ namespace Avalonia.Controls protected override IInputElement? GetControl(NavigationDirection direction, IInputElement? from, bool wrap) { var count = Items.Count; + var fromControl = from as Control; - if (count == 0 || from is not Control fromControl) + if (count == 0 || + fromControl is null && direction is not NavigationDirection.First or NavigationDirection.Last) return null; var horiz = Orientation == Orientation.Horizontal; - var fromIndex = from != null ? IndexFromContainer(fromControl) : -1; + var fromIndex = fromControl != null ? IndexFromContainer(fromControl) : -1; var toIndex = fromIndex; switch (direction) diff --git a/tests/Avalonia.Controls.UnitTests/ListBoxTests.cs b/tests/Avalonia.Controls.UnitTests/ListBoxTests.cs index 98e5017fa9..0e957f9360 100644 --- a/tests/Avalonia.Controls.UnitTests/ListBoxTests.cs +++ b/tests/Avalonia.Controls.UnitTests/ListBoxTests.cs @@ -12,6 +12,7 @@ using Avalonia.Data; using Avalonia.Input; using Avalonia.Layout; using Avalonia.LogicalTree; +using Avalonia.Markup.Xaml.Templates; using Avalonia.Styling; using Avalonia.UnitTests; using Avalonia.VisualTree; @@ -752,6 +753,146 @@ namespace Avalonia.Controls.UnitTests Assert.Equal(Enumerable.Range(0, 10).Select(x => $"Item{10 - x}"), realized); } + [Fact] + public void Arrow_Keys_Should_Move_Selection_Vertical() + { + using var app = UnitTestApplication.Start(TestServices.RealFocus); + var items = Enumerable.Range(0, 10).Select(x => $"Item {x}").ToArray(); + var target = new ListBox + { + Template = ListBoxTemplate(), + ItemsSource = items, + ItemTemplate = new FuncDataTemplate((x, _) => new TextBlock { Height = 10 }), + SelectedIndex = 0, + }; + + Prepare(target); + + RaiseKeyEvent(target, Key.Down); + Assert.Equal(1, target.SelectedIndex); + + RaiseKeyEvent(target, Key.Down); + Assert.Equal(2, target.SelectedIndex); + + RaiseKeyEvent(target, Key.Up); + Assert.Equal(1, target.SelectedIndex); + } + + [Fact] + public void Arrow_Keys_Should_Move_Selection_Horizontal() + { + using var app = UnitTestApplication.Start(TestServices.RealFocus); + var items = Enumerable.Range(0, 10).Select(x => $"Item {x}").ToArray(); + var target = new ListBox + { + Template = ListBoxTemplate(), + ItemsSource = items, + ItemsPanel = new FuncTemplate(() => new VirtualizingStackPanel + { + Orientation = Orientation.Horizontal + }), + ItemTemplate = new FuncDataTemplate((x, _) => new TextBlock { Height = 10 }), + SelectedIndex = 0, + }; + + Prepare(target); + + RaiseKeyEvent(target, Key.Right); + Assert.Equal(1, target.SelectedIndex); + + RaiseKeyEvent(target, Key.Right); + Assert.Equal(2, target.SelectedIndex); + + RaiseKeyEvent(target, Key.Left); + Assert.Equal(1, target.SelectedIndex); + } + + [Fact] + public void Arrow_Keys_Should_Focus_Selection() + { + using var app = UnitTestApplication.Start(TestServices.RealFocus); + var items = Enumerable.Range(0, 10).Select(x => $"Item {x}").ToArray(); + var target = new ListBox + { + Template = ListBoxTemplate(), + ItemsSource = items, + ItemTemplate = new FuncDataTemplate((x, _) => new TextBlock { Height = 10 }), + SelectedIndex = 0, + }; + + Prepare(target); + + RaiseKeyEvent(target, Key.Down); + Assert.True(target.ContainerFromIndex(1).IsFocused); + + RaiseKeyEvent(target, Key.Down); + Assert.True(target.ContainerFromIndex(2).IsFocused); + + RaiseKeyEvent(target, Key.Up); + Assert.True(target.ContainerFromIndex(1).IsFocused); + } + + [Fact] + public void Down_Key_Selecting_From_No_Selection_And_No_Focus_Selects_From_Start() + { + using var app = UnitTestApplication.Start(TestServices.RealFocus); + var target = new ListBox + { + Template = ListBoxTemplate(), + ItemsSource = new[] { "Foo", "Bar", "Baz" }, + Width = 100, + Height = 100, + }; + + Prepare(target); + + RaiseKeyEvent(target, Key.Down); + + Assert.Equal(0, target.SelectedIndex); + } + + [Fact] + public void Down_Key_Selecting_From_No_Selection_Selects_From_Focus() + { + using var app = UnitTestApplication.Start(TestServices.RealFocus); + var target = new ListBox + { + Template = ListBoxTemplate(), + ItemsSource = new[] { "Foo", "Bar", "Baz" }, + Width = 100, + Height = 100, + }; + + Prepare(target); + + target.ContainerFromIndex(1)!.Focus(); + RaiseKeyEvent(target, Key.Down); + + Assert.Equal(2, target.SelectedIndex); + } + + [Fact] + public void Ctrl_Down_Key_Moves_Focus_But_Not_Selection() + { + using var app = UnitTestApplication.Start(TestServices.RealFocus); + var target = new ListBox + { + Template = ListBoxTemplate(), + ItemsSource = new[] { "Foo", "Bar", "Baz" }, + Width = 100, + Height = 100, + SelectedIndex = 0, + }; + + Prepare(target); + + target.ContainerFromIndex(0)!.Focus(); + RaiseKeyEvent(target, Key.Down, KeyModifiers.Control); + + Assert.Equal(0, target.SelectedIndex); + Assert.True(target.ContainerFromIndex(1).IsFocused); + } + [Fact] public void WrapSelection_Should_Wrap() { diff --git a/tests/Avalonia.Controls.UnitTests/ListBoxTests_Multiple.cs b/tests/Avalonia.Controls.UnitTests/ListBoxTests_Multiple.cs index 1b1671a69c..ed19d1dc97 100644 --- a/tests/Avalonia.Controls.UnitTests/ListBoxTests_Multiple.cs +++ b/tests/Avalonia.Controls.UnitTests/ListBoxTests_Multiple.cs @@ -479,6 +479,104 @@ namespace Avalonia.Controls.UnitTests Assert.Equal(1, target.SelectedItems.Count); } } + + [Fact] + public void Shift_Arrow_Key_Selects_Range() + { + using var app = UnitTestApplication.Start(TestServices.RealFocus); + var target = new ListBox + { + Template = new FuncControlTemplate(CreateListBoxTemplate), + ItemsSource = new[] { "Foo", "Bar", "Baz" }, + SelectionMode = SelectionMode.Multiple, + Width = 100, + Height = 100, + SelectedIndex = 0, + }; + + var root = new TestRoot(target); + root.LayoutManager.ExecuteInitialLayoutPass(); + + RaiseKeyEvent(target, Key.Down, KeyModifiers.Shift); + + Assert.Equal(new[] { "Foo", "Bar", }, target.SelectedItems); + Assert.Equal(new[] { 0, 1 }, SelectedContainers(target)); + Assert.True(target.ContainerFromIndex(1).IsFocused); + + RaiseKeyEvent(target, Key.Down, KeyModifiers.Shift); + + Assert.Equal(new[] { "Foo", "Bar", "Baz", }, target.SelectedItems); + Assert.Equal(new[] { 0, 1, 2 }, SelectedContainers(target)); + Assert.True(target.ContainerFromIndex(2).IsFocused); + + RaiseKeyEvent(target, Key.Up, KeyModifiers.Shift); + + Assert.Equal(new[] { "Foo", "Bar", }, target.SelectedItems); + Assert.Equal(new[] { 0, 1 }, SelectedContainers(target)); + Assert.True(target.ContainerFromIndex(1).IsFocused); + } + + [Fact] + public void Shift_Down_Key_Selecting_Selects_Range_End_From_Focus() + { + using var app = UnitTestApplication.Start(TestServices.RealFocus); + var target = new ListBox + { + Template = new FuncControlTemplate(CreateListBoxTemplate), + ItemsSource = new[] { "Foo", "Bar", "Baz" }, + SelectionMode = SelectionMode.Multiple, + Width = 100, + Height = 100, + SelectedIndex = 0, + }; + + var root = new TestRoot(target); + root.LayoutManager.ExecuteInitialLayoutPass(); + + target.ContainerFromIndex(1)!.Focus(); + RaiseKeyEvent(target, Key.Down, KeyModifiers.Shift); + + Assert.Equal(new[] { "Foo", "Bar", "Baz" }, target.SelectedItems); + Assert.Equal(new[] { 0, 1, 2 }, SelectedContainers(target)); + Assert.True(target.ContainerFromIndex(2).IsFocused); + } + + [Fact] + public void Shift_Down_Key_Selecting_Selects_Range_End_From_Focus_Moved_With_Ctrl_Key() + { + using var app = UnitTestApplication.Start(TestServices.RealFocus); + var target = new ListBox + { + Template = new FuncControlTemplate(CreateListBoxTemplate), + ItemsSource = new[] { "Foo", "Bar", "Baz", "Qux" }, + SelectionMode = SelectionMode.Multiple, + Width = 100, + Height = 100, + SelectedIndex = 0, + }; + + var root = new TestRoot(target); + root.LayoutManager.ExecuteInitialLayoutPass(); + + RaiseKeyEvent(target, Key.Down, KeyModifiers.Shift); + + Assert.Equal(new[] { "Foo", "Bar" }, target.SelectedItems); + Assert.Equal(new[] { 0, 1 }, SelectedContainers(target)); + Assert.True(target.ContainerFromIndex(1).IsFocused); + + RaiseKeyEvent(target, Key.Down, KeyModifiers.Control); + + Assert.Equal(new[] { "Foo", "Bar" }, target.SelectedItems); + Assert.Equal(new[] { 0, 1 }, SelectedContainers(target)); + Assert.True(target.ContainerFromIndex(2).IsFocused); + + RaiseKeyEvent(target, Key.Down, KeyModifiers.Shift); + + Assert.Equal(new[] { "Foo", "Bar", "Baz", "Qux" }, target.SelectedItems); + Assert.Equal(new[] { 0, 1, 2, 3 }, SelectedContainers(target)); + Assert.True(target.ContainerFromIndex(3).IsFocused); + } + private Control CreateListBoxTemplate(TemplatedControl parent, INameScope scope) { return new ScrollViewer @@ -501,20 +599,14 @@ namespace Avalonia.Controls.UnitTests }.RegisterInNameScope(scope); } - private static void ApplyTemplate(ListBox target) + private static void RaiseKeyEvent(Control target, Key key, KeyModifiers inputModifiers = 0) { - // Apply the template to the ListBox itself. - target.ApplyTemplate(); - - // Then to its inner ScrollViewer. - var scrollViewer = (ScrollViewer)target.GetVisualChildren().Single(); - scrollViewer.ApplyTemplate(); - - // Then make the ScrollViewer create its child. - ((ContentPresenter)scrollViewer.Presenter).UpdateChild(); - - // Now the ItemsPresenter should be reigstered, so apply its template. - ((Control)target.Presenter).ApplyTemplate(); + target.RaiseEvent(new KeyEventArgs + { + RoutedEvent = InputElement.KeyDownEvent, + KeyModifiers = inputModifiers, + Key = key + }); } private static IEnumerable SelectedContainers(SelectingItemsControl target) From c4fe776b96e34b829ab11d7f29f516527e2a1109 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Fri, 19 May 2023 13:25:33 +0200 Subject: [PATCH 3/7] Allow selection of elements outside viewport. Allow selection of elements outside viewport when using keyboard to select. --- .../VirtualizingStackPanel.cs | 42 +++++++++---------- .../ListBoxTests.cs | 31 ++++++++++++++ 2 files changed, 52 insertions(+), 21 deletions(-) diff --git a/src/Avalonia.Controls/VirtualizingStackPanel.cs b/src/Avalonia.Controls/VirtualizingStackPanel.cs index 2e5113d341..bece4283db 100644 --- a/src/Avalonia.Controls/VirtualizingStackPanel.cs +++ b/src/Avalonia.Controls/VirtualizingStackPanel.cs @@ -70,8 +70,8 @@ namespace Avalonia.Controls private ScrollViewer? _scrollViewer; private Rect _viewport = s_invalidViewport; private Dictionary>? _recyclePool; - private Control? _unrealizedFocusedElement; - private int _unrealizedFocusedIndex = -1; + private Control? _outOfViewportFocusedElement; + private int _outOfViewportFocusedIndex = -1; public VirtualizingStackPanel() { @@ -332,14 +332,21 @@ namespace Avalonia.Controls { if (index < 0 || index >= Items.Count) return null; - if (_realizedElements?.GetElement(index) is { } realized) + if (GetRealizedElement(index) is { } realized) return realized; if (Items[index] is Control c && c.GetValue(RecycleKeyProperty) == s_itemIsItsOwnContainer) return c; return null; } - protected internal override int IndexFromContainer(Control container) => _realizedElements?.GetIndex(container) ?? -1; + protected internal override int IndexFromContainer(Control container) + { + if (container == _scrollToElement) + return _scrollToIndex; + if (container == _outOfViewportFocusedElement) + return _outOfViewportFocusedIndex; + return _realizedElements?.GetIndex(container) ?? -1; + } protected internal override Control? ScrollIntoView(int index) { @@ -590,6 +597,8 @@ namespace Avalonia.Controls { if (_scrollToIndex == index) return _scrollToElement; + if (_outOfViewportFocusedIndex == index) + return _outOfViewportFocusedElement; return _realizedElements?.GetElement(index); } @@ -621,15 +630,6 @@ namespace Avalonia.Controls var generator = ItemContainerGenerator!; - if (_unrealizedFocusedIndex == index && _unrealizedFocusedElement is not null) - { - var element = _unrealizedFocusedElement; - _unrealizedFocusedElement.LostFocus -= OnUnrealizedFocusedElementLostFocus; - _unrealizedFocusedElement = null; - _unrealizedFocusedIndex = -1; - return element; - } - if (_recyclePool?.TryGetValue(recycleKey, out var recyclePool) == true && recyclePool.Count > 0) { var recycled = recyclePool.Pop(); @@ -672,9 +672,9 @@ namespace Avalonia.Controls } else if (element.IsKeyboardFocusWithin) { - _unrealizedFocusedElement = element; - _unrealizedFocusedIndex = index; - _unrealizedFocusedElement.LostFocus += OnUnrealizedFocusedElementLostFocus; + _outOfViewportFocusedElement = element; + _outOfViewportFocusedIndex = index; + _outOfViewportFocusedElement.LostFocus += OnUnrealizedFocusedElementLostFocus; } else { @@ -744,13 +744,13 @@ namespace Avalonia.Controls private void OnUnrealizedFocusedElementLostFocus(object? sender, RoutedEventArgs e) { - if (_unrealizedFocusedElement is null || sender != _unrealizedFocusedElement) + if (_outOfViewportFocusedElement is null || sender != _outOfViewportFocusedElement) return; - _unrealizedFocusedElement.LostFocus -= OnUnrealizedFocusedElementLostFocus; - RecycleElement(_unrealizedFocusedElement, _unrealizedFocusedIndex); - _unrealizedFocusedElement = null; - _unrealizedFocusedIndex = -1; + _outOfViewportFocusedElement.LostFocus -= OnUnrealizedFocusedElementLostFocus; + RecycleElement(_outOfViewportFocusedElement, _outOfViewportFocusedIndex); + _outOfViewportFocusedElement = null; + _outOfViewportFocusedIndex = -1; } /// diff --git a/tests/Avalonia.Controls.UnitTests/ListBoxTests.cs b/tests/Avalonia.Controls.UnitTests/ListBoxTests.cs index 0e957f9360..3b534a0047 100644 --- a/tests/Avalonia.Controls.UnitTests/ListBoxTests.cs +++ b/tests/Avalonia.Controls.UnitTests/ListBoxTests.cs @@ -893,6 +893,37 @@ namespace Avalonia.Controls.UnitTests Assert.True(target.ContainerFromIndex(1).IsFocused); } + [Fact] + public void Down_Key_Brings_Unrealized_Selection_Into_View() + { + using var app = UnitTestApplication.Start(TestServices.RealFocus); + var items = Enumerable.Range(0, 100).Select(x => $"Item {x}").ToArray(); + var target = new ListBox + { + Template = ListBoxTemplate(), + ItemsSource = items, + ItemTemplate = new FuncDataTemplate((x, _) => new TextBlock { Width = 20, Height = 10 }), + Width = 100, + Height = 100, + SelectedIndex = 0, + }; + + Prepare(target); + + target.ContainerFromIndex(0)!.Focus(); + target.Scroll.Offset = new Vector(0, 100); + Layout(target); + + var panel = (VirtualizingStackPanel)target.ItemsPanelRoot; + Assert.Equal(10, panel.FirstRealizedIndex); + + RaiseKeyEvent(target, Key.Down); + + Assert.Equal(1, target.SelectedIndex); + Assert.True(target.ContainerFromIndex(1).IsFocused); + Assert.Equal(new Vector(0, 10), target.Scroll.Offset); + } + [Fact] public void WrapSelection_Should_Wrap() { From 48a79512a92da3a45fae6b157883e2cebd7bc81d Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Fri, 19 May 2023 15:17:46 +0200 Subject: [PATCH 4/7] Fix realization of special elements. We need to clear the scrollTo fields when they're taken for realization, otherwise we can get one element used in multiple positions. --- .../VirtualizingStackPanel.cs | 100 +++++++++++------- 1 file changed, 59 insertions(+), 41 deletions(-) diff --git a/src/Avalonia.Controls/VirtualizingStackPanel.cs b/src/Avalonia.Controls/VirtualizingStackPanel.cs index bece4283db..40c67debe0 100644 --- a/src/Avalonia.Controls/VirtualizingStackPanel.cs +++ b/src/Avalonia.Controls/VirtualizingStackPanel.cs @@ -70,8 +70,8 @@ namespace Avalonia.Controls private ScrollViewer? _scrollViewer; private Rect _viewport = s_invalidViewport; private Dictionary>? _recyclePool; - private Control? _outOfViewportFocusedElement; - private int _outOfViewportFocusedIndex = -1; + private Control? _focusedElement; + private int _focusedIndex = -1; public VirtualizingStackPanel() { @@ -332,6 +332,10 @@ namespace Avalonia.Controls { if (index < 0 || index >= Items.Count) return null; + if (_scrollToIndex == index) + return _scrollToElement; + if (_focusedIndex == index) + return _focusedElement; if (GetRealizedElement(index) is { } realized) return realized; if (Items[index] is Control c && c.GetValue(RecycleKeyProperty) == s_itemIsItsOwnContainer) @@ -343,8 +347,8 @@ namespace Avalonia.Controls { if (container == _scrollToElement) return _scrollToIndex; - if (container == _outOfViewportFocusedElement) - return _outOfViewportFocusedIndex; + if (container == _focusedElement) + return _focusedIndex; return _realizedElements?.GetIndex(container) ?? -1; } @@ -364,16 +368,19 @@ namespace Avalonia.Controls { // Create and measure the element to be brought into view. Store it in a field so that // it can be re-used in the layout pass. - _scrollToElement = GetOrCreateElement(items, index); - _scrollToElement.Measure(Size.Infinity); - _scrollToIndex = index; + var scrollToElement = GetOrCreateElement(items, index); + scrollToElement.Measure(Size.Infinity); // Get the expected position of the elment and put it in place. var anchorU = _realizedElements.GetOrEstimateElementU(index, ref _lastEstimatedElementSizeU); var rect = Orientation == Orientation.Horizontal ? - new Rect(anchorU, 0, _scrollToElement.DesiredSize.Width, _scrollToElement.DesiredSize.Height) : - new Rect(0, anchorU, _scrollToElement.DesiredSize.Width, _scrollToElement.DesiredSize.Height); - _scrollToElement.Arrange(rect); + new Rect(anchorU, 0, scrollToElement.DesiredSize.Width, scrollToElement.DesiredSize.Height) : + new Rect(0, anchorU, scrollToElement.DesiredSize.Width, scrollToElement.DesiredSize.Height); + scrollToElement.Arrange(rect); + + // Store the element and index so that they can be used in the layout pass. + _scrollToElement = scrollToElement; + _scrollToIndex = index; // If the item being brought into view was added since the last layout pass then // our bounds won't be updated, so any containing scroll viewers will not have an @@ -387,7 +394,7 @@ namespace Avalonia.Controls } // Try to bring the item into view. - _scrollToElement.BringIntoView(); + scrollToElement.BringIntoView(); // If the viewport does not contain the item to scroll to, set _isWaitingForViewportUpdate: // this should cause the following chain of events: @@ -406,10 +413,9 @@ namespace Avalonia.Controls root.LayoutManager.ExecuteLayoutPass(); } - var result = _scrollToElement; _scrollToElement = null; _scrollToIndex = -1; - return result; + return scrollToElement; } return null; @@ -572,36 +578,48 @@ namespace Avalonia.Controls { Debug.Assert(ItemContainerGenerator is not null); - var e = GetRealizedElement(index); + if ((GetRealizedElement(index) ?? + GetRealizedElement(index, ref _focusedIndex, ref _focusedElement) ?? + GetRealizedElement(index, ref _scrollToIndex, ref _scrollToElement)) is { } realized) + return realized; - if (e is null) - { - var item = items[index]; - var generator = ItemContainerGenerator!; + var item = items[index]; + var generator = ItemContainerGenerator!; - if (generator.NeedsContainer(item, index, out var recycleKey)) - { - e = GetRecycledElement(item, index, recycleKey) ?? - CreateElement(item, index, recycleKey); - } - else - { - e = GetItemAsOwnContainer(item, index); - } + if (generator.NeedsContainer(item, index, out var recycleKey)) + { + return GetRecycledElement(item, index, recycleKey) ?? + CreateElement(item, index, recycleKey); + } + else + { + return GetItemAsOwnContainer(item, index); } - - return e; } private Control? GetRealizedElement(int index) { - if (_scrollToIndex == index) - return _scrollToElement; - if (_outOfViewportFocusedIndex == index) - return _outOfViewportFocusedElement; return _realizedElements?.GetElement(index); } + private static Control? GetRealizedElement( + int index, + ref int specialIndex, + ref Control? specialElement) + { + if (specialIndex == index) + { + Debug.Assert(specialElement is not null); + + var result = specialElement; + specialIndex = -1; + specialElement = null; + return result; + } + + return null; + } + private Control GetItemAsOwnContainer(object? item, int index) { Debug.Assert(ItemContainerGenerator is not null); @@ -672,9 +690,9 @@ namespace Avalonia.Controls } else if (element.IsKeyboardFocusWithin) { - _outOfViewportFocusedElement = element; - _outOfViewportFocusedIndex = index; - _outOfViewportFocusedElement.LostFocus += OnUnrealizedFocusedElementLostFocus; + _focusedElement = element; + _focusedIndex = index; + _focusedElement.LostFocus += OnUnrealizedFocusedElementLostFocus; } else { @@ -744,13 +762,13 @@ namespace Avalonia.Controls private void OnUnrealizedFocusedElementLostFocus(object? sender, RoutedEventArgs e) { - if (_outOfViewportFocusedElement is null || sender != _outOfViewportFocusedElement) + if (_focusedElement is null || sender != _focusedElement) return; - _outOfViewportFocusedElement.LostFocus -= OnUnrealizedFocusedElementLostFocus; - RecycleElement(_outOfViewportFocusedElement, _outOfViewportFocusedIndex); - _outOfViewportFocusedElement = null; - _outOfViewportFocusedIndex = -1; + _focusedElement.LostFocus -= OnUnrealizedFocusedElementLostFocus; + RecycleElement(_focusedElement, _focusedIndex); + _focusedElement = null; + _focusedIndex = -1; } /// From fc1261a9f12cb27de395c7f41ff14c2003986584 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Fri, 19 May 2023 15:51:31 +0200 Subject: [PATCH 5/7] Hack for ScrollGestureRecognizer. --- src/Avalonia.Controls/ListBoxItem.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/Avalonia.Controls/ListBoxItem.cs b/src/Avalonia.Controls/ListBoxItem.cs index 0a959c5666..987b9c5d2f 100644 --- a/src/Avalonia.Controls/ListBoxItem.cs +++ b/src/Avalonia.Controls/ListBoxItem.cs @@ -72,7 +72,10 @@ namespace Avalonia.Controls // Otherwise perform the selection when the pointer is released as to not // interfere with gestures. _pointerDownPoint = p.Position; - e.Handled = true; + + // Ideally we'd set handled here, but that would prevent the scroll gesture + // recognizer from working. + ////e.Handled = true; } } } From e78bd48a8e7d3f195c792ef88a766c216bd345a6 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Sat, 20 May 2023 15:08:38 +0200 Subject: [PATCH 6/7] Only select if pointer is released in bounds. --- src/Avalonia.Controls/ListBoxItem.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Avalonia.Controls/ListBoxItem.cs b/src/Avalonia.Controls/ListBoxItem.cs index 987b9c5d2f..c2044661e2 100644 --- a/src/Avalonia.Controls/ListBoxItem.cs +++ b/src/Avalonia.Controls/ListBoxItem.cs @@ -95,7 +95,8 @@ namespace Avalonia.Controls var tapRect = new Rect(_pointerDownPoint, new Size()) .Inflate(new Thickness(tapSize.Width, tapSize.Height)); - if (tapRect.ContainsExclusive(point.Position) && + if (new Rect(Bounds.Size).ContainsExclusive(point.Position) && + tapRect.ContainsExclusive(point.Position) && ItemsControl.ItemsControlFromItemContaner(this) is ListBox owner) { if (owner.UpdateSelectionFromPointerEvent(this, e)) From 52efb5dd8fd67da359c8f097bd97c30a1238c284 Mon Sep 17 00:00:00 2001 From: Max Katz Date: Mon, 22 May 2023 19:59:28 -0400 Subject: [PATCH 7/7] Fix merge build error --- src/Avalonia.Controls/Primitives/SelectingItemsControl.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs b/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs index 64ecb4321e..0e627c2a37 100644 --- a/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs +++ b/src/Avalonia.Controls/Primitives/SelectingItemsControl.cs @@ -704,8 +704,8 @@ namespace Avalonia.Controls.Primitives bool wrap = false, bool rangeModifier = false) { - var from = GetContainerFromEventSource(FocusManager.Instance?.Current) ?? - ContainerFromIndex(Selection.AnchorIndex); + var focused = FocusManager.GetFocusManager(this)?.GetFocusedElement(); + var from = GetContainerFromEventSource(focused) ?? ContainerFromIndex(Selection.AnchorIndex); return MoveSelection(from, direction, wrap, rangeModifier); }