Skip to content

Commit 87df2ed

Browse files
committed
Command palette shows your real shortcuts, up to three
- Palette rows now read live effective bindings via `getEffectiveShortcutsReactive(command.id)` instead of the registry defaults in `command.shortcuts`, fixing the long-standing bug where a rebound command showed a combo that no longer worked. - Each combo renders as a non-clickable, dense `ShortcutChip` (the row already owns the click); cap raised from two to three so power users can discover alternates like `⌘3` / `⌘F3`. - Open palette stays in sync: a Settings/MCP rebind updates the rendered row live (reactive store subscription). - Added a `size="sm"` variant to `ShortcutChip` (tighter padding + radius) for dense rows; first consumer is the palette. Catalog section + docs updated. - On the accent-subtle cursor row, chip background lifts to `--color-bg-secondary` so chips stay legible against the tint in both themes. - Removed the dead `formatShortcuts` helper (only used here). - Tests: palette now pins effective-over-default, the three-cap, and live reactivity on rebind.
1 parent e756a37 commit 87df2ed

7 files changed

Lines changed: 165 additions & 26 deletions

File tree

apps/desktop/src/lib/command-palette/CLAUDE.md

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,13 @@ scrollable-region rule has nothing to flag in the empty state, and `aria-expande
5858
**Fuzzy highlight rendering**: `highlightMatches()` converts the flat `matchedIndices` set into `{ text, highlighted }`
5959
segments and renders `<mark class="match-highlight">` for matched chars.
6060

61-
**Shortcut display**: `formatShortcuts()` calls `.slice(0, 2)`; at most 2 shortcuts are shown per command.
61+
**Shortcut display**: each row reads `getEffectiveShortcutsReactive(command.id)` (live effective bindings, NOT the
62+
registry defaults in `command.shortcuts`), caps at three (`MAX_SHORTCUTS_SHOWN`), and renders each combo as a
63+
non-clickable, dense (`size="sm"`) `ShortcutChip`. The chips track rebinds live: `getEffectiveShortcutsReactive`
64+
subscribes the component to the shortcuts-store version, so a Settings/MCP rebind updates the open palette without a
65+
reopen. Chips are non-clickable because the whole row already owns the click (decision (a) in
66+
`docs/specs/shortcut-display-unification-plan.md`). On the accent-subtle cursor row, the chip background is lifted to
67+
`--color-bg-secondary` so it stays legible against the tint (both themes).
6268

6369
**jsdom limitation**: `scrollIntoView` is not implemented in jsdom; tests mock it via
6470
`Element.prototype.scrollIntoView = vi.fn()`.
@@ -97,9 +103,13 @@ enough for ~60 commands that debouncing would only add latency. The `$derived` r
97103
synchronously on every keystroke, keeping results perfectly in sync with the input. Debouncing would be needed if the
98104
command list grew to thousands.
99105

100-
**Decision**: `formatShortcuts()` caps display at 2 shortcuts via `.slice(0, 2)`. **Why**: Some commands have many
101-
shortcuts (e.g., `nav.parent` has `Backspace` and `Cmd+Up`). Showing all of them would crowd the result row. Two is
102-
enough for discoverability; the full list lives in the shortcut settings.
106+
**Decision**: Shortcut display caps at three (`MAX_SHORTCUTS_SHOWN`) and reads the live effective bindings via
107+
`getEffectiveShortcutsReactive`, not the registry defaults. **Why**: Some commands have many shortcuts (for example,
108+
`nav.parent` has `Backspace` and `⌘↑`); showing all crowds the row. Three is the product-decided cap (power users use
109+
the palette to discover alternates like `⌘3` / `⌘F3`) — the dense `size="sm"` chip keeps three legible without dropping
110+
the count. Reading the effective bindings (instead of `command.shortcuts`) fixes the long-standing bug where a rebound
111+
command showed a combo that no longer worked; the reactive read also keeps an open palette in sync with a mid-session
112+
rebind. The full list still lives in the shortcut settings.
103113

104114
## Gotchas
105115

@@ -128,4 +138,6 @@ Add the command to `$lib/commands/command-registry.ts` and handle the ID in the
128138

129139
- `$lib/commands`: `searchCommands`, `getPaletteCommands`, `CommandMatch`
130140
- `$lib/app-status-store`: `pruneRecentCommands`, `pushRecentCommand`
141+
- `$lib/shortcuts/reactive-shortcuts.svelte`: `getEffectiveShortcutsReactive` (live effective bindings per row)
142+
- `$lib/ui/ShortcutChip.svelte`: renders each combo (literal `key` mode, `size="sm"`, non-clickable)
131143
- CSS variables from `app.css` (`--z-modal`, `--color-accent-subtle`, `--color-bg-secondary`, etc.)

apps/desktop/src/lib/command-palette/CommandPalette.svelte

Lines changed: 20 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,11 @@
1111
import { onDestroy, onMount, tick } from 'svelte'
1212
import { searchCommands, getPaletteCommands, type CommandMatch, type CommandId } from '$lib/commands'
1313
import { pruneRecentCommands, pushRecentCommand } from '$lib/app-status-store'
14+
import { getEffectiveShortcutsReactive } from '$lib/shortcuts/reactive-shortcuts.svelte'
15+
import ShortcutChip from '$lib/ui/ShortcutChip.svelte'
16+
17+
/** How many shortcut chips a palette row shows (power users discover alternates). */
18+
const MAX_SHORTCUTS_SHOWN = 3
1419
1520
interface Props {
1621
/** Called when user selects a command */
@@ -167,11 +172,6 @@
167172
168173
return segments
169174
}
170-
171-
/** Format shortcuts for display */
172-
function formatShortcuts(shortcuts: string[]): string {
173-
return shortcuts.slice(0, 2).join(' / ')
174-
}
175175
</script>
176176

177177
<div
@@ -214,6 +214,7 @@
214214
tabindex="-1"
215215
>
216216
{#each results as match, index (match.command.id)}
217+
{@const shortcuts = getEffectiveShortcutsReactive(match.command.id).slice(0, MAX_SHORTCUTS_SHOWN)}
217218
{#if recentCount > 0 && index === 0}
218219
<div class="group-heading">Recent</div>
219220
{/if}
@@ -247,8 +248,14 @@
247248
{/if}
248249
{/each}
249250
</span>
250-
{#if match.command.shortcuts.length > 0}
251-
<span class="shortcuts">{formatShortcuts(match.command.shortcuts)}</span>
251+
{#if shortcuts.length > 0}
252+
<span class="shortcuts">
253+
<!-- Key by index, not the combo string: a user can bind the same
254+
combo twice, and a keyed-by-value each would throw on the dupe. -->
255+
{#each shortcuts as shortcut, i (i)}
256+
<ShortcutChip key={shortcut} size="sm" clickable={false} />
257+
{/each}
258+
</span>
252259
{/if}
253260
</div>
254261
{/each}
@@ -351,8 +358,10 @@
351358
background: var(--color-accent-subtle);
352359
}
353360
354-
.result-item.is-under-cursor .shortcuts {
355-
color: var(--color-text-secondary);
361+
/* On the accent-subtle cursor row, lift the chips off the tinted background so
362+
they stay legible (the default tertiary bg blends into the highlight). */
363+
.result-item.is-under-cursor :global(.shortcut-chip) {
364+
background: var(--color-bg-secondary);
356365
}
357366
358367
.command-name {
@@ -378,9 +387,9 @@
378387
}
379388
380389
.shortcuts {
390+
display: flex;
391+
gap: var(--spacing-xs);
381392
margin-left: var(--spacing-lg);
382-
font-size: var(--font-size-sm);
383-
color: var(--color-text-tertiary);
384393
flex-shrink: 0;
385394
}
386395

apps/desktop/src/lib/command-palette/CommandPalette.test.ts

Lines changed: 91 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,13 @@
1-
import { describe, it, expect, vi, beforeEach } from 'vitest'
1+
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'
22
import { mount, unmount } from 'svelte'
3-
import { tick } from 'svelte'
3+
import { tick, flushSync } from 'svelte'
44
import CommandPalette from './CommandPalette.svelte'
55
import { pruneRecentCommands, pushRecentCommand } from '$lib/app-status-store'
6+
import { setShortcut, resetShortcut } from '$lib/shortcuts/shortcuts-store'
67

8+
// These ids are real command-registry entries, so the palette's effective-shortcut
9+
// reads (which hit the real registry + store, not the mocked `$lib/commands`)
10+
// resolve against known bindings. `app.quit` defaults to ⌘Q, `app.about` is unbound.
711
const ALL_COMMANDS = [
812
{ id: 'app.quit', name: 'Quit Cmdr', scope: 'App', shortcuts: ['⌘Q'], showInPalette: true },
913
{ id: 'app.about', name: 'About Cmdr', scope: 'App', shortcuts: [], showInPalette: true },
@@ -17,6 +21,33 @@ vi.mock('$lib/app-status-store', () => ({
1721
pushRecentCommand: vi.fn().mockResolvedValue(undefined),
1822
}))
1923

24+
// The shortcuts store persists to a Tauri plugin store and syncs menu accelerators;
25+
// stub both so the palette can read/rebind effective shortcuts in jsdom.
26+
vi.mock('@tauri-apps/plugin-store', () => ({
27+
load: vi.fn(() =>
28+
Promise.resolve({
29+
get: vi.fn(() => Promise.resolve(undefined)),
30+
set: vi.fn(() => Promise.resolve()),
31+
save: vi.fn(() => Promise.resolve()),
32+
keys: vi.fn(() => Promise.resolve([])),
33+
delete: vi.fn(() => Promise.resolve()),
34+
}),
35+
),
36+
}))
37+
38+
vi.mock('$lib/ipc/bindings', () => ({
39+
commands: { updateMenuAccelerator: vi.fn(() => Promise.resolve({ status: 'ok' })) },
40+
}))
41+
42+
/** Read the chip text inside the row for a given command id. */
43+
function shortcutTextsFor(target: HTMLElement, commandId: string): string[] {
44+
// Attribute selector, not `#palette-option-app.quit` — a dotted command id would
45+
// parse the part after the dot as a class selector.
46+
const row = target.querySelector(`[id="palette-option-${commandId}"]`)
47+
if (!row) return []
48+
return Array.from(row.querySelectorAll('.shortcuts .shortcut-chip')).map((el) => el.textContent)
49+
}
50+
2051
// Mock the commands module to provide test data
2152
vi.mock('$lib/commands', () => ({
2253
getPaletteCommands: vi.fn(() => ALL_COMMANDS),
@@ -51,6 +82,12 @@ describe('CommandPalette', () => {
5182
vi.mocked(pushRecentCommand).mockClear()
5283
})
5384

85+
afterEach(() => {
86+
// Drop any rebinds so a custom shortcut doesn't leak into the next test.
87+
resetShortcut('app.quit')
88+
resetShortcut('view.showHidden')
89+
})
90+
5491
it('renders the modal with search input', async () => {
5592
const target = document.createElement('div')
5693
mount(CommandPalette, {
@@ -449,6 +486,58 @@ describe('CommandPalette', () => {
449486
expect(input?.getAttribute('aria-expanded')).toBe('false')
450487
})
451488

489+
it('renders the effective custom binding, not the registry default', async () => {
490+
// app.quit defaults to ⌘Q; rebind it and the row must show the custom combo.
491+
setShortcut('app.quit', 0, '⌘0')
492+
493+
const target = document.createElement('div')
494+
mount(CommandPalette, {
495+
target,
496+
props: { onExecute: mockOnExecute, onClose: mockOnClose },
497+
})
498+
await tick()
499+
500+
expect(shortcutTextsFor(target, 'app.quit')).toEqual(['⌘0'])
501+
})
502+
503+
it('caps the shown shortcuts at three', async () => {
504+
// Bind four shortcuts; only the first three render.
505+
setShortcut('view.showHidden', 0, '⌘1')
506+
setShortcut('view.showHidden', 1, '⌘2')
507+
setShortcut('view.showHidden', 2, '⌘3')
508+
setShortcut('view.showHidden', 3, '⌘4')
509+
510+
const target = document.createElement('div')
511+
mount(CommandPalette, {
512+
target,
513+
props: { onExecute: mockOnExecute, onClose: mockOnClose },
514+
})
515+
await tick()
516+
517+
expect(shortcutTextsFor(target, 'view.showHidden')).toEqual(['⌘1', '⌘2', '⌘3'])
518+
})
519+
520+
it('updates a rendered row live when the binding changes while the palette is open', async () => {
521+
// Custom bindings are stored verbatim (no platform conversion), so they assert
522+
// cleanly regardless of the test platform.
523+
setShortcut('app.quit', 0, '⌘7')
524+
525+
const target = document.createElement('div')
526+
mount(CommandPalette, {
527+
target,
528+
props: { onExecute: mockOnExecute, onClose: mockOnClose },
529+
})
530+
await tick()
531+
532+
expect(shortcutTextsFor(target, 'app.quit')).toEqual(['⌘7'])
533+
534+
// Simulate a rebind (the MCP / Settings path) while the palette stays open.
535+
setShortcut('app.quit', 0, '⌘8')
536+
flushSync()
537+
538+
expect(shortcutTextsFor(target, 'app.quit')).toEqual(['⌘8'])
539+
})
540+
452541
it('does not throw if the previously focused element is no longer in the DOM', async () => {
453542
const trigger = document.createElement('button')
454543
document.body.appendChild(trigger)

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

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -338,11 +338,12 @@ the per-component tier rules in `age-tier-utils.ts`, and the HTML-string variant
338338
`ShortcutChip.svelte`: the one component that renders a keyboard shortcut anywhere in the UI, so the look stays uniform
339339
and new call sites can't hand-roll a divergent style. Two mutually exclusive modes:
340340

341-
| Prop | Type | Notes |
342-
| ----------- | ------------ | -------------------------------------------------------------------------------------------------------- |
343-
| `commandId` | `CommandId?` | Dynamic mode. Renders the command's first effective shortcut via `getFirstShortcutReactive`, reactively. |
344-
| `key` | `string?` | Literal mode. A fixed key string (toast snapshots, fixed interaction keys). Never clickable. |
345-
| `clickable` | `boolean?` | Default `true` in `commandId` mode; ignored (forced non-clickable) in literal mode. |
341+
| Prop | Type | Notes |
342+
| ----------- | --------------- | -------------------------------------------------------------------------------------------------------- |
343+
| `commandId` | `CommandId?` | Dynamic mode. Renders the command's first effective shortcut via `getFirstShortcutReactive`, reactively. |
344+
| `key` | `string?` | Literal mode. A fixed key string (toast snapshots, fixed interaction keys). Never clickable. |
345+
| `clickable` | `boolean?` | Default `true` in `commandId` mode; ignored (forced non-clickable) in literal mode. |
346+
| `size` | `'sm' \| 'md'?` | Visual density. `md` (default) is the standalone pill; `sm` tightens padding + radius for dense rows. |
346347

347348
Exactly one of `commandId` / `key` must be set (a dev-time error otherwise).
348349

@@ -367,7 +368,9 @@ loads it via dynamic `import()` inside the click handler only. Keep it that way.
367368
**Visual.** Neutral pill modeled on the Settings `.shortcut-pill` (`--color-bg-tertiary` background, 1px
368369
`--color-border`, `--radius-sm`, `--font-size-xs`), NOT the tooltip's accent chip (`.cmdr-tooltip-kbd` stays its accent
369370
look — different context). The clickable variant adds an accent border + `--color-accent-text` on hover; cursor stays
370-
`default` per the app-wide convention (only `LinkButton` opts into `cursor: pointer`).
371+
`default` per the app-wide convention (only `LinkButton` opts into `cursor: pointer`). `size="sm"` shrinks padding to
372+
`0 var(--spacing-xs)` and the corner radius to `--radius-xs` for dense rows where several chips sit side by side — the
373+
command palette (up to three chips per row) is the first consumer.
371374

372375
The `shortcut-<commandId>` anchor-id convention (shared with the Settings section the deep link targets) lives as the
373376
exported `shortcutAnchorId(commandId)` in `lib/settings/settings-window.ts` so it can't drift.

apps/desktop/src/lib/ui/ShortcutChip.svelte

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,9 +41,15 @@
4141
* rows, F-key bar buttons) where a nested click target would double-activate.
4242
*/
4343
clickable?: boolean
44+
/**
45+
* Visual density. `md` (default) is the standalone pill. `sm` tightens the
46+
* padding and corner radius for dense rows where several chips sit side by
47+
* side (the command palette caps at three).
48+
*/
49+
size?: 'sm' | 'md'
4450
}
4551
46-
const { commandId, key, clickable = true }: Props = $props()
52+
const { commandId, key, clickable = true, size = 'md' }: Props = $props()
4753
4854
if ((commandId === undefined) === (key === undefined)) {
4955
throw new Error('ShortcutChip: set exactly one of `commandId` or `key`')
@@ -69,14 +75,15 @@
6975
<button
7076
type="button"
7177
class="shortcut-chip clickable"
78+
class:sm={size === 'sm'}
7279
aria-label="Customize the {commandName} shortcut"
7380
onclick={handleClick}
7481
use:tooltip={'Customize this shortcut'}
7582
>
7683
<kbd>{value}</kbd>
7784
</button>
7885
{:else}
79-
<kbd class="shortcut-chip">{value}</kbd>
86+
<kbd class="shortcut-chip" class:sm={size === 'sm'}>{value}</kbd>
8087
{/if}
8188
{/if}
8289

@@ -98,6 +105,13 @@
98105
white-space: nowrap;
99106
}
100107
108+
/* Dense variant: tighter padding + corner radius so several chips fit a row
109+
(the command palette shows up to three). */
110+
.shortcut-chip.sm {
111+
padding: 0 var(--spacing-xs);
112+
border-radius: var(--radius-xs);
113+
}
114+
101115
/* The clickable variant is a real <button> wrapping the <kbd>. Reset the button
102116
chrome so it reads as the same pill; the inner <kbd> carries no border of its own. */
103117
button.shortcut-chip {

apps/desktop/src/lib/ui/ShortcutChip.test.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,9 @@ describe('ShortcutChip', () => {
102102

103103
button?.click()
104104
// The dynamic import resolves on a microtask; let the event loop drain.
105-
await vi.waitFor(() => { expect(openShortcutCustomization).toHaveBeenCalledWith('downloads.goToLatest'); })
105+
await vi.waitFor(() => {
106+
expect(openShortcutCustomization).toHaveBeenCalledWith('downloads.goToLatest')
107+
})
106108
})
107109

108110
it('renders a non-clickable kbd when clickable is false', () => {

apps/desktop/src/routes/dev/components/sections/ShortcutChip.svelte

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,16 @@
3030
<span class="row-note">For chips nested inside another interactive control (palette rows, F-key bar)</span>
3131
</div>
3232

33+
<div class="row">
34+
<span class="row-label">Dense (size="sm")</span>
35+
<div class="chips">
36+
<ShortcutChip key="⌘1" size="sm" />
37+
<ShortcutChip key="⌘2" size="sm" />
38+
<ShortcutChip key="⌘3" size="sm" />
39+
</div>
40+
<span class="row-note">Tighter padding + radius for dense rows (the command palette)</span>
41+
</div>
42+
3343
<div class="row">
3444
<span class="row-label">Command (unbound)</span>
3545
<div class="chips">

0 commit comments

Comments
 (0)