From 0d6fd36b1c65fc8a0f913d690eec459abc49d250 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 18687228c13142..4d97b76e90a475 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 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.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 @@ protected void onLayout(boolean changed, int l, int t, int r, int b) { 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 boolean onInterceptTouchEvent(MotionEvent ev) { 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 @@ private void flingAndSnap(int velocityX) { } } + 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 e98c1864c36f55..de0fc82ee66b27 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 @@ protected void onAttachedToWindow() { } } + /** + * 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 afe0725e43cd05..fecbd151d0121a 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 ReactScrollView createViewInstance(ThemedReactContext context) { @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")