From d3a6d4422c93d30700fa4aa31c36cf871598d881 Mon Sep 17 00:00:00 2001 From: Basit Balogun Date: Thu, 21 May 2026 16:46:38 +0100 Subject: [PATCH 1/7] feat: configurable keyboard shortcuts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Users can now remap any of the 17 configurable shortcuts via Settings > Shortcuts (or the ⌘/ sheet). Custom bindings fully replace all defaults (including alternates) and multiple custom combos per action are supported. Bindings persist across sessions via electronStorage. - Add `configurable` flag + `DEFAULT_KEYBINDINGS` map to keyboard-shortcuts.ts - New `keybindingsStore` (persist + electronStorage) with array-based custom combos, conflict detection helper, and individual/bulk reset - New `useShortcut(id)` hook — reactive Zustand selector, feeds useHotkeys - New `Keycap` component extracted to avoid circular imports - New `ShortcutRecorder` component: click + to enter recording mode, captures keydown, shows conflict toast, per-binding × remove, per-shortcut ↩ reset - Update all useHotkeys call sites (GlobalEventHandlers, SpaceSwitcher, usePanelKeyboardShortcuts, ExternalAppsOpener) to use useShortcut() - KeyboardShortcutsSheet: configurable rows render ShortcutRecorder instead of static keycaps; "Reset all shortcuts" button shown when customisations exist Generated-By: PostHog Code Task-Id: 80405bf7-239f-4b60-a1cf-5a4777fb7218 --- .../components/GlobalEventHandlers.tsx | 43 ++-- .../components/KeyboardShortcutsSheet.tsx | 90 ++++---- apps/code/src/renderer/components/Keycap.tsx | 42 ++++ .../renderer/components/ShortcutRecorder.tsx | 193 ++++++++++++++++++ .../src/renderer/components/SpaceSwitcher.tsx | 9 +- .../renderer/constants/keyboard-shortcuts.ts | 67 ++++++ .../panels/hooks/usePanelKeyboardShortcuts.ts | 4 +- .../components/ExternalAppsOpener.tsx | 9 +- apps/code/src/renderer/hooks/useShortcut.ts | 6 + .../src/renderer/stores/keybindingsStore.ts | 88 ++++++++ 10 files changed, 484 insertions(+), 67 deletions(-) create mode 100644 apps/code/src/renderer/components/Keycap.tsx create mode 100644 apps/code/src/renderer/components/ShortcutRecorder.tsx create mode 100644 apps/code/src/renderer/hooks/useShortcut.ts create mode 100644 apps/code/src/renderer/stores/keybindingsStore.ts diff --git a/apps/code/src/renderer/components/GlobalEventHandlers.tsx b/apps/code/src/renderer/components/GlobalEventHandlers.tsx index 8e8842eab..4b5379640 100644 --- a/apps/code/src/renderer/components/GlobalEventHandlers.tsx +++ b/apps/code/src/renderer/components/GlobalEventHandlers.tsx @@ -9,6 +9,7 @@ import { useSidebarStore } from "@features/sidebar/stores/sidebarStore"; import { useTasks } from "@features/tasks/hooks/useTasks"; import { useFocusWorkspace } from "@features/workspace/hooks/useFocusWorkspace"; import { useWorkspaces } from "@features/workspace/hooks/useWorkspace"; +import { useShortcut } from "@hooks/useShortcut"; import { SHORTCUTS } from "@renderer/constants/keyboard-shortcuts"; import { useTRPC } from "@renderer/trpc"; import type { Task } from "@shared/types"; @@ -157,33 +158,43 @@ export function GlobalEventHandlers({ preventDefault: true, } as const; - useHotkeys(SHORTCUTS.COMMAND_MENU, onToggleCommandMenu, { + const commandMenuKey = useShortcut("command-menu"); + const newTaskKey = useShortcut("new-task"); + const settingsKey = useShortcut("settings"); + const goBackKey = useShortcut("go-back"); + const goForwardKey = useShortcut("go-forward"); + const toggleLeftSidebarKey = useShortcut("toggle-left-sidebar"); + const toggleReviewPanelKey = useShortcut("toggle-review-panel"); + const shortcutsSheetKey = useShortcut("shortcuts"); + const inboxKey = useShortcut("inbox"); + const prevTaskKey = useShortcut("prev-task"); + const nextTaskKey = useShortcut("next-task"); + const toggleFocusKey = useShortcut("toggle-focus"); + + useHotkeys(commandMenuKey, onToggleCommandMenu, { ...globalOptions, enabled: !commandMenuOpen, }); - useHotkeys(SHORTCUTS.NEW_TASK, handleFocusTaskMode, globalOptions); - useHotkeys(SHORTCUTS.SETTINGS, handleOpenSettings, globalOptions); - useHotkeys(SHORTCUTS.GO_BACK, goBack, globalOptions); - useHotkeys(SHORTCUTS.GO_FORWARD, goForward, globalOptions); + useHotkeys(newTaskKey, handleFocusTaskMode, globalOptions); + useHotkeys(settingsKey, handleOpenSettings, globalOptions); + useHotkeys(goBackKey, goBack, globalOptions); + useHotkeys(goForwardKey, goForward, globalOptions); + const handleToggleReview = useCallback(() => { if (!currentTaskId) return; const mode = getReviewMode(currentTaskId); setReviewMode(currentTaskId, mode === "closed" ? "split" : "closed"); }, [currentTaskId, getReviewMode, setReviewMode]); - useHotkeys(SHORTCUTS.TOGGLE_LEFT_SIDEBAR, toggleLeftSidebar, globalOptions); - useHotkeys(SHORTCUTS.TOGGLE_REVIEW_PANEL, handleToggleReview, globalOptions); - useHotkeys(SHORTCUTS.SHORTCUTS_SHEET, onToggleShortcutsSheet, globalOptions); - useHotkeys(SHORTCUTS.INBOX, navigateToInbox, globalOptions); - useHotkeys(SHORTCUTS.PREV_TASK, handlePrevTask, globalOptions, [ - handlePrevTask, - ]); - useHotkeys(SHORTCUTS.NEXT_TASK, handleNextTask, globalOptions, [ - handleNextTask, - ]); + useHotkeys(toggleLeftSidebarKey, toggleLeftSidebar, globalOptions); + useHotkeys(toggleReviewPanelKey, handleToggleReview, globalOptions); + useHotkeys(shortcutsSheetKey, onToggleShortcutsSheet, globalOptions); + useHotkeys(inboxKey, navigateToInbox, globalOptions); + useHotkeys(prevTaskKey, handlePrevTask, globalOptions, [handlePrevTask]); + useHotkeys(nextTaskKey, handleNextTask, globalOptions, [handleNextTask]); useHotkeys( - SHORTCUTS.TOGGLE_FOCUS, + toggleFocusKey, handleToggleFocus, { ...globalOptions, diff --git a/apps/code/src/renderer/components/KeyboardShortcutsSheet.tsx b/apps/code/src/renderer/components/KeyboardShortcutsSheet.tsx index c5e973bf0..ca5c59365 100644 --- a/apps/code/src/renderer/components/KeyboardShortcutsSheet.tsx +++ b/apps/code/src/renderer/components/KeyboardShortcutsSheet.tsx @@ -1,49 +1,17 @@ -import { Box, Dialog, Flex, Text } from "@radix-ui/themes"; +import { Keycap } from "@components/Keycap"; +import { ShortcutRecorder } from "@components/ShortcutRecorder"; +import { Box, Button, Dialog, Flex, Text } from "@radix-ui/themes"; import { CATEGORY_LABELS, + type ConfigurableShortcutId, formatHotkeyParts, getShortcutsByCategory, type ShortcutCategory, } from "@renderer/constants/keyboard-shortcuts"; -import { useMemo, useState } from "react"; +import { useKeybindingsStore } from "@stores/keybindingsStore"; +import { useMemo } from "react"; import { useHotkeys } from "react-hotkeys-hook"; -function Keycap({ label, size = "md" }: { label: string; size?: "sm" | "md" }) { - const [pressed, setPressed] = useState(false); - const isSmall = size === "sm"; - const minW = isSmall ? "22px" : "28px"; - const h = isSmall ? "22px" : "28px"; - const fontSize = isSmall ? "11px" : "13px"; - const shadowSize = isSmall ? "2px" : "3px"; - - return ( - // biome-ignore lint/a11y/noStaticElementInteractions: cosmetic press animation - setPressed(true)} - onMouseUp={() => setPressed(false)} - onMouseLeave={() => setPressed(false)} - style={{ - minWidth: minW, - height: h, - fontSize, - fontFamily: "system-ui, -apple-system, sans-serif", - lineHeight: 1, - borderBottomWidth: pressed ? "1px" : shadowSize, - borderBottomColor: "var(--gray-7)", - transform: pressed - ? `translateY(${isSmall ? "1px" : "2px"})` - : "translateY(0)", - transition: - "transform 80ms ease-out, border-bottom-width 80ms ease-out", - }} - className="box-border inline-flex cursor-pointer select-none items-center justify-center rounded-[6px] border border-(--gray-5) bg-(--gray-3) px-[6px] py-0 font-medium text-(--gray-11)" - > - {label} - - ); -} - interface KeyboardShortcutsSheetProps { open: boolean; onOpenChange: (open: boolean) => void; @@ -110,6 +78,13 @@ function ShortcutsHeader() { export function KeyboardShortcutsList() { const shortcutsByCategory = useMemo(() => getShortcutsByCategory(), []); + const hasCustomBindings = useKeybindingsStore((s) => + Object.keys(s.customKeybindings).some( + (k) => + (s.customKeybindings[k as ConfigurableShortcutId]?.length ?? 0) > 0, + ), + ); + const resetAll = useKeybindingsStore((s) => s.resetAll); const categoryOrder: ShortcutCategory[] = [ "general", @@ -149,19 +124,46 @@ export function KeyboardShortcutsList() { align="center" justify="between" px="3" - className="border-b border-b-(--gray-4) pt-[6px] pb-[6px] last:border-b-0 odd:bg-(--gray-2) even:bg-(--gray-1)" + className="group border-b border-b-(--gray-4) pt-[6px] pb-[6px] last:border-b-0 odd:bg-(--gray-2) even:bg-(--gray-1)" > - {shortcut.description} - + + {shortcut.description} + {shortcut.context && ( + + {shortcut.context} + + )} + + {shortcut.configurable ? ( + + ) : ( + + )} ))} ); })} + + {hasCustomBindings && ( + + + + )} ); } diff --git a/apps/code/src/renderer/components/Keycap.tsx b/apps/code/src/renderer/components/Keycap.tsx new file mode 100644 index 000000000..e94f41919 --- /dev/null +++ b/apps/code/src/renderer/components/Keycap.tsx @@ -0,0 +1,42 @@ +import { useState } from "react"; + +interface KeycapProps { + label: string; + size?: "sm" | "md"; +} + +export function Keycap({ label, size = "md" }: KeycapProps) { + const [pressed, setPressed] = useState(false); + const isSmall = size === "sm"; + const minW = isSmall ? "22px" : "28px"; + const h = isSmall ? "22px" : "28px"; + const fontSize = isSmall ? "11px" : "13px"; + const shadowSize = isSmall ? "2px" : "3px"; + + return ( + // biome-ignore lint/a11y/noStaticElementInteractions: cosmetic press animation + setPressed(true)} + onMouseUp={() => setPressed(false)} + onMouseLeave={() => setPressed(false)} + style={{ + minWidth: minW, + height: h, + fontSize, + fontFamily: "system-ui, -apple-system, sans-serif", + lineHeight: 1, + borderBottomWidth: pressed ? "1px" : shadowSize, + borderBottomColor: "var(--gray-7)", + transform: pressed + ? `translateY(${isSmall ? "1px" : "2px"})` + : "translateY(0)", + transition: + "transform 80ms ease-out, border-bottom-width 80ms ease-out", + }} + className="box-border inline-flex cursor-pointer select-none items-center justify-center rounded-[6px] border border-(--gray-5) bg-(--gray-3) px-[6px] py-0 font-medium text-(--gray-11)" + > + {label} + + ); +} diff --git a/apps/code/src/renderer/components/ShortcutRecorder.tsx b/apps/code/src/renderer/components/ShortcutRecorder.tsx new file mode 100644 index 000000000..bdf67ecb1 --- /dev/null +++ b/apps/code/src/renderer/components/ShortcutRecorder.tsx @@ -0,0 +1,193 @@ +import { Keycap } from "@components/Keycap"; +import { ArrowCounterClockwise, Plus, X } from "@phosphor-icons/react"; +import { Flex, Text } from "@radix-ui/themes"; +import { + type ConfigurableShortcutId, + formatHotkeyParts, + KEYBOARD_SHORTCUTS, +} from "@renderer/constants/keyboard-shortcuts"; +import { findConflict, useKeybindingsStore } from "@stores/keybindingsStore"; +import { useCallback, useEffect, useRef, useState } from "react"; +import { toast } from "sonner"; + +function captureCombo(e: KeyboardEvent): string | null { + const bare = ["Meta", "Control", "Shift", "Alt"]; + if (bare.includes(e.key)) return null; + + const parts: string[] = []; + if (e.metaKey || e.ctrlKey) parts.push("mod"); + if (e.shiftKey) parts.push("shift"); + if (e.altKey) parts.push("alt"); + + const key = e.key.toLowerCase(); + parts.push(key); + return parts.join("+"); +} + +interface RecordingInputProps { + onCapture: (combo: string) => void; + onCancel: () => void; +} + +function RecordingInput({ onCapture, onCancel }: RecordingInputProps) { + const ref = useRef(null); + + useEffect(() => { + ref.current?.focus(); + }, []); + + const handleKeyDown = useCallback( + (e: React.KeyboardEvent) => { + e.preventDefault(); + e.stopPropagation(); + if (e.key === "Escape") { + onCancel(); + return; + } + const combo = captureCombo(e.nativeEvent); + if (combo) onCapture(combo); + }, + [onCapture, onCancel], + ); + + return ( + {}} + tabIndex={0} + onKeyDown={handleKeyDown} + onBlur={onCancel} + className="h-[28px] min-w-[120px] cursor-text rounded-[6px] border border-(--accent-8) bg-(--accent-2) px-2 text-center text-(--accent-11) text-[12px] outline-none ring-(--accent-8) ring-1 placeholder:text-(--accent-11)" + /> + ); +} + +interface ShortcutRecorderProps { + id: ConfigurableShortcutId; +} + +export function ShortcutRecorder({ id }: ShortcutRecorderProps) { + const [recording, setRecording] = useState(false); + const customs = useKeybindingsStore((s) => s.customKeybindings[id] ?? []); + const addKeybinding = useKeybindingsStore((s) => s.addKeybinding); + const removeKeybinding = useKeybindingsStore((s) => s.removeKeybinding); + const resetShortcut = useKeybindingsStore((s) => s.resetShortcut); + const hasCustom = customs.length > 0; + + const shortcutEntry = KEYBOARD_SHORTCUTS.find((s) => s.id === id); + + const handleCapture = useCallback( + (combo: string) => { + const conflict = findConflict(combo, id); + if (conflict) { + const conflictEntry = KEYBOARD_SHORTCUTS.find((s) => s.id === conflict); + toast.error( + `Already used by "${conflictEntry?.description ?? conflict}"`, + ); + setRecording(false); + return; + } + addKeybinding(id, combo); + setRecording(false); + }, + [id, addKeybinding], + ); + + if (!shortcutEntry) return null; + + return ( + + {recording ? ( + setRecording(false)} + /> + ) : hasCustom ? ( + customs.map((key, i) => ( + + {i > 0 && ( + + or + + )} + + {formatHotkeyParts(key).map((part) => ( + + ))} + + + + )) + ) : ( + + )} + + {!recording && ( + + )} + + {hasCustom && !recording && ( + + )} + + ); +} + +function DefaultKeys({ + shortcutEntry, +}: { + shortcutEntry: (typeof KEYBOARD_SHORTCUTS)[number]; +}) { + const primaryParts = formatHotkeyParts(shortcutEntry.keys); + const alternateParts = shortcutEntry.alternateKeys + ? formatHotkeyParts(shortcutEntry.alternateKeys) + : null; + + return ( + + + {primaryParts.map((part) => ( + + ))} + + {alternateParts && ( + <> + + or + + + {alternateParts.map((part) => ( + + ))} + + + )} + + ); +} diff --git a/apps/code/src/renderer/components/SpaceSwitcher.tsx b/apps/code/src/renderer/components/SpaceSwitcher.tsx index 2513bea11..15d98ba6d 100644 --- a/apps/code/src/renderer/components/SpaceSwitcher.tsx +++ b/apps/code/src/renderer/components/SpaceSwitcher.tsx @@ -1,5 +1,5 @@ import type { TaskData } from "@features/sidebar/hooks/useSidebarData"; -import { SHORTCUTS } from "@renderer/constants/keyboard-shortcuts"; +import { useShortcut } from "@hooks/useShortcut"; import type { Task } from "@shared/types"; import { useCallback, useMemo } from "react"; import { useHotkeys } from "react-hotkeys-hook"; @@ -88,8 +88,11 @@ export function SpaceSwitcher({ navigateToSlot(next); }, [tasks.length, totalSlots, currentSlot, navigateToSlot]); + const spaceUpKey = useShortcut("space-up"); + const spaceDownKey = useShortcut("space-down"); + useHotkeys( - SHORTCUTS.SPACE_UP, + spaceUpKey, (e) => { if (isInputWithContent()) return; e.preventDefault(); @@ -99,7 +102,7 @@ export function SpaceSwitcher({ [navigatePrev], ); useHotkeys( - SHORTCUTS.SPACE_DOWN, + spaceDownKey, (e) => { if (isInputWithContent()) return; e.preventDefault(); diff --git a/apps/code/src/renderer/constants/keyboard-shortcuts.ts b/apps/code/src/renderer/constants/keyboard-shortcuts.ts index b162013bb..65abd7da8 100644 --- a/apps/code/src/renderer/constants/keyboard-shortcuts.ts +++ b/apps/code/src/renderer/constants/keyboard-shortcuts.ts @@ -35,6 +35,7 @@ export interface KeyboardShortcut { category: ShortcutCategory; context?: string; alternateKeys?: string; + configurable?: boolean; } export const KEYBOARD_SHORTCUTS: KeyboardShortcut[] = [ @@ -44,30 +45,35 @@ export const KEYBOARD_SHORTCUTS: KeyboardShortcut[] = [ description: "New task", category: "general", alternateKeys: "mod+t", + configurable: true, }, { id: "command-menu", keys: SHORTCUTS.COMMAND_MENU, description: "Open command menu", category: "general", + configurable: true, }, { id: "settings", keys: SHORTCUTS.SETTINGS, description: "Open settings", category: "general", + configurable: true, }, { id: "shortcuts", keys: SHORTCUTS.SHORTCUTS_SHEET, description: "Show keyboard shortcuts", category: "general", + configurable: true, }, { id: "inbox", keys: SHORTCUTS.INBOX, description: "Open inbox", category: "navigation", + configurable: true, }, { id: "switch-task", @@ -81,6 +87,7 @@ export const KEYBOARD_SHORTCUTS: KeyboardShortcut[] = [ description: "Previous task", category: "navigation", alternateKeys: "ctrl+shift+tab", + configurable: true, }, { id: "next-task", @@ -88,42 +95,49 @@ export const KEYBOARD_SHORTCUTS: KeyboardShortcut[] = [ description: "Next task", category: "navigation", alternateKeys: "ctrl+tab", + configurable: true, }, { id: "space-up", keys: SHORTCUTS.SPACE_UP, description: "Previous space", category: "navigation", + configurable: true, }, { id: "space-down", keys: SHORTCUTS.SPACE_DOWN, description: "Next space", category: "navigation", + configurable: true, }, { id: "go-back", keys: SHORTCUTS.GO_BACK, description: "Go back", category: "navigation", + configurable: true, }, { id: "go-forward", keys: SHORTCUTS.GO_FORWARD, description: "Go forward", category: "navigation", + configurable: true, }, { id: "toggle-left-sidebar", keys: SHORTCUTS.TOGGLE_LEFT_SIDEBAR, description: "Toggle left sidebar", category: "navigation", + configurable: true, }, { id: "toggle-review-panel", keys: SHORTCUTS.TOGGLE_REVIEW_PANEL, description: "Toggle review panel", category: "navigation", + configurable: true, }, { id: "switch-tab", @@ -138,6 +152,7 @@ export const KEYBOARD_SHORTCUTS: KeyboardShortcut[] = [ description: "Close active tab", category: "panels", context: "Task detail", + configurable: true, }, { id: "open-in-editor", @@ -145,6 +160,7 @@ export const KEYBOARD_SHORTCUTS: KeyboardShortcut[] = [ description: "Open in external editor", category: "panels", context: "Task detail", + configurable: true, }, { id: "copy-path", @@ -152,6 +168,15 @@ export const KEYBOARD_SHORTCUTS: KeyboardShortcut[] = [ description: "Copy file path", category: "panels", context: "Task detail", + configurable: true, + }, + { + id: "toggle-focus", + keys: SHORTCUTS.TOGGLE_FOCUS, + description: "Toggle focus mode", + category: "panels", + context: "Task detail (worktree)", + configurable: true, }, { id: "find-in-conversation", @@ -218,6 +243,48 @@ export const CATEGORY_LABELS: Record = { editor: "Editor", }; +export const CONFIGURABLE_SHORTCUT_IDS = [ + "command-menu", + "new-task", + "settings", + "shortcuts", + "inbox", + "prev-task", + "next-task", + "space-up", + "space-down", + "go-back", + "go-forward", + "toggle-left-sidebar", + "toggle-review-panel", + "close-tab", + "open-in-editor", + "copy-path", + "toggle-focus", +] as const; + +export type ConfigurableShortcutId = (typeof CONFIGURABLE_SHORTCUT_IDS)[number]; + +export const DEFAULT_KEYBINDINGS: Record = { + "command-menu": SHORTCUTS.COMMAND_MENU, + "new-task": SHORTCUTS.NEW_TASK, + settings: SHORTCUTS.SETTINGS, + shortcuts: SHORTCUTS.SHORTCUTS_SHEET, + inbox: SHORTCUTS.INBOX, + "prev-task": SHORTCUTS.PREV_TASK, + "next-task": SHORTCUTS.NEXT_TASK, + "space-up": SHORTCUTS.SPACE_UP, + "space-down": SHORTCUTS.SPACE_DOWN, + "go-back": SHORTCUTS.GO_BACK, + "go-forward": SHORTCUTS.GO_FORWARD, + "toggle-left-sidebar": SHORTCUTS.TOGGLE_LEFT_SIDEBAR, + "toggle-review-panel": SHORTCUTS.TOGGLE_REVIEW_PANEL, + "close-tab": SHORTCUTS.CLOSE_TAB, + "open-in-editor": SHORTCUTS.OPEN_IN_EDITOR, + "copy-path": SHORTCUTS.COPY_PATH, + "toggle-focus": SHORTCUTS.TOGGLE_FOCUS, +}; + export function getShortcutsByCategory(): Record< ShortcutCategory, KeyboardShortcut[] diff --git a/apps/code/src/renderer/features/panels/hooks/usePanelKeyboardShortcuts.ts b/apps/code/src/renderer/features/panels/hooks/usePanelKeyboardShortcuts.ts index d0d5084d3..b84832a2d 100644 --- a/apps/code/src/renderer/features/panels/hooks/usePanelKeyboardShortcuts.ts +++ b/apps/code/src/renderer/features/panels/hooks/usePanelKeyboardShortcuts.ts @@ -1,3 +1,4 @@ +import { useShortcut } from "@hooks/useShortcut"; import { SHORTCUTS } from "@renderer/constants/keyboard-shortcuts"; import { useHotkeys } from "react-hotkeys-hook"; import { usePanelLayoutStore } from "../store/panelLayoutStore"; @@ -5,6 +6,7 @@ import { getLeafPanel } from "../store/panelStoreHelpers"; export function usePanelKeyboardShortcuts(taskId: string): void { const layout = usePanelLayoutStore((state) => state.getLayout(taskId)); + const closeTabKey = useShortcut("close-tab"); useHotkeys( SHORTCUTS.SWITCH_TAB, @@ -42,7 +44,7 @@ export function usePanelKeyboardShortcuts(taskId: string): void { ); useHotkeys( - SHORTCUTS.CLOSE_TAB, + closeTabKey, (event) => { event.preventDefault(); diff --git a/apps/code/src/renderer/features/task-detail/components/ExternalAppsOpener.tsx b/apps/code/src/renderer/features/task-detail/components/ExternalAppsOpener.tsx index 9d71098c8..64b40a0e4 100644 --- a/apps/code/src/renderer/features/task-detail/components/ExternalAppsOpener.tsx +++ b/apps/code/src/renderer/features/task-detail/components/ExternalAppsOpener.tsx @@ -1,4 +1,5 @@ import { useExternalApps } from "@features/external-apps/hooks/useExternalApps"; +import { useShortcut } from "@hooks/useShortcut"; import { CodeIcon, CopyIcon } from "@phosphor-icons/react"; import { Button, @@ -11,7 +12,6 @@ import { DropdownMenuTrigger, Kbd, } from "@posthog/quill"; -import { SHORTCUTS } from "@renderer/constants/keyboard-shortcuts"; import { handleExternalAppAction } from "@utils/handleExternalAppAction"; import { ChevronDown } from "lucide-react"; import { useCallback } from "react"; @@ -62,8 +62,11 @@ export function ExternalAppsOpener({ targetPath }: ExternalAppsOpenerProps) { ); }, [targetPath]); + const openInEditorKey = useShortcut("open-in-editor"); + const copyPathKey = useShortcut("copy-path"); + useHotkeys( - SHORTCUTS.OPEN_IN_EDITOR, + openInEditorKey, (event) => { event.preventDefault(); handleOpenDefault(); @@ -73,7 +76,7 @@ export function ExternalAppsOpener({ targetPath }: ExternalAppsOpenerProps) { ); useHotkeys( - SHORTCUTS.COPY_PATH, + copyPathKey, (event) => { event.preventDefault(); handleCopyPath(); diff --git a/apps/code/src/renderer/hooks/useShortcut.ts b/apps/code/src/renderer/hooks/useShortcut.ts new file mode 100644 index 000000000..36a4521cf --- /dev/null +++ b/apps/code/src/renderer/hooks/useShortcut.ts @@ -0,0 +1,6 @@ +import type { ConfigurableShortcutId } from "@renderer/constants/keyboard-shortcuts"; +import { resolveKey, useKeybindingsStore } from "@stores/keybindingsStore"; + +export function useShortcut(id: ConfigurableShortcutId): string { + return useKeybindingsStore((s) => resolveKey(s.customKeybindings, id)); +} diff --git a/apps/code/src/renderer/stores/keybindingsStore.ts b/apps/code/src/renderer/stores/keybindingsStore.ts new file mode 100644 index 000000000..4dd6de1fb --- /dev/null +++ b/apps/code/src/renderer/stores/keybindingsStore.ts @@ -0,0 +1,88 @@ +import { + CONFIGURABLE_SHORTCUT_IDS, + type ConfigurableShortcutId, + DEFAULT_KEYBINDINGS, +} from "@renderer/constants/keyboard-shortcuts"; +import { electronStorage } from "@utils/electronStorage"; +import { create } from "zustand"; +import { persist } from "zustand/middleware"; + +interface KeybindingsState { + customKeybindings: Partial>; + getKey: (id: ConfigurableShortcutId) => string; + addKeybinding: (id: ConfigurableShortcutId, key: string) => void; + removeKeybinding: (id: ConfigurableShortcutId, key: string) => void; + resetShortcut: (id: ConfigurableShortcutId) => void; + resetAll: () => void; +} + +export function resolveKey( + customKeybindings: Partial>, + id: ConfigurableShortcutId, +): string { + const customs = customKeybindings[id]; + if (customs && customs.length > 0) return customs.join(","); + return DEFAULT_KEYBINDINGS[id]; +} + +export function findConflict( + newKey: string, + excludeId: ConfigurableShortcutId, +): ConfigurableShortcutId | null { + const state = useKeybindingsStore.getState(); + for (const id of CONFIGURABLE_SHORTCUT_IDS) { + if (id === excludeId) continue; + const keyStr = state.getKey(id); + const parts = keyStr.split(",").map((k) => k.trim()); + if (parts.includes(newKey)) return id; + } + return null; +} + +export const useKeybindingsStore = create()( + persist( + (set, get) => ({ + customKeybindings: {}, + + getKey: (id) => resolveKey(get().customKeybindings, id), + + addKeybinding: (id, key) => { + const existing = get().customKeybindings[id] ?? []; + if (existing.includes(key)) return; + set({ + customKeybindings: { + ...get().customKeybindings, + [id]: [...existing, key], + }, + }); + }, + + removeKeybinding: (id, key) => { + const existing = get().customKeybindings[id] ?? []; + const updated = existing.filter((k) => k !== key); + set({ + customKeybindings: { + ...get().customKeybindings, + [id]: updated, + }, + }); + }, + + resetShortcut: (id) => { + const { [id]: _removed, ...rest } = get().customKeybindings; + set({ + customKeybindings: rest as Partial< + Record + >, + }); + }, + + resetAll: () => set({ customKeybindings: {} }), + }), + { + name: "keybindings-storage", + storage: electronStorage, + partialize: (state) => ({ customKeybindings: state.customKeybindings }), + }, + ), +); From becff7260b8baf22de4348530ff407097faccc45 Mon Sep 17 00:00:00 2001 From: Basit Balogun Date: Thu, 21 May 2026 19:14:51 +0100 Subject: [PATCH 2/7] fix: require modifier key when recording custom shortcut Bare letter keys (e.g. just "k") would fire every time that character is typed anywhere in the app. Require at least mod/ctrl/alt to be held. Generated-By: PostHog Code Task-Id: 80405bf7-239f-4b60-a1cf-5a4777fb7218 --- apps/code/src/renderer/components/ShortcutRecorder.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/apps/code/src/renderer/components/ShortcutRecorder.tsx b/apps/code/src/renderer/components/ShortcutRecorder.tsx index bdf67ecb1..eba77ed95 100644 --- a/apps/code/src/renderer/components/ShortcutRecorder.tsx +++ b/apps/code/src/renderer/components/ShortcutRecorder.tsx @@ -13,6 +13,7 @@ import { toast } from "sonner"; function captureCombo(e: KeyboardEvent): string | null { const bare = ["Meta", "Control", "Shift", "Alt"]; if (bare.includes(e.key)) return null; + if (!(e.metaKey || e.ctrlKey || e.altKey)) return null; const parts: string[] = []; if (e.metaKey || e.ctrlKey) parts.push("mod"); From 8cd6271566fbec48eb7c7d8eb07cc0e837a396a7 Mon Sep 17 00:00:00 2001 From: Basit Balogun Date: Thu, 21 May 2026 19:24:44 +0100 Subject: [PATCH 3/7] test: unit tests for keybindingsStore MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 24 tests covering resolveKey, addKeybinding, removeKeybinding, resetShortcut, resetAll, getKey, and findConflict — including conflict detection against comma-separated default alternates. Generated-By: PostHog Code Task-Id: 80405bf7-239f-4b60-a1cf-5a4777fb7218 --- .../renderer/stores/keybindingsStore.test.ts | 230 ++++++++++++++++++ 1 file changed, 230 insertions(+) create mode 100644 apps/code/src/renderer/stores/keybindingsStore.test.ts diff --git a/apps/code/src/renderer/stores/keybindingsStore.test.ts b/apps/code/src/renderer/stores/keybindingsStore.test.ts new file mode 100644 index 000000000..52ab09e19 --- /dev/null +++ b/apps/code/src/renderer/stores/keybindingsStore.test.ts @@ -0,0 +1,230 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +vi.mock("@utils/electronStorage", () => ({ + electronStorage: { + getItem: vi.fn().mockResolvedValue(null), + setItem: vi.fn().mockResolvedValue(undefined), + removeItem: vi.fn().mockResolvedValue(undefined), + }, +})); + +import { DEFAULT_KEYBINDINGS } from "@renderer/constants/keyboard-shortcuts"; +import { + findConflict, + resolveKey, + useKeybindingsStore, +} from "./keybindingsStore"; + +describe("keybindingsStore", () => { + beforeEach(() => { + useKeybindingsStore.setState({ customKeybindings: {} }); + }); + + describe("resolveKey", () => { + it("returns default when no custom binding exists", () => { + expect(resolveKey({}, "command-menu")).toBe( + DEFAULT_KEYBINDINGS["command-menu"], + ); + }); + + it("returns joined custom bindings when present", () => { + expect( + resolveKey({ "command-menu": ["ctrl+p", "ctrl+q"] }, "command-menu"), + ).toBe("ctrl+p,ctrl+q"); + }); + + it("falls back to default when custom array is empty", () => { + expect(resolveKey({ "command-menu": [] }, "command-menu")).toBe( + DEFAULT_KEYBINDINGS["command-menu"], + ); + }); + }); + + describe("addKeybinding", () => { + it("adds a custom binding", () => { + useKeybindingsStore.getState().addKeybinding("command-menu", "ctrl+p"); + expect( + useKeybindingsStore.getState().customKeybindings["command-menu"], + ).toEqual(["ctrl+p"]); + }); + + it("appends a second binding", () => { + useKeybindingsStore.getState().addKeybinding("command-menu", "ctrl+p"); + useKeybindingsStore.getState().addKeybinding("command-menu", "ctrl+q"); + expect( + useKeybindingsStore.getState().customKeybindings["command-menu"], + ).toEqual(["ctrl+p", "ctrl+q"]); + }); + + it("deduplicates identical bindings", () => { + useKeybindingsStore.getState().addKeybinding("command-menu", "ctrl+p"); + useKeybindingsStore.getState().addKeybinding("command-menu", "ctrl+p"); + expect( + useKeybindingsStore.getState().customKeybindings["command-menu"], + ).toEqual(["ctrl+p"]); + }); + + it("custom bindings replace defaults in getKey", () => { + useKeybindingsStore.getState().addKeybinding("command-menu", "ctrl+p"); + expect(useKeybindingsStore.getState().getKey("command-menu")).toBe( + "ctrl+p", + ); + }); + }); + + describe("removeKeybinding", () => { + beforeEach(() => { + useKeybindingsStore.setState({ + customKeybindings: { "command-menu": ["ctrl+p", "ctrl+q"] }, + }); + }); + + it("removes the specified binding", () => { + useKeybindingsStore.getState().removeKeybinding("command-menu", "ctrl+p"); + expect( + useKeybindingsStore.getState().customKeybindings["command-menu"], + ).toEqual(["ctrl+q"]); + }); + + it("leaves an empty array when the last binding is removed", () => { + useKeybindingsStore.getState().removeKeybinding("command-menu", "ctrl+p"); + useKeybindingsStore.getState().removeKeybinding("command-menu", "ctrl+q"); + expect( + useKeybindingsStore.getState().customKeybindings["command-menu"], + ).toEqual([]); + }); + + it("resolveKey falls back to default when custom array is emptied", () => { + useKeybindingsStore.getState().removeKeybinding("command-menu", "ctrl+p"); + useKeybindingsStore.getState().removeKeybinding("command-menu", "ctrl+q"); + expect( + resolveKey( + useKeybindingsStore.getState().customKeybindings, + "command-menu", + ), + ).toBe(DEFAULT_KEYBINDINGS["command-menu"]); + }); + }); + + describe("resetShortcut", () => { + beforeEach(() => { + useKeybindingsStore.setState({ + customKeybindings: { + "command-menu": ["ctrl+p"], + settings: ["ctrl+alt+s"], + }, + }); + }); + + it("removes the entry for the given shortcut", () => { + useKeybindingsStore.getState().resetShortcut("command-menu"); + expect( + useKeybindingsStore.getState().customKeybindings["command-menu"], + ).toBeUndefined(); + }); + + it("does not affect other shortcuts", () => { + useKeybindingsStore.getState().resetShortcut("command-menu"); + expect(useKeybindingsStore.getState().customKeybindings.settings).toEqual( + ["ctrl+alt+s"], + ); + }); + + it("getKey returns default after reset", () => { + useKeybindingsStore.getState().resetShortcut("command-menu"); + expect(useKeybindingsStore.getState().getKey("command-menu")).toBe( + DEFAULT_KEYBINDINGS["command-menu"], + ); + }); + }); + + describe("resetAll", () => { + it("clears all custom bindings", () => { + useKeybindingsStore.setState({ + customKeybindings: { + "command-menu": ["ctrl+p"], + settings: ["ctrl+alt+s"], + inbox: ["ctrl+shift+i"], + }, + }); + useKeybindingsStore.getState().resetAll(); + expect(useKeybindingsStore.getState().customKeybindings).toEqual({}); + }); + + it("all shortcuts return defaults after resetAll", () => { + useKeybindingsStore.setState({ + customKeybindings: { "command-menu": ["ctrl+p"] }, + }); + useKeybindingsStore.getState().resetAll(); + expect(useKeybindingsStore.getState().getKey("command-menu")).toBe( + DEFAULT_KEYBINDINGS["command-menu"], + ); + }); + }); + + describe("getKey", () => { + it("returns the default binding when nothing is customised", () => { + expect(useKeybindingsStore.getState().getKey("command-menu")).toBe( + DEFAULT_KEYBINDINGS["command-menu"], + ); + }); + + it("returns a single custom binding", () => { + useKeybindingsStore.setState({ + customKeybindings: { "command-menu": ["ctrl+p"] }, + }); + expect(useKeybindingsStore.getState().getKey("command-menu")).toBe( + "ctrl+p", + ); + }); + + it("joins multiple custom bindings with comma", () => { + useKeybindingsStore.setState({ + customKeybindings: { "command-menu": ["ctrl+p", "ctrl+q"] }, + }); + expect(useKeybindingsStore.getState().getKey("command-menu")).toBe( + "ctrl+p,ctrl+q", + ); + }); + }); + + describe("findConflict", () => { + beforeEach(() => { + useKeybindingsStore.setState({ customKeybindings: {} }); + }); + + it("returns null when no conflict exists", () => { + expect(findConflict("ctrl+z", "command-menu")).toBeNull(); + }); + + it("detects a conflict with a default binding on another shortcut", () => { + // mod+b is the default for toggle-left-sidebar + expect(findConflict("mod+b", "command-menu")).toBe("toggle-left-sidebar"); + }); + + it("does not flag the excluded shortcut's own default as a conflict", () => { + // mod+k is command-menu's own default — should not conflict with itself + expect(findConflict("mod+k", "command-menu")).toBeNull(); + }); + + it("detects a conflict within comma-separated default alternates", () => { + // prev-task default includes "ctrl+shift+tab" as an alternate + expect(findConflict("ctrl+shift+tab", "command-menu")).toBe("prev-task"); + }); + + it("detects a conflict with a custom binding on another shortcut", () => { + useKeybindingsStore.setState({ + customKeybindings: { settings: ["ctrl+alt+s"] }, + }); + expect(findConflict("ctrl+alt+s", "command-menu")).toBe("settings"); + }); + + it("does not conflict with custom binding on the excluded shortcut itself", () => { + useKeybindingsStore.setState({ + customKeybindings: { "command-menu": ["ctrl+p"] }, + }); + // ctrl+p is a custom binding on command-menu — assigning it to command-menu again is fine + expect(findConflict("ctrl+p", "command-menu")).toBeNull(); + }); + }); +}); From 4ba333931c8c95a5c9e88de42205d2762ea63e4a Mon Sep 17 00:00:00 2001 From: Basit Balogun Date: Thu, 21 May 2026 21:24:30 +0100 Subject: [PATCH 4/7] fix: sync shortcut label displays with live keybindings store - KeyboardShortcutsSheet header now reads the "shortcuts" key via useShortcut() so the trigger keycap updates when remapped - ExternalAppsOpener dropdown labels for open-in-editor and copy-path now derive from useShortcut() + formatHotkeyParts() instead of hardcoded Mac-only symbols test(e2e): add Playwright shortcut sheet tests Covers sheet open/close, category sections, hover controls, recording mode entry/cancellation, bare-key rejection, saving bindings, conflict detection, removing bindings, per-shortcut reset, and reset-all. Generated-By: PostHog Code Task-Id: 80405bf7-239f-4b60-a1cf-5a4777fb7218 --- .../components/KeyboardShortcutsSheet.tsx | 4 +- .../components/ExternalAppsOpener.tsx | 9 +- apps/code/tests/e2e/tests/shortcuts.spec.ts | 311 ++++++++++++++++++ 3 files changed, 321 insertions(+), 3 deletions(-) create mode 100644 apps/code/tests/e2e/tests/shortcuts.spec.ts diff --git a/apps/code/src/renderer/components/KeyboardShortcutsSheet.tsx b/apps/code/src/renderer/components/KeyboardShortcutsSheet.tsx index ca5c59365..0256eb840 100644 --- a/apps/code/src/renderer/components/KeyboardShortcutsSheet.tsx +++ b/apps/code/src/renderer/components/KeyboardShortcutsSheet.tsx @@ -1,5 +1,6 @@ import { Keycap } from "@components/Keycap"; import { ShortcutRecorder } from "@components/ShortcutRecorder"; +import { useShortcut } from "@hooks/useShortcut"; import { Box, Button, Dialog, Flex, Text } from "@radix-ui/themes"; import { CATEGORY_LABELS, @@ -55,7 +56,8 @@ export function KeyboardShortcutsSheet({ } function ShortcutsHeader() { - const triggerParts = formatHotkeyParts("mod+/"); + const shortcutsKey = useShortcut("shortcuts"); + const triggerParts = formatHotkeyParts(shortcutsKey); return ( diff --git a/apps/code/src/renderer/features/task-detail/components/ExternalAppsOpener.tsx b/apps/code/src/renderer/features/task-detail/components/ExternalAppsOpener.tsx index 64b40a0e4..9bf1dd20d 100644 --- a/apps/code/src/renderer/features/task-detail/components/ExternalAppsOpener.tsx +++ b/apps/code/src/renderer/features/task-detail/components/ExternalAppsOpener.tsx @@ -12,6 +12,7 @@ import { DropdownMenuTrigger, Kbd, } from "@posthog/quill"; +import { formatHotkeyParts } from "@renderer/constants/keyboard-shortcuts"; import { handleExternalAppAction } from "@utils/handleExternalAppAction"; import { ChevronDown } from "lucide-react"; import { useCallback } from "react"; @@ -143,7 +144,9 @@ export function ExternalAppsOpener({ targetPath }: ExternalAppsOpenerProps) { {app.name} {app.id === defaultApp?.id && ( - ⌘O + {formatHotkeyParts(openInEditorKey).map((part) => ( + {part} + ))} )} @@ -153,7 +156,9 @@ export function ExternalAppsOpener({ targetPath }: ExternalAppsOpenerProps) { Copy Path - ⌘⇧C + {formatHotkeyParts(copyPathKey).map((part) => ( + {part} + ))} diff --git a/apps/code/tests/e2e/tests/shortcuts.spec.ts b/apps/code/tests/e2e/tests/shortcuts.spec.ts new file mode 100644 index 000000000..bed4c9a89 --- /dev/null +++ b/apps/code/tests/e2e/tests/shortcuts.spec.ts @@ -0,0 +1,311 @@ +import type { Page } from "@playwright/test"; +import { expect, test } from "../fixtures/electron"; + +const isMac = process.platform === "darwin"; +const modKey = isMac ? "Meta" : "Control"; + +// Opens the shortcuts sheet via keyboard shortcut. +async function openShortcutsSheet(window: Page) { + await window.keyboard.press(`${modKey}+Slash`); + await window.getByText("Keyboard Combos").waitFor({ timeout: 5000 }); +} + +// Returns true when the main layout is rendered (requires authenticated state). +async function isMainLayout(window: Page): Promise { + await window.locator("#root > *").waitFor({ timeout: 30000 }); + await window + .locator("text=Loading") + .waitFor({ state: "hidden", timeout: 15000 }) + .catch(() => {}); + return window + .locator("text=New task") + .first() + .isVisible() + .catch(() => false); +} + +// Clears all custom bindings via the Reset all button if it's visible. +async function resetAllIfNeeded(window: Page) { + try { + await openShortcutsSheet(window); + const resetBtn = window.getByText("Reset all shortcuts to defaults"); + const visible = await resetBtn.isVisible().catch(() => false); + if (visible) await resetBtn.click(); + await window.keyboard.press("Escape"); + } catch {} +} + +test.describe("Configurable Keyboard Shortcuts", () => { + test.beforeEach(async ({ window }) => { + const ready = await isMainLayout(window); + if (!ready) test.skip(); + await resetAllIfNeeded(window); + }); + + // ─── Sheet ──────────────────────────────────────────────────────────────── + + test("shortcuts sheet opens via keyboard shortcut", async ({ window }) => { + await openShortcutsSheet(window); + + await expect(window.getByText("Keyboard Combos")).toBeVisible(); + await expect( + window.getByText("Your cheat codes for shipping faster"), + ).toBeVisible(); + }); + + test("shortcuts sheet shows all category sections", async ({ window }) => { + await openShortcutsSheet(window); + + for (const label of ["General", "Navigation", "Panels & Tabs", "Editor"]) { + await expect(window.getByText(label).first()).toBeVisible(); + } + }); + + // ─── Hover controls ─────────────────────────────────────────────────────── + + test("hovering a configurable row reveals the add (+) button", async ({ + window, + }) => { + await openShortcutsSheet(window); + + await window.getByText("Open command menu").hover(); + await expect(window.getByTitle("Add binding").first()).toBeVisible(); + }); + + test("non-configurable rows do not show an add (+) button", async ({ + window, + }) => { + await openShortcutsSheet(window); + + // "Switch to task 1-9" is intentionally non-configurable + await window.getByText("Switch to task 1-9").hover(); + const addBtns = window.getByTitle("Add binding"); + expect(await addBtns.count()).toBe(0); + }); + + // ─── Recording ──────────────────────────────────────────────────────────── + + test("clicking + enters recording mode", async ({ window }) => { + await openShortcutsSheet(window); + + await window.getByText("Open inbox").hover(); + await window.getByTitle("Add binding").click(); + + await expect( + window.locator('input[aria-label="Press new shortcut"]'), + ).toBeVisible(); + }); + + test("pressing Escape cancels recording without closing the sheet", async ({ + window, + }) => { + await openShortcutsSheet(window); + + await window.getByText("Open inbox").hover(); + await window.getByTitle("Add binding").click(); + + const input = window.locator('input[aria-label="Press new shortcut"]'); + await expect(input).toBeVisible(); + + await window.keyboard.press("Escape"); + + // Input should close… + await expect(input).not.toBeVisible(); + // …but the sheet should stay open + await expect(window.getByText("Keyboard Combos")).toBeVisible(); + }); + + test("bare letter key is rejected in recording mode", async ({ window }) => { + await openShortcutsSheet(window); + + await window.getByText("Open inbox").hover(); + await window.getByTitle("Add binding").click(); + + const input = window.locator('input[aria-label="Press new shortcut"]'); + await expect(input).toBeVisible(); + + // Press a bare letter with no modifier — should be ignored + await window.keyboard.press("k"); + + // Input should still be in recording mode (not closed) + await expect(input).toBeVisible(); + }); + + // ─── Saving a binding ───────────────────────────────────────────────────── + + test("recording a valid combo saves it and shows keycap + remove button", async ({ + window, + }) => { + await openShortcutsSheet(window); + + await window.getByText("Open inbox").hover(); + await window.getByTitle("Add binding").click(); + + // Use ControlOrMeta+Shift+Z — not in the default shortcut set + await window.keyboard.press("ControlOrMeta+Shift+Z"); + + // Recording input should close + await expect( + window.locator('input[aria-label="Press new shortcut"]'), + ).not.toBeVisible({ timeout: 3000 }); + + // Remove and reset buttons should now be visible on hover + await window.getByText("Open inbox").hover(); + await expect(window.getByTitle("Remove binding").first()).toBeVisible(); + await expect(window.getByTitle("Reset to default").first()).toBeVisible(); + }); + + test("can add a second binding to the same shortcut", async ({ window }) => { + await openShortcutsSheet(window); + + // Add first binding + await window.getByText("Open inbox").hover(); + await window.getByTitle("Add binding").click(); + await window.keyboard.press("ControlOrMeta+Shift+Z"); + await window + .locator('input[aria-label="Press new shortcut"]') + .waitFor({ state: "hidden" }); + + // Add second binding + await window.getByText("Open inbox").hover(); + await window.getByTitle("Add binding").click(); + await window.keyboard.press("ControlOrMeta+Shift+X"); + await window + .locator('input[aria-label="Press new shortcut"]') + .waitFor({ state: "hidden" }); + + // Both remove buttons should be visible (one per binding) + await window.getByText("Open inbox").hover(); + const removeBtns = window.getByTitle("Remove binding"); + expect(await removeBtns.count()).toBe(2); + }); + + // ─── Conflict detection ─────────────────────────────────────────────────── + + test("assigning an already-used combo shows a conflict toast", async ({ + window, + }) => { + await openShortcutsSheet(window); + + await window.getByText("Open command menu").hover(); + await window.getByTitle("Add binding").click(); + + // mod+b is the default for "Toggle left sidebar" + await window.keyboard.press(`${modKey}+b`); + + await expect( + window.getByText('Already used by "Toggle left sidebar"'), + ).toBeVisible({ timeout: 5000 }); + + // Recording should be cancelled — no remove button + await window.getByText("Open command menu").hover(); + await expect(window.getByTitle("Remove binding")).not.toBeVisible(); + }); + + // ─── Removing a binding ─────────────────────────────────────────────────── + + test("clicking × removes a custom binding", async ({ window }) => { + await openShortcutsSheet(window); + + // Add a binding + await window.getByText("Open inbox").hover(); + await window.getByTitle("Add binding").click(); + await window.keyboard.press("ControlOrMeta+Shift+Z"); + await window + .locator('input[aria-label="Press new shortcut"]') + .waitFor({ state: "hidden" }); + + // Remove it + await window.getByText("Open inbox").hover(); + await window.getByTitle("Remove binding").click(); + + // Remove and reset buttons should now be gone + await window.getByText("Open inbox").hover(); + await expect(window.getByTitle("Remove binding")).not.toBeVisible(); + await expect(window.getByTitle("Reset to default")).not.toBeVisible(); + }); + + // ─── Per-shortcut reset ─────────────────────────────────────────────────── + + test("↩ resets an individual shortcut to its default", async ({ window }) => { + await openShortcutsSheet(window); + + // Add a binding + await window.getByText("Open inbox").hover(); + await window.getByTitle("Add binding").click(); + await window.keyboard.press("ControlOrMeta+Shift+Z"); + await window + .locator('input[aria-label="Press new shortcut"]') + .waitFor({ state: "hidden" }); + + // Reset this shortcut + await window.getByText("Open inbox").hover(); + await window.getByTitle("Reset to default").click(); + + // Should revert — no custom controls visible + await window.getByText("Open inbox").hover(); + await expect(window.getByTitle("Reset to default")).not.toBeVisible(); + await expect(window.getByTitle("Remove binding")).not.toBeVisible(); + }); + + // ─── Reset all ──────────────────────────────────────────────────────────── + + test("Reset all button is hidden when no custom bindings exist", async ({ + window, + }) => { + await openShortcutsSheet(window); + + await expect( + window.getByText("Reset all shortcuts to defaults"), + ).not.toBeVisible(); + }); + + test("Reset all button appears after adding a custom binding", async ({ + window, + }) => { + await openShortcutsSheet(window); + + await window.getByText("Open inbox").hover(); + await window.getByTitle("Add binding").click(); + await window.keyboard.press("ControlOrMeta+Shift+Z"); + await window + .locator('input[aria-label="Press new shortcut"]') + .waitFor({ state: "hidden" }); + + // Scroll to bottom to find the button + const resetAllBtn = window.getByText("Reset all shortcuts to defaults"); + await resetAllBtn.scrollIntoViewIfNeeded(); + await expect(resetAllBtn).toBeVisible(); + }); + + test("clicking Reset all clears all custom bindings", async ({ window }) => { + await openShortcutsSheet(window); + + // Add bindings to two different shortcuts + await window.getByText("Open inbox").hover(); + await window.getByTitle("Add binding").click(); + await window.keyboard.press("ControlOrMeta+Shift+Z"); + await window + .locator('input[aria-label="Press new shortcut"]') + .waitFor({ state: "hidden" }); + + await window.getByText("Open command menu").hover(); + await window.getByTitle("Add binding").click(); + await window.keyboard.press("ControlOrMeta+Shift+X"); + await window + .locator('input[aria-label="Press new shortcut"]') + .waitFor({ state: "hidden" }); + + // Click Reset all + const resetAllBtn = window.getByText("Reset all shortcuts to defaults"); + await resetAllBtn.scrollIntoViewIfNeeded(); + await resetAllBtn.click(); + + // Button should disappear + await expect(resetAllBtn).not.toBeVisible(); + + // Neither row should have custom controls any more + await window.getByText("Open inbox").hover(); + await expect(window.getByTitle("Remove binding")).not.toBeVisible(); + }); +}); From ba2d7e7d8f43eba409574fcd583a5e6a802f5bf9 Mon Sep 17 00:00:00 2001 From: Basit Balogun Date: Thu, 21 May 2026 22:54:53 +0100 Subject: [PATCH 5/7] fix: show Ctrl on Windows and accept Ctrl-triggered shortcuts Hardcoded Cmd glyphs were leaking onto Windows in the send-messages dropdown and the tiptap paste hint, and two handlers were gated on metaKey only so the corresponding shortcut never fired on Windows (mod+1..9 task switching, Cmd/Ctrl-click multi-select in the inbox). Generated-By: PostHog Code Task-Id: 80405bf7-239f-4b60-a1cf-5a4777fb7218 --- .../code/src/renderer/components/GlobalEventHandlers.tsx | 7 +++++-- .../features/inbox/components/InboxSignalsTab.tsx | 9 ++++++--- .../features/inbox/components/list/ReportListPane.tsx | 2 +- .../features/inbox/components/list/ReportListRow.tsx | 8 ++++++-- .../features/message-editor/tiptap/useTiptapEditor.ts | 3 ++- .../settings/components/sections/GeneralSettings.tsx | 5 ++++- 6 files changed, 24 insertions(+), 10 deletions(-) diff --git a/apps/code/src/renderer/components/GlobalEventHandlers.tsx b/apps/code/src/renderer/components/GlobalEventHandlers.tsx index 4b5379640..e69f7ac6a 100644 --- a/apps/code/src/renderer/components/GlobalEventHandlers.tsx +++ b/apps/code/src/renderer/components/GlobalEventHandlers.tsx @@ -12,6 +12,7 @@ import { useWorkspaces } from "@features/workspace/hooks/useWorkspace"; import { useShortcut } from "@hooks/useShortcut"; import { SHORTCUTS } from "@renderer/constants/keyboard-shortcuts"; import { useTRPC } from "@renderer/trpc"; +import { isMac } from "@renderer/utils/platform"; import type { Task } from "@shared/types"; import { useCommandMenuStore } from "@stores/commandMenuStore"; import { useNavigationStore } from "@stores/navigationStore"; @@ -203,11 +204,13 @@ export function GlobalEventHandlers({ [handleToggleFocus], ); - // Task switching with mod+1-9 + // Task switching with mod+1-9. On macOS, Ctrl+1..9 is reserved for + // SWITCH_TAB (panel tabs), so ignore plain-Ctrl there; on Windows/Linux, + // Ctrl IS mod, so the same event must trigger task switching. useHotkeys( SHORTCUTS.SWITCH_TASK, (event, handler) => { - if (event.ctrlKey && !event.metaKey) return; + if (isMac && event.ctrlKey && !event.metaKey) return; const keyPressed = handler.keys?.[0]; if (!keyPressed) return; diff --git a/apps/code/src/renderer/features/inbox/components/InboxSignalsTab.tsx b/apps/code/src/renderer/features/inbox/components/InboxSignalsTab.tsx index d8451b7bd..5a509cf23 100644 --- a/apps/code/src/renderer/features/inbox/components/InboxSignalsTab.tsx +++ b/apps/code/src/renderer/features/inbox/components/InboxSignalsTab.tsx @@ -354,9 +354,12 @@ export function InboxSignalsTab() { return reports.filter((r) => idSet.has(r.id)); }, [reports, selectedReportIds]); - // ── Click handler: plain / cmd / shift ────────────────────────────────── + // ── Click handler: plain / cmd-or-ctrl / shift ────────────────────────── const handleReportClick = useCallback( - (reportId: string, event: { metaKey: boolean; shiftKey: boolean }) => { + ( + reportId: string, + event: { metaKey: boolean; ctrlKey: boolean; shiftKey: boolean }, + ) => { // Selecting a real report clears any discovered-task selection so the // detail pane can swap to the report. useSetupStore.getState().selectDiscoveredTask(null); @@ -366,7 +369,7 @@ export function InboxSignalsTab() { reportId, reportsRef.current.map((r) => r.id), ); - } else if (event.metaKey) { + } else if (event.metaKey || event.ctrlKey) { setPendingInboxOpenMethod("click_cmd"); toggleReportSelection(reportId); } else { diff --git a/apps/code/src/renderer/features/inbox/components/list/ReportListPane.tsx b/apps/code/src/renderer/features/inbox/components/list/ReportListPane.tsx index 8800c8701..86940f24a 100644 --- a/apps/code/src/renderer/features/inbox/components/list/ReportListPane.tsx +++ b/apps/code/src/renderer/features/inbox/components/list/ReportListPane.tsx @@ -68,7 +68,7 @@ interface ReportListPaneProps { selectedReportIds: string[]; onReportClick: ( id: string, - event: { metaKey: boolean; shiftKey: boolean }, + event: { metaKey: boolean; ctrlKey: boolean; shiftKey: boolean }, ) => void; onToggleReportSelection: (id: string) => void; } diff --git a/apps/code/src/renderer/features/inbox/components/list/ReportListRow.tsx b/apps/code/src/renderer/features/inbox/components/list/ReportListRow.tsx index 21a046282..03ebbf3f7 100644 --- a/apps/code/src/renderer/features/inbox/components/list/ReportListRow.tsx +++ b/apps/code/src/renderer/features/inbox/components/list/ReportListRow.tsx @@ -42,7 +42,11 @@ interface ReportListRowProps { report: SignalReport; isSelected: boolean; showCheckbox: boolean; - onClick: (event: { metaKey: boolean; shiftKey: boolean }) => void; + onClick: (event: { + metaKey: boolean; + ctrlKey: boolean; + shiftKey: boolean; + }) => void; onToggleChecked: () => void; index: number; /** Optional badge rendered before the standard status/priority/actionability badges. */ @@ -72,7 +76,7 @@ export function ReportListRow({ if (isInteractiveTarget(e.target)) { return; } - onClick({ metaKey: e.metaKey, shiftKey: e.shiftKey }); + onClick({ metaKey: e.metaKey, ctrlKey: e.ctrlKey, shiftKey: e.shiftKey }); }; const rowBgClass = isSelected ? "bg-gray-3" : ""; diff --git a/apps/code/src/renderer/features/message-editor/tiptap/useTiptapEditor.ts b/apps/code/src/renderer/features/message-editor/tiptap/useTiptapEditor.ts index 9dbe099ac..eebc8bac8 100644 --- a/apps/code/src/renderer/features/message-editor/tiptap/useTiptapEditor.ts +++ b/apps/code/src/renderer/features/message-editor/tiptap/useTiptapEditor.ts @@ -1,5 +1,6 @@ import { sessionStoreSetters } from "@features/sessions/stores/sessionStore"; import { useSettingsStore as useFeatureSettingsStore } from "@features/settings/stores/settingsStore"; +import { formatHotkey } from "@renderer/constants/keyboard-shortcuts"; import { trpc } from "@renderer/trpc/client"; import { toast } from "@renderer/utils/toast"; import type { EditorView } from "@tiptap/pm/view"; @@ -479,7 +480,7 @@ export function useTiptapEditor(options: UseTiptapEditorOptions) { if (clipboardText && clipboardText.length > 200) { showPasteHint( "Pasted as text", - "Use ⌘⇧V to paste as a file attachment instead.", + `Use ${formatHotkey("mod+shift+v")} to paste as a file attachment instead.`, ); } diff --git a/apps/code/src/renderer/features/settings/components/sections/GeneralSettings.tsx b/apps/code/src/renderer/features/settings/components/sections/GeneralSettings.tsx index c64480e83..ea1a639f9 100644 --- a/apps/code/src/renderer/features/settings/components/sections/GeneralSettings.tsx +++ b/apps/code/src/renderer/features/settings/components/sections/GeneralSettings.tsx @@ -19,6 +19,7 @@ import { Switch, Text, } from "@radix-ui/themes"; +import { formatHotkey } from "@renderer/constants/keyboard-shortcuts"; import { useTRPC } from "@renderer/trpc"; import { ANALYTICS_EVENTS } from "@shared/types/analytics"; import type { ThemePreference } from "@stores/themeStore"; @@ -438,7 +439,9 @@ export function GeneralSettings() { Enter - ⌘ Enter + + {formatHotkey("mod+enter")} + From dc8293ff4f2bdcfb6fc70cf80f8635cb33e6491b Mon Sep 17 00:00:00 2001 From: Basit Balogun Date: Sat, 23 May 2026 01:12:36 +0100 Subject: [PATCH 6/7] feat: make prompt-history shortcuts configurable; update E2E tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add prompt-history-prev/next to CONFIGURABLE_SHORTCUT_IDS and DEFAULT_KEYBINDINGS so they appear in the shortcuts sheet and can be rebound like any other shortcut - Add tiptapEventToCombo() — accepts shift-only combos (no Ctrl/Meta required) so shift+up/down can be matched against live bindings - Fix eventToCombo() to normalise Arrow-prefixed key names (ArrowUp to up) - Wire useTiptapEditor to resolve prompt-history keys from the store instead of hardcoding event.shiftKey - Fix paste hint toast to show the live paste-as-file binding instead of the hardcoded mod+shift+v string - Fix noStaticElementInteractions lint on recording modal backdrop - Rewrite E2E shortcut tests to match the current recording modal UI (chips + right-click context menu) rather than the old hover-button and inline-input design --- .../components/KeyboardShortcutsSheet.tsx | 111 +++-- .../renderer/components/ShortcutRecorder.tsx | 457 ++++++++++++------ .../renderer/constants/keyboard-shortcuts.ts | 55 +++ .../message-editor/tiptap/useTiptapEditor.ts | 37 +- .../task-detail/components/TaskDetail.tsx | 4 +- .../renderer/stores/keybindingsStore.test.ts | 86 +++- .../src/renderer/stores/keybindingsStore.ts | 72 ++- apps/code/tests/e2e/tests/shortcuts.spec.ts | 407 +++++++++++----- 8 files changed, 883 insertions(+), 346 deletions(-) diff --git a/apps/code/src/renderer/components/KeyboardShortcutsSheet.tsx b/apps/code/src/renderer/components/KeyboardShortcutsSheet.tsx index 0256eb840..ccd0571ac 100644 --- a/apps/code/src/renderer/components/KeyboardShortcutsSheet.tsx +++ b/apps/code/src/renderer/components/KeyboardShortcutsSheet.tsx @@ -1,5 +1,6 @@ import { Keycap } from "@components/Keycap"; import { ShortcutRecorder } from "@components/ShortcutRecorder"; +import { Tooltip } from "@components/ui/Tooltip"; import { useShortcut } from "@hooks/useShortcut"; import { Box, Button, Dialog, Flex, Text } from "@radix-ui/themes"; import { @@ -34,9 +35,10 @@ export function KeyboardShortcutsSheet({ e.preventDefault()} - className="max-h-[80vh] overflow-hidden" + className="!pb-0 flex max-h-[80vh] flex-col overflow-hidden" > - + {/* Header */} + + + ); +} + function ShortcutsHeader() { const shortcutsKey = useShortcut("shortcuts"); const triggerParts = formatHotkeyParts(shortcutsKey); @@ -80,13 +118,6 @@ function ShortcutsHeader() { export function KeyboardShortcutsList() { const shortcutsByCategory = useMemo(() => getShortcutsByCategory(), []); - const hasCustomBindings = useKeybindingsStore((s) => - Object.keys(s.customKeybindings).some( - (k) => - (s.customKeybindings[k as ConfigurableShortcutId]?.length ?? 0) > 0, - ), - ); - const resetAll = useKeybindingsStore((s) => s.resetAll); const categoryOrder: ShortcutCategory[] = [ "general", @@ -125,10 +156,11 @@ export function KeyboardShortcutsList() { key={shortcut.id} align="center" justify="between" + gap="3" px="3" className="group border-b border-b-(--gray-4) pt-[6px] pb-[6px] last:border-b-0 odd:bg-(--gray-2) even:bg-(--gray-1)" > - + {shortcut.description} {shortcut.context && ( @@ -136,43 +168,30 @@ export function KeyboardShortcutsList() { )} - {shortcut.configurable ? ( - - ) : ( - - )} +
+ {shortcut.configurable ? ( + + ) : ( + + )} +
))} ); })} - - {hasCustomBindings && ( - - - - )} ); } function SingleShortcutKeys({ keys }: { keys: string }) { const parts = formatHotkeyParts(keys); - return ( {parts.map((part) => ( @@ -189,11 +208,7 @@ function ShortcutKeys({ keys: string; alternateKeys?: string; }) { - if (!alternateKeys) { - return ; - } - - return ( + const inner = alternateKeys ? ( @@ -201,5 +216,19 @@ function ShortcutKeys({ + ) : ( + + ); + + return ( + +
+ {inner} +
+ ); } diff --git a/apps/code/src/renderer/components/ShortcutRecorder.tsx b/apps/code/src/renderer/components/ShortcutRecorder.tsx index eba77ed95..50c67f056 100644 --- a/apps/code/src/renderer/components/ShortcutRecorder.tsx +++ b/apps/code/src/renderer/components/ShortcutRecorder.tsx @@ -1,194 +1,355 @@ import { Keycap } from "@components/Keycap"; -import { ArrowCounterClockwise, Plus, X } from "@phosphor-icons/react"; -import { Flex, Text } from "@radix-ui/themes"; +import { Tooltip } from "@components/ui/Tooltip"; +import { ContextMenu, Flex, Text } from "@radix-ui/themes"; import { type ConfigurableShortcutId, + DEFAULT_KEYBINDINGS, + eventToCombo, formatHotkeyParts, KEYBOARD_SHORTCUTS, } from "@renderer/constants/keyboard-shortcuts"; -import { findConflict, useKeybindingsStore } from "@stores/keybindingsStore"; -import { useCallback, useEffect, useRef, useState } from "react"; -import { toast } from "sonner"; - -function captureCombo(e: KeyboardEvent): string | null { - const bare = ["Meta", "Control", "Shift", "Alt"]; - if (bare.includes(e.key)) return null; - if (!(e.metaKey || e.ctrlKey || e.altKey)) return null; - - const parts: string[] = []; - if (e.metaKey || e.ctrlKey) parts.push("mod"); - if (e.shiftKey) parts.push("shift"); - if (e.altKey) parts.push("alt"); - - const key = e.key.toLowerCase(); - parts.push(key); - return parts.join("+"); +import { + findConflict, + MAX_CUSTOM_BINDINGS, + splitBindings, + useKeybindingsStore, +} from "@stores/keybindingsStore"; +import { useCallback, useEffect, useState } from "react"; + +// --------------------------------------------------------------------------- +// Key capture +// --------------------------------------------------------------------------- + +function formatComboLabel(combo: string): string { + return formatHotkeyParts(combo).join("+"); } -interface RecordingInputProps { - onCapture: (combo: string) => void; - onCancel: () => void; +// --------------------------------------------------------------------------- +// Recording modal +// --------------------------------------------------------------------------- + +interface RecordingModalProps { + commandLabel: string; + /** null = adding a new binding, string = the binding key being edited */ + editingKey: string | null; + shortcutId: ConfigurableShortcutId; + onClose: () => void; } -function RecordingInput({ onCapture, onCancel }: RecordingInputProps) { - const ref = useRef(null); +export function ShortcutRecordingModal({ + commandLabel, + editingKey, + shortcutId, + onClose, +}: RecordingModalProps) { + const addKeybinding = useKeybindingsStore((s) => s.addKeybinding); + const updateKeybinding = useKeybindingsStore((s) => s.updateKeybinding); + const [captured, setCaptured] = useState(null); + const [conflict, setConflict] = useState(null); + // Capture at the window level — no focus required on the input element. useEffect(() => { - ref.current?.focus(); - }, []); - - const handleKeyDown = useCallback( - (e: React.KeyboardEvent) => { + const handler = (e: KeyboardEvent) => { e.preventDefault(); e.stopPropagation(); + if (e.key === "Escape") { - onCancel(); + onClose(); return; } - const combo = captureCombo(e.nativeEvent); - if (combo) onCapture(combo); - }, - [onCapture, onCancel], + + if (e.key === "Enter") { + if (captured && !conflict) { + if (editingKey) { + updateKeybinding(shortcutId, editingKey, captured); + } else { + addKeybinding(shortcutId, captured); + } + onClose(); + } + return; + } + + const combo = eventToCombo(e); + if (!combo) return; + + const result = findConflict(combo, shortcutId); + if (result.description) { + setConflict(result.description); + setCaptured(combo); + } else { + setConflict(null); + setCaptured(combo); + } + }; + + window.addEventListener("keydown", handler, { capture: true }); + return () => + window.removeEventListener("keydown", handler, { capture: true }); + }, [ + captured, + conflict, + editingKey, + shortcutId, + addKeybinding, + updateKeybinding, + onClose, + ]); + + const isAdding = editingKey === null; + const title = isAdding + ? `Add new binding for "${commandLabel}"` + : `Edit binding for "${commandLabel}"`; + + return ( + // biome-ignore lint/a11y/noStaticElementInteractions: backdrop dismiss pattern — not an interactive widget +
{ + if (e.target === e.currentTarget) onClose(); + }} + > +
+
+ + {title} + +
+ +
+ {/* Visual input — display only; actual capture happens via window listener */} +
+ {captured ? ( + formatComboLabel(captured) + ) : ( + + Press a key combination... + + )} +
+ {conflict ? ( + + Conflicts with "{conflict}" — press a different + combination + + ) : captured ? ( + + Press Enter to confirm, Escape to cancel + + ) : ( + + Press Escape to cancel + + )} +
+
+
); +} + +// --------------------------------------------------------------------------- +// Binding chip — single keycap group with click + right-click +// --------------------------------------------------------------------------- + +type RecordingMode = { type: "add" } | { type: "edit"; key: string } | null; + +interface BindingChipProps { + combo: string; + commandLabel: string; + canRemove: boolean; + canAddMore: boolean; + isAtDefault: boolean; + onStartRecording: (mode: RecordingMode) => void; + onRemove: () => void; + onReset: () => void; +} + +function BindingChip({ + combo, + commandLabel, + canRemove, + canAddMore, + isAtDefault, + onStartRecording, + onRemove, + onReset, +}: BindingChipProps) { + const parts = formatHotkeyParts(combo); return ( - {}} - tabIndex={0} - onKeyDown={handleKeyDown} - onBlur={onCancel} - className="h-[28px] min-w-[120px] cursor-text rounded-[6px] border border-(--accent-8) bg-(--accent-2) px-2 text-center text-(--accent-11) text-[12px] outline-none ring-(--accent-8) ring-1 placeholder:text-(--accent-11)" - /> + + + + + + + onStartRecording({ type: "edit", key: combo })} + > + Edit binding + + {canAddMore && ( + onStartRecording({ type: "add" })}> + Add another binding + + )} + + {canRemove ? ( + + Remove binding + + ) : ( + + + + Remove binding + + + + )} + {isAtDefault ? ( + + + + Reset to default + + + + ) : ( + + Reset to default + + )} + + ); } +// --------------------------------------------------------------------------- +// ShortcutRecorder — main export +// --------------------------------------------------------------------------- + interface ShortcutRecorderProps { id: ConfigurableShortcutId; + onRecordingChange?: (recording: boolean) => void; } -export function ShortcutRecorder({ id }: ShortcutRecorderProps) { - const [recording, setRecording] = useState(false); +export function ShortcutRecorder({ + id, + onRecordingChange, +}: ShortcutRecorderProps) { + const [recordingMode, setRecordingMode] = useState(null); const customs = useKeybindingsStore((s) => s.customKeybindings[id] ?? []); - const addKeybinding = useKeybindingsStore((s) => s.addKeybinding); const removeKeybinding = useKeybindingsStore((s) => s.removeKeybinding); const resetShortcut = useKeybindingsStore((s) => s.resetShortcut); + const addKeybinding = useKeybindingsStore((s) => s.addKeybinding); const hasCustom = customs.length > 0; const shortcutEntry = KEYBOARD_SHORTCUTS.find((s) => s.id === id); - const handleCapture = useCallback( - (combo: string) => { - const conflict = findConflict(combo, id); - if (conflict) { - const conflictEntry = KEYBOARD_SHORTCUTS.find((s) => s.id === conflict); - toast.error( - `Already used by "${conflictEntry?.description ?? conflict}"`, - ); - setRecording(false); - return; + // The effective list of individual binding strings to render as chips. + // When customised: use custom array. When at default: split the default + // string (e.g. "mod+n,mod+t" → ["mod+n","mod+t"]) so each chip is independent. + const defaultBindings = splitBindings(DEFAULT_KEYBINDINGS[id]); + const effectiveBindings = hasCustom ? customs : defaultBindings; + // canAddMore is based on custom count only — defaults don't consume the budget. + // A user can always add a first custom binding even if there are 2 defaults. + const canAddMore = customs.length < MAX_CUSTOM_BINDINGS; + + const startRecording = useCallback( + (mode: RecordingMode) => { + setRecordingMode(mode); + onRecordingChange?.(true); + }, + [onRecordingChange], + ); + + const stopRecording = useCallback(() => { + setRecordingMode(null); + onRecordingChange?.(false); + }, [onRecordingChange]); + + // Removing a default binding: store the remaining defaults as custom bindings. + const handleRemoveDefault = useCallback( + (key: string) => { + const remaining = defaultBindings.filter((k) => k !== key); + resetShortcut(id); + for (const k of remaining) { + addKeybinding(id, k); } - addKeybinding(id, combo); - setRecording(false); }, - [id, addKeybinding], + [id, defaultBindings, resetShortcut, addKeybinding], ); if (!shortcutEntry) return null; + const commandLabel = shortcutEntry.description; + const isAtDefault = !hasCustom; + return ( - - {recording ? ( - setRecording(false)} + <> + {recordingMode !== null && ( + - ) : hasCustom ? ( - customs.map((key, i) => ( - - {i > 0 && ( - - or - - )} - - {formatHotkeyParts(key).map((part) => ( - - ))} - - - - )) - ) : ( - - )} - - {!recording && ( - - )} - - {hasCustom && !recording && ( - )} - - ); -} - -function DefaultKeys({ - shortcutEntry, -}: { - shortcutEntry: (typeof KEYBOARD_SHORTCUTS)[number]; -}) { - const primaryParts = formatHotkeyParts(shortcutEntry.keys); - const alternateParts = shortcutEntry.alternateKeys - ? formatHotkeyParts(shortcutEntry.alternateKeys) - : null; - return ( - - - {primaryParts.map((part) => ( - - ))} - - {alternateParts && ( - <> - - or - - - {alternateParts.map((part) => ( - - ))} - - - )} - + {/* Horizontal scroll container — hides overflow without showing a scrollbar */} +
+ + {effectiveBindings.map((key, i) => ( + + {i > 0 && ( + + or + + )} + 1} + canAddMore={canAddMore} + isAtDefault={isAtDefault} + onStartRecording={startRecording} + onRemove={ + hasCustom + ? () => removeKeybinding(id, key) + : () => handleRemoveDefault(key) + } + onReset={() => resetShortcut(id)} + /> + + ))} + +
+ ); } diff --git a/apps/code/src/renderer/constants/keyboard-shortcuts.ts b/apps/code/src/renderer/constants/keyboard-shortcuts.ts index 65abd7da8..4241d338d 100644 --- a/apps/code/src/renderer/constants/keyboard-shortcuts.ts +++ b/apps/code/src/renderer/constants/keyboard-shortcuts.ts @@ -22,6 +22,7 @@ export const SHORTCUTS = { SPACE_UP: "mod+up", SPACE_DOWN: "mod+down", FIND_IN_CONVERSATION: "mod+f", + FILE_PICKER: "mod+p", BLUR: "escape", SUBMIT_BLUR: "mod+enter", } as const; @@ -185,12 +186,21 @@ export const KEYBOARD_SHORTCUTS: KeyboardShortcut[] = [ category: "panels", context: "Task detail", }, + { + id: "file-picker", + keys: SHORTCUTS.FILE_PICKER, + description: "Open file picker", + category: "panels", + context: "Task detail", + configurable: true, + }, { id: "paste-as-file", keys: SHORTCUTS.PASTE_AS_FILE, description: "Paste as file attachment", category: "editor", context: "Message editor", + configurable: true, }, { id: "prompt-history-prev", @@ -198,6 +208,7 @@ export const KEYBOARD_SHORTCUTS: KeyboardShortcut[] = [ description: "Previous prompt", category: "editor", context: "Message editor", + configurable: true, }, { id: "prompt-history-next", @@ -205,6 +216,7 @@ export const KEYBOARD_SHORTCUTS: KeyboardShortcut[] = [ description: "Next prompt", category: "editor", context: "Message editor", + configurable: true, }, { id: "editor-bold", @@ -261,6 +273,10 @@ export const CONFIGURABLE_SHORTCUT_IDS = [ "open-in-editor", "copy-path", "toggle-focus", + "file-picker", + "paste-as-file", + "prompt-history-prev", + "prompt-history-next", ] as const; export type ConfigurableShortcutId = (typeof CONFIGURABLE_SHORTCUT_IDS)[number]; @@ -283,6 +299,10 @@ export const DEFAULT_KEYBINDINGS: Record = { "open-in-editor": SHORTCUTS.OPEN_IN_EDITOR, "copy-path": SHORTCUTS.COPY_PATH, "toggle-focus": SHORTCUTS.TOGGLE_FOCUS, + "file-picker": SHORTCUTS.FILE_PICKER, + "paste-as-file": SHORTCUTS.PASTE_AS_FILE, + "prompt-history-prev": "shift+up", + "prompt-history-next": "shift+down", }; export function getShortcutsByCategory(): Record< @@ -301,6 +321,41 @@ export function getShortcutsByCategory(): Record< return grouped; } +/** + * Convert a DOM KeyboardEvent to the normalised combo string used by the + * keybindings store (e.g. "mod+shift+v"). Returns null for bare modifier presses. + */ +export function eventToCombo(e: KeyboardEvent): string | null { + const bare = ["Meta", "Control", "Shift", "Alt"]; + if (bare.includes(e.key)) return null; + if (!(e.metaKey || e.ctrlKey || e.altKey)) return null; + + const parts: string[] = []; + if (e.metaKey || e.ctrlKey) parts.push("mod"); + if (e.shiftKey) parts.push("shift"); + if (e.altKey) parts.push("alt"); + // Normalize "ArrowUp" → "up", "ArrowDown" → "down", etc. to match stored bindings. + parts.push(e.key.toLowerCase().replace(/^arrow/, "")); + return parts.join("+"); +} + +/** + * Like eventToCombo but also accepts shift-only combos (no ctrl/meta/alt required). + * Used inside Tiptap's handleKeyDown to match bindings such as "shift+up". + */ +export function tiptapEventToCombo(e: KeyboardEvent): string | null { + const bare = ["Meta", "Control", "Shift", "Alt"]; + if (bare.includes(e.key)) return null; + if (!(e.metaKey || e.ctrlKey || e.altKey || e.shiftKey)) return null; + + const parts: string[] = []; + if (e.metaKey || e.ctrlKey) parts.push("mod"); + if (e.shiftKey) parts.push("shift"); + if (e.altKey) parts.push("alt"); + parts.push(e.key.toLowerCase().replace(/^arrow/, "")); + return parts.join("+"); +} + function formatKey(key: string): string { const k = key.trim().toLowerCase(); if (k === "mod") return isMac ? "⌘" : "Ctrl"; diff --git a/apps/code/src/renderer/features/message-editor/tiptap/useTiptapEditor.ts b/apps/code/src/renderer/features/message-editor/tiptap/useTiptapEditor.ts index eebc8bac8..95a23ae72 100644 --- a/apps/code/src/renderer/features/message-editor/tiptap/useTiptapEditor.ts +++ b/apps/code/src/renderer/features/message-editor/tiptap/useTiptapEditor.ts @@ -1,8 +1,13 @@ import { sessionStoreSetters } from "@features/sessions/stores/sessionStore"; import { useSettingsStore as useFeatureSettingsStore } from "@features/settings/stores/settingsStore"; -import { formatHotkey } from "@renderer/constants/keyboard-shortcuts"; +import { + eventToCombo, + formatHotkey, + tiptapEventToCombo, +} from "@renderer/constants/keyboard-shortcuts"; import { trpc } from "@renderer/trpc/client"; import { toast } from "@renderer/utils/toast"; +import { splitBindings, useKeybindingsStore } from "@stores/keybindingsStore"; import type { EditorView } from "@tiptap/pm/view"; import { useEditor } from "@tiptap/react"; import { queryClient } from "@utils/queryClient"; @@ -269,11 +274,12 @@ export function useTiptapEditor(options: UseTiptapEditorOptions) { }, }, handleKeyDown: (view, event) => { - if ( - event.key === "v" && - (event.metaKey || event.ctrlKey) && - event.shiftKey - ) { + const eventCombo = eventToCombo(event); + const pasteAsFileKey = useKeybindingsStore + .getState() + .getKey("paste-as-file"); + const pasteAsFileBindings = splitBindings(pasteAsFileKey); + if (eventCombo && pasteAsFileBindings.includes(eventCombo)) { event.preventDefault(); (async () => { try { @@ -313,12 +319,23 @@ export function useTiptapEditor(options: UseTiptapEditorOptions) { const isAtStart = from === 1; const isAtEnd = from === view.state.doc.content.size - 1; - const forceNavigate = event.shiftKey; + const keybindings = useKeybindingsStore.getState(); + const tiptapCombo = tiptapEventToCombo(event); + const forcePrev = + tiptapCombo !== null && + splitBindings(keybindings.getKey("prompt-history-prev")).includes( + tiptapCombo, + ); + const forceNext = + tiptapCombo !== null && + splitBindings(keybindings.getKey("prompt-history-next")).includes( + tiptapCombo, + ); const history = historyGetter?.() ?? []; if ( event.key === "ArrowUp" && - (forceNavigate || isEmpty || isAtStart) + (forcePrev || isEmpty || isAtStart) ) { if (taskId) { const queuedContent = @@ -348,7 +365,7 @@ export function useTiptapEditor(options: UseTiptapEditorOptions) { if ( event.key === "ArrowDown" && - (forceNavigate || isEmpty || isAtEnd) + (forceNext || isEmpty || isAtEnd) ) { const newText = historyActions.navigateDown(history); if (newText !== null) { @@ -480,7 +497,7 @@ export function useTiptapEditor(options: UseTiptapEditorOptions) { if (clipboardText && clipboardText.length > 200) { showPasteHint( "Pasted as text", - `Use ${formatHotkey("mod+shift+v")} to paste as a file attachment instead.`, + `Use ${formatHotkey(useKeybindingsStore.getState().getKey("paste-as-file"))} to paste as a file attachment instead.`, ); } diff --git a/apps/code/src/renderer/features/task-detail/components/TaskDetail.tsx b/apps/code/src/renderer/features/task-detail/components/TaskDetail.tsx index 45f17ada1..579854855 100644 --- a/apps/code/src/renderer/features/task-detail/components/TaskDetail.tsx +++ b/apps/code/src/renderer/features/task-detail/components/TaskDetail.tsx @@ -19,6 +19,7 @@ import { useWorkspace } from "@features/workspace/hooks/useWorkspace"; import { useBlurOnEscape } from "@hooks/useBlurOnEscape"; import { useFileWatcher } from "@hooks/useFileWatcher"; import { useSetHeaderContent } from "@hooks/useSetHeaderContent"; +import { useShortcut } from "@hooks/useShortcut"; import { Box, Flex, Text, Tooltip } from "@radix-ui/themes"; import type { Task } from "@shared/types"; import { useQueryClient } from "@tanstack/react-query"; @@ -66,6 +67,7 @@ export function TaskDetail({ task: initialTask }: TaskDetailProps) { : effectiveRepoPath; const [filePickerOpen, setFilePickerOpen] = useState(false); + const filePickerKey = useShortcut("file-picker"); const { enableScope, disableScope } = useHotkeysContext(); @@ -76,7 +78,7 @@ export function TaskDetail({ task: initialTask }: TaskDetailProps) { }; }, [enableScope, disableScope]); - useHotkeys("mod+p", () => setFilePickerOpen(true), { + useHotkeys(filePickerKey, () => setFilePickerOpen(true), { enableOnContentEditable: true, enableOnFormTags: true, preventDefault: true, diff --git a/apps/code/src/renderer/stores/keybindingsStore.test.ts b/apps/code/src/renderer/stores/keybindingsStore.test.ts index 52ab09e19..ad67b6c01 100644 --- a/apps/code/src/renderer/stores/keybindingsStore.test.ts +++ b/apps/code/src/renderer/stores/keybindingsStore.test.ts @@ -188,43 +188,107 @@ describe("keybindingsStore", () => { }); }); + describe("updateKeybinding", () => { + it("replaces only the edited key when there are existing custom bindings", () => { + useKeybindingsStore.setState({ + customKeybindings: { "new-task": ["ctrl+p", "ctrl+q"] }, + }); + useKeybindingsStore + .getState() + .updateKeybinding("new-task", "ctrl+p", "ctrl+x"); + expect( + useKeybindingsStore.getState().customKeybindings["new-task"], + ).toEqual(["ctrl+x", "ctrl+q"]); + }); + + it("when editing a default binding, copies all defaults and replaces only the target", () => { + // new-task has 2 defaults: mod+n and mod+t + useKeybindingsStore + .getState() + .updateKeybinding("new-task", "mod+n", "ctrl+x"); + expect( + useKeybindingsStore.getState().customKeybindings["new-task"], + ).toEqual(["ctrl+x", "mod+t"]); + }); + + it("when editing the only default binding, stores just the new key", () => { + useKeybindingsStore + .getState() + .updateKeybinding("command-menu", "mod+k", "ctrl+x"); + expect( + useKeybindingsStore.getState().customKeybindings["command-menu"], + ).toEqual(["ctrl+x"]); + }); + }); + + describe("addKeybinding — max binding limit", () => { + it("does not add a third binding beyond the max of 2", () => { + useKeybindingsStore.getState().addKeybinding("command-menu", "ctrl+p"); + useKeybindingsStore.getState().addKeybinding("command-menu", "ctrl+q"); + useKeybindingsStore.getState().addKeybinding("command-menu", "ctrl+r"); + expect( + useKeybindingsStore.getState().customKeybindings["command-menu"], + ).toEqual(["ctrl+p", "ctrl+q"]); + }); + }); + describe("findConflict", () => { beforeEach(() => { useKeybindingsStore.setState({ customKeybindings: {} }); }); - it("returns null when no conflict exists", () => { - expect(findConflict("ctrl+z", "command-menu")).toBeNull(); + it("returns no conflict when key is unused", () => { + const result = findConflict("ctrl+z", "command-menu"); + expect(result.description).toBeNull(); }); - it("detects a conflict with a default binding on another shortcut", () => { - // mod+b is the default for toggle-left-sidebar - expect(findConflict("mod+b", "command-menu")).toBe("toggle-left-sidebar"); + it("detects a conflict with a configurable default binding", () => { + // mod+b is the default for toggle-left-sidebar (configurable) + const result = findConflict("mod+b", "command-menu"); + expect(result.id).toBe("toggle-left-sidebar"); + expect(result.isFixed).toBe(false); }); it("does not flag the excluded shortcut's own default as a conflict", () => { - // mod+k is command-menu's own default — should not conflict with itself - expect(findConflict("mod+k", "command-menu")).toBeNull(); + // mod+k is command-menu's own default + const result = findConflict("mod+k", "command-menu"); + expect(result.description).toBeNull(); }); it("detects a conflict within comma-separated default alternates", () => { // prev-task default includes "ctrl+shift+tab" as an alternate - expect(findConflict("ctrl+shift+tab", "command-menu")).toBe("prev-task"); + const result = findConflict("ctrl+shift+tab", "command-menu"); + expect(result.id).toBe("prev-task"); }); it("detects a conflict with a custom binding on another shortcut", () => { useKeybindingsStore.setState({ customKeybindings: { settings: ["ctrl+alt+s"] }, }); - expect(findConflict("ctrl+alt+s", "command-menu")).toBe("settings"); + const result = findConflict("ctrl+alt+s", "command-menu"); + expect(result.id).toBe("settings"); }); it("does not conflict with custom binding on the excluded shortcut itself", () => { useKeybindingsStore.setState({ customKeybindings: { "command-menu": ["ctrl+p"] }, }); - // ctrl+p is a custom binding on command-menu — assigning it to command-menu again is fine - expect(findConflict("ctrl+p", "command-menu")).toBeNull(); + const result = findConflict("ctrl+p", "command-menu"); + expect(result.description).toBeNull(); + }); + + it("detects mod+, conflict correctly despite comma in the key", () => { + // settings default is mod+, — the comma is part of the key, not a separator + const result = findConflict("mod+,", "command-menu"); + expect(result.id).toBe("settings"); + }); + + it("detects conflicts with non-configurable shortcuts", () => { + // editor-underline (mod+u) is non-configurable (Tiptap internal) and + // not used by any configurable shortcut + const result = findConflict("mod+u", "command-menu"); + expect(result.isFixed).toBe(true); + expect(result.description).toBeTruthy(); }); }); }); diff --git a/apps/code/src/renderer/stores/keybindingsStore.ts b/apps/code/src/renderer/stores/keybindingsStore.ts index 4dd6de1fb..555e230ac 100644 --- a/apps/code/src/renderer/stores/keybindingsStore.ts +++ b/apps/code/src/renderer/stores/keybindingsStore.ts @@ -2,15 +2,23 @@ import { CONFIGURABLE_SHORTCUT_IDS, type ConfigurableShortcutId, DEFAULT_KEYBINDINGS, + KEYBOARD_SHORTCUTS, } from "@renderer/constants/keyboard-shortcuts"; import { electronStorage } from "@utils/electronStorage"; import { create } from "zustand"; import { persist } from "zustand/middleware"; +export const MAX_CUSTOM_BINDINGS = 2; + interface KeybindingsState { customKeybindings: Partial>; getKey: (id: ConfigurableShortcutId) => string; addKeybinding: (id: ConfigurableShortcutId, key: string) => void; + updateKeybinding: ( + id: ConfigurableShortcutId, + oldKey: string, + newKey: string, + ) => void; removeKeybinding: (id: ConfigurableShortcutId, key: string) => void; resetShortcut: (id: ConfigurableShortcutId) => void; resetAll: () => void; @@ -25,18 +33,59 @@ export function resolveKey( return DEFAULT_KEYBINDINGS[id]; } +/** + * Split a keybinding string by comma, but preserve commas that are part of a + * key combo (e.g. "mod+," must not be split at the trailing comma). + * A valid separator comma is one NOT immediately preceded by "+". + */ +export function splitBindings(keyStr: string): string[] { + // Split on commas that are not preceded by "+" + return keyStr + .split(/(? k.trim()) + .filter(Boolean); +} + +export interface ConflictResult { + id: ConfigurableShortcutId | null; + description: string | null; + /** true when the conflicting shortcut is not user-configurable */ + isFixed: boolean; +} + export function findConflict( newKey: string, excludeId: ConfigurableShortcutId, -): ConfigurableShortcutId | null { +): ConflictResult { const state = useKeybindingsStore.getState(); + + // Check configurable shortcuts first (with custom overrides applied) for (const id of CONFIGURABLE_SHORTCUT_IDS) { if (id === excludeId) continue; const keyStr = state.getKey(id); - const parts = keyStr.split(",").map((k) => k.trim()); - if (parts.includes(newKey)) return id; + const parts = splitBindings(keyStr); + if (parts.includes(newKey)) { + const entry = KEYBOARD_SHORTCUTS.find((s) => s.id === id); + return { id, description: entry?.description ?? id, isFixed: false }; + } + } + + // Check non-configurable shortcuts against their static default keys + for (const shortcut of KEYBOARD_SHORTCUTS) { + if ( + CONFIGURABLE_SHORTCUT_IDS.includes(shortcut.id as ConfigurableShortcutId) + ) + continue; + const parts = splitBindings(shortcut.keys); + if (shortcut.alternateKeys) { + parts.push(...splitBindings(shortcut.alternateKeys)); + } + if (parts.includes(newKey)) { + return { id: null, description: shortcut.description, isFixed: true }; + } } - return null; + + return { id: null, description: null, isFixed: false }; } export const useKeybindingsStore = create()( @@ -49,6 +98,7 @@ export const useKeybindingsStore = create()( addKeybinding: (id, key) => { const existing = get().customKeybindings[id] ?? []; if (existing.includes(key)) return; + if (existing.length >= MAX_CUSTOM_BINDINGS) return; set({ customKeybindings: { ...get().customKeybindings, @@ -57,6 +107,20 @@ export const useKeybindingsStore = create()( }); }, + updateKeybinding: (id, oldKey, newKey) => { + const existing = get().customKeybindings[id] ?? []; + // When editing a default binding, copy all defaults first so the other + // defaults are preserved — only the edited key gets replaced. + const base = + existing.length > 0 + ? existing + : splitBindings(DEFAULT_KEYBINDINGS[id]); + const updated = base.map((k) => (k === oldKey ? newKey : k)); + set({ + customKeybindings: { ...get().customKeybindings, [id]: updated }, + }); + }, + removeKeybinding: (id, key) => { const existing = get().customKeybindings[id] ?? []; const updated = existing.filter((k) => k !== key); diff --git a/apps/code/tests/e2e/tests/shortcuts.spec.ts b/apps/code/tests/e2e/tests/shortcuts.spec.ts index bed4c9a89..f9dc76f73 100644 --- a/apps/code/tests/e2e/tests/shortcuts.spec.ts +++ b/apps/code/tests/e2e/tests/shortcuts.spec.ts @@ -4,13 +4,11 @@ import { expect, test } from "../fixtures/electron"; const isMac = process.platform === "darwin"; const modKey = isMac ? "Meta" : "Control"; -// Opens the shortcuts sheet via keyboard shortcut. async function openShortcutsSheet(window: Page) { await window.keyboard.press(`${modKey}+Slash`); await window.getByText("Keyboard Combos").waitFor({ timeout: 5000 }); } -// Returns true when the main layout is rendered (requires authenticated state). async function isMainLayout(window: Page): Promise { await window.locator("#root > *").waitFor({ timeout: 30000 }); await window @@ -24,7 +22,6 @@ async function isMainLayout(window: Page): Promise { .catch(() => false); } -// Clears all custom bindings via the Reset all button if it's visible. async function resetAllIfNeeded(window: Page) { try { await openShortcutsSheet(window); @@ -35,6 +32,32 @@ async function resetAllIfNeeded(window: Page) { } catch {} } +// Returns the chip button(s) for a named shortcut. +// Each individual binding renders as a separate button with this title pattern. +function getChips(window: Page, commandLabel: string) { + return window.locator( + `button[title='Click to edit binding for "${commandLabel}"']`, + ); +} + +// Opens the recording modal via right-click → "Add another binding". +async function openAddRecording(window: Page, commandLabel: string) { + await getChips(window, commandLabel).first().click({ button: "right" }); + await window.getByRole("menuitem", { name: "Add another binding" }).click(); + await window + .getByText(`Add new binding for "${commandLabel}"`) + .waitFor({ timeout: 3000 }); +} + +// Records a combo and confirms with Enter. Assumes the recording modal is already open. +async function recordAndConfirm(window: Page, combo: string) { + await window.keyboard.press(combo); + await window + .getByText("Press Enter to confirm, Escape to cancel") + .waitFor({ timeout: 2000 }); + await window.keyboard.press("Enter"); +} + test.describe("Configurable Keyboard Shortcuts", () => { test.beforeEach(async ({ window }) => { const ready = await isMainLayout(window); @@ -61,39 +84,40 @@ test.describe("Configurable Keyboard Shortcuts", () => { } }); - // ─── Hover controls ─────────────────────────────────────────────────────── + // ─── Configurable vs non-configurable ───────────────────────────────────── - test("hovering a configurable row reveals the add (+) button", async ({ + test("configurable rows expose clickable chip buttons", async ({ window, }) => { await openShortcutsSheet(window); - await window.getByText("Open command menu").hover(); - await expect(window.getByTitle("Add binding").first()).toBeVisible(); + // "Open command menu" is configurable + await expect(getChips(window, "Open command menu").first()).toBeVisible(); }); - test("non-configurable rows do not show an add (+) button", async ({ - window, - }) => { + test("non-configurable rows show a tooltip on hover", async ({ window }) => { await openShortcutsSheet(window); // "Switch to task 1-9" is intentionally non-configurable + // The keycap wrapper has a Tooltip with this text; hover to reveal it await window.getByText("Switch to task 1-9").hover(); - const addBtns = window.getByTitle("Add binding"); - expect(await addBtns.count()).toBe(0); + await expect( + window.getByText("This shortcut cannot be customized"), + ).toBeVisible({ timeout: 2000 }); }); - // ─── Recording ──────────────────────────────────────────────────────────── + // ─── Recording modal ────────────────────────────────────────────────────── - test("clicking + enters recording mode", async ({ window }) => { + test("clicking a chip opens the recording modal in edit mode", async ({ + window, + }) => { await openShortcutsSheet(window); - await window.getByText("Open inbox").hover(); - await window.getByTitle("Add binding").click(); - - await expect( - window.locator('input[aria-label="Press new shortcut"]'), - ).toBeVisible(); + await getChips(window, "Open inbox").first().click(); + await expect(window.getByText('Edit binding for "Open inbox"')).toBeVisible( + { timeout: 3000 }, + ); + await expect(window.getByText("Press a key combination...")).toBeVisible(); }); test("pressing Escape cancels recording without closing the sheet", async ({ @@ -101,151 +125,292 @@ test.describe("Configurable Keyboard Shortcuts", () => { }) => { await openShortcutsSheet(window); - await window.getByText("Open inbox").hover(); - await window.getByTitle("Add binding").click(); - - const input = window.locator('input[aria-label="Press new shortcut"]'); - await expect(input).toBeVisible(); + await getChips(window, "Open inbox").first().click(); + await window + .getByText('Edit binding for "Open inbox"') + .waitFor({ timeout: 3000 }); await window.keyboard.press("Escape"); - // Input should close… - await expect(input).not.toBeVisible(); - // …but the sheet should stay open + // Modal closes, sheet stays open + await expect( + window.getByText('Edit binding for "Open inbox"'), + ).not.toBeVisible(); await expect(window.getByText("Keyboard Combos")).toBeVisible(); }); - test("bare letter key is rejected in recording mode", async ({ window }) => { + test("clicking the backdrop closes recording without saving", async ({ + window, + }) => { await openShortcutsSheet(window); - await window.getByText("Open inbox").hover(); - await window.getByTitle("Add binding").click(); + await getChips(window, "Open inbox").first().click(); + await window + .getByText('Edit binding for "Open inbox"') + .waitFor({ timeout: 3000 }); + + // Click the blurred backdrop — the outer fixed overlay has a backdrop-filter style + await window + .locator('[style*="backdrop-filter"]') + .click({ position: { x: 10, y: 10 } }); + + await expect( + window.getByText('Edit binding for "Open inbox"'), + ).not.toBeVisible({ timeout: 2000 }); + await expect(window.getByText("Keyboard Combos")).toBeVisible(); + }); + + test("bare letter key is ignored in recording mode", async ({ window }) => { + await openShortcutsSheet(window); - const input = window.locator('input[aria-label="Press new shortcut"]'); - await expect(input).toBeVisible(); + await getChips(window, "Open inbox").first().click(); + await window + .getByText('Edit binding for "Open inbox"') + .waitFor({ timeout: 3000 }); - // Press a bare letter with no modifier — should be ignored await window.keyboard.press("k"); - // Input should still be in recording mode (not closed) - await expect(input).toBeVisible(); + // No combo captured — placeholder still shown, modal still open + await expect(window.getByText("Press a key combination...")).toBeVisible(); + await expect( + window.getByText('Edit binding for "Open inbox"'), + ).toBeVisible(); }); - // ─── Saving a binding ───────────────────────────────────────────────────── - - test("recording a valid combo saves it and shows keycap + remove button", async ({ + test("Enter without a captured combo does not close the modal", async ({ window, }) => { await openShortcutsSheet(window); - await window.getByText("Open inbox").hover(); - await window.getByTitle("Add binding").click(); + await getChips(window, "Open inbox").first().click(); + await window + .getByText('Edit binding for "Open inbox"') + .waitFor({ timeout: 3000 }); + + await window.keyboard.press("Enter"); + + // Modal should still be open + await expect( + window.getByText('Edit binding for "Open inbox"'), + ).toBeVisible(); + }); + + // ─── Saving a binding ───────────────────────────────────────────────────── + + test("recording and pressing Enter saves the binding", async ({ window }) => { + await openShortcutsSheet(window); + + await openAddRecording(window, "Open inbox"); - // Use ControlOrMeta+Shift+Z — not in the default shortcut set await window.keyboard.press("ControlOrMeta+Shift+Z"); + await window + .getByText("Press Enter to confirm, Escape to cancel") + .waitFor({ timeout: 2000 }); + await window.keyboard.press("Enter"); - // Recording input should close + // Modal closes await expect( - window.locator('input[aria-label="Press new shortcut"]'), + window.getByText('Add new binding for "Open inbox"'), ).not.toBeVisible({ timeout: 3000 }); - // Remove and reset buttons should now be visible on hover - await window.getByText("Open inbox").hover(); - await expect(window.getByTitle("Remove binding").first()).toBeVisible(); - await expect(window.getByTitle("Reset to default").first()).toBeVisible(); + // The chip for the shortcut should still be visible (now showing the new binding) + await expect(getChips(window, "Open inbox").first()).toBeVisible(); }); - test("can add a second binding to the same shortcut", async ({ window }) => { + test("right-click context menu offers Edit and Add another binding", async ({ + window, + }) => { await openShortcutsSheet(window); - // Add first binding - await window.getByText("Open inbox").hover(); - await window.getByTitle("Add binding").click(); - await window.keyboard.press("ControlOrMeta+Shift+Z"); - await window - .locator('input[aria-label="Press new shortcut"]') - .waitFor({ state: "hidden" }); + await getChips(window, "Open inbox").first().click({ button: "right" }); - // Add second binding - await window.getByText("Open inbox").hover(); - await window.getByTitle("Add binding").click(); - await window.keyboard.press("ControlOrMeta+Shift+X"); + await expect( + window.getByRole("menuitem", { name: "Edit binding" }), + ).toBeVisible({ timeout: 2000 }); + await expect( + window.getByRole("menuitem", { name: "Add another binding" }), + ).toBeVisible(); + }); + + test("can add a second binding via Add another binding", async ({ + window, + }) => { + await openShortcutsSheet(window); + + // Add first custom binding + await openAddRecording(window, "Open inbox"); + await recordAndConfirm(window, "ControlOrMeta+Shift+Z"); + + // Add second custom binding + await getChips(window, "Open inbox").first().click({ button: "right" }); + await window.getByRole("menuitem", { name: "Add another binding" }).click(); await window - .locator('input[aria-label="Press new shortcut"]') - .waitFor({ state: "hidden" }); + .getByText('Add new binding for "Open inbox"') + .waitFor({ timeout: 3000 }); + await recordAndConfirm(window, "ControlOrMeta+Shift+X"); + + // Two chips should now exist for this shortcut + await expect(getChips(window, "Open inbox")).toHaveCount(2, { + timeout: 3000, + }); + }); + + test("Add another binding option is absent at the 2-binding limit", async ({ + window, + }) => { + await openShortcutsSheet(window); + + // Fill both custom binding slots + await openAddRecording(window, "Open inbox"); + await recordAndConfirm(window, "ControlOrMeta+Shift+Z"); - // Both remove buttons should be visible (one per binding) - await window.getByText("Open inbox").hover(); - const removeBtns = window.getByTitle("Remove binding"); - expect(await removeBtns.count()).toBe(2); + await getChips(window, "Open inbox").first().click({ button: "right" }); + await window.getByRole("menuitem", { name: "Add another binding" }).click(); + await recordAndConfirm(window, "ControlOrMeta+Shift+X"); + + // Right-click again — "Add another binding" should be gone + await getChips(window, "Open inbox").first().click({ button: "right" }); + await expect( + window.getByRole("menuitem", { name: "Add another binding" }), + ).not.toBeVisible({ timeout: 1000 }); }); // ─── Conflict detection ─────────────────────────────────────────────────── - test("assigning an already-used combo shows a conflict toast", async ({ + test("pressing an already-used combo shows amber conflict message", async ({ window, }) => { await openShortcutsSheet(window); - await window.getByText("Open command menu").hover(); - await window.getByTitle("Add binding").click(); + await openAddRecording(window, "Open command menu"); // mod+b is the default for "Toggle left sidebar" await window.keyboard.press(`${modKey}+b`); await expect( - window.getByText('Already used by "Toggle left sidebar"'), - ).toBeVisible({ timeout: 5000 }); - - // Recording should be cancelled — no remove button - await window.getByText("Open command menu").hover(); - await expect(window.getByTitle("Remove binding")).not.toBeVisible(); + window.getByText(/Conflicts with "Toggle left sidebar"/), + ).toBeVisible({ timeout: 3000 }); }); - // ─── Removing a binding ─────────────────────────────────────────────────── + test("Enter is blocked while a conflict is shown", async ({ window }) => { + await openShortcutsSheet(window); + + await openAddRecording(window, "Open command menu"); + + await window.keyboard.press(`${modKey}+b`); + await window + .getByText(/Conflicts with "Toggle left sidebar"/) + .waitFor({ timeout: 2000 }); + + // Enter should NOT dismiss the modal while conflict is active + await window.keyboard.press("Enter"); + await expect( + window.getByText(/Conflicts with "Toggle left sidebar"/), + ).toBeVisible(); + }); - test("clicking × removes a custom binding", async ({ window }) => { + test("resolving a conflict allows the binding to be saved", async ({ + window, + }) => { await openShortcutsSheet(window); - // Add a binding - await window.getByText("Open inbox").hover(); - await window.getByTitle("Add binding").click(); + await openAddRecording(window, "Open command menu"); + + // First press a conflicting key, then a safe one + await window.keyboard.press(`${modKey}+b`); + await window.getByText(/Conflicts with/).waitFor({ timeout: 2000 }); + await window.keyboard.press("ControlOrMeta+Shift+Z"); await window - .locator('input[aria-label="Press new shortcut"]') - .waitFor({ state: "hidden" }); + .getByText("Press Enter to confirm, Escape to cancel") + .waitFor({ timeout: 2000 }); + await window.keyboard.press("Enter"); - // Remove it - await window.getByText("Open inbox").hover(); - await window.getByTitle("Remove binding").click(); + await expect( + window.getByText('Add new binding for "Open command menu"'), + ).not.toBeVisible({ timeout: 3000 }); + }); - // Remove and reset buttons should now be gone - await window.getByText("Open inbox").hover(); - await expect(window.getByTitle("Remove binding")).not.toBeVisible(); - await expect(window.getByTitle("Reset to default")).not.toBeVisible(); + // ─── Removing a binding ─────────────────────────────────────────────────── + + test("right-click Remove binding removes a custom binding", async ({ + window, + }) => { + await openShortcutsSheet(window); + + await openAddRecording(window, "Open inbox"); + await recordAndConfirm(window, "ControlOrMeta+Shift+Z"); + + // Add a second so we can remove one without hitting the single-binding guard + await getChips(window, "Open inbox").first().click({ button: "right" }); + await window.getByRole("menuitem", { name: "Add another binding" }).click(); + await recordAndConfirm(window, "ControlOrMeta+Shift+X"); + + await expect(getChips(window, "Open inbox")).toHaveCount(2, { + timeout: 3000, + }); + + await getChips(window, "Open inbox").first().click({ button: "right" }); + await window.getByRole("menuitem", { name: "Remove binding" }).click(); + + await expect(getChips(window, "Open inbox")).toHaveCount(1, { + timeout: 3000, + }); + }); + + test("Remove binding is disabled and shows a tooltip when it is the only binding", async ({ + window, + }) => { + await openShortcutsSheet(window); + + // "Open inbox" has one default binding — Remove should be disabled + await getChips(window, "Open inbox").first().click({ button: "right" }); + + const removeItem = window.getByRole("menuitem", { name: "Remove binding" }); + // Radix disables items via aria-disabled or data-disabled + const isDisabled = + (await removeItem.getAttribute("aria-disabled")) === "true" || + (await removeItem.getAttribute("data-disabled")) !== null; + expect(isDisabled).toBe(true); }); // ─── Per-shortcut reset ─────────────────────────────────────────────────── - test("↩ resets an individual shortcut to its default", async ({ window }) => { + test("Reset to default is disabled when already at default", async ({ + window, + }) => { await openShortcutsSheet(window); - // Add a binding - await window.getByText("Open inbox").hover(); - await window.getByTitle("Add binding").click(); - await window.keyboard.press("ControlOrMeta+Shift+Z"); - await window - .locator('input[aria-label="Press new shortcut"]') - .waitFor({ state: "hidden" }); + await getChips(window, "Open inbox").first().click({ button: "right" }); + + const resetItem = window.getByRole("menuitem", { + name: "Reset to default", + }); + const isDisabled = + (await resetItem.getAttribute("aria-disabled")) === "true" || + (await resetItem.getAttribute("data-disabled")) !== null; + expect(isDisabled).toBe(true); + }); - // Reset this shortcut - await window.getByText("Open inbox").hover(); - await window.getByTitle("Reset to default").click(); + test("Reset to default reverts a customised shortcut", async ({ window }) => { + await openShortcutsSheet(window); - // Should revert — no custom controls visible - await window.getByText("Open inbox").hover(); - await expect(window.getByTitle("Reset to default")).not.toBeVisible(); - await expect(window.getByTitle("Remove binding")).not.toBeVisible(); + await openAddRecording(window, "Open inbox"); + await recordAndConfirm(window, "ControlOrMeta+Shift+Z"); + + // Reset + await getChips(window, "Open inbox").first().click({ button: "right" }); + await window.getByRole("menuitem", { name: "Reset to default" }).click(); + + // Now Reset to default should be disabled again (back at default) + await getChips(window, "Open inbox").first().click({ button: "right" }); + const resetItem = window.getByRole("menuitem", { + name: "Reset to default", + }); + const isDisabled = + (await resetItem.getAttribute("aria-disabled")) === "true" || + (await resetItem.getAttribute("data-disabled")) !== null; + expect(isDisabled).toBe(true); }); // ─── Reset all ──────────────────────────────────────────────────────────── @@ -265,14 +430,9 @@ test.describe("Configurable Keyboard Shortcuts", () => { }) => { await openShortcutsSheet(window); - await window.getByText("Open inbox").hover(); - await window.getByTitle("Add binding").click(); - await window.keyboard.press("ControlOrMeta+Shift+Z"); - await window - .locator('input[aria-label="Press new shortcut"]') - .waitFor({ state: "hidden" }); + await openAddRecording(window, "Open inbox"); + await recordAndConfirm(window, "ControlOrMeta+Shift+Z"); - // Scroll to bottom to find the button const resetAllBtn = window.getByText("Reset all shortcuts to defaults"); await resetAllBtn.scrollIntoViewIfNeeded(); await expect(resetAllBtn).toBeVisible(); @@ -281,31 +441,16 @@ test.describe("Configurable Keyboard Shortcuts", () => { test("clicking Reset all clears all custom bindings", async ({ window }) => { await openShortcutsSheet(window); - // Add bindings to two different shortcuts - await window.getByText("Open inbox").hover(); - await window.getByTitle("Add binding").click(); - await window.keyboard.press("ControlOrMeta+Shift+Z"); - await window - .locator('input[aria-label="Press new shortcut"]') - .waitFor({ state: "hidden" }); + await openAddRecording(window, "Open inbox"); + await recordAndConfirm(window, "ControlOrMeta+Shift+Z"); - await window.getByText("Open command menu").hover(); - await window.getByTitle("Add binding").click(); - await window.keyboard.press("ControlOrMeta+Shift+X"); - await window - .locator('input[aria-label="Press new shortcut"]') - .waitFor({ state: "hidden" }); + await openAddRecording(window, "Open command menu"); + await recordAndConfirm(window, "ControlOrMeta+Shift+X"); - // Click Reset all const resetAllBtn = window.getByText("Reset all shortcuts to defaults"); await resetAllBtn.scrollIntoViewIfNeeded(); await resetAllBtn.click(); - // Button should disappear - await expect(resetAllBtn).not.toBeVisible(); - - // Neither row should have custom controls any more - await window.getByText("Open inbox").hover(); - await expect(window.getByTitle("Remove binding")).not.toBeVisible(); + await expect(resetAllBtn).not.toBeVisible({ timeout: 3000 }); }); }); From aa353a1a9d23d4d8fe701d2d6f9f7bf53ef40310 Mon Sep 17 00:00:00 2001 From: Basit Balogun Date: Sat, 23 May 2026 03:29:58 +0100 Subject: [PATCH 7/7] fix: address Greptile review; clean up comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Deduplicate in updateKeybinding — conflict detection excludes the shortcut being edited so editing one binding to match another on the same shortcut could produce ["ctrl+q","ctrl+q"], duplicate React keys and broken chip reconciliation - Remove ArrowUp/Down gate around prompt-history navigation so custom non-arrow bindings (e.g. Ctrl+K) actually fire when pressed, not just when the physical key is an arrow - Remove obvious section-divider comments and redundant JSX labels (Header, Scrollable list, Sticky footer); keep non-obvious rationale comments (window-level capture, backdrop dismiss, canAddMore budget, dedup note, ArrowKey gate explanation) --- .../components/KeyboardShortcutsSheet.tsx | 3 - .../renderer/components/ShortcutRecorder.tsx | 24 +---- .../message-editor/tiptap/useTiptapEditor.ts | 95 ++++++++++--------- .../src/renderer/stores/keybindingsStore.ts | 8 +- 4 files changed, 58 insertions(+), 72 deletions(-) diff --git a/apps/code/src/renderer/components/KeyboardShortcutsSheet.tsx b/apps/code/src/renderer/components/KeyboardShortcutsSheet.tsx index ccd0571ac..0deadccc1 100644 --- a/apps/code/src/renderer/components/KeyboardShortcutsSheet.tsx +++ b/apps/code/src/renderer/components/KeyboardShortcutsSheet.tsx @@ -37,7 +37,6 @@ export function KeyboardShortcutsSheet({ onEscapeKeyDown={(e) => e.preventDefault()} className="!pb-0 flex max-h-[80vh] flex-col overflow-hidden" > - {/* Header */}