diff --git a/crates/egui/src/containers/area.rs b/crates/egui/src/containers/area.rs index 4b3a4f722..61375b580 100644 --- a/crates/egui/src/containers/area.rs +++ b/crates/egui/src/containers/area.rs @@ -454,14 +454,12 @@ impl Area { state.size = None; } state.pivot = pivot; - state.interactable = interactable; if let Some(new_pos) = new_pos { state.pivot_pos = Some(new_pos); } state.pivot_pos.get_or_insert_with(|| { default_pos.unwrap_or_else(|| automatic_area_position(ctx, constrain_rect, layer_id)) }); - state.interactable = interactable; let size = *state.size.get_or_insert_with(|| { sizing_pass = true; @@ -484,6 +482,10 @@ impl Area { size }); + // We should never be interactable during a sizing pass, since then we are shown at a different + // size which might interfere with hover state of the hovered widget causing popup feedback loops. + state.interactable = interactable && !sizing_pass; + // TODO(emilk): if last frame was sizing pass, it should be considered invisible for smoother fade-in let visible_last_frame = ctx.memory(|mem| mem.areas().visible_last_frame(&layer_id)); diff --git a/crates/egui/src/containers/popup.rs b/crates/egui/src/containers/popup.rs index 080c00bd1..587c77e23 100644 --- a/crates/egui/src/containers/popup.rs +++ b/crates/egui/src/containers/popup.rs @@ -180,6 +180,7 @@ pub struct Popup<'a> { /// Default width passed to the Area width: Option, sense: Sense, + interactable: bool, layout: Layout, frame: Option, style: StyleModifier, @@ -202,6 +203,7 @@ impl<'a> Popup<'a> { gap: 0.0, width: None, sense: Sense::click(), + interactable: true, layout: Layout::default(), frame: None, style: StyleModifier::default(), @@ -369,6 +371,15 @@ impl<'a> Popup<'a> { self } + /// If `false`, the pointer goes straight through the popup and it's widgets to whatever is behind it. + /// + /// Default: `true`. + #[inline] + pub fn interactable(mut self, interactable: bool) -> Self { + self.interactable = interactable; + self + } + /// Set the sense of the popup. #[inline] pub fn sense(mut self, sense: Sense) -> Self { @@ -546,6 +557,7 @@ impl<'a> Popup<'a> { gap, width, sense, + interactable, layout, frame, style, @@ -570,6 +582,7 @@ impl<'a> Popup<'a> { .pivot(pivot) .fixed_pos(anchor) .sense(sense) + .interactable(interactable) .layout(layout) .sizing_pass(!was_open_last_frame) .info(info.unwrap_or_else(|| { diff --git a/crates/egui/src/containers/tooltip.rs b/crates/egui/src/containers/tooltip.rs index 22c319569..a372d6cd1 100644 --- a/crates/egui/src/containers/tooltip.rs +++ b/crates/egui/src/containers/tooltip.rs @@ -129,7 +129,15 @@ impl Tooltip<'_> { }); let tooltip_area_id = Self::tooltip_id(parent_widget, state.tooltip_count); - popup = popup.anchor(state.bounding_rect).id(tooltip_area_id); + + // Tooltips without interactive contents should not be interactable (hover should pass + // through to the widget below). + let interactable = Self::had_interactive_widgets(popup.ctx(), tooltip_area_id); + + popup = popup + .anchor(state.bounding_rect) + .id(tooltip_area_id) + .interactable(interactable); let response = popup.show(|ui| { // By default, the text in tooltips aren't selectable. @@ -192,6 +200,20 @@ impl Tooltip<'_> { widget_id.with(tooltip_count) } + /// Did this tooltip contain anything the user can interact with, last pass? + /// + /// Most tooltips are just text. Those should not react to the pointer at all, + /// or they would steal the hover from the widget they belong to. + fn had_interactive_widgets(ctx: &Context, tooltip_id: Id) -> bool { + let tooltip_layer_id = LayerId::new(Order::Tooltip, tooltip_id); + ctx.viewport(|vp| { + vp.prev_pass + .widgets + .get_layer(tooltip_layer_id) + .any(|w| w.enabled && w.sense.interactive()) + }) + } + /// Should we show a tooltip for this response? /// /// Argument `allow_interactive_tooltip` controls whether mouse can interact with tooltip that @@ -247,15 +269,9 @@ impl Tooltip<'_> { // Check if we should automatically stay open: let tooltip_id = Self::next_tooltip_id(&response.ctx, response.id); - let tooltip_layer_id = LayerId::new(Order::Tooltip, tooltip_id); let tooltip_has_interactive_widget = allow_interactive_tooltip - && response.ctx.viewport(|vp| { - vp.prev_pass - .widgets - .get_layer(tooltip_layer_id) - .any(|w| w.enabled && w.sense.interactive()) - }); + && Self::had_interactive_widgets(&response.ctx, tooltip_id); if tooltip_has_interactive_widget { // We keep the tooltip open if hovered, diff --git a/crates/egui/src/context.rs b/crates/egui/src/context.rs index 78f6ead11..e0705b201 100644 --- a/crates/egui/src/context.rs +++ b/crates/egui/src/context.rs @@ -466,7 +466,13 @@ impl ContextImpl { viewport.this_pass.begin_pass(); { - let mut layers: Vec = viewport.prev_pass.widgets.layer_ids().collect(); + // Areas that are not interactable are click-through: skip them in the hit-test. + let mut layers: Vec = viewport + .prev_pass + .widgets + .layer_ids() + .filter(|layer_id| self.memory.areas().is_interactable(*layer_id)) + .collect(); layers.sort_by(|&a, &b| self.memory.areas().compare_order(a, b)); viewport.hits = if let Some(pos) = viewport.input.pointer.interact_pos() { diff --git a/crates/egui/src/memory/mod.rs b/crates/egui/src/memory/mod.rs index 1f22b3dd4..8fd407cc9 100644 --- a/crates/egui/src/memory/mod.rs +++ b/crates/egui/src/memory/mod.rs @@ -1194,6 +1194,11 @@ impl Areas { self.areas.get_mut(&id) } + /// Can the user interact with this layer or it's widgets, or do clicks go straight through it? + pub(crate) fn is_interactable(&self, layer_id: LayerId) -> bool { + self.get(layer_id.id).is_none_or(|area| area.interactable) + } + /// All layers back-to-front, top is last. pub(crate) fn order(&self) -> &[LayerId] { &self.order diff --git a/tests/egui_tests/tests/regression_tests.rs b/tests/egui_tests/tests/regression_tests.rs index f250b1ab2..34527f4cc 100644 --- a/tests/egui_tests/tests/regression_tests.rs +++ b/tests/egui_tests/tests/regression_tests.rs @@ -1,15 +1,15 @@ use std::sync::Arc; -use egui::ScrollArea; use egui::accesskit::Role; #[cfg(debug_assertions)] use egui::epaint::Shape; use egui::style::ScrollAnimation; use egui::text::{LayoutJob, TextWrapping}; use egui::{ - Align, Button, Color32, FontFamily, FontId, Image, Label, Layout, RichText, Sense, TextBuffer, - TextFormat, TextWrapMode, Ui, include_image, vec2, + Align, Button, Color32, FontFamily, FontId, Image, Label, Layout, Rect, RichText, Sense, + TextBuffer, TextFormat, TextWrapMode, Ui, Vec2, include_image, vec2, }; +use egui::{Pos2, ScrollArea}; use egui_kittest::Harness; use egui_kittest::kittest::{NodeT as _, Queryable as _}; @@ -481,3 +481,81 @@ fn animated_scroll_beats_sticky_bottom() { "animated explicit scroll should leave the sticky bottom" ); } + +/// Tests that tooltips are shown correctly for buttons that are only shown on hover. +/// +/// Basically, this tests that a tooltip overlapping the mouse cursor does not interfere with a +/// buttons hover state. +#[test] +fn tooltip_should_work_for_hover_button() { + let button_rect = Rect::from_min_size(Pos2::new(4.0, 4.0), Vec2::new(80.0, 20.0)); + let mut harness = Harness::builder().with_size((320.0, 80.0)).build_ui(|ui| { + if ui.rect_contains_pointer(button_rect) { + ui.button("A tooltip should be shown") + .on_hover_text("My tooltip"); + } + }); + + harness.hover_at(button_rect.center()); + + harness.run(); + + harness.snapshot("test_tooltip_hover_regression"); +} + +/// Ensure that hovering close to a widget doesn't cause a tooltip feedback loop (due to a +/// difference between `hovered` and `contains_pointer` caused by the interact radius). +#[test] +fn tooltip_covering_button_should_not_cause_feedback_loop() { + let mut harness = Harness::builder().with_size((200.0, 30.0)).build_ui(|ui| { + ui.button("A tooltip should be shown") + .on_hover_text("This tooltip is larger than the button"); + }); + + harness.hover_at( + harness + .get_by_label("A tooltip should be shown") + .rect() + .left_center() + - Vec2::X, + ); + + harness.run(); + + harness.snapshot("tooltip_covering_button_should_not_cause_feedback_loop"); +} + +/// Tests that a tooltip closes when the pointer moves onto a neighboring widget, +/// so that the neighbor can show its own tooltip. +/// +/// The two buttons are only `item_spacing.y` (3 pt) apart, which is less than the +/// hit-test `interact_radius` (5 pt), so the first button is still close enough to +/// interact with when the pointer is on the second one. +#[test] +fn tooltip_should_hand_over_to_neighboring_widget() { + let mut harness = Harness::builder().with_size((300.0, 200.0)).build_ui(|ui| { + ui.button("Button A").on_hover_text("Tooltip A"); + ui.button("Button B").on_hover_text("Tooltip B"); + }); + + let a_rect = harness.get_by_label("Button A").rect(); + let b_rect = harness.get_by_label("Button B").rect(); + + harness.hover_at(a_rect.center_bottom() - Vec2::Y); + harness.run(); + assert!( + harness.query_by_label("Tooltip A").is_some(), + "Tooltip A should be shown when hovering Button A" + ); + + harness.hover_at(b_rect.center_top() + Vec2::Y); + harness.run(); + assert!( + harness.query_by_label("Tooltip B").is_some(), + "Tooltip B should be shown when hovering Button B" + ); + assert!( + harness.query_by_label("Tooltip A").is_none(), + "Tooltip A should be hidden when hovering Button B" + ); +} diff --git a/tests/egui_tests/tests/snapshots/test_tooltip_hover_regression.png b/tests/egui_tests/tests/snapshots/test_tooltip_hover_regression.png new file mode 100644 index 000000000..9f489aa81 --- /dev/null +++ b/tests/egui_tests/tests/snapshots/test_tooltip_hover_regression.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:59bef0e3593896985988c03171c710d8ed31ed8bb89c7a3f63c060c573e4fb74 +size 6510 diff --git a/tests/egui_tests/tests/snapshots/tooltip_covering_button_should_not_cause_feedback_loop.png b/tests/egui_tests/tests/snapshots/tooltip_covering_button_should_not_cause_feedback_loop.png new file mode 100644 index 000000000..53f8ff24d --- /dev/null +++ b/tests/egui_tests/tests/snapshots/tooltip_covering_button_should_not_cause_feedback_loop.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:ed2f80921ab864b0df3303834a8ab943a3aa5e10456896991c2dca81be0c4968 +size 3968