Skip to content

Commit a9e6759

Browse files
janicduplessisfacebook-github-bot
authored andcommitted
Fix maintainVisibleContentPosition on Android during momentum scroll (#43425)
Summary: When using maintainVisibleContentPosition (mvcp) on Android it doesn't work properly when items are added during a momentum scroll. This happens because the android scrollview momentum scroll animation overrides the scroll position that the mvcp implementation sets [here](https://github.com/facebook/react-native/blob/2d547a3252b328251e49dabfeec85f8d46c85411/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/scroll/MaintainVisibleScrollPositionHelper.java#L132). To fix this we need to cancel the momentum scroll animation, update the scroll position then restart the scroll animation with the previous animation remaining momentum. ## Changelog: [ANDROID] [FIXED] - Fix maintainVisibleContentPosition during momentum scroll Pull Request resolved: #43425 Test Plan: Tested in RNTester on Android with both vertical and horizontal scrollviews using the following code: ```ts // packages/rn-tester/js/RNTesterAppShared.js import { Button, SafeAreaView, ScrollView, Text, View, } from 'react-native'; import React, {useLayoutEffect, useRef, useState} from 'react'; const generateUniqueKey = () => `_${Math.random().toString(36).substr(2, 9)}` const initialData = Array.from(Array(100).keys()).map(n => ({ id: generateUniqueKey(), value: n, })) function ListItem({item}) { const color = `hsl(${item.value * 10}, 75%, 85%)`; return ( <View style={{ backgroundColor: color, height: 100, }}> <Text>List item: {item.value}</Text> </View> ); } export default function FlatListRepro() { const numToAdd = 10; const [numbers, setNumbers] = useState(initialData); const ref = useRef(); const addAbove = () => { setNumbers(prev => { const additionalNumbers = Array.from(Array(numToAdd).keys()) .map(n => ({ id: generateUniqueKey(), value: prev[0].value - n - 1, })) .reverse(); return additionalNumbers.concat(prev); }); }; useLayoutEffect(() => { ref.current.scrollTo({y: numbers.length * 100, animated: false}); // eslint-disable-next-line react-hooks/exhaustive-deps }, []); return ( <SafeAreaView style={{flex: 1}}> <View style={{flexDirection: 'row', alignItems: 'center'}}> <Button title="Add Above" onPress={addAbove} /> </View> <View> <ScrollView ref={ref} maintainVisibleContentPosition={{ minIndexForVisible: 0, }} > {numbers.map((item) => ( <ListItem key={item.id} item={item} /> ))} </ScrollView> </View> </SafeAreaView> ) } ``` Before: https://github.com/facebook/react-native/assets/2677334/a7102665-991e-449e-9d0a-ef06c370dc71 After: https://github.com/facebook/react-native/assets/2677334/5430ecb1-26a9-4c92-9f16-c762d75685db Reviewed By: javache Differential Revision: D54883984 Pulled By: NickGerleman fbshipit-source-id: 95fd673a87cf5ada3bf9a7c166bba77ce557c89b
1 parent 5c34ce0 commit a9e6759

File tree

5 files changed

+88
-17
lines changed

5 files changed

+88
-17
lines changed

packages/react-native/ReactAndroid/api/ReactAndroid.api

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6319,6 +6319,7 @@ public class com/facebook/react/views/scroll/ReactHorizontalScrollView : android
63196319
public fun reactSmoothScrollTo (II)V
63206320
public fun requestChildFocus (Landroid/view/View;Landroid/view/View;)V
63216321
public fun scrollTo (II)V
6322+
public fun scrollToPreservingMomentum (II)V
63226323
public fun setBackgroundColor (I)V
63236324
public fun setBorderColor (IFF)V
63246325
public fun setBorderRadius (F)V
@@ -6443,6 +6444,7 @@ public class com/facebook/react/views/scroll/ReactScrollView : android/widget/Sc
64436444
public fun reactSmoothScrollTo (II)V
64446445
public fun requestChildFocus (Landroid/view/View;Landroid/view/View;)V
64456446
public fun scrollTo (II)V
6447+
public fun scrollToPreservingMomentum (II)V
64466448
public fun setBackgroundColor (I)V
64476449
public fun setBorderColor (IFF)V
64486450
public fun setBorderRadius (F)V
@@ -6551,6 +6553,7 @@ public abstract interface class com/facebook/react/views/scroll/ReactScrollViewH
65516553

65526554
public abstract interface class com/facebook/react/views/scroll/ReactScrollViewHelper$HasSmoothScroll {
65536555
public abstract fun reactSmoothScrollTo (II)V
6556+
public abstract fun scrollToPreservingMomentum (II)V
65546557
}
65556558

65566559
public abstract interface class com/facebook/react/views/scroll/ReactScrollViewHelper$HasStateWrapper {

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/scroll/MaintainVisibleScrollPositionHelper.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ private void updateScrollPositionInternal() {
118118
int deltaX = newFrame.left - mPrevFirstVisibleFrame.left;
119119
if (deltaX != 0) {
120120
int scrollX = mScrollView.getScrollX();
121-
mScrollView.scrollTo(scrollX + deltaX, mScrollView.getScrollY());
121+
mScrollView.scrollToPreservingMomentum(scrollX + deltaX, mScrollView.getScrollY());
122122
mPrevFirstVisibleFrame = newFrame;
123123
if (mConfig.autoScrollToTopThreshold != null
124124
&& scrollX <= mConfig.autoScrollToTopThreshold) {
@@ -129,7 +129,7 @@ private void updateScrollPositionInternal() {
129129
int deltaY = newFrame.top - mPrevFirstVisibleFrame.top;
130130
if (deltaY != 0) {
131131
int scrollY = mScrollView.getScrollY();
132-
mScrollView.scrollTo(mScrollView.getScrollX(), scrollY + deltaY);
132+
mScrollView.scrollToPreservingMomentum(mScrollView.getScrollX(), scrollY + deltaY);
133133
mPrevFirstVisibleFrame = newFrame;
134134
if (mConfig.autoScrollToTopThreshold != null
135135
&& scrollY <= mConfig.autoScrollToTopThreshold) {

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactHorizontalScrollView.java

Lines changed: 34 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1324,6 +1324,13 @@ public void scrollTo(int x, int y) {
13241324
setPendingContentOffsets(x, y);
13251325
}
13261326

1327+
/** Scrolls to a new position preserving any momentum scrolling animation. */
1328+
@Override
1329+
public void scrollToPreservingMomentum(int x, int y) {
1330+
scrollTo(x, y);
1331+
recreateFlingAnimation(x, Integer.MAX_VALUE);
1332+
}
1333+
13271334
private boolean isContentReady() {
13281335
View child = getContentView();
13291336
return child != null && child.getWidth() != 0 && child.getHeight() != 0;
@@ -1377,24 +1384,21 @@ public void onLayoutChange(
13771384
}
13781385
}
13791386

1380-
private void adjustPositionForContentChangeRTL(int left, int right, int oldLeft, int oldRight) {
1381-
// If we have any pending custon flings (e.g. from aninmated `scrollTo`, or flinging to a snap
1382-
// point), finish them, commiting the final `scrollX`.
1387+
/**
1388+
* If we are in the middle of a fling animation from the user removing their finger (OverScroller
1389+
* is in `FLING_MODE`), recreate the existing fling animation since it was calculated against
1390+
* outdated scroll offsets.
1391+
*/
1392+
private void recreateFlingAnimation(int scrollX, int maxX) {
1393+
// If we have any pending custom flings (e.g. from animated `scrollTo`, or flinging to a snap
1394+
// point), cancel them.
13831395
// TODO: Can we be more graceful (like OverScroller flings)?
13841396
if (getFlingAnimator().isRunning()) {
1385-
getFlingAnimator().end();
1397+
getFlingAnimator().cancel();
13861398
}
13871399

1388-
int distanceToRightEdge = oldRight - getScrollX();
1389-
int newWidth = right - left;
1390-
int scrollX = newWidth - distanceToRightEdge;
1391-
scrollTo(scrollX, getScrollY());
1392-
1393-
// If we are in the middle of a fling animation from the user removing their finger
1394-
// (OverScroller is in `FLING_MODE`), we must cancel and recreate the existing fling animation
1395-
// since it was calculated against outdated scroll offsets.
13961400
if (mScroller != null && !mScroller.isFinished()) {
1397-
// Calculate the veliocity and position of the fling animation at the time of this layout
1401+
// Calculate the velocity and position of the fling animation at the time of this layout
13981402
// event, which may be later than the last ScrollView tick. These values are not commited to
13991403
// the underlying ScrollView, which will recalculate positions on its next tick.
14001404
int scrollerXBeforeTick = mScroller.getCurrX();
@@ -1413,14 +1417,29 @@ private void adjustPositionForContentChangeRTL(int left, int right, int oldLeft,
14131417
float direction = Math.signum(mScroller.getFinalX() - mScroller.getStartX());
14141418
float flingVelocityX = mScroller.getCurrVelocity() * direction;
14151419

1416-
mScroller.fling(
1417-
scrollX, getScrollY(), (int) flingVelocityX, 0, 0, newWidth - getWidth(), 0, 0);
1420+
mScroller.fling(scrollX, getScrollY(), (int) flingVelocityX, 0, 0, maxX, 0, 0);
14181421
} else {
14191422
scrollTo(scrollX + (mScroller.getCurrX() - scrollerXBeforeTick), getScrollY());
14201423
}
14211424
}
14221425
}
14231426

1427+
private void adjustPositionForContentChangeRTL(int left, int right, int oldLeft, int oldRight) {
1428+
// If we have any pending custom flings (e.g. from animated `scrollTo`, or flinging to a snap
1429+
// point), finish them, committing the final `scrollX`.
1430+
// TODO: Can we be more graceful (like OverScroller flings)?
1431+
if (getFlingAnimator().isRunning()) {
1432+
getFlingAnimator().end();
1433+
}
1434+
1435+
int distanceToRightEdge = oldRight - getScrollX();
1436+
int newWidth = right - left;
1437+
int scrollX = newWidth - distanceToRightEdge;
1438+
scrollTo(scrollX, getScrollY());
1439+
1440+
recreateFlingAnimation(scrollX, newWidth - getWidth());
1441+
}
1442+
14241443
@Nullable
14251444
public StateWrapper getStateWrapper() {
14261445
return mStateWrapper;

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactScrollView.java

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1091,6 +1091,53 @@ public void scrollTo(int x, int y) {
10911091
setPendingContentOffsets(x, y);
10921092
}
10931093

1094+
/**
1095+
* If we are in the middle of a fling animation from the user removing their finger (OverScroller
1096+
* is in `FLING_MODE`), recreate the existing fling animation since it was calculated against
1097+
* outdated scroll offsets.
1098+
*/
1099+
private void recreateFlingAnimation(int scrollY) {
1100+
// If we have any pending custom flings (e.g. from animated `scrollTo`, or flinging to a snap
1101+
// point), cancel them.
1102+
// TODO: Can we be more graceful (like OverScroller flings)?
1103+
if (getFlingAnimator().isRunning()) {
1104+
getFlingAnimator().cancel();
1105+
}
1106+
1107+
if (mScroller != null && !mScroller.isFinished()) {
1108+
// Calculate the velocity and position of the fling animation at the time of this layout
1109+
// event, which may be later than the last ScrollView tick. These values are not committed to
1110+
// the underlying ScrollView, which will recalculate positions on its next tick.
1111+
int scrollerYBeforeTick = mScroller.getCurrY();
1112+
boolean hasMoreTicks = mScroller.computeScrollOffset();
1113+
1114+
// Stop the existing animation at the current state of the scroller. We will then recreate
1115+
// it starting at the adjusted y offset.
1116+
mScroller.forceFinished(true);
1117+
1118+
if (hasMoreTicks) {
1119+
// OverScroller.getCurrVelocity() returns an absolute value of the velocity a current fling
1120+
// animation (only FLING_MODE animations). We derive direction along the Y axis from the
1121+
// start and end of the, animation assuming ScrollView never fires horizontal fling
1122+
// animations.
1123+
// TODO: This does not fully handle overscroll.
1124+
float direction = Math.signum(mScroller.getFinalY() - mScroller.getStartY());
1125+
float flingVelocityY = mScroller.getCurrVelocity() * direction;
1126+
1127+
mScroller.fling(getScrollX(), scrollY, 0, (int) flingVelocityY, 0, 0, 0, Integer.MAX_VALUE);
1128+
} else {
1129+
scrollTo(getScrollX(), scrollY + (mScroller.getCurrX() - scrollerYBeforeTick));
1130+
}
1131+
}
1132+
}
1133+
1134+
/** Scrolls to a new position preserving any momentum scrolling animation. */
1135+
@Override
1136+
public void scrollToPreservingMomentum(int x, int y) {
1137+
scrollTo(x, y);
1138+
recreateFlingAnimation(y);
1139+
}
1140+
10941141
private boolean isContentReady() {
10951142
View child = getContentView();
10961143
return child != null && child.getWidth() != 0 && child.getHeight() != 0;

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactScrollViewHelper.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -597,5 +597,7 @@ public interface HasScrollEventThrottle {
597597

598598
public interface HasSmoothScroll {
599599
void reactSmoothScrollTo(int x, int y);
600+
601+
void scrollToPreservingMomentum(int x, int y);
600602
}
601603
}

0 commit comments

Comments
 (0)