From 8f1dbe7edd1857d6694e995b544a98c968ca4c47 Mon Sep 17 00:00:00 2001 From: lucasmerlin Date: Thu, 3 Jul 2025 15:06:46 +0200 Subject: [PATCH] Make `Memory::keep_popup_open` less of a footgun --- crates/egui/src/containers/combo_box.rs | 4 +-- crates/egui/src/containers/popup.rs | 6 ++--- crates/egui/src/memory/mod.rs | 35 ++++++++++++++----------- crates/egui/src/widgets/color_picker.rs | 2 +- 4 files changed, 25 insertions(+), 22 deletions(-) diff --git a/crates/egui/src/containers/combo_box.rs b/crates/egui/src/containers/combo_box.rs index fc5f33905..84e20d425 100644 --- a/crates/egui/src/containers/combo_box.rs +++ b/crates/egui/src/containers/combo_box.rs @@ -293,7 +293,7 @@ impl ComboBox { /// Check if the [`ComboBox`] with the given id has its popup menu currently opened. pub fn is_open(ctx: &Context, id: Id) -> bool { - ctx.memory(|m| m.is_popup_open(Self::widget_to_popup_id(id))) + ctx.memory(|m| m.is_showing_popup(Self::widget_to_popup_id(id))) } /// Convert a [`ComboBox`] id to the id used to store it's popup state. @@ -315,7 +315,7 @@ fn combo_box_dyn<'c, R>( ) -> InnerResponse> { let popup_id = ComboBox::widget_to_popup_id(button_id); - let is_popup_open = ui.memory(|m| m.is_popup_open(popup_id)); + let is_popup_open = ui.memory(|m| m.is_showing_popup(popup_id)); let wrap_mode = wrap_mode.unwrap_or_else(|| ui.wrap_mode()); diff --git a/crates/egui/src/containers/popup.rs b/crates/egui/src/containers/popup.rs index 434747cda..450f559b7 100644 --- a/crates/egui/src/containers/popup.rs +++ b/crates/egui/src/containers/popup.rs @@ -127,7 +127,7 @@ impl OpenKind<'_> { OpenKind::Open => true, OpenKind::Closed => false, OpenKind::Bool(open) => **open, - OpenKind::Memory { .. } => ctx.memory(|mem| mem.is_popup_open(id)), + OpenKind::Memory { .. } => ctx.memory(|mem| mem.is_showing_popup(id)), } } } @@ -446,7 +446,7 @@ impl<'a> Popup<'a> { OpenKind::Open => true, OpenKind::Closed => false, OpenKind::Bool(open) => **open, - OpenKind::Memory { .. } => self.ctx.memory(|mem| mem.is_popup_open(self.id)), + OpenKind::Memory { .. } => self.ctx.memory(|mem| mem.is_showing_popup(self.id)), } } @@ -531,7 +531,7 @@ impl<'a> Popup<'a> { mem.toggle_popup(id); } None => { - mem.keep_popup_open(id); + mem.show_popup(id); } }); } diff --git a/crates/egui/src/memory/mod.rs b/crates/egui/src/memory/mod.rs index 8f98de305..e2ae9ef96 100644 --- a/crates/egui/src/memory/mod.rs +++ b/crates/egui/src/memory/mod.rs @@ -1017,13 +1017,29 @@ impl OpenPopup { /// Only one can be open at a time. impl Memory { /// Is the given popup open? - pub fn is_popup_open(&self, popup_id: Id) -> bool { + /// + /// NOTE: To actually show the popup, you must call [`Self::show_popup`]. + /// If [`Self::show_popup`] is not called for a frame, the open state will be cleared. + pub fn is_showing_popup(&self, popup_id: Id) -> bool { self.popups .get(&self.viewport_id) .is_some_and(|state| state.id == popup_id) || self.everything_is_visible() } + /// Check if the popup is open and keep it open. + pub fn show_popup(&mut self, popup_id: Id) -> bool { + let should_show = self.is_showing_popup(popup_id); + if should_show { + if let Some(state) = self.popups.get_mut(&self.viewport_id) { + if state.id == popup_id { + state.open_this_frame = true; + } + } + } + should_show + } + /// Is any popup open? pub fn any_popup_open(&self) -> bool { self.popups.contains_key(&self.viewport_id) || self.everything_is_visible() @@ -1037,19 +1053,6 @@ impl Memory { .insert(self.viewport_id, OpenPopup::new(popup_id, None)); } - /// Popups must call this every frame while open. - /// - /// This is needed because in some cases popups can go away without `close_popup` being - /// called. For example, when a context menu is open and the underlying widget stops - /// being rendered. - pub fn keep_popup_open(&mut self, popup_id: Id) { - if let Some(state) = self.popups.get_mut(&self.viewport_id) { - if state.id == popup_id { - state.open_this_frame = true; - } - } - } - /// Open the popup and remember its position. pub fn open_popup_at(&mut self, popup_id: Id, pos: impl Into>) { self.popups @@ -1072,7 +1075,7 @@ impl Memory { /// /// See also [`Self::close_all_popups`] if you want to close any / all currently open popups. pub fn close_popup(&mut self, popup_id: Id) { - if self.is_popup_open(popup_id) { + if self.is_showing_popup(popup_id) { self.popups.remove(&self.viewport_id); } } @@ -1081,7 +1084,7 @@ impl Memory { /// /// Note: At most, only one popup can be open at a time. pub fn toggle_popup(&mut self, popup_id: Id) { - if self.is_popup_open(popup_id) { + if self.is_showing_popup(popup_id) { self.close_popup(popup_id); } else { self.open_popup(popup_id); diff --git a/crates/egui/src/widgets/color_picker.rs b/crates/egui/src/widgets/color_picker.rs index f8605beaf..2adaf956d 100644 --- a/crates/egui/src/widgets/color_picker.rs +++ b/crates/egui/src/widgets/color_picker.rs @@ -491,7 +491,7 @@ pub fn color_picker_color32(ui: &mut Ui, srgba: &mut Color32, alpha: Alpha) -> b pub fn color_edit_button_hsva(ui: &mut Ui, hsva: &mut Hsva, alpha: Alpha) -> Response { let popup_id = ui.auto_id_with("popup"); - let open = ui.memory(|mem| mem.is_popup_open(popup_id)); + let open = ui.memory(|mem| mem.is_showing_popup(popup_id)); let mut button_response = color_button(ui, (*hsva).into(), open); if ui.style().explanation_tooltips { button_response = button_response.on_hover_text("Click to edit color");