From ff0ac948498361eb6524c77bcbee149242e1d158 Mon Sep 17 00:00:00 2001 From: Vincent Bernardoff Date: Sat, 17 Oct 2026 18:08:47 +0200 Subject: [PATCH] fix: keep message action bars stable under the pointer Track pointer geometry during mouse capture so the action bar's own occlusion does not clear its message's hover state. Recheck bounds when the pane loses hover, preserving ownership over the raised buttons and clearing it when the pointer leaves. Keep button events unconsumed. Cover toolbar crossings and stationary reevaluation with a regression and document the hover region. All three message action tests pass. Co-Authored-By: GPT-6 --- crates/tinyclient/src/gui/conversation.rs | 18 ++++-- .../src/gui/conversation/actions.rs | 63 ++++++++++++------- docs/design/polish/01-system.md | 5 ++ 3 files changed, 59 insertions(+), 27 deletions(-) diff --git a/crates/tinyclient/src/gui/conversation.rs b/crates/tinyclient/src/gui/conversation.rs index da7347a..5a78578 100644 --- a/crates/tinyclient/src/gui/conversation.rs +++ b/crates/tinyclient/src/gui/conversation.rs @@ -1145,7 +1145,17 @@ impl Render for Pane { .size_full(); let after = gpui::canvas( |_, _, _| (), - move |_, (), window, cx| this.update(cx, |this, cx| this.after_layout(window, cx)), + move |_, (), window, cx| { + this.update(cx, |this, cx| this.after_layout(window, cx)); + let pane = this.clone(); + // Track geometry before hit testing/occlusion can hide the pane behind + // its own toolbar. Do not consume the event: buttons still receive it. + window.on_mouse_event(move |event: &gpui::MouseMoveEvent, phase, _, cx| { + if phase == gpui::DispatchPhase::Capture { + pane.update(cx, |this, cx| this.point_at(event.position, cx)); + } + }); + }, ) .absolute() .size_0(); @@ -1173,10 +1183,10 @@ impl Render for Pane { } }), ) - .on_mouse_move(cx.listener(|this, event: &gpui::MouseMoveEvent, _, cx| this.point_at(event.position, cx))) - .on_hover(cx.listener(|this, hovered: &bool, _, cx| { + .on_hover(cx.listener(|this, hovered: &bool, window, cx| { if !*hovered { - this.unpoint(cx); + // Occlusion is not a pointer exit; recheck the row/toolbar bounds. + this.point_at(window.mouse_position(), cx); } })) .child(rows) diff --git a/crates/tinyclient/src/gui/conversation/actions.rs b/crates/tinyclient/src/gui/conversation/actions.rs index ac736fa..091ab9b 100644 --- a/crates/tinyclient/src/gui/conversation/actions.rs +++ b/crates/tinyclient/src/gui/conversation/actions.rs @@ -24,6 +24,20 @@ use std::rc::Rc; const TOOLBAR_RISE: f32 = 14.; const TOOLBAR_REACH: f32 = 320.; +/// Hover ownership is geometric: the toolbar occludes the pane's hitbox, but +/// crossing onto its buttons must keep the current hover owner. +fn pointed_row(position: Point, viewport: Bounds, seen: &[(usize, Bounds)], held: Option) -> Option { + if !viewport.contains(&position) { return None; } + let toolbar = held.and_then(|held| seen.iter().find(|(ix, bounds)| { + let rise = Bounds::from_corners( + point(bounds.right() - px(TOOLBAR_REACH), bounds.top() - px(TOOLBAR_RISE)), + point(bounds.right(), bounds.top() + px(TOOLBAR_RISE + 8.)), + ); + *ix == held && rise.contains(&position) + })); + toolbar.or_else(|| seen.iter().find(|(_, bounds)| bounds.contains(&position))).map(|(ix, _)| *ix) +} + /// The context menu of a message, where the pointer was. pub struct Menu { pub message: Rc, @@ -197,36 +211,19 @@ impl Pane { if self.script.holds_hover() { return; } - let viewport = self.list.viewport_bounds(); - let over = viewport.contains(&position).then(|| { + let over = { let seen = self.seen.borrow(); - let row = |bounds: &Bounds| bounds.contains(&position); - // The toolbar rises over the row above; the pointer on it is still on its own row. - let held = self.hover.as_ref().and_then(|id| { - seen.iter().find(|(ix, bounds)| { - let rise = Bounds::from_corners( - point(bounds.right() - px(TOOLBAR_REACH), bounds.top() - px(TOOLBAR_RISE)), - point(bounds.right(), bounds.top() + px(TOOLBAR_RISE + 8.)), - ); - rise.contains(&position) && self.items.get(*ix).and_then(Item::message).is_some_and(|m| m.id == *id) - }) - }); - let found = held.or_else(|| seen.iter().find(|(_, bounds)| row(bounds))); - found.and_then(|(ix, _)| self.items.get(*ix)).and_then(Item::message).filter(|m| self.can(m).act).map(|m| m.id.clone()) - }); - let over = over.flatten(); + let held = self.hover.as_ref().and_then(|id| seen.iter().find(|(ix, _)| self.items.get(*ix).and_then(Item::message).is_some_and(|m| m.id == *id))).map(|(ix, _)| *ix); + pointed_row(position, self.list.viewport_bounds(), &seen, held) + .and_then(|ix| self.items.get(ix)).and_then(Item::message) + .filter(|m| self.can(m).act).map(|m| m.id.clone()) + }; if over != self.hover { self.hover = over; cx.notify(); } } - pub(super) fn unpoint(&mut self, cx: &mut Context) { - if self.menu.is_none() && self.picker.is_none() && self.hover.take().is_some() { - cx.notify(); - } - } - // The toolbar. /// A menu, picker, dialog, viewer or a window-level layer is up: no toolbar shows. @@ -731,6 +728,26 @@ impl Pane { mod tests { use super::*; + #[test] + fn crossing_onto_toolbar_buttons_keeps_the_same_message() { + let viewport = Bounds::from_corners(point(px(0.), px(0.)), point(px(800.), px(600.))); + let seen = [ + (0, Bounds::from_corners(point(px(0.), px(100.)), point(px(800.), px(150.)))), + (1, Bounds::from_corners(point(px(0.), px(150.)), point(px(800.), px(200.)))), + ]; + let mut held = pointed_row(point(px(400.), px(175.)), viewport, &seen, None); + assert_eq!(held, Some(1)); + for position in [point(px(700.), px(151.)), point(px(700.), px(140.)), point(px(760.), px(140.))] { + // The latter points are above the message, in the occluding toolbar. + held = pointed_row(position, viewport, &seen, held); + assert_eq!(held, Some(1)); + assert_eq!(pointed_row(position, viewport, &seen, held), held, "stationary redraw/press keeps the button's owner"); + } + assert_eq!(pointed_row(point(px(100.), px(140.)), viewport, &seen, held), Some(0)); + assert_eq!(pointed_row(point(px(801.), px(140.)), viewport, &seen, held), None); + assert_eq!(pointed_row(point(px(700.), px(-1.)), viewport, &seen, held), None); + } + #[test] fn a_message_link_is_built_from_ids_where_the_network_has_one() { let mut key = protocol::Key { network: Network::Discord, account: "1".into(), id: "C9".into() }; diff --git a/docs/design/polish/01-system.md b/docs/design/polish/01-system.md index 68ff515..8636479 100644 --- a/docs/design/polish/01-system.md +++ b/docs/design/polish/01-system.md @@ -61,6 +61,11 @@ toolbar hides. Destructive items sit last after a separator, `danger` text, `dan the only item (`menu-net` gains "Open in Discord" and "Account settings…" above "Sign out"). **Popover**: S§3, 200–360 wide (pickers declare their own size), 6 from the anchor, no arrow. +**Message action bar.** Hover ownership follows pointer geometry across the +message and its raised action bar. The bar's occlusion of the pane does not +count as leaving it. Moving onto a button or pressing it keeps the same message; +leaving the pane clears hover. Pointer tracking does not consume button events. + **Buttons.** S§3: primary, secondary, ghost, danger, icon. Accept = small primary; Decline = small secondary; no green buttons, no full-radius controls (Leave, hang up, jump-to-bottom and viewer discs become radius 6 / 8 squares). In-caption actions ("Retry") are small ghost buttons.