Skip to content

Commit

Permalink
Fix some issues with tabbing into virtualized list (#13826)
Browse files Browse the repository at this point in the history
* Add failing unit test for scenario 1 in #11878.

* Set TabOnceActiveElement on realized container.

Fixes scenario 1 in #11878.

* Use TabOnceActiveElement to decide focused element.

Fixes scenario #3 in #11878.
  • Loading branch information
grokys authored and maxkatz6 committed Dec 5, 2023
1 parent 4e85d0a commit cd8ab64
Show file tree
Hide file tree
Showing 7 changed files with 65 additions and 12 deletions.
11 changes: 11 additions & 0 deletions src/Avalonia.Controls/ItemsControl.cs
Original file line number Diff line number Diff line change
Expand Up @@ -515,6 +515,17 @@ protected override void OnApplyTemplate(TemplateAppliedEventArgs e)
_itemsPresenter = e.NameScope.Find<ItemsPresenter>("PART_ItemsPresenter");
}

protected override void OnGotFocus(GotFocusEventArgs e)
{
base.OnGotFocus(e);

// If the focus is coming from a child control, set the tab once active element to
// the focused control. This ensures that tabbing back into the control will focus
// the last focused control when TabNavigationMode == Once.
if (e.Source != this && e.Source is IInputElement ie)
KeyboardNavigation.SetTabOnceActiveElement(this, ie);
}

/// <summary>
/// Handles directional navigation within the <see cref="ItemsControl"/>.
/// </summary>
Expand Down
3 changes: 3 additions & 0 deletions src/Avalonia.Controls/Primitives/SelectingItemsControl.cs
Original file line number Diff line number Diff line change
Expand Up @@ -513,6 +513,9 @@ protected internal override void ContainerForItemPreparedOverride(Control contai
var containerIsSelected = GetIsSelected(container);
UpdateSelection(index, containerIsSelected, toggleModifier: true);
}

if (Selection.AnchorIndex == index)
KeyboardNavigation.SetTabOnceActiveElement(this, container);
}

/// <inheritdoc />
Expand Down
34 changes: 23 additions & 11 deletions src/Avalonia.Controls/VirtualizingStackPanel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,16 @@ protected override void OnItemsChanged(IReadOnlyList<object?> items, NotifyColle
}
}

protected override void OnItemsControlChanged(ItemsControl? oldValue)
{
base.OnItemsControlChanged(oldValue);

if (oldValue is not null)
oldValue.PropertyChanged -= OnItemsControlPropertyChanged;
if (ItemsControl is not null)
ItemsControl.PropertyChanged += OnItemsControlPropertyChanged;
}

protected override IInputElement? GetControl(NavigationDirection direction, IInputElement? from, bool wrap)
{
var count = Items.Count;
Expand Down Expand Up @@ -379,7 +389,7 @@ protected internal override int IndexFromContainer(Control container)
var scrollToElement = GetOrCreateElement(items, index);
scrollToElement.Measure(Size.Infinity);

// Get the expected position of the elment and put it in place.
// Get the expected position of the element 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) :
Expand Down Expand Up @@ -689,6 +699,7 @@ private Control CreateElement(object? item, int index, object? recycleKey)

private void RecycleElement(Control element, int index)
{
Debug.Assert(ItemsControl is not null);
Debug.Assert(ItemContainerGenerator is not null);

_scrollViewer?.UnregisterAnchorCandidate(element);
Expand All @@ -703,11 +714,10 @@ private void RecycleElement(Control element, int index)
{
element.IsVisible = false;
}
else if (element.IsKeyboardFocusWithin)
else if (KeyboardNavigation.GetTabOnceActiveElement(ItemsControl) == element)
{
_focusedElement = element;
_focusedIndex = index;
_focusedElement.LostFocus += OnUnrealizedFocusedElementLostFocus;
}
else
{
Expand Down Expand Up @@ -774,15 +784,17 @@ private void OnEffectiveViewportChanged(object? sender, EffectiveViewportChanged
}
}

private void OnUnrealizedFocusedElementLostFocus(object? sender, RoutedEventArgs e)
private void OnItemsControlPropertyChanged(object? sender, AvaloniaPropertyChangedEventArgs e)
{
if (_focusedElement is null || sender != _focusedElement)
return;

_focusedElement.LostFocus -= OnUnrealizedFocusedElementLostFocus;
RecycleElement(_focusedElement, _focusedIndex);
_focusedElement = null;
_focusedIndex = -1;
if (_focusedElement is not null &&
e.Property == KeyboardNavigation.TabOnceActiveElementProperty &&
e.GetOldValue<IInputElement?>() == _focusedElement)
{
// TabOnceActiveElement has moved away from _focusedElement so we can recycle it.
RecycleElement(_focusedElement, _focusedIndex);
_focusedElement = null;
_focusedIndex = -1;
}
}

/// <inheritdoc/>
Expand Down
3 changes: 3 additions & 0 deletions tests/Avalonia.Controls.UnitTests/ListBoxTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,9 @@ public void Selection_Should_Be_Cleared_On_Recycled_Items()
Assert.Equal(10, target.Presenter.Panel.Children.Count);
Assert.True(((ListBoxItem)target.Presenter.Panel.Children[0]).IsSelected);

// The selected item must not be the anchor, otherwise it won't get recycled.
target.Selection.AnchorIndex = -1;

// Scroll down a page.
target.Scroll.Offset = new Vector(0, 10);
Layout(target);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1224,6 +1224,25 @@ public void Nested_ListBox_Does_Not_Change_Parent_SelectedIndex()
Assert.Equal(0, root.SelectedIndex);
}

[Fact]
public void TabOnceActiveElement_Should_Be_Initialized_With_SelectedItem()
{
using (UnitTestApplication.Start(TestServices.MockPlatformRenderInterface))
{
var target = new ListBox
{
Template = Template(),
ItemsSource = new[] { "Foo", "Bar", "Baz " },
SelectedIndex = 1,
};

Prepare(target);

var container = target.ContainerFromIndex(1)!;
Assert.Same(container, KeyboardNavigation.GetTabOnceActiveElement(target));
}
}

[Fact]
public void Setting_SelectedItem_With_Pointer_Should_Set_TabOnceActiveElement()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1110,7 +1110,7 @@ public void Selection_Is_Not_Cleared_On_Recycling_Containers()
Assert.Equal(19, panel.LastRealizedIndex);

// The selection should be preserved.
Assert.Empty(SelectedContainers(target));
Assert.Equal(new[] { 1 }, SelectedContainers(target));
Assert.Equal(1, target.SelectedIndex);
Assert.Same(items[1], target.SelectedItem);
Assert.Equal(new[] { 1 }, target.Selection.SelectedIndexes);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
using Avalonia.Controls.Presenters;
using Avalonia.Controls.Templates;
using Avalonia.Data;
using Avalonia.Input;
using Avalonia.Layout;
using Avalonia.Media;
using Avalonia.Styling;
Expand Down Expand Up @@ -314,15 +315,19 @@ public void Removing_Item_Of_Focused_Element_Clears_Focus()
{
using var app = App();
var (target, scroll, itemsControl) = CreateTarget();
var items = (IList)itemsControl.ItemsSource!;

var focused = target.GetRealizedElements().First()!;
focused.Focusable = true;
focused.Focus();
Assert.True(focused.IsKeyboardFocusWithin);
Assert.Equal(focused, KeyboardNavigation.GetTabOnceActiveElement(itemsControl));

scroll.Offset = new Vector(0, 200);
Layout(target);

items.RemoveAt(0);

Assert.All(target.GetRealizedElements(), x => Assert.False(x!.IsKeyboardFocusWithin));
Assert.All(target.GetRealizedElements(), x => Assert.NotSame(focused, x));
}
Expand Down

0 comments on commit cd8ab64

Please sign in to comment.