Replace UiRect with distinct Margin, Padding, Position and Border types

[ickshonpe]

Objective

Problems with UiRect:

• UiRect is confusingly named (nothing it represents is a rectangle).
• UiRects representing positions should have their fields set to Auto by default, the others 0.
• For padding and borders non-numeric UiRect fields are meaningless (at the moment at least, may change).
• The documentation for UiRect has to explain its different behaviour for each of the four use cases.
• New features might mean the requirements for each of these style properties diverge even further.

relevant discussion at #7656

Solution

Split UiRect into four distinct types, as it represents four very different things (padding, borders and margins might seem similar but they are actually different in sometimes very confusing ways).

Implement the UiRect constructor functions using macros.

---

Changelog

• Removed UiRect.

• Added the Margin, Padding, Position and Border types.

• Implemented the common functionality with macros.

Migration Guide

UiRect has been removed and is replaced by the Margin, Padding, Position and Border types.

[alice-i-cecile]

Very much in favor of this direction: much clearer, and the distinct defaults make it obvious that these should be split.

[ickshonpe]

Being able to split up the documentation is really nice I think.

I'm not quite sure whether it's better to use traits or macros to implement the common functions. I used a trait basically only because it's been a while since I wrote any macros and I'd have had to look up the syntax again before I started.

[alice-i-cecile]

I think the trait will probably be easier to maintain: lots of folks are more comfortable with them and they're easier to read.

No strong feelings though.

[alice-i-cecile]

Ping @nicoburns for review (you're not in the Bevy org so I can't request it directly) :)

[ickshonpe]

No strong feelings though.

No me neither. I guess it doesn't really matter, the implementation would be easy to replace later if anyone hates it.

[nicoburns]

This seems like a reasonable solution. My only concern with separate types would be having to import them all. But this way does have the advantage of having the 1:1 naming with the fields which could make importing easier. I definitely agree that UiRect doesn't make much sense as a name. IMO a Rect should be a struct with x, y, width and height fields if it exists. I was considering renaming this type Edges in Taffy (IIRC this is the name Yoga uses). Frame also seems like a reasonable choice.

For reference, in Taffy:

• The struct is generic over the field type, which solves the "non-numeric fields are meaningless" problem
• We solve the default issue by not implementing default on these types at all.
• We don't have much if any documentation on Rect.

Ultimately I don't think end users should be expected to touch these types at all. I think we should:

• Have builder methods on the top-level Style struct
• Make with_style() methods take FnOnce(&mut Style) rather than Style

so that you can do:

.with_style(|style| {
   style
      .margin_left(5)
      .margin_vertical(10)
      .margin_all(20);
})

And all the documentation can live on the Style struct and it's builder methods (which can be sensibly grouped using multiple impl blocks).

[mockersf]

I think it would be better to wait for an actual use case to split those, then do them as needed

[TimJentzsch]

I strongly agree that the current solution of UiRect should be replaced, the name is quite misleading.

I'm not yet sure if I would prefer entirely separate types like this PR introduces or an Edges/Frames struct that combines the use-cases.

For reference, in Taffy:

• The struct is generic over the field type, which solves the "non-numeric fields are meaningless" problem
• We solve the default issue by not implementing default on these types at all.
• We don't have much if any documentation on Rect.

I think not having a default implementation on the general type is a good idea if we go that route, the default can then be defined on the default of Style.

Ultimately I don't think end users should be expected to touch these types at all. I think we should:

• Have builder methods on the top-level Style struct
• Make with_style() methods take FnOnce(&mut Style) rather than Style

This is an interesting concept and I think quite attractive from the UX point of view.
However, I'm a bit concerned how big the Style class will grow if we introduce builder patterns for all style concepts.
IIRC Cart was previously hesitant about introducing too many builder patterns like this instead of the { ..default() } way to maintain consistency across Bevy.

Personally, I think I'd prefer builder patterns though.

[Weibye]

I am in favour of this change, but I do believe Position should be named Inset instead as that is the correct term for the functionality according to CSS specification: See https://developer.mozilla.org/en-US/docs/Web/CSS/inset

[nicoburns]

Having used bevy's UI a bit more, I'm now in favour of this PR. However, in main, position is now separate top-level left, right, top, and bottom properties, and I think it probably makes sense to stick with this rather than introduce a Position/Inset type. If one is introduced I agree with @Weibye that it ought to be called Inset.

There are also two other naming inconsistencies compared with CSS:

• What is position in CSS is still position_type in bevy.
• Bevy's border property is really border-width in CSS. I'm wondering if we ought to name it border_width and BorderWidth in bevy to match.

Finally, I would be really keen on the constructor methods for these types accepting bare floats and integers which are interpreted as being Val::Px.

[togetherwithasteria]

I am in favour of this change, but I do believe Position should be named Inset instead as that is the correct term for the functionality according to CSS specification: See https://developer.mozilla.org/en-US/docs/Web/CSS/inset

Personally, would prefer left right and things on Style, but I kinda feel that Inset is not a beginner-friendly name?

(I'm sorry if this is a necro post T_T)

[ickshonpe]

No no, it's not even dead really, I closed this because it doesn't make as much sense by itself, so I made a new PR #8096 that combines all the related changes to Val and UiRect.