diff --git a/apps/desktop/src/electron/ElectronMenu.test.ts b/apps/desktop/src/electron/ElectronMenu.test.ts index 756274a614d7..d3ce646e4a02 100644 --- a/apps/desktop/src/electron/ElectronMenu.test.ts +++ b/apps/desktop/src/electron/ElectronMenu.test.ts @@ -77,7 +77,7 @@ describe("ElectronMenu", () => { const electronMenu = yield* ElectronMenu.ElectronMenu; const selectedItemId = yield* electronMenu.showContextMenu({ window: makeWindow(), - items: [{ id: "copy", label: "Copy" }], + items: [{ id: "copy", label: "Copy", accelerator: "Ctrl+Shift+C" }], position: Option.none(), }); @@ -99,7 +99,7 @@ describe("ElectronMenu", () => { const selectedItemId = yield* electronMenu.showContextMenu({ window: makeWindow(2), items: [ - { id: "copy", label: "Copy" }, + { id: "copy", label: "Copy", accelerator: "Ctrl+Shift+C" }, { id: "delete", label: "Delete", destructive: true, separatorBefore: true }, ], position: Option.some({ x: 10.8, y: 20.2 }), @@ -110,6 +110,7 @@ describe("ElectronMenu", () => { assert.equal(popupOptions?.y, 40); assert.deepEqual(buildFromTemplateMock.mock.calls[0]?.[0][0], { label: "Copy", + accelerator: "Ctrl+Shift+C", enabled: true, click: buildFromTemplateMock.mock.calls[0]?.[0][0].click, }); diff --git a/apps/desktop/src/electron/ElectronMenu.ts b/apps/desktop/src/electron/ElectronMenu.ts index b241619cd296..6c7c861b09f7 100644 --- a/apps/desktop/src/electron/ElectronMenu.ts +++ b/apps/desktop/src/electron/ElectronMenu.ts @@ -76,6 +76,7 @@ function normalizeContextMenuItems(source: readonly ContextMenuItem[]): ContextM const normalizedItem: ContextMenuItem = { id: sourceItem.id, label: sourceItem.label, + ...(sourceItem.accelerator ? { accelerator: sourceItem.accelerator } : {}), destructive: sourceItem.destructive === true, disabled: sourceItem.disabled === true, ...(sourceItem.separatorBefore === true ? { separatorBefore: true } : {}), @@ -165,6 +166,7 @@ export const make = Effect.gen(function* () { const itemOption: Electron.MenuItemConstructorOptions = { label: item.label, + ...(item.accelerator ? { accelerator: item.accelerator } : {}), enabled: !item.disabled, }; if (item.children && item.children.length > 0) { diff --git a/apps/web/src/components/ThreadTerminalDrawer.test.ts b/apps/web/src/components/ThreadTerminalDrawer.test.ts index e60d1d71678f..b1a16fd4b9f1 100644 --- a/apps/web/src/components/ThreadTerminalDrawer.test.ts +++ b/apps/web/src/components/ThreadTerminalDrawer.test.ts @@ -1,13 +1,182 @@ -import { describe, expect, it } from "vite-plus/test"; +import { describe, expect, it, vi } from "vite-plus/test"; import { resolveTerminalSelectionActionPosition, + runTerminalMenuRequest, shouldHandleTerminalExit, shouldHandleTerminalSelectionMouseUp, + shouldRestoreTerminalFocusAfterMenuAction, + terminalContextMenuItems, terminalSelectionActionDelayForClickCount, terminalSelectionLineRange, } from "./ThreadTerminalDrawer"; +describe("runTerminalMenuRequest", () => { + it("reports a failure from the current menu request", async () => { + const error = new Error("menu failed"); + const reportOpenError = vi.fn(); + + await runTerminalMenuRequest({ + signal: new AbortController().signal, + isCurrentRequest: () => true, + open: () => Promise.reject(error), + perform: vi.fn(async () => {}), + reportOpenError, + focusTerminal: vi.fn(), + }); + + expect(reportOpenError).toHaveBeenCalledWith(error); + }); + + it("suppresses failures and actions from superseded requests", async () => { + const reportOpenError = vi.fn(); + const perform = vi.fn(async () => {}); + + await runTerminalMenuRequest({ + signal: new AbortController().signal, + isCurrentRequest: () => false, + open: () => Promise.reject(new Error("stale menu failure")), + perform, + reportOpenError, + focusTerminal: vi.fn(), + }); + await runTerminalMenuRequest({ + signal: new AbortController().signal, + isCurrentRequest: () => false, + open: async () => "copy", + perform, + reportOpenError, + focusTerminal: vi.fn(), + }); + + expect(reportOpenError).not.toHaveBeenCalled(); + expect(perform).not.toHaveBeenCalled(); + }); + + it.each([ + ["copy", true], + ["paste", true], + ["add-to-chat", false], + ] as const)("performs %s and restores focus only when required", async (action, shouldFocus) => { + const perform = vi.fn(async () => {}); + const focusTerminal = vi.fn(); + + await runTerminalMenuRequest({ + signal: new AbortController().signal, + isCurrentRequest: () => true, + open: async () => action, + perform, + reportOpenError: vi.fn(), + focusTerminal, + }); + + expect(perform).toHaveBeenCalledWith(action, expect.any(Function)); + expect(focusTerminal).toHaveBeenCalledTimes(shouldFocus ? 1 : 0); + }); + + it("does nothing when the menu is dismissed", async () => { + const perform = vi.fn(async () => {}); + const focusTerminal = vi.fn(); + + await runTerminalMenuRequest({ + signal: new AbortController().signal, + isCurrentRequest: () => true, + open: async () => null, + perform, + reportOpenError: vi.fn(), + focusTerminal, + }); + + expect(perform).not.toHaveBeenCalled(); + expect(focusTerminal).not.toHaveBeenCalled(); + }); + + it("drops an action when the menu owner aborts before it resolves", async () => { + const abortController = new AbortController(); + let resolveMenu: (action: "copy") => void = () => {}; + const perform = vi.fn(async () => {}); + const request = runTerminalMenuRequest({ + signal: abortController.signal, + isCurrentRequest: () => true, + open: () => + new Promise<"copy">((resolve) => { + resolveMenu = resolve; + }), + perform, + reportOpenError: vi.fn(), + focusTerminal: vi.fn(), + }); + + abortController.abort(); + resolveMenu("copy"); + await request; + + expect(perform).not.toHaveBeenCalled(); + }); + + it("does not restore focus when teardown aborts an action in flight", async () => { + const abortController = new AbortController(); + let finishAction: (() => void) | undefined; + const focusTerminal = vi.fn(); + const request = runTerminalMenuRequest({ + signal: abortController.signal, + isCurrentRequest: () => true, + open: async () => "copy", + perform: vi.fn( + (_action: "add-to-chat" | "copy" | "paste", isCurrent: () => boolean) => + new Promise((resolve) => { + expect(isCurrent()).toBe(true); + finishAction = resolve; + }), + ), + reportOpenError: vi.fn(), + focusTerminal, + }); + await vi.waitFor(() => expect(finishAction).toBeDefined()); + + abortController.abort(); + finishAction?.(); + await request; + + expect(focusTerminal).not.toHaveBeenCalled(); + }); +}); + +describe("shouldRestoreTerminalFocusAfterMenuAction", () => { + it("restores focus only after terminal-local actions", () => { + expect(shouldRestoreTerminalFocusAfterMenuAction("copy")).toBe(true); + expect(shouldRestoreTerminalFocusAfterMenuAction("paste")).toBe(true); + expect(shouldRestoreTerminalFocusAfterMenuAction("add-to-chat")).toBe(false); + expect(shouldRestoreTerminalFocusAfterMenuAction(null)).toBe(false); + }); +}); + +describe("terminalContextMenuItems", () => { + it("offers all actions and disables only actions that need a selection", () => { + expect(terminalContextMenuItems({ canAddToChat: false, canCopy: false }, "Win32")).toEqual([ + { id: "add-to-chat", label: "Add to chat", disabled: true }, + { id: "copy", label: "Copy", accelerator: "Ctrl+Shift+C", disabled: true }, + { id: "paste", label: "Paste", accelerator: "Ctrl+Shift+V" }, + ]); + }); + + it("keeps Copy available when selected whitespace cannot be added to chat", () => { + expect(terminalContextMenuItems({ canAddToChat: false, canCopy: true }, "Linux")).toEqual([ + { id: "add-to-chat", label: "Add to chat", disabled: true }, + { id: "copy", label: "Copy", accelerator: "Ctrl+Shift+C", disabled: false }, + { id: "paste", label: "Paste", accelerator: "Ctrl+Shift+V" }, + ]); + }); + + it("uses Command accelerators on macOS", () => { + expect(terminalContextMenuItems({ canAddToChat: true, canCopy: true }, "MacIntel")).toEqual([ + { id: "add-to-chat", label: "Add to chat", disabled: false }, + { id: "copy", label: "Copy", accelerator: "Command+C", disabled: false }, + { id: "paste", label: "Paste", accelerator: "Command+V" }, + ]); + }); +}); + describe("resolveTerminalSelectionActionPosition", () => { it("prefers the selection rect over the last pointer position", () => { expect( diff --git a/apps/web/src/components/ThreadTerminalDrawer.tsx b/apps/web/src/components/ThreadTerminalDrawer.tsx index abd9bf9edfd5..c795f32fa75d 100644 --- a/apps/web/src/components/ThreadTerminalDrawer.tsx +++ b/apps/web/src/components/ThreadTerminalDrawer.tsx @@ -35,7 +35,7 @@ import { Popover, PopoverPopup, PopoverTrigger } from "~/components/ui/popover"; import { Button } from "~/components/ui/button"; import { PanelTabCloseButton } from "~/components/ui/panel-tab-close-button"; import { readTextFromClipboard, writeTextToClipboard } from "~/hooks/useCopyToClipboard"; -import { cn } from "~/lib/utils"; +import { cn, isMacPlatform } from "~/lib/utils"; import { type TerminalContextSelection } from "~/lib/terminalContext"; import { GhosttyTerminalSurface, @@ -260,29 +260,49 @@ export function terminalSelectionLineRange(position: { export type TerminalContextMenuAction = "add-to-chat" | "copy" | "paste"; -/** Post-selection popup: just the two selection actions, always enabled. */ -export function terminalSelectionMenuItems(): ContextMenuItem<"add-to-chat" | "copy">[] { - return [ - { id: "add-to-chat", label: "Add to chat" }, - { id: "copy", label: "Copy" }, - ]; +export function shouldRestoreTerminalFocusAfterMenuAction( + action: TerminalContextMenuAction | null, +): boolean { + return action === "copy" || action === "paste"; } -/** - * Right-click menu for the terminal canvas: the selection actions (disabled - * until a selection exists) plus Paste. Paste is always offered: the browser - * (and Electron's default editing menu) can only paste into an editable - * element, so a canvas terminal never gets a usable entry from them. - */ -export function terminalContextMenuItems(options: { - hasSelection: boolean; -}): ContextMenuItem[] { +export async function runTerminalMenuRequest(options: { + readonly signal: AbortSignal; + readonly isCurrentRequest: () => boolean; + readonly open: () => Promise; + readonly perform: (action: TerminalContextMenuAction, isCurrent: () => boolean) => Promise; + readonly reportOpenError: (error: unknown) => void; + readonly focusTerminal: () => void; +}): Promise { + const isCurrent = () => options.isCurrentRequest() && !options.signal.aborted; + let action: TerminalContextMenuAction | null; + try { + action = await options.open(); + } catch (error) { + if (isCurrent()) options.reportOpenError(error); + return; + } + if (action === null || !isCurrent()) return; + await options.perform(action, isCurrent); + if (shouldRestoreTerminalFocusAfterMenuAction(action) && isCurrent()) { + options.focusTerminal(); + } +} + +export function terminalContextMenuItems( + availability: { readonly canAddToChat: boolean; readonly canCopy: boolean }, + platform = typeof navigator === "undefined" ? "" : navigator.platform, +): ContextMenuItem[] { + const isMac = isMacPlatform(platform); return [ - ...terminalSelectionMenuItems().map((item) => ({ - ...item, - disabled: !options.hasSelection, - })), - { id: "paste", label: "Paste" }, + { id: "add-to-chat", label: "Add to chat", disabled: !availability.canAddToChat }, + { + id: "copy", + label: "Copy", + accelerator: isMac ? "Command+C" : "Ctrl+Shift+C", + disabled: !availability.canCopy, + }, + { id: "paste", label: "Paste", accelerator: isMac ? "Command+V" : "Ctrl+Shift+V" }, ]; } @@ -528,8 +548,12 @@ export function TerminalViewport({ synchronizeTerminalStatus(terminal, latestSession.status); if (autoFocus) window.requestAnimationFrame(() => terminal.focus()); + let terminalMenuAbortController: AbortController | null = null; const clearSelectionAction = () => { selectionActionRequestIdRef.current += 1; + terminalMenuAbortController?.abort(); + terminalMenuAbortController = null; + openSelectionMenuRequestIdRef.current = null; if (selectionActionTimerRef.current !== null) { window.clearTimeout(selectionActionTimerRef.current); selectionActionTimerRef.current = null; @@ -540,7 +564,7 @@ export function TerminalViewport({ const readSelectionAction = (): { position: { x: number; y: number }; clipboardText: string; - selection: TerminalContextSelection; + selection: TerminalContextSelection | null; } | null => { const activeTerminal = terminalRef.current; const mountElement = containerRef.current; @@ -550,10 +574,9 @@ export function TerminalViewport({ const selectionText = activeTerminal.getSelection(); const selectionPosition = activeTerminal.getSelectionPosition(); const normalizedText = selectionText.replace(/\r\n/g, "\n").replace(/^\n+|\n+$/g, ""); - if (!selectionPosition || normalizedText.length === 0) { + if (selectionText.length === 0) { return null; } - const { lineStart, lineEnd } = terminalSelectionLineRange(selectionPosition); const bounds = mountElement.getBoundingClientRect(); const position = resolveTerminalSelectionActionPosition({ bounds, @@ -563,63 +586,75 @@ export function TerminalViewport({ return { position, clipboardText: selectionText, - selection: { - terminalId, - terminalLabel: readTerminalLabel(), - lineStart, - lineEnd, - text: normalizedText, - }, + selection: + selectionPosition && normalizedText.length > 0 + ? { + terminalId, + terminalLabel: readTerminalLabel(), + ...terminalSelectionLineRange(selectionPosition), + text: normalizedText, + } + : null, }; }; - const addSelectionToChat = (selection: TerminalContextSelection) => { - handleAddTerminalContext(selection); - terminalRef.current?.clearSelection(); - terminalRef.current?.focus(); - }; - - // A selection-action flow that was superseded while its async work ran - // must go silent: no error message, no focus steal. - const reportIfCurrent = (requestId: number, error: unknown, fallback: string) => { - if (requestId !== selectionActionRequestIdRef.current) return; - const activeTerminal = terminalRef.current; - if (activeTerminal) { - writeSystemMessage(activeTerminal, error instanceof Error ? error.message : fallback); - } - }; - - const focusIfCurrent = (requestId: number) => { - if (requestId === selectionActionRequestIdRef.current) { - terminalRef.current?.focus(); - } - }; - - const copySelection = async (text: string, requestId: number) => { - try { - await writeTextToClipboard(text, "terminal selection"); - } catch (error) { - reportIfCurrent(requestId, error, "Unable to copy terminal selection"); + const performTerminalMenuAction = async ( + clicked: TerminalContextMenuAction, + selectionAction: ReturnType, + isCurrent: () => boolean, + ) => { + switch (clicked) { + case "add-to-chat": + if (!selectionAction?.selection || !isCurrent()) return; + handleAddTerminalContext(selectionAction.selection); + terminalRef.current?.clearSelection(); + return; + case "copy": + if (!selectionAction || !isCurrent()) return; + try { + await writeTextToClipboard(selectionAction.clipboardText, "terminal selection"); + } catch (error) { + if (!isCurrent()) return; + const activeTerminal = terminalRef.current; + if (activeTerminal) { + writeSystemMessage( + activeTerminal, + error instanceof Error ? error.message : "Unable to copy terminal selection", + ); + } + } + return; + case "paste": { + if (!isCurrent()) return; + const activeTerminal = terminalRef.current; + if (!activeTerminal) return; + try { + // The surface claims the paste race before reading so a shortcut + // cannot deliver the same clipboard content a second time. + await activeTerminal.pasteFromClipboard( + () => readTextFromClipboard("terminal input"), + isCurrent, + ); + } catch (error) { + if (!isCurrent()) return; + writeSystemMessage( + activeTerminal, + error instanceof Error ? error.message : "Unable to read the clipboard", + ); + } + return; + } } - focusIfCurrent(requestId); }; - const pasteFromClipboard = async (requestId: number) => { + const reportMenuOpenError = (error: unknown) => { const activeTerminal = terminalRef.current; - if (!activeTerminal) return; - try { - // The surface owns the read so it can claim the paste race before it - // starts: a paste shortcut fired while the menu read is in flight - // supersedes this paste instead of landing alongside it. - await activeTerminal.pasteFromClipboard( - () => readTextFromClipboard("terminal input"), - () => requestId === selectionActionRequestIdRef.current, + if (activeTerminal) { + writeSystemMessage( + activeTerminal, + error instanceof Error ? error.message : "Unable to open the terminal context menu", ); - } catch (error) { - reportIfCurrent(requestId, error, "Unable to read the clipboard"); - return; } - focusIfCurrent(requestId); }; const showTerminalContextMenu = async (event: MouseEvent) => { @@ -632,30 +667,35 @@ export function TerminalViewport({ clearSelectionAction(); const selectionAction = readSelectionAction(); const requestId = selectionActionRequestIdRef.current; - let clicked: TerminalContextMenuAction | null; + const abortController = new AbortController(); + terminalMenuAbortController = abortController; try { - clicked = await localApi.contextMenu.show( - terminalContextMenuItems({ hasSelection: selectionAction !== null }), - { x: event.clientX, y: event.clientY }, - ); - } catch (error) { - reportIfCurrent(requestId, error, "Unable to open the terminal context menu"); - focusIfCurrent(requestId); - return; - } - if (requestId !== selectionActionRequestIdRef.current || clicked === null) { - return; - } - switch (clicked) { - case "add-to-chat": - if (selectionAction) addSelectionToChat(selectionAction.selection); - return; - case "copy": - if (selectionAction) await copySelection(selectionAction.clipboardText, requestId); - return; - case "paste": - await pasteFromClipboard(requestId); - return; + await runTerminalMenuRequest({ + signal: abortController.signal, + isCurrentRequest: () => requestId === selectionActionRequestIdRef.current, + open: () => + localApi.contextMenu.show( + terminalContextMenuItems({ + canAddToChat: selectionAction?.selection != null, + canCopy: selectionAction !== null, + }), + { x: event.clientX, y: event.clientY }, + { + layout: "compact", + presentation: "styled", + restoreFocus: "on-dismiss", + signal: abortController.signal, + }, + ), + perform: (clicked, isCurrent) => + performTerminalMenuAction(clicked, selectionAction, isCurrent), + reportOpenError: reportMenuOpenError, + focusTerminal: () => terminalRef.current?.focus(), + }); + } finally { + if (terminalMenuAbortController === abortController) { + terminalMenuAbortController = null; + } } }; @@ -673,24 +713,39 @@ export function TerminalViewport({ return; } const requestId = ++selectionActionRequestIdRef.current; + const abortController = new AbortController(); + terminalMenuAbortController = abortController; openSelectionMenuRequestIdRef.current = requestId; - const clicked = await localApi.contextMenu - .show(terminalSelectionMenuItems(), nextAction.position) - .finally(() => { - if (openSelectionMenuRequestIdRef.current === requestId) { - openSelectionMenuRequestIdRef.current = null; - } + try { + await runTerminalMenuRequest({ + signal: abortController.signal, + isCurrentRequest: () => requestId === selectionActionRequestIdRef.current, + open: () => + localApi.contextMenu.show( + terminalContextMenuItems({ + canAddToChat: nextAction.selection !== null, + canCopy: true, + }), + nextAction.position, + { + layout: "compact", + presentation: "styled", + restoreFocus: "on-dismiss", + signal: abortController.signal, + }, + ), + perform: (clicked, isCurrent) => + performTerminalMenuAction(clicked, nextAction, isCurrent), + reportOpenError: reportMenuOpenError, + focusTerminal: () => terminalRef.current?.focus(), }); - if (requestId !== selectionActionRequestIdRef.current || clicked === null) { - return; - } - switch (clicked) { - case "add-to-chat": - addSelectionToChat(nextAction.selection); - return; - case "copy": - await copySelection(nextAction.clipboardText, requestId); - return; + } finally { + if (terminalMenuAbortController === abortController) { + terminalMenuAbortController = null; + } + if (openSelectionMenuRequestIdRef.current === requestId) { + openSelectionMenuRequestIdRef.current = null; + } } }; @@ -810,12 +865,6 @@ export function TerminalViewport({ }); if (!shouldClear) return; clearSelectionAction(); - // A copy shortcut that clears the selection (Ctrl+C) must also close - // the context menu that appears with the selection, but a clear that - // never opened a menu must not dismiss an unrelated one. - if (openSelectionMenuRequestIdRef.current !== null) { - void localApi?.contextMenu.close(); - } } const handleMouseUp = (event: MouseEvent) => { diff --git a/apps/web/src/contextMenuFallback.test.ts b/apps/web/src/contextMenuFallback.test.ts index ddc5da05f806..24f016ac3ac3 100644 --- a/apps/web/src/contextMenuFallback.test.ts +++ b/apps/web/src/contextMenuFallback.test.ts @@ -1,6 +1,10 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vite-plus/test"; -import { dismissContextMenu, showContextMenuFallback } from "./contextMenuFallback"; +import { + contextMenuAcceleratorAction, + dismissContextMenu, + showContextMenuFallback, +} from "./contextMenuFallback"; type FakeListener = (event: FakeDomEvent) => void; @@ -17,6 +21,8 @@ class FakeDomEvent { preventDefault() { this.defaultPrevented = true; } + + stopPropagation() {} } class FakeElement { @@ -174,6 +180,13 @@ class FakeDocument { } } + dispatchEvent(event: FakeDomEvent) { + for (const listener of this.listeners.get(event.type) ?? []) { + listener(event); + } + return true; + } + querySelectorAll(tagName: string) { return this.body.querySelectorAll(tagName); } @@ -247,6 +260,116 @@ describe("showContextMenuFallback", () => { await expect(selectionPromise).resolves.toBe("rename"); }); + it("renders shortcut hints next to menu labels", () => { + void showContextMenuFallback( + [ + { id: "copy", label: "Copy", accelerator: "Ctrl+Shift+C" }, + { id: "paste", label: "Paste", accelerator: "Command+Shift+V" }, + ], + undefined, + { layout: "compact" }, + ); + + const shortcuts = (document as unknown as FakeDocument) + .querySelectorAll("kbd") + .map((element) => element.textContent); + expect(shortcuts).toEqual(["Ctrl+Shift+C", "⇧⌘V"]); + const menu = (document as unknown as FakeDocument) + .querySelectorAll("div") + .find((element) => element.className.includes("dropdown-glass")); + expect(menu?.className).toContain("min-w-56"); + }); + + it("keeps the default menu layout when no layout is requested", () => { + void showContextMenuFallback([{ id: "rename", label: "Rename" }]); + + const menu = (document as unknown as FakeDocument) + .querySelectorAll("div") + .find((element) => element.className.includes("dropdown-glass")); + expect(menu?.className).toContain("min-w-32"); + expect(menu?.className).not.toContain("min-w-56"); + }); + + it("runs an accelerator before the focused terminal handles it", async () => { + const selectionPromise = showContextMenuFallback([ + { id: "copy", label: "Copy", accelerator: "Ctrl+Shift+C" }, + ]); + const event = new KeyboardEvent("keydown", { + altKey: false, + ctrlKey: true, + isComposing: false, + key: "c", + metaKey: false, + shiftKey: true, + }); + + (document as unknown as FakeDocument).dispatchEvent(event as unknown as FakeDomEvent); + + expect(event.defaultPrevented).toBe(true); + await expect(selectionPromise).resolves.toBe("copy"); + }); + + it("closes when the page or terminal is scrolled", async () => { + const selectionPromise = showContextMenuFallback([{ id: "copy", label: "Copy" }]); + + (document as unknown as FakeDocument).dispatchEvent(new FakeDomEvent("wheel")); + + await expect(selectionPromise).resolves.toBeNull(); + }); + + it("closes without restoring focus when its owner aborts", async () => { + const invoker = (document as unknown as FakeDocument).createElement("button"); + (document as unknown as FakeDocument).body.appendChild(invoker); + invoker.focus(); + const abortController = new AbortController(); + const selectionPromise = showContextMenuFallback([{ id: "copy", label: "Copy" }], undefined, { + restoreFocus: "on-dismiss", + signal: abortController.signal, + }); + const action = findButton("Copy"); + action?.dispatchEvent(new MouseEvent("mouseenter", { bubbles: true })); + + abortController.abort(); + + await expect(selectionPromise).resolves.toBeNull(); + expect(findButton("Copy")).toBeUndefined(); + expect(invoker.focused).toBe(false); + }); + + it("restores focus after interactive dismissal when requested", async () => { + const invoker = (document as unknown as FakeDocument).createElement("button"); + (document as unknown as FakeDocument).body.appendChild(invoker); + invoker.focus(); + const selectionPromise = showContextMenuFallback([{ id: "copy", label: "Copy" }], undefined, { + restoreFocus: "on-dismiss", + }); + findButton("Copy")?.dispatchEvent(new MouseEvent("mouseenter", { bubbles: true })); + + (document as unknown as FakeDocument).dispatchEvent( + new KeyboardEvent("keydown", { isComposing: false, key: "Escape" }), + ); + + await expect(selectionPromise).resolves.toBeNull(); + expect(invoker.focused).toBe(true); + }); + + it("leaves action focus restoration to the caller", async () => { + const invoker = (document as unknown as FakeDocument).createElement("button"); + (document as unknown as FakeDocument).body.appendChild(invoker); + invoker.focus(); + const selectionPromise = showContextMenuFallback( + [{ id: "add-to-chat", label: "Add to chat" }], + undefined, + { restoreFocus: "on-dismiss" }, + ); + const action = findButton("Add to chat"); + action?.dispatchEvent(new MouseEvent("mouseenter", { bubbles: true })); + action?.dispatchEvent(new MouseEvent("click", { bubbles: true })); + + await expect(selectionPromise).resolves.toBe("add-to-chat"); + expect(invoker.focused).toBe(false); + }); + it("ignores a click from the gesture that opened the menu", async () => { let enablePointerSelection: ((time: number) => void) | undefined; vi.stubGlobal("requestAnimationFrame", (callback: (time: number) => void) => { @@ -328,6 +451,47 @@ describe("showContextMenuFallback", () => { }); }); +describe("contextMenuAcceleratorAction", () => { + const event = (overrides: Partial[1]> = {}) => ({ + altKey: false, + ctrlKey: false, + isComposing: false, + key: "", + metaKey: false, + shiftKey: false, + ...overrides, + }); + + it("matches Windows, Linux, and macOS accelerators", () => { + const items = [ + { id: "copy", label: "Copy", accelerator: "Ctrl+Shift+C" }, + { id: "paste", label: "Paste", accelerator: "Command+V" }, + ] as const; + + expect( + contextMenuAcceleratorAction(items, event({ key: "c", ctrlKey: true, shiftKey: true })), + ).toBe("copy"); + expect(contextMenuAcceleratorAction(items, event({ key: "V", metaKey: true }))).toBe("paste"); + }); + + it("ignores disabled, partial, and composing shortcuts", () => { + const items = [ + { id: "copy", label: "Copy", accelerator: "Ctrl+Shift+C", disabled: true }, + ] as const; + + expect( + contextMenuAcceleratorAction(items, event({ key: "c", ctrlKey: true, shiftKey: true })), + ).toBeNull(); + expect(contextMenuAcceleratorAction(items, event({ key: "c", ctrlKey: true }))).toBeNull(); + expect( + contextMenuAcceleratorAction(items, { + ...event({ key: "c", ctrlKey: true, shiftKey: true }), + isComposing: true, + }), + ).toBeNull(); + }); +}); + describe("dismissContextMenu", () => { it("resolves an open menu with null", async () => { const selectionPromise = showContextMenuFallback([ diff --git a/apps/web/src/contextMenuFallback.ts b/apps/web/src/contextMenuFallback.ts index ce8b8950a8b9..8592e4b1afc5 100644 --- a/apps/web/src/contextMenuFallback.ts +++ b/apps/web/src/contextMenuFallback.ts @@ -197,6 +197,44 @@ export function dismissContextMenu(): void { activeContextMenuDismiss = null; } +export function contextMenuAcceleratorAction( + items: readonly ContextMenuItem[], + event: Pick, +): T | null { + if (event.isComposing) return null; + + for (const item of items) { + if (item.disabled) continue; + if (item.children) { + const childAction = contextMenuAcceleratorAction(item.children, event); + if (childAction !== null) return childAction; + } + if (!item.accelerator) continue; + + const parts = item.accelerator.toLowerCase().split("+"); + if ( + event.key.toLowerCase() === parts.at(-1) && + event.altKey === parts.includes("alt") && + event.ctrlKey === parts.includes("ctrl") && + event.metaKey === (parts.includes("command") || parts.includes("cmd")) && + event.shiftKey === parts.includes("shift") + ) { + return item.id; + } + } + + return null; +} + +function formatContextMenuAccelerator(accelerator: string): string { + const parts = accelerator.split("+"); + if (!parts.some((part) => part === "Command" || part === "Cmd")) return accelerator; + const key = parts.at(-1) ?? ""; + return `${parts.includes("Ctrl") ? "⌃" : ""}${parts.includes("Alt") ? "⌥" : ""}${ + parts.includes("Shift") ? "⇧" : "" + }⌘${key}`; +} + /** * Imperative DOM-based context menu for non-Electron environments. * Supports nested submenus and resolves with the clicked leaf item id. @@ -204,8 +242,17 @@ export function dismissContextMenu(): void { export function showContextMenuFallback( items: readonly ContextMenuItem[], position?: { x: number; y: number }, + options?: { + readonly layout?: "default" | "compact"; + readonly restoreFocus?: boolean | "on-dismiss"; + readonly signal?: AbortSignal; + }, ): Promise { return new Promise((resolve) => { + if (options?.signal?.aborted) { + resolve(null); + return; + } const previouslyFocusedElement = document.activeElement instanceof HTMLElement ? document.activeElement : null; const menuStack: HTMLDivElement[] = []; @@ -213,9 +260,9 @@ export function showContextMenuFallback( let isDisposed = false; let canDismissFromPointer = false; - const dismiss = () => cleanup(null); + const dismiss = () => cleanup(null, "programmatic"); - const cleanup = (result: T | null) => { + const cleanup = (result: T | null, reason: "action" | "interaction" | "programmatic") => { if (isDisposed) { return; } @@ -223,23 +270,40 @@ export function showContextMenuFallback( if (activeContextMenuDismiss === dismiss) { activeContextMenuDismiss = null; } - document.removeEventListener("keydown", onKeyDown); + document.removeEventListener("keydown", onKeyDown, true); document.removeEventListener("pointerdown", onPointerDown, true); document.removeEventListener("contextmenu", onContextMenu, true); + document.removeEventListener("wheel", onWheel, true); + options?.signal?.removeEventListener("abort", onAbort); const shouldRestoreFocus = isNodeWithinMenuStack(document.activeElement, menuStack); for (const menu of menuStack) { menu.remove(); } - if (shouldRestoreFocus && previouslyFocusedElement?.isConnected) { + if ( + options?.restoreFocus !== false && + (options?.restoreFocus !== "on-dismiss" || reason === "interaction") && + shouldRestoreFocus && + previouslyFocusedElement?.isConnected + ) { previouslyFocusedElement.focus({ preventScroll: true }); } resolve(result); }; + const onAbort = () => cleanup(null, "programmatic"); + const onKeyDown = (event: KeyboardEvent) => { + if (event.isComposing) return; if (event.key === "Escape") { event.preventDefault(); - cleanup(null); + cleanup(null, "interaction"); + return; + } + const action = contextMenuAcceleratorAction(items, event); + if (action !== null) { + event.preventDefault(); + event.stopPropagation(); + cleanup(action, "action"); } }; @@ -247,7 +311,7 @@ export function showContextMenuFallback( if (!canDismissFromPointer || isNodeWithinMenuStack(event.target, menuStack)) { return; } - cleanup(null); + cleanup(null, "interaction"); }; const onContextMenu = (event: MouseEvent) => { @@ -255,7 +319,13 @@ export function showContextMenuFallback( return; } event.preventDefault(); - cleanup(null); + cleanup(null, "interaction"); + }; + + const onWheel = (event: WheelEvent) => { + if (!isNodeWithinMenuStack(event.target, menuStack)) { + cleanup(null, "interaction"); + } }; const closeMenusFromLevel = (level: number) => { @@ -274,11 +344,14 @@ export function showContextMenuFallback( ) => { closeMenusFromLevel(level); + const usesCompactLayout = options?.layout === "compact"; const menu = document.createElement("div"); - menu.className = - "dropdown-glass fixed z-[10000] min-w-32 max-w-sm overflow-hidden rounded-lg bg-clip-padding text-popover-foreground outline-none"; - menu.style.cssText = - "position:fixed;z-index:10000;min-width:8rem;max-width:24rem;overflow:hidden;border-radius:var(--radius-lg);background-clip:padding-box;color:var(--contrast-popover-foreground);outline:none;pointer-events:auto;"; + menu.className = `dropdown-glass fixed z-[10000] max-w-sm overflow-hidden rounded-lg bg-clip-padding text-popover-foreground outline-none ${ + usesCompactLayout ? "min-w-56" : "min-w-32" + }`; + menu.style.cssText = `position:fixed;z-index:10000;min-width:${ + usesCompactLayout ? "min(14rem,calc(100vw - 0.75rem))" : "8rem" + };max-width:24rem;overflow:hidden;border-radius:var(--radius-lg);background-clip:padding-box;color:var(--contrast-popover-foreground);outline:none;pointer-events:auto;`; menu.style.left = `${preferredLeft}px`; menu.style.top = `${preferredTop}px`; menu.dataset.level = String(level); @@ -316,15 +389,16 @@ export function showContextMenuFallback( button.type = "button"; const isDisabled = item.disabled === true; button.disabled = isDisabled; - const rowBase = - "flex w-full cursor-default select-none items-center gap-2 rounded-sm px-2 py-1 text-left outline-none transition-colors sm:min-h-7 sm:text-sm min-h-8 text-base"; + const rowBase = usesCompactLayout + ? "flex min-h-8 w-full cursor-default select-none items-center gap-2 rounded-sm px-2 py-1 text-left text-sm outline-none transition-colors sm:min-h-7 sm:text-xs" + : "flex min-h-8 w-full cursor-default select-none items-center gap-2 rounded-sm px-2 py-1 text-left text-base outline-none transition-colors sm:min-h-7 sm:text-sm"; button.className = isDisabled ? `${rowBase} pointer-events-none cursor-not-allowed text-muted-foreground opacity-64` : isLeafDestructive ? `${rowBase} text-destructive-foreground hover:bg-destructive/10 hover:text-destructive-foreground` : `${rowBase} text-foreground hover:bg-accent hover:text-accent-foreground`; button.style.cssText = - "display:flex;width:100%;min-height:1.75rem;align-items:center;gap:0.5rem;border:0;border-radius:var(--radius-sm);background:transparent;padding:0.25rem 0.5rem;color:var(--contrast-foreground);font-family:var(--font-sans,system-ui,sans-serif);font-size:0.875rem;line-height:1.25rem;text-align:left;cursor:default;"; + "display:flex;width:100%;align-items:center;gap:0.5rem;border:0;border-radius:var(--radius-sm);background:transparent;padding:0.25rem 0.5rem;color:var(--contrast-foreground);font-family:var(--font-sans,system-ui,sans-serif);font-weight:400;line-height:1.25rem;text-align:left;cursor:default;"; if (isLeafDestructive) { button.style.color = "var(--destructive-foreground)"; } @@ -346,6 +420,16 @@ export function showContextMenuFallback( label.textContent = item.label; button.appendChild(label); + if (item.accelerator) { + const accelerator = document.createElement("kbd"); + accelerator.className = + "ms-auto shrink-0 font-medium font-sans text-secondary-label text-xs tracking-widest"; + accelerator.style.cssText = + "margin-inline-start:auto;flex-shrink:0;color:var(--contrast-secondary-label);font-family:var(--font-sans,system-ui,sans-serif);font-size:0.75rem;font-weight:500;letter-spacing:0.1em;"; + accelerator.textContent = formatContextMenuAccelerator(item.accelerator); + button.appendChild(accelerator); + } + if (hasChildren) { button.setAttribute("aria-haspopup", "menu"); button.setAttribute("aria-expanded", "false"); @@ -431,7 +515,7 @@ export function showContextMenuFallback( closeMenusFromLevel(level + 1); }); button.addEventListener("click", () => { - if (canDismissFromPointer) cleanup(item.id); + if (canDismissFromPointer) cleanup(item.id, "action"); }); } } @@ -454,9 +538,11 @@ export function showContextMenuFallback( }); }; - document.addEventListener("keydown", onKeyDown); + document.addEventListener("keydown", onKeyDown, true); document.addEventListener("pointerdown", onPointerDown, true); document.addEventListener("contextmenu", onContextMenu, true); + document.addEventListener("wheel", onWheel, { capture: true, passive: true }); + options?.signal?.addEventListener("abort", onAbort, { once: true }); openMenu(items, position?.x ?? 0, position?.y ?? 0, 0); // Only one fallback menu can be open at a time: a new show must dismiss // any prior one, or its DOM and listeners leak and close() can only ever diff --git a/apps/web/src/localApi.test.ts b/apps/web/src/localApi.test.ts index 9220252cb20e..7f3620613c64 100644 --- a/apps/web/src/localApi.test.ts +++ b/apps/web/src/localApi.test.ts @@ -6,13 +6,17 @@ import { } from "@t3tools/contracts"; import { afterEach, beforeEach, describe, expect, it, vi } from "vite-plus/test"; -const showContextMenuFallbackMock = - vi.fn< - ( - items: readonly ContextMenuItem[], - position?: { x: number; y: number }, - ) => Promise - >(); +const showContextMenuFallbackMock = vi.fn< + ( + items: readonly ContextMenuItem[], + position?: { x: number; y: number }, + options?: { + readonly layout?: "default" | "compact"; + readonly restoreFocus?: boolean | "on-dismiss"; + readonly signal?: AbortSignal; + }, + ) => Promise +>(); const dismissContextMenuMock = vi.fn<() => void>(); const requestConfirmDialogMock = @@ -84,7 +88,7 @@ describe("LocalApi", () => { const items = [{ id: "rename", label: "Rename" }] as const; await expect(createLocalApi().contextMenu.show(items, { x: 4, y: 5 })).resolves.toBe("rename"); - expect(showContextMenuFallbackMock).toHaveBeenCalledWith(items, { x: 4, y: 5 }); + expect(showContextMenuFallbackMock).toHaveBeenCalledWith(items, { x: 4, y: 5 }, undefined); }); it("dismisses an open browser context menu without a desktop bridge", async () => { @@ -95,6 +99,40 @@ describe("LocalApi", () => { expect(dismissContextMenuMock).toHaveBeenCalledOnce(); }); + it("uses the styled fallback on desktop when the caller owns its lifecycle", async () => { + const showContextMenu = vi.fn().mockResolvedValue("native"); + testWindow().desktopBridge = { showContextMenu } as unknown as DesktopBridge; + showContextMenuFallbackMock.mockResolvedValue("copy"); + const abortController = new AbortController(); + const { createLocalApi } = await import("./localApi"); + const items = [{ id: "copy", label: "Copy" }] as const; + + await expect( + createLocalApi().contextMenu.show(items, undefined, { + layout: "compact", + presentation: "styled", + restoreFocus: "on-dismiss", + signal: abortController.signal, + }), + ).resolves.toBe("copy"); + + expect(showContextMenu).not.toHaveBeenCalled(); + expect(showContextMenuFallbackMock).toHaveBeenCalledWith(items, undefined, { + layout: "compact", + restoreFocus: "on-dismiss", + signal: abortController.signal, + }); + }); + + it("dismisses a styled fallback when a desktop bridge exists", async () => { + testWindow().desktopBridge = { showContextMenu: vi.fn() } as unknown as DesktopBridge; + const { createLocalApi } = await import("./localApi"); + + await createLocalApi().contextMenu.close(); + + expect(dismissContextMenuMock).toHaveBeenCalledOnce(); + }); + it("uses the themed confirmation host when it is available", async () => { requestConfirmDialogMock.mockResolvedValue(true); const { createLocalApi } = await import("./localApi"); diff --git a/apps/web/src/localApi.ts b/apps/web/src/localApi.ts index 863388106a3e..eef5a0e197d2 100644 --- a/apps/web/src/localApi.ts +++ b/apps/web/src/localApi.ts @@ -35,19 +35,36 @@ function createBrowserLocalApi(): LocalApi { show: async ( items: readonly ContextMenuItem[], position?: { x: number; y: number }, + options?: { + readonly layout?: "default" | "compact"; + readonly presentation?: "native" | "styled"; + readonly restoreFocus?: boolean | "on-dismiss"; + readonly signal?: AbortSignal; + }, ): Promise => { - if (window.desktopBridge) { + if (window.desktopBridge && options?.presentation !== "styled") { return window.desktopBridge.showContextMenu(items, position) as Promise; } - return showContextMenuFallback(items, position); + return showContextMenuFallback( + items, + position, + options?.layout !== undefined || + options?.restoreFocus !== undefined || + options?.signal !== undefined + ? { + ...(options.layout === undefined ? {} : { layout: options.layout }), + ...(options.restoreFocus === undefined + ? {} + : { restoreFocus: options.restoreFocus }), + ...(options.signal === undefined ? {} : { signal: options.signal }), + } + : undefined, + ); }, - // A native desktop menu blocks keyboard input and closes on outside - // interaction, so nothing to do there; the DOM fallback needs an explicit - // dismiss when the state behind it goes away. + // Native desktop menus close themselves. Callers can still request the + // styled fallback on desktop, so always dismiss that host as well. close: async () => { - if (!window.desktopBridge) { - dismissContextMenu(); - } + dismissContextMenu(); }, }, persistence: { diff --git a/packages/contracts/src/ipc.ts b/packages/contracts/src/ipc.ts index e753596f3d33..42c9042e2e91 100644 --- a/packages/contracts/src/ipc.ts +++ b/packages/contracts/src/ipc.ts @@ -105,6 +105,8 @@ import type { export interface ContextMenuItem { id: T; label: string; + /** Shortcut notation displayed and handled by menu hosts that support accelerators. */ + accelerator?: string; destructive?: boolean; disabled?: boolean; /** Renders as a non-interactive section header label. Web fallback only — stripped on desktop native menus. */ @@ -119,6 +121,7 @@ export interface ContextMenuItem { export interface ContextMenuItemSchemaType { readonly id: string; readonly label: string; + readonly accelerator?: string; readonly destructive?: boolean; readonly disabled?: boolean; readonly header?: boolean; @@ -130,6 +133,7 @@ export interface ContextMenuItemSchemaType { export const ContextMenuItemSchema: Schema.Codec = Schema.Struct({ id: Schema.String, label: Schema.String, + accelerator: Schema.optionalKey(Schema.String), destructive: Schema.optionalKey(Schema.Boolean), disabled: Schema.optionalKey(Schema.Boolean), header: Schema.optionalKey(Schema.Boolean), @@ -1258,6 +1262,12 @@ export interface LocalApi { show: ( items: readonly ContextMenuItem[], position?: { x: number; y: number }, + options?: { + readonly layout?: "default" | "compact"; + readonly presentation?: "native" | "styled"; + readonly restoreFocus?: boolean | "on-dismiss"; + readonly signal?: AbortSignal; + }, ) => Promise; close: () => Promise; };