From ae231c83220941e0e5aae74ff184467f75438452 Mon Sep 17 00:00:00 2001 From: Onti Vals Date: Mon, 10 Jun 2019 03:44:16 -0700 Subject: [PATCH] Scrolling fixes (#25105) Summary: Scrolling improvements in ReactAndroid: 1. Issue: With current ReactHorizontalScrollView behavior, it treats all views as focusable, regardless of if they are in view or not. This is fine for non-paged horizontal scroll view, but when paged this allows focus on elements that are not within the current page. Combined with logic to scroll to the focused view, this breaks the paging for ReactHorizontalScrollView. Fix: limit the focusable elements to only elements that are currently in view when ReactHorizontalScrollView has paging enabled 2. Issue: When keyboard is attached and user tries to navigate through Tab key, Scroll views do not scroll to the focused child. Since ReactScrollView handles layout changes on JS side, it does not call super.onlayout due to which mIsLayoutDirty flag in android ScrollView remains true and prevents scrolling to child when requestChildFocus is called. Fix: To fix the focus navigation, we are overriding requestChildFocus method in ReactScrollView. We are not checking any dirty layout flag and scrolling to child directly. This will fix focus navigation issue for KeyEvents which are not handled by android's ScrollView, for example: KEYCODE_TAB. Same applies to ReactHorizontalScrollView. 3. Set Android ScrollView to be non-focusable when scroll is disabled. Prior to this change, non-scrollable Scrollview would still be focusable, causing a poor keyboarding experience ## Changelog [Android] [Fixed] Scrolling improvements in ReactAndroid Pull Request resolved: https://github.com/facebook/react-native/pull/25105 Differential Revision: D15737563 Pulled By: mdvacca fbshipit-source-id: 0d57563415c68668dc1acb05fb3399e6645c9595 --- .../scroll/ReactHorizontalScrollView.java | 147 ++++++++++++++++++ .../react/views/scroll/ReactScrollView.java | 29 ++++ .../views/scroll/ReactScrollViewManager.java | 4 + 3 files changed, 180 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 18687228c13..4d97b76e90a 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 @@ -18,6 +18,7 @@ import android.hardware.SensorManager; import androidx.core.view.ViewCompat; import androidx.core.text.TextUtilsCompat; import android.util.Log; +import android.view.FocusFinder; import android.view.MotionEvent; import android.view.View; import android.view.ViewConfiguration; @@ -37,6 +38,8 @@ import java.lang.reflect.Field; import java.util.List; import java.util.Locale; import javax.annotation.Nullable; +import java.util.ArrayList; +import java.util.List; /** * Similar to {@link ReactScrollView} but only supports horizontal scrolling. @@ -72,6 +75,9 @@ public class ReactHorizontalScrollView extends HorizontalScrollView implements private boolean mSnapToStart = true; private boolean mSnapToEnd = true; private ReactViewBackgroundManager mReactBackgroundManager; + private boolean mPagedArrowScrolling = false; + + private final Rect mTempRect = new Rect(); public ReactHorizontalScrollView(Context context) { this(context, null); @@ -221,6 +227,82 @@ public class ReactHorizontalScrollView extends HorizontalScrollView implements scrollTo(getScrollX(), getScrollY()); } + /** + * Since ReactHorizontalScrollView handles layout changes on JS side, it does not call super.onlayout + * due to which mIsLayoutDirty flag in HorizontalScrollView remains true and prevents scrolling to child + * when requestChildFocus is called. + * Overriding this method and scrolling to child without checking any layout dirty flag. This will fix + * focus navigation issue for KeyEvents which are not handled in HorizontalScrollView, for example: KEYCODE_TAB. + */ + @Override + public void requestChildFocus(View child, View focused) { + if (focused != null && !mPagingEnabled) { + scrollToChild(focused); + } + super.requestChildFocus(child, focused); + } + + @Override + public void addFocusables(ArrayList views, int direction, int focusableMode) { + if (mPagingEnabled && !mPagedArrowScrolling) { + // Only add elements within the current page to list of focusables + ArrayList candidateViews = new ArrayList(); + super.addFocusables(candidateViews, direction, focusableMode); + for (View candidate : candidateViews) { + // We must also include the currently focused in the focusables list or focus search will always + // return the first element within the focusables list + if (isScrolledInView(candidate) || isPartiallyScrolledInView(candidate) || candidate.isFocused()) { + views.add(candidate); + } + } + } else { + super.addFocusables(views, direction, focusableMode); + } + } + + /** + * Calculates the x delta required to scroll the given descendent into view + */ + private int getScrollDelta(View descendent) { + descendent.getDrawingRect(mTempRect); + offsetDescendantRectToMyCoords(descendent, mTempRect); + return computeScrollDeltaToGetChildRectOnScreen(mTempRect); + } + + /** + * Returns whether the given descendent is scrolled fully in view + */ + private boolean isScrolledInView(View descendent) { + return getScrollDelta(descendent) == 0; + } + + + /** + * Returns whether the given descendent is partially scrolled in view + */ + private boolean isPartiallyScrolledInView(View descendent) { + int scrollDelta = getScrollDelta(descendent); + descendent.getDrawingRect(mTempRect); + return scrollDelta != 0 && Math.abs(scrollDelta) < mTempRect.width(); + } + + /** + * Returns whether the given descendent is "mostly" (>50%) scrolled in view + */ + private boolean isMostlyScrolledInView(View descendent) { + int scrollDelta = getScrollDelta(descendent); + descendent.getDrawingRect(mTempRect); + return scrollDelta != 0 && Math.abs(scrollDelta) < (mTempRect.width() / 2); + } + + private void scrollToChild(View child) { + int scrollDelta = getScrollDelta(child); + + if (scrollDelta != 0) { + scrollBy(scrollDelta, 0); + } + } + @Override protected void onScrollChanged(int x, int y, int oldX, int oldY) { super.onScrollChanged(x, y, oldX, oldY); @@ -263,6 +345,48 @@ public class ReactHorizontalScrollView extends HorizontalScrollView implements return false; } + @Override + public boolean pageScroll(int direction) { + boolean handled = super.pageScroll(direction); + + if (mPagingEnabled && handled) { + handlePostTouchScrolling(0, 0); + } + + return handled; + } + + @Override + public boolean arrowScroll(int direction) { + boolean handled = false; + + if (mPagingEnabled) { + mPagedArrowScrolling = true; + + if (getChildCount() > 0) { + View currentFocused = findFocus(); + View nextFocused = FocusFinder.getInstance().findNextFocus(this, currentFocused, direction); + View rootChild = getChildAt(0); + if (rootChild != null && nextFocused != null && nextFocused.getParent() == rootChild) { + if (!isScrolledInView(nextFocused) && !isMostlyScrolledInView(nextFocused)) { + smoothScrollToNextPage(direction); + } + nextFocused.requestFocus(); + handled = true; + } else { + smoothScrollToNextPage(direction); + handled = true; + } + } + + mPagedArrowScrolling = false; + } else { + handled = super.arrowScroll(direction); + } + + return handled; + } + @Override public boolean onTouchEvent(MotionEvent ev) { if (!mScrollEnabled) { @@ -706,6 +830,29 @@ public class ReactHorizontalScrollView extends HorizontalScrollView implements } } + private void smoothScrollToNextPage(int direction) { + int width = getWidth(); + int currentX = getScrollX(); + + int page = currentX / width; + if (currentX % width != 0) { + page++; + } + + if (direction == View.FOCUS_LEFT) { + page = page - 1; + } else { + page = page + 1; + } + + if (page < 0) { + page = 0; + } + + smoothScrollTo(page * width, getScrollY()); + handlePostTouchScrolling(0, 0); + } + @Override public void setBackgroundColor(int color) { mReactBackgroundManager.setBackgroundColor(color); 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 e98c1864c36..de0fc82ee66 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 @@ -205,6 +205,35 @@ public class ReactScrollView extends ScrollView implements ReactClippingViewGrou } } + /** + * Since ReactScrollView handles layout changes on JS side, it does not call super.onlayout + * due to which mIsLayoutDirty flag in ScrollView remains true and prevents scrolling to child + * when requestChildFocus is called. + * Overriding this method and scrolling to child without checking any layout dirty flag. This will fix + * focus navigation issue for KeyEvents which are not handled by ScrollView, for example: KEYCODE_TAB. + */ + @Override + public void requestChildFocus(View child, View focused) { + if (focused != null) { + scrollToChild(focused); + } + super.requestChildFocus(child, focused); + } + + private void scrollToChild(View child) { + Rect tempRect = new Rect(); + child.getDrawingRect(tempRect); + + /* Offset from child's local coordinates to ScrollView coordinates */ + offsetDescendantRectToMyCoords(child, tempRect); + + int scrollDelta = computeScrollDeltaToGetChildRectOnScreen(tempRect); + + if (scrollDelta != 0) { + scrollBy(0, scrollDelta); + } + } + @Override protected void onScrollChanged(int x, int y, int oldX, int oldY) { super.onScrollChanged(x, y, oldX, oldY); 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 afe0725e43c..fecbd151d01 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 @@ -70,6 +70,10 @@ public class ReactScrollViewManager @ReactProp(name = "scrollEnabled", defaultBoolean = true) public void setScrollEnabled(ReactScrollView view, boolean value) { view.setScrollEnabled(value); + + // Set focusable to match whether scroll is enabled. This improves keyboarding + // experience by not making scrollview a tab stop when you cannot interact with it. + view.setFocusable(value); } @ReactProp(name = "showsVerticalScrollIndicator")