From 513ff5307c0ec571ce255c55d47cf63841c4e033 Mon Sep 17 00:00:00 2001 From: Gijs Weterings Date: Wed, 16 Apr 2025 07:28:41 -0700 Subject: [PATCH] Back out "Fix keyboard navigation for FlatList with `removeClippedSubviews` enabled" Summary: Original commit changeset: b55b7735a307 Original Phabricator Diff: D71324219 Changelog: [Internal] Differential Revision: D73103167 fbshipit-source-id: 99f2ed13a482958a8c71ba2dd1cfc2a5de4786d3 --- .../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, 7 insertions(+), 192 deletions(-) diff --git a/packages/react-native/ReactAndroid/api/ReactAndroid.api b/packages/react-native/ReactAndroid/api/ReactAndroid.api index ff493913ad3..94e3f83f4af 100644 --- a/packages/react-native/ReactAndroid/api/ReactAndroid.api +++ b/packages/react-native/ReactAndroid/api/ReactAndroid.api @@ -2343,8 +2343,6 @@ 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; @@ -3996,7 +3994,6 @@ 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 final class com/facebook/react/uimanager/ReactClippingViewGroupHelper { @@ -5898,7 +5895,6 @@ 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,7 +5957,6 @@ 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 { @@ -6027,7 +6022,6 @@ 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; @@ -6091,7 +6085,6 @@ 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 { @@ -6149,7 +6142,6 @@ 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 @@ -6159,7 +6151,6 @@ 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 @@ -6952,7 +6943,6 @@ 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 2dd7615a067..8e0c683e158 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,7 +27,6 @@ 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; @@ -263,52 +262,6 @@ 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 b62626efe7c..dbd63a29541 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,8 +32,6 @@ 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 0c3ed4e7b25..a3aa393ecf1 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,7 +11,6 @@ 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; @@ -32,7 +31,6 @@ 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; @@ -66,7 +64,6 @@ 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) @@ -774,24 +771,8 @@ 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; } @@ -803,7 +784,7 @@ public class ReactHorizontalScrollView extends HorizontalScrollView ReactClippingViewGroupHelper.calculateClippingRect(this, mClippingRect); View contentView = getContentView(); if (contentView instanceof ReactClippingViewGroup) { - ((ReactClippingViewGroup) contentView).updateClippingRect(excludedViewId); + ((ReactClippingViewGroup) contentView).updateClippingRect(); } } 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 8cdbcae3d8a..5967ea0e818 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,7 +11,6 @@ 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; @@ -32,7 +31,6 @@ 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; @@ -65,7 +63,6 @@ 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 @@ -362,18 +359,6 @@ 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 @@ -543,11 +528,6 @@ public class ReactScrollView extends ScrollView @Override public void updateClippingRect() { - updateClippingRect(null); - } - - @Override - public void updateClippingRect(@Nullable Set excludedViewsSet) { if (!mRemoveClippedSubviews) { return; } @@ -559,7 +539,7 @@ public class ReactScrollView extends ScrollView ReactClippingViewGroupHelper.calculateClippingRect(this, mClippingRect); View contentView = getContentView(); if (contentView instanceof ReactClippingViewGroup) { - ((ReactClippingViewGroup) contentView).updateClippingRect(excludedViewsSet); + ((ReactClippingViewGroup) contentView).updateClippingRect(); } } 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 a86e571c612..1e920bc36c1 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,19 +11,15 @@ 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 @@ -466,67 +462,6 @@ 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 c2996ed2056..2d7563395e6 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,11 +406,6 @@ public class ReactViewGroup extends ViewGroup @Override public void updateClippingRect() { - updateClippingRect(null); - } - - @Override - public void updateClippingRect(@Nullable Set excludedViewsSet) { if (!mRemoveClippedSubviews) { return; } @@ -419,7 +414,7 @@ public class ReactViewGroup extends ViewGroup Assertions.assertNotNull(mAllChildren); ReactClippingViewGroupHelper.calculateClippingRect(this, mClippingRect); - updateClippingToRect(mClippingRect, excludedViewsSet); + updateClippingToRect(mClippingRect); } @Override @@ -443,16 +438,12 @@ 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, excludedViewsSet); + updateSubviewClipStatus(clippingRect, i, clippedSoFar); } catch (IndexOutOfBoundsException e) { int realClippedSoFar = 0; Set uniqueViews = new HashSet<>(); @@ -486,11 +477,6 @@ 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]; @@ -506,22 +492,14 @@ 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() - && !shouldSkipView) { + if (!intersects && !isViewClipped(child, idx) && !isAnimating && child != getFocusedChild()) { 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 (shouldSkipView || (intersects && isViewClipped(child, idx))) { + } else if (intersects && isViewClipped(child, idx)) { int adjustedIdx = idx - clippedSoFar; Assertions.assertCondition(adjustedIdx >= 0); setViewClipped(child, false); @@ -536,7 +514,7 @@ public class ReactViewGroup extends ViewGroup if (child instanceof ReactClippingViewGroup) { ReactClippingViewGroup clippingChild = (ReactClippingViewGroup) child; if (clippingChild.getRemoveClippedSubviews()) { - clippingChild.updateClippingRect(excludedViewsSet); + clippingChild.updateClippingRect(); } } }