react-native-windows icon indicating copy to clipboard operation
react-native-windows copied to clipboard

Use programmatic focus for UIManager.focus

Open rozele opened this issue 3 years ago • 5 comments

Description

Type of Change

Erase all that don't apply.

  • Bug fix (non-breaking change which fixes an issue)

Why

Switching from xaml::FocusState::Programmatic to xaml::FocusState::Keyboard causes focus rings to be shown for "default focus" scenarios in our app (e.g., defaulting a "Done" button in a modal), which are not intended to have focus rings (at least until the user explicitly changes focus with the Keyboard).

Resolves #10265

What

This change reverts to Programmatic focus in the UIManager.

Microsoft Reviewers: Open in CodeFlow

rozele avatar Jul 15 '22 20:07 rozele

Personally, I think using Keyboard focus should perhaps be a quirk setting if it's absolutely needed. But probably shouldn't be needed...

rozele avatar Jul 15 '22 20:07 rozele

This was changed from Programmatic to Keyboard in this change: https://github.com/microsoft/react-native-windows/commit/a8e7166c6b76d1457db12ac1504b164c0c4d3cf0#diff-e63b432a6fbc1ea988ae7f645810a9864e66544c6e455fa125adce119777393e In this PR: https://github.com/microsoft/react-native-windows/pull/6220 See this commentary: https://github.com/microsoft/react-native-windows/pull/6220#discussion_r506607481

What is the correct behavior?

chrisglein avatar Jul 18 '22 18:07 chrisglein

@chrisglein so for our app the correct behavior is Programmatic.

An even better solution would be to change the focus API in React Native to optionally pass an enum for the type of focus change.

In the near term, I think it's pretty rare for someone to imperatively change focus in reaction to a keyboard event (e.g., handling a Tab key event in JS and switching focus), perhaps somewhat less rare for some kind of directional focus component to respond to arrow keys in JS (though these should probably also use native XYNavigation APIs).

At least in our app, all of our keyboard based navigation is handled natively, and for any imperative focus (e.g., giving default focus to a button on a modal or popover) we do not want a focus ring displayed, so FocusKind::Programmatic is the correct behavior.

rozele avatar Jul 19 '22 14:07 rozele

I agree that programmatic seems like the right case for the default. The whole existence of programmatic is basically to attempt to handle the case where the callers doesn't know if it should be pointer or keyboard. Its not perfect by any means but its the platforms best guess at this point.

We should still look into adding an override that allows apps to specify which they want.

There is another parameter that is also something that we might want to look at adding at the same time. In multi-window apps there is also a case to be made for focus activating the window in addition to moving element focus within the window, dependent on use case. In the office-rn we have an additional parameter to the focus method to control that behavior - See: https://github.com/microsoft/react-native-windows/blob/main/packages/%40office-iss/react-native-win32/src/Libraries/Utilities/FocusManager.win32.js

acoates-ms avatar Jul 21 '22 20:07 acoates-ms

@acoates-ms shameless plug, I also think we need a quirk setting to default to polite or aggressive focus behavior :) #8393

IMO, polite focus behavior is a better default than aggressive because aggressive behavior can always be implemented imperatively by focusing the window first, but the inverse is not true (can't really implement polite behavior when aggressive is default).

rozele avatar Jul 21 '22 21:07 rozele