diff --git a/web/src/components/session/chat/textarea-controls.js b/web/src/components/session/chat/textarea-controls.js index b9e8aba5..309ed9a0 100644 --- a/web/src/components/session/chat/textarea-controls.js +++ b/web/src/components/session/chat/textarea-controls.js @@ -1,3 +1,5 @@ +import { matchesAction } from '../../../shared/keybindings.js'; + export function setupTextareaControls({ windowImpl = window, textarea, @@ -35,11 +37,11 @@ export function setupTextareaControls({ event.preventDefault(); form?.requestSubmit?.(); } - if (event.key === 'Tab' && event.shiftKey) { + if (matchesAction('cycle-thinking-level', event)) { event.preventDefault(); getThinkingSelector()?.cycle?.(); } - if (event.ctrlKey && (event.key.toLowerCase() === 'i' || event.key.toLowerCase() === 'l')) { + if (matchesAction('open-model-selector', event)) { event.preventDefault(); getModelSelector()?.open?.(); } diff --git a/web/src/routes/SessionsPage.svelte b/web/src/routes/SessionsPage.svelte index b2dee728..26d29546 100644 --- a/web/src/routes/SessionsPage.svelte +++ b/web/src/routes/SessionsPage.svelte @@ -9,6 +9,7 @@ import { createStatusEvents } from '../shared/status-events.js'; import { openSessionPalette, refreshSessionPalette } from '../shared/command-palette-runtime.js'; import { setupKeyboardNav } from '../shared/keyboard-nav.js'; + import { matchesAction } from '../shared/keybindings.js'; import { toggleTheme, syncThemeIcons } from '../shared/theme.js'; import { configureSettingsSync, @@ -253,14 +254,14 @@ } catch {} const keydown = (e) => { - if ((e.metaKey || e.ctrlKey) && e.shiftKey && e.key.toLowerCase() === 'l') { + if (matchesAction('toggle-theme', e)) { e.preventDefault(); e.stopPropagation(); toggleTheme(window, document); syncThemeIcons(document); return; } - if ((e.metaKey || e.ctrlKey) && e.key.toLowerCase() === 'k') { + if (matchesAction('open-palette', e)) { e.preventDefault(); openPalette(); return; diff --git a/web/src/session/session-globals.js b/web/src/session/session-globals.js index 64d33df0..667b54e2 100644 --- a/web/src/session/session-globals.js +++ b/web/src/session/session-globals.js @@ -10,6 +10,7 @@ import * as doneNotifier from './chat/done-notifier.js'; import * as sidebarApi from './ui/sidebar.js'; import { openSessionPalette } from '../shared/command-palette-runtime.js'; import { setupKeyboardNav } from '../shared/keyboard-nav.js'; +import { matchesAction } from '../shared/keybindings.js'; import { openShortcuts } from './session-modals.svelte.js'; import { sessionRuntime } from './session-runtime.js'; import { toggleTheme, syncThemeIcons } from '../shared/theme.js'; @@ -42,7 +43,7 @@ export function setupSessionGlobals({ windowImpl, documentImpl }) { // ── Global keyboard shortcuts ────────────────────────────────────────────── // Cmd+K — session list palette on(target, 'keydown', (e) => { - if ((e.metaKey || e.ctrlKey) && e.key.toLowerCase() === 'k') { + if (matchesAction('open-palette', e)) { e.preventDefault(); openSessionPalette(); } @@ -50,7 +51,7 @@ export function setupSessionGlobals({ windowImpl, documentImpl }) { // Cmd+B — toggle sidebar/tree on(target, 'keydown', (e) => { - if ((e.metaKey || e.ctrlKey) && e.key.toLowerCase() === 'b') { + if (matchesAction('toggle-sidebar', e)) { e.preventDefault(); const sidebar = documentImpl.getElementById('sidebar'); if (sidebarApi.isMobileLayout({ windowImpl: target })) { @@ -67,7 +68,7 @@ export function setupSessionGlobals({ windowImpl, documentImpl }) { // Cmd+T — new session on(target, 'keydown', (e) => { - if ((e.metaKey || e.ctrlKey) && e.key.toLowerCase() === 't') { + if (matchesAction('new-session', e)) { e.preventDefault(); const newBtn = documentImpl.getElementById('new-btn'); if (newBtn) newBtn.click(); @@ -80,7 +81,7 @@ export function setupSessionGlobals({ windowImpl, documentImpl }) { target, 'keydown', (e) => { - if ((e.metaKey || e.ctrlKey) && e.shiftKey && e.key.toLowerCase() === 'l') { + if (matchesAction('toggle-theme', e)) { e.preventDefault(); e.stopPropagation(); toggleTheme(target, documentImpl); @@ -92,7 +93,7 @@ export function setupSessionGlobals({ windowImpl, documentImpl }) { // Cmd+Shift+N — toggle scratchpad (right sidebar) on(target, 'keydown', (e) => { - if ((e.metaKey || e.ctrlKey) && e.shiftKey && e.key.toLowerCase() === 'n') { + if (matchesAction('toggle-scratchpad', e)) { e.preventDefault(); sessionRuntime.rightSidebar?.toggle(); } @@ -101,7 +102,7 @@ export function setupSessionGlobals({ windowImpl, documentImpl }) { // Cmd+/ — keyboard shortcuts help modal (the Svelte // component, opened via the shared sessionModals store). on(target, 'keydown', (e) => { - if ((e.metaKey || e.ctrlKey) && e.key === '/') { + if (matchesAction('open-shortcuts-help', e)) { e.preventDefault(); openShortcuts(); } diff --git a/web/src/session/ui/search-filters.js b/web/src/session/ui/search-filters.js index 931d80bd..a59c462c 100644 --- a/web/src/session/ui/search-filters.js +++ b/web/src/session/ui/search-filters.js @@ -1,3 +1,5 @@ +import { matchesAction } from '../../shared/keybindings.js'; + export function setupSessionSearchAndFilters({ documentImpl = document, getLeafId, @@ -68,14 +70,13 @@ export function setupSessionKeyboardShortcuts({ return; } - const key = e.key.toLowerCase(); - if (key === 't') { + if (matchesAction('toggle-thinking', e)) { e.preventDefault(); toggleThinking(); - } else if (key === 'o') { + } else if (matchesAction('toggle-tools', e)) { e.preventDefault(); toggleToolsVisibility(); - } else if (key === 'p') { + } else if (matchesAction('toggle-tool-outputs', e)) { e.preventDefault(); toggleToolOutputs(); } diff --git a/web/src/shared/keybindings.js b/web/src/shared/keybindings.js new file mode 100644 index 00000000..30033ab0 --- /dev/null +++ b/web/src/shared/keybindings.js @@ -0,0 +1,133 @@ +// Central registry of remappable keyboard actions. +// +// Before this module, every shortcut was an inline `e.key === '...'` check +// scattered across the nav, session, filter, and composer handlers. Those +// handlers now ask the registry whether an event matches a named action, so a +// single source of truth defines the default binding for each one — and a later +// change can layer user overrides on top without touching the handlers again. +// +// Scope: only the global / navigation / composer shortcuts live here. Structural +// modal keys (Escape-to-close, arrow navigation, Tab focus-traps) and the +// composer's Enter-to-submit stay hardcoded in their components — they are UI +// affordances, not preferences. The multi-key `g g` sequence also stays in +// keyboard-nav.js; the registry models single chords only. + +// A combo is a `+`-joined string of optional modifiers followed by one key, +// e.g. `mod+k`, `mod+shift+l`, `shift+i`, `ctrl+i`, `j`. +// +// mod → the platform command key: metaKey OR ctrlKey (⌘ on macOS, Ctrl +// elsewhere). This mirrors the existing `e.metaKey || e.ctrlKey` guards. +// ctrl → ctrlKey specifically, independent of ⌘. Used by the composer +// model-selector, which historically checked only ctrlKey. +// shift → shiftKey. alt → altKey. +// +// The final token is the key, matched case-insensitively against event.key +// (so `shift+i` matches the 'I' that Shift+i produces). + +export const KEY_ACTIONS = [ + // General + { id: 'open-palette', category: 'general', combo: 'mod+k' }, + { id: 'toggle-sidebar', category: 'general', combo: 'mod+b' }, + { id: 'new-session', category: 'general', combo: 'mod+t' }, + { id: 'toggle-theme', category: 'general', combo: 'mod+shift+l' }, + { id: 'toggle-scratchpad', category: 'general', combo: 'mod+shift+n' }, + { id: 'open-shortcuts-help', category: 'general', combo: 'mod+/' }, + { id: 'open-settings', category: 'general', combo: 'mod+,' }, + + // Navigation. The calling handler already excludes editable targets and + // command modifiers. These match the literal produced key (`j`, `k`, and the + // shifted `G`/`I`) exactly as the original `e.key === …` checks did, so + // Caps Lock and every other path behave identically. + { id: 'scroll-down', category: 'navigation', combo: 'j', literalKey: true }, + { id: 'scroll-up', category: 'navigation', combo: 'k', literalKey: true }, + { id: 'scroll-bottom', category: 'navigation', combo: 'shift+g', literalKey: true }, + { id: 'focus-composer', category: 'navigation', combo: 'shift+i', literalKey: true }, + + // Entry toggles. Their original handler matched the key alone with no + // modifier guard, so these stay key-only (`plain`) to preserve that exactly. + { id: 'toggle-thinking', category: 'toggles', combo: 't', plain: true }, + { id: 'toggle-tools', category: 'toggles', combo: 'o', plain: true }, + { id: 'toggle-tool-outputs', category: 'toggles', combo: 'p', plain: true }, + + // Composer + { id: 'cycle-thinking-level', category: 'composer', combo: 'shift+tab' }, + // Historically opened by Ctrl+I or Ctrl+L; both remain until the settings UI + // lets users pick one. + { id: 'open-model-selector', category: 'composer', combo: 'ctrl+i', aliases: ['ctrl+l'] }, +]; + +const ACTIONS_BY_ID = new Map(KEY_ACTIONS.map((a) => [a.id, a])); + +// Default combo for an action id (throws in tests via the map miss if unknown). +export function defaultCombo(actionId) { + return ACTIONS_BY_ID.get(actionId)?.combo ?? null; +} + +// Parse a combo string into required modifiers and a normalized key. +export function parseCombo(combo) { + const tokens = String(combo).split('+'); + const key = tokens[tokens.length - 1].toLowerCase(); + const mods = new Set(tokens.slice(0, -1)); + return { + key, + mod: mods.has('mod'), + ctrl: mods.has('ctrl'), + shift: mods.has('shift'), + alt: mods.has('alt'), + }; +} + +// expectedEventKey returns the KeyboardEvent.key value a combo produces, for +// combos matched by their literal key: a shifted single letter arrives +// uppercased (`shift+g` → `G`), everything else unchanged (`j` → `j`). +export function expectedEventKey(combo) { + const { key, shift } = parseCombo(combo); + if (shift && /^[a-z]$/.test(key)) return key.toUpperCase(); + return key; +} + +// comboMatchesEvent reports whether a modifier-bearing chord matches an event. +// Modifiers not named in the combo must be absent, so `mod+k` never fires for +// `mod+shift+k`. `mod` accepts meta or ctrl; `ctrl` requires ctrl specifically. +export function comboMatchesEvent(combo, event) { + const want = parseCombo(combo); + const hasCommand = Boolean(event.metaKey || event.ctrlKey); + + if (want.mod) { + if (!hasCommand) return false; + } else if (want.ctrl) { + if (!event.ctrlKey) return false; + } else if (hasCommand || event.altKey) { + // Plain chords (no command modifier requested) must not carry ⌘/Ctrl/Alt. + return false; + } + // Shift is only enforced where it changes meaning. For letters (compared + // case-insensitively) and named keys like Tab, event.key is the same with or + // without Shift, so `mod+k` must reject Cmd+Shift+K. For punctuation, + // event.key is already the shifted result — some layouts need Shift to type + // `/` at all — so an unrequested Shift there is layout noise, not a + // different chord. + const shiftChangesMeaning = /^[a-z]$/.test(want.key) || want.key.length > 1; + if (want.shift && !event.shiftKey) return false; + if (!want.shift && event.shiftKey && shiftChangesMeaning) return false; + if (want.alt !== Boolean(event.altKey)) return false; + return String(event.key).toLowerCase() === want.key; +} + +// matchesAction reports whether an event triggers the named action under its +// current binding. Plain-key actions (`plain: true`) compare only the key, so +// the caller's own editable/modifier guard stays authoritative — preserving the +// exact behavior of the pre-registry handlers. +export function matchesAction(actionId, event, bindings = {}) { + const action = ACTIONS_BY_ID.get(actionId); + if (!action) return false; + const combo = bindings[actionId] || action.combo; + if (action.literalKey) { + return event.key === expectedEventKey(combo); + } + if (action.plain) { + return String(event.key).toLowerCase() === parseCombo(combo).key; + } + if (comboMatchesEvent(combo, event)) return true; + return (action.aliases || []).some((alias) => comboMatchesEvent(alias, event)); +} diff --git a/web/src/shared/keybindings.test.js b/web/src/shared/keybindings.test.js new file mode 100644 index 00000000..d5ff69a0 --- /dev/null +++ b/web/src/shared/keybindings.test.js @@ -0,0 +1,147 @@ +import { describe, it, expect } from 'vitest'; +import { + KEY_ACTIONS, + defaultCombo, + parseCombo, + comboMatchesEvent, + expectedEventKey, + matchesAction, +} from './keybindings.js'; + +const ev = (key, mods = {}) => ({ + key, + metaKey: false, + ctrlKey: false, + shiftKey: false, + altKey: false, + ...mods, +}); + +describe('parseCombo', () => { + it('splits modifiers and key', () => { + expect(parseCombo('mod+shift+l')).toEqual({ + key: 'l', + mod: true, + ctrl: false, + shift: true, + alt: false, + }); + }); + + it('treats a lone key as no modifiers', () => { + expect(parseCombo('j')).toMatchObject({ key: 'j', mod: false, shift: false }); + }); +}); + +describe('expectedEventKey', () => { + it('uppercases a shifted single letter', () => { + expect(expectedEventKey('shift+g')).toBe('G'); + expect(expectedEventKey('shift+i')).toBe('I'); + }); + + it('leaves bare keys unchanged', () => { + expect(expectedEventKey('j')).toBe('j'); + }); +}); + +describe('comboMatchesEvent', () => { + it('mod accepts either meta or ctrl', () => { + expect(comboMatchesEvent('mod+k', ev('k', { metaKey: true }))).toBe(true); + expect(comboMatchesEvent('mod+k', ev('k', { ctrlKey: true }))).toBe(true); + }); + + it('requires the command modifier for mod chords', () => { + expect(comboMatchesEvent('mod+k', ev('k'))).toBe(false); + }); + + it('rejects extra modifiers not named in the combo', () => { + expect(comboMatchesEvent('mod+k', ev('k', { metaKey: true, shiftKey: true }))).toBe(false); + }); + + it('distinguishes ctrl-only from mod', () => { + expect(comboMatchesEvent('ctrl+i', ev('i', { ctrlKey: true }))).toBe(true); + // ⌘I (meta, not ctrl) must not trigger a ctrl-only chord. + expect(comboMatchesEvent('ctrl+i', ev('i', { metaKey: true }))).toBe(false); + }); + + it('matches shift chords against the shifted key value', () => { + expect(comboMatchesEvent('shift+i', ev('I', { shiftKey: true }))).toBe(true); + expect(comboMatchesEvent('mod+shift+l', ev('L', { metaKey: true, shiftKey: true }))).toBe(true); + // A bare shift chord must not fire when the command modifier is held. + expect(comboMatchesEvent('shift+i', ev('I', { shiftKey: true, ctrlKey: true }))).toBe(false); + }); + + it('rejects plain chords carrying a command modifier', () => { + expect(comboMatchesEvent('shift+i', ev('I', { shiftKey: true }))).toBe(true); + expect(comboMatchesEvent('shift+i', ev('I', { shiftKey: true, metaKey: true }))).toBe(false); + }); + + it('is case-insensitive on the key', () => { + expect(comboMatchesEvent('mod+k', ev('K', { metaKey: true }))).toBe(true); + }); + + it('ignores Shift on punctuation keys (layouts where / requires Shift)', () => { + expect(comboMatchesEvent('mod+/', ev('/', { metaKey: true, shiftKey: true }))).toBe(true); + expect(comboMatchesEvent('mod+,', ev(',', { ctrlKey: true, shiftKey: true }))).toBe(true); + // Letters and named keys still enforce Shift absence. + expect(comboMatchesEvent('mod+k', ev('K', { metaKey: true, shiftKey: true }))).toBe(false); + }); +}); + +describe('matchesAction', () => { + it('matches every default binding', () => { + expect(matchesAction('open-palette', ev('k', { metaKey: true }))).toBe(true); + expect(matchesAction('toggle-theme', ev('l', { ctrlKey: true, shiftKey: true }))).toBe(true); + expect(matchesAction('open-settings', ev(',', { metaKey: true }))).toBe(true); + expect(matchesAction('cycle-thinking-level', ev('Tab', { shiftKey: true }))).toBe(true); + }); + + it('opens shortcuts help on layouts where / is a shifted key', () => { + expect(matchesAction('open-shortcuts-help', ev('/', { metaKey: true, shiftKey: true }))).toBe( + true, + ); + expect(matchesAction('open-shortcuts-help', ev('/', { metaKey: true }))).toBe(true); + }); + + it('honors alias bindings (Ctrl+L still opens the model selector)', () => { + expect(matchesAction('open-model-selector', ev('i', { ctrlKey: true }))).toBe(true); + expect(matchesAction('open-model-selector', ev('l', { ctrlKey: true }))).toBe(true); + }); + + it('matches plain-key toggles on the key alone', () => { + // Their handler owns the editable/modifier guard, so plain keys match + // regardless of modifiers — preserving the prior t/o/p behavior. + expect(matchesAction('toggle-thinking', ev('t'))).toBe(true); + expect(matchesAction('toggle-thinking', ev('T', { shiftKey: true }))).toBe(true); + }); + + it('enforces shift on nav chords', () => { + expect(matchesAction('scroll-down', ev('j'))).toBe(true); + expect(matchesAction('scroll-down', ev('k'))).toBe(false); + // focus-composer is shift+i: bare 'i' must not trigger it. + expect(matchesAction('focus-composer', ev('I', { shiftKey: true }))).toBe(true); + expect(matchesAction('focus-composer', ev('i'))).toBe(false); + expect(matchesAction('scroll-bottom', ev('G', { shiftKey: true }))).toBe(true); + }); + + it('applies an override binding when provided', () => { + const bindings = { 'open-palette': 'mod+p' }; + expect(matchesAction('open-palette', ev('p', { metaKey: true }), bindings)).toBe(true); + expect(matchesAction('open-palette', ev('k', { metaKey: true }), bindings)).toBe(false); + }); + + it('returns false for an unknown action', () => { + expect(matchesAction('nope', ev('k', { metaKey: true }))).toBe(false); + }); +}); + +describe('registry integrity', () => { + it('exposes a unique id and category for every action', () => { + const ids = KEY_ACTIONS.map((a) => a.id); + expect(new Set(ids).size).toBe(ids.length); + for (const a of KEY_ACTIONS) { + expect(a.category).toBeTruthy(); + expect(defaultCombo(a.id)).toBe(a.combo); + } + }); +}); diff --git a/web/src/shared/keyboard-nav.js b/web/src/shared/keyboard-nav.js index 63e96e90..32b1cff4 100644 --- a/web/src/shared/keyboard-nav.js +++ b/web/src/shared/keyboard-nav.js @@ -1,4 +1,5 @@ import { navigate } from './navigation.js'; +import { matchesAction } from './keybindings.js'; const SCROLL_AMOUNT = 300; const GG_TIMEOUT = 500; // ms window for double-tap 'gg' @@ -84,7 +85,7 @@ export function setupKeyboardNav({ // Cmd/Ctrl+, opens the global settings page (standard macOS preferences // shortcut). Works regardless of focus, like a native app. documentImpl.addEventListener('keydown', (e) => { - if ((e.metaKey || e.ctrlKey) && !e.shiftKey && !e.altKey && e.key === ',') { + if (matchesAction('open-settings', e)) { e.preventDefault(); navigate('/settings', { windowImpl }); } @@ -94,7 +95,7 @@ export function setupKeyboardNav({ if (e.metaKey || e.ctrlKey || e.altKey) return; if (isEditableTarget(documentImpl.activeElement)) return; - if (e.key === 'j') { + if (matchesAction('scroll-down', e)) { e.preventDefault(); const content = typeof documentImpl.getElementById === 'function' @@ -105,7 +106,7 @@ export function setupKeyboardNav({ } else { windowImpl.scrollBy({ top: SCROLL_AMOUNT, behavior: 'instant' }); } - } else if (e.key === 'k') { + } else if (matchesAction('scroll-up', e)) { e.preventDefault(); const content = typeof documentImpl.getElementById === 'function' @@ -137,7 +138,7 @@ export function setupKeyboardNav({ ggTimer = null; }, GG_TIMEOUT); } - } else if (e.key === 'G') { + } else if (matchesAction('scroll-bottom', e)) { e.preventDefault(); const content = typeof documentImpl.getElementById === 'function' @@ -151,7 +152,7 @@ export function setupKeyboardNav({ behavior: 'instant', }); } - } else if (e.key === 'I') { + } else if (matchesAction('focus-composer', e)) { e.preventDefault(); const el = documentImpl.querySelector(focusSelector); if (el) el.focus(); diff --git a/web/src/shared/keyboard-nav.test.js b/web/src/shared/keyboard-nav.test.js index 84a8bbd5..5694ad29 100644 --- a/web/src/shared/keyboard-nav.test.js +++ b/web/src/shared/keyboard-nav.test.js @@ -209,14 +209,19 @@ describe('setupKeyboardNav', () => { expect(win.history.pushState).toHaveBeenCalledWith({}, '', '/settings'); }); - it('does not navigate to /settings on Cmd+Shift+,', () => { + it('tolerates Shift on layouts where "," is a shifted key', () => { const doc = createMockDocument(); const win = createMockWindow(); setupKeyboardNav({ windowImpl: win, documentImpl: doc }); - doc._dispatch('keydown', { key: ',', metaKey: true, shiftKey: true }); + // On a US layout Cmd+Shift+, produces '<' — must not navigate. + doc._dispatch('keydown', { key: '<', metaKey: true, shiftKey: true }); expect(win.history.pushState).not.toHaveBeenCalled(); + + // On layouts where typing ',' itself requires Shift, the chord works. + doc._dispatch('keydown', { key: ',', metaKey: true, shiftKey: true }); + expect(win.history.pushState).toHaveBeenCalledWith({}, '', '/settings'); }); it('scrolls down on j', () => {