From ea6928fcb95c19e84f8f2851f28d0cd4343c6a26 Mon Sep 17 00:00:00 2001 From: Nick Gerleman Date: Wed, 10 Jul 2024 20:17:03 -0700 Subject: [PATCH] Fix Android removeClippedSubviews in horizontal ScrollView in RTL (#45356) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/45356 Subview clipping was disabled for RTL on Android due to a bug where TextInputs automatically blur when selected. This bug is reintroduced when `set_android_layout_direction` is set. When `set_android_layout_direction` is enabled, and we use View `getLayoutDirection()` instead of `I18nManager.isRTL()`, the layout direction is not known until layer in the mounting process. This defeats the check a `setRemoveClippedSubviews()` prop setter to ignore the prop if the view is RTL (since it doesn't invalidate when a new layout direction is set). The root cause of the underlying RTL bug is due to updating clipping status triggered by `onSizeChanged()`, which is called before `onLayout()`, where `ReactHorizontalScrollContainerView` offsets content in RTL. This also seems potentially erroneous, as we do not update the clipping rect on position change (unless that is handled elsewhere). I moved the check to `onLayout()`, called after ReactHorizontalScrollView will change metrics, which seems to fix the issue. I then removed the exclusion in `removeClippedSubviews` prop setter. Changelog: [Android][Fixed] - Fix Android removeClippedSubviews in RTL Reviewed By: mdvacca Differential Revision: D59566611 fbshipit-source-id: a2eb12b984dc78940756804b6b7a3950377af9de --- .../ReactAndroid/api/ReactAndroid.api | 2 -- .../ReactHorizontalScrollContainerView.java | 15 --------------- .../facebook/react/views/view/ReactViewGroup.java | 15 ++++++--------- 3 files changed, 6 insertions(+), 26 deletions(-) diff --git a/packages/react-native/ReactAndroid/api/ReactAndroid.api b/packages/react-native/ReactAndroid/api/ReactAndroid.api index ee036c0404d..8516b794266 100644 --- a/packages/react-native/ReactAndroid/api/ReactAndroid.api +++ b/packages/react-native/ReactAndroid/api/ReactAndroid.api @@ -6645,7 +6645,6 @@ public class com/facebook/react/views/scroll/ReactHorizontalScrollContainerView public fun (Landroid/content/Context;)V public fun getLayoutDirection ()I protected fun onLayout (ZIIII)V - public fun setRemoveClippedSubviews (Z)V } public final class com/facebook/react/views/scroll/ReactHorizontalScrollContainerViewManager : com/facebook/react/views/view/ReactClippingViewManager { @@ -7933,7 +7932,6 @@ public class com/facebook/react/views/view/ReactViewGroup : android/view/ViewGro public fun onInterceptTouchEvent (Landroid/view/MotionEvent;)Z protected fun onLayout (ZIIII)V protected fun onMeasure (II)V - protected fun onSizeChanged (IIII)V public fun onTouchEvent (Landroid/view/MotionEvent;)Z public fun removeView (Landroid/view/View;)V public fun removeViewAt (I)V diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactHorizontalScrollContainerView.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactHorizontalScrollContainerView.java index fbb10ecfc40..76e0c1c8ce3 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactHorizontalScrollContainerView.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactHorizontalScrollContainerView.java @@ -36,21 +36,6 @@ public class ReactHorizontalScrollContainerView extends ReactViewGroup { return mLayoutDirection; } - @Override - public void setRemoveClippedSubviews(boolean removeClippedSubviews) { - // Clipping doesn't work well for horizontal scroll views in RTL mode - in both - // Fabric and non-Fabric - especially with TextInputs. The behavior you could see - // is TextInputs being blurred immediately after being focused. So, for now, - // it's easier to just disable this for these specific RTL views. - // TODO T86027499: support `setRemoveClippedSubviews` in RTL mode - if (getLayoutDirection() == LAYOUT_DIRECTION_RTL) { - super.setRemoveClippedSubviews(false); - return; - } - - super.setRemoveClippedSubviews(removeClippedSubviews); - } - @Override protected void onLayout(boolean changed, int left, int top, int right, int bottom) { if (getLayoutDirection() == LAYOUT_DIRECTION_RTL) { 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 b50a6257b25..263b765d8ca 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 @@ -195,7 +195,12 @@ public class ReactViewGroup extends ViewGroup @Override protected void onLayout(boolean changed, int left, int top, int right, int bottom) { - // No-op since UIManagerModule handles actually laying out children. + // If the size or position of the view has changed it may intersect different children than + // before. If "removeClippedSubviews" is set, we must re-evaluate intersection to render newly + // visible children, and remove those no longer visible. + if (changed && mRemoveClippedSubviews) { + updateClippingRect(); + } } @Override @@ -482,14 +487,6 @@ public class ReactViewGroup extends ViewGroup return super.getChildVisibleRect(child, r, offset); } - @Override - protected void onSizeChanged(int w, int h, int oldw, int oldh) { - super.onSizeChanged(w, h, oldw, oldh); - if (mRemoveClippedSubviews) { - updateClippingRect(); - } - } - @Override protected void onAttachedToWindow() { super.onAttachedToWindow();