From b800ff51c05e679ec8ac933a7015f5618aa2d5fc Mon Sep 17 00:00:00 2001 From: "gcp-cherry-pick-bot[bot]" <98988430+gcp-cherry-pick-bot[bot]@users.noreply.github.com> Date: Wed, 1 Jan 2025 21:07:39 +0200 Subject: [PATCH] Remove stuck tooltips (cherry-pick #22548) (#22550) Cherry-picked 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 Co-authored-by: Kirill Bulatov --- 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 db5036102b1111936371ff53f3bb230c9bdbac08..96e584274b6401227fc1587bd8f6f2d60022acd4 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 36f42c137c1f96fb819b98fd7d7f3947e0b3cba5..755ffabf163d69074823656eeef14fe2494b9eba 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 caa0c605d6605b4f543dc142f15a4768f822d8eb..489a7980148a52b01fcbe0f51b37d25fb16bf457 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 222acb386c7bda937c8cc7651dd5069a829aae55..86c2232279642805b5cf401f914a5555595aab8a 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(),