From 6d206a3f54725f7f53692222293a9d1e58b11ca4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hanno=20J=2E=20G=C3=B6decke?= Date: Thu, 13 Jul 2023 19:11:07 -0700 Subject: [PATCH] Add workaround for android API 33 ANR when inverting ScrollView (#38071) Summary: This PR is a result of this PR, which got merged but then reverted: - https://github.com/facebook/react-native/pull/37913 We are trying to implement a workaround for https://github.com/facebook/react-native/issues/35350, so react-native users on android API 33+ can use `` without running into ANRs. This is the native part, where we add a new internal prop named `isInvertedVirtualizedList`, which can in a follow up change be used to achieve the final fix as proposed in https://github.com/facebook/react-native/pull/37913 However as NickGerleman pointed out, its important that we first ship the native change. ## Changelog: [ANDROID] [ADDED] - Native part of fixing ANR when having an inverted FlatList on android API 33+ Pull Request resolved: https://github.com/facebook/react-native/pull/38071 Test Plan: - Check the RN tester app and see that scrollview is still working as expected - Add the `isInvertedVirtualizedList` prop as test to a scrollview and see how the scrollbar will change position. Reviewed By: rozele Differential Revision: D47062200 Pulled By: NickGerleman fbshipit-source-id: d20eebeec757d9aaeced8561f53556bbb4a492e4 --- .../ScrollView/ScrollViewNativeComponent.js | 1 + .../ScrollViewNativeComponentType.js | 1 + .../ScrollView/ScrollViewViewConfig.js | 1 + .../views/scroll/ReactScrollViewManager.java | 18 ++++++++++++++++++ .../components/scrollview/ScrollViewProps.cpp | 16 +++++++++++++++- .../components/scrollview/ScrollViewProps.h | 1 + 6 files changed, 37 insertions(+), 1 deletion(-) diff --git a/packages/react-native/Libraries/Components/ScrollView/ScrollViewNativeComponent.js b/packages/react-native/Libraries/Components/ScrollView/ScrollViewNativeComponent.js index 2ba1886f478863..69ebcef0d56971 100644 --- a/packages/react-native/Libraries/Components/ScrollView/ScrollViewNativeComponent.js +++ b/packages/react-native/Libraries/Components/ScrollView/ScrollViewNativeComponent.js @@ -86,6 +86,7 @@ export const __INTERNAL_VIEW_CONFIG: PartialViewConfig = process: require('../../StyleSheet/processColor').default, }, pointerEvents: true, + isInvertedVirtualizedList: true, }, } : { diff --git a/packages/react-native/Libraries/Components/ScrollView/ScrollViewNativeComponentType.js b/packages/react-native/Libraries/Components/ScrollView/ScrollViewNativeComponentType.js index b1b2df5d84e370..f5a9632ee11999 100644 --- a/packages/react-native/Libraries/Components/ScrollView/ScrollViewNativeComponentType.js +++ b/packages/react-native/Libraries/Components/ScrollView/ScrollViewNativeComponentType.js @@ -41,6 +41,7 @@ export type ScrollViewNativeProps = $ReadOnly<{ endFillColor?: ?ColorValue, fadingEdgeLength?: ?number, indicatorStyle?: ?('default' | 'black' | 'white'), + isInvertedVirtualizedList?: ?boolean, keyboardDismissMode?: ?('none' | 'on-drag' | 'interactive'), maintainVisibleContentPosition?: ?$ReadOnly<{ minIndexForVisible: number, diff --git a/packages/react-native/Libraries/Components/ScrollView/ScrollViewViewConfig.js b/packages/react-native/Libraries/Components/ScrollView/ScrollViewViewConfig.js index 450b56657ec89a..55ba7b7abb0e88 100644 --- a/packages/react-native/Libraries/Components/ScrollView/ScrollViewViewConfig.js +++ b/packages/react-native/Libraries/Components/ScrollView/ScrollViewViewConfig.js @@ -46,6 +46,7 @@ const ScrollViewViewConfig = { fadingEdgeLength: true, indicatorStyle: true, inverted: true, + isInvertedVirtualizedList: true, keyboardDismissMode: true, maintainVisibleContentPosition: true, maximumZoomScale: true, 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 46e0ccf36c6e9d..b41b0858f6f83b 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 @@ -379,4 +379,22 @@ public void setPointerEvents(ReactScrollView view, @Nullable String pointerEvent public void setScrollEventThrottle(ReactScrollView view, int scrollEventThrottle) { view.setScrollEventThrottle(scrollEventThrottle); } + + @ReactProp(name = "isInvertedVirtualizedList") + public void setIsInvertedVirtualizedList(ReactScrollView view, boolean applyFix) { + // Usually when inverting the scroll view we are using scaleY: -1 on the list + // and on the parent container. HOWEVER, starting from android API 33 there is + // a bug that can cause an ANR due to that. Thus we are using different transform + // commands to circumvent the ANR. This however causes the vertical scrollbar to + // be on the wrong side. Thus we are moving it to the other side, when the list + // is inverted. + // See also: + // - https://github.com/facebook/react-native/issues/35350 + // - https://issuetracker.google.com/issues/287304310 + if (applyFix) { + view.setVerticalScrollbarPosition(View.SCROLLBAR_POSITION_LEFT); + } else { + view.setVerticalScrollbarPosition(View.SCROLLBAR_POSITION_DEFAULT); + } + } } diff --git a/packages/react-native/ReactCommon/react/renderer/components/scrollview/ScrollViewProps.cpp b/packages/react-native/ReactCommon/react/renderer/components/scrollview/ScrollViewProps.cpp index fb70d8a6176cab..87c7a5c6574223 100644 --- a/packages/react-native/ReactCommon/react/renderer/components/scrollview/ScrollViewProps.cpp +++ b/packages/react-native/ReactCommon/react/renderer/components/scrollview/ScrollViewProps.cpp @@ -319,6 +319,15 @@ ScrollViewProps::ScrollViewProps( rawProps, "scrollToOverflowEnabled", sourceProps.scrollToOverflowEnabled, + {})), + isInvertedVirtualizedList( + CoreFeatures::enablePropIteratorSetter + ? sourceProps.isInvertedVirtualizedList + : convertRawProp( + context, + rawProps, + "isInvertedVirtualizedList", + sourceProps.isInvertedVirtualizedList, {})) {} void ScrollViewProps::setProp( @@ -368,6 +377,7 @@ void ScrollViewProps::setProp( RAW_SET_PROP_SWITCH_CASE_BASIC(snapToEnd); RAW_SET_PROP_SWITCH_CASE_BASIC(contentInsetAdjustmentBehavior); RAW_SET_PROP_SWITCH_CASE_BASIC(scrollToOverflowEnabled); + RAW_SET_PROP_SWITCH_CASE_BASIC(isInvertedVirtualizedList); } } @@ -492,7 +502,11 @@ SharedDebugStringConvertibleList ScrollViewProps::getDebugProps() const { debugStringConvertibleItem( "snapToStart", snapToStart, defaultScrollViewProps.snapToStart), debugStringConvertibleItem( - "snapToEnd", snapToEnd, defaultScrollViewProps.snapToEnd)}; + "snapToEnd", snapToEnd, defaultScrollViewProps.snapToEnd), + debugStringConvertibleItem( + "isInvertedVirtualizedList", + snapToEnd, + defaultScrollViewProps.isInvertedVirtualizedList)}; } #endif diff --git a/packages/react-native/ReactCommon/react/renderer/components/scrollview/ScrollViewProps.h b/packages/react-native/ReactCommon/react/renderer/components/scrollview/ScrollViewProps.h index dea44da3af5d2b..72482e2002ec9d 100644 --- a/packages/react-native/ReactCommon/react/renderer/components/scrollview/ScrollViewProps.h +++ b/packages/react-native/ReactCommon/react/renderer/components/scrollview/ScrollViewProps.h @@ -68,6 +68,7 @@ class ScrollViewProps final : public ViewProps { ContentInsetAdjustmentBehavior contentInsetAdjustmentBehavior{ ContentInsetAdjustmentBehavior::Never}; bool scrollToOverflowEnabled{false}; + bool isInvertedVirtualizedList{false}; #pragma mark - DebugStringConvertible