fix(menu): keep submenus open during diagonal pointer movement (#2153)

Moving the pointer diagonally from a menu item toward its open submenu crossed sibling items, whose hover timers switched or closed the submenu.

While a submenu is open, a sibling hover is deferred when the pointer is inside the triangle from its previous position to the submenu's near corners, including flipped submenus. Stopping on a sibling still switches after 300 ms, moving away keeps the normal delays, and entering the submenu cancels pending switches. This lives in `MenuList`, so every menu and nested level gets it.

Fixes #2128

e2e/projectTree.spec.ts

@@ -50,6 +50,29 @@ test("Git ignore submenu offers shared and local destinations for files and fold
   }
 });
 
+test("keeps New open on a slow diagonal path across sibling items", async ({ workbench: page }) => {
+  await page.getByRole("treeitem", { name: "README.md", exact: true }).click({ button: "right" });
+  const parent = page.getByRole("menuitem", { name: "New", exact: true });
+  const box = (await parent.boundingBox())!;
+  const start = { x: box.x + 20, y: box.y + box.height / 2 };
+  await page.mouse.move(start.x, start.y);
+  await expect(parent).toHaveAttribute("aria-expanded", "true");
+  const submenu = page
+    .getByRole("menu")
+    .filter({ has: page.getByRole("menuitem", { name: "Directory", exact: true }) });
+  const target = (await submenu.getByRole("menuitem").last().boundingBox())!;
+  const end = { x: target.x + 20, y: target.y + target.height / 2 };
+  for (let step = 1; step <= 10; step++) {
+    await page.mouse.move(start.x + ((end.x - start.x) * step) / 10, start.y + ((end.y - start.y) * step) / 10);
+    await page.waitForTimeout(75);
+    await expect(parent).toHaveAttribute("aria-expanded", "true");
+  }
+  await page.waitForTimeout(350);
+  await expect(parent).toHaveAttribute("aria-expanded", "true");
+  await page.keyboard.press("Escape");
+  await page.keyboard.press("Escape");
+});
+
 // macOS WebKit can deliver a tap's pointerup before its pointerdown, leaving a press with no button held (#760).
 const settle = (page: Page) =>
   page.evaluate(() => new Promise((done) => requestAnimationFrame(() => requestAnimationFrame(done))));

src/ui/Menu.tsx

@@ -21,6 +21,7 @@ import { Popup, type PopupAnchor } from "./Popup";
 import type { Placement } from "@floating-ui/react-dom";
 import styles from "./Menu.module.css";
 import { tx } from "@/platform/i18n";
+import { isSubmenuAim, type MenuPointer } from "./menuAim";
 
 export type MenuItem =
   | { kind: "separator" }
@@ -125,6 +126,10 @@ export function MenuList({ items, onDone, onCloseLevel, autoFocusFirst = true, .
   const [subAnchor, setSubAnchor] = useState<HTMLElement | null>(null);
   const itemRefs = useRef<Array<HTMLButtonElement | null>>([]);
   const hoverTimer = useRef<ReturnType<typeof setTimeout> | undefined>(undefined);
+  const submenuRef = useRef<HTMLDivElement | null>(null);
+  const pointer = useRef<MenuPointer | null>(null);
+  const previousPointer = useRef<MenuPointer | null>(null);
+  const pendingAim = useRef<number | null>(null);
   // Pointer movement highlights a row without focusing it. Focusing on every
   // enter drops the hover paint in the Windows webview, so the menu looks frozen.
   const pointerHover = useRef(false);
@@ -163,6 +168,7 @@ export function MenuList({ items, onDone, onCloseLevel, autoFocusFirst = true, .
   };
 
   const activate = (i: number) => {
+    cancelHover();
     const item = resolved[i];
     if (!selectable(item)) return;
     if (item!.submenu) {
@@ -174,21 +180,85 @@ export function MenuList({ items, onDone, onCloseLevel, autoFocusFirst = true, .
     item!.run?.();
   };
 
-  const hover = (i: number) => {
-    const item = resolved[i];
-    if (!item || item.disabled || item.kind !== "item") return;
-    if (hovered.current !== i) {
-      hovered.current = i;
-      if (i !== active) {
-        pointerHover.current = true;
-        focusItem(i);
-      }
-    }
+  const cancelHover = () => {
     clearTimeout(hoverTimer.current);
-    hoverTimer.current = setTimeout(() => setOpenSub(item.submenu ? i : null), item.submenu ? 120 : 200);
+    pendingAim.current = null;
+  };
+
+  const applyHover = (i: number, immediate = false) => {
+    cancelHover();
+    if (i !== active) {
+      pointerHover.current = true;
+      focusItem(i);
+    }
+    const next = resolved[i]?.submenu ? i : null;
+    if (immediate) setOpenSub(next);
+    else hoverTimer.current = setTimeout(() => setOpenSub(next), next === null ? 200 : 120);
+  };
+
+  const recordPointer = (point: MenuPointer) => {
+    if (pointer.current?.x === point.x && pointer.current.y === point.y) return false;
+    previousPointer.current = pointer.current;
+    pointer.current = point;
+    return true;
+  };
+
+  const aiming = () => {
+    const rect = submenuRef.current?.parentElement?.getBoundingClientRect();
+    return !!rect && !!pointer.current && isSubmenuAim(previousPointer.current, pointer.current, rect);
+  };
+
+  const deferHover = (i: number) => {
+    cancelHover();
+    pendingAim.current = i;
+    hoverTimer.current = setTimeout(() => applyHover(i, true), 300);
+  };
+
+  const pointerMove = (point: MenuPointer) => {
+    if (!recordPointer(point) || pendingAim.current === null) return;
+    const i = pendingAim.current;
+    if (aiming()) deferHover(i);
+    else applyHover(i);
+  };
+
+  const enterSubmenu = () => {
+    cancelHover();
+    hovered.current = -1;
+    pointer.current = previousPointer.current = null;
+    if (openSub !== null && active !== openSub) {
+      pointerHover.current = true;
+      setActive(openSub);
+    }
+  };
+
+  // Track movement across the gap too; submenu portals are outside this level's DOM.
+  useEffect(() => {
+    if (openSub === null) return;
+    const onMove = (e: PointerEvent) => {
+      if (submenuRef.current?.parentElement?.contains(e.target as Node)) enterSubmenu();
+      else pointerMove({ x: e.clientX, y: e.clientY });
+    };
+    document.addEventListener("pointermove", onMove);
+    return () => document.removeEventListener("pointermove", onMove);
+  });
+
+  const hover = (i: number, point: MenuPointer) => {
+    const item = resolved[i];
+    if (!selectable(item) || hovered.current === i) return;
+    hovered.current = i;
+    recordPointer(point);
+    if (openSub !== null && openSub !== i && aiming()) deferHover(i);
+    else applyHover(i);
+  };
+
+  const leave = (i: number) => {
+    // A compatibility mouseleave can follow pointerenter on the next row.
+    if (hovered.current === i) hovered.current = -1;
   };
 
   const onKeyDown = (e: ReactKeyboardEvent) => {
+    cancelHover();
+    pointer.current = previousPointer.current = null;
     switch (e.key) {
       case "ArrowDown":
         move(1);
@@ -235,7 +305,15 @@ export function MenuList({ items, onDone, onCloseLevel, autoFocusFirst = true, .
   };
 
   return (
-    <div role="menu" aria-label={aria["aria-label"]} className={styles.menu} onKeyDown={onKeyDown}>
+    <div
+      role="menu"
+      aria-label={aria["aria-label"]}
+      className={styles.menu}
+      onKeyDown={onKeyDown}
+      onPointerMove={(e) => {
+        if (e.currentTarget.contains(e.target as Node)) pointerMove({ x: e.clientX, y: e.clientY });
+      }}
+    >
       {resolved.map((r, i) => {
         if (r.kind === "separator") return <div key={r.key} role="separator" className={styles.separator} />;
         if (r.kind === "header")
@@ -264,10 +342,10 @@ export function MenuList({ items, onDone, onCloseLevel, autoFocusFirst = true, .
                 r.disabled && styles.disabled,
                 r.danger && styles.danger,
               )}
-              onPointerEnter={() => hover(i)}
-              onMouseEnter={() => hover(i)}
-              onPointerLeave={() => (hovered.current = -1)}
-              onMouseLeave={() => (hovered.current = -1)}
+              onPointerEnter={(e) => hover(i, { x: e.clientX, y: e.clientY })}
+              onMouseEnter={(e) => hover(i, { x: e.clientX, y: e.clientY })}
+              onPointerLeave={() => leave(i)}
+              onMouseLeave={() => leave(i)}
               onClick={() => activate(i)}
             >
               <span className={styles.icon} data-menu-icon={!r.checked && r.icon ? "" : undefined}>
@@ -290,14 +368,16 @@ export function MenuList({ items, onDone, onCloseLevel, autoFocusFirst = true, .
                 onClose={() => setOpenSub(null)}
                 className={styles.popup}
               >
-                <MenuList
-                  items={r.submenu!}
-                  onDone={onDone}
-                  onCloseLevel={() => {
-                    setOpenSub(null);
-                    itemRefs.current[i]?.focus();
-                  }}
-                />
+                <div ref={submenuRef} onPointerEnter={enterSubmenu}>
+                  <MenuList
+                    items={r.submenu!}
+                    onDone={onDone}
+                    onCloseLevel={() => {
+                      setOpenSub(null);
+                      itemRefs.current[i]?.focus();
+                    }}
+                  />
+                </div>
               </Popup>
             )}
           </Fragment>

src/ui/Popup.test.tsx

@@ -228,6 +228,91 @@ describe("Menu action enablement", () => {
     expect(active).toEqual(["Save", "Close", "Save"]);
   });
 
+  it.each(["enter submenu", "enter padding", "stop", "move away", "switch submenu", "unmount"])(
+    "protects a diagonal submenu path until the pointer chooses to %s",
+    (choice) => {
+      vi.useFakeTimers();
+      try {
+        const onActive = vi.fn();
+        const { unmount } = render(
+          <MenuList
+            items={[
+              { label: "Parent", submenu: [{ label: "Child" }] },
+              {
+                label: "Sibling",
+                onActive,
+                submenu: choice === "switch submenu" ? [{ label: "Other child" }] : undefined,
+              },
+            ]}
+            onDone={() => {}}
+          />,
+        );
+        const parent = screen.getByRole("menuitem", { name: "Parent" });
+        const sibling = screen.getByRole("menuitem", { name: "Sibling" });
+        fireEvent.pointerEnter(parent, { clientX: 40, clientY: 20 });
+        act(() => vi.advanceTimersByTime(120));
+        const child = screen.getByRole("menuitem", { name: "Child" });
+        vi.spyOn(child.closest("[data-popup]")!, "getBoundingClientRect").mockReturnValue({
+          left: 182,
+          right: 362,
+          top: 6,
+          bottom: 150,
+          width: 180,
+          height: 144,
+          x: 182,
+          y: 6,
+          toJSON: () => ({}),
+        });
+        fireEvent.pointerMove(parent, { clientX: 50, clientY: 20 });
+        fireEvent.pointerLeave(parent);
+        fireEvent.pointerEnter(sibling, { clientX: 70.5, clientY: 35.5 });
+        fireEvent.mouseLeave(parent);
+        fireEvent.mouseEnter(sibling, { clientX: 70, clientY: 35 });
+        act(() => vi.advanceTimersByTime(250));
+        expect(parent).toHaveClass(menuStyles.active!);
+        expect(parent).toHaveAttribute("aria-expanded", "true");
+        expect(onActive).not.toHaveBeenCalled();
+        fireEvent.pointerMove(sibling, { clientX: 100, clientY: 45 });
+        act(() => vi.advanceTimersByTime(250));
+        expect(parent).toHaveAttribute("aria-expanded", "true");
+
+        if (choice === "unmount") {
+          unmount();
+          act(() => vi.advanceTimersByTime(500));
+          expect(onActive).not.toHaveBeenCalled();
+        } else if (choice === "enter submenu" || choice === "enter padding") {
+          fireEvent.pointerLeave(sibling);
+          fireEvent.pointerMove(document.body, { clientX: 181, clientY: 65 });
+          if (choice === "enter padding") {
+            fireEvent.pointerMove(child.closest("[data-popup]")!, { clientX: 184, clientY: 8 });
+          } else {
+            fireEvent.pointerEnter(child, { clientX: 190, clientY: 65 });
+          }
+          act(() => vi.advanceTimersByTime(500));
+          expect(parent).toHaveClass(menuStyles.active!);
+          expect(parent).toHaveAttribute("aria-expanded", "true");
+          expect(onActive).not.toHaveBeenCalled();
+        } else {
+          if (choice === "move away") {
+            fireEvent.pointerMove(sibling, { clientX: 60, clientY: 45 });
+            expect(sibling).toHaveClass(menuStyles.active!);
+            act(() => vi.advanceTimersByTime(200));
+          } else {
+            act(() => vi.advanceTimersByTime(50));
+          }
+          expect(parent).toHaveAttribute("aria-expanded", "false");
+          expect(sibling).toHaveClass(menuStyles.active!);
+          expect(onActive).toHaveBeenCalledTimes(1);
+          if (choice === "switch submenu") expect(screen.getByText("Other child")).toBeInTheDocument();
+        }
+        unmount();
+      } finally {
+        vi.restoreAllMocks();
+        vi.useRealTimers();
+      }
+    },
+  );
+
   it("follows context changes after the menu opened", () => {
     const dispose = registerAction({ id: "test.menuWhen", title: "Needs Flag", when: "qaMenuFlag", run: () => {} });
     setContext("qaMenuFlag", false);

src/ui/menuAim.test.ts

@@ -0,0 +1,34 @@
+import { describe, expect, it } from "vitest";
+import { isSubmenuAim } from "./menuAim";
+
+describe("isSubmenuAim", () => {
+  const right = { left: 182, right: 362, top: 6, bottom: 150 };
+
+  it("accepts diagonal movement toward either near corner and the gap", () => {
+    expect(isSubmenuAim({ x: 50, y: 20 }, { x: 100, y: 45 }, right)).toBe(true);
+    expect(isSubmenuAim({ x: 50, y: 80 }, { x: 100, y: 55 }, right)).toBe(true);
+    expect(isSubmenuAim({ x: 170, y: 40 }, { x: 181, y: 60 }, right)).toBe(true);
+    expect(isSubmenuAim({ x: 170, y: 40 }, { x: 182, y: 155 }, right)).toBe(true);
+  });
+
+  it("rejects movement away, outside the triangle, or beyond the submenu", () => {
+    expect(isSubmenuAim({ x: 100, y: 45 }, { x: 60, y: 45 }, right)).toBe(false);
+    expect(isSubmenuAim({ x: 50, y: 20 }, { x: 60, y: 100 }, right)).toBe(false);
+    expect(isSubmenuAim({ x: 50, y: 20 }, { x: 50, y: 35 }, right)).toBe(false);
+    expect(isSubmenuAim({ x: 50, y: 20 }, { x: 400, y: 45 }, right)).toBe(false);
+  });
+
+  it("uses the right edge of a submenu flipped to the left", () => {
+    const left = { left: 6, right: 180, top: 6, bottom: 150 };
+    expect(isSubmenuAim({ x: 300, y: 20 }, { x: 250, y: 45 }, left)).toBe(true);
+    expect(isSubmenuAim({ x: 250, y: 45 }, { x: 300, y: 45 }, left)).toBe(false);
+  });
+
+  it("rejects a first move, a stationary pointer and degenerate triangles", () => {
+    expect(isSubmenuAim(null, { x: 100, y: 45 }, right)).toBe(false);
+    expect(isSubmenuAim({ x: 50, y: 20 }, { x: 50, y: 20 }, right)).toBe(false);
+    expect(isSubmenuAim({ x: 182, y: 20 }, { x: 182, y: 45 }, right)).toBe(false);
+    expect(isSubmenuAim({ x: 200, y: 20 }, { x: 210, y: 45 }, right)).toBe(false);
+    expect(isSubmenuAim({ x: 50, y: 20 }, { x: 100, y: 45 }, { ...right, bottom: 6 })).toBe(false);
+  });
+});

src/ui/menuAim.ts

@@ -0,0 +1,20 @@
+export interface MenuPointer {
+  x: number;
+  y: number;
+}
+
+type SubmenuRect = Pick<DOMRect, "left" | "right" | "top" | "bottom">;
+
+/** Whether this move stays in the triangle leading to the submenu's near edge. */
+export function isSubmenuAim(previous: MenuPointer | null, current: MenuPointer, rect: SubmenuRect): boolean {
+  if (!previous || rect.right <= rect.left || rect.bottom <= rect.top) return false;
+  const edge = previous.x < rect.left ? rect.left : rect.right;
+  const distance = edge - previous.x;
+  if (!distance || (previous.x >= rect.left && previous.x <= rect.right)) return false;
+  const progress = (current.x - previous.x) / distance;
+  if (progress <= 0 || progress > 1) return false;
+  const tolerance = 8;
+  const top = previous.y + (rect.top - tolerance - previous.y) * progress;
+  const bottom = previous.y + (rect.bottom + tolerance - previous.y) * progress;
+  return current.y >= top && current.y <= bottom;
+}