From 96659f8e83e68f6330aaa59e3d5fb0953c67f1d1 Mon Sep 17 00:00:00 2001 From: Genki Kondo Date: Tue, 7 Mar 2023 12:30:14 -0800 Subject: [PATCH] Fix overflowInset to take into account hitSlop Summary: Previously, the touch area could never extend past the parent view bounds. Thus, hitSlop would not have any effect if it extended beyond any ancestor view's view bounds. In this diff, we apply hitSlop when calculating overflowInset. Specifically, we make sure to union the hit slop areas of all children when calculating the contentFrame. overflowInset is then used for hit testing (as in [TouchTargetHelper.java](https://www.internalfb.com/code/fbsource/[b0f630bb24271d8ed543e98ff144590290e19805]/xplat/js/react-native-github/ReactAndroid/src/main/java/com/facebook/react/uimanager/TouchTargetHelper.java?lines=195)). A risk is that other optimizations that may potentially rely on overflowInset beyond hit testing (such as clipToBounds) may not be as efficient with a large hitSlop. Changelog: [General][Fixed] - Fix touch handling so that hitSlop can extend beyond parent view bounds. Reviewed By: javache Differential Revision: D43854774 fbshipit-source-id: 160bef135b8487c28c4ada662577c35a7a36f484 --- .../view/YogaLayoutableShadowNode.cpp | 10 +++ .../components/view/tests/LayoutTest.cpp | 90 ++++++++++++++++++- .../react/renderer/graphics/RectangleEdges.h | 10 +++ 3 files changed, 108 insertions(+), 2 deletions(-) diff --git a/ReactCommon/react/renderer/components/view/YogaLayoutableShadowNode.cpp b/ReactCommon/react/renderer/components/view/YogaLayoutableShadowNode.cpp index ef72d45011fed7..84581375c554ea 100644 --- a/ReactCommon/react/renderer/components/view/YogaLayoutableShadowNode.cpp +++ b/ReactCommon/react/renderer/components/view/YogaLayoutableShadowNode.cpp @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -624,12 +625,19 @@ void YogaLayoutableShadowNode::layout(LayoutContext layoutContext) { auto layoutMetricsWithOverflowInset = childNode.getLayoutMetrics(); if (layoutMetricsWithOverflowInset.displayType != DisplayType::None) { + auto viewChildNode = traitCast(&childNode); + auto hitSlop = viewChildNode != nullptr + ? viewChildNode->getConcreteProps().hitSlop + : EdgeInsets{}; + // The contentFrame should always union with existing child node layout + // overflowInset. The transform may in a deferred animation and not // applied yet. contentFrame.unionInPlace(insetBy( layoutMetricsWithOverflowInset.frame, layoutMetricsWithOverflowInset.overflowInset)); + contentFrame.unionInPlace( + outsetBy(layoutMetricsWithOverflowInset.frame, hitSlop)); auto childTransform = childNode.getTransform(); if (childTransform != Transform::Identity()) { @@ -639,6 +647,8 @@ void YogaLayoutableShadowNode::layout(LayoutContext layoutContext) { contentFrame.unionInPlace(insetBy( layoutMetricsWithOverflowInset.frame * childTransform, layoutMetricsWithOverflowInset.overflowInset * childTransform)); + contentFrame.unionInPlace(outsetBy( + layoutMetricsWithOverflowInset.frame * childTransform, hitSlop)); } } } diff --git a/ReactCommon/react/renderer/components/view/tests/LayoutTest.cpp b/ReactCommon/react/renderer/components/view/tests/LayoutTest.cpp index 5891c27647980d..8a5f1a9f7d23d6 100644 --- a/ReactCommon/react/renderer/components/view/tests/LayoutTest.cpp +++ b/ReactCommon/react/renderer/components/view/tests/LayoutTest.cpp @@ -45,7 +45,14 @@ namespace facebook::react { // ***********************│ │************************************ // ***********************└──────────┘************************************ -enum TestCase { AS_IS, CLIPPING, TRANSFORM_SCALE, TRANSFORM_TRANSLATE }; +enum TestCase { + AS_IS, + CLIPPING, + HIT_SLOP, + HIT_SLOP_TRANSFORM_TRANSLATE, + TRANSFORM_SCALE, + TRANSFORM_TRANSLATE, +}; class LayoutTest : public ::testing::Test { protected: @@ -105,9 +112,14 @@ class LayoutTest : public ::testing::Test { props.transform = props.transform * Transform::Scale(2, 2, 1); } - if (testCase == TRANSFORM_TRANSLATE) { + if (testCase == TRANSFORM_TRANSLATE || testCase == HIT_SLOP_TRANSFORM_TRANSLATE) { props.transform = props.transform * Transform::Translate(10, 10, 0); } + + if (testCase == HIT_SLOP || testCase == HIT_SLOP_TRANSFORM_TRANSLATE) { + props.hitSlop = EdgeInsets{50, 50, 50, 50}; + } + return sharedProps; }) .children({ @@ -358,4 +370,78 @@ TEST_F(LayoutTest, overflowInsetTransformScaleTest) { EXPECT_EQ(layoutMetricsABC.overflowInset.bottom, 0); } +TEST_F(LayoutTest, overflowInsetHitSlopTest) { + initialize(HIT_SLOP); + + auto layoutMetricsA = viewShadowNodeA_->getLayoutMetrics(); + + EXPECT_EQ(layoutMetricsA.frame.size.width, 50); + EXPECT_EQ(layoutMetricsA.frame.size.height, 50); + + // Change on parent node + EXPECT_EQ(layoutMetricsA.overflowInset.left, -50); + EXPECT_EQ(layoutMetricsA.overflowInset.top, -40); + EXPECT_EQ(layoutMetricsA.overflowInset.right, -80); + EXPECT_EQ(layoutMetricsA.overflowInset.bottom, -100); + + auto layoutMetricsAB = viewShadowNodeAB_->getLayoutMetrics(); + + EXPECT_EQ(layoutMetricsAB.frame.size.width, 30); + EXPECT_EQ(layoutMetricsAB.frame.size.height, 90); + + // No change on self node + EXPECT_EQ(layoutMetricsAB.overflowInset.left, -60); + EXPECT_EQ(layoutMetricsAB.overflowInset.top, -40); + EXPECT_EQ(layoutMetricsAB.overflowInset.right, -90); + EXPECT_EQ(layoutMetricsAB.overflowInset.bottom, 0); + + auto layoutMetricsABC = viewShadowNodeABC_->getLayoutMetrics(); + + EXPECT_EQ(layoutMetricsABC.frame.size.width, 110); + EXPECT_EQ(layoutMetricsABC.frame.size.height, 20); + + // No change on child node + EXPECT_EQ(layoutMetricsABC.overflowInset.left, 0); + EXPECT_EQ(layoutMetricsABC.overflowInset.top, -50); + EXPECT_EQ(layoutMetricsABC.overflowInset.right, 0); + EXPECT_EQ(layoutMetricsABC.overflowInset.bottom, 0); +} + +TEST_F(LayoutTest, overflowInsetHitSlopTransformTranslateTest) { + initialize(HIT_SLOP_TRANSFORM_TRANSLATE); + + auto layoutMetricsA = viewShadowNodeA_->getLayoutMetrics(); + + EXPECT_EQ(layoutMetricsA.frame.size.width, 50); + EXPECT_EQ(layoutMetricsA.frame.size.height, 50); + + // Change on parent node + EXPECT_EQ(layoutMetricsA.overflowInset.left, -50); + EXPECT_EQ(layoutMetricsA.overflowInset.top, -40); + EXPECT_EQ(layoutMetricsA.overflowInset.right, -90); + EXPECT_EQ(layoutMetricsA.overflowInset.bottom, -110); + + auto layoutMetricsAB = viewShadowNodeAB_->getLayoutMetrics(); + + EXPECT_EQ(layoutMetricsAB.frame.size.width, 30); + EXPECT_EQ(layoutMetricsAB.frame.size.height, 90); + + // No change on self node + EXPECT_EQ(layoutMetricsAB.overflowInset.left, -60); + EXPECT_EQ(layoutMetricsAB.overflowInset.top, -40); + EXPECT_EQ(layoutMetricsAB.overflowInset.right, -90); + EXPECT_EQ(layoutMetricsAB.overflowInset.bottom, 0); + + auto layoutMetricsABC = viewShadowNodeABC_->getLayoutMetrics(); + + EXPECT_EQ(layoutMetricsABC.frame.size.width, 110); + EXPECT_EQ(layoutMetricsABC.frame.size.height, 20); + + // No change on child node + EXPECT_EQ(layoutMetricsABC.overflowInset.left, 0); + EXPECT_EQ(layoutMetricsABC.overflowInset.top, -50); + EXPECT_EQ(layoutMetricsABC.overflowInset.right, 0); + EXPECT_EQ(layoutMetricsABC.overflowInset.bottom, 0); +} + } // namespace facebook::react diff --git a/ReactCommon/react/renderer/graphics/RectangleEdges.h b/ReactCommon/react/renderer/graphics/RectangleEdges.h index c7948635c0d689..4452475ff5ac5b 100644 --- a/ReactCommon/react/renderer/graphics/RectangleEdges.h +++ b/ReactCommon/react/renderer/graphics/RectangleEdges.h @@ -83,6 +83,16 @@ inline Rect insetBy(Rect const &rect, EdgeInsets const &insets) noexcept { rect.size.height - insets.top - insets.bottom}}; } +/* + * Adjusts a rectangle by the given edge outsets. + */ +inline Rect outsetBy(Rect const &rect, EdgeInsets const &outsets) noexcept { + return Rect{ + {rect.origin.x - outsets.left, rect.origin.y - outsets.top}, + {rect.size.width + outsets.left + outsets.right, + rect.size.height + outsets.top + outsets.bottom}}; +} + } // namespace react } // namespace facebook