From cbb0ccb185fcd4f5ef8181a6deffaa359d028255 Mon Sep 17 00:00:00 2001 From: Tomas Reimers Date: Tue, 12 Sep 2017 18:24:44 -0700 Subject: [PATCH] mActivelyScrolling expected to be true so long as events are being fired. Summary: **Problem:** It was observed that in [this code path](https://github.com/facebook/react-native/blob/master/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactHorizontalScrollView.java#L292) (i.e. horizontal, paging-enabled scroll view) if you tried to programmatically call the scrollTo method within ~1s of the onMomentumScrollEnd event (which should only be called after all scrolling has ended), the scrollView would scroll to the new location, and then scroll BACK to the original location. For example, assume you had released the scrollView at location B, and the nearest page boundary is A. Then, 1000ms later, you call scrollTo position C. The order of operations would be: 1. Begin scrolling to A from position B (as it is the nearest page boundary) 2. Reach position A 3. Fire onMomentumScrollEnd 4. 1000ms later call scrollTo C 5. scrollView scrolls to C 6. scrollView scrolls BACK to position A (for no apparent reason). **Reason:** I suspect this is because the [smoothlyScrollTo](https://github.com/facebook/react-native/blob/master/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactHorizontalScrollView.java#L292) will continue to animate towards A, but the [scrollEvents will not fire](https://github.com/facebook/react-native/blob/f954f3d9b674b13977f722bc3b8dc6c1b99fe6c7/ReactAndroid/src/main/java/com/facebook/react/views/scroll/OnScrollDispatchHelper.java#L45) as they are too close to each other. So the true order of events is: 1. Begin scrolling to A from position B (as it is the nearest page boundary) [begin smoothlyScrollTo] [scroll towards position A] [mActivelyScrolling is true] 2. Reach position A [mActivelyScrolling is true] [scroll towards position A] [mActivelyScrolling is false, as there is another scrollEvent, but because it is close enough to the same location it is ignored] 3. Fire onMomentumScrollEnd 4. 1000ms later call scrollTo C [scroll towards position C] 5. scrollView scrolls to C [scroll towards position A as the original smoothlyScrollTo animation was never completed] 6. scrollView scrolls BACK to position A. This is an untested hypothesis, but seems to explain the behavior, and the solution is more semantically correct anyway. If there is an easy way to rebuild the android binaries happy to test it myself! Just let me know! **Solution:** Move the mActivelyAnimating outside the mOnScrollDispatchHelper.onScrollChanged helper, because the HorizontalScrollView event should be considered to be animating so long as onScrollChanged events are being fired. Closes https://github.com/facebook/react-native/pull/15146 Reviewed By: AaaChiuuu Differential Revision: D5792026 Pulled By: tomasreimers fbshipit-source-id: 9654fda038d4a687cc32f4c32dc312baa34627ed --- .../react/views/scroll/ReactHorizontalScrollView.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactHorizontalScrollView.java b/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactHorizontalScrollView.java index 3acb61e9c..25e3d04e7 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactHorizontalScrollView.java +++ b/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactHorizontalScrollView.java @@ -113,13 +113,13 @@ public class ReactHorizontalScrollView extends HorizontalScrollView implements protected void onScrollChanged(int x, int y, int oldX, int oldY) { super.onScrollChanged(x, y, oldX, oldY); + mActivelyScrolling = true; + if (mOnScrollDispatchHelper.onScrollChanged(x, y)) { if (mRemoveClippedSubviews) { updateClippingRect(); } - mActivelyScrolling = true; - ReactScrollViewHelper.emitScrollEvent( this, mOnScrollDispatchHelper.getXFlingVelocity(),