Skip to content

Commit

Permalink
Temporarily disable support for transformations in findNodeAtPoint in…
Browse files Browse the repository at this point in the history
…frastructure

Summary:
Changelog: [Internal]

FindNoteAtPoint takes transformations into account. There is however a problem with how transformation in ScrollView. In ScrollViewShadowNode, getTransform is overriden. In the override we apply `Transform::Translate(-contentOffset.x, -contentOffset.y, 0)`, this means that ScrollView has moved by {-x,-y}. However this is only true for its children, not for scroll view itself. This trips off findNodeAtPoint and if you scroll more than than screen's height, point will not be evaluated as inside ScrollView.

Until we can figure out how to deal with this properly, I think it is better to disable it as usage of JS Inspector is more common in ScrollViews than it is with transformed views.

Reviewed By: shergin

Differential Revision: D22332779

fbshipit-source-id: f48c9ae67a595e6954c2b70fb287db7f8c74378b
  • Loading branch information
sammy-SC authored and facebook-github-bot committed Jul 2, 2020
1 parent ffa0725 commit 40f37b9
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 17 deletions.
3 changes: 1 addition & 2 deletions ReactCommon/fabric/core/layout/LayoutableShadowNode.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -217,8 +217,7 @@ ShadowNode::Shared LayoutableShadowNode::findNodeAtPoint(
return nullptr;
}
auto frame = layoutableShadowNode->getLayoutMetrics().frame;
auto transformedFrame = frame * layoutableShadowNode->getTransform();
auto isPointInside = transformedFrame.containsPoint(point);
auto isPointInside = frame.containsPoint(point);

if (!isPointInside) {
return nullptr;
Expand Down
33 changes: 18 additions & 15 deletions ReactCommon/fabric/core/tests/FindNodeAtPointTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -102,18 +102,21 @@ TEST_F(FindNodeAtPointTest, withoutTransform) {
LayoutableShadowNode::findNodeAtPoint(nodeA_, {1001, 1001}), nullptr);
}

TEST_F(FindNodeAtPointTest, viewIsTranslated) {
nodeA_->_transform =
Transform::Identity() * Transform::Translate(-100, -100, 0);

EXPECT_EQ(
LayoutableShadowNode::findNodeAtPoint(nodeA_, {15, 15})->getTag(),
nodeAAA_->getTag());
EXPECT_EQ(LayoutableShadowNode::findNodeAtPoint(nodeA_, {5, 5}), nodeAA_);
}

TEST_F(FindNodeAtPointTest, viewIsScaled) {
nodeAAA_->_transform = Transform::Identity() * Transform::Scale(0.5, 0.5, 0);

EXPECT_EQ(LayoutableShadowNode::findNodeAtPoint(nodeA_, {119, 119}), nodeAA_);
}
// Uncomment once T69368852 is resolved.
// TEST_F(FindNodeAtPointTest, viewIsTranslated) {
// nodeA_->_transform =
// Transform::Identity() * Transform::Translate(-100, -100, 0);

// EXPECT_EQ(
// LayoutableShadowNode::findNodeAtPoint(nodeA_, {15, 15})->getTag(),
// nodeAAA_->getTag());
// EXPECT_EQ(LayoutableShadowNode::findNodeAtPoint(nodeA_, {5, 5}), nodeAA_);
// }

// TEST_F(FindNodeAtPointTest, viewIsScaled) {
// nodeAAA_->_transform = Transform::Identity() * Transform::Scale(0.5, 0.5,
// 0);

// EXPECT_EQ(LayoutableShadowNode::findNodeAtPoint(nodeA_, {119,
// 119})->getTag(), nodeAA_->getTag());
// }

0 comments on commit 40f37b9

Please sign in to comment.