From bfca7cfe7a8e0da10fdb776100fbc706233c0d8d Mon Sep 17 00:00:00 2001 From: Nick Gerleman Date: Tue, 29 Oct 2024 16:29:36 -0700 Subject: [PATCH] Fix legacy arch RTL horizontal ScrollView regression (#47282) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/47282 D63318754 fixed a class of issues with RTL horizontal scrollviews by moving logic from native Android view layer to Fabric ShadowNode layer. I realized quite a bit later this is problematic for legacy arch, since it now never runs RTL translation code, and all our RTL screenshot tests are against new arch. It's tricky to port Fabric ShadowNode related code to Paper since its shadownodes are more coupled to Yoga nodes, and the existing solution for contextual layout direction didn't quite work, so I went with the original logic we had for this, where we use global layout direction to determine whether to offset, and disable removeClippedSubviews, and this is applied via onLayout. This is wrong in several ways, but not a regression compared to previous legacy arch behavior. The fully correct behavior will require new arch. Changelog: [Android][Fixed] - Fix legacy arch RTL horizontal ScrollView regression Reviewed By: rshest Differential Revision: D65139747 fbshipit-source-id: 5662c0744c3c402efe583e602b2272662b4d5476 --- .../ReactAndroid/api/ReactAndroid.api | 3 ++ ...eactHorizontalScrollContainerLegacyView.kt | 46 +++++++++++++++++++ ...actHorizontalScrollContainerViewManager.kt | 27 +++++++++++ 3 files changed, 76 insertions(+) create mode 100644 packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactHorizontalScrollContainerLegacyView.kt diff --git a/packages/react-native/ReactAndroid/api/ReactAndroid.api b/packages/react-native/ReactAndroid/api/ReactAndroid.api index 79e57da000f..6eba8f35ba9 100644 --- a/packages/react-native/ReactAndroid/api/ReactAndroid.api +++ b/packages/react-native/ReactAndroid/api/ReactAndroid.api @@ -6959,6 +6959,9 @@ public final class com/facebook/react/views/scroll/ReactHorizontalScrollContaine 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 (ILcom/facebook/react/uimanager/ThemedReactContext;Lcom/facebook/react/uimanager/ReactStylesDiffMap;Lcom/facebook/react/uimanager/StateWrapper;)Landroid/view/View; + public synthetic fun createViewInstance (Lcom/facebook/react/uimanager/ThemedReactContext;)Landroid/view/View; + public fun createViewInstance (Lcom/facebook/react/uimanager/ThemedReactContext;)Lcom/facebook/react/views/view/ReactViewGroup; public fun getName ()Ljava/lang/String; } diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactHorizontalScrollContainerLegacyView.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactHorizontalScrollContainerLegacyView.kt new file mode 100644 index 00000000000..84c7647053a --- /dev/null +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactHorizontalScrollContainerLegacyView.kt @@ -0,0 +1,46 @@ +/* + * 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 com.facebook.react.modules.i18nmanager.I18nUtil +import com.facebook.react.views.view.ReactViewGroup + +/** + * Used by legacy/Paper renderer to perform offsetting of scroll content when the app-wide layout + * direction is RTL. Contextually set layout direction is not respected by legacy renderer. + */ +internal class ReactHorizontalScrollContainerLegacyView(context: Context) : + ReactViewGroup(context) { + private val isRTL: Boolean = I18nUtil.instance.isRTL(context) + + override fun setRemoveClippedSubviews(removeClippedSubviews: Boolean) { + // removeClippedSubviews logic may read metrics before the offsetting we do in onLayout() and is + // such unsafe + if (isRTL) { + super.setRemoveClippedSubviews(false) + return + } + + super.setRemoveClippedSubviews(removeClippedSubviews) + } + + protected override fun onLayout(changed: Boolean, left: Int, top: Int, right: Int, bottom: Int) { + if (isRTL) { + // 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 + val newLeft = 0 + val width = right - left + val 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 f1f31d8487a..c9d6c01e065 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,6 +8,12 @@ package com.facebook.react.views.scroll import com.facebook.react.module.annotations.ReactModule +import com.facebook.react.uimanager.ReactStylesDiffMap +import com.facebook.react.uimanager.StateWrapper +import com.facebook.react.uimanager.ThemedReactContext +import com.facebook.react.uimanager.common.UIManagerType +import com.facebook.react.uimanager.common.ViewUtil +import com.facebook.react.views.view.ReactViewGroup import com.facebook.react.views.view.ReactViewManager /** View manager for {@link ReactHorizontalScrollContainerView} components. */ @@ -15,7 +21,28 @@ import com.facebook.react.views.view.ReactViewManager public class ReactHorizontalScrollContainerViewManager : ReactViewManager() { override public fun getName(): String = REACT_CLASS + protected override fun createViewInstance( + reactTag: Int, + context: ThemedReactContext, + initialProps: ReactStylesDiffMap?, + stateWrapper: StateWrapper? + ): ReactViewGroup { + check(uiManagerType == null) + uiManagerType = ViewUtil.getUIManagerType(reactTag) + val view = super.createViewInstance(reactTag, context, initialProps, stateWrapper) + uiManagerType = null + return view + } + + public override fun createViewInstance(context: ThemedReactContext): ReactViewGroup { + return when (checkNotNull(uiManagerType)) { + UIManagerType.FABRIC -> ReactViewGroup(context) + else -> ReactHorizontalScrollContainerLegacyView(context) + } + } + public companion object { public const val REACT_CLASS: String = "AndroidHorizontalScrollContentView" + private @UIManagerType var uiManagerType: Int? = null } }