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
facebook · Feb 9, 2023 · 6018c19 · 6018c19
1 parent 673c761
commit 6018c19
Show file tree

Hide file tree

Showing 2 changed files with 155 additions and 0 deletions.
diff --git 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
@@ -67,6 +67,152 @@ TEST(LayoutableShadowNodeTest, relativeLayoutMetrics) {
   EXPECT_EQ(relativeLayoutMetrics.frame.origin.y, 20);
 }
 
+/*
+ * ┌───────────────────┐
+ * │<View displayNone> │
+ * │                   │
+ * └───────────────────┘
+ */
+TEST(LayoutableShadowNodeTest, relativeLayoutMetricsOnNodeWithDisplayNone) {
+  auto builder = simpleComponentBuilder();
+
+  // clang-format off
+  auto element =
+    Element<ViewShadowNode>()
+      .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);
+}
+
+/*
+ * ┌────────────────────┐
+ * │<View displayNone>  │
+ * │                    │
+ * │   ┌────────────────┴──┐
+ * │   │<View>             │
+ * └───┤                   │
+ *     │                   │
+ *     │                   │
+ *     └───────────────────┘
+ */
+TEST(
+    LayoutableShadowNodeTest,
+    relativeLayoutMetricsOnChildrenOfParentWithDisplayNone) {
+  auto builder = simpleComponentBuilder();
+
+  auto childShadowNode = std::shared_ptr<ViewShadowNode>{};
+  // clang-format off
+  auto element =
+    Element<ViewShadowNode>()
+      .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<ViewShadowNode>()
+        .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);
+}
+
+/*
+ * ┌────────────────────┐
+ * │<View displayNone>  │
+ * │                    │
+ * │   ┌────────────────┴──┐
+ * │   │<View ancestor>    │
+ * └───┤                   │
+ *     │                   │
+ *     │   ┌───────────────┴──┐
+ *     └───|<View descendant> |
+ *         |                  |
+ *         └──────────────────┘
+ */
+TEST(
+    LayoutableShadowNodeTest,
+    relativeLayoutMetricsOnOuterParentWithDisplayNone) {
+  auto builder = simpleComponentBuilder();
+
+  auto parentShadowNode = std::shared_ptr<ViewShadowNode>{};
+  auto childShadowNode = std::shared_ptr<ViewShadowNode>{};
+  // clang-format off
+  auto element =
+    Element<ViewShadowNode>()
+      .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<ViewShadowNode>()
+        .finalize([](ViewShadowNode &shadowNode){
+          auto layoutMetrics = EmptyLayoutMetrics;
+          layoutMetrics.frame.origin = {50, 50};
+          layoutMetrics.frame.size = {200, 200};
+          shadowNode.setLayoutMetrics(layoutMetrics);
+        })
+        .children({
+          Element<ViewShadowNode>()
+            .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);
+}
+
 /*
  * ┌──────────────┐
  * │<ScrollView>  │