From 513e9669e78a4bfd9b0380335c61f581343c4009 Mon Sep 17 00:00:00 2001 From: Nick Gerleman Date: Fri, 27 Sep 2024 19:19:53 -0700 Subject: [PATCH] Fix interactions between removeClippedSubviews and RTL by applying HorizontalScrollContentView translation during layout metric assignment (#46685) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/46685 This attempts a similar fix to D59566611, but keeps the removedClippedSubviews logic as is, instead moving the current translation done in ReactHorizontalScrollContainerView onLayout to ShadowNode layout metric assignment (so the world is consistent from very early on, and `removeClippedSubviews` never sees coordinates before translation), I suspect doing this at the ShadowNode layer might also result in some fixes to DevTools in RTL. This should let us roll out setAndroidLayoutDirection again. Changelog: [Android][Fixed] - Fix interactions between removeClippedSubviews and RTL Reviewed By: mdvacca Differential Revision: D63318754 fbshipit-source-id: 828e103e2ad21c7e886e39c163474b10ebd5099e --- .../ReactAndroid/api/ReactAndroid.api | 13 +--- .../ReactHorizontalScrollContainerView.java | 68 ------------------- ...actHorizontalScrollContainerViewManager.kt | 11 +-- .../react/fabric/CoreComponentsRegistry.cpp | 1 + ...ntalScrollContentViewComponentDescriptor.h | 18 +++++ ...dHorizontalScrollContentViewShadowNode.cpp | 26 +++++++ ...oidHorizontalScrollContentViewShadowNode.h | 26 +++++++ ...izontalScrollContentViewNativeComponent.js | 1 + 8 files changed, 76 insertions(+), 88 deletions(-) delete mode 100644 packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactHorizontalScrollContainerView.java create mode 100644 packages/react-native/ReactCommon/react/renderer/components/scrollview/AndroidHorizontalScrollContentViewComponentDescriptor.h create mode 100644 packages/react-native/ReactCommon/react/renderer/components/scrollview/AndroidHorizontalScrollContentViewShadowNode.cpp create mode 100644 packages/react-native/ReactCommon/react/renderer/components/scrollview/AndroidHorizontalScrollContentViewShadowNode.h diff --git a/packages/react-native/ReactAndroid/api/ReactAndroid.api b/packages/react-native/ReactAndroid/api/ReactAndroid.api index 17d02584a08771..085cd2e001ee32 100644 --- a/packages/react-native/ReactAndroid/api/ReactAndroid.api +++ b/packages/react-native/ReactAndroid/api/ReactAndroid.api @@ -7029,19 +7029,10 @@ public final class com/facebook/react/views/scroll/OnScrollDispatchHelper { public final fun onScrollChanged (II)Z } -public class com/facebook/react/views/scroll/ReactHorizontalScrollContainerView : com/facebook/react/views/view/ReactViewGroup { - 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 { +public final class com/facebook/react/views/scroll/ReactHorizontalScrollContainerViewManager : com/facebook/react/views/view/ReactViewManager { public static final field Companion Lcom/facebook/react/views/scroll/ReactHorizontalScrollContainerViewManager$Companion; public static final field REACT_CLASS Ljava/lang/String; public fun ()V - public synthetic fun createViewInstance (Lcom/facebook/react/uimanager/ThemedReactContext;)Landroid/view/View; - public fun createViewInstance (Lcom/facebook/react/uimanager/ThemedReactContext;)Lcom/facebook/react/views/scroll/ReactHorizontalScrollContainerView; public fun getName ()Ljava/lang/String; } @@ -7049,7 +7040,7 @@ public class com/facebook/react/views/scroll/ReactHorizontalScrollContainerViewM public fun ()V public fun getProperties (Ljava/util/Map;)V public synthetic fun setProperty (Lcom/facebook/react/uimanager/ViewManager;Landroid/view/View;Ljava/lang/String;Ljava/lang/Object;)V - public fun setProperty (Lcom/facebook/react/views/scroll/ReactHorizontalScrollContainerViewManager;Lcom/facebook/react/views/scroll/ReactHorizontalScrollContainerView;Ljava/lang/String;Ljava/lang/Object;)V + public fun setProperty (Lcom/facebook/react/views/scroll/ReactHorizontalScrollContainerViewManager;Lcom/facebook/react/views/view/ReactViewGroup;Ljava/lang/String;Ljava/lang/Object;)V } public final class com/facebook/react/views/scroll/ReactHorizontalScrollContainerViewManager$Companion { 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 deleted file mode 100644 index fbb10ecfc40c17..00000000000000 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactHorizontalScrollContainerView.java +++ /dev/null @@ -1,68 +0,0 @@ -/* - * Copyright (c) Meta Platforms, Inc. and affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - */ - -package com.facebook.react.views.scroll; - -import android.content.Context; -import androidx.core.view.ViewCompat; -import com.facebook.infer.annotation.Nullsafe; -import com.facebook.react.internal.featureflags.ReactNativeFeatureFlags; -import com.facebook.react.modules.i18nmanager.I18nUtil; -import com.facebook.react.views.view.ReactViewGroup; - -/** Container of Horizontal scrollViews that supports RTL scrolling. */ -@Nullsafe(Nullsafe.Mode.LOCAL) -public class ReactHorizontalScrollContainerView extends ReactViewGroup { - - private int mLayoutDirection; - - public ReactHorizontalScrollContainerView(Context context) { - super(context); - mLayoutDirection = - I18nUtil.getInstance().isRTL(context) - ? ViewCompat.LAYOUT_DIRECTION_RTL - : ViewCompat.LAYOUT_DIRECTION_LTR; - } - - @Override - public int getLayoutDirection() { - if (ReactNativeFeatureFlags.setAndroidLayoutDirection()) { - return super.getLayoutDirection(); - } - 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) { - // When the layout direction is RTL, we expect Yoga to give us a layout - // that extends off the screen to the left so we re-center it with left=0 - int newLeft = 0; - int width = right - left; - int newRight = newLeft + width; - setLeft(newLeft); - setTop(top); - setRight(newRight); - setBottom(bottom); - } - } -} diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactHorizontalScrollContainerViewManager.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactHorizontalScrollContainerViewManager.kt index bbe48afd4b53fc..f1f31d8487a363 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactHorizontalScrollContainerViewManager.kt +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactHorizontalScrollContainerViewManager.kt @@ -8,20 +8,13 @@ package com.facebook.react.views.scroll import com.facebook.react.module.annotations.ReactModule -import com.facebook.react.uimanager.ThemedReactContext -import com.facebook.react.views.view.ReactClippingViewManager +import com.facebook.react.views.view.ReactViewManager /** View manager for {@link ReactHorizontalScrollContainerView} components. */ @ReactModule(name = ReactHorizontalScrollContainerViewManager.REACT_CLASS) -public class ReactHorizontalScrollContainerViewManager : - ReactClippingViewManager() { - +public class ReactHorizontalScrollContainerViewManager : ReactViewManager() { override public fun getName(): String = REACT_CLASS - override public fun createViewInstance( - context: ThemedReactContext - ): ReactHorizontalScrollContainerView = ReactHorizontalScrollContainerView(context) - public companion object { public const val REACT_CLASS: String = "AndroidHorizontalScrollContentView" } diff --git a/packages/react-native/ReactAndroid/src/main/jni/react/fabric/CoreComponentsRegistry.cpp b/packages/react-native/ReactAndroid/src/main/jni/react/fabric/CoreComponentsRegistry.cpp index 669fe3d6bcfce0..aa2cd5ffbe0a0e 100644 --- a/packages/react-native/ReactAndroid/src/main/jni/react/fabric/CoreComponentsRegistry.cpp +++ b/packages/react-native/ReactAndroid/src/main/jni/react/fabric/CoreComponentsRegistry.cpp @@ -17,6 +17,7 @@ #include #include #include +#include #include #include #include diff --git a/packages/react-native/ReactCommon/react/renderer/components/scrollview/AndroidHorizontalScrollContentViewComponentDescriptor.h b/packages/react-native/ReactCommon/react/renderer/components/scrollview/AndroidHorizontalScrollContentViewComponentDescriptor.h new file mode 100644 index 00000000000000..2705dabee11d9e --- /dev/null +++ b/packages/react-native/ReactCommon/react/renderer/components/scrollview/AndroidHorizontalScrollContentViewComponentDescriptor.h @@ -0,0 +1,18 @@ +/* + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +#pragma once + +#include +#include + +namespace facebook::react { + +using AndroidHorizontalScrollContentViewComponentDescriptor = + ConcreteComponentDescriptor; + +} // namespace facebook::react diff --git a/packages/react-native/ReactCommon/react/renderer/components/scrollview/AndroidHorizontalScrollContentViewShadowNode.cpp b/packages/react-native/ReactCommon/react/renderer/components/scrollview/AndroidHorizontalScrollContentViewShadowNode.cpp new file mode 100644 index 00000000000000..d7975f0ba396c8 --- /dev/null +++ b/packages/react-native/ReactCommon/react/renderer/components/scrollview/AndroidHorizontalScrollContentViewShadowNode.cpp @@ -0,0 +1,26 @@ +/* + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +#include + +namespace facebook::react { + +const char AndroidHorizontalScrollContentViewShadowNodeComponentName[] = + "AndroidHorizontalScrollContentView"; + +void AndroidHorizontalScrollContentViewShadowNode::layout( + LayoutContext layoutContext) { + ConcreteViewShadowNode::layout(layoutContext); + + // When the layout direction is RTL, we expect Yoga to give us a layout + // that extends off the screen to the left so we re-center it with left=0 + if (layoutMetrics_.layoutDirection == LayoutDirection::RightToLeft) { + layoutMetrics_.frame.origin.x = 0; + } +} + +} // namespace facebook::react diff --git a/packages/react-native/ReactCommon/react/renderer/components/scrollview/AndroidHorizontalScrollContentViewShadowNode.h b/packages/react-native/ReactCommon/react/renderer/components/scrollview/AndroidHorizontalScrollContentViewShadowNode.h new file mode 100644 index 00000000000000..7381418b0386d5 --- /dev/null +++ b/packages/react-native/ReactCommon/react/renderer/components/scrollview/AndroidHorizontalScrollContentViewShadowNode.h @@ -0,0 +1,26 @@ +/* + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +#pragma once + +#include +#include + +namespace facebook::react { + +extern const char AndroidHorizontalScrollContentViewShadowNodeComponentName[]; + +class AndroidHorizontalScrollContentViewShadowNode final + : public ConcreteViewShadowNode< + AndroidHorizontalScrollContentViewShadowNodeComponentName, + ViewProps> { + public: + using ConcreteViewShadowNode::ConcreteViewShadowNode; + void layout(LayoutContext layoutContext) override; +}; + +} // namespace facebook::react diff --git a/packages/react-native/src/private/specs/components/AndroidHorizontalScrollContentViewNativeComponent.js b/packages/react-native/src/private/specs/components/AndroidHorizontalScrollContentViewNativeComponent.js index 98bf5c55ee4d87..7b0f3bdfe1aeec 100644 --- a/packages/react-native/src/private/specs/components/AndroidHorizontalScrollContentViewNativeComponent.js +++ b/packages/react-native/src/private/specs/components/AndroidHorizontalScrollContentViewNativeComponent.js @@ -23,4 +23,5 @@ type NativeType = HostComponent; export default (codegenNativeComponent( 'AndroidHorizontalScrollContentView', + {interfaceOnly: true}, ): NativeType);