From 0b754cfa8b08a0e8556819cfe1587990e1f7a0d3 Mon Sep 17 00:00:00 2001 From: Harry Yu Date: Wed, 3 Jun 2020 01:22:19 -0700 Subject: [PATCH] Fix to make taps on views outside parent bounds work on Android See https://github.com/facebook/react-native/pull/29039 --- .../PointerEvents/PointerEventsExample.js | 80 ++++++++- .../react/uimanager/ReactOverflowView.java | 26 +++ .../react/uimanager/TouchTargetHelper.java | 160 +++++++++++------- .../scroll/ReactHorizontalScrollView.java | 8 +- .../react/views/scroll/ReactScrollView.java | 9 +- .../react/views/view/ReactViewGroup.java | 5 +- 6 files changed, 225 insertions(+), 63 deletions(-) create mode 100644 ReactAndroid/src/main/java/com/facebook/react/uimanager/ReactOverflowView.java diff --git a/RNTester/js/examples/PointerEvents/PointerEventsExample.js b/RNTester/js/examples/PointerEvents/PointerEventsExample.js index fcca307e9eac05..570b69f2bdd653 100644 --- a/RNTester/js/examples/PointerEvents/PointerEventsExample.js +++ b/RNTester/js/examples/PointerEvents/PointerEventsExample.js @@ -14,7 +14,15 @@ 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, +|}>; + +class ExampleBox extends React.Component { state = { log: [], }; @@ -165,6 +173,50 @@ 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 + + + ); + } +} + +class OverflowVisibleExample extends React.Component { + render() { + return ; + } +} + +class OverflowHiddenExample extends React.Component { + render() { + return ; + } +} + type ExampleClass = { Component: React.ComponentType, title: string, @@ -191,6 +243,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.', + }, + { + Component: OverflowHiddenExample, + title: '`overflow: hidden`', + description: + '`overflow: hidden` style should only allow subelements within the parent box to be touchable. The part of the `position: absolute` extending outside its parent should not trigger touch events.', + }, ]; const infoToExample = info => { @@ -221,6 +285,20 @@ 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, + }, logText: { fontSize: 9, }, 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..c7f9dad95fb4f5 --- /dev/null +++ b/ReactAndroid/src/main/java/com/facebook/react/uimanager/ReactOverflowView.java @@ -0,0 +1,26 @@ +/* + * 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.graphics.Rect; +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..cf61539e607365 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/uimanager/TouchTargetHelper.java +++ b/ReactAndroid/src/main/java/com/facebook/react/uimanager/TouchTargetHelper.java @@ -18,6 +18,8 @@ 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 * uses the event coordinates to traverse the view hierarchy and return a suitable view. @@ -80,7 +82,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 +102,20 @@ 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,18 +127,21 @@ 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 = + private static View findTouchTargetView( + float[] eventCoords, View view, EnumSet allowReturnTouchTargetTypes) { + 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 = + 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)) { + View child = viewGroup.getChildAt(childIndex); + PointF childPoint = mTempPoint; + getChildPoint(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 // We need to store the existing X,Y for the viewGroup away as it is possible this child @@ -132,22 +151,66 @@ 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) + && !isTouchPointInView(restoreX, restoreY, view)) { + inOverflowBounds = false; + } + } + if (inOverflowBounds) { + return targetView; + } } eventCoords[0] = restoreX; eventCoords[1] = restoreY; } } - return viewGroup; + + if (allowReturnTouchTargetTypes.contains(TouchTargetReturnType.SELF) + && isTouchPointInView(eventCoords[0], eventCoords[1], view)) { + return view; + } + + return null; + } + + /** + * 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.getRight() - view.getLeft()) + hitSlopRect.right) + && (y >= -hitSlopRect.top + && y < (view.getBottom() - view.getTop()) + hitSlopRect.bottom)) { + return true; + } + + return false; + } else { + if ((x >= 0 && x < (view.getRight() - view.getLeft())) + && (y >= 0 && y < (view.getBottom() - view.getTop()))) { + return true; + } + + return false; + } } /** - * Returns whether the touch point is within the child View It is transform aware and will invert + * 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 +225,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,32 +255,32 @@ 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) { + 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) { @@ -246,10 +290,8 @@ private static boolean isTransformedTouchPointInView( 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 e41ae5caa2d568..92731e35011eb9 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 @@ -29,6 +29,7 @@ import com.facebook.react.uimanager.MeasureSpecAssertions; 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; @@ -39,7 +40,7 @@ /** Similar to {@link ReactScrollView} but only supports horizontal scrolling. */ public class ReactHorizontalScrollView extends HorizontalScrollView - implements ReactClippingViewGroup { + implements ReactClippingViewGroup, ReactOverflowView { private static @Nullable Field sScrollerField; private static boolean sTriedToGetScrollerField = false; @@ -191,6 +192,11 @@ public void setOverflow(String overflow) { invalidate(); } + @Override + public @Nullable String getOverflow() { + return mOverflow; + } + @Override protected void onDraw(Canvas canvas) { getDrawingRect(mRect); 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 8108106a271927..162381f311a8e2 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 @@ -28,6 +28,7 @@ import com.facebook.react.uimanager.MeasureSpecAssertions; 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; @@ -44,7 +45,8 @@ public class ReactScrollView extends ScrollView implements ReactClippingViewGroup, ViewGroup.OnHierarchyChangeListener, - View.OnLayoutChangeListener { + View.OnLayoutChangeListener, + ReactOverflowView { private static @Nullable Field sScrollerField; private static boolean sTriedToGetScrollerField = false; @@ -177,6 +179,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 a191dd2cb19b11..55831cb8985e71 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 @@ -41,6 +41,7 @@ import com.facebook.react.uimanager.PointerEvents; 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; @@ -58,7 +59,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; @@ -681,6 +683,7 @@ public void setOverflow(String overflow) { invalidate(); } + @Override public @Nullable String getOverflow() { return mOverflow; }