Prevent getRelativeLayoutMetrics to measure across ShadowNode which is root

Summary:
Changelog: [Internal]

# Analysis
Measure returns following values for `frame.y` when tapping item in bottom sheet.

Fabric 412.33331298828125.
Paper 49.

In Paper, the frame.y returned is the position of tapped item in bottom sheet relative to the bottom sheet itself, which is correct.

This can happen in both BottomSheet and Modal.

# Why it happens?
In [UIManager.getRelativeLayoutMetrics](https://our.intern.facebook.com/intern/diffusion/FBS/browse/master/xplat/js/react-native-github/ReactCommon/fabric/uimanager/UIManager.cpp?commit=a372cf516ba1245ad9462e68376ee759c118a884&lines=172-181) if `ancestorShadowNode` is nullptr we populate `ancestorShadowNode` with `rootShadowNode` for the surface.
Problem is that BottomSheet that is presented, is not a separate surface. This means that we climb up the shadow node hierarchy all the way to root shadow node and keep adding offsets. Even though we should stop when we hit shadow node representing BottomSheet.

# How could be this fixed?

I think we should add a new shadow node trait that would mark node as root node. As we are traversing the shadow node tree upwards and adding offset of that node, once we hit a node that is "anchor",  we would immediately stop the traversal.
This solution is inspired by Paper where the node representing BottomSheet has a special flag which marks it as "root" node.

accepttoship

Reviewed By: JoshuaGross, mdvacca

Differential Revision: D19640454

fbshipit-source-id: bde623b1f41a9745a41f0aada7221bf924fad453
This commit is contained in:
Samuel Susla 2020-01-30 15:28:21 -08:00 коммит произвёл Facebook Github Bot
Родитель e1b8c954ef
Коммит fdd133e214
3 изменённых файлов: 56 добавлений и 10 удалений

Просмотреть файл

@ -43,6 +43,11 @@ static LayoutMetrics calculateOffsetForLayoutMetrics(
for (auto it = ancestors.rbegin(); it != ancestors.rend() - 1; ++it) { for (auto it = ancestors.rbegin(); it != ancestors.rend() - 1; ++it) {
auto &currentShadowNode = it->first.get(); auto &currentShadowNode = it->first.get();
if (currentShadowNode.getTraits().check(
ShadowNodeTraits::Trait::RootNodeKind)) {
break;
}
auto layoutableCurrentShadowNode = auto layoutableCurrentShadowNode =
dynamic_cast<LayoutableShadowNode const *>(&currentShadowNode); dynamic_cast<LayoutableShadowNode const *>(&currentShadowNode);

Просмотреть файл

@ -41,6 +41,12 @@ class ShadowNodeTraits {
// Inherits `BaseTextShadowNode`. // Inherits `BaseTextShadowNode`.
TextKind = 1 << 3, TextKind = 1 << 3,
// Used when calculating relative layout in
// LayoutableShadowNode::getRelativeLayoutMetrics. This trait marks node as
// root, so when calculating relative layout, the calculation will not
// traverse beyond this node. See T61257516 for details.
RootNodeKind = 1 << 4,
// Inherits `YogaLayoutableShadowNode` and enforces that the `YGNode` is a // Inherits `YogaLayoutableShadowNode` and enforces that the `YGNode` is a
// leaf. // leaf.
LeafYogaNode = 1 << 10, LeafYogaNode = 1 << 10,

Просмотреть файл

@ -31,7 +31,7 @@ class LayoutableShadowNodeTest : public ::testing::Test {
familyA, familyA,
ShadowNodeTraits{}); ShadowNodeTraits{});
auto familyB = std::make_shared<ShadowNodeFamily>( auto familyAA = std::make_shared<ShadowNodeFamily>(
ShadowNodeFamilyFragment{ ShadowNodeFamilyFragment{
/* .tag = */ 10, /* .tag = */ 10,
/* .surfaceId = */ 1, /* .surfaceId = */ 1,
@ -39,20 +39,41 @@ class LayoutableShadowNodeTest : public ::testing::Test {
}, },
componentDescriptor_); componentDescriptor_);
nodeB_ = std::make_shared<TestShadowNode>( auto traits = TestShadowNode::BaseTraits();
traits.set(ShadowNodeTraits::Trait::RootNodeKind);
nodeAA_ = std::make_shared<TestShadowNode>(
ShadowNodeFragment{ ShadowNodeFragment{
/* .props = */ std::make_shared<const TestProps>(), /* .props = */ std::make_shared<const TestProps>(),
/* .children = */ ShadowNode::emptySharedShadowNodeSharedList(), /* .children = */ ShadowNode::emptySharedShadowNodeSharedList(),
}, },
familyB, familyAA,
traits);
auto familyAAA = std::make_shared<ShadowNodeFamily>(
ShadowNodeFamilyFragment{
/* .tag = */ 11,
/* .surfaceId = */ 1,
/* .eventEmitter = */ nullptr,
},
componentDescriptor_);
nodeAAA_ = std::make_shared<TestShadowNode>(
ShadowNodeFragment{
/* .props = */ std::make_shared<const TestProps>(),
/* .children = */ ShadowNode::emptySharedShadowNodeSharedList(),
},
familyAAA,
ShadowNodeTraits{}); ShadowNodeTraits{});
nodeA_->appendChild(nodeB_); nodeA_->appendChild(nodeAA_);
nodeAA_->appendChild(nodeAAA_);
} }
std::shared_ptr<EventDispatcher const> eventDispatcher_; std::shared_ptr<EventDispatcher const> eventDispatcher_;
std::shared_ptr<TestShadowNode> nodeA_; std::shared_ptr<TestShadowNode> nodeA_;
std::shared_ptr<TestShadowNode> nodeB_; std::shared_ptr<TestShadowNode> nodeAA_;
std::shared_ptr<TestShadowNode> nodeAAA_;
TestComponentDescriptor componentDescriptor_; TestComponentDescriptor componentDescriptor_;
}; };
@ -61,9 +82,9 @@ TEST_F(LayoutableShadowNodeTest, relativeLayoutMetrics) {
layoutMetrics.frame.origin = {10, 20}; layoutMetrics.frame.origin = {10, 20};
layoutMetrics.frame.size = {100, 200}; layoutMetrics.frame.size = {100, 200};
nodeA_->setLayoutMetrics(layoutMetrics); nodeA_->setLayoutMetrics(layoutMetrics);
nodeB_->setLayoutMetrics(layoutMetrics); nodeAA_->setLayoutMetrics(layoutMetrics);
auto relativeLayoutMetrics = nodeB_->getRelativeLayoutMetrics(*nodeA_, {}); auto relativeLayoutMetrics = nodeAA_->getRelativeLayoutMetrics(*nodeA_, {});
// A is a parent to B, A has origin {10, 10}, B has origin {10, 10}. // A is a parent to B, A has origin {10, 10}, B has origin {10, 10}.
// B's relative origin to A should be {10, 10}. // B's relative origin to A should be {10, 10}.
@ -89,15 +110,29 @@ TEST_F(LayoutableShadowNodeTest, relativeLayoutMetricsOnSameNode) {
TEST_F(LayoutableShadowNodeTest, relativeLayourMetricsOnClonedNode) { TEST_F(LayoutableShadowNodeTest, relativeLayourMetricsOnClonedNode) {
// B is cloned and mutated. // B is cloned and mutated.
auto nodeBRevision2 = std::static_pointer_cast<TestShadowNode>( auto nodeBRevision2 = std::static_pointer_cast<TestShadowNode>(
componentDescriptor_.cloneShadowNode(*nodeB_, ShadowNodeFragment{})); componentDescriptor_.cloneShadowNode(*nodeAA_, ShadowNodeFragment{}));
auto layoutMetrics = EmptyLayoutMetrics; auto layoutMetrics = EmptyLayoutMetrics;
layoutMetrics.frame.size = {500, 600}; layoutMetrics.frame.size = {500, 600};
nodeBRevision2->setLayoutMetrics(layoutMetrics); nodeBRevision2->setLayoutMetrics(layoutMetrics);
nodeA_->replaceChild(*nodeB_, nodeBRevision2); nodeA_->replaceChild(*nodeAA_, nodeBRevision2);
// Even if we ask old ShadowNode for its relative layoutMetrics, it needs to // Even if we ask old ShadowNode for its relative layoutMetrics, it needs to
// return correct, new layoutMetrics. D19433873 has more about the issue. // return correct, new layoutMetrics. D19433873 has more about the issue.
auto newRelativeLayoutMetrics = nodeB_->getRelativeLayoutMetrics(*nodeA_, {}); auto newRelativeLayoutMetrics =
nodeAA_->getRelativeLayoutMetrics(*nodeA_, {});
EXPECT_EQ(newRelativeLayoutMetrics.frame.size.width, 500); EXPECT_EQ(newRelativeLayoutMetrics.frame.size.width, 500);
EXPECT_EQ(newRelativeLayoutMetrics.frame.size.height, 600); EXPECT_EQ(newRelativeLayoutMetrics.frame.size.height, 600);
} }
TEST_F(LayoutableShadowNodeTest, relativeLayoutMetricsOnSameNode2) {
auto layoutMetrics = EmptyLayoutMetrics;
nodeA_->setLayoutMetrics(layoutMetrics);
layoutMetrics.frame.origin = {10, 10};
nodeAA_->setLayoutMetrics(layoutMetrics);
nodeAAA_->setLayoutMetrics(layoutMetrics);
auto relativeLayoutMetrics = nodeAAA_->getRelativeLayoutMetrics(*nodeA_, {});
EXPECT_EQ(relativeLayoutMetrics.frame.origin.x, 10);
EXPECT_EQ(relativeLayoutMetrics.frame.origin.y, 10);
}