diff --git a/crates/egui/src/containers/menu.rs b/crates/egui/src/containers/menu.rs index 1bd5954c8..f9dd51ae2 100644 --- a/crates/egui/src/containers/menu.rs +++ b/crates/egui/src/containers/menu.rs @@ -521,8 +521,8 @@ impl SubMenu { // Only edge case is the user hovering this submenu's button, so we also check // if we clicked outside the parent menu (which we luckily have access to here). let clicked_outside = is_deepest_submenu - && popup_response.response.clicked_elsewhere() - && menu_root_response.clicked_elsewhere(); + && popup_response.response.clicked_elsewhere_excluding_child_layers() + && menu_root_response.clicked_elsewhere_excluding_child_layers(); // We never automatically close when a submenu button is clicked, (so menus work // on touch devices) diff --git a/crates/egui/src/containers/popup.rs b/crates/egui/src/containers/popup.rs index 0fb2a9f2a..2181408eb 100644 --- a/crates/egui/src/containers/popup.rs +++ b/crates/egui/src/containers/popup.rs @@ -513,19 +513,19 @@ impl<'a> Popup<'a> { if open { match self.anchor { PopupAnchor::PointerFixed => { - self.ctx.memory_mut(|mem| mem.open_popup_at(id, hover_pos)); + self.ctx.memory_mut(|mem| mem.open_popup_at(self.layer_id, id, hover_pos)); } - _ => Popup::open_id(&self.ctx, id), + _ => Popup::open_id(&self.ctx, self.layer_id, id), } } else { Self::close_id(&self.ctx, id); } } Some(SetOpenCommand::Toggle) => { - Self::toggle_id(&self.ctx, id); + Self::toggle_id(&self.ctx, self.layer_id, id); } None => { - self.ctx.memory_mut(|mem| mem.keep_popup_open(id)); + self.ctx.memory_mut(|mem| mem.keep_popup_open(self.layer_id, id)); } } } @@ -570,7 +570,7 @@ impl<'a> Popup<'a> { let (pivot, anchor) = best_align.pivot_pos(&anchor_rect, gap); let mut area = Area::new(id) - .order(kind.order()) + .order(layer_id.order) .pivot(pivot) .fixed_pos(anchor) .sense(sense) @@ -589,6 +589,7 @@ impl<'a> Popup<'a> { } let mut response = area.show(&ctx, |ui| { + ui.set_sublayer(layer_id, ui.layer_id()); style.apply(ui.style_mut()); let frame = frame.unwrap_or_else(|| Frame::popup(ui.style())); frame.show(ui, content).inner @@ -600,7 +601,7 @@ impl<'a> Popup<'a> { let closed_by_click = match close_behavior { PopupCloseBehavior::CloseOnClick => close_click, PopupCloseBehavior::CloseOnClickOutside => { - close_click && response.response.clicked_elsewhere() + close_click && response.response.clicked_elsewhere_excluding_child_layers() } PopupCloseBehavior::IgnoreClicks => false, }; @@ -670,15 +671,15 @@ impl Popup<'_> { /// If you are NOT using [`Popup::show`], you must /// also call [`crate::Memory::keep_popup_open`] as long as /// you're showing the popup. - pub fn open_id(ctx: &Context, popup_id: Id) { - ctx.memory_mut(|mem| mem.open_popup(popup_id)); + pub fn open_id(ctx: &Context, layer_id: LayerId, popup_id: Id) { + ctx.memory_mut(|mem| mem.open_popup(layer_id, popup_id)); } /// Toggle the given popup between closed and open. /// /// Note: At most, only one popup can be open at a time. - pub fn toggle_id(ctx: &Context, popup_id: Id) { - ctx.memory_mut(|mem| mem.toggle_popup(popup_id)); + pub fn toggle_id(ctx: &Context, layer_id: LayerId, popup_id: Id) { + ctx.memory_mut(|mem| mem.toggle_popup(layer_id, popup_id)); } /// Close all currently open popups. diff --git a/crates/egui/src/memory/mod.rs b/crates/egui/src/memory/mod.rs index defb14a39..24903126a 100644 --- a/crates/egui/src/memory/mod.rs +++ b/crates/egui/src/memory/mod.rs @@ -6,8 +6,8 @@ use ahash::{HashMap, HashSet}; use epaint::emath::TSTransform; use crate::{ - EventFilter, Id, IdMap, LayerId, Order, Pos2, Rangef, RawInput, Rect, Style, Vec2, ViewportId, - ViewportIdMap, ViewportIdSet, area, vec2, + area, vec2, EventFilter, Id, IdMap, LayerId, Order, Pos2, Rangef, RawInput, Rect, Style, + Vec2, ViewportId, ViewportIdMap, ViewportIdSet, }; mod theme; @@ -115,7 +115,7 @@ pub struct Memory { /// If position is [`None`], the popup position will be calculated based on some configuration /// (e.g. relative to some other widget). #[cfg_attr(feature = "persistence", serde(skip))] - popups: ViewportIdMap, + popups: ViewportIdMap>, } impl Default for Memory { @@ -743,7 +743,7 @@ impl Memory { // Cleanup self.interactions.retain(|id, _| viewports.contains(id)); self.areas.retain(|id, _| viewports.contains(id)); - self.popups.retain(|id, _| viewports.contains(id)); + // self.popups.retain(|id, _| viewports.contains(id)); TODO self.areas.entry(self.viewport_id).or_default(); @@ -763,12 +763,12 @@ impl Memory { self.focus_mut().end_pass(used_ids); // Clean up abandoned popups. - if let Some(popup) = self.popups.get_mut(&self.viewport_id) { - if popup.open_this_frame { + if let Some(popups) = self.popups.get_mut(&self.viewport_id) { + popups.retain(|_, popup| { + let open_this_frame = popup.open_this_frame; popup.open_this_frame = false; - } else { - self.popups.remove(&self.viewport_id); - } + open_this_frame + }) } } @@ -1013,23 +1013,31 @@ impl Memory { pub fn is_popup_open(&self, popup_id: Id) -> bool { self.popups .get(&self.viewport_id) - .is_some_and(|state| state.id == popup_id) + .is_some_and(|state| { + state.values().any(|popup| popup.id == popup_id) + }) || self.everything_is_visible() } /// Is any popup open? #[deprecated = "Use Popup::is_any_open instead"] pub fn any_popup_open(&self) -> bool { - self.popups.contains_key(&self.viewport_id) || self.everything_is_visible() + self.popups + .get(&self.viewport_id) + .is_some_and(|state| !state.is_empty()) + || self.everything_is_visible() } /// Open the given popup and close all others. /// /// Note that you must call `keep_popup_open` on subsequent frames as long as the popup is open. #[deprecated = "Use Popup::open_id instead"] - pub fn open_popup(&mut self, popup_id: Id) { + pub fn open_popup(&mut self, layer: LayerId, popup_id: Id) { + // TODO: Close non-parent popups self.popups - .insert(self.viewport_id, OpenPopup::new(popup_id, None)); + .entry(self.viewport_id) + .or_default() + .insert(layer, OpenPopup::new(popup_id, None)); } /// Popups must call this every frame while open. @@ -1038,26 +1046,34 @@ impl Memory { /// called. For example, when a context menu is open and the underlying widget stops /// being rendered. #[deprecated = "Use Popup::show instead"] - pub fn keep_popup_open(&mut self, popup_id: Id) { - if let Some(state) = self.popups.get_mut(&self.viewport_id) - && state.id == popup_id - { - state.open_this_frame = true; + pub fn keep_popup_open(&mut self, layer_id: LayerId, popup_id: Id) { + if let Some(state) = self.popups.get_mut(&self.viewport_id) { + if let Some(popup) = state.get_mut(&layer_id) { + if popup.id == popup_id { + popup.open_this_frame = true; + } + } } } /// Open the popup and remember its position. #[deprecated = "Use Popup with PopupAnchor::Position instead"] - pub fn open_popup_at(&mut self, popup_id: Id, pos: impl Into>) { + pub fn open_popup_at(&mut self, layer_id: LayerId, popup_id: Id, pos: impl Into>) { self.popups - .insert(self.viewport_id, OpenPopup::new(popup_id, pos.into())); + .entry(self.viewport_id) + .or_default() + .insert(layer_id, OpenPopup::new(popup_id, pos.into())); } /// Get the position for this popup. #[deprecated = "Use Popup::position_of_id instead"] pub fn popup_position(&self, id: Id) -> Option { - let state = self.popups.get(&self.viewport_id)?; - if state.id == id { state.pos } else { None } + self.popups.get(&self.viewport_id).and_then(|state| { + state + .values() + .find(|popup| popup.id == id) + .and_then(|popup| popup.pos) + }) } /// Close any currently open popup. @@ -1071,9 +1087,8 @@ impl Memory { /// See also [`Self::close_all_popups`] if you want to close any / all currently open popups. #[deprecated = "Use Popup::close_id instead"] pub fn close_popup(&mut self, popup_id: Id) { - #[expect(deprecated)] - if self.is_popup_open(popup_id) { - self.popups.remove(&self.viewport_id); + if let Some(state) = self.popups.get_mut(&self.viewport_id) { + state.retain(|_, popup| popup.id != popup_id); } } @@ -1081,12 +1096,11 @@ impl Memory { /// /// Note: At most, only one popup can be open at a time. #[deprecated = "Use Popup::toggle_id instead"] - pub fn toggle_popup(&mut self, popup_id: Id) { - #[expect(deprecated)] + pub fn toggle_popup(&mut self, layer_id: LayerId, popup_id: Id) { if self.is_popup_open(popup_id) { self.close_popup(popup_id); } else { - self.open_popup(popup_id); + self.open_popup(layer_id, popup_id); } } } @@ -1294,6 +1308,17 @@ impl Areas { self.sublayers.get(&layer_id).into_iter().flatten().copied() } + pub fn is_child_recursive(&self, parent: LayerId, child: LayerId) -> bool { + if let Some(children) = self.sublayers.get(&parent) { + for &c in children { + if c == child || self.is_child_recursive(c, child) { + return true; + } + } + } + false + } + pub(crate) fn is_sublayer(&self, layer: &LayerId) -> bool { self.parent_layer(*layer).is_some() } diff --git a/crates/egui/src/response.rs b/crates/egui/src/response.rs index 495d5dcdf..968c93321 100644 --- a/crates/egui/src/response.rs +++ b/crates/egui/src/response.rs @@ -1,10 +1,10 @@ use std::{any::Any, sync::Arc}; use crate::{ - Context, CursorIcon, Id, LayerId, PointerButton, Popup, PopupKind, Sense, Tooltip, Ui, - WidgetRect, WidgetText, - emath::{Align, Pos2, Rect, Vec2}, - pass_state, + emath::{Align, Pos2, Rect, Vec2}, pass_state, Context, CursorIcon, Id, LayerId, PointerButton, Popup, PopupKind, Sense, + Tooltip, Ui, + WidgetRect, + WidgetText, }; // ---------------------------------------------------------------------------- @@ -259,6 +259,28 @@ impl Response { } } + pub fn clicked_elsewhere_excluding_child_layers(&self) -> bool { + let clicked_elsewhere = self.clicked_elsewhere(); + if !clicked_elsewhere { + return false; + } + + if let Some(pos) = self.ctx.input(|i| i.pointer.interact_pos()) { + let layer_under_pointer = self.ctx.layer_id_at(pos); + if let Some(layer_under_pointer) = layer_under_pointer { + let child_clicked = self.ctx.memory(|mem| { + mem.areas() + .is_child_recursive(self.layer_id, layer_under_pointer) + }); + !child_clicked + } else { + clicked_elsewhere + } + } else { + clicked_elsewhere + } + } + /// Was the widget enabled? /// If false, there was no interaction attempted /// and the widget should be drawn in a gray disabled look. diff --git a/crates/egui_demo_lib/src/demo/popups.rs b/crates/egui_demo_lib/src/demo/popups.rs index 144826e0a..0e0420254 100644 --- a/crates/egui_demo_lib/src/demo/popups.rs +++ b/crates/egui_demo_lib/src/demo/popups.rs @@ -1,9 +1,9 @@ use crate::rust_view_ui; -use egui::color_picker::{Alpha, color_picker_color32}; +use egui::color_picker::{color_picker_color32, Alpha}; use egui::containers::menu::{MenuConfig, SubMenuButton}; use egui::{ - Align, Align2, Atom, Button, ComboBox, Frame, Id, Layout, Popup, PopupCloseBehavior, RectAlign, - RichText, Tooltip, Ui, UiBuilder, include_image, + include_image, Align, Align2, Atom, Button, ComboBox, Frame, Id, Layout, Popup, PopupCloseBehavior, + RectAlign, RichText, Tooltip, Ui, UiBuilder, }; /// Showcase [`Popup`]. @@ -119,6 +119,13 @@ impl PopupsDemo { if ui.button("Open…").clicked() { ui.close(); } + + ComboBox::new("Combobox in menu", "") + .selected_text(if self.checked { "Option 1" } else { "Option 2" }) + .show_ui(ui, |ui| { + ui.selectable_value(&mut self.checked, true, "Option 1"); + ui.selectable_value(&mut self.checked, false, "Option 2"); + }); }); } } diff --git a/crates/egui_kittest/src/node.rs b/crates/egui_kittest/src/node.rs index 94940ffff..ad27edfca 100644 --- a/crates/egui_kittest/src/node.rs +++ b/crates/egui_kittest/src/node.rs @@ -17,6 +17,14 @@ pub struct Node<'tree> { pub(crate) queue: &'tree EventQueue, } +impl PartialEq for Node<'_> { + fn eq(&self, other: &Self) -> bool { + self.accesskit_node.id() == other.accesskit_node.id() + } +} + +impl Eq for Node<'_> {} + impl Debug for Node<'_> { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { debug_fmt_node(self, f) diff --git a/crates/egui_kittest/tests/popup.rs b/crates/egui_kittest/tests/popup.rs deleted file mode 100644 index a8d3bcc9d..000000000 --- a/crates/egui_kittest/tests/popup.rs +++ /dev/null @@ -1,31 +0,0 @@ -use kittest::Queryable as _; - -#[test] -fn test_interactive_tooltip() { - struct State { - link_clicked: bool, - } - - let mut harness = egui_kittest::Harness::new_ui_state( - |ui, state| { - ui.label("I have a tooltip").on_hover_ui(|ui| { - if ui.link("link").clicked() { - state.link_clicked = true; - } - }); - }, - State { - link_clicked: false, - }, - ); - - harness.get_by_label_contains("tooltip").hover(); - harness.run(); - harness.get_by_label("link").hover(); - harness.run(); - harness.get_by_label("link").click(); - - harness.run(); - - assert!(harness.state().link_clicked); -} diff --git a/tests/egui_tests/tests/snapshots/combobox_in_popup.png b/tests/egui_tests/tests/snapshots/combobox_in_popup.png new file mode 100644 index 000000000..9f325e95e --- /dev/null +++ b/tests/egui_tests/tests/snapshots/combobox_in_popup.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:e8876d1b19c1d8c0cac30edc022913ca88ba7afaedf677e9c5de07c3b45e3b8a +size 10595 diff --git a/tests/egui_tests/tests/test_popups.rs b/tests/egui_tests/tests/test_popups.rs new file mode 100644 index 000000000..660c1316e --- /dev/null +++ b/tests/egui_tests/tests/test_popups.rs @@ -0,0 +1,88 @@ +use egui::{ComboBox, Popup, PopupCloseBehavior, Vec2}; +use egui_kittest::kittest::Queryable as _; +use egui_kittest::Harness; + +#[test] +fn test_interactive_tooltip() { + struct State { + link_clicked: bool, + } + + let mut harness = egui_kittest::Harness::new_ui_state( + |ui, state| { + ui.label("I have a tooltip").on_hover_ui(|ui| { + if ui.link("link").clicked() { + state.link_clicked = true; + } + }); + }, + State { + link_clicked: false, + }, + ); + + harness.get_by_label_contains("tooltip").hover(); + harness.run(); + harness.get_by_label("link").hover(); + harness.run(); + harness.get_by_label("link").click(); + + harness.run(); + + assert!(harness.state().link_clicked); +} + +#[test] +fn test_combobox_in_popup() { + let mut harness = Harness::builder() + .with_size(Vec2::new(200.0, 200.0)) + .build_ui_state( + |ui, state| { + let response = ui.button("Open Popup"); + Popup::menu(&response) + .close_behavior(PopupCloseBehavior::CloseOnClickOutside) + .show(|ui| { + ui.heading("Popup"); + ComboBox::new("combo", "") + .selected_text("Select an option") + .close_behavior(PopupCloseBehavior::CloseOnClickOutside) + .show_ui(ui, |ui| { + ui.selectable_value(state, 0, "Option 0"); + ui.selectable_value(state, 1, "Option 1"); + }); + }); + }, + 0, + ); + + harness.get_by_label("Open Popup").click(); + harness.run(); + harness.get_by_value("Select an option").click(); + harness.run(); + harness.get_by_label("Option 1").click(); + harness.run(); + assert_eq!(*harness.state(), 1); + + // The parent popup should not close when clicking on the child popup + harness.get_by_label("Option 0").click(); + harness.run(); + assert_eq!(*harness.state(), 0); + + harness.snapshot("combobox_in_popup"); + + // Clicking the parent popup should close the child popup + harness.get_by_label("Popup").click(); + harness.run(); + assert_eq!(harness.query_by_label("Option 0"), None); + + harness.get_by_value("Select an option").click(); + harness.run(); + + assert_eq!(harness.query_by_label("Option 0").is_some(), true); + + // Clicking outside should close both popups + harness.get_by_label("Open Popup").click(); + harness.run(); + + assert_eq!(harness.query_by_label("Popup"), None); +}