Generalize RTL Scroll Correction Logic (#38526)

Summary:
Pull Request resolved: #38526

This changes the logic we use to correct scroll position on RTL ScrollView content change, to a new more generalized strategy of keeping a constant right-edge offset on layout change. This includes first layout, so that the initial scroll position is correct.

Changelog:
[Android][Fixed] - Generalize RTL Scroll Correction Logic

Reviewed By: yungsters

Differential Revision: D47627115

fbshipit-source-id: 33b2aae0cb603b7f7f2e2e6c127622fd531230e8

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactHorizontalScrollContainerView.java

@@ -16,15 +16,13 @@
 public class ReactHorizontalScrollContainerView extends ReactViewGroup {
 
   private int mLayoutDirection;
-  private int mCurrentWidth;
 
   public ReactHorizontalScrollContainerView(Context context) {
     super(context);
     mLayoutDirection =
         I18nUtil.getInstance().isRTL(context)
             ? ViewCompat.LAYOUT_DIRECTION_RTL
             : ViewCompat.LAYOUT_DIRECTION_LTR;
-    mCurrentWidth = 0;
   }
 
   @Override
@@ -50,24 +48,7 @@ protected void onLayout(boolean changed, int left, int top, int right, int botto
       int newLeft = 0;
       int width = right - left;
       int newRight = newLeft + width;
-      setLeft(newLeft);
-      setRight(newRight);
-
-      /**
-       * Note: in RTL mode, *when layout width changes*, we adjust the scroll position. Practically,
-       * this means that on the first (meaningful) layout we will go from position 0 to position
-       * (right - screenWidth). In theory this means if the width of the view ever changes during
-       * layout again, scrolling could jump. Which shouldn't happen in theory, but... if you find a
-       * weird product bug that looks related, keep this in mind.
-       */
-      if (mCurrentWidth != getWidth()) {
-        // Call with the present values in order to re-layout if necessary
-        ReactHorizontalScrollView parent = (ReactHorizontalScrollView) getParent();
-        // Fix the ScrollX position when using RTL language
-        int offsetX = parent.getScrollX() + getWidth() - mCurrentWidth - parent.getWidth();
-        parent.scrollTo(offsetX, parent.getScrollY());
-      }
+      setLeftTopRightBottom(newLeft, top, newRight, bottom);
     }
-    mCurrentWidth = getWidth();
   }
 }

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactHorizontalScrollView.java

@@ -20,6 +20,7 @@
 import android.graphics.Rect;
 import android.graphics.drawable.ColorDrawable;
 import android.graphics.drawable.Drawable;
+import android.os.Build;
 import android.view.FocusFinder;
 import android.view.KeyEvent;
 import android.view.MotionEvent;
@@ -130,6 +131,8 @@ public ReactHorizontalScrollView(Context context, @Nullable FpsListener fpsListe
     mScroller = getOverScrollerFromParent();
     mReactScrollViewScrollState =
         new ReactScrollViewScrollState(
+            // TODO: The ScrollView content may be laid out in a different direction than the
+            // instance if the `direction` style is set on the ScrollView or above it.
             I18nUtil.getInstance().isRTL(context)
                 ? ViewCompat.LAYOUT_DIRECTION_RTL
                 : ViewCompat.LAYOUT_DIRECTION_LTR);
@@ -606,7 +609,9 @@ public void fling(int velocityX) {
     // Hence, we can use the absolute value from whatever the OS gives
     // us and use the sign of what mOnScrollDispatchHelper has tracked.
     final int correctedVelocityX =
-        (int) (Math.abs(velocityX) * Math.signum(mOnScrollDispatchHelper.getXFlingVelocity()));
+        Build.VERSION.SDK_INT == Build.VERSION_CODES.P
+            ? (int) (Math.abs(velocityX) * Math.signum(mOnScrollDispatchHelper.getXFlingVelocity()))
+            : velocityX;
 
     if (mPagingEnabled) {
       flingAndSnap(correctedVelocityX);
@@ -1294,11 +1299,62 @@ public void onLayoutChange(
       return;
     }
 
-    if (mMaintainVisibleContentPositionHelper != null) {
+    // Adjust the scroll position to follow new content. In RTL, this means we keep a constant
+    // offset from the right edge instead of the left edge, so content added to the end of the flow
+    // does not shift layout. If `maintainVisibleContentPosition` is enabled, we try to adjust
+    // position so that the viewport keeps the same insets to previously visible views. TODO: MVCP
+    // does not work in RTL.
+    if (mReactScrollViewScrollState.getLayoutDirection() == LAYOUT_DIRECTION_RTL) {
+      adjustPositionForContentChangeRTL(left, right, oldLeft, oldRight);
+    } else if (mMaintainVisibleContentPositionHelper != null) {
       mMaintainVisibleContentPositionHelper.updateScrollPosition();
     }
   }
 
+  private void adjustPositionForContentChangeRTL(int left, int right, int oldLeft, int oldRight) {
+    // If we have any pending custon flings (e.g. from aninmated `scrollTo`, or flinging to a snap
+    // point), finish them, commiting the final `scrollX`.
+    // TODO: Can we be more graceful (like OverScroller flings)?
+    if (getFlingAnimator().isRunning()) {
+      getFlingAnimator().end();
+    }
+
+    int distanceToRightEdge = oldRight - getScrollX();
+    int newWidth = right - left;
+    int scrollX = newWidth - distanceToRightEdge;
+    scrollTo(scrollX, getScrollY());
+
+    // If we are in the middle of a fling animation from the user removing their finger
+    // (OverScroller is in `FLING_MODE`), we must cancel and recreate the existing fling animation
+    // since it was calculated against outdated scroll offsets.
+    if (mScroller != null && !mScroller.isFinished()) {
+      // Calculate the veliocity and position of the fling animation at the time of this layout
+      // event, which may be later than the last ScrollView tick. These values are not commited to
+      // the underlying ScrollView, which will recalculate positions on its next tick.
+      int scrollerXBeforeTick = mScroller.getCurrX();
+      boolean hasMoreTicks = mScroller.computeScrollOffset();
+
+      // Stop the existing animation at the current state of the scroller. We will then recreate
+      // it starting at the adjusted x offset.
+      mScroller.forceFinished(true);
+
+      if (hasMoreTicks) {
+        // OverScroller.getCurrVelocity() returns an absolute value of the velocity a current fling
+        // animation (only FLING_MODE animations). We derive direction along the X axis from the
+        // start and end of the, animation assuming HorizontalScrollView never fires vertical fling
+        // animations.
+        // TODO: This does not fully handle overscroll.
+        float direction = Math.signum(mScroller.getFinalX() - mScroller.getStartX());
+        float flingVelocityX = mScroller.getCurrVelocity() * direction;
+
+        mScroller.fling(
+            scrollX, getScrollY(), (int) flingVelocityX, 0, 0, newWidth - getWidth(), 0, 0);
+      } else {
+        scrollTo(scrollX + (mScroller.getCurrX() - scrollerXBeforeTick), getScrollY());
+      }
+    }
+  }
+
   @Override
   public FabricViewStateManager getFabricViewStateManager() {
     return mFabricViewStateManager;