From 0facbc2398e91ce0c526b91ad21ca8dc46482da2 Mon Sep 17 00:00:00 2001 From: Soe Lynn Date: Tue, 30 Apr 2024 00:50:09 -0700 Subject: [PATCH] Fix findNodeAtPoint returns incorrect view Summary: - This fix majority work is based off of hoxyq https://www.internalfb.com/diff/D56185630 Changelog: [Internal] `Expectation`: In React DevTools, user should be able to select an element on screen and it will show you what React component rendered it. This doesn't work in RN app that is using JS navigation `Root Cause`: In Fabric, when we try to find `ShadowNode` in the `ShadowTree`, `pointerEvents` props are not considered during the lookup of node using coordinate. Hence, in React DevTools when we inspect element, it was hightlighting the overlay `View` with `pointerEvents` props `box-none` was getting highlighted instead of its children view in the hierarchy. Differential Revision: D56334314 --- .../renderer/components/root/RootShadowNode.h | 14 ++ .../components/view/ConcreteViewShadowNode.h | 14 ++ .../renderer/core/LayoutableShadowNode.cpp | 12 +- .../renderer/core/LayoutableShadowNode.h | 3 + .../core/tests/FindNodeAtPointTest.cpp | 140 ++++++++++++++++++ 5 files changed, 182 insertions(+), 1 deletion(-) diff --git a/packages/react-native/ReactCommon/react/renderer/components/root/RootShadowNode.h b/packages/react-native/ReactCommon/react/renderer/components/root/RootShadowNode.h index 321ea590d4c0a2..dcaca61a7fa0d6 100644 --- a/packages/react-native/ReactCommon/react/renderer/components/root/RootShadowNode.h +++ b/packages/react-native/ReactCommon/react/renderer/components/root/RootShadowNode.h @@ -47,6 +47,20 @@ class RootShadowNode final bool layoutIfNeeded( std::vector* affectedNodes = {}); + /* + * Since this is the RootShadowNode, it cannot be a touch target. + */ + bool canBeTouchTarget() const override { + return false; + } + + /* + * Since this is the RootShadowNode, ONLY its children can be touch targets. + */ + bool canChildrenBeTouchTarget() const override { + return true; + } + /* * Clones the node with given `layoutConstraints` and `layoutContext`. */ diff --git a/packages/react-native/ReactCommon/react/renderer/components/view/ConcreteViewShadowNode.h b/packages/react-native/ReactCommon/react/renderer/components/view/ConcreteViewShadowNode.h index 299b8db6ff3e27..4a67cdeaf05c20 100644 --- a/packages/react-native/ReactCommon/react/renderer/components/view/ConcreteViewShadowNode.h +++ b/packages/react-native/ReactCommon/react/renderer/components/view/ConcreteViewShadowNode.h @@ -84,6 +84,20 @@ class ConcreteViewShadowNode : public ConcreteShadowNode< return BaseShadowNode::getConcreteProps().resolveTransform(layoutMetrics); } + bool canBeTouchTarget() const override { + return BaseShadowNode::getConcreteProps().pointerEvents == + PointerEventsMode::Auto || + BaseShadowNode::getConcreteProps().pointerEvents == + PointerEventsMode::BoxOnly; + } + + bool canChildrenBeTouchTarget() const override { + return BaseShadowNode::getConcreteProps().pointerEvents == + PointerEventsMode::Auto || + BaseShadowNode::getConcreteProps().pointerEvents == + PointerEventsMode::BoxNone; + } + private: void initialize() noexcept { auto& props = BaseShadowNode::getConcreteProps(); diff --git a/packages/react-native/ReactCommon/react/renderer/core/LayoutableShadowNode.cpp b/packages/react-native/ReactCommon/react/renderer/core/LayoutableShadowNode.cpp index 8ea6f8f822368c..e013646decdbb3 100644 --- a/packages/react-native/ReactCommon/react/renderer/core/LayoutableShadowNode.cpp +++ b/packages/react-native/ReactCommon/react/renderer/core/LayoutableShadowNode.cpp @@ -314,12 +314,22 @@ ShadowNode::Shared LayoutableShadowNode::findNodeAtPoint( if (layoutableShadowNode == nullptr) { return nullptr; } + + if (!layoutableShadowNode->canBeTouchTarget() && + !layoutableShadowNode->canChildrenBeTouchTarget()) { + return nullptr; + } + auto frame = layoutableShadowNode->getLayoutMetrics().frame; auto transformedFrame = frame * layoutableShadowNode->getTransform(); auto isPointInside = transformedFrame.containsPoint(point); if (!isPointInside) { return nullptr; + } else if ( + layoutableShadowNode->canBeTouchTarget() && + !layoutableShadowNode->canChildrenBeTouchTarget()) { + return node; } auto newPoint = point - transformedFrame.origin - @@ -340,7 +350,7 @@ ShadowNode::Shared LayoutableShadowNode::findNodeAtPoint( return hitView; } } - return isPointInside ? node : nullptr; + return layoutableShadowNode->canBeTouchTarget() ? node : nullptr; } #if RN_DEBUG_STRING_CONVERTIBLE diff --git a/packages/react-native/ReactCommon/react/renderer/core/LayoutableShadowNode.h b/packages/react-native/ReactCommon/react/renderer/core/LayoutableShadowNode.h index f23d931c137a3f..e492ecde326124 100644 --- a/packages/react-native/ReactCommon/react/renderer/core/LayoutableShadowNode.h +++ b/packages/react-native/ReactCommon/react/renderer/core/LayoutableShadowNode.h @@ -153,6 +153,9 @@ class LayoutableShadowNode : public ShadowNode { virtual Float firstBaseline(Size size) const; virtual Float lastBaseline(Size size) const; + virtual bool canBeTouchTarget() const = 0; + virtual bool canChildrenBeTouchTarget() const = 0; + /* * Returns layoutable children to iterate on. */ diff --git a/packages/react-native/ReactCommon/react/renderer/core/tests/FindNodeAtPointTest.cpp b/packages/react-native/ReactCommon/react/renderer/core/tests/FindNodeAtPointTest.cpp index c5f74fd84fb94c..0b95fc5443bbd4 100644 --- a/packages/react-native/ReactCommon/react/renderer/core/tests/FindNodeAtPointTest.cpp +++ b/packages/react-native/ReactCommon/react/renderer/core/tests/FindNodeAtPointTest.cpp @@ -226,3 +226,143 @@ TEST(FindNodeAtPointTest, overlappingViewsWithZIndex) { EXPECT_EQ( LayoutableShadowNode::findNodeAtPoint(parentShadowNode, {50, 50})->getTag(), 2); } + +TEST(FindNodeAtPointTest, overlappingViewsWithParentPointerEventsBoxOnly) { + auto builder = simpleComponentBuilder(); + + // clang-format off + auto element = + Element() + .tag(1) + .props([] { + auto sharedProps = std::make_shared(); + sharedProps->pointerEvents = PointerEventsMode::BoxOnly; + return sharedProps; + }) + .finalize([](ViewShadowNode &shadowNode){ + auto layoutMetrics = EmptyLayoutMetrics; + layoutMetrics.frame.size = {100, 100}; + shadowNode.setLayoutMetrics(layoutMetrics); + }) + .children({ + Element() + .tag(2) + .finalize([](ViewShadowNode &shadowNode){ + auto layoutMetrics = EmptyLayoutMetrics; + layoutMetrics.frame.origin = {50, 50}; + layoutMetrics.frame.size = {50, 50}; + shadowNode.setLayoutMetrics(layoutMetrics); + }), + Element() + .tag(3) + .finalize([](ViewShadowNode &shadowNode){ + auto layoutMetrics = EmptyLayoutMetrics; + layoutMetrics.frame.origin = {50, 50}; + layoutMetrics.frame.size = {50, 50}; + shadowNode.setLayoutMetrics(layoutMetrics); + }) + }); + + auto parentShadowNode = builder.build(element); + + EXPECT_EQ( + LayoutableShadowNode::findNodeAtPoint(parentShadowNode, {60, 60})->getTag(), 1); +} + +TEST(FindNodeAtPointTest, overlappingViewsWithParentPointerEventsBoxNone) { + auto builder = simpleComponentBuilder(); + + // clang-format off + auto element = + Element() + .tag(1) + .props([] { + auto sharedProps = std::make_shared(); + sharedProps->pointerEvents = PointerEventsMode::BoxNone; + return sharedProps; + }) + .finalize([](ViewShadowNode &shadowNode){ + auto layoutMetrics = EmptyLayoutMetrics; + layoutMetrics.frame.size = {100, 100}; + shadowNode.setLayoutMetrics(layoutMetrics); + }) + .children({ + Element() + .tag(2) + .props([] { + auto sharedProps = std::make_shared(); + sharedProps->zIndex = 1; + auto &yogaStyle = sharedProps->yogaStyle; + yogaStyle.setPositionType(yoga::PositionType::Absolute); + return sharedProps; + }) + .finalize([](ViewShadowNode &shadowNode){ + auto layoutMetrics = EmptyLayoutMetrics; + layoutMetrics.frame.origin = {25, 25}; + layoutMetrics.frame.size = {50, 50}; + shadowNode.setLayoutMetrics(layoutMetrics); + }), + Element() + .tag(3) + .finalize([](ViewShadowNode &shadowNode){ + auto layoutMetrics = EmptyLayoutMetrics; + layoutMetrics.frame.origin = {50, 50}; + layoutMetrics.frame.size = {50, 50}; + shadowNode.setLayoutMetrics(layoutMetrics); + }) + }); + + auto parentShadowNode = builder.build(element); + + EXPECT_EQ( + LayoutableShadowNode::findNodeAtPoint(parentShadowNode, {50, 50})->getTag(), 2); +} + +TEST(FindNodeAtPointTest, overlappingViewsWithParentPointerEventsNone) { + auto builder = simpleComponentBuilder(); + + // clang-format off + auto element = + Element() + .tag(1) + .props([] { + auto sharedProps = std::make_shared(); + sharedProps->pointerEvents = PointerEventsMode::None; + return sharedProps; + }) + .finalize([](ViewShadowNode &shadowNode){ + auto layoutMetrics = EmptyLayoutMetrics; + layoutMetrics.frame.size = {100, 100}; + shadowNode.setLayoutMetrics(layoutMetrics); + }) + .children({ + Element() + .tag(2) + .props([] { + auto sharedProps = std::make_shared(); + sharedProps->zIndex = 1; + auto &yogaStyle = sharedProps->yogaStyle; + yogaStyle.setPositionType(yoga::PositionType::Absolute); + return sharedProps; + }) + .finalize([](ViewShadowNode &shadowNode){ + auto layoutMetrics = EmptyLayoutMetrics; + layoutMetrics.frame.origin = {25, 25}; + layoutMetrics.frame.size = {50, 50}; + shadowNode.setLayoutMetrics(layoutMetrics); + }), + Element() + .tag(3) + .finalize([](ViewShadowNode &shadowNode){ + auto layoutMetrics = EmptyLayoutMetrics; + layoutMetrics.frame.origin = {50, 50}; + layoutMetrics.frame.size = {50, 50}; + shadowNode.setLayoutMetrics(layoutMetrics); + }) + }); + + auto parentShadowNode = builder.build(element); + + EXPECT_EQ( + LayoutableShadowNode::findNodeAtPoint(parentShadowNode, {50, 50}), nullptr); +}