Skip to content

Commit

Permalink
Add workaround for android API 33 ANR when inverting ScrollView (face…
Browse files Browse the repository at this point in the history
…book#38071)

Summary:
This PR is a result of this PR, which got merged but then reverted:

- facebook#37913

We are trying to implement a workaround for facebook#35350, so react-native users on android API 33+ can use `<FlatList inverted={true} />` 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 facebook#37913

However as NickGerleman pointed out, its important that we first ship the native change.

## Changelog:

<!-- Help reviewers and the release process by writing your own changelog entry.

Pick one each for the category and type tags:

[ANDROID|GENERAL|IOS|INTERNAL] [BREAKING|ADDED|CHANGED|DEPRECATED|REMOVED|FIXED|SECURITY] - Message

For more details, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests
-->

[ANDROID] [ADDED] - Native part of fixing ANR when having an inverted FlatList on android API 33+

Pull Request resolved: facebook#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
  • Loading branch information
hannojg authored and Kudo committed Aug 21, 2023
1 parent 626fe5a commit 265352d
Show file tree
Hide file tree
Showing 6 changed files with 37 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ export const __INTERNAL_VIEW_CONFIG: PartialViewConfig =
process: require('../../StyleSheet/processColor').default,
},
pointerEvents: true,
isInvertedVirtualizedList: true,
},
}
: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ const ScrollViewViewConfig = {
fadingEdgeLength: true,
indicatorStyle: true,
inverted: true,
isInvertedVirtualizedList: true,
keyboardDismissMode: true,
maintainVisibleContentPosition: true,
maximumZoomScale: true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -319,6 +319,15 @@ ScrollViewProps::ScrollViewProps(
rawProps,
"scrollToOverflowEnabled",
sourceProps.scrollToOverflowEnabled,
{})),
isInvertedVirtualizedList(
CoreFeatures::enablePropIteratorSetter
? sourceProps.isInvertedVirtualizedList
: convertRawProp(
context,
rawProps,
"isInvertedVirtualizedList",
sourceProps.isInvertedVirtualizedList,
{})) {}

void ScrollViewProps::setProp(
Expand Down Expand Up @@ -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);
}
}

Expand Down Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ class ScrollViewProps final : public ViewProps {
ContentInsetAdjustmentBehavior contentInsetAdjustmentBehavior{
ContentInsetAdjustmentBehavior::Never};
bool scrollToOverflowEnabled{false};
bool isInvertedVirtualizedList{false};

#pragma mark - DebugStringConvertible

Expand Down

0 comments on commit 265352d

Please sign in to comment.