Skip to content

Conversation

vladbat00
Copy link

  • I have followed the instructions in the PR template

The PR makes it so a popup is closed on a press of a button but not click (i.e. release) of a button - this fixes the issue of a popup immediately closing after releasing a button.

I can reproduce the issue with the following code in bevy_egui:

    let popup = egui::Popup::new(
        egui::Id::new("A unique popup"),
        ctx.clone(),
        egui::PopupAnchor::PointerFixed,
        egui::LayerId::new(egui::Order::Foreground, egui::Id::new("popup layer")),
    );

    let mut open_command = None;
    for message in mouse_button_input_reader.read() {
        if message.state.is_pressed() && message.button == MouseButton::Right {
            open_command = Some(SetOpenCommand::Bool(true));
        }
    }

    popup.open_memory(open_command).show(|ui| {
        ui.label("This is a popup");
    });

When a popup is open due to a "press" event, it will get closed on the next "release" event (as they aren't likely to happen at the same frame). One could argue that the intended usage here is to open a popup only on a release of a button, but for users that have it open on a button press the current egui behavior could be counterintuitive.

I haven't tested this extensively, but if you find that this fix breaks any other behavior or some intended design where we do expect a popup close only on a button release, feel free to close this.

Copy link

github-actions bot commented Oct 10, 2025

Preview available at https://egui-pr-preview.github.io/pr/7624-fix-popup-close-click
Note that it might take a couple seconds for the update to show up after the preview_build workflow has completed.

View snapshot changes at kitdiff

Copy link
Owner

@emilk emilk left a comment

Choose a reason for hiding this comment

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

Makes sense to me, but please set up a kittest test for this, probably in crates/egui_kittest/tests/popup.rs.

Comment on lines 597 to 602
let close_click = was_open_last_frame && ctx.input(|i| i.pointer.any_pressed());

let closed_by_click = match close_behavior {
PopupCloseBehavior::CloseOnClick => close_click,
PopupCloseBehavior::CloseOnClickOutside => {
close_click && response.response.clicked_elsewhere()
Copy link
Owner

Choose a reason for hiding this comment

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

I guess there was (and still is) a loop-hole here when it comes to drags. For instance: should dragging a slider close the popup?

The new code will close the popup when starting a drag, which is different, but probably good. But I think the interaction with clicked_elsewhere will be a bit wrong. Worth doing some manual tests

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants