Skip to content

Commit e7a3f47

Browse files
j-piaseckifacebook-github-bot
authored andcommitted
Add support for display: contents (#46584)
Summary: 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 facebook/yoga#1725 X-link: facebook/yoga#1726 This PR adds a few things over the corresponding one in Yoga: - new value in the `DisplayType` enum - `Contents` - new `ShadowNodeTrait` - `ForceFlattenView`, that forces the node not to form a host view - updates TS types to include `display: contents` - aliases `display: contents` to `display: none` on the old architecture ## Changelog: [GENERAL] [ADDED] - Added support for `display: contents` Pull Request resolved: #46584 Test Plan: <details> <summary>So far I've been testing on relatively simple snippets like this one and on entirety of RNTester by inserting views with `display: contents` in random places and seeing if anything breaks.</summary> ```jsx import React from 'react'; import { Button, Pressable, SafeAreaView, ScrollView, TextInput, View, Text } from 'react-native'; export default function App() { const [toggle, setToggle] = React.useState(false); return ( <View style={{flex: 1, paddingTop: 100}}> <SafeAreaView style={{width: '100%', height: 200}}> <Pressable style={{width: 100, height: 100, backgroundColor: 'black'}} onPress={() => setToggle(!toggle)}> <ScrollView /> </Pressable> <View style={{display: 'flex', flexDirection: 'row', flex: 1, backgroundColor: 'magenta'}}> <SafeAreaView style={{ // display: 'contents', flex: 1, }}> <View style={{ display: 'contents', width: '100%', height: 200, }}> <View style={{ display: 'contents', flex: 1, }}> { toggle && <View style={{flex: 1, backgroundColor: 'yellow'}} /> } <View style={{flex: 1, backgroundColor: 'blue'}} /> <View style={{flex: 1, backgroundColor: 'cyan'}} /> </View> </View> </SafeAreaView> </View> {/* <View style={{width: 100, height: 100, backgroundColor: 'magenta', display: 'flex'}} /> */} <TextInput style={{width: 200, height: 100, backgroundColor: 'red', display: 'flex'}}> <Text style={{color: 'white'}}>Hello</Text> <Text style={{color: 'green'}}>World</Text> </TextInput> </SafeAreaView> </View> ); } ``` </details> Reviewed By: joevilches Differential Revision: D64584476 Pulled By: NickGerleman fbshipit-source-id: bec77b5087ff95f0980cf02274fbb2c8581eb3c0
1 parent e5296cf commit e7a3f47

File tree

7 files changed

+26
-9
lines changed

7 files changed

+26
-9
lines changed

packages/react-native/Libraries/StyleSheet/StyleSheetTypes.d.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ export interface FlexStyle {
5656
borderWidth?: number | undefined;
5757
bottom?: DimensionValue | undefined;
5858
boxSizing?: 'border-box' | 'content-box' | undefined;
59-
display?: 'none' | 'flex' | undefined;
59+
display?: 'none' | 'flex' | 'contents' | undefined;
6060
end?: DimensionValue | undefined;
6161
flex?: number | undefined;
6262
flexBasis?: DimensionValue | undefined;

packages/react-native/Libraries/StyleSheet/StyleSheetTypes.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ type ____LayoutStyle_Internal = $ReadOnly<{
5858
* It works similarly to `display` in CSS, but only support 'flex' and 'none'.
5959
* 'flex' is the default.
6060
*/
61-
display?: 'none' | 'flex',
61+
display?: 'none' | 'flex' | 'contents',
6262

6363
/** `width` sets the width of this component.
6464
*

packages/react-native/Libraries/__tests__/__snapshots__/public-api-test.js.snap

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7927,7 +7927,7 @@ export type DimensionValue = number | string | \\"auto\\" | AnimatedNode | null;
79277927
export type AnimatableNumericValue = number | AnimatedNode;
79287928
export type CursorValue = \\"auto\\" | \\"pointer\\";
79297929
type ____LayoutStyle_Internal = $ReadOnly<{
7930-
display?: \\"none\\" | \\"flex\\",
7930+
display?: \\"none\\" | \\"flex\\" | \\"contents\\",
79317931
width?: DimensionValue,
79327932
height?: DimensionValue,
79337933
bottom?: DimensionValue,

packages/react-native/ReactCommon/react/renderer/components/view/YogaLayoutableShadowNode.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -388,6 +388,12 @@ void YogaLayoutableShadowNode::updateYogaProps() {
388388
!viewProps.filter.empty();
389389
YGNodeSetAlwaysFormsContainingBlock(&yogaNode_, alwaysFormsContainingBlock);
390390
}
391+
392+
if (yogaNode_.style().display() == yoga::Display::Contents) {
393+
ShadowNode::traits_.set(ShadowNodeTraits::ForceFlattenView);
394+
} else {
395+
ShadowNode::traits_.unset(ShadowNodeTraits::ForceFlattenView);
396+
}
391397
}
392398

393399
/*static*/ yoga::Style YogaLayoutableShadowNode::applyAliasedProps(

packages/react-native/ReactCommon/react/renderer/components/view/conversions.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -439,6 +439,10 @@ inline void fromRawValue(
439439
result = yoga::Display::None;
440440
return;
441441
}
442+
if (stringValue == "contents") {
443+
result = yoga::Display::Contents;
444+
return;
445+
}
442446
LOG(ERROR) << "Could not parse yoga::Display: " << stringValue;
443447
}
444448

packages/react-native/ReactCommon/react/renderer/core/ShadowNodeTraits.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,9 @@ class ShadowNodeTraits {
7575

7676
// Inherits `YogaLayoutableShadowNode` and has a custom baseline function.
7777
BaselineYogaNode = 1 << 10,
78+
79+
// Forces the node not to form a host view.
80+
ForceFlattenView = 1 << 11,
7881
};
7982

8083
/*

packages/react-native/ReactCommon/react/renderer/mounting/Differentiator.cpp

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -245,13 +245,17 @@ static void sliceChildShadowNodeViewPairsRecursively(
245245
// shuffling their children around.
246246
bool childrenFormStackingContexts = shadowNode.getTraits().check(
247247
ShadowNodeTraits::Trait::ChildrenFormStackingContext);
248-
bool isConcreteView =
249-
childShadowNode.getTraits().check(ShadowNodeTraits::Trait::FormsView) ||
250-
childrenFormStackingContexts;
251-
bool areChildrenFlattened =
248+
bool isConcreteView = (childShadowNode.getTraits().check(
249+
ShadowNodeTraits::Trait::FormsView) ||
250+
childrenFormStackingContexts) &&
252251
!childShadowNode.getTraits().check(
253-
ShadowNodeTraits::Trait::FormsStackingContext) &&
254-
!childrenFormStackingContexts;
252+
ShadowNodeTraits::Trait::ForceFlattenView);
253+
bool areChildrenFlattened =
254+
(!childShadowNode.getTraits().check(
255+
ShadowNodeTraits::Trait::FormsStackingContext) &&
256+
!childrenFormStackingContexts) ||
257+
childShadowNode.getTraits().check(
258+
ShadowNodeTraits::Trait::ForceFlattenView);
255259

256260
Point storedOrigin = {};
257261
if (areChildrenFlattened) {

0 commit comments

Comments
 (0)