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

Setting Display::None doesn't hide UI elements #7952

Closed
Pascualex opened this issue Mar 7, 2023 · 10 comments · Fixed by DioxusLabs/taffy#377, #7953, DioxusLabs/taffy#380, DioxusLabs/taffy#382 or #7959
Labels
A-UI Graphical user interfaces, styles, layouts, and widgets C-Bug An unexpected or incorrect behavior
Milestone

Comments

@Pascualex
Copy link
Contributor

Pascualex commented Mar 7, 2023

Bevy version

0.10

What you did

I updated an old project where I toggle the display of certain UI elements.

What went wrong

Once they were set to Display::Flex for the first time, setting Display::None didn't hide them.

Additional information

I built a minimal repro. The node spawns with Display::None. Pressing space for the first time toggles the display, showing the node. Pressing space a second time toggles the display but doesn't hide the node. This example works as expected when ran with Bevy 0.9.

use bevy::prelude::*;

fn main() {
    App::new()
        .add_plugins(DefaultPlugins)
        .add_startup_system(spawn_camera_and_node)
        .add_system(toggle_display)
        .run();
}

fn spawn_camera_and_node(mut commands: Commands) {
    commands.spawn(Camera2dBundle::default());
    commands.spawn(NodeBundle {
        style: Style {
            display: Display::None,
            size: Size::new(Val::Percent(100.0), Val::Percent(100.0)),
            ..default()
        },
        background_color: Color::RED.into(),
        ..default()
    });
}

fn toggle_display(mut query: Query<&mut Style>, input: Res<Input<KeyCode>>) {
    if input.just_pressed(KeyCode::Space) {
        let mut style = query.single_mut();
        style.display = match style.display {
            Display::Flex => Display::None,
            Display::None => Display::Flex,
        };
    }
}
@Pascualex Pascualex added C-Bug An unexpected or incorrect behavior S-Needs-Triage This issue needs to be labelled labels Mar 7, 2023
@mockersf mockersf added this to the 0.10.1 milestone Mar 7, 2023
@mockersf mockersf added A-UI Graphical user interfaces, styles, layouts, and widgets and removed S-Needs-Triage This issue needs to be labelled labels Mar 7, 2023
@rparrett
Copy link
Contributor

rparrett commented Mar 7, 2023

Bisected to 76058bc

@nicoburns
Copy link
Contributor

This one I can reproduce in Taffy. "leaf" display mode for nodes with no children is taking precedence over display: none. Fix incoming. Just trying to work out why our tests didn't catch this as we do have tests specifically testing display: none.

@nicoburns
Copy link
Contributor

Ah. So this bug only triggers if the display: none node is the only node in the hierarchy. If it either has children OR is itself a child of another node then it work fine!

@nicoburns
Copy link
Contributor

nicoburns commented Mar 7, 2023

@Pascualex Fix submitted in the form of #7953. In the meantime I believe you ought to be able to work around this issue by introducing an extra (useless) UI node. EDIT: while the workaround should work, I believe you can also get access to the new (fixed) version of Taffy by running cargo update which is probably a better solution.

@kelden
Copy link

kelden commented Mar 7, 2023

@nicoburns I have an empty menu node that I have created in a startup system with Display::None. It is correctly hidden at first, but when I toggle it to Flex and back to None it stays visible. This happens with or without children nodes, after updating to Taffy 0.3.4 with cargo update.

The minimal example still doesn't work for me after updating with cargo update.

@Pascualex
Copy link
Contributor Author

@nicoburns that was quick! Thank you.

@Pascualex
Copy link
Contributor Author

Yeah, @kelden is right, the minimal example is still failing for me too. Even though as you pointed cargo update does update Taffy to 0.3.4.

@nicoburns
Copy link
Contributor

This needs to be reopened, as it's not completely fixed.

@nicoburns
Copy link
Contributor

nicoburns commented Mar 7, 2023

@Pascualex @kelden Taffy v0.3.5 has now been released which I have confirmed fixes the minimal example for me. You should be able to get it by running cargo update again.

@Pascualex
Copy link
Contributor Author

@nicoburns yes, it works as expected now. Thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment