Skip to content

Commit 68bb234

Browse files
j-piaseckifacebook-github-bot
authored andcommitted
Add support for display: contents style (#1726)
Summary: X-link: facebook/react-native#47035 This PR adds support for `display: contents` style by effectively skipping nodes with `display: contents` set during layout. This required changes in the logic related to children traversal - before this PR a node would be always laid out in the context of its direct parent. After this PR that assumption is no longer true - `display: contents` allows nodes to be skipped, i.e.: ```html <div id="node1"> <div id="node2" style="display: contents;"> <div id="node3" /> </div> </div> ``` `node3` will be laid out as if it were a child of `node1`. Because of this, iterating over direct children of a node is no longer correct to achieve the correct layout. This PR introduces `LayoutableChildren::Iterator` which can traverse the subtree of a given node in a way that nodes with `display: contents` are replaced with their concrete children. A tree like this: ```mermaid flowchart TD A((A)) B((B)) C((C)) D((D)) E((E)) F((F)) G((G)) H((H)) I((I)) J((J)) A --> B A --> C B --> D B --> E C --> F D --> G F --> H G --> I H --> J style B fill:#50 style C fill:#50 style D fill:#50 style H fill:#50 style I fill:#50 ``` would be laid out as if the green nodes (ones with `display: contents`) did not exist. It also changes the logic where children were accessed by index to use the iterator instead as random access would be non-trivial to implement and it's not really necessary - the iteration was always sequential and indices were only used as boundaries. There's one place where knowledge of layoutable children is required to calculate the gap. An optimization for this is for a node to keep a counter of how many `display: contents` nodes are its children. If there are none, a short path of just returning the size of the children vector can be taken, otherwise it needs to iterate over layoutable children and count them, since the structure may be complex. One more major change this PR introduces is `cleanupContentsNodesRecursively`. Since nodes with `display: contents` would be entirely skipped during the layout pass, they would keep previous metrics, would be kept as dirty, and, in the case of nested `contents` nodes, would not be cloned, breaking `doesOwn` relation. All of this is handled in the new method which clones `contents` nodes recursively, sets empty layout, and marks them as clean and having a new layout so that it can be used on the React Native side. Relies on #1725 Changelog: [Internal] Pull Request resolved: #1726 Test Plan: Added tests for `display: contents` based on existing tests for `display: none` and ensured that all the tests were passing. Reviewed By: joevilches Differential Revision: D64404340 Pulled By: NickGerleman fbshipit-source-id: f6f6e9a6fad82873f18c8a0ead58aad897df5d09
1 parent 5687182 commit 68bb234

26 files changed

+2590
-44
lines changed

enums.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@
3131
"SpaceEvenly",
3232
],
3333
"PositionType": ["Static", "Relative", "Absolute"],
34-
"Display": ["Flex", "None"],
34+
"Display": ["Flex", "None", "Contents"],
3535
"Wrap": ["NoWrap", "Wrap", "WrapReverse"],
3636
"BoxSizing": ["BorderBox", "ContentBox"],
3737
"MeasureMode": ["Undefined", "Exactly", "AtMost"],

gentest/fixtures/YGDisplayTest.html

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,3 +29,61 @@
2929
<div id="display_none_with_position_absolute" style="width: 100px; height: 100px;">
3030
<div style="display:none; position: absolute; width: 100px; height: 100px"></div>
3131
</div>
32+
33+
<div id="display_contents" style="width: 100px; height: 100px; flex-direction: row;">
34+
<div style="display: contents;">
35+
<div style="flex: 1; height: 10px;"></div>
36+
<div style="flex: 1; height: 20px;"></div>
37+
</div>
38+
</div>
39+
40+
<div id="display_contents_fixed_size" style="width: 100px; height: 100px; flex-direction: row;">
41+
<div style="display: contents; width: 50px; height: 50px;">
42+
<div style="flex: 1; height: 10px;"></div>
43+
<div style="flex: 1; height: 20px;"></div>
44+
</div>
45+
</div>
46+
47+
<div id="display_contents_with_margin" style="width: 100px; height: 100px; flex-direction: row;">
48+
<div style="width: 20px; height: 20px; display: contents; margin: 10px;"></div>
49+
<div style="flex-grow: 1;"></div>
50+
</div>
51+
52+
<div id="display_contents_with_padding" style="width: 100px; height: 100px; flex-direction: row;">
53+
<div style="display: contents; padding: 10px;">
54+
<div style="flex: 1; height: 10px;"></div>
55+
<div style="flex: 1; height: 20px;"></div>
56+
</div>
57+
</div>
58+
59+
<div id="display_contents_with_position" style="width: 100px; height: 100px; flex-direction: row;">
60+
<div style="display: contents; top: 10px;">
61+
<div style="flex: 1; height: 10px;"></div>
62+
<div style="flex: 1; height: 20px;"></div>
63+
</div>
64+
</div>
65+
66+
<div id="display_contents_with_position_absolute" style="width: 100px; height: 100px; flex-direction: row;">
67+
<div style="display: contents; position: absolute; width: 50px; height: 50px;">
68+
<div style="flex: 1; height: 10px;"></div>
69+
<div style="flex: 1; height: 20px;"></div>
70+
</div>
71+
</div>
72+
73+
<div id="display_contents_nested" style="width: 100px; height: 100px; flex-direction: row;">
74+
<div style="display: contents;">
75+
<div style="display: contents;">
76+
<div style="flex: 1; height: 10px;"></div>
77+
<div style="flex: 1; height: 20px;"></div>
78+
</div>
79+
</div>
80+
</div>
81+
82+
<div id="display_contents_with_siblings" style="width: 100px; height: 100px; flex-direction: row;">
83+
<div style="flex: 1; height: 30px;"></div>
84+
<div style="display: contents;">
85+
<div style="flex: 1; height: 10px;"></div>
86+
<div style="flex: 1; height: 20px;"></div>
87+
</div>
88+
<div style="flex: 1; height: 30px;"></div>
89+
</div>

gentest/gentest-cpp.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,7 @@ CPPEmitter.prototype = Object.create(Emitter.prototype, {
160160

161161
YGDisplayFlex: {value: 'YGDisplayFlex'},
162162
YGDisplayNone: {value: 'YGDisplayNone'},
163+
YGDisplayContents: {value: 'YGDisplayContents'},
163164
YGAuto: {value: 'YGAuto'},
164165

165166
YGNodeCalculateLayout: {

gentest/gentest-java.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,7 @@ JavaEmitter.prototype = Object.create(Emitter.prototype, {
195195

196196
YGDisplayFlex: {value: 'YogaDisplay.FLEX'},
197197
YGDisplayNone: {value: 'YogaDisplay.NONE'},
198+
YGDisplayContents: {value: 'YogaDisplay.CONTENTS'},
198199
YGAuto: {value: 'YogaConstants.AUTO'},
199200

200201
YGWrapNoWrap: {value: 'YogaWrap.NO_WRAP'},

gentest/gentest-javascript.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,7 @@ JavascriptEmitter.prototype = Object.create(Emitter.prototype, {
171171

172172
YGDisplayFlex: {value: 'Display.Flex'},
173173
YGDisplayNone: {value: 'Display.None'},
174+
YGDisplayContents: {value: 'Display.Contents'},
174175

175176
YGBoxSizingBorderBox: {value: 'BoxSizing.BorderBox'},
176177
YGBoxSizingContentBox: {value: 'BoxSizing.ContentBox'},

gentest/gentest.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -665,6 +665,8 @@ function displayValue(e, value) {
665665
return e.YGDisplayFlex;
666666
case 'none':
667667
return e.YGDisplayNone;
668+
case 'contents':
669+
return e.YGDisplayContents;
668670
}
669671
}
670672

java/com/facebook/yoga/YogaDisplay.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,8 @@
1111

1212
public enum YogaDisplay {
1313
FLEX(0),
14-
NONE(1);
14+
NONE(1),
15+
CONTENTS(2);
1516

1617
private final int mIntValue;
1718

@@ -27,6 +28,7 @@ public static YogaDisplay fromInt(int value) {
2728
switch (value) {
2829
case 0: return FLEX;
2930
case 1: return NONE;
31+
case 2: return CONTENTS;
3032
default: throw new IllegalArgumentException("Unknown enum value: " + value);
3133
}
3234
}
Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
/*
2+
* Copyright (c) Meta Platforms, Inc. and affiliates.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*
7+
* @generated SignedSource<<be0d102e74e15f15050520f21afcaee1>>
8+
* generated by gentest/gentest-driver.ts from gentest/fixtures/YGDisplayContentsTest.html
9+
*/
10+
11+
package com.facebook.yoga;
12+
13+
import static org.junit.Assert.assertEquals;
14+
15+
import org.junit.Ignore;
16+
import org.junit.Test;
17+
import org.junit.runner.RunWith;
18+
import org.junit.runners.Parameterized;
19+
import com.facebook.yoga.utils.TestUtils;
20+
21+
@RunWith(Parameterized.class)
22+
public class YGDisplayContentsTest {
23+
@Parameterized.Parameters(name = "{0}")
24+
public static Iterable<TestParametrization.NodeFactory> nodeFactories() {
25+
return TestParametrization.nodeFactories();
26+
}
27+
28+
@Parameterized.Parameter public TestParametrization.NodeFactory mNodeFactory;
29+
30+
@Test
31+
public void test_test1() {
32+
YogaConfig config = YogaConfigFactory.create();
33+
34+
final YogaNode root = createNode(config);
35+
root.setFlexDirection(YogaFlexDirection.ROW);
36+
root.setPositionType(YogaPositionType.ABSOLUTE);
37+
root.setWidth(100f);
38+
root.setHeight(100f);
39+
40+
final YogaNode root_child0 = createNode(config);
41+
root_child0.setDisplay(YogaDisplay.CONTENTS);
42+
root.addChildAt(root_child0, 0);
43+
44+
final YogaNode root_child0_child0 = createNode(config);
45+
root_child0_child0.setFlexGrow(1f);
46+
root_child0_child0.setFlexShrink(1f);
47+
root_child0_child0.setFlexBasisPercent(0f);
48+
root_child0_child0.setHeight(10f);
49+
root_child0.addChildAt(root_child0_child0, 0);
50+
51+
final YogaNode root_child0_child1 = createNode(config);
52+
root_child0_child1.setFlexGrow(1f);
53+
root_child0_child1.setFlexShrink(1f);
54+
root_child0_child1.setFlexBasisPercent(0f);
55+
root_child0_child1.setHeight(20f);
56+
root_child0.addChildAt(root_child0_child1, 1);
57+
root.setDirection(YogaDirection.LTR);
58+
root.calculateLayout(YogaConstants.UNDEFINED, YogaConstants.UNDEFINED);
59+
60+
assertEquals(0f, root.getLayoutX(), 0.0f);
61+
assertEquals(0f, root.getLayoutY(), 0.0f);
62+
assertEquals(100f, root.getLayoutWidth(), 0.0f);
63+
assertEquals(100f, root.getLayoutHeight(), 0.0f);
64+
65+
assertEquals(0f, root_child0.getLayoutX(), 0.0f);
66+
assertEquals(0f, root_child0.getLayoutY(), 0.0f);
67+
assertEquals(0f, root_child0.getLayoutWidth(), 0.0f);
68+
assertEquals(0f, root_child0.getLayoutHeight(), 0.0f);
69+
70+
assertEquals(0f, root_child0_child0.getLayoutX(), 0.0f);
71+
assertEquals(0f, root_child0_child0.getLayoutY(), 0.0f);
72+
assertEquals(50f, root_child0_child0.getLayoutWidth(), 0.0f);
73+
assertEquals(10f, root_child0_child0.getLayoutHeight(), 0.0f);
74+
75+
assertEquals(50f, root_child0_child1.getLayoutX(), 0.0f);
76+
assertEquals(0f, root_child0_child1.getLayoutY(), 0.0f);
77+
assertEquals(50f, root_child0_child1.getLayoutWidth(), 0.0f);
78+
assertEquals(20f, root_child0_child1.getLayoutHeight(), 0.0f);
79+
80+
root.setDirection(YogaDirection.RTL);
81+
root.calculateLayout(YogaConstants.UNDEFINED, YogaConstants.UNDEFINED);
82+
83+
assertEquals(0f, root.getLayoutX(), 0.0f);
84+
assertEquals(0f, root.getLayoutY(), 0.0f);
85+
assertEquals(100f, root.getLayoutWidth(), 0.0f);
86+
assertEquals(100f, root.getLayoutHeight(), 0.0f);
87+
88+
assertEquals(0f, root_child0.getLayoutX(), 0.0f);
89+
assertEquals(0f, root_child0.getLayoutY(), 0.0f);
90+
assertEquals(0f, root_child0.getLayoutWidth(), 0.0f);
91+
assertEquals(0f, root_child0.getLayoutHeight(), 0.0f);
92+
93+
assertEquals(50f, root_child0_child0.getLayoutX(), 0.0f);
94+
assertEquals(0f, root_child0_child0.getLayoutY(), 0.0f);
95+
assertEquals(50f, root_child0_child0.getLayoutWidth(), 0.0f);
96+
assertEquals(10f, root_child0_child0.getLayoutHeight(), 0.0f);
97+
98+
assertEquals(0f, root_child0_child1.getLayoutX(), 0.0f);
99+
assertEquals(0f, root_child0_child1.getLayoutY(), 0.0f);
100+
assertEquals(50f, root_child0_child1.getLayoutWidth(), 0.0f);
101+
assertEquals(20f, root_child0_child1.getLayoutHeight(), 0.0f);
102+
}
103+
104+
private YogaNode createNode(YogaConfig config) {
105+
return mNodeFactory.create(config);
106+
}
107+
}

0 commit comments

Comments
 (0)