From 806f7ffe8a2161ca7acefbeed16e84853868af49 Mon Sep 17 00:00:00 2001 From: Andrew Date: Sun, 6 Sep 2026 13:42:50 -0400 Subject: [PATCH 1/3] fix(desktop): focus message link menus opened with the keyboard --- .../desktop/src/electron/ElectronMenu.test.ts | 26 +++++++++++++++++++ apps/desktop/src/electron/ElectronMenu.ts | 6 ++++- apps/desktop/src/ipc/methods/window.ts | 2 ++ apps/desktop/src/preload.ts | 3 ++- apps/web/src/components/ChatMarkdown.tsx | 11 +++++--- apps/web/src/localApi.test.ts | 24 ++++++++++++++++- apps/web/src/localApi.ts | 25 +++++++++++++++++- packages/contracts/src/ipc.ts | 2 ++ 8 files changed, 91 insertions(+), 8 deletions(-) diff --git a/apps/desktop/src/electron/ElectronMenu.test.ts b/apps/desktop/src/electron/ElectronMenu.test.ts index 756274a614d7..477edca6120a 100644 --- a/apps/desktop/src/electron/ElectronMenu.test.ts +++ b/apps/desktop/src/electron/ElectronMenu.test.ts @@ -58,6 +58,32 @@ describe("ElectronMenu", () => { }).pipe(Effect.provide(TestLayer)), ); + it.effect("preserves keyboard invocation without changing the default pointer menu", () => + Effect.gen(function* () { + const electronMenu = yield* ElectronMenu.ElectronMenu; + for (const sourceType of [undefined, "mouse", "keyboard"] as const) { + let popupOptions: Electron.PopupOptions | undefined; + buildFromTemplateMock.mockImplementation(() => ({ + popup: (options: Electron.PopupOptions) => { + popupOptions = options; + options.callback?.(); + }, + })); + const selectedItemId = yield* electronMenu.showContextMenu({ + window: makeWindow(2), + items: [{ id: "copy", label: "Copy Link" }], + position: Option.some({ x: 10, y: 20 }), + ...(sourceType === undefined ? {} : { sourceType }), + }); + assert.isTrue(Option.isNone(selectedItemId)); + assert.equal(popupOptions?.sourceType, sourceType); + assert.equal(popupOptions?.x, 20); + assert.equal(popupOptions?.y, 40); + if (sourceType === undefined) assert.notProperty(popupOptions, "sourceType"); + } + }).pipe(Effect.provide(TestLayer)), + ); + it.effect("resolves with the clicked leaf item id", () => Effect.gen(function* () { buildFromTemplateMock.mockImplementation( diff --git a/apps/desktop/src/electron/ElectronMenu.ts b/apps/desktop/src/electron/ElectronMenu.ts index b241619cd296..53cf71b42e70 100644 --- a/apps/desktop/src/electron/ElectronMenu.ts +++ b/apps/desktop/src/electron/ElectronMenu.ts @@ -17,6 +17,7 @@ export interface ElectronMenuContextInput { readonly window: Electron.BrowserWindow; readonly items: readonly ContextMenuItem[]; readonly position: Option.Option; + readonly sourceType?: "mouse" | "keyboard"; } export interface ElectronMenuTemplateInput { @@ -252,7 +253,10 @@ export const make = Effect.gen(function* () { callback: () => complete(Option.none()), }), }); - menu.popup(popupOptions); + menu.popup({ + ...popupOptions, + ...(input.sourceType === undefined ? {} : { sourceType: input.sourceType }), + }); } catch (cause) { if (completed) { return; diff --git a/apps/desktop/src/ipc/methods/window.ts b/apps/desktop/src/ipc/methods/window.ts index 61de1361a311..b3ae3633edef 100644 --- a/apps/desktop/src/ipc/methods/window.ts +++ b/apps/desktop/src/ipc/methods/window.ts @@ -50,6 +50,7 @@ const ContextMenuPosition = Schema.Struct({ const ContextMenuInput = Schema.Struct({ items: Schema.Array(ContextMenuItemSchema), position: Schema.optionalKey(ContextMenuPosition), + sourceType: Schema.optionalKey(Schema.Literals(["mouse", "keyboard"])), }); function toWebSocketBaseUrl(httpBaseUrl: URL): string { @@ -284,6 +285,7 @@ export const showContextMenu = DesktopIpc.makeIpcMethod({ window: window.value, items: input.items, position: Option.fromNullishOr(input.position), + ...(input.sourceType === undefined ? {} : { sourceType: input.sourceType }), }); return Option.getOrNull(selectedItemId); }), diff --git a/apps/desktop/src/preload.ts b/apps/desktop/src/preload.ts index 74001dd785d3..9755a0a0f446 100644 --- a/apps/desktop/src/preload.ts +++ b/apps/desktop/src/preload.ts @@ -110,10 +110,11 @@ contextBridge.exposeInMainWorld("desktopBridge", { ipcRenderer.invoke(IpcChannels.PICK_PROJECT_FAVICON_CHANNEL, initialPath), pickThemeFiles: () => ipcRenderer.invoke(IpcChannels.PICK_THEME_FILES_CHANNEL, undefined), setTheme: (theme) => ipcRenderer.invoke(IpcChannels.SET_THEME_CHANNEL, theme), - showContextMenu: (items, position) => + showContextMenu: (items, position, sourceType) => ipcRenderer.invoke(IpcChannels.CONTEXT_MENU_CHANNEL, { items, ...(position === undefined ? {} : { position }), + ...(sourceType === undefined ? {} : { sourceType }), }), openExternal: (url: string) => ipcRenderer.invoke(IpcChannels.OPEN_EXTERNAL_CHANNEL, url), openSystemSettings: (pane: string) => diff --git a/apps/web/src/components/ChatMarkdown.tsx b/apps/web/src/components/ChatMarkdown.tsx index d72001eb9ddd..b046c84e81fd 100644 --- a/apps/web/src/components/ChatMarkdown.tsx +++ b/apps/web/src/components/ChatMarkdown.tsx @@ -140,7 +140,7 @@ import { shouldOpenMarkdownFileLinkInEditor, type MarkdownFileLinkMeta, } from "../markdown-links"; -import { readLocalApi } from "../localApi"; +import { contextMenuSourceType, readLocalApi } from "../localApi"; import { useAssetUrlRefresh, useAssetUrlState } from "../assets/assetUrls"; import { cn } from "../lib/utils"; import { useRemoteOpenResolution, type RemoteOpenMode } from "../remoteOpen"; @@ -1969,7 +1969,7 @@ const MarkdownFileLink = memo(function MarkdownFileLink({ ); const showFileContextMenu = useCallback( - async (position: { x: number; y: number }) => { + async (position: { x: number; y: number }, sourceType?: "mouse" | "keyboard") => { const api = readLocalApi(); if (!api) return; @@ -1986,6 +1986,7 @@ const MarkdownFileLink = memo(function MarkdownFileLink({ { id: "copy-full", label: "Copy full path" }, ] as const, position, + sourceType, ); if (clicked === "preview-media") { @@ -2045,7 +2046,7 @@ const MarkdownFileLink = memo(function MarkdownFileLink({ return { x: bounds.left, y: bounds.bottom }; })() : { x: event.clientX, y: event.clientY }; - void showFileContextMenu(position); + void showFileContextMenu(position, contextMenuSourceType(event.nativeEvent)); }, [showFileContextMenu], ); @@ -2851,6 +2852,7 @@ const CHAT_MARKDOWN_COMPONENTS = { const api = readLocalApi(); if (!api) return; const pullRequest = resolveThreadPullRequest(href); + const sourceType = contextMenuSourceType(event.nativeEvent); const currentPullRequest = threadRef === undefined ? null : readThreadShell(threadRef)?.linkedPullRequest; const threadLinkAction = @@ -2864,7 +2866,8 @@ const CHAT_MARKDOWN_COMPONENTS = { canOpenInPreview, threadLinkAction, position: { x: event.clientX, y: event.clientY }, - showContextMenu: (items, position) => api.contextMenu.show(items, position), + showContextMenu: (items, position) => + api.contextMenu.show(items, position, sourceType), openInPreview: async (target) => { const result = await openExternalLinkInPreview(target); if (result._tag === "Failure" && !isAtomCommandInterrupted(result)) { diff --git a/apps/web/src/localApi.test.ts b/apps/web/src/localApi.test.ts index 33c53a86c4e2..ab6a284ade78 100644 --- a/apps/web/src/localApi.test.ts +++ b/apps/web/src/localApi.test.ts @@ -70,6 +70,26 @@ afterEach(() => { }); describe("LocalApi", () => { + it.each([ + { type: "contextmenu", button: -1, detail: 0, touch: false, expected: "keyboard" }, + { type: "contextmenu", button: 2, detail: 0, touch: false, expected: undefined }, + { type: "contextmenu", button: -1, detail: 0, touch: true, expected: undefined }, + { type: "contextmenu", button: -1, detail: 0, touch: undefined, expected: undefined }, + { type: "click", button: 0, detail: 0, touch: false, expected: "keyboard" }, + { type: "click", button: 0, detail: 1, touch: false, expected: undefined }, + { type: "click", button: 0, detail: 1, touch: true, expected: undefined }, + ])("classifies menu invocation $type/$button/$detail/touch=$touch", async (input) => { + const { contextMenuSourceType } = await import("./localApi"); + expect( + contextMenuSourceType({ + type: input.type, + button: input.button, + detail: input.detail, + sourceCapabilities: input.touch === undefined ? null : { firesTouchEvents: input.touch }, + }), + ).toBe(input.expected); + }); + it("keeps backend operations out of the local host facade", async () => { const { createLocalApi } = await import("./localApi"); const api = createLocalApi(); @@ -144,7 +164,9 @@ describe("LocalApi", () => { await expect(api.persistence.getClientSettings()).resolves.toEqual(DEFAULT_CLIENT_SETTINGS); await api.persistence.setClientSettings(DEFAULT_CLIENT_SETTINGS); - expect(showContextMenu).toHaveBeenCalledWith(items, undefined); + expect(showContextMenu).toHaveBeenCalledWith(items, undefined, undefined); + await api.contextMenu.show(items, { x: 4, y: 5 }, "keyboard"); + expect(showContextMenu).toHaveBeenLastCalledWith(items, { x: 4, y: 5 }, "keyboard"); expect(pickFolder).toHaveBeenCalledWith({ initialPath: "/tmp" }); expect(getClientSettings).toHaveBeenCalledTimes(1); expect(setClientSettings).toHaveBeenCalledWith(DEFAULT_CLIENT_SETTINGS); diff --git a/apps/web/src/localApi.ts b/apps/web/src/localApi.ts index cafd04f9c048..158657de4c7a 100644 --- a/apps/web/src/localApi.ts +++ b/apps/web/src/localApi.ts @@ -6,6 +6,24 @@ import { readBrowserClientSettings, writeBrowserClientSettings } from "./clientP let cachedApi: LocalApi | undefined; +export function contextMenuSourceType( + event: Pick & { + sourceCapabilities?: { firesTouchEvents: boolean } | null; + }, +) { + // Chromium uses no button for keyboard context menus and touch long-press. + // Only the latter reports touch capabilities. Keep unknown sources unchanged. + if ( + (event.type === "contextmenu" && + event.button === -1 && + event.sourceCapabilities?.firesTouchEvents === false) || + (event.type === "click" && event.detail === 0) + ) { + return "keyboard"; + } + return undefined; +} + function createBrowserLocalApi(): LocalApi { return { dialogs: { @@ -45,9 +63,14 @@ function createBrowserLocalApi(): LocalApi { show: async ( items: readonly ContextMenuItem[], position?: { x: number; y: number }, + sourceType?: "mouse" | "keyboard", ): Promise => { if (window.desktopBridge) { - return window.desktopBridge.showContextMenu(items, position) as Promise; + return window.desktopBridge.showContextMenu( + items, + position, + sourceType, + ) as Promise; } return showContextMenuFallback(items, position); }, diff --git a/packages/contracts/src/ipc.ts b/packages/contracts/src/ipc.ts index a14a4ed0a08a..8358e7f62aa9 100644 --- a/packages/contracts/src/ipc.ts +++ b/packages/contracts/src/ipc.ts @@ -1124,6 +1124,7 @@ export interface DesktopBridge { showContextMenu: ( items: readonly ContextMenuItem[], position?: { x: number; y: number }, + sourceType?: "mouse" | "keyboard", ) => Promise; openExternal: (url: string) => Promise; /** @@ -1286,6 +1287,7 @@ export interface LocalApi { show: ( items: readonly ContextMenuItem[], position?: { x: number; y: number }, + sourceType?: "mouse" | "keyboard", ) => Promise; close: () => Promise; }; From 354fb35a33b0800a7847234328cd6945fad8a917 Mon Sep 17 00:00:00 2001 From: Andrew Date: Sun, 6 Sep 2026 14:27:37 -0400 Subject: [PATCH 2/3] fix(web): keep message link menu focus in the page --- .../desktop/src/electron/ElectronMenu.test.ts | 26 --- apps/desktop/src/electron/ElectronMenu.ts | 6 +- apps/desktop/src/ipc/methods/window.ts | 2 - apps/desktop/src/preload.ts | 3 +- apps/web/src/components/ChatMarkdown.tsx | 207 +++++++++--------- .../components/chat/useMessageLinkMenu.tsx | 80 +++++++ apps/web/src/localApi.test.ts | 24 +- apps/web/src/localApi.ts | 25 +-- packages/contracts/src/ipc.ts | 2 - 9 files changed, 192 insertions(+), 183 deletions(-) create mode 100644 apps/web/src/components/chat/useMessageLinkMenu.tsx diff --git a/apps/desktop/src/electron/ElectronMenu.test.ts b/apps/desktop/src/electron/ElectronMenu.test.ts index 477edca6120a..756274a614d7 100644 --- a/apps/desktop/src/electron/ElectronMenu.test.ts +++ b/apps/desktop/src/electron/ElectronMenu.test.ts @@ -58,32 +58,6 @@ describe("ElectronMenu", () => { }).pipe(Effect.provide(TestLayer)), ); - it.effect("preserves keyboard invocation without changing the default pointer menu", () => - Effect.gen(function* () { - const electronMenu = yield* ElectronMenu.ElectronMenu; - for (const sourceType of [undefined, "mouse", "keyboard"] as const) { - let popupOptions: Electron.PopupOptions | undefined; - buildFromTemplateMock.mockImplementation(() => ({ - popup: (options: Electron.PopupOptions) => { - popupOptions = options; - options.callback?.(); - }, - })); - const selectedItemId = yield* electronMenu.showContextMenu({ - window: makeWindow(2), - items: [{ id: "copy", label: "Copy Link" }], - position: Option.some({ x: 10, y: 20 }), - ...(sourceType === undefined ? {} : { sourceType }), - }); - assert.isTrue(Option.isNone(selectedItemId)); - assert.equal(popupOptions?.sourceType, sourceType); - assert.equal(popupOptions?.x, 20); - assert.equal(popupOptions?.y, 40); - if (sourceType === undefined) assert.notProperty(popupOptions, "sourceType"); - } - }).pipe(Effect.provide(TestLayer)), - ); - it.effect("resolves with the clicked leaf item id", () => Effect.gen(function* () { buildFromTemplateMock.mockImplementation( diff --git a/apps/desktop/src/electron/ElectronMenu.ts b/apps/desktop/src/electron/ElectronMenu.ts index 53cf71b42e70..b241619cd296 100644 --- a/apps/desktop/src/electron/ElectronMenu.ts +++ b/apps/desktop/src/electron/ElectronMenu.ts @@ -17,7 +17,6 @@ export interface ElectronMenuContextInput { readonly window: Electron.BrowserWindow; readonly items: readonly ContextMenuItem[]; readonly position: Option.Option; - readonly sourceType?: "mouse" | "keyboard"; } export interface ElectronMenuTemplateInput { @@ -253,10 +252,7 @@ export const make = Effect.gen(function* () { callback: () => complete(Option.none()), }), }); - menu.popup({ - ...popupOptions, - ...(input.sourceType === undefined ? {} : { sourceType: input.sourceType }), - }); + menu.popup(popupOptions); } catch (cause) { if (completed) { return; diff --git a/apps/desktop/src/ipc/methods/window.ts b/apps/desktop/src/ipc/methods/window.ts index b3ae3633edef..61de1361a311 100644 --- a/apps/desktop/src/ipc/methods/window.ts +++ b/apps/desktop/src/ipc/methods/window.ts @@ -50,7 +50,6 @@ const ContextMenuPosition = Schema.Struct({ const ContextMenuInput = Schema.Struct({ items: Schema.Array(ContextMenuItemSchema), position: Schema.optionalKey(ContextMenuPosition), - sourceType: Schema.optionalKey(Schema.Literals(["mouse", "keyboard"])), }); function toWebSocketBaseUrl(httpBaseUrl: URL): string { @@ -285,7 +284,6 @@ export const showContextMenu = DesktopIpc.makeIpcMethod({ window: window.value, items: input.items, position: Option.fromNullishOr(input.position), - ...(input.sourceType === undefined ? {} : { sourceType: input.sourceType }), }); return Option.getOrNull(selectedItemId); }), diff --git a/apps/desktop/src/preload.ts b/apps/desktop/src/preload.ts index 9755a0a0f446..74001dd785d3 100644 --- a/apps/desktop/src/preload.ts +++ b/apps/desktop/src/preload.ts @@ -110,11 +110,10 @@ contextBridge.exposeInMainWorld("desktopBridge", { ipcRenderer.invoke(IpcChannels.PICK_PROJECT_FAVICON_CHANNEL, initialPath), pickThemeFiles: () => ipcRenderer.invoke(IpcChannels.PICK_THEME_FILES_CHANNEL, undefined), setTheme: (theme) => ipcRenderer.invoke(IpcChannels.SET_THEME_CHANNEL, theme), - showContextMenu: (items, position, sourceType) => + showContextMenu: (items, position) => ipcRenderer.invoke(IpcChannels.CONTEXT_MENU_CHANNEL, { items, ...(position === undefined ? {} : { position }), - ...(sourceType === undefined ? {} : { sourceType }), }), openExternal: (url: string) => ipcRenderer.invoke(IpcChannels.OPEN_EXTERNAL_CHANNEL, url), openSystemSettings: (pane: string) => diff --git a/apps/web/src/components/ChatMarkdown.tsx b/apps/web/src/components/ChatMarkdown.tsx index b046c84e81fd..fa4786321478 100644 --- a/apps/web/src/components/ChatMarkdown.tsx +++ b/apps/web/src/components/ChatMarkdown.tsx @@ -140,7 +140,8 @@ import { shouldOpenMarkdownFileLinkInEditor, type MarkdownFileLinkMeta, } from "../markdown-links"; -import { contextMenuSourceType, readLocalApi } from "../localApi"; +import { readLocalApi } from "../localApi"; +import { useMessageLinkMenu } from "./chat/useMessageLinkMenu"; import { useAssetUrlRefresh, useAssetUrlState } from "../assets/assetUrls"; import { cn } from "../lib/utils"; import { useRemoteOpenResolution, type RemoteOpenMode } from "../remoteOpen"; @@ -1803,6 +1804,7 @@ const MarkdownFileLink = memo(function MarkdownFileLink({ revealLabel, className, }: MarkdownFileLinkProps) { + const { show: showLinkMenu, menu: linkMenu } = useMessageLinkMenu(); const handleOpenInEditor = useCallback(() => { if (!onOpen) { return; @@ -1969,12 +1971,9 @@ const MarkdownFileLink = memo(function MarkdownFileLink({ ); const showFileContextMenu = useCallback( - async (position: { x: number; y: number }, sourceType?: "mouse" | "keyboard") => { - const api = readLocalApi(); - if (!api) return; - + async (position: { x: number; y: number }, trigger: HTMLElement) => { try { - const clicked = await api.contextMenu.show( + const clicked = await showLinkMenu( [ ...(onOpenMedia ? ([{ id: "preview-media", label: "Preview media" }] as const) : []), ...(onOpen ? ([{ id: "open", label: openInEditorMenuLabel }] as const) : []), @@ -1986,7 +1985,7 @@ const MarkdownFileLink = memo(function MarkdownFileLink({ { id: "copy-full", label: "Copy full path" }, ] as const, position, - sourceType, + trigger, ); if (clicked === "preview-media") { @@ -2020,6 +2019,7 @@ const MarkdownFileLink = memo(function MarkdownFileLink({ } }, [ + showLinkMenu, displayPath, handleCopy, handleOpenInBrowser, @@ -2046,7 +2046,7 @@ const MarkdownFileLink = memo(function MarkdownFileLink({ return { x: bounds.left, y: bounds.bottom }; })() : { x: event.clientX, y: event.clientY }; - void showFileContextMenu(position, contextMenuSourceType(event.nativeEvent)); + void showFileContextMenu(position, event.currentTarget); }, [showFileContextMenu], ); @@ -2068,66 +2068,69 @@ const MarkdownFileLink = memo(function MarkdownFileLink({ }); return ( - - { - event.preventDefault(); - event.stopPropagation(); - if (onOpen && shouldOpenMarkdownFileLinkInEditor(event)) { - handleOpenInEditor(); - return; - } - if (useBrowserPrimaryAction) { - handleOpenInBrowser(); - return; - } - handleOpenInFilePreview(); - }} - onContextMenu={handleContextMenu} - > - - - ) : ( - - ) - } - /> - - {/* The full path: the chip already shows the shortened form, and a link + <> + {linkMenu} + + { + event.preventDefault(); + event.stopPropagation(); + if (onOpen && shouldOpenMarkdownFileLinkInEditor(event)) { + handleOpenInEditor(); + return; + } + if (useBrowserPrimaryAction) { + handleOpenInBrowser(); + return; + } + handleOpenInFilePreview(); + }} + onContextMenu={handleContextMenu} + > + + + ) : ( + + ) + } + /> + + {/* The full path: the chip already shows the shortened form, and a link to the workspace root collapses to a bare label that repeats it. */} -
- {targetPath} -
- - +
+ {targetPath} +
+ + + ); }, areMarkdownFileLinkPropsEqual); @@ -2716,6 +2719,7 @@ const CHAT_MARKDOWN_COMPONENTS = { ); }, a: function MarkdownAnchor({ node, href, children, title: _title, ...props }) { + const { show: showLinkMenu, menu: linkMenu } = useMessageLinkMenu(); const { cwd, environmentId, @@ -2851,8 +2855,8 @@ const CHAT_MARKDOWN_COMPONENTS = { event.stopPropagation(); const api = readLocalApi(); if (!api) return; + const trigger = event.currentTarget; const pullRequest = resolveThreadPullRequest(href); - const sourceType = contextMenuSourceType(event.nativeEvent); const currentPullRequest = threadRef === undefined ? null : readThreadShell(threadRef)?.linkedPullRequest; const threadLinkAction = @@ -2866,8 +2870,7 @@ const CHAT_MARKDOWN_COMPONENTS = { canOpenInPreview, threadLinkAction, position: { x: event.clientX, y: event.clientY }, - showContextMenu: (items, position) => - api.contextMenu.show(items, position, sourceType), + showContextMenu: (items, position) => showLinkMenu(items, position, trigger), openInPreview: async (target) => { const result = await openExternalLinkInPreview(target); if (result._tag === "Failure" && !isAtomCommandInterrupted(result)) { @@ -2915,38 +2918,44 @@ const CHAT_MARKDOWN_COMPONENTS = { } if (pullRequestPreviewTarget !== null) { return ( - - openChangeRequestLink( - { - metaKey: false, - ctrlKey: false, - preventDefault: () => undefined, - stopPropagation: () => undefined, - }, - targetUrl, - undefined, - environmentId ?? undefined, - ) - } - onOpenFallback={openDeferredMarkdownLink} - /> + <> + {linkMenu} + + openChangeRequestLink( + { + metaKey: false, + ctrlKey: false, + preventDefault: () => undefined, + stopPropagation: () => undefined, + }, + targetUrl, + undefined, + environmentId ?? undefined, + ) + } + onOpenFallback={openDeferredMarkdownLink} + /> + ); } return ( - - - - {href} - - + <> + {linkMenu} + + + + {href} + + + ); } diff --git a/apps/web/src/components/chat/useMessageLinkMenu.tsx b/apps/web/src/components/chat/useMessageLinkMenu.tsx new file mode 100644 index 000000000000..8d8fd91044ca --- /dev/null +++ b/apps/web/src/components/chat/useMessageLinkMenu.tsx @@ -0,0 +1,80 @@ +import { useCallback, useEffect, useRef, useState } from "react"; + +import { Menu, MenuItem, MenuPopup } from "../ui/menu"; + +interface LinkMenuItem { + id: string; + label: string; +} + +interface LinkMenuState { + items: readonly LinkMenuItem[]; + trigger: HTMLElement; + position: { x: number; y: number }; +} + +/** Keeps message link focus and menu focus in the same accessibility tree. */ +export function useMessageLinkMenu() { + const [state, setState] = useState(null); + const pending = useRef<((value: string | null) => void) | null>(null); + const complete = useCallback((value: string | null) => { + const resolve = pending.current; + pending.current = null; + setState(null); + resolve?.(value); + }, []); + + useEffect(() => () => pending.current?.(null), []); + + const show = useCallback( + ( + items: readonly { id: T; label: string }[], + position: { x: number; y: number }, + trigger: HTMLElement, + ): Promise => { + pending.current?.(null); + pending.current = null; + if (items.length === 0) { + setState(null); + return Promise.resolve(null); + } + return new Promise((resolve) => { + pending.current = (value) => resolve(value as T | null); + setState({ items, position, trigger }); + }); + }, + [], + ); + + const menu = state && ( + { + if (!open) complete(null); + }} + > + new DOMRect(state.position.x, state.position.y, 0, 0), + }} + aria-label="Link options" + finalFocus={() => state.trigger} + onFocus={(event) => { + if (event.target === event.currentTarget) { + event.currentTarget.querySelector('[role="menuitem"]')?.focus(); + } + }} + > + {state.items.map((item) => ( + complete(item.id)}> + {item.label} + + ))} + + + ); + + return { show, menu }; +} diff --git a/apps/web/src/localApi.test.ts b/apps/web/src/localApi.test.ts index ab6a284ade78..33c53a86c4e2 100644 --- a/apps/web/src/localApi.test.ts +++ b/apps/web/src/localApi.test.ts @@ -70,26 +70,6 @@ afterEach(() => { }); describe("LocalApi", () => { - it.each([ - { type: "contextmenu", button: -1, detail: 0, touch: false, expected: "keyboard" }, - { type: "contextmenu", button: 2, detail: 0, touch: false, expected: undefined }, - { type: "contextmenu", button: -1, detail: 0, touch: true, expected: undefined }, - { type: "contextmenu", button: -1, detail: 0, touch: undefined, expected: undefined }, - { type: "click", button: 0, detail: 0, touch: false, expected: "keyboard" }, - { type: "click", button: 0, detail: 1, touch: false, expected: undefined }, - { type: "click", button: 0, detail: 1, touch: true, expected: undefined }, - ])("classifies menu invocation $type/$button/$detail/touch=$touch", async (input) => { - const { contextMenuSourceType } = await import("./localApi"); - expect( - contextMenuSourceType({ - type: input.type, - button: input.button, - detail: input.detail, - sourceCapabilities: input.touch === undefined ? null : { firesTouchEvents: input.touch }, - }), - ).toBe(input.expected); - }); - it("keeps backend operations out of the local host facade", async () => { const { createLocalApi } = await import("./localApi"); const api = createLocalApi(); @@ -164,9 +144,7 @@ describe("LocalApi", () => { await expect(api.persistence.getClientSettings()).resolves.toEqual(DEFAULT_CLIENT_SETTINGS); await api.persistence.setClientSettings(DEFAULT_CLIENT_SETTINGS); - expect(showContextMenu).toHaveBeenCalledWith(items, undefined, undefined); - await api.contextMenu.show(items, { x: 4, y: 5 }, "keyboard"); - expect(showContextMenu).toHaveBeenLastCalledWith(items, { x: 4, y: 5 }, "keyboard"); + expect(showContextMenu).toHaveBeenCalledWith(items, undefined); expect(pickFolder).toHaveBeenCalledWith({ initialPath: "/tmp" }); expect(getClientSettings).toHaveBeenCalledTimes(1); expect(setClientSettings).toHaveBeenCalledWith(DEFAULT_CLIENT_SETTINGS); diff --git a/apps/web/src/localApi.ts b/apps/web/src/localApi.ts index 158657de4c7a..cafd04f9c048 100644 --- a/apps/web/src/localApi.ts +++ b/apps/web/src/localApi.ts @@ -6,24 +6,6 @@ import { readBrowserClientSettings, writeBrowserClientSettings } from "./clientP let cachedApi: LocalApi | undefined; -export function contextMenuSourceType( - event: Pick & { - sourceCapabilities?: { firesTouchEvents: boolean } | null; - }, -) { - // Chromium uses no button for keyboard context menus and touch long-press. - // Only the latter reports touch capabilities. Keep unknown sources unchanged. - if ( - (event.type === "contextmenu" && - event.button === -1 && - event.sourceCapabilities?.firesTouchEvents === false) || - (event.type === "click" && event.detail === 0) - ) { - return "keyboard"; - } - return undefined; -} - function createBrowserLocalApi(): LocalApi { return { dialogs: { @@ -63,14 +45,9 @@ function createBrowserLocalApi(): LocalApi { show: async ( items: readonly ContextMenuItem[], position?: { x: number; y: number }, - sourceType?: "mouse" | "keyboard", ): Promise => { if (window.desktopBridge) { - return window.desktopBridge.showContextMenu( - items, - position, - sourceType, - ) as Promise; + return window.desktopBridge.showContextMenu(items, position) as Promise; } return showContextMenuFallback(items, position); }, diff --git a/packages/contracts/src/ipc.ts b/packages/contracts/src/ipc.ts index 8358e7f62aa9..a14a4ed0a08a 100644 --- a/packages/contracts/src/ipc.ts +++ b/packages/contracts/src/ipc.ts @@ -1124,7 +1124,6 @@ export interface DesktopBridge { showContextMenu: ( items: readonly ContextMenuItem[], position?: { x: number; y: number }, - sourceType?: "mouse" | "keyboard", ) => Promise; openExternal: (url: string) => Promise; /** @@ -1287,7 +1286,6 @@ export interface LocalApi { show: ( items: readonly ContextMenuItem[], position?: { x: number; y: number }, - sourceType?: "mouse" | "keyboard", ) => Promise; close: () => Promise; }; From 75c7277561dc34845624d5ef409d5e910f23571e Mon Sep 17 00:00:00 2001 From: Andrew Johnson Date: Fri, 11 Sep 2026 23:16:07 +0000 Subject: [PATCH 3/3] fix(web): anchor keyboard link menus beside their trigger --- apps/web/src/components/ChatMarkdown.tsx | 9 +-- .../chat/useMessageLinkMenu.test.tsx | 59 +++++++++++++++++++ .../components/chat/useMessageLinkMenu.tsx | 4 ++ 3 files changed, 64 insertions(+), 8 deletions(-) create mode 100644 apps/web/src/components/chat/useMessageLinkMenu.test.tsx diff --git a/apps/web/src/components/ChatMarkdown.tsx b/apps/web/src/components/ChatMarkdown.tsx index 24447f1ef3ce..e2f3bae90913 100644 --- a/apps/web/src/components/ChatMarkdown.tsx +++ b/apps/web/src/components/ChatMarkdown.tsx @@ -2060,14 +2060,7 @@ const MarkdownFileLink = memo(function MarkdownFileLink({ (event: ReactMouseEvent) => { event.preventDefault(); event.stopPropagation(); - const position = - event.clientX === 0 && event.clientY === 0 - ? (() => { - const bounds = event.currentTarget.getBoundingClientRect(); - return { x: bounds.left, y: bounds.bottom }; - })() - : { x: event.clientX, y: event.clientY }; - void showFileContextMenu(position, event.currentTarget); + void showFileContextMenu({ x: event.clientX, y: event.clientY }, event.currentTarget); }, [showFileContextMenu], ); diff --git a/apps/web/src/components/chat/useMessageLinkMenu.test.tsx b/apps/web/src/components/chat/useMessageLinkMenu.test.tsx new file mode 100644 index 000000000000..a0192ffa5038 --- /dev/null +++ b/apps/web/src/components/chat/useMessageLinkMenu.test.tsx @@ -0,0 +1,59 @@ +import { act, createRef, useImperativeHandle } from "react"; +import { create, type ReactTestRenderer } from "react-test-renderer"; +import { afterEach, describe, expect, it, vi } from "vite-plus/test"; + +import { useMessageLinkMenu } from "./useMessageLinkMenu"; + +let renderer: ReactTestRenderer | undefined; + +afterEach(async () => { + await act(() => renderer?.unmount()); + renderer = undefined; + vi.unstubAllGlobals(); +}); + +describe("message link menu positioning", () => { + it.each([ + { position: { x: 0, y: 0 }, expected: { x: 120, y: 260 } }, + { position: { x: 145, y: 245 }, expected: { x: 145, y: 245 } }, + { position: { x: 0, y: 245 }, expected: { x: 0, y: 245 } }, + { position: { x: 145, y: 0 }, expected: { x: 145, y: 0 } }, + ])("anchors a menu opened at $position to $expected", async ({ position, expected }) => { + vi.stubGlobal("IS_REACT_ACT_ENVIRONMENT", true); + vi.stubGlobal( + "DOMRect", + class { + constructor( + readonly x: number, + readonly y: number, + readonly width: number, + readonly height: number, + ) {} + }, + ); + const linkMenuRef = createRef>(); + function TestLinkMenu() { + const linkMenu = useMessageLinkMenu(); + useImperativeHandle(linkMenuRef, () => linkMenu, [linkMenu]); + return null; + } + const trigger = { + getBoundingClientRect: () => ({ left: 120, bottom: 260 }), + } as HTMLElement; + + await act(() => { + renderer = create(); + }); + await act(() => { + void linkMenuRef.current!.show([{ id: "copy-link", label: "Copy Link" }], position, trigger); + }); + + // Evaluate the virtual anchor consumed by the popup's positioning engine. + const popup = linkMenuRef.current!.menu?.props.children; + expect(popup.props.anchor.getBoundingClientRect()).toMatchObject({ + ...expected, + width: 0, + height: 0, + }); + }); +}); diff --git a/apps/web/src/components/chat/useMessageLinkMenu.tsx b/apps/web/src/components/chat/useMessageLinkMenu.tsx index 8d8fd91044ca..32e1c1f01593 100644 --- a/apps/web/src/components/chat/useMessageLinkMenu.tsx +++ b/apps/web/src/components/chat/useMessageLinkMenu.tsx @@ -38,6 +38,10 @@ export function useMessageLinkMenu() { setState(null); return Promise.resolve(null); } + if (position.x === 0 && position.y === 0) { + const bounds = trigger.getBoundingClientRect(); + position = { x: bounds.left, y: bounds.bottom }; + } return new Promise((resolve) => { pending.current = (value) => resolve(value as T | null); setState({ items, position, trigger });