Refactor Node fields (previously Style)

[viridia]

What problem does this solve or what need does it fill?

Bevy uses the word "Style" to represent a set of styles that can be applied to a NodeBundle. Unfortunately, the name is rather broad, which causes problems for a number of reasons:

• Not all style properties are represented in Style, so the name is misleading.
• Style only applies to NodeBundles, not other kinds of entities (such as Text) which might also need styles.
• Many UI frameworks which want to define a more comprehensive, all-encompassing "style" structure are forced to pick a different name, despite the fact that the word "Style" is a natural fit for such a struct. (Yes, you can use namespace qualification, but that leads to a different kind of ugliness).

What solution would you like?

I propose renaming "Style" to something more specific such as "NodeStyle". We can keep a type alias "Style" for backwards compatibility.

[alice-i-cecile]

I would definitely like a rename here, but this does add a bit of verbosity. This is inherited from CSS, but ultimately something like Layout would be much more clear.

IMO no type alias: users can just define their own.

[nicoburns]

Perhaps LayoutStyle? IMO Style is the properties which determine the final layout (via the layout algorithms), but is not the layout itself (which would be x, y, z, width, height for each node).

[nicoburns]

I have been thinking we could prefix a lot of the bevy_ui types with Ui (so UiNodeBundle, etc) as it's a short prefix and makes it really clear that it's UI related. That doesn't fix the issue of text nodes needing styles. But perhaps UiStyle or UiLayoutStyle could also be an option.

[ickshonpe]

Another alternative might be LayoutConstraints.

Constraints has better fitting semantics than Style I think. It might help users better understand how the layout properties are composed and applied.

[viridia]

I've edited the title of the bug to reflect the current discussion.

[JMS55]

I'm in favor of changing this to LayoutConstraints. I ran into the same confusion in bevy_dioxus.

[CooCooCaCha]

Imo we should still use the postfix Style for styles. I much prefer the suggestion above LayoutStyle or NodeStyle.

LayoutConstraints is much less clear.

1. People are familiar with the term styles and bevy uses that term elsewhere. Constraints sounds like it introduces a new concept that is separate from styles when in reality it's just a different set of styles.
2. Constraints sounds like constraint-based layout which is a completely different layout algorithm.

[viridia]

@cart 's latest BSN design doc (#14437) diverges from what has been proposed here. I have no particular preference either way, but I do want to highlight the differences:

• Cart's proposal is to move most of the properties of Style into Node. (Which properties won't get moved, or how they will be organized, is as yet undetermined.)
• The proposal given above is to simply rename Style to Layout and otherwise keep things as they are.

My understanding is that the original motivation for bundling all those properties Style into a single component is that these are all of the properties that Taffy cares about, that is, all of the properties which can potentially affect the 2d coordinates of the element, but not any other visual aspect such as color or pickability; this means that Taffy only needs to look at a single component when doing calculations. However, I suspect that splitting things up into multiple components won't be that much of a problem, it's just a Query.

Note that one important ability remains unchanged even under both proposals: the process for getting the logical rect for a UI element, which requires Node and GlobalTransform components to be queried.

In any case, we should either close or repurpose this issue if we're going to go with Cart's scheme.

[alice-i-cecile]

I've changed my mind after some time spent dogfooding this. IMO we should split Style into FlexLayout, GridLayout and AbsoluteLayout. The useless fields are a serious UX / learning hazard, and also make the component substantially larger than it needs to be.

[nicoburns]

I've changed my mind after some time spent dogfooding this. IMO we should split Style into FlexLayout, GridLayout and AbsoluteLayout. The useless fields are a serious UX / learning hazard, and also make the component substantially larger than it needs to be.

The way in which Taffy's new traitified API split's things up might make sense (separate "container" and "child" variants for each layout mode because a node might (for example) simultaneously be a flexbox child while being grid container.

There is a "CoreStyle" trait which contains things like display, width, height, aspect_ratio that apply pretty globally.

[alice-i-cecile]

Ah right, that's a great idea. It also makes things even more granular in a natural way, which I like.

[viridia]

I agree with @nicoburns approach: separate "layout props for child" from "layout props for container". But I'd add one more thing: there should be four types of containers:

• FlexContainer - arranges children using the existing Display::Flex layout
• GridContainer - arranges children using the existing Display::Grid layout
• BlockContainer - arranges children using the existing Display::Block layout
• TextFlowContainer - arranges children using a word-wrapping algorithm from cosmic-text. This would dovetail with the "spans as entities" proposal.

This means that the extra overhead of doing text flow layout would be strictly opt-in.

For children, I don't think we need to have separate "FlexChild" and "GridChild" components - the "Align" and "Justify" properties can continue to be used regardless of the type of container.

[nicoburns]

For children, I don't think we need to have separate "FlexChild" and "GridChild" components - the "Align" and "Justify" properties can continue to be used regardless of the type of container.

Align and Justify work for both, but:

• flex_basis, flex_grow, and flex_shrink are flexbox specific
• grid_row_start, grid_row_end, grid_column_start, grid_column_end are grid specific.

Whether that justifies separate components I don't know (but IMO it might be a clearer API)

[viridia]

OK, then, how about:

• Containers
  • FlexLayout
  • GridLayout
  • BlockLayout
  • TextFlowLayout (assuming you're on board with this idea)
• Children
  • FlexSize (containing flex_grow, flex_shrink, and flex_basis.)
  • GridPosition - containing grid_row_start, etc. (Alternate name: GridPlacement).
  • AlignSelf and JustifySelf can be put in Node unless you can think of a better place to put them.

[alice-i-cecile]

I think that that model is quite a bit harder to learn than Nico's original idea.

[viridia]

Well, I was just trying to impose some kind of categorization without creating too much duplication. I don't have strong opinions here.

In any case, we should either come up with a concrete proposal, or close this issue.

[alice-i-cecile]

I'm strongly in favor of Nico's suggestion.

[viridia]

I'm confused, I thought what I was proposing was compatible with that suggestion:

separate "container" and "child" variants for each layout mode

That's what my FlexLayout, GridLayout was supposed to represent: the component for containers. GridPosition and such on was intended to be the component for children.

The thing to understand here is that this can't be completely orthogonal: for example, Block layout doesn't have any special properties for children that I know of, so there's no BlockChild component.

a "CoreStyle" trait which contains things like display, width, height, aspect_ratio

Again, this is in line with what I described.

The question is what to do about child attributes that apply to more than one layout mode but not all? AlignSelf and JustifySelf affect the children of both Flex and Grid layout elements. For Block layout, I don't know how it works in Taffy, but on the web horizontal alignment for block child elements is controlled via Margin::Auto, not AlignSelf or JustifySelf.

The only other deviation is my proposal for a fourth layout type, to support the "text spans as entities" feature. My reasoning for making this a layout type is that some people might object to the additional computational overhead of running a word-wrap and text measurement computation on every UI node; so I propose that we only run that algorithm when the user explicitly asks for it, by setting the layout type.

Thus, if you have an element with a bunch of children that are Text entities, what you get depends on the layout type:

• If it's flex, each child gets a separate flex cell (same as now).
• If it's grid, each child gets a separate grid cell (same as now).
• If it's block, each child is put on it's own row (slightly different behavior from the web, but backwards compatible with Bevy and cheaper)
• If it's text flow, the children are merged into a word-wrapped paragraph (new behavior).

[viridia]

Several people have brought up the idea of splitting up Node (what used to be Style). Here's a proposal that splits things up quite a lot: https://hackmd.io/@dreamertalin/ByEzvWOZyg

@nicoburns @alice-i-cecile

[nicoburns]

Several people have brought up the idea of splitting up Node (what used to be Style). Here's a proposal that splits things up quite a lot: https://hackmd.io/@dreamertalin/ByEzvWOZyg

I'd probably move the alignment styles into the relevant Container and Item components, duplicating where necessary (these are tiny single-byte enum types). That would reduce the number of components, and make it clear which alignment styles apply to which container/item modes.

[nicoburns]

Also, NodeOffset should probably be NodeInset as that's the standard CSS name for these properties (and they are indeed insets)

[viridia]

Q: Would you also duplicate Gap? My first draft had it that way.

[nicoburns]

Maybe? I considered suggesting that, but I think it's less clear cut. What's the motivation for the change? Just ergonomics?

[viridia]

Just consistency: Since the rationale for putting align/justify inside the Flex and Grid structs is that they only apply to those display modes, the same argument holds true for gaps.

Of course, these are larger values than the enums, so that's a counter-argument.