From 9a4ef162bbb4da321253a108284daceb98c8f0e3 Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Sun, 8 Nov 2026 20:53:51 +0800 Subject: [PATCH] fix(keymap): open shortcut menu when activating an action (#2148) Double-clicking an action in Settings > Keymap went straight to the Keyboard Shortcut dialog, where mouse clicks do nothing, so trying to bind Alt+Click there got no reaction. Add Mouse Shortcut was reachable only by right-click. Double-click and Enter now open the same menu as right-click, at the pointer or the selected row. Sibling covered: the keyboard dialog no longer inherits mouse recording mode from an earlier Add Mouse Shortcut. Fixes #2126 --- e2e/keymap.spec.ts | 41 ++++++++++++ .../settings/__tests__/dialog.test.tsx | 67 ++++++++++++++++++- src/features/settings/keymap/KeymapPage.tsx | 14 +++- 3 files changed, 118 insertions(+), 4 deletions(-) create mode 100644 e2e/keymap.spec.ts diff --git a/e2e/keymap.spec.ts b/e2e/keymap.spec.ts new file mode 100644 index 000000000..3789dd1a8 --- /dev/null +++ b/e2e/keymap.spec.ts @@ -0,0 +1,41 @@ +import { expect, test } from "./fixtures"; + +for (const activation of ["double-click", "Enter"] as const) { + test(`Keymap ${activation} offers mouse and keyboard shortcuts`, async ({ workbench: page }) => { + await page.keyboard.press("Meta+,"); + const settings = page.getByRole("dialog", { name: "Settings", exact: true }); + await settings.getByRole("treeitem", { name: "Keymap", exact: true }).click(); + await settings.getByRole("textbox", { name: "Search actions", exact: true }).fill("Find Action"); + const row = settings.getByRole("tree", { name: "Actions" }).getByRole("treeitem", { name: /Find Action/ }); + const activate = async () => { + if (activation === "double-click") await row.dblclick(); + else { + await row.click(); + await page.keyboard.press("Enter"); + } + }; + await activate(); + const keyboard = page.getByRole("menuitem", { name: "Add Keyboard Shortcut", exact: true }); + await expect(keyboard).toBeFocused(); + await page.keyboard.press("ArrowDown"); + await expect(page.getByRole("menuitem", { name: "Add Mouse Shortcut", exact: true })).toBeFocused(); + await page.keyboard.press("Enter"); + const mouseDialog = page.getByRole("dialog", { name: "Mouse Shortcut: Find Action…", exact: true }); + await mouseDialog.getByRole("textbox", { name: "Mouse shortcut" }).click({ modifiers: ["Alt"] }); + await expect(mouseDialog.getByRole("button", { name: "OK", exact: true })).toBeEnabled(); + await mouseDialog.getByRole("button", { name: "OK", exact: true }).click(); + await page + .getByRole("dialog", { name: "Warning", exact: true }) + .getByRole("button", { name: "Leave", exact: true }) + .click(); + await expect(row).toContainText("Click"); + await activate(); + await keyboard.click(); + const keyboardDialog = page.getByRole("dialog", { name: "Keyboard Shortcut: Find Action…", exact: true }); + await expect(keyboardDialog.getByRole("textbox", { name: "First stroke" })).toBeFocused(); + await keyboardDialog.getByRole("button", { name: "Cancel", exact: true }).click(); + await row.click({ button: "right" }); + await expect(keyboard).toBeVisible(); + await expect(page.getByRole("menuitem", { name: "Add Mouse Shortcut", exact: true })).toBeVisible(); + }); +} diff --git a/src/features/settings/__tests__/dialog.test.tsx b/src/features/settings/__tests__/dialog.test.tsx index d4503c563..97a425f38 100644 --- a/src/features/settings/__tests__/dialog.test.tsx +++ b/src/features/settings/__tests__/dialog.test.tsx @@ -25,7 +25,7 @@ import { registerAction, executeAction } from "@/platform/actions"; import { usePlatform } from "@/platform/platformInfo"; import { useProject } from "@/platform/project"; import { useKeymap } from "@/platform/keymap"; -import { ContextMenuHost } from "@/ui"; +import { ContextMenuHost, hideContextMenu, useContextMenuStore } from "@/ui"; import { PageContext, useSetting } from "../components/context"; import { DEFAULT_SETTINGS, setAt } from "../model/schema"; import { openSettings, stage, useSettingsDialog } from "../model/staging"; @@ -53,6 +53,7 @@ beforeEach(() => { afterEach(() => { act(() => useSettingsDialog.setState({ open: false, projectStructureOpen: false })); + act(hideContextMenu); dispose(); }); @@ -328,8 +329,10 @@ describe("Settings dialog", () => { }); registerAction({ id: "t.mine", title: "My Action", category: "Code", when: "editorFocus", run: () => {} }); const dialog = await openDialog("keymap"); + render(); const row = within(dialog).getByText("My Action"); fireEvent.doubleClick(row); + fireEvent.click(await screen.findByRole("menuitem", { name: "Add Keyboard Shortcut" })); const recorder = await screen.findByRole("dialog", { name: /Keyboard Shortcut: My Action/ }); const first = within(recorder).getByRole("textbox", { name: "First stroke" }); fireEvent.keyDown(first, { code: "KeyD", key: "d", metaKey: true }); @@ -349,6 +352,67 @@ describe("Settings dialog", () => { expect(useSettingsDialog.getState().open).toBe(false); }); + it.each(["double-click", "Enter"])( + "opens the shortcut menu on %s and clears mouse recording mode", + async (activation) => { + render(); + registerAction({ id: "t.activate", title: "Activation Target", category: "Code", run: () => {} }); + const dialog = await openDialog("keymap"); + const row = within(dialog).getByText("Activation Target").closest('[role="treeitem"]')!; + const tree = within(dialog).getByRole("tree", { name: "Actions" }); + const rect = vi.spyOn(row, "getBoundingClientRect").mockReturnValue({ + left: 100, + bottom: 150, + top: 126, + right: 500, + width: 400, + height: 24, + x: 100, + y: 126, + toJSON: () => ({}), + }); + const activateRow = () => { + fireEvent.mouseDown(row, { button: 0 }); + if (activation === "double-click") fireEvent.doubleClick(row, { clientX: 220, clientY: 138 }); + else fireEvent.keyDown(tree, { key: "Enter" }); + }; + activateRow(); + expect(useContextMenuStore.getState().menu).toMatchObject( + activation === "double-click" ? { x: 220, y: 138 } : { x: 100, y: 150 }, + ); + expect(screen.queryByRole("dialog", { name: /Shortcut: Activation Target/ })).not.toBeInTheDocument(); + const keyboard = await screen.findByRole("menuitem", { name: "Add Keyboard Shortcut" }); + expect(keyboard).toHaveFocus(); + fireEvent.keyDown(keyboard, { key: "ArrowDown" }); + const mouse = screen.getByRole("menuitem", { name: "Add Mouse Shortcut" }); + expect(mouse).toHaveFocus(); + fireEvent.keyDown(mouse, { key: "Enter" }); + const recorder = await screen.findByRole("dialog", { name: "Mouse Shortcut: Activation Target" }); + fireEvent.mouseDown(within(recorder).getByRole("textbox", { name: "Mouse shortcut" }), { + button: 0, + altKey: true, + }); + fireEvent.click(within(recorder).getByRole("button", { name: "OK" })); + const overrides = useSettingsDialog.getState().staged["global|keymapOverrides"]?.value; + expect(overrides).toEqual({ "t.activate": ["Alt+Button1"] }); + activateRow(); + fireEvent.click(await screen.findByRole("menuitem", { name: "Add Keyboard Shortcut" })); + const keys = await screen.findByRole("dialog", { name: "Keyboard Shortcut: Activation Target" }); + expect(within(keys).getByRole("textbox", { name: "First stroke" })).toBeInTheDocument(); + fireEvent.click(within(keys).getByRole("button", { name: "Cancel" })); + fireEvent.contextMenu(row); + fireEvent.click(await screen.findByRole("menuitem", { name: "Add Mouse Shortcut" })); + fireEvent.click( + within(await screen.findByRole("dialog", { name: "Mouse Shortcut: Activation Target" })).getByRole("button", { + name: "Cancel", + }), + ); + fireEvent.click(within(dialog).getByRole("button", { name: "Add Keyboard Shortcut" })); + expect(await screen.findByRole("dialog", { name: "Keyboard Shortcut: Activation Target" })).toBeInTheDocument(); + rect.mockRestore(); + }, + ); + it("duplicates the keymap from the gear menu and edits the copy", async () => { render(); registerAction({ id: "t.dup", title: "Dup Target", category: "Code", run: () => {} }); @@ -364,6 +428,7 @@ describe("Settings dialog", () => { "custom.macos-copy": { name: "macOS copy", parent: "intellij-macos", overrides: {} }, }); fireEvent.doubleClick(within(dialog).getByText("Dup Target")); + fireEvent.click(await screen.findByRole("menuitem", { name: "Add Keyboard Shortcut" })); const recorder = await screen.findByRole("dialog", { name: /Keyboard Shortcut: Dup Target/ }); fireEvent.keyDown(within(recorder).getByRole("textbox", { name: "First stroke" }), { code: "F9", diff --git a/src/features/settings/keymap/KeymapPage.tsx b/src/features/settings/keymap/KeymapPage.tsx index 935aa9c8f..f97ad6654 100644 --- a/src/features/settings/keymap/KeymapPage.tsx +++ b/src/features/settings/keymap/KeymapPage.tsx @@ -2,8 +2,8 @@ * Keymap page (IDEA layout): keymap selector with a gear menu (Duplicate… / * Rename… / Restore Defaults / Delete), search by name or by recorded shortcut, * and the action tree grouped as Main Menu › File…Help, Editor Actions, Tool - * Windows, Other, Plugins. Add / Remove / Reset via the context menu (Enter / - * double-click adds). Edits are staged as the `keymap`, `keymapOverrides` + * Windows, Other, Plugins. Enter / double-click opens the context menu to add, + * remove or reset shortcuts. Edits are staged as the `keymap`, `keymapOverrides` * (bundled keymaps) and `customKeymaps` (user keymaps) settings, global scope. */ import { useMemo, useRef, useState, type MouseEvent } from "react"; @@ -396,7 +396,15 @@ export function KeymapPage() { multiSelect={false} speedSearch={false} onSelectionChange={(_sel, node) => setLead(node?.kind === "action" ? node.key : null)} - onActivate={(n) => n.kind === "action" && setRecordFor(n.action)} + onActivate={(n, e) => { + if (n.kind !== "action") return; + if ("clientX" in e) showContextMenu(e, menuFor(n.action)); + else { + const row = document.getElementById(e.currentTarget.getAttribute("aria-activedescendant") ?? ""); + const rect = row?.getBoundingClientRect(); + if (rect) showContextMenu({ x: rect.left, y: rect.bottom }, menuFor(n.action)); + } + }} onContextMenu={(n, _sel, e: MouseEvent) => { if (n?.kind !== "action") return; e.preventDefault();