From fc9f2fe0ea20d6e73be8fdf4eb47affaddea6427 Mon Sep 17 00:00:00 2001 From: Jorge Cabiedes Acosta Date: Mon, 14 Apr 2025 18:08:36 -0700 Subject: [PATCH] Fix keyboard navigation for FlatList with `removeClippedSubviews` enabled (#50105) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/50105 Pull Request resolved: https://github.com/facebook/react-native/pull/49543 When using `ReactScrollView` or `ReactHorizontalScrollView` Views with `removeClippedSubviews` keyboard navigation didn't work. This is because keyboard navigation relies on Android's View hierarchy to find the next focusable element. With `removeClippedSubviews` the next View might've been removed from the hierarchy. With this change we delegate the job of figuring out the next focusable element to the Shadow Tree, which will always contain layout information of the next element of the ScrollView. We then prevent the clipping of the topmost parent of the next focusable view to lay out the entire containing element in case we have some necessary context in the parent Changelog: [Android][Fixed] - Fix keyboard navigation on lists with `removeClippedSubviews` enabled Reviewed By: joevilches Differential Revision: D71324219 fbshipit-source-id: b55b7735a30714b2a5e1c9e0ed4ae84ab43f6694 --- .../ReactAndroid/api/ReactAndroid.api | 10 +++ .../react/fabric/FabricUIManager.java | 47 ++++++++++++++ .../react/uimanager/ReactClippingViewGroup.kt | 2 + .../scroll/ReactHorizontalScrollView.java | 21 +++++- .../react/views/scroll/ReactScrollView.java | 22 ++++++- .../views/scroll/ReactScrollViewHelper.kt | 65 +++++++++++++++++++ .../react/views/view/ReactViewGroup.java | 32 +++++++-- 7 files changed, 192 insertions(+), 7 deletions(-) diff --git a/packages/react-native/ReactAndroid/api/ReactAndroid.api b/packages/react-native/ReactAndroid/api/ReactAndroid.api index a9ab77950e8..f0b6d0b3a3e 100644 --- a/packages/react-native/ReactAndroid/api/ReactAndroid.api +++ b/packages/react-native/ReactAndroid/api/ReactAndroid.api @@ -2343,6 +2343,8 @@ public class com/facebook/react/fabric/FabricUIManager : com/facebook/react/brid public fun dispatchCommand (IILcom/facebook/react/bridge/ReadableArray;)V public fun dispatchCommand (IILjava/lang/String;Lcom/facebook/react/bridge/ReadableArray;)V public fun dispatchCommand (ILjava/lang/String;Lcom/facebook/react/bridge/ReadableArray;)V + public fun findNextFocusableElement (III)Ljava/lang/Integer; + public fun findRelativeTopMostParent (II)Ljava/lang/Integer; public fun getColor (I[Ljava/lang/String;)I public fun getEventDispatcher ()Lcom/facebook/react/uimanager/events/EventDispatcher; public fun getPerformanceCounters ()Ljava/util/Map; @@ -3982,6 +3984,7 @@ public abstract interface class com/facebook/react/uimanager/ReactClippingViewGr public abstract fun getRemoveClippedSubviews ()Z public abstract fun setRemoveClippedSubviews (Z)V public abstract fun updateClippingRect ()V + public abstract fun updateClippingRect (Ljava/util/Set;)V } public class com/facebook/react/uimanager/ReactClippingViewGroupHelper { @@ -5899,6 +5902,7 @@ public class com/facebook/react/views/scroll/ReactHorizontalScrollView : android public fun executeKeyEvent (Landroid/view/KeyEvent;)Z public fun flashScrollIndicators ()V public fun fling (I)V + public fun focusSearch (Landroid/view/View;I)Landroid/view/View; public fun getChildVisibleRect (Landroid/view/View;Landroid/graphics/Rect;Landroid/graphics/Point;)Z public fun getClippingRect (Landroid/graphics/Rect;)V public fun getFlingAnimator ()Landroid/animation/ValueAnimator; @@ -5961,6 +5965,7 @@ public class com/facebook/react/views/scroll/ReactHorizontalScrollView : android public fun setStateWrapper (Lcom/facebook/react/uimanager/StateWrapper;)V public fun startFlingAnimator (II)V public fun updateClippingRect ()V + public fun updateClippingRect (Ljava/util/Set;)V } public class com/facebook/react/views/scroll/ReactHorizontalScrollViewManager : com/facebook/react/uimanager/ViewGroupManager, com/facebook/react/views/scroll/ReactScrollViewCommandHelper$ScrollCommandHandler { @@ -6021,6 +6026,7 @@ public class com/facebook/react/views/scroll/ReactScrollView : android/widget/Sc public fun executeKeyEvent (Landroid/view/KeyEvent;)Z public fun flashScrollIndicators ()V public fun fling (I)V + public fun focusSearch (Landroid/view/View;I)Landroid/view/View; public fun getChildVisibleRect (Landroid/view/View;Landroid/graphics/Rect;Landroid/graphics/Point;)Z public fun getClippingRect (Landroid/graphics/Rect;)V public fun getFlingAnimator ()Landroid/animation/ValueAnimator; @@ -6084,6 +6090,7 @@ public class com/facebook/react/views/scroll/ReactScrollView : android/widget/Sc public fun setStateWrapper (Lcom/facebook/react/uimanager/StateWrapper;)V public fun startFlingAnimator (II)V public fun updateClippingRect ()V + public fun updateClippingRect (Ljava/util/Set;)V } public final class com/facebook/react/views/scroll/ReactScrollViewCommandHelper { @@ -6141,6 +6148,7 @@ public final class com/facebook/react/views/scroll/ReactScrollViewHelper { public static final fun emitScrollEvent (Landroid/view/ViewGroup;FF)V public static final fun emitScrollMomentumBeginEvent (Landroid/view/ViewGroup;II)V public static final fun emitScrollMomentumEndEvent (Landroid/view/ViewGroup;)V + public static final fun findNextFocusableView (Landroid/view/ViewGroup;Landroid/view/View;IZ)Landroid/view/View; public static final fun forceUpdateState (Landroid/view/ViewGroup;)V public static final fun getDefaultScrollAnimationDuration (Landroid/content/Context;)I public static final fun getNextFlingStartValue (Landroid/view/ViewGroup;III)I @@ -6150,6 +6158,7 @@ public final class com/facebook/react/views/scroll/ReactScrollViewHelper { public final fun registerFlingAnimator (Landroid/view/ViewGroup;)V public static final fun removeLayoutChangeListener (Lcom/facebook/react/views/scroll/ReactScrollViewHelper$LayoutChangeListener;)V public static final fun removeScrollListener (Lcom/facebook/react/views/scroll/ReactScrollViewHelper$ScrollListener;)V + public static final fun resolveAbsoluteDirection (IZI)I public static final fun smoothScrollTo (Landroid/view/ViewGroup;II)V public static final fun updateFabricScrollState (Landroid/view/ViewGroup;)V public final fun updateFabricScrollState (Landroid/view/ViewGroup;II)V @@ -6937,6 +6946,7 @@ public class com/facebook/react/views/view/ReactViewGroup : android/view/ViewGro public fun setRemoveClippedSubviews (Z)V public fun setTranslucentBackgroundDrawable (Landroid/graphics/drawable/Drawable;)V public fun updateClippingRect ()V + public fun updateClippingRect (Ljava/util/Set;)V public fun updateDrawingOrder ()V } diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java index 8e0c683e158..2dd7615a067 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java @@ -27,6 +27,7 @@ import android.view.accessibility.AccessibilityEvent; import androidx.annotation.AnyThread; import androidx.annotation.Nullable; import androidx.annotation.UiThread; +import androidx.core.view.ViewCompat.FocusRealDirection; import com.facebook.common.logging.FLog; import com.facebook.infer.annotation.Assertions; import com.facebook.infer.annotation.Nullsafe; @@ -262,6 +263,52 @@ public class FabricUIManager return rootTag; } + /** + * Find the next focusable element's id and position relative to the parent from the shadow tree + * based on the current focusable element and the direction. + * + * @return A NextFocusableNode object where the 'id' is the reactId/Tag of the next focusable + * view, returns null if no view could be found + */ + public @Nullable Integer findNextFocusableElement( + int parentTag, int focusedTag, @FocusRealDirection int direction) { + if (mBinding == null) { + return null; + } + + int generalizedDirection; + + switch (direction) { + case View.FOCUS_DOWN: + generalizedDirection = 0; + break; + case View.FOCUS_UP: + generalizedDirection = 1; + break; + case View.FOCUS_RIGHT: + generalizedDirection = 2; + break; + case View.FOCUS_LEFT: + generalizedDirection = 3; + break; + default: + return null; + } + + int serializedNextFocusableNodeMetrics = + mBinding.findNextFocusableElement(parentTag, focusedTag, generalizedDirection); + + if (serializedNextFocusableNodeMetrics == -1) { + return null; + } + + return serializedNextFocusableNodeMetrics; + } + + public @Nullable Integer findRelativeTopMostParent(int rootTag, int childTag) { + return mBinding != null ? mBinding.findRelativeTopMostParent(rootTag, childTag) : null; + } + @Override @AnyThread @ThreadConfined(ANY) diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/ReactClippingViewGroup.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/ReactClippingViewGroup.kt index dbd63a29541..b62626efe7c 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/ReactClippingViewGroup.kt +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/ReactClippingViewGroup.kt @@ -32,6 +32,8 @@ public interface ReactClippingViewGroup { */ public fun updateClippingRect() + public fun updateClippingRect(excludedView: Set?) + /** * Get rectangular bounds to which view is currently clipped to. Called only on views that has set * `removeCLippedSubviews` property value to `true`. diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactHorizontalScrollView.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactHorizontalScrollView.java index a3aa393ecf1..0c3ed4e7b25 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactHorizontalScrollView.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactHorizontalScrollView.java @@ -11,6 +11,7 @@ import static com.facebook.react.views.scroll.ReactScrollViewHelper.SNAP_ALIGNME import static com.facebook.react.views.scroll.ReactScrollViewHelper.SNAP_ALIGNMENT_DISABLED; import static com.facebook.react.views.scroll.ReactScrollViewHelper.SNAP_ALIGNMENT_END; import static com.facebook.react.views.scroll.ReactScrollViewHelper.SNAP_ALIGNMENT_START; +import static com.facebook.react.views.scroll.ReactScrollViewHelper.findNextFocusableView; import android.animation.ObjectAnimator; import android.animation.ValueAnimator; @@ -31,6 +32,7 @@ import android.widget.HorizontalScrollView; import android.widget.OverScroller; import androidx.annotation.Nullable; import androidx.core.view.ViewCompat; +import androidx.core.view.ViewCompat.FocusRealDirection; import com.facebook.common.logging.FLog; import com.facebook.infer.annotation.Assertions; import com.facebook.infer.annotation.Nullsafe; @@ -64,6 +66,7 @@ import com.facebook.systrace.Systrace; import java.lang.reflect.Field; import java.util.ArrayList; import java.util.List; +import java.util.Set; /** Similar to {@link ReactScrollView} but only supports horizontal scrolling. */ @Nullsafe(Nullsafe.Mode.LOCAL) @@ -771,8 +774,24 @@ public class ReactHorizontalScrollView extends HorizontalScrollView } } + @Override + public @Nullable View focusSearch(View focused, @FocusRealDirection int direction) { + @Nullable View nextfocusableView = findNextFocusableView(this, focused, direction, true); + + if (nextfocusableView != null) { + return nextfocusableView; + } + + return super.focusSearch(focused, direction); + } + @Override public void updateClippingRect() { + updateClippingRect(null); + } + + @Override + public void updateClippingRect(@Nullable Set excludedViewId) { if (!mRemoveClippedSubviews) { return; } @@ -784,7 +803,7 @@ public class ReactHorizontalScrollView extends HorizontalScrollView ReactClippingViewGroupHelper.calculateClippingRect(this, mClippingRect); View contentView = getContentView(); if (contentView instanceof ReactClippingViewGroup) { - ((ReactClippingViewGroup) contentView).updateClippingRect(); + ((ReactClippingViewGroup) contentView).updateClippingRect(excludedViewId); } } finally { Systrace.endSection(Systrace.TRACE_TAG_REACT); diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactScrollView.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactScrollView.java index d463a90a8aa..82f968b36bd 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactScrollView.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactScrollView.java @@ -11,6 +11,7 @@ import static com.facebook.react.views.scroll.ReactScrollViewHelper.SNAP_ALIGNME import static com.facebook.react.views.scroll.ReactScrollViewHelper.SNAP_ALIGNMENT_DISABLED; import static com.facebook.react.views.scroll.ReactScrollViewHelper.SNAP_ALIGNMENT_END; import static com.facebook.react.views.scroll.ReactScrollViewHelper.SNAP_ALIGNMENT_START; +import static com.facebook.react.views.scroll.ReactScrollViewHelper.findNextFocusableView; import android.animation.ObjectAnimator; import android.animation.ValueAnimator; @@ -31,6 +32,7 @@ import android.widget.ScrollView; import androidx.annotation.NonNull; import androidx.annotation.Nullable; import androidx.core.view.ViewCompat; +import androidx.core.view.ViewCompat.FocusRealDirection; import com.facebook.common.logging.FLog; import com.facebook.infer.annotation.Assertions; import com.facebook.infer.annotation.Nullsafe; @@ -63,6 +65,7 @@ import com.facebook.react.views.scroll.ReactScrollViewHelper.ReactScrollViewScro import com.facebook.systrace.Systrace; import java.lang.reflect.Field; import java.util.List; +import java.util.Set; /** * A simple subclass of ScrollView that doesn't dispatch measure and layout to its children and has @@ -359,6 +362,18 @@ public class ReactScrollView extends ScrollView } } + @Override + public @Nullable View focusSearch(View focused, @FocusRealDirection int direction) { + + @Nullable View nextfocusableView = findNextFocusableView(this, focused, direction, false); + + if (nextfocusableView != null) { + return nextfocusableView; + } + + return super.focusSearch(focused, direction); + } + /** * Since ReactScrollView handles layout changes on JS side, it does not call super.onlayout due to * which mIsLayoutDirty flag in ScrollView remains true and prevents scrolling to child when @@ -528,6 +543,11 @@ public class ReactScrollView extends ScrollView @Override public void updateClippingRect() { + updateClippingRect(null); + } + + @Override + public void updateClippingRect(@Nullable Set excludedViewsSet) { if (!mRemoveClippedSubviews) { return; } @@ -539,7 +559,7 @@ public class ReactScrollView extends ScrollView ReactClippingViewGroupHelper.calculateClippingRect(this, mClippingRect); View contentView = getContentView(); if (contentView instanceof ReactClippingViewGroup) { - ((ReactClippingViewGroup) contentView).updateClippingRect(); + ((ReactClippingViewGroup) contentView).updateClippingRect(excludedViewsSet); } } finally { Systrace.endSection(Systrace.TRACE_TAG_REACT); diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactScrollViewHelper.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactScrollViewHelper.kt index 1e920bc36c1..a86e571c612 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactScrollViewHelper.kt +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactScrollViewHelper.kt @@ -11,15 +11,19 @@ import android.animation.Animator import android.animation.ValueAnimator import android.content.Context import android.graphics.Point +import android.view.FocusFinder import android.view.View import android.view.ViewGroup import android.widget.OverScroller +import androidx.core.view.ViewCompat.FocusRealDirection import com.facebook.common.logging.FLog import com.facebook.react.bridge.ReactContext import com.facebook.react.bridge.WritableMap import com.facebook.react.bridge.WritableNativeMap import com.facebook.react.common.ReactConstants +import com.facebook.react.fabric.FabricUIManager import com.facebook.react.uimanager.PixelUtil.toDIPFromPixel +import com.facebook.react.uimanager.ReactClippingViewGroup import com.facebook.react.uimanager.StateWrapper import com.facebook.react.uimanager.UIManagerHelper import com.facebook.react.uimanager.common.UIManagerType @@ -462,6 +466,67 @@ public object ReactScrollViewHelper { return Point(scroller.finalX, scroller.finalY) } + @JvmStatic + public fun findNextFocusableView( + host: ViewGroup, + focused: View, + @FocusRealDirection direction: Int, + horizontal: Boolean + ): View? { + val absDir = resolveAbsoluteDirection(direction, horizontal, host.getLayoutDirection()) + + /* + * Check if we can focus the next element in the absolute direction within the ScrollView this + * would mean the view is not clipped, if we can't, look into the shadow tree to find the next + * focusable element + */ + val ff = FocusFinder.getInstance() + val result = ff.findNextFocus(host, focused, absDir) + + if (result != null) { + return result + } + + if (host !is ReactClippingViewGroup) { + return null + } + + val uimanager = + UIManagerHelper.getUIManager(host.context as ReactContext, UIManagerType.FABRIC) + ?: return null + + val nextFocusableViewId = + (uimanager as FabricUIManager).findNextFocusableElement( + host.getChildAt(0).id, focused.id, absDir) ?: return null + + val nextFocusTopMostParentId = + uimanager.findRelativeTopMostParent(host.getChildAt(0).id, nextFocusableViewId) + ?: return null + + host.updateClippingRect(setOf(nextFocusableViewId, nextFocusTopMostParentId)) + + return host.findViewById(nextFocusableViewId) + } + + @JvmStatic + public fun resolveAbsoluteDirection( + @FocusRealDirection direction: Int, + horizontal: Boolean, + layoutDirection: Int + ): Int { + val rtl: Boolean = layoutDirection == View.LAYOUT_DIRECTION_RTL + + return if (direction == View.FOCUS_FORWARD || direction == View.FOCUS_BACKWARD) { + if (horizontal) { + if ((direction == View.FOCUS_FORWARD) != rtl) View.FOCUS_RIGHT else View.FOCUS_LEFT + } else { + if (direction == View.FOCUS_FORWARD) View.FOCUS_DOWN else View.FOCUS_UP + } + } else { + direction + } + } + public interface ScrollListener { public fun onScroll( scrollView: ViewGroup?, diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/view/ReactViewGroup.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/view/ReactViewGroup.java index 2d7563395e6..c2996ed2056 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/view/ReactViewGroup.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/view/ReactViewGroup.java @@ -406,6 +406,11 @@ public class ReactViewGroup extends ViewGroup @Override public void updateClippingRect() { + updateClippingRect(null); + } + + @Override + public void updateClippingRect(@Nullable Set excludedViewsSet) { if (!mRemoveClippedSubviews) { return; } @@ -414,7 +419,7 @@ public class ReactViewGroup extends ViewGroup Assertions.assertNotNull(mAllChildren); ReactClippingViewGroupHelper.calculateClippingRect(this, mClippingRect); - updateClippingToRect(mClippingRect); + updateClippingToRect(mClippingRect, excludedViewsSet); } @Override @@ -438,12 +443,16 @@ public class ReactViewGroup extends ViewGroup } private void updateClippingToRect(Rect clippingRect) { + updateClippingToRect(clippingRect, null); + } + + private void updateClippingToRect(Rect clippingRect, @Nullable Set excludedViewsSet) { Assertions.assertNotNull(mAllChildren); mInSubviewClippingLoop = true; int clippedSoFar = 0; for (int i = 0; i < mAllChildrenCount; i++) { try { - updateSubviewClipStatus(clippingRect, i, clippedSoFar); + updateSubviewClipStatus(clippingRect, i, clippedSoFar, excludedViewsSet); } catch (IndexOutOfBoundsException e) { int realClippedSoFar = 0; Set uniqueViews = new HashSet<>(); @@ -477,6 +486,11 @@ public class ReactViewGroup extends ViewGroup } private void updateSubviewClipStatus(Rect clippingRect, int idx, int clippedSoFar) { + updateSubviewClipStatus(clippingRect, idx, clippedSoFar, null); + } + + private void updateSubviewClipStatus( + Rect clippingRect, int idx, int clippedSoFar, @Nullable Set excludedViewsSet) { UiThreadUtil.assertOnUiThread(); View child = Assertions.assertNotNull(mAllChildren)[idx]; @@ -492,14 +506,22 @@ public class ReactViewGroup extends ViewGroup // it won't be size and located properly. Animation animation = child.getAnimation(); boolean isAnimating = animation != null && !animation.hasEnded(); + boolean shouldSkipView = excludedViewsSet != null && excludedViewsSet.contains(child.getId()); + if (excludedViewsSet != null) { + needUpdateClippingRecursive = true; + } // We don't want to clip a view that is currently focused at that might break focus navigation - if (!intersects && !isViewClipped(child, idx) && !isAnimating && child != getFocusedChild()) { + if (!intersects + && !isViewClipped(child, idx) + && !isAnimating + && child != getFocusedChild() + && !shouldSkipView) { setViewClipped(child, true); // We can try saving on invalidate call here as the view that we remove is out of visible area // therefore invalidation is not necessary. removeViewInLayout(child); needUpdateClippingRecursive = true; - } else if (intersects && isViewClipped(child, idx)) { + } else if (shouldSkipView || (intersects && isViewClipped(child, idx))) { int adjustedIdx = idx - clippedSoFar; Assertions.assertCondition(adjustedIdx >= 0); setViewClipped(child, false); @@ -514,7 +536,7 @@ public class ReactViewGroup extends ViewGroup if (child instanceof ReactClippingViewGroup) { ReactClippingViewGroup clippingChild = (ReactClippingViewGroup) child; if (clippingChild.getRemoveClippedSubviews()) { - clippingChild.updateClippingRect(); + clippingChild.updateClippingRect(excludedViewsSet); } } }