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
zhxlp committed Aug 1, 2023
1 parent a2e0984 commit 9c37a37
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 2 deletions.
11 changes: 10 additions & 1 deletion Libraries/Lists/VirtualizedList.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

const Batchinator = require('../Interaction/Batchinator');
const FillRateHelper = require('./FillRateHelper');
const Platform = require('../Utilities/Platform');
const ReactNative = require('../Renderer/shims/ReactNative');
const RefreshControl = require('../Components/RefreshControl/RefreshControl');
const ScrollView = require('../Components/ScrollView/ScrollView');
Expand Down Expand Up @@ -2119,7 +2120,15 @@ function describeNestedLists(childList: {

const styles = StyleSheet.create({
verticallyInverted: {
transform: [{scaleY: -1}],
transform:
// Android 13 Bug Workaround:
// On Android, we need to invert both axes to mitigate a native bug
// that could lead to ANRs.
// Simply using scaleY: -1 leads to the application of scaleY and
// rotationX natively, resulting in the ANR.
// For more information, refer to the following Android tracking issue:
// https://issuetracker.google.com/issues/287304310
Platform.OS === 'android' ? [{scale: -1}] : [{scaleY: -1}],
},
horizontallyInverted: {
transform: [{scaleX: -1}],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -352,4 +352,22 @@ public static Map<String, Object> createExportedCustomDirectEventTypeConstants()
MapBuilder.of("registrationName", "onMomentumScrollEnd"))
.build();
}

@ReactProp(name = "inverted")
public void setInverted(ReactScrollView view, boolean inverted) {
// 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 (inverted) {
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 @@ -218,6 +218,12 @@ ScrollViewProps::ScrollViewProps(
rawProps,
"scrollToOverflowEnabled",
sourceProps.scrollToOverflowEnabled,
{})),
inverted(convertRawProp(
context,
rawProps,
"inverted",
sourceProps.inverted,
{})) {}

#pragma mark - DebugStringConvertible
Expand Down Expand Up @@ -337,7 +343,9 @@ SharedDebugStringConvertibleList ScrollViewProps::getDebugProps() const {
debugStringConvertibleItem(
"snapToStart", snapToStart, defaultScrollViewProps.snapToStart),
debugStringConvertibleItem(
"snapToEnd", snapToEnd, defaultScrollViewProps.snapToEnd)};
"snapToEnd", snapToEnd, defaultScrollViewProps.snapToEnd),
debugStringConvertibleItem(
"inverted", inverted, defaultScrollViewProps.inverted)};
}
#endif

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

#pragma mark - DebugStringConvertible

Expand Down

0 comments on commit 9c37a37

Please sign in to comment.