From fc032cd8d889d828edad3ea4b735205092cf0d40 Mon Sep 17 00:00:00 2001 From: Joshua Gross Date: Fri, 5 Mar 2021 22:46:44 -0800 Subject: [PATCH] Fix RTL content jumping Summary: Whenever layout updates in a horizontal scrollview, in RTL mode we adjust the position - the impact *should* be that initially, we jump from position 0 to the right side of the scroll view, such that scrolling starts from the right. However, we were doing this entirely too aggressively before. We should only make this adjustment *if the layout changes the width*. Changelog: [Android][Changed] Fixed jumpy RTL horizontal ScrollViews. If you have Android-specific JS hacks for handling RTL in ScrollViews, you probably can/probably want to remove them, because they should be reliable now and require fewer hacks. Reviewed By: mdvacca Differential Revision: D26771366 fbshipit-source-id: de11bd1cae1414018d88ce44b3583a8b15f3b330 --- .../ReactHorizontalScrollContainerView.java | 26 ++++++++++--------- 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactHorizontalScrollContainerView.java b/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactHorizontalScrollContainerView.java index 0445fbdf36cf4a..fcd473f4edd62c 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactHorizontalScrollContainerView.java +++ b/ReactAndroid/src/main/java/com/facebook/react/views/scroll/ReactHorizontalScrollContainerView.java @@ -44,13 +44,6 @@ public void setRemoveClippedSubviews(boolean removeClippedSubviews) { @Override protected void onLayout(boolean changed, int left, int top, int right, int bottom) { - /** - * 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 (mLayoutDirection == LAYOUT_DIRECTION_RTL) { // When the layout direction is RTL, we expect Yoga to give us a layout // that extends off the screen to the left so we re-center it with left=0 @@ -60,11 +53,20 @@ protected void onLayout(boolean changed, int left, int top, int right, int botto setLeft(newLeft); setRight(newRight); - // 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.reactScrollTo(offsetX, parent.getScrollY()); + /** + * 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.reactScrollTo(offsetX, parent.getScrollY()); + } } mCurrentWidth = getWidth(); }