Skip to content

Commit

Permalink
Fix findNodeAtPoint returns incorrect view
Browse files Browse the repository at this point in the history
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.

Reviewed By: javache

Differential Revision: D56334314
  • Loading branch information
realsoelynn authored and facebook-github-bot committed May 1, 2024
1 parent 639d890 commit da45bce
Show file tree
Hide file tree
Showing 4 changed files with 174 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,20 @@ class ConcreteViewShadowNode : public ConcreteShadowNode<
return BaseShadowNode::getConcreteProps().resolveTransform(layoutMetrics);
}

bool canBeTouchTarget() const override {
auto pointerEvents =
BaseShadowNode::getConcreteProps().ViewProps::pointerEvents;
return pointerEvents == PointerEventsMode::Auto ||
pointerEvents == PointerEventsMode::BoxOnly;
}

bool canChildrenBeTouchTarget() const override {
auto pointerEvents =
BaseShadowNode::getConcreteProps().ViewProps::pointerEvents;
return pointerEvents == PointerEventsMode::Auto ||
pointerEvents == PointerEventsMode::BoxNone;
}

private:
void initialize() noexcept {
auto& props = BaseShadowNode::getConcreteProps();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,14 @@ Point LayoutableShadowNode::getContentOriginOffset() const {
return {0, 0};
}

bool LayoutableShadowNode::canBeTouchTarget() const {
return false;
}

bool LayoutableShadowNode::canChildrenBeTouchTarget() const {
return true;
}

LayoutableShadowNode::UnsharedList
LayoutableShadowNode::getLayoutableChildNodes() const {
LayoutableShadowNode::UnsharedList layoutableChildren;
Expand Down Expand Up @@ -314,12 +322,20 @@ 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->canChildrenBeTouchTarget()) {
return node;
}

auto newPoint = point - transformedFrame.origin -
Expand All @@ -340,7 +356,7 @@ ShadowNode::Shared LayoutableShadowNode::findNodeAtPoint(
return hitView;
}
}
return isPointInside ? node : nullptr;
return layoutableShadowNode->canBeTouchTarget() ? node : nullptr;
}

#if RN_DEBUG_STRING_CONVERTIBLE
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,9 @@ class LayoutableShadowNode : public ShadowNode {
virtual Float firstBaseline(Size size) const;
virtual Float lastBaseline(Size size) const;

virtual bool canBeTouchTarget() const;
virtual bool canChildrenBeTouchTarget() const;

/*
* Returns layoutable children to iterate on.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<ViewShadowNode>()
.tag(1)
.props([] {
auto sharedProps = std::make_shared<ViewShadowNodeProps>();
sharedProps->pointerEvents = PointerEventsMode::BoxOnly;
return sharedProps;
})
.finalize([](ViewShadowNode &shadowNode){
auto layoutMetrics = EmptyLayoutMetrics;
layoutMetrics.frame.size = {100, 100};
shadowNode.setLayoutMetrics(layoutMetrics);
})
.children({
Element<ViewShadowNode>()
.tag(2)
.finalize([](ViewShadowNode &shadowNode){
auto layoutMetrics = EmptyLayoutMetrics;
layoutMetrics.frame.origin = {50, 50};
layoutMetrics.frame.size = {50, 50};
shadowNode.setLayoutMetrics(layoutMetrics);
}),
Element<ViewShadowNode>()
.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<ViewShadowNode>()
.tag(1)
.props([] {
auto sharedProps = std::make_shared<ViewShadowNodeProps>();
sharedProps->pointerEvents = PointerEventsMode::BoxNone;
return sharedProps;
})
.finalize([](ViewShadowNode &shadowNode){
auto layoutMetrics = EmptyLayoutMetrics;
layoutMetrics.frame.size = {100, 100};
shadowNode.setLayoutMetrics(layoutMetrics);
})
.children({
Element<ViewShadowNode>()
.tag(2)
.props([] {
auto sharedProps = std::make_shared<ViewShadowNodeProps>();
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<ViewShadowNode>()
.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<ViewShadowNode>()
.tag(1)
.props([] {
auto sharedProps = std::make_shared<ViewShadowNodeProps>();
sharedProps->pointerEvents = PointerEventsMode::None;
return sharedProps;
})
.finalize([](ViewShadowNode &shadowNode){
auto layoutMetrics = EmptyLayoutMetrics;
layoutMetrics.frame.size = {100, 100};
shadowNode.setLayoutMetrics(layoutMetrics);
})
.children({
Element<ViewShadowNode>()
.tag(2)
.props([] {
auto sharedProps = std::make_shared<ViewShadowNodeProps>();
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<ViewShadowNode>()
.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);
}

0 comments on commit da45bce

Please sign in to comment.