From e35a963bfb93bbbdd92f4dd74d14e2ad6df5e14a Mon Sep 17 00:00:00 2001 From: Harry Yu Date: Fri, 6 Aug 2021 12:31:45 -0700 Subject: [PATCH] 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 https://github.com/facebook/react-native/issues/21455 - fixes https://github.com/facebook/react-native/issues/27061 - fixes https://github.com/facebook/react-native/issues/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 [Android] [Fixed] - Allow taps on views outside the bounds of a parent with `overflow: hidden` Pull Request resolved: https://github.com/facebook/react-native/pull/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 --- .../react/uimanager/ReactOverflowView.java | 25 +++ .../react/uimanager/TouchTargetHelper.java | 169 +++++++++++------- .../scroll/ReactHorizontalScrollView.java | 10 +- .../react/views/scroll/ReactScrollView.java | 9 +- .../react/views/view/ReactViewGroup.java | 5 +- .../PointerEvents/PointerEventsExample.js | 104 ++++++++++- 6 files changed, 248 insertions(+), 74 deletions(-) create mode 100644 ReactAndroid/src/main/java/com/facebook/react/uimanager/ReactOverflowView.java diff --git a/ReactAndroid/src/main/java/com/facebook/react/uimanager/ReactOverflowView.java b/ReactAndroid/src/main/java/com/facebook/react/uimanager/ReactOverflowView.java new file mode 100644 index 00000000000000..89c437e3869660 --- /dev/null +++ b/ReactAndroid/src/main/java/com/facebook/react/uimanager/ReactOverflowView.java @@ -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(); +} diff --git a/ReactAndroid/src/main/java/com/facebook/react/uimanager/TouchTargetHelper.java b/ReactAndroid/src/main/java/com/facebook/react/uimanager/TouchTargetHelper.java index fbcda93d07cae2..c4e41b2ceeb35a 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/uimanager/TouchTargetHelper.java +++ b/ReactAndroid/src/main/java/com/facebook/react/uimanager/TouchTargetHelper.java @@ -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 @@ -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) { @@ -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 @@ -111,20 +120,23 @@ 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 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]; @@ -132,22 +144,64 @@ private static View findTouchTargetView(float[] eventCoords, ViewGroup viewGroup 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(); @@ -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); } /** @@ -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( 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 0784d5ab22966d..ab8ab73ffc8321 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 @@ -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; @@ -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(); @@ -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) { 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 5fcd6ef8bc651c..9fc5b76829c841 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 @@ -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; @@ -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; @@ -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); diff --git a/ReactAndroid/src/main/java/com/facebook/react/views/view/ReactViewGroup.java b/ReactAndroid/src/main/java/com/facebook/react/views/view/ReactViewGroup.java index 091c9a247b45e1..52104c96d3cd97 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/views/view/ReactViewGroup.java +++ b/ReactAndroid/src/main/java/com/facebook/react/views/view/ReactViewGroup.java @@ -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; @@ -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; @@ -718,6 +720,7 @@ public void setOverflow(String overflow) { invalidate(); } + @Override public @Nullable String getOverflow() { return mOverflow; } diff --git a/packages/rn-tester/js/examples/PointerEvents/PointerEventsExample.js b/packages/rn-tester/js/examples/PointerEvents/PointerEventsExample.js index 800e7395f6b0d3..ff5aaab93117ae 100644 --- a/packages/rn-tester/js/examples/PointerEvents/PointerEventsExample.js +++ b/packages/rn-tester/js/examples/PointerEvents/PointerEventsExample.js @@ -14,12 +14,25 @@ const React = require('react'); const {StyleSheet, Text, View} = require('react-native'); -class ExampleBox extends React.Component<$FlowFixMeProps, $FlowFixMeState> { +type ExampleBoxComponentProps = $ReadOnly<{| + onLog: (msg: string) => void, +|}>; + +type ExampleBoxProps = $ReadOnly<{| + Component: React.ComponentType, +|}>; + +type ExampleBoxState = $ReadOnly<{| + log: string[], +|}>; + +class ExampleBox extends React.Component { state = { log: [], }; - handleLog = msg => { + handleLog = (msg: string) => { + // $FlowFixMe this.state.log = this.state.log.concat([msg]); }; @@ -32,10 +45,12 @@ class ExampleBox extends React.Component<$FlowFixMeProps, $FlowFixMeState> { * happens. */ handleTouchCapture = () => { + // $FlowFixMe this.state.log = this.state.log.concat(['---']); }; render() { + const {Component} = this.props; return ( { * native_oss) This comment suppresses an error when upgrading * Flow's support for React. To see the error delete this comment * and run Flow. */} - + @@ -166,6 +181,57 @@ class BoxOnlyExample extends React.Component<$FlowFixMeProps> { } } +type OverflowExampleProps = $ReadOnly<{| + overflow: 'hidden' | 'visible', + onLog: (msg: string) => void, +|}>; + +class OverflowExample extends React.Component { + render() { + const {overflow} = this.props; + return ( + this.props.onLog(`A overflow ${overflow} touched`)} + style={[ + styles.box, + styles.boxWithOverflowSet, + {overflow: this.props.overflow}, + ]}> + A: overflow: {overflow} + this.props.onLog('B overflowing touched')} + style={[styles.box, styles.boxOverflowing]}> + B: overflowing + + this.props.onLog('C fully outside touched')} + style={[styles.box, styles.boxFullyOutside]}> + C: fully outside + + this.props.onLog('D fully outside child touched') + } + style={[styles.box, styles.boxFullyOutsideChild]}> + D: child of fully outside + + + + ); + } +} + +class OverflowVisibleExample extends React.Component { + render() { + return ; + } +} + +class OverflowHiddenExample extends React.Component { + render() { + return ; + } +} + type ExampleClass = { Component: React.ComponentType, title: string, @@ -192,6 +258,18 @@ const exampleClasses: Array = [ description: "`box-only` causes touch events on the container's child components to pass through and will only detect touch events on the container itself.", }, + { + Component: OverflowVisibleExample, + title: '`overflow: visible`', + description: + '`overflow: visible` style should allow subelements that are outside of the parent box to be touchable. Tapping the parts of Box B outside Box A should print "B touched" and "A touched", and tapping Box C should also print "C touched" and "A touched".', + }, + { + Component: OverflowHiddenExample, + title: '`overflow: hidden`', + description: + '`overflow: hidden` style should only allow subelements within the parent box to be touchable. Tapping just below Box A (where Box B would otherwise extend if it weren\'t cut off) should not trigger any touches or messages. Touching Box D (inside the bounds) should print "D touched" and "A touched".', + }, ]; const infoToExample = info => { @@ -222,6 +300,26 @@ const styles = StyleSheet.create({ boxPassedThrough: { borderColor: '#99bbee', }, + boxWithOverflowSet: { + paddingBottom: 40, + marginBottom: 50, + }, + boxOverflowing: { + position: 'absolute', + top: 30, + paddingBottom: 40, + }, + boxFullyOutside: { + position: 'absolute', + left: 200, + top: 65, + }, + boxFullyOutsideChild: { + position: 'absolute', + left: 0, + top: -65, + width: 100, + }, logText: { fontSize: 9, },