Skip to content

Commit

Permalink
Mostly fix hover tooltips not respecting occlusion (#24319)
Browse files Browse the repository at this point in the history 
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 <[email protected]>
  • Loading branch information
@mgsloan @probably-neb
mgsloan and probably-neb authored Feb 5, 2025
1 parent e1919b4 commit 1dbca5d
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 13 deletions.
39 changes: 31 additions & 8 deletions crates/gpui/src/elements/div.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
);
}
Expand Down Expand Up @@ -2361,6 +2370,7 @@ pub(crate) fn register_tooltip_mouse_handlers(
tooltip_id: Option<TooltipId>,
build_tooltip: Rc<dyn Fn(&mut Window, &mut App) -> Option<(AnyView, bool)>>,
check_is_hovered: Rc<dyn Fn(&Window) -> bool>,
check_is_hovered_during_prepaint: Rc<dyn Fn(&Window) -> bool>,
window: &mut Window,
) {
window.on_mouse_event({
Expand All @@ -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,
Expand All @@ -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<RefCell<Option<ActiveTooltip>>>,
build_tooltip: &Rc<dyn Fn(&mut Window, &mut App) -> Option<(AnyView, bool)>>,
check_is_hovered: &Rc<dyn Fn(&Window) -> bool>,
check_is_hovered_during_prepaint: &Rc<dyn Fn(&Window) -> bool>,
phase: DispatchPhase,
window: &mut Window,
cx: &mut App,
Expand Down Expand Up @@ -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| {
Expand All @@ -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,
Expand Down
25 changes: 20 additions & 5 deletions crates/gpui/src/elements/text.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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())
Expand All @@ -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,
);
}
Expand Down

0 comments on commit 1dbca5d

Please sign in to comment.