From 0fa7e01c5fa9d0d644848d15010afc6d60ebd303 Mon Sep 17 00:00:00 2001 From: Nick Gerleman Date: Mon, 16 Sep 2024 19:14:53 -0700 Subject: [PATCH] Remove ReactScrollView Legacy Background Path (#46168) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/46168 ## This Diff This removes the legacy path from ReactScrollView and its view manager. ## This Stack This removes the non-Style-applicator background management paths of the different native components. There have been multiple conflicting changes, and bugs added bc harder to reason about, which motivates making this change as soon as possible. This also lets us formalize guarantees that BaseViewManager may safely manipulate background styling of all built in native components. There is one still known issue, where BackgroundStyleApplicator does not propagate I18nManager derived layout direction to borders (compared to Android derived root direction). This is mostly an issue for apps that with LTR and RTL context, or force a layout direction, which I would guess is relatively rare, so my plan is to forward fix this later this by enabling set_android_layout_direction which will solve that problem mopre generically. Changelog: [Internal] Reviewed By: tdn120 Differential Revision: D61658081 fbshipit-source-id: d6db43e25faf8e1ebd42d2816c7a915b3ed9404e --- .../react/views/scroll/ReactScrollView.java | 61 +++++------------- .../views/scroll/ReactScrollViewManager.java | 63 +++++-------------- 2 files changed, 29 insertions(+), 95 deletions(-) diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactScrollView.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactScrollView.java index 3995b563300..3d18ba1986b 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactScrollView.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactScrollView.java @@ -39,7 +39,6 @@ import com.facebook.react.animated.NativeAnimatedModule; import com.facebook.react.bridge.ReactContext; import com.facebook.react.bridge.ReadableMap; import com.facebook.react.common.ReactConstants; -import com.facebook.react.internal.featureflags.ReactNativeFeatureFlags; import com.facebook.react.uimanager.BackgroundStyleApplicator; import com.facebook.react.uimanager.LengthPercentage; import com.facebook.react.uimanager.LengthPercentageType; @@ -50,7 +49,6 @@ import com.facebook.react.uimanager.ReactClippingViewGroup; import com.facebook.react.uimanager.ReactClippingViewGroupHelper; import com.facebook.react.uimanager.ReactOverflowViewWithInset; import com.facebook.react.uimanager.StateWrapper; -import com.facebook.react.uimanager.ViewProps; import com.facebook.react.uimanager.events.NativeGestureUtil; import com.facebook.react.uimanager.style.BorderRadiusProp; import com.facebook.react.uimanager.style.BorderStyle; @@ -62,7 +60,6 @@ import com.facebook.react.views.scroll.ReactScrollViewHelper.HasScrollState; import com.facebook.react.views.scroll.ReactScrollViewHelper.HasSmoothScroll; import com.facebook.react.views.scroll.ReactScrollViewHelper.HasStateWrapper; import com.facebook.react.views.scroll.ReactScrollViewHelper.ReactScrollViewScrollState; -import com.facebook.react.views.view.ReactViewBackgroundManager; import com.facebook.systrace.Systrace; import java.lang.reflect.Field; import java.util.List; @@ -119,7 +116,6 @@ public class ReactScrollView extends ScrollView private boolean mSnapToEnd = true; private int mSnapToAlignment = SNAP_ALIGNMENT_DISABLED; private @Nullable View mContentView; - private ReactViewBackgroundManager mReactBackgroundManager; private @Nullable ReadableMap mCurrentContentOffset = null; private int pendingContentOffsetX = UNSET_CONTENT_OFFSET; private int pendingContentOffsetY = UNSET_CONTENT_OFFSET; @@ -140,13 +136,11 @@ public class ReactScrollView extends ScrollView public ReactScrollView(Context context, @Nullable FpsListener fpsListener) { super(context); mFpsListener = fpsListener; - mReactBackgroundManager = new ReactViewBackgroundManager(this); mScroller = getOverScrollerFromParent(); setOnHierarchyChangeListener(this); setScrollBarStyle(SCROLLBARS_OUTSIDE_OVERLAY); setClipChildren(false); - mReactBackgroundManager.setOverflow(ViewProps.SCROLL); ViewCompat.setAccessibilityDelegate(this, new ReactScrollViewAccessibilityDelegate()); } @@ -278,7 +272,6 @@ public class ReactScrollView extends ScrollView mOverflow = parsedOverflow == null ? Overflow.SCROLL : parsedOverflow; } - mReactBackgroundManager.setOverflow(overflow == null ? ViewProps.SCROLL : overflow); invalidate(); } @@ -684,12 +677,8 @@ public class ReactScrollView extends ScrollView @Override public void onDraw(Canvas canvas) { - if (ReactNativeFeatureFlags.enableBackgroundStyleApplicator()) { - if (mOverflow != Overflow.VISIBLE) { - BackgroundStyleApplicator.clipToPaddingBox(this, canvas); - } - } else { - mReactBackgroundManager.maybeClipToPaddingBox(canvas); + if (mOverflow != Overflow.VISIBLE) { + BackgroundStyleApplicator.clipToPaddingBox(this, canvas); } super.onDraw(canvas); } @@ -1261,28 +1250,16 @@ public class ReactScrollView extends ScrollView @Override public void setBackgroundColor(int color) { - if (ReactNativeFeatureFlags.enableBackgroundStyleApplicator()) { - BackgroundStyleApplicator.setBackgroundColor(this, color); - } else { - mReactBackgroundManager.setBackgroundColor(color); - } + BackgroundStyleApplicator.setBackgroundColor(this, color); } public void setBorderWidth(int position, float width) { - if (ReactNativeFeatureFlags.enableBackgroundStyleApplicator()) { - BackgroundStyleApplicator.setBorderWidth( - this, LogicalEdge.values()[position], PixelUtil.toDIPFromPixel(width)); - } else { - mReactBackgroundManager.setBorderWidth(position, width); - } + BackgroundStyleApplicator.setBorderWidth( + this, LogicalEdge.values()[position], PixelUtil.toDIPFromPixel(width)); } public void setBorderColor(int position, @Nullable Integer color) { - if (ReactNativeFeatureFlags.enableBackgroundStyleApplicator()) { - BackgroundStyleApplicator.setBorderColor(this, LogicalEdge.values()[position], color); - } else { - mReactBackgroundManager.setBorderColor(position, color); - } + BackgroundStyleApplicator.setBorderColor(this, LogicalEdge.values()[position], color); } public void setBorderRadius(float borderRadius) { @@ -1290,26 +1267,18 @@ public class ReactScrollView extends ScrollView } public void setBorderRadius(float borderRadius, int position) { - if (ReactNativeFeatureFlags.enableBackgroundStyleApplicator()) { - @Nullable - LengthPercentage radius = - Float.isNaN(borderRadius) - ? null - : new LengthPercentage( - PixelUtil.toDIPFromPixel(borderRadius), LengthPercentageType.POINT); - BackgroundStyleApplicator.setBorderRadius(this, BorderRadiusProp.values()[position], radius); - } else { - mReactBackgroundManager.setBorderRadius(borderRadius, position); - } + @Nullable + LengthPercentage radius = + Float.isNaN(borderRadius) + ? null + : new LengthPercentage( + PixelUtil.toDIPFromPixel(borderRadius), LengthPercentageType.POINT); + BackgroundStyleApplicator.setBorderRadius(this, BorderRadiusProp.values()[position], radius); } public void setBorderStyle(@Nullable String style) { - if (ReactNativeFeatureFlags.enableBackgroundStyleApplicator()) { - BackgroundStyleApplicator.setBorderStyle( - this, style == null ? null : BorderStyle.fromString(style)); - } else { - mReactBackgroundManager.setBorderStyle(style); - } + BackgroundStyleApplicator.setBorderStyle( + this, style == null ? null : BorderStyle.fromString(style)); } /** diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactScrollViewManager.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactScrollViewManager.java index 94f45bb7a2c..8ed2e47d8a3 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactScrollViewManager.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactScrollViewManager.java @@ -17,7 +17,6 @@ import com.facebook.react.bridge.ReadableArray; import com.facebook.react.bridge.ReadableMap; import com.facebook.react.bridge.RetryableMountingLayerException; import com.facebook.react.common.MapBuilder; -import com.facebook.react.internal.featureflags.ReactNativeFeatureFlags; import com.facebook.react.module.annotations.ReactModule; import com.facebook.react.uimanager.BackgroundStyleApplicator; import com.facebook.react.uimanager.LengthPercentage; @@ -245,36 +244,20 @@ public class ReactScrollViewManager extends ViewGroupManager }, defaultFloat = Float.NaN) public void setBorderRadius(ReactScrollView view, int index, float borderRadius) { - if (ReactNativeFeatureFlags.enableBackgroundStyleApplicator()) { - @Nullable - LengthPercentage radius = - Float.isNaN(borderRadius) - ? null - : new LengthPercentage(borderRadius, LengthPercentageType.POINT); - BackgroundStyleApplicator.setBorderRadius(view, BorderRadiusProp.values()[index], radius); - } else { - if (!Float.isNaN(borderRadius)) { - borderRadius = PixelUtil.toPixelFromDIP(borderRadius); - } - - if (index == 0) { - view.setBorderRadius(borderRadius); - } else { - view.setBorderRadius(borderRadius, index - 1); - } - } + @Nullable + LengthPercentage radius = + Float.isNaN(borderRadius) + ? null + : new LengthPercentage(borderRadius, LengthPercentageType.POINT); + BackgroundStyleApplicator.setBorderRadius(view, BorderRadiusProp.values()[index], radius); } @ReactProp(name = "borderStyle") public void setBorderStyle(ReactScrollView view, @Nullable String borderStyle) { - if (ReactNativeFeatureFlags.enableBackgroundStyleApplicator()) { - @Nullable - BorderStyle parsedBorderStyle = - borderStyle == null ? null : BorderStyle.fromString(borderStyle); - BackgroundStyleApplicator.setBorderStyle(view, parsedBorderStyle); - } else { - view.setBorderStyle(borderStyle); - } + @Nullable + BorderStyle parsedBorderStyle = + borderStyle == null ? null : BorderStyle.fromString(borderStyle); + BackgroundStyleApplicator.setBorderStyle(view, parsedBorderStyle); } @ReactPropGroup( @@ -287,15 +270,7 @@ public class ReactScrollViewManager extends ViewGroupManager }, defaultFloat = Float.NaN) public void setBorderWidth(ReactScrollView view, int index, float width) { - if (ReactNativeFeatureFlags.enableBackgroundStyleApplicator()) { - BackgroundStyleApplicator.setBorderWidth(view, LogicalEdge.values()[index], width); - } else { - if (!Float.isNaN(width)) { - width = PixelUtil.toPixelFromDIP(width); - } - - view.setBorderWidth(SPACING_TYPES[index], width); - } + BackgroundStyleApplicator.setBorderWidth(view, LogicalEdge.values()[index], width); } @ReactPropGroup( @@ -308,11 +283,7 @@ public class ReactScrollViewManager extends ViewGroupManager }, customType = "Color") public void setBorderColor(ReactScrollView view, int index, @Nullable Integer color) { - if (ReactNativeFeatureFlags.enableBackgroundStyleApplicator()) { - BackgroundStyleApplicator.setBorderColor(view, LogicalEdge.ALL, color); - } else { - view.setBorderColor(SPACING_TYPES[index], color); - } + BackgroundStyleApplicator.setBorderColor(view, LogicalEdge.ALL, color); } @ReactProp(name = "overflow") @@ -374,18 +345,12 @@ public class ReactScrollViewManager extends ViewGroupManager @ReactProp(name = ViewProps.BOX_SHADOW, customType = "BoxShadow") public void setBoxShadow(ReactScrollView view, @Nullable ReadableArray shadows) { - if (ReactNativeFeatureFlags.enableBackgroundStyleApplicator()) { - BackgroundStyleApplicator.setBoxShadow(view, shadows); - } + BackgroundStyleApplicator.setBoxShadow(view, shadows); } @Override public void setBackgroundColor(ReactScrollView view, @ColorInt int backgroundColor) { - if (ReactNativeFeatureFlags.enableBackgroundStyleApplicator()) { - BackgroundStyleApplicator.setBackgroundColor(view, backgroundColor); - } else { - super.setBackgroundColor(view, backgroundColor); - } + BackgroundStyleApplicator.setBackgroundColor(view, backgroundColor); } @Override