From 4bea2c48e4f9d976f6a038b19968ddfa08185f55 Mon Sep 17 00:00:00 2001 From: Eric Rozell Date: Wed, 10 Mar 2021 13:08:48 -0800 Subject: [PATCH] Fixes layout of nodes with YGDisplayNone and YGPositionTypeAbsolute (#1068) Summary: Pull Request resolved: https://github.com/facebook/yoga/pull/1068 There is an issue in react-native when the Yoga node position type is set to absolute and display: none is set where the node layout calculation gives the absolute dimensions, rather than the expected 0 x 0. Here are some OSS issues tracking this: https://github.com/facebook/react-native/issues/18415 https://github.com/microsoft/react-native-windows/issues/7289 ## Changelog [General] [Fix] - Fixes layout of nodes with YGDisplayNone and YGPositionTypeAbsolute Reviewed By: Andrey-Mishanin Differential Revision: D26849307 fbshipit-source-id: 197618aa3c4e1b3b7efeba7ea4efd30b2d1c982d --- csharp/tests/Facebook.Yoga/YGDisplayTest.cs | 49 ++++++++++++++++-- gentest/fixtures/YGDisplayTest.html | 4 ++ gentest/gentest.js | 6 +-- gentest/gentest.rb | 15 +++--- .../com/facebook/yoga/YGDisplayTest.java | 48 ++++++++++++++++-- .../tests/Facebook.Yoga/YGDisplayTest.js | 50 ++++++++++++++++++- tests/YGDisplayTest.cpp | 47 ++++++++++++++++- yoga/Yoga.cpp | 3 +- 8 files changed, 201 insertions(+), 21 deletions(-) diff --git a/csharp/tests/Facebook.Yoga/YGDisplayTest.cs b/csharp/tests/Facebook.Yoga/YGDisplayTest.cs index 237686ba01..659cb37f28 100644 --- a/csharp/tests/Facebook.Yoga/YGDisplayTest.cs +++ b/csharp/tests/Facebook.Yoga/YGDisplayTest.cs @@ -1,9 +1,10 @@ -/** +/* * Copyright (c) Facebook, Inc. and its affiliates. * - * This source code is licensed under the MIT license found in the LICENSE - * file in the root directory of this source tree. + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. */ + // @Generated by gentest/gentest.rb from gentest/fixtures/YGDisplayTest.html using System; @@ -333,5 +334,47 @@ public void Test_display_none_with_position() Assert.AreEqual(0f, root_child1.LayoutHeight); } + [Test] + public void Test_display_none_with_position_absolute() + { + YogaConfig config = new YogaConfig(); + + YogaNode root = new YogaNode(config); + root.Width = 100; + root.Height = 100; + + YogaNode root_child0 = new YogaNode(config); + root_child0.PositionType = YogaPositionType.Absolute; + root_child0.Width = 100; + root_child0.Height = 100; + root_child0.Display = YogaDisplay.None; + root.Insert(0, root_child0); + root.StyleDirection = YogaDirection.LTR; + root.CalculateLayout(); + + Assert.AreEqual(0f, root.LayoutX); + Assert.AreEqual(0f, root.LayoutY); + Assert.AreEqual(100f, root.LayoutWidth); + Assert.AreEqual(100f, root.LayoutHeight); + + Assert.AreEqual(0f, root_child0.LayoutX); + Assert.AreEqual(0f, root_child0.LayoutY); + Assert.AreEqual(0f, root_child0.LayoutWidth); + Assert.AreEqual(0f, root_child0.LayoutHeight); + + root.StyleDirection = YogaDirection.RTL; + root.CalculateLayout(); + + Assert.AreEqual(0f, root.LayoutX); + Assert.AreEqual(0f, root.LayoutY); + Assert.AreEqual(100f, root.LayoutWidth); + Assert.AreEqual(100f, root.LayoutHeight); + + Assert.AreEqual(0f, root_child0.LayoutX); + Assert.AreEqual(0f, root_child0.LayoutY); + Assert.AreEqual(0f, root_child0.LayoutWidth); + Assert.AreEqual(0f, root_child0.LayoutHeight); + } + } } diff --git a/gentest/fixtures/YGDisplayTest.html b/gentest/fixtures/YGDisplayTest.html index 74d11ba477..f11533f88c 100644 --- a/gentest/fixtures/YGDisplayTest.html +++ b/gentest/fixtures/YGDisplayTest.html @@ -25,3 +25,7 @@
+ +
+
+
diff --git a/gentest/gentest.js b/gentest/gentest.js index ca37092886..12e76f33ba 100755 --- a/gentest/gentest.js +++ b/gentest/gentest.js @@ -41,11 +41,11 @@ function assert(condition, message) { function printTest(e, LTRContainer, RTLContainer, genericContainer) { e.push([ - '/**', + '/*', ' * Copyright (c) Facebook, Inc. and its affiliates.', ' *', - ' * This source code is licensed under the MIT license found in the LICENSE', - ' * file in the root directory of this source tree.', + ' * This source code is licensed under the MIT license found in the', + ' * LICENSE file in the root directory of this source tree.', ' */', '// @Generated by gentest/gentest.rb from gentest/fixtures/' + document.title + '.html', '', diff --git a/gentest/gentest.rb b/gentest/gentest.rb index 2211450140..01fec93e05 100644 --- a/gentest/gentest.rb +++ b/gentest/gentest.rb @@ -7,13 +7,14 @@ require 'watir' require 'fileutils' -caps = Selenium::WebDriver::Remote::Capabilities.chrome( - "loggingPrefs"=>{ - "browser"=>"ALL", - "performance"=>"ALL" - } -) -browser = Watir::Browser.new(:chrome, :desired_capabilities => caps, :switches => ['--force-device-scale-factor=1', '--window-position=0,0']) +browser = Watir::Browser.new(:chrome, "goog:loggingPrefs" => { + "browser" => "ALL", + "performance" => "ALL" + }, + "chromeOptions" => { + "w3c" => "false" + }, + :switches => ['--force-device-scale-factor=1', '--window-position=0,0']) Dir.chdir(File.dirname($0)) diff --git a/java/tests/com/facebook/yoga/YGDisplayTest.java b/java/tests/com/facebook/yoga/YGDisplayTest.java index f1e6f89679..999548bd82 100644 --- a/java/tests/com/facebook/yoga/YGDisplayTest.java +++ b/java/tests/com/facebook/yoga/YGDisplayTest.java @@ -1,9 +1,10 @@ -/** +/* * Copyright (c) Facebook, Inc. and its affiliates. * - * This source code is licensed under the MIT license found in the LICENSE - * file in the root directory of this source tree. + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. */ + // @Generated by gentest/gentest.rb from gentest/fixtures/YGDisplayTest.html package com.facebook.yoga; @@ -337,6 +338,47 @@ public void test_display_none_with_position() { assertEquals(0f, root_child1.getLayoutHeight(), 0.0f); } + @Test + public void test_display_none_with_position_absolute() { + YogaConfig config = YogaConfigFactory.create(); + + final YogaNode root = createNode(config); + root.setWidth(100f); + root.setHeight(100f); + + final YogaNode root_child0 = createNode(config); + root_child0.setPositionType(YogaPositionType.ABSOLUTE); + root_child0.setWidth(100f); + root_child0.setHeight(100f); + root_child0.setDisplay(YogaDisplay.NONE); + root.addChildAt(root_child0, 0); + root.setDirection(YogaDirection.LTR); + root.calculateLayout(YogaConstants.UNDEFINED, YogaConstants.UNDEFINED); + + assertEquals(0f, root.getLayoutX(), 0.0f); + assertEquals(0f, root.getLayoutY(), 0.0f); + assertEquals(100f, root.getLayoutWidth(), 0.0f); + assertEquals(100f, root.getLayoutHeight(), 0.0f); + + assertEquals(0f, root_child0.getLayoutX(), 0.0f); + assertEquals(0f, root_child0.getLayoutY(), 0.0f); + assertEquals(0f, root_child0.getLayoutWidth(), 0.0f); + assertEquals(0f, root_child0.getLayoutHeight(), 0.0f); + + root.setDirection(YogaDirection.RTL); + root.calculateLayout(YogaConstants.UNDEFINED, YogaConstants.UNDEFINED); + + assertEquals(0f, root.getLayoutX(), 0.0f); + assertEquals(0f, root.getLayoutY(), 0.0f); + assertEquals(100f, root.getLayoutWidth(), 0.0f); + assertEquals(100f, root.getLayoutHeight(), 0.0f); + + assertEquals(0f, root_child0.getLayoutX(), 0.0f); + assertEquals(0f, root_child0.getLayoutY(), 0.0f); + assertEquals(0f, root_child0.getLayoutWidth(), 0.0f); + assertEquals(0f, root_child0.getLayoutHeight(), 0.0f); + } + private YogaNode createNode(YogaConfig config) { return mNodeFactory.create(config); } diff --git a/javascript/tests/Facebook.Yoga/YGDisplayTest.js b/javascript/tests/Facebook.Yoga/YGDisplayTest.js index e5be4a556d..cf65ab2265 100644 --- a/javascript/tests/Facebook.Yoga/YGDisplayTest.js +++ b/javascript/tests/Facebook.Yoga/YGDisplayTest.js @@ -1,9 +1,10 @@ /** * Copyright (c) Facebook, Inc. and its affiliates. * - * This source code is licensed under the MIT license found in the LICENSE - * file in the root directory of this source tree. + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. */ + // @Generated by gentest/gentest.rb from gentest/fixtures/YGDisplayTest.html var Yoga = Yoga || require("../../sources/entry-" + process.env.TEST_ENTRY); @@ -342,3 +343,48 @@ it("display_none_with_position", function () { config.free(); } }); +it("display_none_with_position_absolute", function () { + var config = Yoga.Config.create(); + + try { + var root = Yoga.Node.create(config); + root.setWidth(100); + root.setHeight(100); + + var root_child0 = Yoga.Node.create(config); + root_child0.setPositionType(Yoga.POSITION_TYPE_ABSOLUTE); + root_child0.setWidth(100); + root_child0.setHeight(100); + root_child0.setDisplay(Yoga.DISPLAY_NONE); + root.insertChild(root_child0, 0); + root.calculateLayout(Yoga.UNDEFINED, Yoga.UNDEFINED, Yoga.DIRECTION_LTR); + + console.assert(0 === root.getComputedLeft(), "0 === root.getComputedLeft() (" + root.getComputedLeft() + ")"); + console.assert(0 === root.getComputedTop(), "0 === root.getComputedTop() (" + root.getComputedTop() + ")"); + console.assert(100 === root.getComputedWidth(), "100 === root.getComputedWidth() (" + root.getComputedWidth() + ")"); + console.assert(100 === root.getComputedHeight(), "100 === root.getComputedHeight() (" + root.getComputedHeight() + ")"); + + console.assert(0 === root_child0.getComputedLeft(), "0 === root_child0.getComputedLeft() (" + root_child0.getComputedLeft() + ")"); + console.assert(0 === root_child0.getComputedTop(), "0 === root_child0.getComputedTop() (" + root_child0.getComputedTop() + ")"); + console.assert(0 === root_child0.getComputedWidth(), "0 === root_child0.getComputedWidth() (" + root_child0.getComputedWidth() + ")"); + console.assert(0 === root_child0.getComputedHeight(), "0 === root_child0.getComputedHeight() (" + root_child0.getComputedHeight() + ")"); + + root.calculateLayout(Yoga.UNDEFINED, Yoga.UNDEFINED, Yoga.DIRECTION_RTL); + + console.assert(0 === root.getComputedLeft(), "0 === root.getComputedLeft() (" + root.getComputedLeft() + ")"); + console.assert(0 === root.getComputedTop(), "0 === root.getComputedTop() (" + root.getComputedTop() + ")"); + console.assert(100 === root.getComputedWidth(), "100 === root.getComputedWidth() (" + root.getComputedWidth() + ")"); + console.assert(100 === root.getComputedHeight(), "100 === root.getComputedHeight() (" + root.getComputedHeight() + ")"); + + console.assert(0 === root_child0.getComputedLeft(), "0 === root_child0.getComputedLeft() (" + root_child0.getComputedLeft() + ")"); + console.assert(0 === root_child0.getComputedTop(), "0 === root_child0.getComputedTop() (" + root_child0.getComputedTop() + ")"); + console.assert(0 === root_child0.getComputedWidth(), "0 === root_child0.getComputedWidth() (" + root_child0.getComputedWidth() + ")"); + console.assert(0 === root_child0.getComputedHeight(), "0 === root_child0.getComputedHeight() (" + root_child0.getComputedHeight() + ")"); + } finally { + if (typeof root !== "undefined") { + root.freeRecursive(); + } + + config.free(); + } +}); diff --git a/tests/YGDisplayTest.cpp b/tests/YGDisplayTest.cpp index ebd2bcd543..edc533589e 100644 --- a/tests/YGDisplayTest.cpp +++ b/tests/YGDisplayTest.cpp @@ -1,9 +1,10 @@ /* * Copyright (c) Facebook, Inc. and its affiliates. * - * This source code is licensed under the MIT license found in the LICENSE - * file in the root directory of this source tree. + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. */ + // @Generated by gentest/gentest.rb from gentest/fixtures/YGDisplayTest.html #include @@ -327,3 +328,45 @@ TEST(YogaTest, display_none_with_position) { YGConfigFree(config); } + +TEST(YogaTest, display_none_with_position_absolute) { + const YGConfigRef config = YGConfigNew(); + + const YGNodeRef root = YGNodeNewWithConfig(config); + YGNodeStyleSetWidth(root, 100); + YGNodeStyleSetHeight(root, 100); + + const YGNodeRef root_child0 = YGNodeNewWithConfig(config); + YGNodeStyleSetPositionType(root_child0, YGPositionTypeAbsolute); + YGNodeStyleSetWidth(root_child0, 100); + YGNodeStyleSetHeight(root_child0, 100); + YGNodeStyleSetDisplay(root_child0, YGDisplayNone); + YGNodeInsertChild(root, root_child0, 0); + YGNodeCalculateLayout(root, YGUndefined, YGUndefined, YGDirectionLTR); + + ASSERT_FLOAT_EQ(0, YGNodeLayoutGetLeft(root)); + ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root)); + ASSERT_FLOAT_EQ(100, YGNodeLayoutGetWidth(root)); + ASSERT_FLOAT_EQ(100, YGNodeLayoutGetHeight(root)); + + ASSERT_FLOAT_EQ(0, YGNodeLayoutGetLeft(root_child0)); + ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root_child0)); + ASSERT_FLOAT_EQ(0, YGNodeLayoutGetWidth(root_child0)); + ASSERT_FLOAT_EQ(0, YGNodeLayoutGetHeight(root_child0)); + + YGNodeCalculateLayout(root, YGUndefined, YGUndefined, YGDirectionRTL); + + ASSERT_FLOAT_EQ(0, YGNodeLayoutGetLeft(root)); + ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root)); + ASSERT_FLOAT_EQ(100, YGNodeLayoutGetWidth(root)); + ASSERT_FLOAT_EQ(100, YGNodeLayoutGetHeight(root)); + + ASSERT_FLOAT_EQ(0, YGNodeLayoutGetLeft(root_child0)); + ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root_child0)); + ASSERT_FLOAT_EQ(0, YGNodeLayoutGetWidth(root_child0)); + ASSERT_FLOAT_EQ(0, YGNodeLayoutGetHeight(root_child0)); + + YGNodeFreeRecursive(root); + + YGConfigFree(config); +} diff --git a/yoga/Yoga.cpp b/yoga/Yoga.cpp index 2db6be21d9..f24563df61 100644 --- a/yoga/Yoga.cpp +++ b/yoga/Yoga.cpp @@ -3557,7 +3557,8 @@ static void YGNodelayoutImpl( if (performLayout) { // STEP 10: SIZING AND POSITIONING ABSOLUTE CHILDREN for (auto child : node->getChildren()) { - if (child->getStyle().positionType() != YGPositionTypeAbsolute) { + if (child->getStyle().display() == YGDisplayNone || + child->getStyle().positionType() != YGPositionTypeAbsolute) { continue; } YGNodeAbsoluteLayoutChild(