Repository navigation
Follow Android's tab order in a ScrollView's clipped focus search - #59074
Draft
YevheniiKotyrlo wants to merge 2 commits into
Draft
YevheniiKotyrlo wants to merge 2 commits into
YevheniiKotyrlo wants to merge 2 commits into
Conversation
ReactScrollView.focusSearch hands FOCUS_FORWARD and FOCUS_BACKWARD to the shadow-tree search whenever the platform's next view lies outside the scroll view. That search took anything below or to the right as the next view, which is not Android's order: ViewGroup.addFocusables sorts each parent's children into rows by top edge, then by start edge, and walks the tree depth first. When the last view in Android's order had another focusable below or to its right, Tab went back to that view and the two alternated instead of leaving the scroll view. FocusOrderingHelper now lists the focusable views in Android's tab order, following the mounting layer's view flattening, and forward and backward take the neighbour in that list, or nothing past either end. RNTester gains a keyboard focus order example for ScrollView.
Contributor
Author
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
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:
When the last focusable view of a
ScrollViewin Android's tab order has another focusable below it or to its right, Tab moves back to that view instead of leaving the scroll view, and the two alternate for as long as Tab is pressed.ReactScrollView.focusSearchhandsFOCUS_FORWARDandFOCUS_BACKWARDto the shadow-tree search whenever the platform's next view lies outside the scroll view, so that clipped children can be reached. For those two directionsFocusOrderingHelpertook any focusable below or to the right of the focused one, which is not Android's order:ViewGroup.addFocusablessorts each parent's children into rows by top edge and then by start edge, and lists a parent before the views it hosts. A card whose action sits below its title, with a dismiss button at its top end, is ordered action then dismiss by Android; from dismiss, the search found the action below it.FocusOrderingHelper::collectFocusablesInTabOrdernow builds Android's order over the shadow tree: it takes each parent's children the way the mounting layer flattens them (sliceChildShadowNodeViewPairs) and sorts them likeFocusFinder.FocusSorter.findNextInTabOrdertakes the neighbour in that list, or nothing past either end, and thenfocusSearchkeeps the platform's answer. The geometric rules for the two relative directions go; the four absolute directions are unchanged. RNTester gains a ScrollView keyboard focus order example.#59052 keeps
no-hide-descendantssubtrees out of the directional search; whichever of the two lands second, I'll carry that rule into the tab order walk.Changelog:
[ANDROID] [FIXED] - Tab no longer alternates between two views at the end of a
ScrollViewinstead of leaving itTest Plan:
react/fabric/test/FocusOrderingHelperTabOrderTest.cpp: rows by top and then left edge, right-to-left rows, a row that grows while views overlap it, a view hosting its children ordered as one block, a flattened view's children joining its parent's rows, a focusable parent before its children, the card above, and both ends of the order. The repository's CMake build has no target for that directory, so I added one to ReactAndroid's x86_64 CMake build, linked againstlibreactnative.so, and ran it on an Android 16 emulator. WithfindNextInTabOrderanswering bymain'sisCandidateandisBetterCandidate, the card case fails: forward from dismiss gives the action. With this change all eight pass.mainfocus goes Dismiss, Action, Dismiss, Action, Dismiss; with this change it goes Dismiss, then the button after the scroll view, then the bottom navigation. Two rounds per build gave the same sequence. Images in the comment below.yarn flow-checkpasses; ESLint and Prettier are clean on the example and clang-format on the changed C++ files; the RNTester debug build compiles the native change.