Seperate ui::Node style fields into seperate required components

[mendelsshop]

Objective

Fixes #16580

Solution

Extract out all style fields from the ui::Node struct to separate (required on Node) components.
Possibly group similar style components (like width and height) into one component as opposed to individual components.
The only place these fields are used are in the conversion to taffy styles, so I created a struct to represent a query (with QueryData) of all the style components and pass that around.

Testing

This PR doesn't add anything new, so I am relying on the current tests for ui::Node.
I have tested it with cargo run -p ci.
Unless cargo run -p ci runs it on different architecture/platforms, I have run it on Linux x86_64.

---

Showcase

This section is optional. If this PR does not include a visual change or does not add a new feature, you can delete this section.

• Help others understand the result of this PR by showcasing your awesome work!
• If the Migration Guide adequately covers the changes, you can delete this section

While a showcase should aim to be brief and digestible, you can use a toggleable section to save space on longer showcases:

Click to view showcase

println!("My super cool code.");

[github-actions]

Welcome, new contributor!

Please make sure you've read our contributing guide and we look forward to reviewing your pull request shortly ✨

[mendelsshop]

As a proof of concept, I have only done it for aspect_ratio so far. Just want to get some feedback if this is the right approach.

[ickshonpe]

There have been a couple of attempts to do this before but it never works out, look at #5511 and #5513 to see the problems.

The only change that I feel has a real practical benefit would be to refactor Node into to two separate and exclusive components, one for flex layout, and one for grid layout.

[mendelsshop]

The only change that I feel has a real practical benefit would be to refactor Node into to two separate and exclusive components, one for flex layout, and one for grid layout.

Wouldn't you also want a bare Node like component for when its display: None? Just trying to imagine how to do this.

[ickshonpe]

I think under this scheme either the display field would become boolean on both the flex and grid components, with false removing the node and its descendants from the layout. Or it would use an optional marker component 'RemoveFromLayout' (or whatever) that denotes the same thing by its presence.

[mendelsshop]

So something like this(?):

#[derive(Component, Clone, PartialEq, Debug, Reflect)]
pub struct Block {
    pub display: bool,
    pub justify_items: JustifyItems,
    pub justify_self: JustifySelf,
}
#[derive(Component, Clone, PartialEq, Debug, Reflect)]
pub struct Flex {
    pub display: bool,
    pub flex_direction: FlexDirection,
    pub flex_wrap: FlexWrap,
    pub flex_grow: f32,
    pub flex_shrink: f32,
    pub flex_basis: Val,
}
#[derive(Component, Clone, PartialEq, Debug, Reflect)]
pub struct Grid {
    pub display: bool,
    pub justify_items: JustifyItems,
    pub justify_self: JustifySelf,
    pub grid_auto_flow: GridAutoFlow,
    pub grid_template_rows: Vec<RepeatedGridTrack>,
    pub grid_template_columns: Vec<RepeatedGridTrack>,
    pub grid_auto_rows: Vec<GridTrack>,
    pub grid_auto_columns: Vec<GridTrack>,
    pub grid_row: GridPlacement,
    pub grid_column: GridPlacement,
}

And then keep the rest of the common fields in the Node struct. (I put some of the justify fields in Grid and Block, because they are ignored in Flex).

Also, does mutually exclusive mean an enum or something bevy specific?

[alice-i-cecile]

IIRC @viridia or @ickshonpe or @nicoburns had a really nice idea to split this by FlexParent/FlexChild/GridParent/GridChild, to avoid wasted data while enforcing valid state. I'm still partial to that design.

[viridia]

IIRC @viridia or @ickshonpe or @nicoburns had a really nice idea to split this by FlexParent/FlexChild/GridParent/GridChild, to avoid wasted data while enforcing valid state. I'm still partial to that design.

I was thinking:

• Node
• FlexContainer
• FlexItem
• GridContainer
• GridItem

As far as display goes: meh. Whether we leave it alone, or turn it into something implicit, I don't have a strong opinion. I'd say "whichever performs better".

[ickshonpe]

As far as display goes: meh. Whether we leave it alone, or turn it into something implicit, I don't have a strong opinion. I'd say "whichever performs better".

Performance-wise it's not going to be significant either way.

But it occured to while looking at the FPS overlay PR, that since a lot of users are confused about whether they should use Display::None or Visibility, maybe the two should be combined. The value of the Visibility component is meaningless if Display::None is also set. I think an interface like this makes more sense:

pub enum UiVisibility {
    /// An entity with `UiVisibility::Inherited` will inherit the UiVisibility of its [`ChildOf`] target.
    ///
    /// A root-level entity that is set to `Inherited` will be visible.
    #[default]
    Inherited,
    /// An entity with `UiVisibility::Hidden` will be unconditionally hidden.
    Hidden,
    /// An entity with `UiVisibility::Visible` will be unconditionally visible.
    ///
    /// Note that an entity with `UiVisibility::Visible` will be visible regardless of whether the
    /// [`ChildOf`] target entity is hidden.
    Visible,
    /// The entity and it's decendents are removed from the UI layout and do not occupy any space.
    Removed,
}

It does mean even further divergence from the rendering APIs, though, which isn't great.

[mendelsshop]

I was thinking:

* `Node`

* `FlexContainer`

* `FlexItem`

* `GridContainer`

* `GridItem`

I assume these would not be mutually exclusive, because it is possible to have a grid item also contain another grid or the like.

[mendelsshop]

I just pushed the initial splitting of Node, I also added BlockItem and BlockContainer.
Figuring out which properties go where was a bit complicated, it seems the CSS spec was arguing with itself in some places, I think I have it right.
Now I just need to update all uses of the fields that used to rely on them being in Node.

[nicoburns]

Checkout Taffy's *Style traits for a guide on what goes where https://docs.rs/taffy/latest/taffy/style/index.html#traits

[mendelsshop]

The only difference between what I have so far and what taffy does is that for blocks, I added more align/justify properties from the table in https://www.w3.org/TR/2025/WD-css-align-3-20250311/#overview.

[nicoburns]

The only difference between what I have so far and what taffy does is that for blocks, I added more align/justify properties from the table in https://www.w3.org/TR/2025/WD-css-align-3-20250311/#overview.

That makes sense, although Taffy doesn't implement those properties for Blocks yet (they were only very recently added to the standard - I don't think Firefox implements them all yet either).

[mendelsshop]

I have the conversion to taffy style done, the only annoyance was that for some of the properties (especially the justify/align ones), could be duplicated across different containers or items.
And the way I am doing the conversion is that I made all the items and containers optional components as part of the Node query (557dbd8#diff-5c400b2e41f9544b35a0aecff4885259e19f56cfe5ad96174db742d8e5128a44R81-R89).
So they are not exclusive, and it is possible for the justify_content to be from both flex and grid, in which case which do you pick (right now it just hard coded to go block otherwise grid otherwise flex).

One way to fix this would be to make the container mutually exclusive and the item mutually exclusive (each node can "have" only one container and one item), but I don't know the best way to do this.