Skip to content

Commit

Permalink
Fix incorrect platform auto-scroll to right when height changes
Browse files Browse the repository at this point in the history
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
  • Loading branch information
JoshuaGross authored and facebook-github-bot committed Mar 11, 2021
1 parent 310a6bc commit b9b23e1
Showing 1 changed file with 36 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -275,7 +279,15 @@ protected void onMeasure(int widthMeasureSpec, int heightMeasureSpec) {
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
Expand All @@ -284,6 +296,30 @@ protected void onLayout(boolean changed, int l, int t, int r, int b) {
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.
Expand Down

0 comments on commit b9b23e1

Please sign in to comment.