Repository navigation
Let Android keyboard focus reach a clipped direct child of a ScrollView - #59073
Draft
YevheniiKotyrlo wants to merge 2 commits into
Draft
YevheniiKotyrlo wants to merge 2 commits into
YevheniiKotyrlo wants to merge 2 commits into
Conversation
getRelativeAncestorList returned null whenever the next focusable node had fewer than two ancestors below the root, which is every direct child of a ScrollView's content container. findNextFocusableView reads null as "no next view", so with removeClippedSubviews keyboard focus left the scroll view instead of moving to a clipped direct child. Null now means only that the child is not under the root, and a direct child gets an empty array. The tag walk moves into FocusOrderingHelper::getRelativeAncestorTags, covered by a gtest, and the binding's Kotlin declaration returns IntArray? to match the null it already returned. RNTester's removeClippedSubviews example makes its rows focusable, so Tab can be tried with and without clipping.
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:
With
removeClippedSubviews, Tab on a hardware keyboard skips the focusable direct children of aScrollView's content once they are clipped: focus leaves the scroll view instead.ReactScrollView.focusSearchasksReactScrollViewHelper.findNextFocusableViewfor the clipped target, which un-clips it together with the ancestorsgetRelativeAncestorListreturns. That method answersnullwhenever the ancestor list holds fewer than two entries, and a direct child's list holds exactly one, the content container, so the search gives up. The guard comes fromfindRelativeTopMostParent, where a direct child had no parent to return; the list form has an empty answer for it.getRelativeAncestorListnow returnsnullonly when the child does not descend from the root, and an empty array for a direct child. The tag walk moves intoFocusOrderingHelper::getRelativeAncestorTagsso a test can reach it without aJavaVM; for the same reasonFocusOrderingHelper.hdrops its include ofFabricUIManagerBinding.h, which it does not use, and gains#pragma once. The binding's Kotlin declaration returnsIntArray?, matching thenullit already returned. RNTester's removeClippedSubviews example makes its rows focusable so Tab can be tried with and without clipping.Changelog:
[ANDROID] [FIXED] - Keyboard focus reaches a clipped direct child of a
ScrollViewwithremoveClippedSubviewsTest Plan:
react/fabric/test/FocusOrderingHelperTest.cpp: a direct child gets an empty list, a nested child its intermediate ancestors outermost first, and a node outside the root or the root itself gets none. 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: with the guard as it is onmainthe direct-child case fails (has_value()is false); with this change all four pass.removeClippedSubviews: true, Tab from Item 0 on an Android 16 emulator: onmainfocus goes through Item 4, the last row still attached, and then to the toggle below the list; with this change it goes on through Item 11, scrolling each clipped row into view, and then to the toggle. Two rounds per build gave the same sequence. Images in the comment below.yarn lintandyarn flow-checkpass; prettier, clang-format and ktfmt are clean on the changed files; the RNTester debug build compiles the Kotlin and native changes.