Skip to content

Commit

Permalink
Fix maintainVisibleContentPosition on Android during momentum scroll (f…
Browse files Browse the repository at this point in the history
…acebook#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: facebook#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
  • Loading branch information
janicduplessis authored and facebook-github-bot committed Mar 15, 2024
1 parent 5c34ce0 commit a9e6759
Show file tree
Hide file tree
Showing 5 changed files with 88 additions and 17 deletions.
3 changes: 3 additions & 0 deletions packages/react-native/ReactAndroid/api/ReactAndroid.api
Original file line number Diff line number Diff line change
Expand Up @@ -6319,6 +6319,7 @@ public class com/facebook/react/views/scroll/ReactHorizontalScrollView : android
public fun reactSmoothScrollTo (II)V
public fun requestChildFocus (Landroid/view/View;Landroid/view/View;)V
public fun scrollTo (II)V
public fun scrollToPreservingMomentum (II)V
public fun setBackgroundColor (I)V
public fun setBorderColor (IFF)V
public fun setBorderRadius (F)V
Expand Down Expand Up @@ -6443,6 +6444,7 @@ public class com/facebook/react/views/scroll/ReactScrollView : android/widget/Sc
public fun reactSmoothScrollTo (II)V
public fun requestChildFocus (Landroid/view/View;Landroid/view/View;)V
public fun scrollTo (II)V
public fun scrollToPreservingMomentum (II)V
public fun setBackgroundColor (I)V
public fun setBorderColor (IFF)V
public fun setBorderRadius (F)V
Expand Down Expand Up @@ -6551,6 +6553,7 @@ public abstract interface class com/facebook/react/views/scroll/ReactScrollViewH

public abstract interface class com/facebook/react/views/scroll/ReactScrollViewHelper$HasSmoothScroll {
public abstract fun reactSmoothScrollTo (II)V
public abstract fun scrollToPreservingMomentum (II)V
}

public abstract interface class com/facebook/react/views/scroll/ReactScrollViewHelper$HasStateWrapper {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ private void updateScrollPositionInternal() {
int deltaX = newFrame.left - mPrevFirstVisibleFrame.left;
if (deltaX != 0) {
int scrollX = mScrollView.getScrollX();
mScrollView.scrollTo(scrollX + deltaX, mScrollView.getScrollY());
mScrollView.scrollToPreservingMomentum(scrollX + deltaX, mScrollView.getScrollY());
mPrevFirstVisibleFrame = newFrame;
if (mConfig.autoScrollToTopThreshold != null
&& scrollX <= mConfig.autoScrollToTopThreshold) {
Expand All @@ -129,7 +129,7 @@ private void updateScrollPositionInternal() {
int deltaY = newFrame.top - mPrevFirstVisibleFrame.top;
if (deltaY != 0) {
int scrollY = mScrollView.getScrollY();
mScrollView.scrollTo(mScrollView.getScrollX(), scrollY + deltaY);
mScrollView.scrollToPreservingMomentum(mScrollView.getScrollX(), scrollY + deltaY);
mPrevFirstVisibleFrame = newFrame;
if (mConfig.autoScrollToTopThreshold != null
&& scrollY <= mConfig.autoScrollToTopThreshold) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1324,6 +1324,13 @@ public void scrollTo(int x, int y) {
setPendingContentOffsets(x, y);
}

/** Scrolls to a new position preserving any momentum scrolling animation. */
@Override
public void scrollToPreservingMomentum(int x, int y) {
scrollTo(x, y);
recreateFlingAnimation(x, Integer.MAX_VALUE);
}

private boolean isContentReady() {
View child = getContentView();
return child != null && child.getWidth() != 0 && child.getHeight() != 0;
Expand Down Expand Up @@ -1377,24 +1384,21 @@ public void onLayoutChange(
}
}

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

int distanceToRightEdge = oldRight - getScrollX();
int newWidth = right - left;
int scrollX = newWidth - distanceToRightEdge;
scrollTo(scrollX, getScrollY());

// If we are in the middle of a fling animation from the user removing their finger
// (OverScroller is in `FLING_MODE`), we must cancel and recreate the existing fling animation
// since it was calculated against outdated scroll offsets.
if (mScroller != null && !mScroller.isFinished()) {
// Calculate the veliocity and position of the fling animation at the time of this layout
// Calculate the velocity and position of the fling animation at the time of this layout
// event, which may be later than the last ScrollView tick. These values are not commited to
// the underlying ScrollView, which will recalculate positions on its next tick.
int scrollerXBeforeTick = mScroller.getCurrX();
Expand All @@ -1413,14 +1417,29 @@ private void adjustPositionForContentChangeRTL(int left, int right, int oldLeft,
float direction = Math.signum(mScroller.getFinalX() - mScroller.getStartX());
float flingVelocityX = mScroller.getCurrVelocity() * direction;

mScroller.fling(
scrollX, getScrollY(), (int) flingVelocityX, 0, 0, newWidth - getWidth(), 0, 0);
mScroller.fling(scrollX, getScrollY(), (int) flingVelocityX, 0, 0, maxX, 0, 0);
} else {
scrollTo(scrollX + (mScroller.getCurrX() - scrollerXBeforeTick), getScrollY());
}
}
}

private void adjustPositionForContentChangeRTL(int left, int right, int oldLeft, int oldRight) {
// If we have any pending custom flings (e.g. from animated `scrollTo`, or flinging to a snap
// point), finish them, committing the final `scrollX`.
// TODO: Can we be more graceful (like OverScroller flings)?
if (getFlingAnimator().isRunning()) {
getFlingAnimator().end();
}

int distanceToRightEdge = oldRight - getScrollX();
int newWidth = right - left;
int scrollX = newWidth - distanceToRightEdge;
scrollTo(scrollX, getScrollY());

recreateFlingAnimation(scrollX, newWidth - getWidth());
}

@Nullable
public StateWrapper getStateWrapper() {
return mStateWrapper;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1091,6 +1091,53 @@ public void scrollTo(int x, int y) {
setPendingContentOffsets(x, y);
}

/**
* If we are in the middle of a fling animation from the user removing their finger (OverScroller
* is in `FLING_MODE`), recreate the existing fling animation since it was calculated against
* outdated scroll offsets.
*/
private void recreateFlingAnimation(int scrollY) {
// If we have any pending custom flings (e.g. from animated `scrollTo`, or flinging to a snap
// point), cancel them.
// TODO: Can we be more graceful (like OverScroller flings)?
if (getFlingAnimator().isRunning()) {
getFlingAnimator().cancel();
}

if (mScroller != null && !mScroller.isFinished()) {
// Calculate the velocity and position of the fling animation at the time of this layout
// event, which may be later than the last ScrollView tick. These values are not committed to
// the underlying ScrollView, which will recalculate positions on its next tick.
int scrollerYBeforeTick = mScroller.getCurrY();
boolean hasMoreTicks = mScroller.computeScrollOffset();

// Stop the existing animation at the current state of the scroller. We will then recreate
// it starting at the adjusted y offset.
mScroller.forceFinished(true);

if (hasMoreTicks) {
// OverScroller.getCurrVelocity() returns an absolute value of the velocity a current fling
// animation (only FLING_MODE animations). We derive direction along the Y axis from the
// start and end of the, animation assuming ScrollView never fires horizontal fling
// animations.
// TODO: This does not fully handle overscroll.
float direction = Math.signum(mScroller.getFinalY() - mScroller.getStartY());
float flingVelocityY = mScroller.getCurrVelocity() * direction;

mScroller.fling(getScrollX(), scrollY, 0, (int) flingVelocityY, 0, 0, 0, Integer.MAX_VALUE);
} else {
scrollTo(getScrollX(), scrollY + (mScroller.getCurrX() - scrollerYBeforeTick));
}
}
}

/** Scrolls to a new position preserving any momentum scrolling animation. */
@Override
public void scrollToPreservingMomentum(int x, int y) {
scrollTo(x, y);
recreateFlingAnimation(y);
}

private boolean isContentReady() {
View child = getContentView();
return child != null && child.getWidth() != 0 && child.getHeight() != 0;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -597,5 +597,7 @@ public interface HasScrollEventThrottle {

public interface HasSmoothScroll {
void reactSmoothScrollTo(int x, int y);

void scrollToPreservingMomentum(int x, int y);
}
}

0 comments on commit a9e6759

Please sign in to comment.