Make Memory::keep_popup_open less of a footgun

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<Option<R>> {
     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());
 

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);
                 }
             });
         }

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<Option<Pos2>>) {
         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);

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");