Skip to content

Commit e40bcc2

Browse files
committed
Centralize keyboard shortcut dispatch
- New shortcut-dispatch.ts: reverse lookup map from key combo → command ID - handleGlobalKeyDown now routes Tier 1 shortcuts through handleCommandExecute - Removed duplicate F-key handlers from DualPaneExplorer and FilePane - Removed legacy isCommandPaletteShortcut/isSettingsShortcut predicates - Added app.settings command to registry - Custom shortcut bindings now work at runtime (not just display)
1 parent 6333d68 commit e40bcc2

9 files changed

Lines changed: 544 additions & 87 deletions

File tree

apps/desktop/src/lib/commands/CLAUDE.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,8 @@ The uFuzzy instance is a module-level singleton (created once at import time).
7070

7171
1. Add an entry to the `commands` array in `command-registry.ts`.
7272
2. Add a `case` for its `id` in the `handleCommandExecute` switch in `routes/(main)/+page.svelte`.
73-
3. No changes needed to the palette, fuzzy search, or types.
73+
3. No changes needed to the palette, fuzzy search, types, or keyboard dispatch. Commands with `showInPalette: true` are
74+
automatically dispatched from keyboard shortcuts via centralized dispatch (`../shortcuts/shortcut-dispatch.ts`).
7475

7576
## Dependencies
7677

apps/desktop/src/lib/commands/command-registry.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ export const commands: Command[] = [
2626
showInPalette: false, // Don't show the palette in itself
2727
shortcuts: ['⌘⇧P'],
2828
},
29+
{ id: 'app.settings', name: 'Open settings', scope: 'App', showInPalette: true, shortcuts: ['⌘,'] },
2930

3031
// ============================================================================
3132
// Main window - View commands

apps/desktop/src/lib/file-explorer/pane/DualPaneExplorer.svelte

Lines changed: 8 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -602,12 +602,6 @@
602602
return false
603603
}
604604
605-
function handleTabKey() {
606-
const newFocus = focusedPane === 'left' ? 'right' : 'left'
607-
focusedPane = newFocus
608-
saveAppStatus({ focusedPane: newFocus })
609-
}
610-
611605
function handleEscapeDuringLoading(): boolean {
612606
const paneRef = getPaneRef(focusedPane)
613607
// eslint-disable-next-line @typescript-eslint/no-unsafe-call
@@ -619,57 +613,19 @@
619613
return false
620614
}
621615
622-
/** Handles function key shortcuts (F1-F8). Returns true if a function key was handled. */
616+
/** Handles the F1 key (volume chooser toggle). Returns true if handled. */
623617
function handleFunctionKey(e: KeyboardEvent): boolean {
624-
switch (e.key) {
625-
case 'F1':
626-
// eslint-disable-next-line @typescript-eslint/no-unsafe-call
627-
getPaneRef('right')?.closeVolumeChooser()
628-
// eslint-disable-next-line @typescript-eslint/no-unsafe-call
629-
getPaneRef('left')?.toggleVolumeChooser()
630-
return true
631-
case 'F2':
632-
startRename()
633-
return true
634-
case 'F3':
635-
void openViewerForCursor()
636-
return true
637-
case 'F5':
638-
void openTransferDialog('copy')
639-
return true
640-
case 'F6':
641-
if (e.shiftKey) {
642-
// Shift+F6 = Rename
643-
// eslint-disable-next-line @typescript-eslint/no-unsafe-call
644-
getPaneRef(focusedPane)?.startRename()
645-
} else {
646-
void openTransferDialog('move')
647-
}
648-
return true
649-
case 'F7':
650-
void openNewFolderDialog()
651-
return true
652-
case 'F8':
653-
void openDeleteDialog(e.shiftKey)
654-
return true
655-
default:
656-
return false
618+
if (e.key === 'F1') {
619+
// eslint-disable-next-line @typescript-eslint/no-unsafe-call
620+
getPaneRef('right')?.closeVolumeChooser()
621+
// eslint-disable-next-line @typescript-eslint/no-unsafe-call
622+
getPaneRef('left')?.toggleVolumeChooser()
623+
return true
657624
}
625+
return false
658626
}
659627
660628
function handleKeyDown(e: KeyboardEvent) {
661-
if (e.key === 'Tab' && e.ctrlKey) {
662-
e.preventDefault()
663-
cycleTab(e.shiftKey ? 'prev' : 'next')
664-
return
665-
}
666-
667-
if (e.key === 'Tab') {
668-
e.preventDefault()
669-
handleTabKey()
670-
return
671-
}
672-
673629
// ESC during loading = cancel and go back
674630
if (e.key === 'Escape' && handleEscapeDuringLoading()) {
675631
e.preventDefault()

apps/desktop/src/lib/file-explorer/pane/FilePane.svelte

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@
3030
listen,
3131
onMtpDeviceRemoved,
3232
openFile,
33-
openInEditor,
3433
showFileContextMenu,
3534
type UnlistenFn,
3635
updateMenuContext,
@@ -1113,16 +1112,6 @@
11131112
}
11141113
}
11151114
1116-
// Handle F4 key - open file in default text editor
1117-
if (e.key === 'F4') {
1118-
const entry = getEntryUnderCursor()
1119-
if (entry && !entry.isDirectory) {
1120-
e.preventDefault()
1121-
void openInEditor(entry.path)
1122-
return
1123-
}
1124-
}
1125-
11261115
// Handle Backspace or ⌘↑ - go to parent directory
11271116
if ((e.key === 'Backspace' || (e.key === 'ArrowUp' && e.metaKey)) && hasParent) {
11281117
e.preventDefault()

apps/desktop/src/lib/shortcuts/CLAUDE.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,16 @@ No normalization—shortcuts are stored exactly as displayed.
6666
Main window listens for MCP events to modify shortcuts even when settings window is closed. This allows AI agents to
6767
customize shortcuts on the fly.
6868

69+
### Centralized dispatch (`shortcut-dispatch.ts`)
70+
71+
Builds a reverse lookup `Map<shortcutString, commandId>` for Tier 1 commands (those with `showInPalette: true` plus
72+
`app.commandPalette`). On every keypress, `handleGlobalKeyDown()` in `+page.svelte` calls `formatKeyCombo(e)` and
73+
`lookupCommand()` to find a matching command, then routes through `handleCommandExecute()` -- the same path used by the
74+
command palette and MCP events. Rebuilds automatically when custom shortcuts change via `onShortcutChange`.
75+
76+
Tier 2 commands (arrows, Space, Enter, Backspace, etc.) are not in the dispatch map. Unmatched keypresses propagate
77+
normally to component-level handlers in DualPaneExplorer and FilePane.
78+
6979
## Key decisions
7080

7181
### Why platform-specific storage?
Lines changed: 168 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,168 @@
1+
import { describe, it, expect, beforeEach, vi } from 'vitest'
2+
3+
// Shared test state — the mock factory closures capture these references
4+
const listeners = new Set<(commandId: string) => void>()
5+
const customOverrides = new Map<string, string[]>()
6+
7+
// Mock the shortcuts store before importing the module under test
8+
vi.mock('./shortcuts-store', () => ({
9+
getEffectiveShortcuts: vi.fn(),
10+
onShortcutChange: vi.fn((listener: (commandId: string) => void) => {
11+
listeners.add(listener)
12+
return () => listeners.delete(listener)
13+
}),
14+
}))
15+
16+
// Mock the command registry with a controlled set of commands
17+
vi.mock('$lib/commands/command-registry', () => ({
18+
commands: [
19+
// Tier 1: showInPalette true, has shortcut
20+
{ id: 'app.quit', name: 'Quit', scope: 'App', showInPalette: true, shortcuts: ['⌘Q'] },
21+
{
22+
id: 'file.rename',
23+
name: 'Rename',
24+
scope: 'Main window/File list',
25+
showInPalette: true,
26+
shortcuts: ['F2', '⇧F6'],
27+
},
28+
{ id: 'file.copy', name: 'Copy', scope: 'Main window/File list', showInPalette: true, shortcuts: ['F5'] },
29+
{ id: 'view.showHidden', name: 'Toggle hidden', scope: 'Main window', showInPalette: true, shortcuts: ['⌘⇧.'] },
30+
// Tier 1: showInPalette false but in ALWAYS_DISPATCH_IDS
31+
{
32+
id: 'app.commandPalette',
33+
name: 'Open command palette',
34+
scope: 'App',
35+
showInPalette: false,
36+
shortcuts: ['⌘⇧P'],
37+
},
38+
// Tier 2: showInPalette false, basic nav — should NOT be in dispatch
39+
{
40+
id: 'nav.up',
41+
name: 'Select previous',
42+
scope: 'Main window/File list',
43+
showInPalette: false,
44+
shortcuts: ['↑'],
45+
},
46+
{ id: 'nav.down', name: 'Select next', scope: 'Main window/File list', showInPalette: false, shortcuts: ['↓'] },
47+
// Tier 2: palette-internal
48+
{
49+
id: 'palette.close',
50+
name: 'Close palette',
51+
scope: 'Command palette',
52+
showInPalette: false,
53+
shortcuts: ['Escape'],
54+
},
55+
],
56+
}))
57+
58+
import { getEffectiveShortcuts, onShortcutChange } from './shortcuts-store'
59+
import { commands } from '$lib/commands/command-registry'
60+
import { lookupCommand, initShortcutDispatch, destroyShortcutDispatch } from './shortcut-dispatch'
61+
62+
/**
63+
* Wire up getEffectiveShortcuts to return registry defaults
64+
* unless a custom override exists.
65+
*/
66+
function setupEffectiveShortcuts() {
67+
vi.mocked(getEffectiveShortcuts).mockImplementation((commandId: string) => {
68+
const override = customOverrides.get(commandId)
69+
if (override) {
70+
return [...override]
71+
}
72+
const cmd = commands.find((c) => c.id === commandId)
73+
return [...(cmd?.shortcuts ?? [])]
74+
})
75+
}
76+
77+
describe('shortcut-dispatch', () => {
78+
beforeEach(() => {
79+
destroyShortcutDispatch()
80+
customOverrides.clear()
81+
listeners.clear()
82+
vi.clearAllMocks()
83+
setupEffectiveShortcuts()
84+
})
85+
86+
describe('lookupCommand', () => {
87+
it('returns the correct command ID for a Tier 1 shortcut', () => {
88+
initShortcutDispatch()
89+
expect(lookupCommand('⌘Q')).toBe('app.quit')
90+
})
91+
92+
it('handles commands with multiple shortcuts', () => {
93+
initShortcutDispatch()
94+
expect(lookupCommand('F2')).toBe('file.rename')
95+
expect(lookupCommand('⇧F6')).toBe('file.rename')
96+
})
97+
98+
it('returns undefined for unregistered key combos', () => {
99+
initShortcutDispatch()
100+
expect(lookupCommand('⌘Z')).toBeUndefined()
101+
expect(lookupCommand('F12')).toBeUndefined()
102+
})
103+
104+
it('returns undefined for Tier 2 (non-palette) command shortcuts', () => {
105+
initShortcutDispatch()
106+
// nav.up (↑) and nav.down (↓) have showInPalette: false
107+
expect(lookupCommand('↑')).toBeUndefined()
108+
expect(lookupCommand('↓')).toBeUndefined()
109+
})
110+
111+
it('includes app.commandPalette despite showInPalette: false', () => {
112+
initShortcutDispatch()
113+
expect(lookupCommand('⌘⇧P')).toBe('app.commandPalette')
114+
})
115+
116+
it('returns undefined before init is called', () => {
117+
// Map is empty before init
118+
expect(lookupCommand('⌘Q')).toBeUndefined()
119+
})
120+
})
121+
122+
describe('custom shortcut overrides', () => {
123+
it('uses the new binding after a custom override', () => {
124+
initShortcutDispatch()
125+
126+
// Override file.copy from F5 to F9
127+
customOverrides.set('file.copy', ['F9'])
128+
129+
// Trigger the change listener
130+
for (const listener of listeners) {
131+
listener('file.copy')
132+
}
133+
134+
expect(lookupCommand('F9')).toBe('file.copy')
135+
expect(lookupCommand('F5')).toBeUndefined()
136+
})
137+
138+
it('handles adding a shortcut to a command that had none', () => {
139+
initShortcutDispatch()
140+
141+
customOverrides.set('view.showHidden', ['⌘⇧.', '⌘⇧H'])
142+
143+
for (const listener of listeners) {
144+
listener('view.showHidden')
145+
}
146+
147+
expect(lookupCommand('⌘⇧H')).toBe('view.showHidden')
148+
expect(lookupCommand('⌘⇧.')).toBe('view.showHidden')
149+
})
150+
})
151+
152+
describe('initShortcutDispatch', () => {
153+
it('subscribes to shortcut changes', () => {
154+
initShortcutDispatch()
155+
expect(onShortcutChange).toHaveBeenCalledOnce()
156+
})
157+
})
158+
159+
describe('destroyShortcutDispatch', () => {
160+
it('clears the map after destroy', () => {
161+
initShortcutDispatch()
162+
expect(lookupCommand('⌘Q')).toBe('app.quit')
163+
164+
destroyShortcutDispatch()
165+
expect(lookupCommand('⌘Q')).toBeUndefined()
166+
})
167+
})
168+
})
Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
/**
2+
* Centralized shortcut dispatch — reverse lookup from shortcut strings to command IDs.
3+
*
4+
* Builds a Map<shortcutString, commandId> for Tier 1 commands (those eligible for
5+
* central keyboard dispatch). Rebuilds automatically when custom shortcuts change.
6+
*/
7+
8+
import { commands } from '$lib/commands/command-registry'
9+
import { getEffectiveShortcuts, onShortcutChange } from './shortcuts-store'
10+
11+
// Command IDs that have showInPalette: false but still need central dispatch
12+
const ALWAYS_DISPATCH_IDS = new Set(['app.commandPalette'])
13+
14+
let shortcutMap = new Map<string, string>()
15+
let unsubscribe: (() => void) | null = null
16+
17+
/**
18+
* Check whether a command is Tier 1 (centrally dispatched).
19+
* Tier 1 = showInPalette OR in the always-dispatch list.
20+
*/
21+
function isTier1(command: { id: string; showInPalette: boolean }): boolean {
22+
return command.showInPalette || ALWAYS_DISPATCH_IDS.has(command.id)
23+
}
24+
25+
/** Build the reverse lookup map from scratch. */
26+
function buildShortcutMap(): Map<string, string> {
27+
const map = new Map<string, string>()
28+
29+
for (const command of commands) {
30+
if (!isTier1(command)) continue
31+
32+
const shortcuts = getEffectiveShortcuts(command.id)
33+
for (const shortcut of shortcuts) {
34+
// First match wins — skip if shortcut already claimed
35+
if (!map.has(shortcut)) {
36+
map.set(shortcut, command.id)
37+
}
38+
}
39+
}
40+
41+
return map
42+
}
43+
44+
/** Look up which command ID a shortcut string maps to, if any. */
45+
export function lookupCommand(shortcutString: string): string | undefined {
46+
return shortcutMap.get(shortcutString)
47+
}
48+
49+
/** Initialize the dispatch map and subscribe to shortcut changes. */
50+
export function initShortcutDispatch(): void {
51+
shortcutMap = buildShortcutMap()
52+
unsubscribe = onShortcutChange(() => {
53+
shortcutMap = buildShortcutMap()
54+
})
55+
}
56+
57+
/** Tear down: unsubscribe from shortcut changes and clear the map. */
58+
export function destroyShortcutDispatch(): void {
59+
unsubscribe?.()
60+
unsubscribe = null
61+
shortcutMap = new Map()
62+
}

0 commit comments

Comments
 (0)