From b9b23e1ab138189d2a4c22b13ba6ad8f8957579e Mon Sep 17 00:00:00 2001 From: Joshua Gross Date: Thu, 11 Mar 2021 10:55:03 -0800 Subject: [PATCH] Fix incorrect platform auto-scroll to right when height changes Summary: When the height of a HorizontalScrollView changes and there is a `layout` event, it can cause the underlying platform View code to scroll slightly to the right. I'm... not really sure why, even after looking at the View code for a while. But it is clearly detectable and mirrors issues with RTL that were fixed recently. This might warrant more investigation, but I believe the fix is relatively safe - we detect if there's an autoscroll only if the height changes and only if the scroll happens in "layout". That scopes the hack pretty well to just this bug. There aren't really times when we actually want layout to scroll to the right, so... I think this is reasonable. Changelog: [Changed][Android] Fixed issue that causes HorizontalScrollView to shift to the right when a TextInput is selected and keyboard pops up Reviewed By: mdvacca Differential Revision: D26972710 fbshipit-source-id: 441b1a3f07b9b68195a9e5e9a0c8d75c9d24a109 --- .../scroll/ReactHorizontalScrollView.java | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) 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 6cb0293d747..52e4b28de55 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 @@ -54,12 +54,16 @@ public class ReactHorizontalScrollView extends HorizontalScrollView private static boolean DEBUG_MODE = false && ReactBuildConfig.DEBUG; private static String TAG = ReactHorizontalScrollView.class.getSimpleName(); + private static int NO_SCROLL_POSITION = Integer.MIN_VALUE; + private static @Nullable Field sScrollerField; private static boolean sTriedToGetScrollerField = false; private static final String CONTENT_OFFSET_LEFT = "contentOffsetLeft"; private static final String CONTENT_OFFSET_TOP = "contentOffsetTop"; private int mLayoutDirection; + private int mScrollXAfterMeasure = NO_SCROLL_POSITION; + private static final int UNSET_CONTENT_OFFSET = -1; private final OnScrollDispatchHelper mOnScrollDispatchHelper = new OnScrollDispatchHelper(); @@ -275,7 +279,15 @@ public class ReactHorizontalScrollView extends HorizontalScrollView measuredHeight); } + boolean measuredHeightChanged = getMeasuredHeight() != measuredHeight; + setMeasuredDimension(measuredWidth, measuredHeight); + + // See how `mScrollXAfterMeasure` is used in `onLayout`, and why we only enable the + // hack if the height has changed. + if (measuredHeightChanged && mScroller != null) { + mScrollXAfterMeasure = mScroller.getCurrX(); + } } @Override @@ -284,6 +296,30 @@ public class ReactHorizontalScrollView extends HorizontalScrollView FLog.i(TAG, "onLayout[%d] l %d t %d r %d b %d", getId(), l, t, r, b); } + // Has the scrollX changed between the last onMeasure and this layout? + // If so, cancel the animation. + // Essentially, if the height changes (due to keyboard popping up, for instance) the + // underlying View.layout method will in some cases scroll to an incorrect X position - + // see also the hacks in `fling`. The order of layout is called in the order of: onMeasure, + // layout, onLayout. + // We cannot override `layout` but we can detect the sequence of events between onMeasure + // and onLayout. + if (mScrollXAfterMeasure != NO_SCROLL_POSITION + && mScroller != null + && mScrollXAfterMeasure != mScroller.getFinalX() + && !mScroller.isFinished()) { + if (DEBUG_MODE) { + FLog.i( + TAG, + "onLayout[%d] scroll hack enabled: reset to previous scrollX position of %d", + getId(), + mScrollXAfterMeasure); + } + mScroller.startScroll(mScrollXAfterMeasure, mScroller.getFinalY(), 0, 0); + mScroller.forceFinished(true); + mScrollXAfterMeasure = NO_SCROLL_POSITION; + } + // Call with the present values in order to re-layout if necessary // If a "pending" value has been set, we restore that value. // That value gets cleared by reactScrollTo.