From 1dbca5d9a09cbcf719d3123d20e8191707d1f3a9 Mon Sep 17 00:00:00 2001 From: Michael Sloan Date: Wed, 5 Feb 2025 16:08:56 -0700 Subject: [PATCH] Mostly fix hover tooltips not respecting occlusion (#24319) Regression in #22644 Unfortunately not a full fix, In the case where a tooltip gets displayed and then gets occluded after display, it will stick around until the mouse exits the hover bounds. Release Notes: - N/A Co-authored-by: Ben --- crates/gpui/src/elements/div.rs | 39 +++++++++++++++++++++++++------- crates/gpui/src/elements/text.rs | 25 ++++++++++++++++---- 2 files changed, 51 insertions(+), 13 deletions(-) diff --git a/crates/gpui/src/elements/div.rs b/crates/gpui/src/elements/div.rs index f9ff17ea3c8af0..6460172168c255 100644 --- a/crates/gpui/src/elements/div.rs +++ b/crates/gpui/src/elements/div.rs @@ -2008,18 +2008,27 @@ impl Interactivity { let build_tooltip = Rc::new(move |window: &mut Window, cx: &mut App| { Some(((tooltip_builder.build)(window, cx), tooltip_is_hoverable)) }); - // Use bounds instead of testing hitbox since check_is_hovered is also called - // during prepaint. - let source_bounds = hitbox.bounds; - let check_is_hovered = Rc::new(move |window: &Window| { - pending_mouse_down.borrow().is_none() - && source_bounds.contains(&window.mouse_position()) + // Use bounds instead of testing hitbox since this is called during prepaint. + let check_is_hovered_during_prepaint = Rc::new({ + let pending_mouse_down = pending_mouse_down.clone(); + let source_bounds = hitbox.bounds; + move |window: &Window| { + pending_mouse_down.borrow().is_none() + && source_bounds.contains(&window.mouse_position()) + } + }); + let check_is_hovered = Rc::new({ + let hitbox = hitbox.clone(); + move |window: &Window| { + pending_mouse_down.borrow().is_none() && hitbox.is_hovered(window) + } }); register_tooltip_mouse_handlers( &active_tooltip, self.tooltip_id, build_tooltip, check_is_hovered, + check_is_hovered_during_prepaint, window, ); } @@ -2361,6 +2370,7 @@ pub(crate) fn register_tooltip_mouse_handlers( tooltip_id: Option, build_tooltip: Rc Option<(AnyView, bool)>>, check_is_hovered: Rc bool>, + check_is_hovered_during_prepaint: Rc bool>, window: &mut Window, ) { window.on_mouse_event({ @@ -2372,6 +2382,7 @@ pub(crate) fn register_tooltip_mouse_handlers( &active_tooltip, &build_tooltip, &check_is_hovered, + &check_is_hovered_during_prepaint, phase, window, cx, @@ -2398,10 +2409,22 @@ pub(crate) fn register_tooltip_mouse_handlers( }); } +/// Handles displaying tooltips when an element is hovered. +/// +/// The mouse hovering logic also relies on being called from window prepaint in order to handle the +/// case where the element the tooltip is on is not rendered - in that case its mouse listeners are +/// also not registered. During window prepaint, the hitbox information is not available, so +/// `check_is_hovered_during_prepaint` is used which bases the check off of the absolute bounds of +/// the element. +/// +/// TODO: There's a minor bug due to the use of absolute bounds while checking during prepaint - it +/// does not know if the hitbox is occluded. In the case where a tooltip gets displayed and then +/// gets occluded after display, it will stick around until the mouse exits the hover bounds. fn handle_tooltip_mouse_move( active_tooltip: &Rc>>, build_tooltip: &Rc Option<(AnyView, bool)>>, check_is_hovered: &Rc bool>, + check_is_hovered_during_prepaint: &Rc bool>, phase: DispatchPhase, window: &mut Window, cx: &mut App, @@ -2447,7 +2470,7 @@ fn handle_tooltip_mouse_move( let delayed_show_task = window.spawn(cx, { let active_tooltip = active_tooltip.clone(); let build_tooltip = build_tooltip.clone(); - let check_is_hovered = check_is_hovered.clone(); + let check_is_hovered_during_prepaint = check_is_hovered_during_prepaint.clone(); move |mut cx| async move { cx.background_executor().timer(TOOLTIP_SHOW_DELAY).await; cx.update(|window, cx| { @@ -2463,7 +2486,7 @@ fn handle_tooltip_mouse_move( handle_tooltip_check_visible_and_update( &active_tooltip, tooltip_is_hoverable, - &check_is_hovered, + &check_is_hovered_during_prepaint, tooltip_bounds, window, cx, diff --git a/crates/gpui/src/elements/text.rs b/crates/gpui/src/elements/text.rs index 36771ed5ecc567..21913af93f2770 100644 --- a/crates/gpui/src/elements/text.rs +++ b/crates/gpui/src/elements/text.rs @@ -734,7 +734,6 @@ impl Element for InteractiveText { if let Some(tooltip_builder) = self.tooltip_builder.clone() { let active_tooltip = interactive_state.active_tooltip.clone(); - let pending_mouse_down = interactive_state.mouse_down_index.clone(); let build_tooltip = Rc::new({ let tooltip_is_hoverable = false; let text_layout = text_layout.clone(); @@ -746,11 +745,12 @@ impl Element for InteractiveText { .map(|view| (view, tooltip_is_hoverable)) } }); - // Use bounds instead of testing hitbox since check_is_hovered is also - // called during prepaint. - let source_bounds = hitbox.bounds; - let check_is_hovered = Rc::new({ + + // Use bounds instead of testing hitbox since this is called during prepaint. + let check_is_hovered_during_prepaint = Rc::new({ + let source_bounds = hitbox.bounds; let text_layout = text_layout.clone(); + let pending_mouse_down = interactive_state.mouse_down_index.clone(); move |window: &Window| { text_layout .index_for_position(window.mouse_position()) @@ -759,11 +759,26 @@ impl Element for InteractiveText { && pending_mouse_down.get().is_none() } }); + + let check_is_hovered = Rc::new({ + let hitbox = hitbox.clone(); + let text_layout = text_layout.clone(); + let pending_mouse_down = interactive_state.mouse_down_index.clone(); + move |window: &Window| { + text_layout + .index_for_position(window.mouse_position()) + .is_ok() + && hitbox.is_hovered(window) + && pending_mouse_down.get().is_none() + } + }); + register_tooltip_mouse_handlers( &active_tooltip, self.tooltip_id, build_tooltip, check_is_hovered, + check_is_hovered_during_prepaint, window, ); }