Skip to content
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

Always resolvable Val alternative with no Auto variant #9688

Open
ickshonpe opened this issue Sep 4, 2023 · 3 comments
Open

Always resolvable Val alternative with no Auto variant #9688

ickshonpe opened this issue Sep 4, 2023 · 3 comments
Labels
A-UI Graphical user interfaces, styles, layouts, and widgets C-Usability A targeted quality-of-life change that makes Bevy easier to use

Comments

@ickshonpe
Copy link
Contributor

ickshonpe commented Sep 4, 2023

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

Val's Auto variant doesn't make sense for certain Style fields, such as the gap properties. In those cases internally Auto just gets mapped to Taffy's LengthPercentage::Points(0.)

This might not seem that significant but it's been bothering me intensely the last year. It's never ideal to have multiple representations mapping to the same underlying value like this.

What solution would you like?

My proposal is PR #8096 which I submitted a while ago. It needs an update to support the recent changes to Val, and the macro that implements some of the helper functions could be improved maybe. There wouldn't be any major changes though, so reviews won't be wasted.

Making Margin, Padding and Border into separate types might be controversial. I'm not bothered if we have a generic UiRect<T> or UiRect (with Val edge values) and NumRect (with Nums) instead. The only important change is adding the always resolvable type.

What alternative(s) have you considered?

There are some discussions at: #7656, #7969, #7710, #7569.

@ickshonpe ickshonpe added C-Feature A new feature, making something new possible S-Needs-Triage This issue needs to be labelled labels Sep 4, 2023
@alice-i-cecile alice-i-cecile added A-UI Graphical user interfaces, styles, layouts, and widgets C-Usability A targeted quality-of-life change that makes Bevy easier to use and removed C-Feature A new feature, making something new possible S-Needs-Triage This issue needs to be labelled labels Sep 4, 2023
@nicoburns
Copy link
Contributor

nicoburns commented Sep 5, 2023

Hmm... I'm a little concerned about discoverability here. Perhaps we could add the type you suggest, and then type alias Margin, Padding, Border, etc to the appropriate type?

It may also be worth giving some consideration to how we might want to handle this in a future where there are additional variants for some properties such as width/height (e.g. MinContent, MaxContent, FitContentPx, FitContentPercent, and possibly a morphorm-like Stretch or Fr unit).

@ickshonpe
Copy link
Contributor Author

Yeah I'm rethinking the separate Margin, Padding, Border thing again. The main motivation was that it would allow us to have separate docs for each type. I think probably it's not worth it, and better just to make UiRect generic. It's easy to write these things but maintaining them is another matter. I don't want to repeat the recent Val maintenance I did, spend hours fixing lots of trivial but buggy functions and then realize after they've been fixed that they are completely useless anyway.

My personal preference has always been to flatten everything. margin_left, border_thickness_left, etc fields on Style, with helper functions to set values on multiple edges.

@viridia
Copy link
Contributor

viridia commented Sep 8, 2023

Just as an aside, in the CSS documentation, this type is called a "Length". I prefer that over "Val", as it more clearly narrows the meaning (a "value" can be almost anything).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-UI Graphical user interfaces, styles, layouts, and widgets C-Usability A targeted quality-of-life change that makes Bevy easier to use
Projects
None yet
Development

No branches or pull requests

4 participants