Skip to content

Commit

Permalink
Fix overflowInset to take into account hitSlop
Browse files Browse the repository at this point in the history
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
  • Loading branch information
genkikondo authored and facebook-github-bot committed Mar 7, 2023
1 parent f264fe1 commit 96659f8
Show file tree
Hide file tree
Showing 3 changed files with 108 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include <react/debug/flags.h>
#include <react/debug/react_native_assert.h>
#include <react/renderer/components/view/ViewProps.h>
#include <react/renderer/components/view/ViewShadowNode.h>
#include <react/renderer/components/view/conversions.h>
#include <react/renderer/core/LayoutConstraints.h>
#include <react/renderer/core/LayoutContext.h>
Expand Down Expand Up @@ -624,12 +625,19 @@ void YogaLayoutableShadowNode::layout(LayoutContext layoutContext) {

auto layoutMetricsWithOverflowInset = childNode.getLayoutMetrics();
if (layoutMetricsWithOverflowInset.displayType != DisplayType::None) {
auto viewChildNode = traitCast<ViewShadowNode const *>(&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()) {
Expand All @@ -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));
}
}
}
Expand Down
90 changes: 88 additions & 2 deletions ReactCommon/react/renderer/components/view/tests/LayoutTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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({
Expand Down Expand Up @@ -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
10 changes: 10 additions & 0 deletions ReactCommon/react/renderer/graphics/RectangleEdges.h
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down

0 comments on commit 96659f8

Please sign in to comment.