Skip to content

Commit

Permalink
Scrolling fixes (#25105)
Browse files Browse the repository at this point in the history
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: #25105

Differential Revision: D15737563

Pulled By: mdvacca

fbshipit-source-id: 0d57563415c68668dc1acb05fb3399e6645c9595
  • Loading branch information
ontzic authored and facebook-github-bot committed Jun 10, 2019
1 parent d0792d4 commit ae231c8
Show file tree
Hide file tree
Showing 3 changed files with 180 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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.
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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<View> views, int direction, int focusableMode) {
if (mPagingEnabled && !mPagedArrowScrolling) {
// Only add elements within the current page to list of focusables
ArrayList<View> candidateViews = new ArrayList<View>();
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);
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down

0 comments on commit ae231c8

Please sign in to comment.