Skip to content

Conversation

@j-piasecki
Copy link
Contributor

Summary

Adds unit tests that directly cover the order in which LayoutableChildren iterator goes over the descendant nodes. The covered cases are as follows (nodes with display: contents are marked green):

Single display: contents node

flowchart TD
R((R)) --> A((A))
R --> B((B))
R --> C((C))

B --> D((D))
B --> E((E))

style B fill:#090
Loading

Correct order: A, D, E, C

Multiple display: contents nodes

flowchart TD
R((R)) --> A((A))
R --> B((B))
R --> C((C))

A --> D((D))
A --> E((E))

B --> F((F))
B --> G((G))

C --> H((H))
C --> I((I))

style A fill:#090
style B fill:#090
style C fill:#090
Loading

Correct order: D, E, F, G, H, I

Nested display: contents nodes

flowchart TD
R((R)) --> A((A))
R --> B((B))
R --> C((C))

B --> D((D))
B --> E((E))

E --> F((F))
E --> G((G))

style B fill:#090
style E fill:#090
Loading

Correct order: A, D, F, G, C

Leaf display: contents node

flowchart TD
R((R)) --> A((A))
R --> B((B))
R --> C((C))

style B fill:#090
Loading

Correct order: A, C

Root display: contents node

flowchart TD
R((R)) --> A((A))
R --> B((B))
R --> C((C))

style R fill:#090
Loading

Correct order: A, B, C - LayoutableChildren goes over the children with display: contents property, setting it on the root node should have no effect.

@vercel
Copy link

vercel bot commented Oct 25, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
yoga-website ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 25, 2024 8:36am

@facebook-github-bot facebook-github-bot added CLA Signed Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. labels Oct 25, 2024
@facebook-github-bot
Copy link
Contributor

@NickGerleman has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

NickGerleman pushed a commit to NickGerleman/react-native that referenced this pull request Oct 25, 2024
Summary:
Adds unit tests that directly cover the order in which `LayoutableChildren` iterator goes over the descendant nodes. The covered cases are as follows (nodes with `display: contents` are marked green):

### Single `display: contents` node

```mermaid
flowchart TD
R((R)) --> A((A))
R --> B((B))
R --> C((C))

B --> D((D))
B --> E((E))

style B fill:facebook/yoga#90
```

Correct order: `A, D, E, C`

### Multiple `display: contents` nodes

```mermaid
flowchart TD
R((R)) --> A((A))
R --> B((B))
R --> C((C))

A --> D((D))
A --> E((E))

B --> F((F))
B --> G((G))

C --> H((H))
C --> I((I))

style A fill:facebook/yoga#90
style B fill:facebook/yoga#90
style C fill:facebook/yoga#90
```

Correct order: `D, E, F, G, H, I`

### Nested `display: contents` nodes

```mermaid
flowchart TD
R((R)) --> A((A))
R --> B((B))
R --> C((C))

B --> D((D))
B --> E((E))

E --> F((F))
E --> G((G))

style B fill:facebook/yoga#90
style E fill:facebook/yoga#90
```

Correct order: `A, D, F, G, C`

### Leaf `display: contents` node

```mermaid
flowchart TD
R((R)) --> A((A))
R --> B((B))
R --> C((C))

style B fill:facebook/yoga#90
```

Correct order: `A, C`

### Root `display: contents` node

```mermaid
flowchart TD
R((R)) --> A((A))
R --> B((B))
R --> C((C))

style R fill:facebook/yoga#90
```

Correct order: `A, B, C` - `LayoutableChildren` goes over the children with `display: contents` property, setting it on the root node should have no effect.

X-link: facebook/yoga#1731

Differential Revision: D64981779

Pulled By: NickGerleman
@facebook-github-bot
Copy link
Contributor

@NickGerleman merged this pull request in 6fed4df.

facebook-github-bot pushed a commit to facebook/litho that referenced this pull request Oct 26, 2024
Summary:
Adds unit tests that directly cover the order in which `LayoutableChildren` iterator goes over the descendant nodes. The covered cases are as follows (nodes with `display: contents` are marked green):

### Single `display: contents` node

```mermaid
flowchart TD
R((R)) --> A((A))
R --> B((B))
R --> C((C))

B --> D((D))
B --> E((E))

style B fill:facebook/yoga#90
```

Correct order: `A, D, E, C`

### Multiple `display: contents` nodes

```mermaid
flowchart TD
R((R)) --> A((A))
R --> B((B))
R --> C((C))

A --> D((D))
A --> E((E))

B --> F((F))
B --> G((G))

C --> H((H))
C --> I((I))

style A fill:facebook/yoga#90
style B fill:facebook/yoga#90
style C fill:facebook/yoga#90
```

Correct order: `D, E, F, G, H, I`

### Nested `display: contents` nodes

```mermaid
flowchart TD
R((R)) --> A((A))
R --> B((B))
R --> C((C))

B --> D((D))
B --> E((E))

E --> F((F))
E --> G((G))

style B fill:facebook/yoga#90
style E fill:facebook/yoga#90
```

Correct order: `A, D, F, G, C`

### Leaf `display: contents` node

```mermaid
flowchart TD
R((R)) --> A((A))
R --> B((B))
R --> C((C))

style B fill:facebook/yoga#90
```

Correct order: `A, C`

### Root `display: contents` node

```mermaid
flowchart TD
R((R)) --> A((A))
R --> B((B))
R --> C((C))

style R fill:facebook/yoga#90
```

Correct order: `A, B, C` - `LayoutableChildren` goes over the children with `display: contents` property, setting it on the root node should have no effect.

Changelog: [Internal]

X-link: facebook/yoga#1731

Reviewed By: joevilches

Differential Revision: D64981779

Pulled By: NickGerleman

fbshipit-source-id: ee39759c663a40f96ad313f1b775d53ab68fb442
facebook-github-bot pushed a commit to facebook/react-native that referenced this pull request Oct 26, 2024
Summary:
Adds unit tests that directly cover the order in which `LayoutableChildren` iterator goes over the descendant nodes. The covered cases are as follows (nodes with `display: contents` are marked green):

### Single `display: contents` node

```mermaid
flowchart TD
R((R)) --> A((A))
R --> B((B))
R --> C((C))

B --> D((D))
B --> E((E))

style B fill:facebook/yoga#90
```

Correct order: `A, D, E, C`

### Multiple `display: contents` nodes

```mermaid
flowchart TD
R((R)) --> A((A))
R --> B((B))
R --> C((C))

A --> D((D))
A --> E((E))

B --> F((F))
B --> G((G))

C --> H((H))
C --> I((I))

style A fill:facebook/yoga#90
style B fill:facebook/yoga#90
style C fill:facebook/yoga#90
```

Correct order: `D, E, F, G, H, I`

### Nested `display: contents` nodes

```mermaid
flowchart TD
R((R)) --> A((A))
R --> B((B))
R --> C((C))

B --> D((D))
B --> E((E))

E --> F((F))
E --> G((G))

style B fill:facebook/yoga#90
style E fill:facebook/yoga#90
```

Correct order: `A, D, F, G, C`

### Leaf `display: contents` node

```mermaid
flowchart TD
R((R)) --> A((A))
R --> B((B))
R --> C((C))

style B fill:facebook/yoga#90
```

Correct order: `A, C`

### Root `display: contents` node

```mermaid
flowchart TD
R((R)) --> A((A))
R --> B((B))
R --> C((C))

style R fill:facebook/yoga#90
```

Correct order: `A, B, C` - `LayoutableChildren` goes over the children with `display: contents` property, setting it on the root node should have no effect.

Changelog: [Internal]

X-link: facebook/yoga#1731

Reviewed By: joevilches

Differential Revision: D64981779

Pulled By: NickGerleman

fbshipit-source-id: ee39759c663a40f96ad313f1b775d53ab68fb442
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed Merged Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants