Permalink
Browse files

mActivelyScrolling expected to be true so long as events are being fi…

…red.

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 #15146

Reviewed By: AaaChiuuu

Differential Revision: D5792026

Pulled By: tomasreimers

fbshipit-source-id: 9654fda038d4a687cc32f4c32dc312baa34627ed
  • Loading branch information...
tomasreimers authored and facebook-github-bot committed Sep 13, 2017
1 parent 1712664 commit cbb0ccb185fcd4f5ef8181a6deffaa359d028255
@@ -113,13 +113,13 @@ protected void onLayout(boolean changed, int l, int t, int r, int b) {
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(),

0 comments on commit cbb0ccb

Please sign in to comment.