From 06668fcbacd750771f1d53cce829dc55e86f3f3c Mon Sep 17 00:00:00 2001 From: AntoineDoubovetzky Date: Thu, 13 Jul 2023 20:32:21 -0700 Subject: [PATCH] (Android/ScrollView) Fix onMomentumScrollEnd being called multiple times (#32433) Summary: I noticed that `onMomentumScrollEnd` is called multiple times on Android. 1. It is always called three times with the last value 2. It is sometimes called many times befire the scrolling stops when the pagingEnabled prop is true See: I used the following code to get the logs: ``` import React from 'react'; import {SafeAreaView, ScrollView, Text, View} from 'react-native'; const App = () => { const onMomentumScrollEnd = ({nativeEvent}) => { console.log( 'onMomentumScrollEnd', nativeEvent.contentOffset.x, nativeEvent.contentOffset.y, ); }; const onMomentumScrollBegin = ({nativeEvent}) => { console.log( 'onMomentumScrollBegin', nativeEvent.contentOffset.x, nativeEvent.contentOffset.y, ); }; return ( {new Array(10).fill(0).map((_, index) => { return ( {index} ); })} ); }; export default App; ``` From what I understood: 1. We do not check that `mStableFrames` is >= 3 when emitting the event (line 798) and we keep executing the runnable, so it is emitted until `mStableFrames` equals 3. When `mStableFrames` equals 3 we stop executing the runnable (line 809). That's why it gets called 3 times. 2. When `pagingEnabled` is true, the `postOnAnimationDelayed` method is called twice (line 794 and line 806). I believe it causes the runnable to be executing too often, and the `onMomentumScrollEnd` event to be emitted too many times. I updated the code so: 1. The event is emitted only once, and at the same time we stop executing the runnable 2. The `postOnAnimationDelayed` method is called at most once per execution of the runnable ## Changelog [Android] [Fixed] - Fix ScrollView's onMomentumScrollEnd being called multiple times on Android Pull Request resolved: https://github.com/facebook/react-native/pull/32433 Test Plan: I tested using the code above with every combination of `horizontal` and `pagingEnabled` values. Reviewed By: NickGerleman Differential Revision: D47297163 Pulled By: ryancat fbshipit-source-id: 7c31175d941ff13bed20dac03fb92d2b56e94dec --- .../scroll/ReactHorizontalScrollView.java | 36 ++++++++----------- .../react/views/scroll/ReactScrollView.java | 36 ++++++++----------- 2 files changed, 30 insertions(+), 42 deletions(-) diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactHorizontalScrollView.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactHorizontalScrollView.java index c5271853b4d09c..57a208c36ab96d 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactHorizontalScrollView.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactHorizontalScrollView.java @@ -825,7 +825,6 @@ private void handlePostTouchScrolling(int velocityX, int velocityY) { new Runnable() { private boolean mSnappingToPage = false; - private boolean mRunning = true; private int mStableFrames = 0; @Override @@ -834,7 +833,8 @@ public void run() { // We are still scrolling. mActivelyScrolling = false; mStableFrames = 0; - mRunning = true; + ViewCompat.postOnAnimationDelayed( + ReactHorizontalScrollView.this, this, ReactScrollViewHelper.MOMENTUM_DELAY); } else { // There has not been a scroll update since the last time this Runnable executed. ReactScrollViewHelper.updateFabricScrollState(ReactHorizontalScrollView.this); @@ -847,31 +847,25 @@ public void run() { // fire before the first scroll event of an animated scroll/fling, and stop // immediately. mStableFrames++; - mRunning = (mStableFrames < 3); - - if (mPagingEnabled && !mSnappingToPage) { - // Only if we have pagingEnabled and we have not snapped to the page do we - // need to continue checking for the scroll. And we cause that scroll by asking for - // it - mSnappingToPage = true; - flingAndSnap(0); - ViewCompat.postOnAnimationDelayed( - ReactHorizontalScrollView.this, this, ReactScrollViewHelper.MOMENTUM_DELAY); - } else { + + if (mStableFrames >= 3) { + mPostTouchRunnable = null; if (mSendMomentumEvents) { ReactScrollViewHelper.emitScrollMomentumEndEvent(ReactHorizontalScrollView.this); } disableFpsListener(); + } else { + if (mPagingEnabled && !mSnappingToPage) { + // If we have pagingEnabled and we have not snapped to the page + // we need to cause that scroll by asking for it + mSnappingToPage = true; + flingAndSnap(0); + } + // The scrollview has not "stabilized" so we just post to check again a frame later + ViewCompat.postOnAnimationDelayed( + ReactHorizontalScrollView.this, this, ReactScrollViewHelper.MOMENTUM_DELAY); } } - - // We are still scrolling so we just post to check again a frame later - if (mRunning) { - ViewCompat.postOnAnimationDelayed( - ReactHorizontalScrollView.this, this, ReactScrollViewHelper.MOMENTUM_DELAY); - } else { - mPostTouchRunnable = null; - } } }; ViewCompat.postOnAnimationDelayed( diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactScrollView.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactScrollView.java index 6bda8b8826b474..86d9dafffe2d67 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactScrollView.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactScrollView.java @@ -645,7 +645,6 @@ private void handlePostTouchScrolling(int velocityX, int velocityY) { new Runnable() { private boolean mSnappingToPage = false; - private boolean mRunning = true; private int mStableFrames = 0; @Override @@ -654,7 +653,8 @@ public void run() { // We are still scrolling. mActivelyScrolling = false; mStableFrames = 0; - mRunning = true; + ViewCompat.postOnAnimationDelayed( + ReactScrollView.this, this, ReactScrollViewHelper.MOMENTUM_DELAY); } else { // There has not been a scroll update since the last time this Runnable executed. ReactScrollViewHelper.updateFabricScrollState(ReactScrollView.this); @@ -667,31 +667,25 @@ public void run() { // fire before the first scroll event of an animated scroll/fling, and stop // immediately. mStableFrames++; - mRunning = (mStableFrames < 3); - - if (mPagingEnabled && !mSnappingToPage) { - // Only if we have pagingEnabled and we have not snapped to the page do we - // need to continue checking for the scroll. And we cause that scroll by asking for - // it - mSnappingToPage = true; - flingAndSnap(0); - ViewCompat.postOnAnimationDelayed( - ReactScrollView.this, this, ReactScrollViewHelper.MOMENTUM_DELAY); - } else { + + if (mStableFrames >= 3) { + mPostTouchRunnable = null; if (mSendMomentumEvents) { ReactScrollViewHelper.emitScrollMomentumEndEvent(ReactScrollView.this); } disableFpsListener(); + } else { + if (mPagingEnabled && !mSnappingToPage) { + // If we have pagingEnabled and we have not snapped to the page + // we need to cause that scroll by asking for it + mSnappingToPage = true; + flingAndSnap(0); + } + // The scrollview has not "stabilized" so we just post to check again a frame later + ViewCompat.postOnAnimationDelayed( + ReactScrollView.this, this, ReactScrollViewHelper.MOMENTUM_DELAY); } } - - // We are still scrolling so we just post to check again a frame later - if (mRunning) { - ViewCompat.postOnAnimationDelayed( - ReactScrollView.this, this, ReactScrollViewHelper.MOMENTUM_DELAY); - } else { - mPostTouchRunnable = null; - } } }; ViewCompat.postOnAnimationDelayed(