From e7a3f479fe37a5d503770bafc03c80b1c2dcb8f7 Mon Sep 17 00:00:00 2001 From: Jakub Piasecki Date: Fri, 18 Oct 2024 22:05:41 -0700 Subject: [PATCH] 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
``` `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:https://github.com/facebook/react-native/issues/050 style C fill:https://github.com/facebook/react-native/issues/050 style D fill:https://github.com/facebook/react-native/issues/050 style H fill:https://github.com/facebook/react-native/issues/050 style I fill:https://github.com/facebook/react-native/issues/050 ``` 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 https://github.com/facebook/yoga/pull/1725 X-link: https://github.com/facebook/yoga/pull/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: https://github.com/facebook/react-native/pull/46584 Test Plan:
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. ```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 ( setToggle(!toggle)}> { toggle && } {/* */} Hello World ); } ```
Reviewed By: joevilches Differential Revision: D64584476 Pulled By: NickGerleman fbshipit-source-id: bec77b5087ff95f0980cf02274fbb2c8581eb3c0 --- .../Libraries/StyleSheet/StyleSheetTypes.d.ts | 2 +- .../Libraries/StyleSheet/StyleSheetTypes.js | 2 +- .../__snapshots__/public-api-test.js.snap | 2 +- .../components/view/YogaLayoutableShadowNode.cpp | 6 ++++++ .../react/renderer/components/view/conversions.h | 4 ++++ .../react/renderer/core/ShadowNodeTraits.h | 3 +++ .../react/renderer/mounting/Differentiator.cpp | 16 ++++++++++------ 7 files changed, 26 insertions(+), 9 deletions(-) diff --git a/packages/react-native/Libraries/StyleSheet/StyleSheetTypes.d.ts b/packages/react-native/Libraries/StyleSheet/StyleSheetTypes.d.ts index 88f80f23a11ff8..19ea741ae11364 100644 --- a/packages/react-native/Libraries/StyleSheet/StyleSheetTypes.d.ts +++ b/packages/react-native/Libraries/StyleSheet/StyleSheetTypes.d.ts @@ -56,7 +56,7 @@ export interface FlexStyle { borderWidth?: number | undefined; bottom?: DimensionValue | undefined; boxSizing?: 'border-box' | 'content-box' | undefined; - display?: 'none' | 'flex' | undefined; + display?: 'none' | 'flex' | 'contents' | undefined; end?: DimensionValue | undefined; flex?: number | undefined; flexBasis?: DimensionValue | undefined; diff --git a/packages/react-native/Libraries/StyleSheet/StyleSheetTypes.js b/packages/react-native/Libraries/StyleSheet/StyleSheetTypes.js index d6902bc8bcd3d2..ebc1dfa98b09c7 100644 --- a/packages/react-native/Libraries/StyleSheet/StyleSheetTypes.js +++ b/packages/react-native/Libraries/StyleSheet/StyleSheetTypes.js @@ -58,7 +58,7 @@ type ____LayoutStyle_Internal = $ReadOnly<{ * It works similarly to `display` in CSS, but only support 'flex' and 'none'. * 'flex' is the default. */ - display?: 'none' | 'flex', + display?: 'none' | 'flex' | 'contents', /** `width` sets the width of this component. * diff --git a/packages/react-native/Libraries/__tests__/__snapshots__/public-api-test.js.snap b/packages/react-native/Libraries/__tests__/__snapshots__/public-api-test.js.snap index 9ecb0df2141aeb..13ddb0a6a944bd 100644 --- a/packages/react-native/Libraries/__tests__/__snapshots__/public-api-test.js.snap +++ b/packages/react-native/Libraries/__tests__/__snapshots__/public-api-test.js.snap @@ -7927,7 +7927,7 @@ export type DimensionValue = number | string | \\"auto\\" | AnimatedNode | null; export type AnimatableNumericValue = number | AnimatedNode; export type CursorValue = \\"auto\\" | \\"pointer\\"; type ____LayoutStyle_Internal = $ReadOnly<{ - display?: \\"none\\" | \\"flex\\", + display?: \\"none\\" | \\"flex\\" | \\"contents\\", width?: DimensionValue, height?: DimensionValue, bottom?: DimensionValue, diff --git a/packages/react-native/ReactCommon/react/renderer/components/view/YogaLayoutableShadowNode.cpp b/packages/react-native/ReactCommon/react/renderer/components/view/YogaLayoutableShadowNode.cpp index c57292dfbc54a5..6294af2df33875 100644 --- a/packages/react-native/ReactCommon/react/renderer/components/view/YogaLayoutableShadowNode.cpp +++ b/packages/react-native/ReactCommon/react/renderer/components/view/YogaLayoutableShadowNode.cpp @@ -388,6 +388,12 @@ void YogaLayoutableShadowNode::updateYogaProps() { !viewProps.filter.empty(); YGNodeSetAlwaysFormsContainingBlock(&yogaNode_, alwaysFormsContainingBlock); } + + if (yogaNode_.style().display() == yoga::Display::Contents) { + ShadowNode::traits_.set(ShadowNodeTraits::ForceFlattenView); + } else { + ShadowNode::traits_.unset(ShadowNodeTraits::ForceFlattenView); + } } /*static*/ yoga::Style YogaLayoutableShadowNode::applyAliasedProps( diff --git a/packages/react-native/ReactCommon/react/renderer/components/view/conversions.h b/packages/react-native/ReactCommon/react/renderer/components/view/conversions.h index 826666b5f7fa8a..58e04bba088aae 100644 --- a/packages/react-native/ReactCommon/react/renderer/components/view/conversions.h +++ b/packages/react-native/ReactCommon/react/renderer/components/view/conversions.h @@ -439,6 +439,10 @@ inline void fromRawValue( result = yoga::Display::None; return; } + if (stringValue == "contents") { + result = yoga::Display::Contents; + return; + } LOG(ERROR) << "Could not parse yoga::Display: " << stringValue; } diff --git a/packages/react-native/ReactCommon/react/renderer/core/ShadowNodeTraits.h b/packages/react-native/ReactCommon/react/renderer/core/ShadowNodeTraits.h index 926a97cae6012c..841a3d7e3a7fe5 100644 --- a/packages/react-native/ReactCommon/react/renderer/core/ShadowNodeTraits.h +++ b/packages/react-native/ReactCommon/react/renderer/core/ShadowNodeTraits.h @@ -75,6 +75,9 @@ class ShadowNodeTraits { // Inherits `YogaLayoutableShadowNode` and has a custom baseline function. BaselineYogaNode = 1 << 10, + + // Forces the node not to form a host view. + ForceFlattenView = 1 << 11, }; /* diff --git a/packages/react-native/ReactCommon/react/renderer/mounting/Differentiator.cpp b/packages/react-native/ReactCommon/react/renderer/mounting/Differentiator.cpp index caa55984462579..fc14dfdc540ce3 100644 --- a/packages/react-native/ReactCommon/react/renderer/mounting/Differentiator.cpp +++ b/packages/react-native/ReactCommon/react/renderer/mounting/Differentiator.cpp @@ -245,13 +245,17 @@ static void sliceChildShadowNodeViewPairsRecursively( // shuffling their children around. bool childrenFormStackingContexts = shadowNode.getTraits().check( ShadowNodeTraits::Trait::ChildrenFormStackingContext); - bool isConcreteView = - childShadowNode.getTraits().check(ShadowNodeTraits::Trait::FormsView) || - childrenFormStackingContexts; - bool areChildrenFlattened = + bool isConcreteView = (childShadowNode.getTraits().check( + ShadowNodeTraits::Trait::FormsView) || + childrenFormStackingContexts) && !childShadowNode.getTraits().check( - ShadowNodeTraits::Trait::FormsStackingContext) && - !childrenFormStackingContexts; + ShadowNodeTraits::Trait::ForceFlattenView); + bool areChildrenFlattened = + (!childShadowNode.getTraits().check( + ShadowNodeTraits::Trait::FormsStackingContext) && + !childrenFormStackingContexts) || + childShadowNode.getTraits().check( + ShadowNodeTraits::Trait::ForceFlattenView); Point storedOrigin = {}; if (areChildrenFlattened) {