From 4d5de201c462817d948ad9019b2f96bf4d88b74d Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Sun, 8 Nov 2026 21:28:06 +0800 Subject: [PATCH] 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 | 23 ++++++++ src/ui/Menu.tsx | 128 ++++++++++++++++++++++++++++++++-------- src/ui/Popup.test.tsx | 85 ++++++++++++++++++++++++++ src/ui/menuAim.test.ts | 34 +++++++++++ src/ui/menuAim.ts | 20 +++++++ 5 files changed, 266 insertions(+), 24 deletions(-) create mode 100644 src/ui/menuAim.test.ts create mode 100644 src/ui/menuAim.ts diff --git a/e2e/projectTree.spec.ts b/e2e/projectTree.spec.ts index ccb12ed38..af73482ac 100644 --- a/e2e/projectTree.spec.ts +++ b/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)))); diff --git a/src/ui/Menu.tsx b/src/ui/Menu.tsx index 3dcb46738..ab74a835c 100644 --- a/src/ui/Menu.tsx +++ b/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(null); const itemRefs = useRef>([]); const hoverTimer = useRef | undefined>(undefined); + const submenuRef = useRef(null); + const pointer = useRef(null); + const previousPointer = useRef(null); + const pendingAim = useRef(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 ( -
+
{ + if (e.currentTarget.contains(e.target as Node)) pointerMove({ x: e.clientX, y: e.clientY }); + }} + > {resolved.map((r, i) => { if (r.kind === "separator") return
; 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)} > @@ -290,14 +368,16 @@ export function MenuList({ items, onDone, onCloseLevel, autoFocusFirst = true, . onClose={() => setOpenSub(null)} className={styles.popup} > - { - setOpenSub(null); - itemRefs.current[i]?.focus(); - }} - /> +
+ { + setOpenSub(null); + itemRefs.current[i]?.focus(); + }} + /> +
)} diff --git a/src/ui/Popup.test.tsx b/src/ui/Popup.test.tsx index 2d0c2492c..d47436f7e 100644 --- a/src/ui/Popup.test.tsx +++ b/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( + {}} + />, + ); + 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); diff --git a/src/ui/menuAim.test.ts b/src/ui/menuAim.test.ts new file mode 100644 index 000000000..d30050a23 --- /dev/null +++ b/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); + }); +}); diff --git a/src/ui/menuAim.ts b/src/ui/menuAim.ts new file mode 100644 index 000000000..9fb067dae --- /dev/null +++ b/src/ui/menuAim.ts @@ -0,0 +1,20 @@ +export interface MenuPointer { + x: number; + y: number; +} + +type SubmenuRect = Pick; + +/** 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; +}