-
-
Notifications
You must be signed in to change notification settings - Fork 3.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add an enum with only the numeric variants of Val
#7569
Conversation
Added a type `Breadth`, that is similar to `Val` but with only evaluatable variants. Gave UiRect a type parameter, so it can take `Breadth` or `Val` values. Added tests for `Breadth` and `UiRect`. Changed style properties to use `Breadth` instead of `Val` for the padding and border properties. Changed `bevy_ui::flex::convert::from_rect` to take an `UiRect<T: Into<Val>>` instead of a `UiRect`. Made minimal necessary changes to the examples.
…padding and border values in `bevy_ui::flex::convert::from_style`
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I definitely like this direction. Unusable values should be unrepresentable. Great implementations with nice error types!
Labelling this with controversial, since this is a major use-facing UX change.
This seems like a reasonable change to introduce (although IMO it does add friction when authoring styles as you have to remember which properties correspond to which types). I'm not keen on the name though. Perhaps |
I think if we went that way, using And then we could implement |
I like this better than |
Problem
The
Style
propertiespadding
andborder
can be assigned values ofVal::Auto
orVal::Undefined
when only the numeric variants make sense. There isn't a sensible way for the UI to determine these values automatically, except to just set them to zero.Solution
Breadth
with only the evaluatable variants ofVal
.UiRect
generic.Style
'spadding
andborder
fields toUiRect<Breadth>
.I'm not sure I like the name
Breadth
, but it's shorter thanThickness
and seems more descriptive thanLength
.Another option would be to rename
Val
toAutoVal
and renameBreadth
toVal
.AutoVal
would be used a lot more though, so this might be annoying.Perhaps
NumVal
orEVal
forE
valuatableVal
ue.Num
would be nice but ambiguous with a lot of other things.There might be objections to making
UiRect
generic. Could be divided intoNumRect
andAutoRect
or something instead. Don't care either way, or about any of the namings.Changelog
Breadth
, that is similar toVal
but withoutVal
's non-numeric variants.UiRect
into a generic struct.padding
andborder
fields ofStyle
toUiRect<Breadth>
.Migration Guide
UiRect
is now a generic struct with a type parameter, which isVal
by default.A new enum
Breadth
has been added which is similar toVal
but withoutVal
's non-numeric variantsVal::Auto
andVal::Undefined
.The
padding
andborder
fields ofStyle
have been changed to typeUiRect<Breadth>
. Anypadding
andborder
values ofVal::Auto
orVal::Undefined
should be replaced withBreadth::Px(0.)
, which is the default value forBreadth
.