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

Size Constraints Example #7956

Merged
merged 13 commits into from
Apr 24, 2023

Conversation

ickshonpe
Copy link
Contributor

@ickshonpe ickshonpe commented Mar 7, 2023

Objective

Add a simple example demonstrating how to use size constraints.

Related to #7946

Solution

Capture

Changelog

  • Added the example size_constraints

@alice-i-cecile alice-i-cecile added C-Examples An addition or correction to our examples A-UI Graphical user interfaces, styles, layouts, and widgets labels Mar 7, 2023
@james7132 james7132 added this to the 0.10.1 milestone Mar 7, 2023
@ickshonpe
Copy link
Contributor Author

ickshonpe commented Mar 7, 2023

Just spent ten minutes trying to fix why the min_size and max_size constraints didn't work. That's because of #7946 which was the reason for making the example in the first place 😅.

Still needs a bit work on the presentation and some explanatory text, it'll wait till #7948 is merged.

@nicoburns
Copy link
Contributor

nicoburns commented Mar 23, 2023

It might be a good idea to give the node a non-zero content size to illustrate the difference between min_size: auto and min_size: 0. Then again, maybe that overcomplicates things. This does seem useful as it is.

@ickshonpe
Copy link
Contributor Author

It might be a good idea to give the node a non-zero content size to illustrate the difference between min_size: auto and min_size: 0. Then again, maybe that overcomplicates things. This does seem useful as it is.

Yeah, material for a separate example I think

@cart cart removed this from the 0.10.1 milestone Mar 27, 2023
@alice-i-cecile
Copy link
Member

The flex-basis property seems to have no effect here. If that's expected, I think we should probably just cut it from this example (and add another one to demonstrate this).

@nicoburns
Copy link
Contributor

The flex-basis property seems to have no effect here. If that's expected, I think we should probably just cut it from this example (and add another one to demonstrate this).

Flex basis would have an effect if there were two child nodes (where it could be controlled separately). Which might make for a better example? Otherwise I agree that flex-basis probably ought to be removed.

@ickshonpe
Copy link
Contributor Author

The flex-basis property seems to have no effect here. If that's expected, I think we should probably just cut it from this example (and add another one to demonstrate this).

That's odd, flex_basis is working for me. If you look at the screenshot at the top of the page, the bar is set to 75% using the flex-basis property. Does it not respond even with everything else set to Auto?

@tim-blackbird
Copy link
Contributor

flex_basis is working for me as well.

@alice-i-cecile, you could try running cargo update to make sure you're on the latest taffy patch.

@nicoburns
Copy link
Contributor

Ah, I didn't actually try it. I was just mentally simulating it. But I'd hallucinated a flex-grow: 1 in my mental model.

@alice-i-cecile
Copy link
Member

Working correctly now! I had to change only flex_basis while leaving other values to auto to see an effect :)

Copy link
Contributor

@nicoburns nicoburns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some explanatory text that explains which combinations are good to test and why some things work like they do (that's rendered in the example) would be nice but I wouldn't consider that necessary or blocking.

Also, damn bevy_ui is verbose! We badly need that sugar! But that's not an issue with this example.

@alice-i-cecile alice-i-cecile added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Apr 22, 2023
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Apr 24, 2023
Merged via the queue into bevyengine:main with commit f336093 Apr 24, 2023
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-Examples An addition or correction to our examples S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants