From c11bde7bf4e428be7639aaf9ffc7ea3dc5becf16 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Wed, 1 Jan 2025 20:47:10 +0200 Subject: [PATCH] Remove stuck tooltips (#22548) Closes https://github.com/zed-industries/zed/issues/21657 Follow-up of https://github.com/zed-industries/zed/pull/22488 Previous PR broke git blame tooltips, which are expected to be open when hovered, even if the mouse cursor is moved away from the actual blame entry that caused the tooltip to appear. Current version moves the invalidation logic into `prepaint_tooltip`, where the new data about the tooltip origin is used to ensure we invalidate only tooltips that have no mouse cursor in either origin bounds or tooltip bounds (if it's hoverable). Release Notes: - Fixed tooltips getting stuck --- crates/gpui/src/app.rs | 12 +++++++++--- crates/gpui/src/elements/div.rs | 3 +++ crates/gpui/src/elements/text.rs | 3 +++ crates/gpui/src/window.rs | 13 +++++++++++++ 4 files changed, 28 insertions(+), 3 deletions(-) diff --git a/crates/gpui/src/app.rs b/crates/gpui/src/app.rs index db5036102b..96e584274b 100644 --- a/crates/gpui/src/app.rs +++ b/crates/gpui/src/app.rs @@ -33,9 +33,9 @@ use util::ResultExt; use crate::{ current_platform, hash, init_app_menus, Action, ActionRegistry, Any, AnyView, AnyWindowHandle, - Asset, AssetSource, BackgroundExecutor, ClipboardItem, Context, DispatchPhase, DisplayId, - Entity, EventEmitter, FocusHandle, FocusId, ForegroundExecutor, Global, KeyBinding, Keymap, - Keystroke, LayoutId, Menu, MenuItem, OwnedMenu, PathPromptOptions, Pixels, Platform, + Asset, AssetSource, BackgroundExecutor, Bounds, ClipboardItem, Context, DispatchPhase, + DisplayId, Entity, EventEmitter, FocusHandle, FocusId, ForegroundExecutor, Global, KeyBinding, + Keymap, Keystroke, LayoutId, Menu, MenuItem, OwnedMenu, PathPromptOptions, Pixels, Platform, PlatformDisplay, Point, PromptBuilder, PromptHandle, PromptLevel, Render, RenderablePromptHandle, Reservation, ScreenCaptureSource, SharedString, SubscriberSet, Subscription, SvgRenderer, Task, TextSystem, View, ViewContext, Window, WindowAppearance, @@ -1612,6 +1612,12 @@ pub struct AnyTooltip { /// The absolute position of the mouse when the tooltip was deployed. pub mouse_position: Point, + + /// Whether the tooltitp can be hovered or not. + pub hoverable: bool, + + /// Bounds of the element that triggered the tooltip appearance. + pub origin_bounds: Bounds, } /// A keystroke event, and potentially the associated action diff --git a/crates/gpui/src/elements/div.rs b/crates/gpui/src/elements/div.rs index 36f42c137c..755ffabf16 100644 --- a/crates/gpui/src/elements/div.rs +++ b/crates/gpui/src/elements/div.rs @@ -1923,6 +1923,7 @@ impl Interactivity { cx.on_mouse_event({ let active_tooltip = active_tooltip.clone(); let hitbox = hitbox.clone(); + let source_bounds = hitbox.bounds; let tooltip_id = self.tooltip_id; move |_: &MouseMoveEvent, phase, cx| { let is_hovered = @@ -1952,6 +1953,8 @@ impl Interactivity { tooltip: Some(AnyTooltip { view: build_tooltip(cx), mouse_position: cx.mouse_position(), + hoverable: tooltip_is_hoverable, + origin_bounds: source_bounds, }), _task: None, }); diff --git a/crates/gpui/src/elements/text.rs b/crates/gpui/src/elements/text.rs index caa0c605d6..489a798014 100644 --- a/crates/gpui/src/elements/text.rs +++ b/crates/gpui/src/elements/text.rs @@ -675,6 +675,7 @@ impl Element for InteractiveText { if let Some(tooltip_builder) = self.tooltip_builder.clone() { let hitbox = hitbox.clone(); + let source_bounds = hitbox.bounds; let active_tooltip = interactive_state.active_tooltip.clone(); let pending_mouse_down = interactive_state.mouse_down_index.clone(); let text_layout = text_layout.clone(); @@ -708,6 +709,8 @@ impl Element for InteractiveText { tooltip: Some(AnyTooltip { view: tooltip, mouse_position: cx.mouse_position(), + hoverable: true, + origin_bounds: source_bounds, }), _task: None, } diff --git a/crates/gpui/src/window.rs b/crates/gpui/src/window.rs index 222acb386c..86c2232279 100644 --- a/crates/gpui/src/window.rs +++ b/crates/gpui/src/window.rs @@ -1557,6 +1557,19 @@ impl<'a> WindowContext<'a> { let tooltip_size = element.layout_as_root(AvailableSpace::min_size(), self); let mut tooltip_bounds = Bounds::new(mouse_position + point(px(1.), px(1.)), tooltip_size); + // Element's parent can get hidden (e.g. via the `visible_on_hover` method), + // and element's `paint` won't be called (ergo, mouse listeners also won't be active) to detect that the tooltip has to be removed. + // Ensure it's not stuck around in such cases. + let invalidate_tooltip = !tooltip_request + .tooltip + .origin_bounds + .contains(&self.mouse_position()) + && (!tooltip_request.tooltip.hoverable + || !tooltip_bounds.contains(&self.mouse_position())); + if invalidate_tooltip { + return None; + } + let window_bounds = Bounds { origin: Point::default(), size: self.viewport_size(),