Make sure all tooltips close if you open a menu in the same layer (#4766)

crates/egui/src/containers/popup.rs

@@ -6,6 +6,29 @@ use crate::*;
 
 // ----------------------------------------------------------------------------
 
+fn when_was_a_toolip_last_shown_id() -> Id {
+    Id::new("when_was_a_toolip_last_shown")
+}
+
+pub fn seconds_since_last_tooltip(ctx: &Context) -> f32 {
+    let when_was_a_toolip_last_shown =
+        ctx.data(|d| d.get_temp::<f64>(when_was_a_toolip_last_shown_id()));
+
+    if let Some(when_was_a_toolip_last_shown) = when_was_a_toolip_last_shown {
+        let now = ctx.input(|i| i.time);
+        (now - when_was_a_toolip_last_shown) as f32
+    } else {
+        f32::INFINITY
+    }
+}
+
+fn remember_that_tooltip_was_shown(ctx: &Context) {
+    let now = ctx.input(|i| i.time);
+    ctx.data_mut(|data| data.insert_temp::<f64>(when_was_a_toolip_last_shown_id(), now));
+}
+
+// ----------------------------------------------------------------------------
+
 /// Show a tooltip at the current pointer position (if any).
 ///
 /// Most of the time it is easier to use [`Response::on_hover_ui`].
@@ -123,14 +146,16 @@ fn show_tooltip_at_dyn<'c, R>(
         widget_rect = transform * widget_rect;
     }
 
-    // if there are multiple tooltips open they should use the same common_id for the `tooltip_size` caching to work.
+    remember_that_tooltip_was_shown(ctx);
+
     let mut state = ctx.frame_state_mut(|fs| {
         // Remember that this is the widget showing the tooltip:
-        fs.tooltip_state
-            .per_layer_tooltip_widget
-            .insert(parent_layer, widget_id);
+        fs.layers
+            .entry(parent_layer)
+            .or_default()
+            .widget_with_tooltip = Some(widget_id);
 
-        fs.tooltip_state
+        fs.tooltips
             .widget_tooltips
             .get(&widget_id)
             .copied()
@@ -174,7 +199,7 @@ fn show_tooltip_at_dyn<'c, R>(
 
     state.tooltip_count += 1;
     state.bounding_rect = state.bounding_rect.union(response.rect);
-    ctx.frame_state_mut(|fs| fs.tooltip_state.widget_tooltips.insert(widget_id, state));
+    ctx.frame_state_mut(|fs| fs.tooltips.widget_tooltips.insert(widget_id, state));
 
     inner
 }
@@ -182,7 +207,7 @@ fn show_tooltip_at_dyn<'c, R>(
 /// What is the id of the next tooltip for this widget?
 pub fn next_tooltip_id(ctx: &Context, widget_id: Id) -> Id {
     let tooltip_count = ctx.frame_state(|fs| {
-        fs.tooltip_state
+        fs.tooltips
             .widget_tooltips
             .get(&widget_id)
             .map_or(0, |state| state.tooltip_count)
@@ -351,53 +376,61 @@ pub fn popup_above_or_below_widget<R>(
     close_behavior: PopupCloseBehavior,
     add_contents: impl FnOnce(&mut Ui) -> R,
 ) -> Option<R> {
-    if parent_ui.memory(|mem| mem.is_popup_open(popup_id)) {
-        let (mut pos, pivot) = match above_or_below {
-            AboveOrBelow::Above => (widget_response.rect.left_top(), Align2::LEFT_BOTTOM),
-            AboveOrBelow::Below => (widget_response.rect.left_bottom(), Align2::LEFT_TOP),
-        };
-        if let Some(transform) = parent_ui
-            .ctx()
-            .memory(|m| m.layer_transforms.get(&parent_ui.layer_id()).copied())
-        {
-            pos = transform * pos;
-        }
+    if !parent_ui.memory(|mem| mem.is_popup_open(popup_id)) {
+        return None;
+    }
+
+    let (mut pos, pivot) = match above_or_below {
+        AboveOrBelow::Above => (widget_response.rect.left_top(), Align2::LEFT_BOTTOM),
+        AboveOrBelow::Below => (widget_response.rect.left_bottom(), Align2::LEFT_TOP),
+    };
+    if let Some(transform) = parent_ui
+        .ctx()
+        .memory(|m| m.layer_transforms.get(&parent_ui.layer_id()).copied())
+    {
+        pos = transform * pos;
+    }
+
+    let frame = Frame::popup(parent_ui.style());
+    let frame_margin = frame.total_margin();
+    let inner_width = widget_response.rect.width() - frame_margin.sum().x;
+
+    parent_ui.ctx().frame_state_mut(|fs| {
+        fs.layers
+            .entry(parent_ui.layer_id())
+            .or_default()
+            .open_popups
+            .insert(popup_id)
+    });
 
-        let frame = Frame::popup(parent_ui.style());
-        let frame_margin = frame.total_margin();
-        let inner_width = widget_response.rect.width() - frame_margin.sum().x;
-
-        let response = Area::new(popup_id)
-            .kind(UiKind::Popup)
-            .order(Order::Foreground)
-            .fixed_pos(pos)
-            .default_width(inner_width)
-            .pivot(pivot)
-            .show(parent_ui.ctx(), |ui| {
-                frame
-                    .show(ui, |ui| {
-                        ui.with_layout(Layout::top_down_justified(Align::LEFT), |ui| {
-                            ui.set_min_width(inner_width);
-                            add_contents(ui)
-                        })
-                        .inner
+    let response = Area::new(popup_id)
+        .kind(UiKind::Popup)
+        .order(Order::Foreground)
+        .fixed_pos(pos)
+        .default_width(inner_width)
+        .pivot(pivot)
+        .show(parent_ui.ctx(), |ui| {
+            frame
+                .show(ui, |ui| {
+                    ui.with_layout(Layout::top_down_justified(Align::LEFT), |ui| {
+                        ui.set_min_width(inner_width);
+                        add_contents(ui)
                     })
                     .inner
-            });
-
-        let should_close = match close_behavior {
-            PopupCloseBehavior::CloseOnClick => widget_response.clicked_elsewhere(),
-            PopupCloseBehavior::CloseOnClickOutside => {
-                widget_response.clicked_elsewhere() && response.response.clicked_elsewhere()
-            }
-            PopupCloseBehavior::IgnoreClicks => false,
-        };
-
-        if parent_ui.input(|i| i.key_pressed(Key::Escape)) || should_close {
-            parent_ui.memory_mut(|mem| mem.close_popup());
+                })
+                .inner
+        });
+
+    let should_close = match close_behavior {
+        PopupCloseBehavior::CloseOnClick => widget_response.clicked_elsewhere(),
+        PopupCloseBehavior::CloseOnClickOutside => {
+            widget_response.clicked_elsewhere() && response.response.clicked_elsewhere()
         }
-        Some(response.inner)
-    } else {
-        None
+        PopupCloseBehavior::IgnoreClicks => false,
+    };
+
+    if parent_ui.input(|i| i.key_pressed(Key::Escape)) || should_close {
+        parent_ui.memory_mut(|mem| mem.close_popup());
     }
+    Some(response.inner)
 }

crates/egui/src/frame_state.rs

@@ -1,3 +1,5 @@
+use ahash::{HashMap, HashSet};
+
 use crate::{id::IdSet, *};
 
 /// Reset at the start of each frame.
@@ -6,22 +8,12 @@ pub struct TooltipFrameState {
     /// If a tooltip has been shown this frame, where was it?
     /// This is used to prevent multiple tooltips to cover each other.
     pub widget_tooltips: IdMap<PerWidgetTooltipState>,
-
-    /// For each layer, which widget is showing a tooltip (if any)?
-    ///
-    /// Only one widget per layer may show a tooltip.
-    /// But if a tooltip contains a tooltip, you can show a tooltip on top of a tooltip.
-    pub per_layer_tooltip_widget: ahash::HashMap<LayerId, Id>,
 }
 
 impl TooltipFrameState {
     pub fn clear(&mut self) {
-        let Self {
-            widget_tooltips,
-            per_layer_tooltip_widget,
-        } = self;
+        let Self { widget_tooltips } = self;
         widget_tooltips.clear();
-        per_layer_tooltip_widget.clear();
     }
 }
 
@@ -34,6 +26,20 @@ pub struct PerWidgetTooltipState {
     pub tooltip_count: usize,
 }
 
+#[derive(Clone, Debug, Default)]
+pub struct PerLayerState {
+    /// Is there any open popup (menus, combo-boxes, etc)?
+    ///
+    /// Does NOT include tooltips.
+    pub open_popups: HashSet<Id>,
+
+    /// Which widget is showing a tooltip (if any)?
+    ///
+    /// Only one widget per layer may show a tooltip.
+    /// But if a tooltip contains a tooltip, you can show a tooltip on top of a tooltip.
+    pub widget_with_tooltip: Option<Id>,
+}
+
 #[cfg(feature = "accesskit")]
 #[derive(Clone)]
 pub struct AccessKitFrameState {
@@ -53,6 +59,13 @@ pub struct FrameState {
     /// All widgets produced this frame.
     pub widgets: WidgetRects,
 
+    /// Per-layer state.
+    ///
+    /// Not all layers registers themselves there though.
+    pub layers: HashMap<LayerId, PerLayerState>,
+
+    pub tooltips: TooltipFrameState,
+
     /// Starts off as the `screen_rect`, shrinks as panels are added.
     /// The [`CentralPanel`] does not change this.
     /// This is the area available to Window's.
@@ -65,8 +78,6 @@ pub struct FrameState {
     /// How much space is used by panels.
     pub used_by_panels: Rect,
 
-    pub tooltip_state: TooltipFrameState,
-
     /// The current scroll area should scroll to this range (horizontal, vertical).
     pub scroll_target: [Option<(Rangef, Option<Align>)>; 2],
 
@@ -96,10 +107,11 @@ impl Default for FrameState {
         Self {
             used_ids: Default::default(),
             widgets: Default::default(),
+            layers: Default::default(),
+            tooltips: Default::default(),
             available_rect: Rect::NAN,
             unused_rect: Rect::NAN,
             used_by_panels: Rect::NAN,
-            tooltip_state: Default::default(),
             scroll_target: [None, None],
             scroll_delta: Vec2::default(),
             #[cfg(feature = "accesskit")]
@@ -118,10 +130,11 @@ impl FrameState {
         let Self {
             used_ids,
             widgets,
+            tooltips,
+            layers,
             available_rect,
             unused_rect,
             used_by_panels,
-            tooltip_state,
             scroll_target,
             scroll_delta,
             #[cfg(feature = "accesskit")]
@@ -134,10 +147,11 @@ impl FrameState {
 
         used_ids.clear();
         widgets.clear();
+        tooltips.clear();
+        layers.clear();
         *available_rect = screen_rect;
         *unused_rect = screen_rect;
         *used_by_panels = Rect::NOTHING;
-        tooltip_state.clear();
         *scroll_target = [None, None];
         *scroll_delta = Vec2::default();
 

crates/egui/src/menu.rs

@@ -135,6 +135,7 @@ pub(crate) fn submenu_button<R>(
 /// wrapper for the contents of every menu.
 fn menu_popup<'c, R>(
     ctx: &Context,
+    parent_layer: LayerId,
     menu_state_arc: &Arc<RwLock<MenuState>>,
     menu_id: Id,
     add_contents: impl FnOnce(&mut Ui) -> R + 'c,
@@ -145,7 +146,17 @@ fn menu_popup<'c, R>(
         menu_state.rect.min
     };
 
-    let area = Area::new(menu_id.with("__menu"))
+    let area_id = menu_id.with("__menu");
+
+    ctx.frame_state_mut(|fs| {
+        fs.layers
+            .entry(parent_layer)
+            .or_default()
+            .open_popups
+            .insert(area_id)
+    });
+
+    let area = Area::new(area_id)
         .kind(UiKind::Menu)
         .order(Order::Foreground)
         .fixed_pos(pos)
@@ -320,7 +331,13 @@ impl MenuRoot {
         add_contents: impl FnOnce(&mut Ui) -> R,
     ) -> (MenuResponse, Option<InnerResponse<R>>) {
         if self.id == button.id {
-            let inner_response = menu_popup(&button.ctx, &self.menu_state, self.id, add_contents);
+            let inner_response = menu_popup(
+                &button.ctx,
+                button.layer_id,
+                &self.menu_state,
+                self.id,
+                add_contents,
+            );
             let menu_state = self.menu_state.read();
 
             let escape_pressed = button.ctx.input(|i| i.key_pressed(Key::Escape));
@@ -580,10 +597,10 @@ impl SubMenu {
         self.parent_state
             .write()
             .submenu_button_interaction(ui, sub_id, &response);
-        let inner = self
-            .parent_state
-            .write()
-            .show_submenu(ui.ctx(), sub_id, add_contents);
+        let inner =
+            self.parent_state
+                .write()
+                .show_submenu(ui.ctx(), ui.layer_id(), sub_id, add_contents);
         InnerResponse::new(inner, response)
     }
 }
@@ -624,11 +641,12 @@ impl MenuState {
     fn show_submenu<R>(
         &mut self,
         ctx: &Context,
+        parent_layer: LayerId,
         id: Id,
         add_contents: impl FnOnce(&mut Ui) -> R,
     ) -> Option<R> {
         let (sub_response, response) = self.submenu(id).map(|sub| {
-            let inner_response = menu_popup(ctx, sub, id, add_contents);
+            let inner_response = menu_popup(ctx, parent_layer, sub, id, add_contents);
             (sub.read().response, inner_response.inner)
         })?;
         self.cascade_close_response(sub_response);

crates/egui/src/response.rs

@@ -2,8 +2,8 @@ use std::{any::Any, sync::Arc};
 
 use crate::{
     emath::{Align, Pos2, Rect, Vec2},
-    menu, AreaState, ComboBox, Context, CursorIcon, Id, LayerId, Order, PointerButton, Sense, Ui,
-    WidgetRect, WidgetText,
+    menu, AreaState, Context, CursorIcon, Id, LayerId, Order, PointerButton, Sense, Ui, WidgetRect,
+    WidgetText,
 };
 
 // ----------------------------------------------------------------------------
@@ -601,9 +601,19 @@ impl Response {
             return true;
         }
 
-        let is_tooltip_open = self.is_tooltip_open();
+        let any_open_popups = self.ctx.prev_frame_state(|fs| {
+            fs.layers
+                .get(&self.layer_id)
+                .map_or(false, |layer| !layer.open_popups.is_empty())
+        });
+        if any_open_popups {
+            // Hide tooltips if the user opens a popup (menu, combo-box, etc) in the same layer.
+            return false;
+        }
 
-        if is_tooltip_open {
+        let is_our_tooltip_open = self.is_tooltip_open();
+
+        if is_our_tooltip_open {
             let (pointer_pos, pointer_dir) = self
                 .ctx
                 .input(|i| (i.pointer.hover_pos(), i.pointer.direction()));
@@ -647,11 +657,11 @@ impl Response {
 
         let is_other_tooltip_open = self.ctx.prev_frame_state(|fs| {
             if let Some(already_open_tooltip) = fs
-                .tooltip_state
-                .per_layer_tooltip_widget
+                .layers
                 .get(&self.layer_id)
+                .and_then(|layer| layer.widget_with_tooltip)
             {
-                already_open_tooltip != &self.id
+                already_open_tooltip != self.id
             } else {
                 false
             }
@@ -670,21 +680,6 @@ impl Response {
             return false;
         }
 
-        if self.context_menu_opened() {
-            return false;
-        }
-
-        if ComboBox::is_open(&self.ctx, self.id) {
-            return false; // Don't cover the open ComboBox with a tooltip
-        }
-
-        let when_was_a_toolip_last_shown_id = Id::new("when_was_a_toolip_last_shown");
-        let now = self.ctx.input(|i| i.time);
-
-        let when_was_a_toolip_last_shown = self
-            .ctx
-            .data(|d| d.get_temp::<f64>(when_was_a_toolip_last_shown_id));
-
         let tooltip_delay = self.ctx.style().interaction.tooltip_delay;
         let tooltip_grace_time = self.ctx.style().interaction.tooltip_grace_time;
 
@@ -693,10 +688,10 @@ impl Response {
         // another widget should show the tooltip for that widget right away.
 
         // Let the user quickly move over some dead space to hover the next thing
-        let tooltip_was_recently_shown = when_was_a_toolip_last_shown
-            .map_or(false, |time| ((now - time) as f32) < tooltip_grace_time);
+        let tooltip_was_recently_shown =
+            crate::popup::seconds_since_last_tooltip(&self.ctx) < tooltip_grace_time;
 
-        if !tooltip_was_recently_shown && !is_tooltip_open {
+        if !tooltip_was_recently_shown && !is_our_tooltip_open {
             if self.ctx.style().interaction.show_tooltips_only_when_still {
                 // We only show the tooltip when the mouse pointer is still.
                 if !self.ctx.input(|i| i.pointer.is_still()) {
@@ -729,10 +724,6 @@ impl Response {
 
         // All checks passed: show the tooltip!
 
-        // Remember that we're showing a tooltip
-        self.ctx
-            .data_mut(|data| data.insert_temp::<f64>(when_was_a_toolip_last_shown_id, now));
-
         true
     }