Skip to content

Commit

Permalink
Fix legacy arch RTL horizontal ScrollView regression
Browse files Browse the repository at this point in the history
Summary:
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 we didn't have any screenshot tests covering this at the time.

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

Differential Revision: D65139747
  • Loading branch information
NickGerleman authored and facebook-github-bot committed Oct 29, 2024
1 parent fe6f470 commit ae16c21
Show file tree
Hide file tree
Showing 2 changed files with 72 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
/*
* 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.
*/
public 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)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,41 @@
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. */
@ReactModule(name = ReactHorizontalScrollContainerViewManager.REACT_CLASS)
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
}
}

0 comments on commit ae16c21

Please sign in to comment.