Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Fix to make taps on views outside parent bounds work on Android (#29039)
Summary:
By default, Views in React Native have `overflow: visible`. When a child view is outside of the parent view's boundaries, it's visible on Android, but not tappable. This behaviour is incorrect, and doesn't match iOS behaviour.

- Taps on Views outside the bounds of a parent with `overflow: visible` (or unset) should register
- Taps on Views outside the bounds of a parent with `overflow: hidden` should continue to not register

Related issues:

- fixes #21455
- fixes #27061
- fixes #27232

### Fix

- Made `findTouchTargetView` not check that the touch was in the bounds of the immediate children, but instead
  - Check that the touch is in its own bounds when returning itself
  - Check that the touch for a child is in its own bounds only when `overflow: hidden` is set
- Modified related code to adjust to this change
- Added RNTesterApp test

## Changelog

<!-- Help reviewers and the release process by writing your own changelog entry. For an example, see:
https://github.com/facebook/react-native/wiki/Changelog
-->

[Android] [Fixed] - Allow taps on views outside the bounds of a parent with `overflow: hidden`

Pull Request resolved: #29039

Test Plan:
This can be tested with 2 examples added to the bottom of the PointerEvents page of the RNTesterApp:

| Before | After |
| --- | --- |
| ![Before](https://user-images.githubusercontent.com/2937410/83610933-19079b00-a535-11ea-8add-22daae0191e1.gif) | ![After](https://user-images.githubusercontent.com/2937410/83610583-8830bf80-a534-11ea-97e2-71e180a70343.gif) |

Reviewed By: ShikaSD

Differential Revision: D30104853

Pulled By: JoshuaGross

fbshipit-source-id: 644a109706258bfe829096354dfe477599e2db23
  • Loading branch information
hsource authored and facebook-github-bot committed Aug 6, 2021
1 parent c677e19 commit e35a963
Show file tree
Hide file tree
Showing 6 changed files with 248 additions and 74 deletions.
@@ -0,0 +1,25 @@
/*
* Copyright (c) Facebook, Inc. and its affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

package com.facebook.react.uimanager;

import android.view.View;
import androidx.annotation.Nullable;

/**
* Interface that should be implemented by {@link View} subclasses that support {@code overflow}
* style. This allows the overflow information to be used by {@link TouchTargetHelper} to determine
* if a View is touchable.
*/
public interface ReactOverflowView {
/**
* Gets the overflow state of a view. If set, this should be one of {@link ViewProps#HIDDEN},
* {@link ViewProps#VISIBLE} or {@link ViewProps#SCROLL}.
*/
@Nullable
String getOverflow();
}
Expand Up @@ -17,6 +17,7 @@
import com.facebook.react.bridge.JSApplicationIllegalArgumentException;
import com.facebook.react.bridge.UiThreadUtil;
import com.facebook.react.touch.ReactHitSlopView;
import java.util.EnumSet;

/**
* Class responsible for identifying which react view should handle a given {@link MotionEvent}. It
Expand Down Expand Up @@ -80,7 +81,7 @@ public static int findTargetTagAndCoordinatesForTouch(
// Store eventCoords in array so that they are modified to be relative to the targetView found.
viewCoords[0] = eventX;
viewCoords[1] = eventY;
View nativeTargetView = findTouchTargetView(viewCoords, viewGroup);
View nativeTargetView = findTouchTargetViewWithPointerEvents(viewCoords, viewGroup);
if (nativeTargetView != null) {
View reactTargetView = findClosestReactAncestor(nativeTargetView);
if (reactTargetView != null) {
Expand All @@ -100,6 +101,14 @@ private static View findClosestReactAncestor(View view) {
return view;
}

/** Types of allowed return values from {@link #findTouchTargetView}. */
private enum TouchTargetReturnType {
/** Allow returning the view passed in through the parameters. */
SELF,
/** Allow returning children of the view passed in through parameters. */
CHILD,
}

/**
* Returns the touch target View that is either viewGroup or one if its descendants. This is a
* recursive DFS since view the entire tree must be parsed until the target is found. If the
Expand All @@ -111,43 +120,88 @@ private static View findClosestReactAncestor(View view) {
* be relative to the current viewGroup. When the method returns, it will contain the eventCoords
* relative to the targetView found.
*/
private static View findTouchTargetView(float[] eventCoords, ViewGroup viewGroup) {
int childrenCount = viewGroup.getChildCount();
// Consider z-index when determining the touch target.
ReactZIndexedViewGroup zIndexedViewGroup =
viewGroup instanceof ReactZIndexedViewGroup ? (ReactZIndexedViewGroup) viewGroup : null;
for (int i = childrenCount - 1; i >= 0; i--) {
int childIndex =
zIndexedViewGroup != null ? zIndexedViewGroup.getZIndexMappedChildIndex(i) : i;
View child = viewGroup.getChildAt(childIndex);
PointF childPoint = mTempPoint;
if (isTransformedTouchPointInView(
eventCoords[0], eventCoords[1], viewGroup, child, childPoint)) {
// If it is contained within the child View, the childPoint value will contain the view
// coordinates relative to the child
private static View findTouchTargetView(
float[] eventCoords, View view, EnumSet<TouchTargetReturnType> allowReturnTouchTargetTypes) {
// We prefer returning a child, so we check for a child that can handle the touch first
if (allowReturnTouchTargetTypes.contains(TouchTargetReturnType.CHILD)
&& view instanceof ViewGroup) {
ViewGroup viewGroup = (ViewGroup) view;
int childrenCount = viewGroup.getChildCount();
// Consider z-index when determining the touch target.
ReactZIndexedViewGroup zIndexedViewGroup =
viewGroup instanceof ReactZIndexedViewGroup ? (ReactZIndexedViewGroup) viewGroup : null;
for (int i = childrenCount - 1; i >= 0; i--) {
int childIndex =
zIndexedViewGroup != null ? zIndexedViewGroup.getZIndexMappedChildIndex(i) : i;
View child = viewGroup.getChildAt(childIndex);
PointF childPoint = mTempPoint;
getChildPoint(eventCoords[0], eventCoords[1], viewGroup, child, childPoint);
// The childPoint value will contain the view coordinates relative to the child.
// We need to store the existing X,Y for the viewGroup away as it is possible this child
// will not actually be the target and so we restore them if not
float restoreX = eventCoords[0];
float restoreY = eventCoords[1];
eventCoords[0] = childPoint.x;
eventCoords[1] = childPoint.y;
View targetView = findTouchTargetViewWithPointerEvents(eventCoords, child);

if (targetView != null) {
return targetView;
// We don't allow touches on views that are outside the bounds of an `overflow: hidden`
// View
boolean inOverflowBounds = true;
if (viewGroup instanceof ReactOverflowView) {
@Nullable String overflow = ((ReactOverflowView) viewGroup).getOverflow();
if ((ViewProps.HIDDEN.equals(overflow) || ViewProps.SCROLL.equals(overflow))
&& !isTouchPointInView(restoreX, restoreY, view)) {
inOverflowBounds = false;
}
}
if (inOverflowBounds) {
return targetView;
}
}
eventCoords[0] = restoreX;
eventCoords[1] = restoreY;
}
}
return viewGroup;

// Check if parent can handle the touch after the children
if (allowReturnTouchTargetTypes.contains(TouchTargetReturnType.SELF)
&& isTouchPointInView(eventCoords[0], eventCoords[1], view)) {
return view;
}

return null;
}

/**
* Returns whether the touch point is within the child View It is transform aware and will invert
* the transform Matrix to find the true local points This code is taken from {@link
* Checks whether a touch at {@code x} and {@code y} are within the bounds of the View. Both
* {@code x} and {@code y} must be relative to the top-left corner of the view.
*/
private static boolean isTouchPointInView(float x, float y, View view) {
if (view instanceof ReactHitSlopView && ((ReactHitSlopView) view).getHitSlopRect() != null) {
Rect hitSlopRect = ((ReactHitSlopView) view).getHitSlopRect();
if ((x >= -hitSlopRect.left && x < (view.getWidth()) + hitSlopRect.right)
&& (y >= -hitSlopRect.top && y < (view.getHeight()) + hitSlopRect.bottom)) {
return true;
}

return false;
} else {
if ((x >= 0 && x < (view.getWidth())) && (y >= 0 && y < (view.getHeight()))) {
return true;
}

return false;
}
}

/**
* Returns the coordinates of a touch in the child View. It is transform aware and will invert the
* transform Matrix to find the true local points This code is taken from {@link
* ViewGroup#isTransformedTouchPointInView()}
*/
private static boolean isTransformedTouchPointInView(
private static void getChildPoint(
float x, float y, ViewGroup parent, View child, PointF outLocalPoint) {
float localX = x + parent.getScrollX() - child.getLeft();
float localY = y + parent.getScrollY() - child.getTop();
Expand All @@ -162,26 +216,7 @@ private static boolean isTransformedTouchPointInView(
localX = localXY[0];
localY = localXY[1];
}
if (child instanceof ReactHitSlopView && ((ReactHitSlopView) child).getHitSlopRect() != null) {
Rect hitSlopRect = ((ReactHitSlopView) child).getHitSlopRect();
if ((localX >= -hitSlopRect.left
&& localX < (child.getRight() - child.getLeft()) + hitSlopRect.right)
&& (localY >= -hitSlopRect.top
&& localY < (child.getBottom() - child.getTop()) + hitSlopRect.bottom)) {
outLocalPoint.set(localX, localY);
return true;
}

return false;
} else {
if ((localX >= 0 && localX < (child.getRight() - child.getLeft()))
&& (localY >= 0 && localY < (child.getBottom() - child.getTop()))) {
outLocalPoint.set(localX, localY);
return true;
}

return false;
}
outLocalPoint.set(localX, localY);
}

/**
Expand Down Expand Up @@ -211,45 +246,43 @@ private static boolean isTransformedTouchPointInView(
return null;

} else if (pointerEvents == PointerEvents.BOX_ONLY) {
// This view is the target, its children don't matter
return view;
// This view may be the target, its children don't matter
return findTouchTargetView(eventCoords, view, EnumSet.of(TouchTargetReturnType.SELF));

} else if (pointerEvents == PointerEvents.BOX_NONE) {
// This view can't be the target, but its children might.
if (view instanceof ViewGroup) {
View targetView = findTouchTargetView(eventCoords, (ViewGroup) view);
if (targetView != view) {
return targetView;
}
View targetView =
findTouchTargetView(eventCoords, view, EnumSet.of(TouchTargetReturnType.CHILD));
if (targetView != null) {
return targetView;
}

// PointerEvents.BOX_NONE means that this react element cannot receive pointer events.
// However, there might be virtual children that can receive pointer events, in which case
// we still want to return this View and dispatch a pointer event to the virtual element.
// Note that this currently only applies to Nodes/FlatViewGroup as it's the only class that
// is both a ViewGroup and ReactCompoundView (ReactTextView is a ReactCompoundView but not a
// ViewGroup).
if (view instanceof ReactCompoundView) {
int reactTag =
((ReactCompoundView) view).reactTagForTouch(eventCoords[0], eventCoords[1]);
if (reactTag != view.getId()) {
// make sure we exclude the View itself because of the PointerEvents.BOX_NONE
return view;
}
// PointerEvents.BOX_NONE means that this react element cannot receive pointer events.
// However, there might be virtual children that can receive pointer events, in which case
// we still want to return this View and dispatch a pointer event to the virtual element.
// Note that this currently only applies to Nodes/FlatViewGroup as it's the only class that
// is both a ViewGroup and ReactCompoundView (ReactTextView is a ReactCompoundView but not a
// ViewGroup).
if (view instanceof ReactCompoundView
&& isTouchPointInView(eventCoords[0], eventCoords[1], view)) {
int reactTag = ((ReactCompoundView) view).reactTagForTouch(eventCoords[0], eventCoords[1]);
if (reactTag != view.getId()) {
// make sure we exclude the View itself because of the PointerEvents.BOX_NONE
return view;
}
}

return null;

} else if (pointerEvents == PointerEvents.AUTO) {
// Either this view or one of its children is the target
if (view instanceof ReactCompoundViewGroup) {
if (((ReactCompoundViewGroup) view).interceptsTouchEvent(eventCoords[0], eventCoords[1])) {
return view;
}
if (view instanceof ReactCompoundViewGroup
&& isTouchPointInView(eventCoords[0], eventCoords[1], view)
&& ((ReactCompoundViewGroup) view).interceptsTouchEvent(eventCoords[0], eventCoords[1])) {
return view;
}
if (view instanceof ViewGroup) {
return findTouchTargetView(eventCoords, (ViewGroup) view);
}
return view;
return findTouchTargetView(
eventCoords, view, EnumSet.of(TouchTargetReturnType.SELF, TouchTargetReturnType.CHILD));

} else {
throw new JSApplicationIllegalArgumentException(
Expand Down
Expand Up @@ -40,6 +40,7 @@
import com.facebook.react.uimanager.PixelUtil;
import com.facebook.react.uimanager.ReactClippingViewGroup;
import com.facebook.react.uimanager.ReactClippingViewGroupHelper;
import com.facebook.react.uimanager.ReactOverflowView;
import com.facebook.react.uimanager.ViewProps;
import com.facebook.react.uimanager.events.NativeGestureUtil;
import com.facebook.react.views.view.ReactViewBackgroundManager;
Expand All @@ -49,7 +50,9 @@

/** Similar to {@link ReactScrollView} but only supports horizontal scrolling. */
public class ReactHorizontalScrollView extends HorizontalScrollView
implements ReactClippingViewGroup, FabricViewStateManager.HasFabricViewStateManager {
implements ReactClippingViewGroup,
FabricViewStateManager.HasFabricViewStateManager,
ReactOverflowView {

private static boolean DEBUG_MODE = false && ReactBuildConfig.DEBUG;
private static String TAG = ReactHorizontalScrollView.class.getSimpleName();
Expand Down Expand Up @@ -245,6 +248,11 @@ public void setOverflow(String overflow) {
invalidate();
}

@Override
public @Nullable String getOverflow() {
return mOverflow;
}

@Override
protected void onDraw(Canvas canvas) {
if (DEBUG_MODE) {
Expand Down
Expand Up @@ -37,6 +37,7 @@
import com.facebook.react.uimanager.PixelUtil;
import com.facebook.react.uimanager.ReactClippingViewGroup;
import com.facebook.react.uimanager.ReactClippingViewGroupHelper;
import com.facebook.react.uimanager.ReactOverflowView;
import com.facebook.react.uimanager.ViewProps;
import com.facebook.react.uimanager.common.UIManagerType;
import com.facebook.react.uimanager.common.ViewUtil;
Expand All @@ -56,7 +57,8 @@ public class ReactScrollView extends ScrollView
implements ReactClippingViewGroup,
ViewGroup.OnHierarchyChangeListener,
View.OnLayoutChangeListener,
FabricViewStateManager.HasFabricViewStateManager {
FabricViewStateManager.HasFabricViewStateManager,
ReactOverflowView {

private static @Nullable Field sScrollerField;
private static boolean sTriedToGetScrollerField = false;
Expand Down Expand Up @@ -225,6 +227,11 @@ public void setOverflow(String overflow) {
invalidate();
}

@Override
public @Nullable String getOverflow() {
return mOverflow;
}

@Override
protected void onMeasure(int widthMeasureSpec, int heightMeasureSpec) {
MeasureSpecAssertions.assertExplicitMeasureSpec(widthMeasureSpec, heightMeasureSpec);
Expand Down
Expand Up @@ -43,6 +43,7 @@
import com.facebook.react.uimanager.ReactClippingProhibitedView;
import com.facebook.react.uimanager.ReactClippingViewGroup;
import com.facebook.react.uimanager.ReactClippingViewGroupHelper;
import com.facebook.react.uimanager.ReactOverflowView;
import com.facebook.react.uimanager.ReactPointerEventsView;
import com.facebook.react.uimanager.ReactZIndexedViewGroup;
import com.facebook.react.uimanager.RootView;
Expand All @@ -63,7 +64,8 @@ public class ReactViewGroup extends ViewGroup
ReactClippingViewGroup,
ReactPointerEventsView,
ReactHitSlopView,
ReactZIndexedViewGroup {
ReactZIndexedViewGroup,
ReactOverflowView {

private static final int ARRAY_CAPACITY_INCREMENT = 12;
private static final int DEFAULT_BACKGROUND_COLOR = Color.TRANSPARENT;
Expand Down Expand Up @@ -718,6 +720,7 @@ public void setOverflow(String overflow) {
invalidate();
}

@Override
public @Nullable String getOverflow() {
return mOverflow;
}
Expand Down

1 comment on commit e35a963

@Hritik-Arora
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With this change, the touch area due to hitslop can extend beyond the parent boundaries, even though the child view could be well inside the parent view.

Please sign in to comment.