From 6018c199917403c5f9f5159697dbc61903b9642d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Norte?= Date: Thu, 9 Feb 2023 09:36:08 -0800 Subject: [PATCH] Fix computation of relative layouts when nodes are not displayed Summary: Changelog: [General][Fixed] Fix computation of relative layout to return empty layout for nodes with display: none and children. Reviewed By: sammy-SC Differential Revision: D43045691 fbshipit-source-id: c7605cf320809ab1975e420ac1de58d72e1b8e44 --- .../renderer/core/LayoutableShadowNode.cpp | 9 ++ .../core/tests/LayoutableShadowNodeTest.cpp | 146 ++++++++++++++++++ 2 files changed, 155 insertions(+) diff --git a/ReactCommon/react/renderer/core/LayoutableShadowNode.cpp b/ReactCommon/react/renderer/core/LayoutableShadowNode.cpp index 0f785759e9becd..2c086283c0fd43 100644 --- a/ReactCommon/react/renderer/core/LayoutableShadowNode.cpp +++ b/ReactCommon/react/renderer/core/LayoutableShadowNode.cpp @@ -93,6 +93,9 @@ LayoutMetrics LayoutableShadowNode::computeRelativeLayoutMetrics( // Layout metrics of a node computed relatively to the same node are equal // to `transform`-ed layout metrics of the node with zero `origin`. auto layoutMetrics = ancestorNode.getLayoutMetrics(); + if (layoutMetrics.displayType == DisplayType::None) { + return EmptyLayoutMetrics; + } if (policy.includeTransform) { layoutMetrics.frame = layoutMetrics.frame * ancestorNode.getTransform(); } @@ -180,6 +183,12 @@ LayoutMetrics LayoutableShadowNode::computeRelativeLayoutMetrics( return EmptyLayoutMetrics; } + // Descendants of display: none don't have relative layout metrics. + if (currentShadowNode->getLayoutMetrics().displayType == + DisplayType::None) { + return EmptyLayoutMetrics; + } + auto currentFrame = shouldCalculateTransformedFrames ? transformedFrames[i] : currentShadowNode->getLayoutMetrics().frame; diff --git a/ReactCommon/react/renderer/core/tests/LayoutableShadowNodeTest.cpp b/ReactCommon/react/renderer/core/tests/LayoutableShadowNodeTest.cpp index f8b0789e48b97d..c2b444ba534b0a 100644 --- a/ReactCommon/react/renderer/core/tests/LayoutableShadowNodeTest.cpp +++ b/ReactCommon/react/renderer/core/tests/LayoutableShadowNodeTest.cpp @@ -67,6 +67,152 @@ TEST(LayoutableShadowNodeTest, relativeLayoutMetrics) { EXPECT_EQ(relativeLayoutMetrics.frame.origin.y, 20); } +/* + * ┌───────────────────┐ + * │ │ + * │ │ + * └───────────────────┘ + */ +TEST(LayoutableShadowNodeTest, relativeLayoutMetricsOnNodeWithDisplayNone) { + auto builder = simpleComponentBuilder(); + + // clang-format off + auto element = + Element() + .finalize([](ViewShadowNode &shadowNode){ + auto layoutMetrics = EmptyLayoutMetrics; + layoutMetrics.frame.size = {100, 200}; + layoutMetrics.frame.origin = {10, 20}; + layoutMetrics.displayType = DisplayType::None; + shadowNode.setLayoutMetrics(layoutMetrics); + }); + // clang-format on + + auto shadowNode = builder.build(element); + + auto relativeLayoutMetrics = + LayoutableShadowNode::computeRelativeLayoutMetrics( + shadowNode->getFamily(), *shadowNode, {}); + + // The node has display: none, so the relative layout is empty. + EXPECT_TRUE(relativeLayoutMetrics == EmptyLayoutMetrics); +} + +/* + * ┌────────────────────┐ + * │ │ + * │ │ + * │ ┌────────────────┴──┐ + * │ │ │ + * └───┤ │ + * │ │ + * │ │ + * └───────────────────┘ + */ +TEST( + LayoutableShadowNodeTest, + relativeLayoutMetricsOnChildrenOfParentWithDisplayNone) { + auto builder = simpleComponentBuilder(); + + auto childShadowNode = std::shared_ptr{}; + // clang-format off + auto element = + Element() + .finalize([](ViewShadowNode &shadowNode){ + auto layoutMetrics = EmptyLayoutMetrics; + layoutMetrics.frame.origin = {10, 20}; + layoutMetrics.frame.size = {100, 200}; + layoutMetrics.displayType = DisplayType::None; + shadowNode.setLayoutMetrics(layoutMetrics); + }) + .children({ + Element() + .finalize([](ViewShadowNode &shadowNode){ + auto layoutMetrics = EmptyLayoutMetrics; + layoutMetrics.frame.origin = {10, 20}; + layoutMetrics.frame.size = {100, 200}; + shadowNode.setLayoutMetrics(layoutMetrics); + }) + .reference(childShadowNode) + }); + // clang-format on + + auto parentShadowNode = builder.build(element); + + auto relativeLayoutMetrics = + LayoutableShadowNode::computeRelativeLayoutMetrics( + childShadowNode->getFamily(), *parentShadowNode, {}); + + // A parent node in the hierarchy has display: none, so the relative layout + // is empty. + EXPECT_TRUE(relativeLayoutMetrics == EmptyLayoutMetrics); +} + +/* + * ┌────────────────────┐ + * │ │ + * │ │ + * │ ┌────────────────┴──┐ + * │ │ │ + * └───┤ │ + * │ │ + * │ ┌───────────────┴──┐ + * └───| | + * | | + * └──────────────────┘ + */ +TEST( + LayoutableShadowNodeTest, + relativeLayoutMetricsOnOuterParentWithDisplayNone) { + auto builder = simpleComponentBuilder(); + + auto parentShadowNode = std::shared_ptr{}; + auto childShadowNode = std::shared_ptr{}; + // clang-format off + auto element = + Element() + .finalize([](ViewShadowNode &shadowNode){ + auto layoutMetrics = EmptyLayoutMetrics; + layoutMetrics.frame.origin = {0, 0}; + layoutMetrics.frame.size = {1000, 1000}; + layoutMetrics.displayType = DisplayType::None; + shadowNode.setLayoutMetrics(layoutMetrics); + }) + .children({ + Element() + .finalize([](ViewShadowNode &shadowNode){ + auto layoutMetrics = EmptyLayoutMetrics; + layoutMetrics.frame.origin = {50, 50}; + layoutMetrics.frame.size = {200, 200}; + shadowNode.setLayoutMetrics(layoutMetrics); + }) + .children({ + Element() + .finalize([](ViewShadowNode &shadowNode){ + auto layoutMetrics = EmptyLayoutMetrics; + layoutMetrics.frame.origin = {10, 20}; + layoutMetrics.frame.size = {100, 200}; + shadowNode.setLayoutMetrics(layoutMetrics); + }).reference(childShadowNode) + }) + .reference(parentShadowNode) + }); + // clang-format on + + auto outerParentShadowNode = builder.build(element); + + auto relativeLayoutMetrics = + LayoutableShadowNode::computeRelativeLayoutMetrics( + childShadowNode->getFamily(), *parentShadowNode, {}); + + // The root view has display: none, but all the views between the descendant + // and the ancestor are visible, so we compute relative layout as usual. + EXPECT_EQ(relativeLayoutMetrics.frame.size.width, 100); + EXPECT_EQ(relativeLayoutMetrics.frame.size.height, 200); + EXPECT_EQ(relativeLayoutMetrics.frame.origin.x, 10); + EXPECT_EQ(relativeLayoutMetrics.frame.origin.y, 20); +} + /* * ┌──────────────┐ * │ │