From b07a65b9d9fa4bc6d40283277df3dad5a25e3100 Mon Sep 17 00:00:00 2001 From: Samuel Susla Date: Thu, 13 Feb 2020 12:58:45 -0800 Subject: [PATCH] Fix ScrollView state not being set if scrollTo command was called Summary: # Problem `UIManager::getRelativeLayoutMetrics` returns incorrect `frame.origin.y` value. Just quick reiteration how calculation of `frame.origin` works. We take frame of the target shadow node, travers hierarchy to the root and keep adding `frame.origin` of each ancestor to target shadow node's origin. One more important piece of information, to calculate scroll view's `frame.origin`, we need to have its contentOffset which gets passed to Fabric core through state. # So where does it go wrong? Problem is that on Android, calling view command `scrollTo` doesn't set its internal state correctly. So when we calculate the layoutmetrics, scroll view's `frame.origin` is off by whatever value was used in `scrollTo`. # The fix In `ReactScrollView`, correctly set state after `scrollTo` is called on it. Changelog: [Internal] Reviewed By: JoshuaGross Differential Revision: D19835549 fbshipit-source-id: d56e7b0b05023c0497e52c8b46fdcf58ca78b4a5 --- .../react/views/scroll/ReactScrollView.java | 30 ++++++++++++++++--- .../views/scroll/ReactScrollViewManager.java | 8 ++--- 2 files changed, 30 insertions(+), 8 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactScrollView.java b/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactScrollView.java index 48664a960ca..8004cc2ac63 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactScrollView.java +++ b/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactScrollView.java @@ -200,7 +200,7 @@ public class ReactScrollView extends ScrollView @Override protected void onLayout(boolean changed, int l, int t, int r, int b) { // Call with the present values in order to re-layout if necessary - scrollTo(getScrollX(), getScrollY()); + reactScrollTo(getScrollX(), getScrollY()); } @Override @@ -597,7 +597,7 @@ public class ReactScrollView extends ScrollView targetOffset = currentPage * interval; if (targetOffset != currentOffset) { mActivelyScrolling = true; - smoothScrollTo(getScrollX(), (int) targetOffset); + reactSmoothScrollTo(getScrollX(), (int) targetOffset); } } @@ -715,7 +715,7 @@ public class ReactScrollView extends ScrollView postInvalidateOnAnimation(); } else { - smoothScrollTo(getScrollX(), targetOffset); + reactSmoothScrollTo(getScrollX(), targetOffset); } } @@ -768,6 +768,28 @@ public class ReactScrollView extends ScrollView mContentView = null; } + /** + * Calls `smoothScrollTo` and updates state. + * + *

`smoothScrollTo` changes `contentOffset` and we need to keep `contentOffset` in sync between + * scroll view and state. Calling raw `smoothScrollTo` doesn't update state. + */ + public void reactSmoothScrollTo(int x, int y) { + smoothScrollTo(x, y); + updateStateOnScroll(); + } + + /** + * Calls `reactScrollTo` and updates state. + * + *

`reactScrollTo` changes `contentOffset` and we need to keep `contentOffset` in sync between + * scroll view and state. Calling raw `reactScrollTo` doesn't update state. + */ + public void reactScrollTo(int x, int y) { + scrollTo(x, y); + updateStateOnScroll(); + } + /** * Called when a mContentView's layout has changed. Fixes the scroll position if it's too large * after the content resizes. Without this, the user would see a blank ScrollView when the scroll @@ -791,7 +813,7 @@ public class ReactScrollView extends ScrollView int currentScrollY = getScrollY(); int maxScrollY = getMaxScrollY(); if (currentScrollY > maxScrollY) { - scrollTo(getScrollX(), maxScrollY); + reactScrollTo(getScrollX(), maxScrollY); } } diff --git a/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactScrollViewManager.java b/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactScrollViewManager.java index d7fc393f4e4..d528caa2ea6 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactScrollViewManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactScrollViewManager.java @@ -201,9 +201,9 @@ public class ReactScrollViewManager extends ViewGroupManager public void scrollTo( ReactScrollView scrollView, ReactScrollViewCommandHelper.ScrollToCommandData data) { if (data.mAnimated) { - scrollView.smoothScrollTo(data.mDestX, data.mDestY); + scrollView.reactSmoothScrollTo(data.mDestX, data.mDestY); } else { - scrollView.scrollTo(data.mDestX, data.mDestY); + scrollView.reactScrollTo(data.mDestX, data.mDestY); } } @@ -275,9 +275,9 @@ public class ReactScrollViewManager extends ViewGroupManager // ScrollView always has one child - the scrollable area int bottom = scrollView.getChildAt(0).getHeight() + scrollView.getPaddingBottom(); if (data.mAnimated) { - scrollView.smoothScrollTo(scrollView.getScrollX(), bottom); + scrollView.reactSmoothScrollTo(scrollView.getScrollX(), bottom); } else { - scrollView.scrollTo(scrollView.getScrollX(), bottom); + scrollView.reactScrollTo(scrollView.getScrollX(), bottom); } }