Menus: a row exit drops its delayed mark; icon rows keep the aim

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

src/menus/aim.rs

@@ -1,4 +1,4 @@
-//! Hover intent: open after a pause, hold while aiming at a submenu.
+//! Hover intent: pause before opening, hold while aiming.
 
 use std::time::Duration;
 
@@ -14,30 +14,30 @@ pub(super) const OPEN: Duration = Duration::from_millis(150);
 /// A row crossed toward an open submenu waits this long.
 pub(super) const HOLD: Duration = Duration::from_millis(300);
 
-/// The pointer moved over row `ix` at `depth`, a submenu row when `sub`.
+/// Pointer over a row; `sub` when it opens a submenu.
 pub(super) fn hover(
     state: &Entity<Open>,
     menu: &Menu,
-    (depth, ix): (usize, usize),
+    (depth, cell): (usize, (usize, usize)),
     sub: bool,
     at: Point<Pixels>,
     cx: &mut App,
 ) {
     let seen = state.update(cx, |open, _| {
         let from = open.pointer.replace(at);
-        open.over = Some((depth, ix));
+        open.over = Some((depth, cell));
         let level = open.levels.get(depth)?;
         let shown = (open.levels.get(depth + 1)).map(|level| level.panel);
-        Some((from, level.at, shown))
+        Some((from, (level.at, level.column), shown))
     });
     let Some((from, marked, shown)) = seen else {
         return;
     };
-    if marked == Some(ix) {
+    if marked == (Some(cell.0), cell.1) {
         if !sub || shown.is_some() {
             state.update(cx, |open, _| open.pending = None);
         } else if state.read(cx).pending.is_none() {
-            later(state, menu, (depth, ix), OPEN, cx);
+            later(state, menu, (depth, cell), OPEN, cx);
         }
         return;
     }
@@ -45,21 +45,30 @@ pub(super) fn hover(
         .zip(shown.filter(|panel| *panel != Bounds::default()))
         .is_some_and(|(from, panel)| toward(from, at, panel));
     if aimed {
-        log::debug!("menu: row {ix} of level {depth} waits on the aim");
-        return later(state, menu, (depth, ix), HOLD, cx);
+        log::debug!("menu: row {} of level {depth} waits on the aim", cell.0);
+        return later(state, menu, (depth, cell), HOLD, cx);
     }
     state.update(cx, |open, _| open.pending = None);
-    mark(state, menu, depth, (ix, 0), false, cx);
+    mark(state, menu, depth, cell, false, cx);
     if sub {
-        later(state, menu, (depth, ix), OPEN, cx);
+        later(state, menu, (depth, cell), OPEN, cx);
     }
 }
 
-/// Marks row `ix` after `wait`, its submenu open, if still under the pointer.
+/// The pointer left a row: its delayed mark is dropped.
+pub(super) fn leave(state: &Entity<Open>, over: (usize, (usize, usize)), cx: &mut App) {
+    state.update(cx, |open, _| {
+        if open.over == Some(over) {
+            (open.over, open.pending) = (None, None);
+        }
+    });
+}
+
+/// Marks the row after `wait` if still under the pointer.
 fn later(
     state: &Entity<Open>,
     menu: &Menu,
-    (depth, ix): (usize, usize),
+    (depth, cell): (usize, (usize, usize)),
     wait: Duration,
     cx: &mut App,
 ) {
@@ -67,17 +76,17 @@ fn later(
     let task = cx.spawn(async move |cx| {
         cx.background_executor().timer(wait).await;
         cx.update(|cx| {
-            if state_.read(cx).over != Some((depth, ix)) {
-                return log::debug!("menu: the pointer left row {ix} before it opened");
+            if state_.read(cx).over != Some((depth, cell)) {
+                return log::debug!("menu: the pointer left row {} before it opened", cell.0);
             }
             state_.update(cx, |open, _| open.pending = None);
-            mark(&state_, &menu, depth, (ix, 0), true, cx);
+            mark(&state_, &menu, depth, cell, true, cx);
         });
     });
     state.update(cx, |open, _| open.pending = Some(task));
 }
 
-/// Whether `to` lies in the triangle from `from` to `panel`'s near edge.
+/// Whether `to` lies between `from` and `panel`'s near edge.
 pub(super) fn toward(from: Point<Pixels>, to: Point<Pixels>, panel: Bounds<Pixels>) -> bool {
     let near = if panel.left() >= from.x {
         panel.left()

src/menus/draw.rs

@@ -8,7 +8,7 @@ use gpui::{
 
 use super::{
     aim,
-    menu::{Open, at_depth, choose, mark},
+    menu::{Open, at_depth, choose},
     model::{Entry, Kind, Menu, MenuItem},
 };
 use crate::{
@@ -194,17 +194,18 @@ pub(super) fn panel(
                         choose(&click, &menu, depth, (ix, column), &close, window, cx)
                     });
                     let drawn = button(item, (ix, column), current, press, cx);
+                    let left = state.clone();
+                    let here = (depth, (ix, column));
                     drawn.when(!item.disabled, |button| {
-                        button.on_mouse_move(move |_, _, cx| {
-                            hover.update(cx, |open, _| {
-                                (open.over, open.pending) = (Some((depth, ix)), None);
-                            });
-                            let open = hover.read(cx);
-                            let marked = open.levels.get(depth).map(|l| (l.at, l.column));
-                            if marked != Some((Some(ix), column)) {
-                                mark(&hover, &hover_menu, depth, (ix, column), false, cx);
-                            }
-                        })
+                        button
+                            .on_mouse_move(move |event, _, cx| {
+                                aim::hover(&hover, &hover_menu, here, false, event.position, cx)
+                            })
+                            .on_hover(move |hovered, _, cx| {
+                                if !hovered {
+                                    aim::leave(&left, here, cx);
+                                }
+                            })
                     })
                 }))
                 .into_any_element(),
@@ -218,8 +219,21 @@ pub(super) fn panel(
                 row(item, ix, marked, cx)
                     .debug_selector(move || format!("menu-row-{label}"))
                     .when(!item.disabled, |row| {
+                        let left = state.clone();
                         row.on_mouse_move(move |event, _, cx| {
-                            aim::hover(&hover, &hover_menu, (depth, ix), sub, event.position, cx)
+                            aim::hover(
+                                &hover,
+                                &hover_menu,
+                                (depth, (ix, 0)),
+                                sub,
+                                event.position,
+                                cx,
+                            )
+                        })
+                        .on_hover(move |hovered, _, cx| {
+                            if !hovered {
+                                aim::leave(&left, (depth, (ix, 0)), cx);
+                            }
                         })
                         .on_mouse_down(
                             MouseButton::Left,
@@ -241,20 +255,12 @@ pub(super) fn panel(
                     .into_any_element()
             }
         });
-    let left = state.clone();
     surface((id.clone(), format!("panel-{depth}")), cx)
         .role(Role::Menu)
         .debug_selector(move || format!("menu-panel-{depth}"))
         .relative()
         // Rows under this panel take no hover.
         .occlude()
-        .on_mouse_move(move |_, _, cx| {
-            left.update(cx, |open, _| {
-                if open.over.is_some_and(|(over, _)| over != depth) {
-                    open.over = None;
-                }
-            })
-        })
         // Presses here are the menu's, not its host's.
         .on_mouse_down(MouseButton::Left, |_, window, cx| {
             window.prevent_default();
@@ -274,7 +280,7 @@ pub(super) fn panel(
         }))
 }
 
-/// Records a box into the host's state: the host's own, a marked row's or a panel's.
+/// Records the host's, a marked row's or a panel's box.
 pub(super) fn measure(
     state: Entity<Open>,
     write: impl Fn(&mut Open, Bounds<Pixels>) -> bool + 'static,

src/menus/menu.rs

@@ -82,8 +82,8 @@ pub(crate) struct Open {
     takeover: Option<Entity<Takeover>>,
     /// The pointer's last place over a row.
     pub(super) pointer: Option<Point<Pixels>>,
-    /// The row under the pointer: depth and index.
-    pub(super) over: Option<(usize, usize)>,
+    /// The row under the pointer: depth, index and column.
+    pub(super) over: Option<(usize, (usize, usize))>,
     /// A delayed mark; dropping it cancels.
     pub(super) pending: Option<Task<()>>,
 }
@@ -105,19 +105,20 @@ impl Open {
 
     /// Opens at `spot`, with the first row marked when `marked`.
     pub(crate) fn show(state: &Entity<Open>, menu: &Menu, spot: Spot, marked: bool, cx: &mut App) {
-        let first = menu.step(None, 1).filter(|_| marked);
-        state.update(cx, |open, cx| {
-            open.spot = Some(spot);
-            open.levels = vec![first_of(menu, first)];
-            cx.notify();
-        });
+        state.update(cx, |open, _| open.spot = Some(spot));
+        Self::begin(state, menu, menu.step(None, 1).filter(|_| marked), cx);
     }
 
     /// Starts over on new rows with the first marked; the menu stays where it hangs.
     pub(crate) fn restart(state: &Entity<Open>, menu: &Menu, cx: &mut App) {
-        let first = menu.step(None, 1);
+        Self::begin(state, menu, menu.step(None, 1), cx);
+    }
+
+    /// Fresh levels marking `first`; the old hover is dropped.
+    fn begin(state: &Entity<Open>, menu: &Menu, first: Option<usize>, cx: &mut App) {
         state.update(cx, |open, cx| {
             open.levels = vec![first_of(menu, first)];
+            (open.pointer, open.over, open.pending) = (None, None, None);
             cx.notify();
         });
     }
@@ -461,7 +462,7 @@ pub(crate) fn hang(
             width if width > Pixels::ZERO => width,
             _ => cx.theme().menu_width().to_pixels(window.rem_size()),
         };
-        // No room on the right: open on the left, over nothing.
+        // No room on the right: open on the left.
         let left = row.right() + width > room && row.left() >= width;
         let (corner, from, slide) = match left {
             true => (Anchor::TopRight, row.origin, -1.0),

src/menus/tests/aim.rs

@@ -8,53 +8,89 @@ use gpui::{
 use super::settle;
 use crate::{
     menus::{ContextMenu, Menu, MenuItem, aim},
-    primitives::FocusScope,
+    primitives::{FocusScope, IconName},
     theme::Theme,
 };
 
-/// A region whose context menu has two submenus and a plain row.
+/// Two submenus and a plain row.
+fn menu() -> Menu {
+    let insert = Menu::new()
+        .item(MenuItem::new("Row above"))
+        .item(MenuItem::new("Row below"))
+        .item(MenuItem::new("Column left"));
+    let delete = Menu::new()
+        .item(MenuItem::new("Delete row"))
+        .item(MenuItem::new("Delete column"));
+    Menu::new()
+        .item(MenuItem::submenu("Insert", insert))
+        .item(MenuItem::submenu("Delete", delete))
+        .item(MenuItem::new("Plain"))
+}
+
+/// A submenu over icon buttons and a disabled row.
+fn tools() -> Menu {
+    let insert = Menu::new()
+        .item(MenuItem::new("Row above"))
+        .item(MenuItem::new("Row below"))
+        .item(MenuItem::new("Column left"))
+        .item(MenuItem::new("Column right"));
+    Menu::new()
+        .item(MenuItem::submenu("Insert", insert))
+        .buttons([
+            MenuItem::new("Cut").icon(IconName::Scissors),
+            MenuItem::new("Copy").icon(IconName::Copy),
+            MenuItem::new("Paste").icon(IconName::Clipboard),
+        ])
+        .item(MenuItem::new("Gone").disabled(true))
+        .item(MenuItem::new("Plain"))
+}
+
+/// A region whose right click opens `menu`.
 struct Region {
     root: FocusHandle,
+    menu: fn() -> Menu,
 }
 
 impl Render for Region {
     fn render(&mut self, _: &mut Window, _: &mut Context<Self>) -> impl IntoElement {
-        let insert = Menu::new()
-            .item(MenuItem::new("Row above"))
-            .item(MenuItem::new("Row below"))
-            .item(MenuItem::new("Column left"));
-        let delete = Menu::new()
-            .item(MenuItem::new("Delete row"))
-            .item(MenuItem::new("Delete column"));
-        let menu = Menu::new()
-            .item(MenuItem::submenu("Insert", insert))
-            .item(MenuItem::submenu("Delete", delete))
-            .item(MenuItem::new("Plain"));
         FocusScope::new(&self.root).size_full().child(
-            ContextMenu::new("region", menu)
+            ContextMenu::new("region", (self.menu)())
                 .size_full()
                 .child(div().size_full()),
         )
     }
 }
 
-/// A region in a window `width` wide, its menu opened at `x`.
+/// A window `width` wide, `menu` opened at `x`.
 fn region(width: f32, x: f32, cx: &mut TestAppContext) -> &mut VisualTestContext {
+    region_of(menu, width, x, cx)
+}
+
+fn region_of(
+    menu: fn() -> Menu,
+    width: f32,
+    x: f32,
+    cx: &mut TestAppContext,
+) -> &mut VisualTestContext {
     cx.update(|cx| {
         Theme::init(cx);
         crate::forms::bind_keys(cx);
     });
     let (_, cx) = cx.add_window_view(|_, cx| Region {
         root: cx.focus_handle(),
+        menu,
     });
     cx.simulate_resize(size(px(width), px(600.0)));
     settle(cx);
-    let at = point(px(x), px(20.0));
+    right_click(point(px(x), px(20.0)), cx);
+    cx
+}
+
+fn right_click(at: gpui::Point<Pixels>, cx: &mut VisualTestContext) {
     cx.simulate_mouse_move(at, None, Modifiers::none());
     cx.simulate_mouse_down(at, MouseButton::Right, Modifiers::none());
     cx.simulate_mouse_up(at, MouseButton::Right, Modifiers::none());
     settle(cx);
-    cx
 }
 
 fn bounds(name: &str, cx: &mut VisualTestContext) -> Option<Bounds<Pixels>> {
@@ -141,7 +177,7 @@ fn a_submenu_without_room_on_the_right_opens_on_the_left(cx: &mut TestAppContext
     rest_on_insert(cx);
     let parent = bounds("menu-panel-0", cx).expect("the menu");
     let sub = bounds("menu-panel-1", cx).expect("the submenu");
-    // It meets the row's edge, as on the right, sliding in.
+    // It meets the row's edge, as on the right.
     let edge = row("Insert", cx).left() + crate::motion::NUDGE;
     assert!(sub.left() < parent.left(), "{sub:?} left of {parent:?}");
     assert!(sub.right() <= edge, "{sub:?} leaves the rows of {parent:?}");
@@ -165,3 +201,83 @@ fn rows_under_a_submenu_take_no_hover(cx: &mut TestAppContext) {
         "Plain under the submenu stayed unmarked"
     );
 }
+
+#[gpui::test]
+fn leaving_a_row_drops_its_delayed_open(cx: &mut TestAppContext) {
+    let cx = region(1000.0, 40.0, cx);
+    let insert = row("Insert", cx);
+    move_to(insert.center(), cx);
+    wait(aim::OPEN / 2, cx);
+    let panel = bounds("menu-panel-0", cx).expect("the menu");
+    move_to(point(panel.left() + px(1.0), insert.center().y), cx);
+    wait(aim::OPEN, cx);
+    assert!(!shown("Row above", cx), "padding is not the row");
+    move_to(insert.center(), cx);
+    wait(aim::OPEN / 2, cx);
+    assert!(!shown("Row above", cx), "re-entry waits the full pause");
+    wait(aim::OPEN / 2, cx);
+    assert!(shown("Row above", cx));
+}
+
+#[gpui::test]
+fn leaving_the_menu_drops_the_delayed_open(cx: &mut TestAppContext) {
+    let cx = region(1000.0, 40.0, cx);
+    move_to(row("Insert", cx).center(), cx);
+    wait(aim::OPEN / 2, cx);
+    move_to(point(px(900.0), px(500.0)), cx);
+    wait(aim::OPEN, cx);
+    assert!(!shown("Row above", cx));
+}
+
+#[gpui::test]
+fn a_reopened_menu_forgets_the_old_hover(cx: &mut TestAppContext) {
+    let cx = region(1000.0, 40.0, cx);
+    move_to(row("Insert", cx).center(), cx);
+    wait(aim::OPEN / 2, cx);
+    right_click(point(px(500.0), px(300.0)), cx);
+    wait(aim::OPEN, cx);
+    assert!(!shown("Row above", cx));
+}
+
+#[gpui::test]
+fn leaving_a_crossed_row_keeps_the_submenu(cx: &mut TestAppContext) {
+    let cx = region(1000.0, 40.0, cx);
+    rest_on_insert(cx);
+    let delete = row("Delete", cx);
+    move_to(point(delete.right() - px(16.0), delete.center().y), cx);
+    move_to(point(px(900.0), px(500.0)), cx);
+    wait(aim::HOLD, cx);
+    assert!(shown("Row above", cx), "Delete was abandoned");
+    assert!(!shown("Delete row", cx));
+}
+
+#[gpui::test]
+fn a_disabled_row_drops_the_delayed_open(cx: &mut TestAppContext) {
+    let cx = region_of(tools, 1000.0, 40.0, cx);
+    move_to(row("Insert", cx).center(), cx);
+    wait(aim::OPEN / 2, cx);
+    move_to(row("Gone", cx).center(), cx);
+    wait(aim::OPEN, cx);
+    assert!(!shown("Row above", cx));
+}
+
+#[gpui::test]
+fn aiming_across_icon_buttons_holds_the_submenu(cx: &mut TestAppContext) {
+    let cx = region_of(tools, 1000.0, 40.0, cx);
+    let insert = row("Insert", cx);
+    let from = point(insert.left() + px(4.0), insert.center().y);
+    move_to(from, cx);
+    wait(aim::OPEN, cx);
+    let sub = bounds("menu-panel-1", cx).expect("Insert's submenu");
+    let paste = bounds("menu-button-1-2", cx).expect("the paste button");
+    assert!(aim::toward(from, paste.center(), sub), "the crossing aims");
+    move_to(paste.center(), cx);
+    assert!(shown("Row above", cx), "crossing a button keeps it");
+    move_to(row("Row below", cx).center(), cx);
+    wait(aim::HOLD, cx);
+    assert!(shown("Row above", cx), "the submenu was reached");
+    move_to(from, cx);
+    move_to(paste.center(), cx);
+    wait(aim::HOLD, cx);
+    assert!(!shown("Row above", cx), "resting on the button marks it");
+}