Repository navigation
Keep focusable={false} Android text out of keyboard navigation - #59139
Draft
YevheniiKotyrlo wants to merge 3 commits into
Draft
YevheniiKotyrlo wants to merge 3 commits into
YevheniiKotyrlo wants to merge 3 commits into
Conversation
An accessible view keeps isFocusable for accessibility services, so
setFocusable(false) left it in Tab and arrow-key navigation: every
Pressable is accessible, and tabIndex={-1} or focusable={false} on one
changed nothing about where a hardware keyboard moved focus.
ReactViewGroup now records the prop and skips the view in addFocusables
and requestFocus, the two ways keyboard navigation reaches it, while its
descendants, accessibility services and requestFocusFromJS still do.
When the next focus lies outside a scroll view, ReactScrollView and its
horizontal and nested siblings ask the shadow tree for the next
focusable element inside it, and that search offers every focusable or
accessible node. A view focusable={false} keeps out of keyboard
navigation is still accessible, so the search handed it back, the view
refused focus and Tab stopped moving. findNextFocusableView now moves
on from a candidate whose addFocusables adds nothing, and ends at a
candidate it already offered, since forward candidates can lead back to
each other. ReactViewGroup's addFocusables and requestFocus overrides
are added to ReactAndroid.api.
A Text with onPress or onLongPress is accessible, so setAccessible made it focusable, and Tab, Shift+Tab and the arrow keys of a hardware keyboard stopped on it. Text had no focusable prop to take it out, so a pressable label between two inputs was always an extra Tab stop. The text view managers now take focusable, and ReactTextView and PreparedLayoutTextView skip an excluded text in addFocusables and requestFocus while isFocusable stays with accessible, so accessibility services still reach it. A text that holds focus stays a candidate, an inline view inside one stays reachable, and a touch still focuses a selectable text, which text selection needs. TextProps and the legacy Text types declare the prop; the base view config already sends it.
meta-cla
Bot
added
the
CLA Signed
Contributor
Author
Device evidence — before / afterUNFIXED — from touch mode, the first Tab focuses the First box and the second lands on the pressable FIXED — the second Tab passes over the text to the Last box.
Screen recordings of RNTester debug builds of #59048's head and of this PR's head on an Android 16 (API 36) emulator, 1140×2400 at density 480, with Tab pressed twice from touch mode. The scene is two |
|
Warning JavaScript API change detected This PR commits an update to
This change was flagged as: |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary:
On Android, a
TextwithonPressoronLongPressisaccessibleunless told otherwise, andsetAccessiblemakes it focusable, so Tab, Shift+Tab and the arrow keys of a hardware keyboard stop on it.Texthas nofocusableprop to take it out, so a pressable label above an input is always an extra Tab stop. #34737 asks for keyboard focus to be specified separately from screen-reader focus.This gives
Textthefocusableprop with the meaning #59048 givesfocusable={false}onView.ReactTextViewandPreparedLayoutTextViewrecord it and skip an excluded text at the two points keyboard navigation reaches it —addFocusables, where an inline view inside the text stays a candidate, andrequestFocus— whileisFocusablestays withaccessible, so accessibility services still treat the text as one element and activate it. A text that holds focus stays a candidate, so focus search can move on from it. In touch moderequestFocusstill succeeds, because aselectabletext takes focus from the long press that starts a selection. The base view config already sendsfocusableto the native text component;TextPropsand the legacy types now declare it.This is stacked on #59048, whose change to
ReactScrollViewHelper.findNextFocusableViewpasses over a scroll-view candidate that takes no keyboard focus; only the last commit is new.Changelog:
[ANDROID] [ADDED] -
Textacceptsfocusable={false}, which takes an accessible text out of hardware-keyboard navigation while accessibility services still reach itTest Plan:
./gradlew :packages:react-native:ReactAndroid:testDebugUnitTest --tests "com.facebook.react.views.text.ReactTextViewKeyboardFocusTest" --tests "com.facebook.react.views.text.PreparedLayoutTextViewKeyboardFocusTest" -Preact.internal.useHermesStable=true: with the text view sources from the base, 5 of the 14 new tests fail —addFocusablesreturns the excluded text (Expecting actual: [View, ReactTextView, View] to contain exactly (and in same order): [View, View], the same forPreparedLayoutTextViewand beside an inline view) andrequestFocus(FOCUS_DOWN)gives it focus. With the change all 14 pass, including Tab and Shift+Tab from a focused excluded text,focusable={true}and a removed prop returning the text, recycling, and a touch still focusing an excludedselectabletext../gradlew :packages:react-native:ReactAndroid:testDebugUnitTest :packages:react-native:ReactAndroid:compileReleaseKotlin :packages:react-native:ReactAndroid:compileReleaseJavaWithJavac -PenableWarningsAsErrors=true -Preact.internal.useHermesStable=true: 782 tests, 0 failures, and the release sources compile.yarn flow-check,yarn lint,yarn test-typescript-legacy,yarn build-typesandyarn test-generated-typescriptpass,ReactNativeApi.d.tsis regenerated, and ktfmt and google-java-format pass on the changed files.Textmarkedfocusable={false}without the change and passes over it with the change, in a plain view and in a scroll view.Text, where it stopped on each label before.