-
-
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
Reducing nesting in UI examples #3852
Conversation
examples/ui/button.rs
Outdated
// vertically center child text | ||
align_items: AlignItems::Center, | ||
..Default::default() | ||
fn button_text(asset_server: &AssetServer) -> TextBundle { |
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 think I'd prefer passing in the text as an argument to at least hint at the fact that it is an abstraction.
examples/ui/button.rs
Outdated
commands.spawn_bundle(button()).with_children(|parent| { | ||
parent.spawn_bundle(button_text(&asset_server)); |
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.
This is fine for now, but it still scales awfully with more elements nested.
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 like the direction this is going. It's nice that layout and styling isn't mixed together, but this also shows how painful the whole parent,with_children is
Yeah, there is also a rare There is also the option of putting |
Yep. My opinion is that this API should just be removed. |
What would replace it though? |
|
color: Color::NONE.into(), | ||
..Default::default() | ||
}) | ||
.spawn_bundle(root_element()) |
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.
You could reduce the nesting by reusing the root element's EntityCommands
instead of having a root level with_children
,
let root = root.spawn_bundle(root_element());
// left vertical fill (border)
root.spawn_bundle(left_vertical_fill_border())
.with_children(|parent| {
...
});
// right vertical fill
root.spawn_bundle(right_vertical_fill())
.with_children(|parent| {
...
});
etc
Gonna wait for a UI api refactor |
Objective
Reduce the nesting in UI examples through extraction.
Fixes #3850
Status
Testing which layouts scale better.