Skip to content

Commit

Permalink
Cleanup "setAndroidLayoutDirection" (facebook#47736)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: facebook#47736

This has been 100% enabled in production across all the apps for more than a month, after an experiment a long time ago, and we kinda depend on it now for correct border drawing in some cases. Let's clean it up!

Changelog: [Internal]

Reviewed By: joevilches

Differential Revision: D66208418

fbshipit-source-id: 582a38bd84a2d085ba5c4aac4cd478680dd206cc
  • Loading branch information
NickGerleman authored and facebook-github-bot committed Nov 20, 2024
1 parent a865975 commit e88974f
Show file tree
Hide file tree
Showing 27 changed files with 57 additions and 226 deletions.
3 changes: 1 addition & 2 deletions packages/react-native/ReactAndroid/api/ReactAndroid.api
Original file line number Diff line number Diff line change
Expand Up @@ -7028,11 +7028,10 @@ public abstract interface class com/facebook/react/views/scroll/ReactScrollViewH
}

public final class com/facebook/react/views/scroll/ReactScrollViewHelper$ReactScrollViewScrollState {
public fun <init> (I)V
public fun <init> ()V
public final fun getDecelerationRate ()F
public final fun getFinalAnimatedPositionScroll ()Landroid/graphics/Point;
public final fun getLastStateUpdateScroll ()Landroid/graphics/Point;
public final fun getLayoutDirection ()I
public final fun getScrollAwayPaddingTop ()I
public final fun isCanceled ()Z
public final fun isFinished ()Z
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -800,12 +800,10 @@ public void updateLayout(
throw new IllegalStateException("Unable to find View for tag: " + reactTag);
}

if (ReactNativeFeatureFlags.setAndroidLayoutDirection()) {
viewToUpdate.setLayoutDirection(
layoutDirection == 1
? View.LAYOUT_DIRECTION_LTR
: layoutDirection == 2 ? View.LAYOUT_DIRECTION_RTL : View.LAYOUT_DIRECTION_INHERIT);
}
viewToUpdate.setLayoutDirection(
layoutDirection == 1
? View.LAYOUT_DIRECTION_LTR
: layoutDirection == 2 ? View.LAYOUT_DIRECTION_RTL : View.LAYOUT_DIRECTION_INHERIT);

// Even though we have exact dimensions, we still call measure because some platform views (e.g.
// Switch) assume that method will always be called before onLayout and onDraw. They use it to
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -153,15 +153,9 @@ public void execute(MountingManager mountingManager) {
int width = mIntBuffer[i++];
int height = mIntBuffer[i++];
int displayType = mIntBuffer[i++];

if (ReactNativeFeatureFlags.setAndroidLayoutDirection()) {
int layoutDirection = mIntBuffer[i++];
surfaceMountingManager.updateLayout(
reactTag, parentTag, x, y, width, height, displayType, layoutDirection);
} else {
surfaceMountingManager.updateLayout(
reactTag, parentTag, x, y, width, height, displayType, 0);
}
int layoutDirection = mIntBuffer[i++];
surfaceMountingManager.updateLayout(
reactTag, parentTag, x, y, width, height, displayType, layoutDirection);
} else if (type == INSTRUCTION_UPDATE_PADDING) {
surfaceMountingManager.updatePadding(
mIntBuffer[i++], mIntBuffer[i++], mIntBuffer[i++], mIntBuffer[i++], mIntBuffer[i++]);
Expand Down Expand Up @@ -251,8 +245,7 @@ public String toString() {
int w = mIntBuffer[i++];
int h = mIntBuffer[i++];
int displayType = mIntBuffer[i++];
int layoutDirection =
ReactNativeFeatureFlags.setAndroidLayoutDirection() ? mIntBuffer[i++] : 0;
int layoutDirection = mIntBuffer[i++];
s.append(
String.format(
"UPDATE LAYOUT [%d]->[%d]: x:%d y:%d w:%d h:%d displayType:%d layoutDirection:"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @generated SignedSource<<124e8e4892062fa63a9a70ae0c53653d>>
* @generated SignedSource<<b1ad8179cf22c2e97f623435bf152aab>>
*/

/**
Expand Down Expand Up @@ -238,12 +238,6 @@ public object ReactNativeFeatureFlags {
@JvmStatic
public fun loadVectorDrawablesOnImages(): Boolean = accessor.loadVectorDrawablesOnImages()

/**
* Propagate layout direction to Android views.
*/
@JvmStatic
public fun setAndroidLayoutDirection(): Boolean = accessor.setAndroidLayoutDirection()

/**
* Enables storing js caller stack when creating promise in native module. This is useful in case of Promise rejection and tracing the cause.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @generated SignedSource<<28da40176c64703dcab4bc0d1af078a8>>
* @generated SignedSource<<7dd21b62dfe429aecdbd179cb482fb26>>
*/

/**
Expand Down Expand Up @@ -55,7 +55,6 @@ public class ReactNativeFeatureFlagsCxxAccessor : ReactNativeFeatureFlagsAccesso
private var initEagerTurboModulesOnNativeModulesQueueAndroidCache: Boolean? = null
private var lazyAnimationCallbacksCache: Boolean? = null
private var loadVectorDrawablesOnImagesCache: Boolean? = null
private var setAndroidLayoutDirectionCache: Boolean? = null
private var traceTurboModulePromiseRejectionsOnAndroidCache: Boolean? = null
private var useAlwaysAvailableJSErrorHandlingCache: Boolean? = null
private var useFabricInteropCache: Boolean? = null
Expand Down Expand Up @@ -382,15 +381,6 @@ public class ReactNativeFeatureFlagsCxxAccessor : ReactNativeFeatureFlagsAccesso
return cached
}

override fun setAndroidLayoutDirection(): Boolean {
var cached = setAndroidLayoutDirectionCache
if (cached == null) {
cached = ReactNativeFeatureFlagsCxxInterop.setAndroidLayoutDirection()
setAndroidLayoutDirectionCache = cached
}
return cached
}

override fun traceTurboModulePromiseRejectionsOnAndroid(): Boolean {
var cached = traceTurboModulePromiseRejectionsOnAndroidCache
if (cached == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @generated SignedSource<<e53c4f66b6261f983ad1502adc31f1dc>>
* @generated SignedSource<<fcfded14a5fe05501892993c1f58f3ed>>
*/

/**
Expand Down Expand Up @@ -98,8 +98,6 @@ public object ReactNativeFeatureFlagsCxxInterop {

@DoNotStrip @JvmStatic public external fun loadVectorDrawablesOnImages(): Boolean

@DoNotStrip @JvmStatic public external fun setAndroidLayoutDirection(): Boolean

@DoNotStrip @JvmStatic public external fun traceTurboModulePromiseRejectionsOnAndroid(): Boolean

@DoNotStrip @JvmStatic public external fun useAlwaysAvailableJSErrorHandling(): Boolean
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @generated SignedSource<<3d25d7b8ee194721c39fb5eb5eda87d4>>
* @generated SignedSource<<86d8dd894e80a788af7f47172d30d33c>>
*/

/**
Expand Down Expand Up @@ -93,8 +93,6 @@ public open class ReactNativeFeatureFlagsDefaults : ReactNativeFeatureFlagsProvi

override fun loadVectorDrawablesOnImages(): Boolean = false

override fun setAndroidLayoutDirection(): Boolean = true

override fun traceTurboModulePromiseRejectionsOnAndroid(): Boolean = false

override fun useAlwaysAvailableJSErrorHandling(): Boolean = false
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @generated SignedSource<<fb8747b040cd9c29e9ca0a7c1647ad91>>
* @generated SignedSource<<4bb5173b9ba1d3e620f3a7d613b27ac7>>
*/

/**
Expand Down Expand Up @@ -59,7 +59,6 @@ public class ReactNativeFeatureFlagsLocalAccessor : ReactNativeFeatureFlagsAcces
private var initEagerTurboModulesOnNativeModulesQueueAndroidCache: Boolean? = null
private var lazyAnimationCallbacksCache: Boolean? = null
private var loadVectorDrawablesOnImagesCache: Boolean? = null
private var setAndroidLayoutDirectionCache: Boolean? = null
private var traceTurboModulePromiseRejectionsOnAndroidCache: Boolean? = null
private var useAlwaysAvailableJSErrorHandlingCache: Boolean? = null
private var useFabricInteropCache: Boolean? = null
Expand Down Expand Up @@ -421,16 +420,6 @@ public class ReactNativeFeatureFlagsLocalAccessor : ReactNativeFeatureFlagsAcces
return cached
}

override fun setAndroidLayoutDirection(): Boolean {
var cached = setAndroidLayoutDirectionCache
if (cached == null) {
cached = currentProvider.setAndroidLayoutDirection()
accessedFeatureFlags.add("setAndroidLayoutDirection")
setAndroidLayoutDirectionCache = cached
}
return cached
}

override fun traceTurboModulePromiseRejectionsOnAndroid(): Boolean {
var cached = traceTurboModulePromiseRejectionsOnAndroidCache
if (cached == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @generated SignedSource<<24bae6a173c941923c84f418a114ec2f>>
* @generated SignedSource<<2217f2b9992c9424c3d1d221bdfa2236>>
*/

/**
Expand Down Expand Up @@ -93,8 +93,6 @@ public interface ReactNativeFeatureFlagsProvider {

@DoNotStrip public fun loadVectorDrawablesOnImages(): Boolean

@DoNotStrip public fun setAndroidLayoutDirection(): Boolean

@DoNotStrip public fun traceTurboModulePromiseRejectionsOnAndroid(): Boolean

@DoNotStrip public fun useAlwaysAvailableJSErrorHandling(): Boolean
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
import com.facebook.react.bridge.SoftAssertions;
import com.facebook.react.bridge.UiThreadUtil;
import com.facebook.react.common.build.ReactBuildConfig;
import com.facebook.react.internal.featureflags.ReactNativeFeatureFlags;
import com.facebook.react.touch.JSResponderHandler;
import com.facebook.react.uimanager.layoutanimation.LayoutAnimationController;
import com.facebook.react.uimanager.layoutanimation.LayoutAnimationListener;
Expand Down Expand Up @@ -178,9 +177,7 @@ public synchronized void updateLayout(
try {
View viewToUpdate = resolveView(tag);

if (ReactNativeFeatureFlags.setAndroidLayoutDirection()) {
viewToUpdate.setLayoutDirection(LayoutDirectionUtil.toAndroidFromYoga(layoutDirection));
}
viewToUpdate.setLayoutDirection(LayoutDirectionUtil.toAndroidFromYoga(layoutDirection));

// Even though we have exact dimensions, we still call measure because some platform views
// (e.g.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,6 @@
import com.facebook.react.bridge.ReactContext;
import com.facebook.react.common.ReactConstants;
import com.facebook.react.common.build.ReactBuildConfig;
import com.facebook.react.internal.featureflags.ReactNativeFeatureFlags;
import com.facebook.react.modules.i18nmanager.I18nUtil;
import com.facebook.react.uimanager.BackgroundStyleApplicator;
import com.facebook.react.uimanager.LengthPercentage;
import com.facebook.react.uimanager.LengthPercentageType;
Expand Down Expand Up @@ -139,13 +137,7 @@ public ReactHorizontalScrollView(Context context, @Nullable FpsListener fpsListe
ViewCompat.setAccessibilityDelegate(this, new ReactScrollViewAccessibilityDelegate());

mScroller = getOverScrollerFromParent();
mReactScrollViewScrollState =
new ReactScrollViewScrollState(
// TODO: The ScrollView content may be laid out in a different direction than the
// instance if the `direction` style is set on the ScrollView or above it.
I18nUtil.getInstance().isRTL(context)
? ViewCompat.LAYOUT_DIRECTION_RTL
: ViewCompat.LAYOUT_DIRECTION_LTR);
mReactScrollViewScrollState = new ReactScrollViewScrollState();

setOnHierarchyChangeListener(this);
setClipChildren(false);
Expand Down Expand Up @@ -1088,13 +1080,9 @@ private void flingAndSnap(int velocityX) {
int firstOffset = 0;
int lastOffset = maximumOffset;
int width = getWidth() - ViewCompat.getPaddingStart(this) - ViewCompat.getPaddingEnd(this);
int layoutDirection =
ReactNativeFeatureFlags.setAndroidLayoutDirection()
? getLayoutDirection()
: mReactScrollViewScrollState.getLayoutDirection();

// offsets are from the right edge in RTL layouts
if (layoutDirection == LAYOUT_DIRECTION_RTL) {
if (getLayoutDirection() == LAYOUT_DIRECTION_RTL) {
targetOffset = maximumOffset - targetOffset;
velocityX = -velocityX;
}
Expand Down Expand Up @@ -1184,7 +1172,7 @@ private void flingAndSnap(int velocityX) {
// if scrolling after the last snap offset and snapping to the
// end of the list is disabled, then we allow free scrolling
int currentOffset = getScrollX();
if (layoutDirection == LAYOUT_DIRECTION_RTL) {
if (getLayoutDirection() == LAYOUT_DIRECTION_RTL) {
currentOffset = maximumOffset - currentOffset;
}
if (!mSnapToEnd && targetOffset >= lastOffset) {
Expand Down Expand Up @@ -1224,7 +1212,7 @@ private void flingAndSnap(int velocityX) {
// Make sure the new offset isn't out of bounds
targetOffset = Math.min(Math.max(0, targetOffset), maximumOffset);

if (layoutDirection == LAYOUT_DIRECTION_RTL) {
if (getLayoutDirection() == LAYOUT_DIRECTION_RTL) {
targetOffset = maximumOffset - targetOffset;
velocityX = -velocityX;
}
Expand Down Expand Up @@ -1423,11 +1411,7 @@ public void onLayoutChange(
// does not shift layout. If `maintainVisibleContentPosition` is enabled, we try to adjust
// position so that the viewport keeps the same insets to previously visible views. TODO: MVCP
// does not work in RTL.
int layoutDirection =
ReactNativeFeatureFlags.setAndroidLayoutDirection()
? v.getLayoutDirection()
: mReactScrollViewScrollState.getLayoutDirection();
if (layoutDirection == LAYOUT_DIRECTION_RTL) {
if (v.getLayoutDirection() == LAYOUT_DIRECTION_RTL) {
adjustPositionForContentChangeRTL(left, right, oldLeft, oldRight);
} else if (mMaintainVisibleContentPositionHelper != null) {
mMaintainVisibleContentPositionHelper.updateScrollPosition();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ public class ReactScrollView extends ScrollView
private int pendingContentOffsetY = UNSET_CONTENT_OFFSET;
private @Nullable StateWrapper mStateWrapper = null;
private final ReactScrollViewScrollState mReactScrollViewScrollState =
new ReactScrollViewScrollState(ViewCompat.LAYOUT_DIRECTION_LTR);
new ReactScrollViewScrollState();
private final ValueAnimator DEFAULT_FLING_ANIMATOR = ObjectAnimator.ofInt(this, "scrollY", 0, 0);
private PointerEvents mPointerEvents = PointerEvents.AUTO;
private long mLastScrollDispatchTime = 0;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -331,25 +331,9 @@ public object ReactScrollViewHelper {
val scrollPos = scrollState.lastStateUpdateScroll
val scrollX = scrollPos.x
val scrollY = scrollPos.y
val layoutDirection = scrollState.layoutDirection
val fabricScrollX =
if (layoutDirection == View.LAYOUT_DIRECTION_RTL) {
// getScrollX returns offset from left even when layout direction is RTL.
// The following line calculates offset from right.
val child = scrollView.getChildAt(0)
val contentWidth = child?.width ?: 0
-(contentWidth - scrollX - scrollView.width)
} else {
scrollX
}
if (DEBUG_MODE) {
FLog.i(
TAG,
"updateFabricScrollState[%d] scrollX %d scrollY %d fabricScrollX %d",
scrollView.id,
scrollX,
scrollY,
fabricScrollX)
TAG, "updateFabricScrollState[%d] scrollX %d scrollY %d", scrollView.id, scrollX, scrollY)
}
val stateWrapper = scrollView.stateWrapper
if (stateWrapper != null) {
Expand Down Expand Up @@ -507,13 +491,7 @@ public object ReactScrollViewHelper {
}
}

public class ReactScrollViewScrollState(
/**
* Get the layout direction. Can be either scrollView.LAYOUT_DIRECTION_RTL (1) or
* scrollView.LAYOUT_DIRECTION_LTR (0). If the value is -1, it means unknown layout.
*/
public val layoutDirection: Int
) {
public class ReactScrollViewScrollState() {

/** Get the position after current animation is finished */
public val finalAnimatedPositionScroll: Point = Point()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,7 @@ inline int getIntBufferSizeForType(CppMountItem::Type mountItemType) {
case CppMountItem::Type::UpdatePadding:
return 5; // tag, top, left, bottom, right
case CppMountItem::Type::UpdateLayout:
return ReactNativeFeatureFlags::setAndroidLayoutDirection()
? 8 // tag, parentTag, x, y, w, h, DisplayType, LayoutDirection
: 7; // tag, parentTag, x, y, w, h, DisplayType
return 8; // tag, parentTag, x, y, w, h, DisplayType, LayoutDirection
case CppMountItem::Type::UpdateOverflowInset:
return 5; // tag, left, top, right, bottom
case CppMountItem::Undefined:
Expand Down Expand Up @@ -377,26 +375,15 @@ inline void writeUpdateLayoutMountItem(
int w = round(scale(frame.size.width, pointScaleFactor));
int h = round(scale(frame.size.height, pointScaleFactor));

if (ReactNativeFeatureFlags::setAndroidLayoutDirection()) {
buffer.writeIntArray(std::array<int, 8>{
mountItem.newChildShadowView.tag,
mountItem.parentShadowView.tag,
x,
y,
w,
h,
toInt(layoutMetrics.displayType),
toInt(layoutMetrics.layoutDirection)});
} else {
buffer.writeIntArray(std::array<int, 7>{
mountItem.newChildShadowView.tag,
mountItem.parentShadowView.tag,
x,
y,
w,
h,
toInt(layoutMetrics.displayType)});
}
buffer.writeIntArray(std::array<int, 8>{
mountItem.newChildShadowView.tag,
mountItem.parentShadowView.tag,
x,
y,
w,
h,
toInt(layoutMetrics.displayType),
toInt(layoutMetrics.layoutDirection)});
}

inline void writeUpdateEventEmitterMountItem(
Expand Down
Loading

0 comments on commit e88974f

Please sign in to comment.