Skip to content

Commit 76c2184

Browse files
LLUhlinLLUhlin
committed
Fix keyboard focus skipping id-less views inside ScrollView on Android
focusSearch kept Android's result only if findViewById(nextFocus.id) found it, so focusable views without an id, such as the AndroidComposeView inside a ComposeView, were replaced by the shadow tree search and skipped by Tab and Shift+Tab. Check whether the view is inside the scroll view by walking its parents instead.
1 parent b8ec87e commit 76c2184

5 files changed

Lines changed: 96 additions & 4 deletions

File tree

‎packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactHorizontalScrollView.kt‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ import com.facebook.react.views.scroll.ReactScrollViewHelper.SNAP_ALIGNMENT_DISA
6262
import com.facebook.react.views.scroll.ReactScrollViewHelper.SNAP_ALIGNMENT_END
6363
import com.facebook.react.views.scroll.ReactScrollViewHelper.SNAP_ALIGNMENT_START
6464
import com.facebook.react.views.scroll.ReactScrollViewHelper.findNextFocusableView
65+
import com.facebook.react.views.scroll.ReactScrollViewHelper.isSelfOrDescendant
6566
import com.facebook.react.views.view.ImportantForInteractionHelper
6667
import com.facebook.systrace.Systrace
6768
import kotlin.math.abs
@@ -912,7 +913,7 @@ constructor(context: Context, private val fpsListener: FpsListener? = null) :
912913
if (ReactNativeFeatureFlags.enableCustomFocusSearchOnClippedElementsAndroid()) {
913914
// If we can find the next focus and it is a child of this view, return it, else it means we
914915
// are leaving the scroll view and we should try to find a clipped element
915-
if (nextFocus != null && findViewById<View>(nextFocus.id) != null) {
916+
if (nextFocus != null && isSelfOrDescendant(this, nextFocus)) {
916917
return nextFocus
917918
}
918919
val nextFocusableView = findNextFocusableView(this, focused, direction)

‎packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactNestedScrollView.kt‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
* This source code is licensed under the MIT license found in the
55
* LICENSE file in the root directory of this source tree.
66
*
7-
* @generated SignedSource<<37e2e57a137f899bb94e78bd265de562>>
7+
* @generated SignedSource<<3cd05abbef2823a5f07515fe17ce394c>>
88
*/
99

1010
/**
@@ -64,6 +64,7 @@ import com.facebook.react.views.scroll.ReactScrollViewHelper.SNAP_ALIGNMENT_DISA
6464
import com.facebook.react.views.scroll.ReactScrollViewHelper.SNAP_ALIGNMENT_END
6565
import com.facebook.react.views.scroll.ReactScrollViewHelper.SNAP_ALIGNMENT_START
6666
import com.facebook.react.views.scroll.ReactScrollViewHelper.findNextFocusableView
67+
import com.facebook.react.views.scroll.ReactScrollViewHelper.isSelfOrDescendant
6768
import com.facebook.react.views.view.ImportantForInteractionHelper
6869
import com.facebook.systrace.Systrace
6970
import kotlin.math.abs
@@ -465,7 +466,7 @@ constructor(context: Context, private val fpsListener: FpsListener? = null) :
465466
val nextFocus = super.focusSearch(focused, direction)
466467

467468
if (ReactNativeFeatureFlags.enableCustomFocusSearchOnClippedElementsAndroid()) {
468-
if (nextFocus != null && findViewById<View>(nextFocus.id) != null) {
469+
if (nextFocus != null && isSelfOrDescendant(this, nextFocus)) {
469470
return nextFocus
470471
}
471472
val nextFocusableView = findNextFocusableView(this, focused, direction)

‎packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactScrollView.kt‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ import com.facebook.react.views.scroll.ReactScrollViewHelper.SNAP_ALIGNMENT_DISA
5656
import com.facebook.react.views.scroll.ReactScrollViewHelper.SNAP_ALIGNMENT_END
5757
import com.facebook.react.views.scroll.ReactScrollViewHelper.SNAP_ALIGNMENT_START
5858
import com.facebook.react.views.scroll.ReactScrollViewHelper.findNextFocusableView
59+
import com.facebook.react.views.scroll.ReactScrollViewHelper.isSelfOrDescendant
5960
import com.facebook.react.views.view.ImportantForInteractionHelper
6061
import com.facebook.systrace.Systrace
6162
import kotlin.math.abs
@@ -457,7 +458,7 @@ constructor(context: Context, private val fpsListener: FpsListener? = null) :
457458
val nextFocus = super.focusSearch(focused, direction)
458459

459460
if (ReactNativeFeatureFlags.enableCustomFocusSearchOnClippedElementsAndroid()) {
460-
if (nextFocus != null && findViewById<View>(nextFocus.id) != null) {
461+
if (nextFocus != null && isSelfOrDescendant(this, nextFocus)) {
461462
return nextFocus
462463
}
463464
val nextFocusableView = findNextFocusableView(this, focused, direction)

‎packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactScrollViewHelper.kt‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -597,6 +597,23 @@ public object ReactScrollViewHelper {
597597
return host.findViewById(nextFocusableViewId)
598598
}
599599

600+
/**
601+
* Returns whether [view] is [host] or one of its descendants. Unlike looking the view up by id,
602+
* this also holds for views without an id, such as the `AndroidComposeView` that a `ComposeView`
603+
* adds as its child.
604+
*/
605+
@JvmStatic
606+
internal fun isSelfOrDescendant(host: ViewGroup, view: View): Boolean {
607+
var current: View? = view
608+
while (current != null) {
609+
if (current === host) {
610+
return true
611+
}
612+
current = current.parent as? View
613+
}
614+
return false
615+
}
616+
600617
@JvmStatic
601618
public fun resolveAbsoluteDirection(
602619
@FocusRealDirection direction: Int,

‎packages/react-native/ReactAndroid/src/test/java/com/facebook/react/views/scroll/ReactScrollViewHelperFocusTest.kt‎

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
/*
2+
* Copyright (c) Meta Platforms, Inc. and affiliates.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*/
7+
8+
package com.facebook.react.views.scroll
9+
10+
import android.view.View
11+
import android.widget.FrameLayout
12+
import org.assertj.core.api.Assertions.assertThat
13+
import org.junit.Before
14+
import org.junit.Test
15+
import org.junit.runner.RunWith
16+
import org.robolectric.RobolectricTestRunner
17+
import org.robolectric.RuntimeEnvironment
18+
19+
/** Verifies the containment check used by the scroll views' keyboard focus search. */
20+
@RunWith(RobolectricTestRunner::class)
21+
class ReactScrollViewHelperFocusTest {
22+
23+
private lateinit var host: FrameLayout
24+
private lateinit var content: FrameLayout
25+
26+
@Before
27+
fun setup() {
28+
val context = RuntimeEnvironment.getApplication()
29+
host = FrameLayout(context)
30+
content = FrameLayout(context)
31+
host.addView(content)
32+
}
33+
34+
@Test
35+
fun hostItselfIsIncluded() {
36+
assertThat(ReactScrollViewHelper.isSelfOrDescendant(host, host)).isTrue()
37+
}
38+
39+
@Test
40+
fun descendantWithIdIsIncluded() {
41+
val child = View(host.context).apply { id = 42 }
42+
content.addView(child)
43+
44+
assertThat(ReactScrollViewHelper.isSelfOrDescendant(host, child)).isTrue()
45+
}
46+
47+
@Test
48+
fun descendantWithoutIdIsIncluded() {
49+
// For example the AndroidComposeView that a ComposeView adds as its child.
50+
val child = View(host.context)
51+
content.addView(child)
52+
53+
assertThat(child.id).isEqualTo(View.NO_ID)
54+
assertThat(ReactScrollViewHelper.isSelfOrDescendant(host, child)).isTrue()
55+
}
56+
57+
@Test
58+
fun viewOutsideHostIsExcludedEvenIfIdMatchesADescendant() {
59+
content.addView(View(host.context).apply { id = 42 })
60+
val parent = FrameLayout(host.context)
61+
parent.addView(host)
62+
val outside = View(host.context).apply { id = 42 }
63+
parent.addView(outside)
64+
65+
assertThat(ReactScrollViewHelper.isSelfOrDescendant(host, outside)).isFalse()
66+
}
67+
68+
@Test
69+
fun detachedViewIsExcluded() {
70+
assertThat(ReactScrollViewHelper.isSelfOrDescendant(host, View(host.context))).isFalse()
71+
}
72+
}

0 commit comments

Comments
 (0)