From 56a554402f559db20dc7c5078e85b64b1d4191de Mon Sep 17 00:00:00 2001 From: setkyar Date: Mon, 17 Aug 2026 13:17:57 +0700 Subject: [PATCH 1/3] refactor(web): route keyboard shortcuts through a central registry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Introduce web/src/shared/keybindings.js: a single source of truth for the remappable global/navigation/composer shortcuts, plus matcher helpers. Migrate the five inline handlers (keyboard-nav, session-globals, SessionsPage index, search-filters, textarea-controls) to ask the registry `matchesAction(id, e)` instead of hardcoding `e.key === …` / modifier checks. Pure refactor — defaults are unchanged and no override loading exists yet, so behavior is identical. All 722 frontend tests pass with no changes to the existing handler tests, confirming parity. Structural modal keys (Escape, arrows, Tab focus-traps), Enter-to-submit, and the multi-key `g g` sequence are intentionally left hardcoded; they are UI affordances, not preferences. Groundwork for #43 (customizable shortcuts). The settings UI, override persistence, conflict detection, and modal reflection land in a follow-up PR stacked on this one. --- .../session/chat/textarea-controls.js | 6 +- web/src/routes/SessionsPage.svelte | 5 +- web/src/session/session-globals.js | 13 +- web/src/session/ui/search-filters.js | 9 +- web/src/shared/keybindings.js | 125 ++++++++++++++++ web/src/shared/keybindings.test.js | 133 ++++++++++++++++++ web/src/shared/keyboard-nav.js | 11 +- 7 files changed, 283 insertions(+), 19 deletions(-) create mode 100644 web/src/shared/keybindings.js create mode 100644 web/src/shared/keybindings.test.js 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..749c3c44 --- /dev/null +++ b/web/src/shared/keybindings.js @@ -0,0 +1,125 @@ +// 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; + } + if (want.shift !== Boolean(event.shiftKey)) 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..a44614e3 --- /dev/null +++ b/web/src/shared/keybindings.test.js @@ -0,0 +1,133 @@ +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); + }); +}); + +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('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(); From 9dc6a1264f4cd19ad93ee3ea2a6607a0ddc12e20 Mon Sep 17 00:00:00 2001 From: setkyar Date: Tue, 18 Aug 2026 12:46:51 +0700 Subject: [PATCH 2/3] fix(web): don't require Shift absence for punctuation keybindings On layouts where '/' is a shifted key (German, French, ...), Cmd+/ arrives with shiftKey=true and the strict modifier check made the shortcuts-help binding unreachable. Shift is now only enforced where it changes meaning: letters and named keys like Tab. For punctuation, event.key is already the shifted result, so an unrequested Shift is layout noise. --- web/src/shared/keybindings.js | 10 +++++++++- web/src/shared/keybindings.test.js | 14 ++++++++++++++ web/src/shared/keyboard-nav.test.js | 9 +++++++-- 3 files changed, 30 insertions(+), 3 deletions(-) diff --git a/web/src/shared/keybindings.js b/web/src/shared/keybindings.js index 749c3c44..30033ab0 100644 --- a/web/src/shared/keybindings.js +++ b/web/src/shared/keybindings.js @@ -101,7 +101,15 @@ export function comboMatchesEvent(combo, event) { // Plain chords (no command modifier requested) must not carry ⌘/Ctrl/Alt. return false; } - if (want.shift !== Boolean(event.shiftKey)) 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; } diff --git a/web/src/shared/keybindings.test.js b/web/src/shared/keybindings.test.js index a44614e3..d5ff69a0 100644 --- a/web/src/shared/keybindings.test.js +++ b/web/src/shared/keybindings.test.js @@ -79,6 +79,13 @@ describe('comboMatchesEvent', () => { 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', () => { @@ -89,6 +96,13 @@ describe('matchesAction', () => { 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); 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', () => { From b9be10e2211d6a3ee3960db765aa667cde385bd2 Mon Sep 17 00:00:00 2001 From: setkyar Date: Tue, 18 Aug 2026 13:16:58 +0700 Subject: [PATCH 3/3] fix(web): apply Manage Projects filter to the session sidebar GET /api/projects now accepts filtered=1, which applies the enabled-projects allowlist server-side (so pagination and totals stay correct) while always keeping the current session's project visible. The sidebar Projects tab and the Sessions tab's project switcher pass the flag; the Manage Projects modal keeps fetching the full list. Fixes #107 --- docs/architecture/backend.md | 2 +- docs/architecture/system-overview.md | 5 ++ internal/server/projects.go | 13 ++++ internal/server/projects_test.go | 59 +++++++++++++++++++ .../session/SessionSidebarProjects.svelte | 1 + .../session/SessionSidebarProjects.test.js | 2 + .../session/SessionSidebarSessions.svelte | 2 +- web/src/index/sessions.js | 9 ++- 8 files changed, 90 insertions(+), 3 deletions(-) diff --git a/docs/architecture/backend.md b/docs/architecture/backend.md index 6815c5c8..9c756f48 100644 --- a/docs/architecture/backend.md +++ b/docs/architecture/backend.md @@ -281,7 +281,7 @@ type piRPCWorker struct { | `/api/settings` | GET/POST | `handleGetSettings` / `handleSaveSettings` | Server-backed user settings (SQLite) | | `/api/btw` | GET | `handleGetBtw` | Resolve the btw scratch-chat session for a parent (SQLite) | | `/api/btw/new` | POST | `handleNewBtw` | Create a new btw scratch-chat session (SQLite) | -| `/api/projects` | GET/POST | `handleApiProjects` / `handleUpdateProject` | List projects + filter state (`limit`/`offset`, optional `current` priority + `sessionLimit` bundled summaries, active session IDs per project); enable/disable/register/remove, bulk enable-all/disable-all, enable-filter/disable-filter (SQLite) | +| `/api/projects` | GET/POST | `handleApiProjects` / `handleUpdateProject` | List projects + filter state (`limit`/`offset`, optional `current` priority + `sessionLimit` bundled summaries, active session IDs per project, `filtered=1` to apply the enabled-projects allowlist with the current project always kept); enable/disable/register/remove, bulk enable-all/disable-all, enable-filter/disable-filter (SQLite) | | `/api/sounds` | GET | `handleApiSounds` | List available notification sounds | | `/sounds/` | GET | `handleSounds` | Serve a sound asset (no auth) | | `/custom-themes.css` | GET | `handleCustomThemes` | User custom theme CSS | diff --git a/docs/architecture/system-overview.md b/docs/architecture/system-overview.md index f58e412b..91a9b646 100644 --- a/docs/architecture/system-overview.md +++ b/docs/architecture/system-overview.md @@ -167,6 +167,11 @@ across devices. See `internal/server/projects.go`. (no client flash) and is a no-op while the master switch is off. Manage via the index menu → **Manage Projects** (search, select/deselect-all, register, and the filter switch), backed by `GET/POST /api/projects`. +- The session sidebar (Projects tab and the Sessions tab's project switcher) + requests `GET /api/projects?filtered=1`, which applies the same allowlist + server-side — except the current session's project, which is always included so + the project you are in never disappears. The Manage Projects modal omits the + param and keeps seeing every project. ## Startup Order diff --git a/internal/server/projects.go b/internal/server/projects.go index 4102f019..3699d620 100644 --- a/internal/server/projects.go +++ b/internal/server/projects.go @@ -251,6 +251,19 @@ func (s *Server) handleApiProjects(w http.ResponseWriter, r *http.Request) { RunningSessionIDs: runningByProject[p], }) } + // filtered=1 applies the Manage Projects allowlist (used by the session + // sidebar). The current project is always kept so the project you are in + // never disappears; the modal omits the param to keep seeing everything. + if q.Get("filtered") == "1" && s.projectFilterEnabled() { + kept := make([]projectEntry, 0, len(entries)) + for _, entry := range entries { + if entry.Enabled || entry.Path == currentProject { + kept = append(kept, entry) + } + } + entries = kept + } + sort.Slice(entries, func(i, j int) bool { if (entries[i].Path == currentProject) != (entries[j].Path == currentProject) { return entries[i].Path == currentProject diff --git a/internal/server/projects_test.go b/internal/server/projects_test.go index 44ec118e..0809ba42 100644 --- a/internal/server/projects_test.go +++ b/internal/server/projects_test.go @@ -302,6 +302,65 @@ func TestHandleApiProjects(t *testing.T) { } } +func TestHandleApiProjectsFiltered(t *testing.T) { + sessionsDir := t.TempDir() + writeSessionWithCWD(t, filepath.Join(sessionsDir, "sub1"), "a.jsonl", "/home/user/project-a") + writeSessionWithCWD(t, filepath.Join(sessionsDir, "sub2"), "b.jsonl", "/home/user/project-b") + + s := &Server{db: newProjectPrefsDB(t), sessionsDir: sessionsDir, cache: sessions.NewCache(), now: time.Now} + // Seed both projects, then disable project-b. + s.syncProjectPrefs([]string{"/home/user/project-a", "/home/user/project-b"}) + if _, err := s.db.Exec("UPDATE project_prefs SET enabled = 0 WHERE project_path = ?", "/home/user/project-b"); err != nil { + t.Fatal(err) + } + + getPaths := func(url string) ([]string, int) { + t.Helper() + req := httptest.NewRequest(http.MethodGet, url, nil) + w := httptest.NewRecorder() + s.handleApiProjects(w, req) + if w.Code != http.StatusOK { + t.Fatalf("status = %d", w.Code) + } + var payload struct { + Projects []projectEntry `json:"projects"` + Total int `json:"total"` + } + if err := json.Unmarshal(w.Body.Bytes(), &payload); err != nil { + t.Fatal(err) + } + paths := make([]string, 0, len(payload.Projects)) + for _, p := range payload.Projects { + paths = append(paths, p.Path) + } + return paths, payload.Total + } + + // Master filter off: filtered=1 is a no-op. + if paths, total := getPaths("/api/projects?filtered=1"); len(paths) != 2 || total != 2 { + t.Fatalf("filter off: got %v (total %d), want both projects", paths, total) + } + + s.setProjectFilterEnabled(true) + + // Without filtered=1 (Manage Projects modal) everything still shows. + if paths, total := getPaths("/api/projects"); len(paths) != 2 || total != 2 { + t.Fatalf("no filtered param: got %v (total %d), want both projects", paths, total) + } + + // filtered=1 drops the disabled project and total reflects it. + paths, total := getPaths("/api/projects?filtered=1") + if len(paths) != 1 || paths[0] != "/home/user/project-a" || total != 1 { + t.Fatalf("filtered: got %v (total %d), want only project-a", paths, total) + } + + // The current project is kept even when disabled. + paths, total = getPaths("/api/projects?filtered=1¤t=/home/user/project-b") + if len(paths) != 2 || total != 2 || paths[0] != "/home/user/project-b" { + t.Fatalf("filtered current: got %v (total %d), want project-b first", paths, total) + } +} + func TestHandleApiProjectsPagination(t *testing.T) { sessionsDir := t.TempDir() for i := range 25 { diff --git a/web/src/components/session/SessionSidebarProjects.svelte b/web/src/components/session/SessionSidebarProjects.svelte index d927c830..38f325d8 100644 --- a/web/src/components/session/SessionSidebarProjects.svelte +++ b/web/src/components/session/SessionSidebarProjects.svelte @@ -122,6 +122,7 @@ offset: projects.length, currentProject: cwd, currentSessionLimit: sessionPageSize, + filtered: true, }); if (destroyed) return; const knownProjectPaths = new Set(projects.map((project) => project.path)); diff --git a/web/src/components/session/SessionSidebarProjects.test.js b/web/src/components/session/SessionSidebarProjects.test.js index 392104ce..70fdd06a 100644 --- a/web/src/components/session/SessionSidebarProjects.test.js +++ b/web/src/components/session/SessionSidebarProjects.test.js @@ -58,6 +58,7 @@ describe('SessionSidebarProjects', () => { offset: 0, currentProject: '/repo/pi-web', currentSessionLimit: 5, + filtered: true, }); expect(fetchSessions).toHaveBeenCalledWith({ project: '/repo/pi-web', @@ -190,6 +191,7 @@ describe('SessionSidebarProjects', () => { offset: 20, currentProject: '/repo/current', currentSessionLimit: 5, + filtered: true, }); }); diff --git a/web/src/components/session/SessionSidebarSessions.svelte b/web/src/components/session/SessionSidebarSessions.svelte index 9022ea8b..99e472eb 100644 --- a/web/src/components/session/SessionSidebarSessions.svelte +++ b/web/src/components/session/SessionSidebarSessions.svelte @@ -131,7 +131,7 @@ await tick(); projectSearchEl?.focus(); try { - const response = await fetchProjects(); + const response = await fetchProjects({ filtered: true }); projects = Array.isArray(response.projects) ? response.projects : []; } catch (err) { projects = []; diff --git a/web/src/index/sessions.js b/web/src/index/sessions.js index 6ae3c418..f41149ee 100644 --- a/web/src/index/sessions.js +++ b/web/src/index/sessions.js @@ -136,7 +136,13 @@ export function defaultFetchRecent() { export function defaultCreateSession(path) { return postJSON('/api/new-session', { path }); } -export function defaultFetchProjects({ limit, offset, currentProject, currentSessionLimit } = {}) { +export function defaultFetchProjects({ + limit, + offset, + currentProject, + currentSessionLimit, + filtered, +} = {}) { const params = new URLSearchParams(); if (Number.isFinite(limit) && limit > 0) params.set('limit', String(limit)); if (Number.isFinite(offset) && offset > 0) params.set('offset', String(offset)); @@ -144,6 +150,7 @@ export function defaultFetchProjects({ limit, offset, currentProject, currentSes if (Number.isFinite(currentSessionLimit) && currentSessionLimit > 0) { params.set('sessionLimit', String(currentSessionLimit)); } + if (filtered) params.set('filtered', '1'); const qs = params.toString(); return getJSON('/api/projects' + (qs ? '?' + qs : '')); }