Skip to content

Fix manual Popup not closing#7383

Merged
lucasmerlin merged 3 commits intomainfrom
lucas/fix-manual-popup-close
Aug 7, 2025
Merged

Fix manual Popup not closing#7383
lucasmerlin merged 3 commits intomainfrom
lucas/fix-manual-popup-close

Conversation

@lucasmerlin
Copy link
Copy Markdown
Collaborator

@lucasmerlin lucasmerlin commented Jul 22, 2025

Fixes manually created popups (via Popup::new) not closing, since widget_clicked_elsewhere was always false.

This example would never close:

    let mut open = true;

    eframe::run_simple_native("My egui App", options, move |ctx, _frame| {
        egui::CentralPanel::default().show(ctx, |ui| {
            let response = egui::Popup::new(
                Id::new("popup"),
                ctx.clone(),
                PopupAnchor::Position(Pos2::new(10.0, 10.0)),
                LayerId::new(Order::Foreground, Id::new("popup")),
            )
            .open(open)
            .show(|ui| {
                ui.label("This is a popup!");
                ui.label("You can put anything in here.");
            });

            if let Some(response) = response {
                if response.response.should_close() {
                    open = false;
                }
            }
        });
    })

I also noticed that the Color submenu in the popups example had a double arrow (must have been broken in the atoms PR):

Screenshot 2025-08-07 at 13 42 28

Also fixed this in the PR.

@lucasmerlin lucasmerlin added bug Something is broken egui labels Jul 22, 2025
@lucasmerlin lucasmerlin added this to the 0.32.1 milestone Jul 22, 2025
@github-actions
Copy link
Copy Markdown

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


let closed_by_click = match close_behavior {
PopupCloseBehavior::CloseOnClick => widget_clicked_elsewhere,
PopupCloseBehavior::CloseOnClick => close_click,
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Shouldn't this close on any click?

Suggested change
PopupCloseBehavior::CloseOnClick => close_click,
PopupCloseBehavior::CloseOnClick => ctx.input(|i| i.pointer.any_click()),

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

We need to check if the click was the click that was used to open the popup, so we don't immediately close it again.

Previously we did this via the widget_clicked_elsewhere, which is also what we did in the old popup api. But I just realized this also won't work when using Popup::new so I came up with a better check. Now, on click, the popup will only close when it was already open last frame (by checking if a reasponse exists via ctx.read_response).

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Nice!

Copy link
Copy Markdown
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.

Nice


let closed_by_click = match close_behavior {
PopupCloseBehavior::CloseOnClick => widget_clicked_elsewhere,
PopupCloseBehavior::CloseOnClick => close_click,
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Nice!

@lucasmerlin lucasmerlin merged commit e8e99a0 into main Aug 7, 2025
47 checks passed
@lucasmerlin lucasmerlin deleted the lucas/fix-manual-popup-close branch August 7, 2025 12:18
emilk pushed a commit that referenced this pull request Aug 15, 2025
Fixes manually created popups (via `Popup::new`) not closing, since
widget_clicked_elsewhere was always false.

This example would never close:

```rs
    let mut open = true;

    eframe::run_simple_native("My egui App", options, move |ctx, _frame| {
        egui::CentralPanel::default().show(ctx, |ui| {
            let response = egui::Popup::new(
                Id::new("popup"),
                ctx.clone(),
                PopupAnchor::Position(Pos2::new(10.0, 10.0)),
                LayerId::new(Order::Foreground, Id::new("popup")),
            )
            .open(open)
            .show(|ui| {
                ui.label("This is a popup!");
                ui.label("You can put anything in here.");
            });

            if let Some(response) = response {
                if response.response.should_close() {
                    open = false;
                }
            }
        });
    })
```

I also noticed that the Color submenu in the popups example had a double
arrow (must have been broken in the atoms PR):

<img width="248" height="110" alt="Screenshot 2025-08-07 at 13 42 28"
src="https://github.com/user-attachments/assets/a4e0c267-ae71-4b2c-a1f0-f53f9662d026"
/>

Also fixed this in the PR.
Masterchef365 pushed a commit to Masterchef365/egui that referenced this pull request Apr 3, 2026
Fixes manually created popups (via `Popup::new`) not closing, since
widget_clicked_elsewhere was always false.

This example would never close:

```rs
    let mut open = true;

    eframe::run_simple_native("My egui App", options, move |ctx, _frame| {
        egui::CentralPanel::default().show(ctx, |ui| {
            let response = egui::Popup::new(
                Id::new("popup"),
                ctx.clone(),
                PopupAnchor::Position(Pos2::new(10.0, 10.0)),
                LayerId::new(Order::Foreground, Id::new("popup")),
            )
            .open(open)
            .show(|ui| {
                ui.label("This is a popup!");
                ui.label("You can put anything in here.");
            });

            if let Some(response) = response {
                if response.response.should_close() {
                    open = false;
                }
            }
        });
    })
```

I also noticed that the Color submenu in the popups example had a double
arrow (must have been broken in the atoms PR):

<img width="248" height="110" alt="Screenshot 2025-08-07 at 13 42 28"
src="https://github.com/user-attachments/assets/a4e0c267-ae71-4b2c-a1f0-f53f9662d026"
/>

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

Labels

bug Something is broken egui

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants