Improving FocusManager and keyboard navigation

[amwx]

Is your feature request related to a problem? Please describe. Handling focus and keyboard navigation in Avalonia is somewhat lacking and often (when porting controls from WinUI) leads to re-inventing the wheel to get a desired behavior. Some panels implement INavigableContainer to try to make it easier, and there's also ICustomKeyboardNavigation, but these feel more like temporary solutions. They also require you to create an entirely new control to get a desired behavior. It'd be a much better solution to implement a better FocusManager and port XYFocusKeyboardNavigation over to make most of this work out of the box. There's also other things like focus type isn't preserved when moving focus scopes (launch a popup with the keyboard, and focus moves as normal pointer/programmatic focus)

I've been reading the UWP docs on all this so I'm mostly raising this issue (besides starting an API spec) to see if there were plans to do this for 11.0 - as I don't want to start looking into this if there are plans already underway. Otherwise, or if nothings been started, I want to look to see how hard it would be and if successful I'll make a PR.

Looking at UWP/WinUI's API for this:

public class FocusManager
{
    // Find the first focusable element within a scope (a specific control, e.g., a grid) or null starts from root
    public static DependencyObject FindFirstFocusableElement(DependencyObject searchScope);

    // Find the last focusable element within a scope (a specific control, e.g., a grid) or null starts from root
    public static DependencyObject FindLastFocusableElement(DependencyObject searchScope);

    // Retrieves the element that should receive focus based on the specified navigation direction.
    public static DependencyObject FindNextElement(FocusNavigationDirection focusNavigationDirection);

    // Retrieves the element that should receive focus based on the specified navigation direction (cannot be used with tab navigation, see remarks).
    public static DependencyObject FindNextElement(FocusNavigationDirection focusNavigationDirection, FindNextElementOptions focusNavigationOptions);

    // Retrieves the element that should receive focus based on the specified navigation direction.
    public static UIElement FindNextFocusableElement(FocusNavigationDirection focusNavigationDirection);

    // Retrieves the element that should receive focus based on the specified navigation direction and hint rectangle.
    public static UIElement FindNextFocusableElement(FocusNavigationDirection focusNavigationDirection, Rect hintRect);

    // Retrieves the element in the UI that has focus.
    public static object GetFocusedElement();

    // Retrieves the focused element within the Xaml island container. (not needed)
    public static object GetFocusedElement(XamlRoot xamlRoot);

    // Asynchronously attempts to set focus on an element when the application is initialized.
    public static IAsyncOperation<FocusMovementResult> TryFocusAsync(DependencyObject element, FocusState value);

    // Attempts to change focus from the element with focus to the next focusable element in the specified direction.
    public static bool TryMoveFocus(FocusNavigationDirection focusNavigationDirection);

    // Attempts to change focus from the element with focus to the next focusable element in the specified direction, using the specified navigation options.
    public static bool TryMoveFocus(FocusNavigationDirection focusNavigationDirection, FindNextElementOptions focusNavigationOptions);

    // Asynchronously attempts to change focus from the current element with focus to the next focusable element in the specified direction.
    public static IAsyncOperation<FocusMovementResult> TryMoveFocusAsync(FocusNavigationDirection focusNavigationDirection);

    //Asynchronously attempts to change focus from the current element with focus to the next focusable element in the specified direction and subject to the specified navigation options.
    public static IAsyncOperation<FocusMovementResult> TryMoveFocusAsync(FocusNavigationDirection focusNavigationDirection, FindNextElementOptions focusNavigationOptions);

    // Occurs before an element actually receives focus. This event is raised synchronously to ensure focus isn't moved while the event is bubbling.
    public static event EventHandler<GettingFocusEventArgs> GettingFocus;

    // Occurs when an element within a container element (a focus scope) receives focus. This event is raised asynchronously, so focus might move before bubbling is complete.
    public static event EventHandler<FocusManagerGotFocusEventArgs> GotFocus;

    // Occurs before focus moves from the current element with focus to the target element. This event is raised synchronously to ensure focus isn't moved while the event is bubbling.
    public static event EventHandler<LosingFocusEventArgs> LosingFocus;

    // Occurs when an element within a container element (a focus scope) loses focus. This event is raised asynchronously, so focus might move again before bubbling is complete.
    public static event EventHandler<FocusManagerLostFocusEventArgs> LostFocus;
}

The events in FocusManager also exist in UIElement and are raised in the order, where UIElement bubbles up to the FocusManager. 1- UIElement.LosingFocus ->FocusManager.LosingFocus 2- UIElement.GettingFocus -> FocusManager.GettingFocus 3- UIElement.LostFocus (raised by element that lost focus) 4- FocusManager.LostFocus (raised even if UIElement.LostFocus is handled) 5- UIElement.GotFocus (raised by element that received focus) 6- FocusManager.GotFocus (raised even if UIElement.GotFocus is handled)

The duplicate events in UIElement and FocusManger are to handle the case where focus changes scope from main window to a popup, since a new visual tree is created for the popup and the event bubbling wouldn't properly propagate: https://docs.microsoft.com/en-us/uwp/api/windows.ui.xaml.input.focusmanager?view=winrt-22000

More on focus: https://github.com/MicrosoftDocs/windows-uwp/blob/docs/hub/apps/design/input/focus-navigation-programmatic.md

XYFocusKeyboardNavigation provides an easy way to activate navigation via the keyboard arrows (or gamepad in UWP) within a specified scope, for example a grid. It can be set to Auto: inherited from a parent control, Disabled: no arrow key 2D navigation, or Enabled: use arrow keys for 2D navigation.

<Grid XYFocusKeyboardNavigation="Enabled">
    <Container1 />
    <Button />
    <Button />
</Grid>

Different strategies exist and can be set for determining how focus moves within an app

• Auto: Indicates that navigation strategy is inherited from the element's ancestors. If all ancestors have a value of Auto, the fallback strategy is Projection.
• NavigationDirectionDistance: Indicates that focus moves to the element closest to the axis of the navigation direction.
• Projection: Indicates that focus moves to the first element encountered when projecting the edge of the currently focused element in the direction of navigation.
• RectilinearDistance: Indicates that focus moves to the closest element based on the shortest 2D distance (Manhattan metric).

And for more complex navigation scenarios, XYFocusLeft, XYFocusRight, XYFocusDown, and XYFocusUp exist to specify how XYFocus should move in the given navigation direction

More: https://docs.microsoft.com/en-us/windows/apps/design/input/gamepad-and-remote-interactions#xy-focus-navigation-and-interaction

[mysteryx93]

There are quire a few tickets regarding keyboard navigation problems, I'll link some of them here. Probably more tickets could be resolved at the same time.

https://github.com/AvaloniaUI/Avalonia/issues/7606 https://github.com/AvaloniaUI/Avalonia/issues/7610 https://github.com/AvaloniaUI/Avalonia/issues/7406

[maxkatz6]

Note, current keyboard navigation was ported from WPF. It has inconveniences, but should be enough most of the time (there are still problems like TreeView).

UWP has way more flexible API, so it would be nice to consider it.

[grokys]

Yep, makes a lot of sense to follow UWP here.

Note, current keyboard navigation was ported from WPF.

Not really, it's kind of a pile of hack upon hack starting from the very beginning of Avalonia (before WPF was open-sourced) and is well due a refactor!

[grokys]

Also: one thing that Avalonia has that UWP doesn't have is focus scopes. I'm not sure how necessary these are.

[amwx]

As I see it, focus scopes help in the case of multiple windows, which I think is how they're implemented now (really only on TopLevels), but probably not really necessary since you could just redefine the scope as the root visual/top level. IFocusScope doesn't have any members so it seems it was only added as a concept and never really expanded upon (though correct me if I'm wrong). UWP defines scopes as essentially container controls for which you want to confine focus searching to - so a Grid is a focus scope for all the visual descendants it holds. I'm not sure where having two completely separate focus scopes within the same window is needed, and the new APIs from UWP should make it easy enough to control focus navigation in different areas of an app anyway

[amwx]

Few more questions: 1- WinUI doesn't have the same values of KeyboardNavigationMode as WPF/Avalonia, only defining Cycle, Local, and Once

WinUI Cycle -> behaves the same WinUI Local -> behaves the same as Avalonia Continue (usually) WinUI Once -> pretty much the same as well.

Contained isn't present - but its similar to cycle except it doesn't wrap around, and I think we'd be ok to drop that. IMO it's better to cycle/wrap around than deadlock keyboard nav at the end of a container. None is also not used, and can be fakes setting AllowFocusOnInteraction to false, but we don't have the property (closest is Focusable). So I think keeping that would also work.

2- Should KeyboardNavigation.TabNavigation be moved from an attached property to a normal StyledProperty on InputElement? Seems TabIndex and IsTabStop were moved already, so this would eliminate the attached property, and make it more consistent. Not sure what to do about TabOnceActiveElement as that would be the last remaining property in KeyboardNavigation (WinUI also doesn't have this property). WinUI also changed the name to TabFocusNavigation, should we do the same or leave it?

EDIT: Another open question- would it be better do this after the core libraries are merged in #5831, given there may be structural changes/additions to Avalonia.Input to handle the UWP style focus classes. Just trying to make sure this change doesn't generate any headaches.

[amwx]

Update: I am currently working on porting what I can of the WinUI focus manager. Since the code isn't open-sourced yet, I'm adapting the WinUI API, but recycling the existing logic that was ported from WPF - which actually seems to work well, so the WPF logic isn't a million miles away from what UWP uses - at least from my early tests implementing this. But I think the modern API will help solidify the focus APIs and make things easier moving forward - and clean up the scattered logic that currently exists.

My current plan is to get the initial FocusManager changes in and then do XYFocus as a separate PR. The FocusManager changes are actually quite significant and XYFocus is complex, so I think it's best to separate them.

First PR (which hopefully I'll get a PR started within the next day or so) will target:

• Updating the FocusManager API to follow UWP, as laid out in the OP. This will focus *only* on tab navigation, directional changes relate to XYFocus. This is the most significant change.
• Updating InputElement to include new properties and events needed (will include XYFocus properties now)
• Ensuring focus state is preserved when changing scopes (window -> popup & back, etc.)
• Ensuring focus properly moves when changing scopes (focusing popups, focusing other windows, window/top-level deactivation, etc)

From my above comment (now striked-out): As of now, I'm going to keep the existing values of KeyboardNavigationMode even though WinUI doesn't have all of them - since the tab logic is recycled from WPF. I also still think TabNavigation should be moved to InputElement and not be an AttachedProperty, but will leave that alone as well.