From 1244eef3d4b30eb4eeff39c8617aee50f06eec00 Mon Sep 17 00:00:00 2001 From: Chris0Jeky Date: Sat, 5 Sep 2026 05:00:01 +0100 Subject: [PATCH 1/5] test(shortcuts): pin the shell keyboard guard defects red Adds the failing behaviour specs for #2621 and the #1968 keyboard-guard LOWs before the fix: board f and n reaching the board from behind the open help dialog, the command palette and quick capture opening over a modal they do not own, ? toggling over another modal, Shift+H navigating Home, and ? being unreachable when AltGr sets altKey. --- .../src/tests/components/AppShell.spec.ts | 206 ++++++++++++++++++ 1 file changed, 206 insertions(+) diff --git a/frontend/taskdeck-web/src/tests/components/AppShell.spec.ts b/frontend/taskdeck-web/src/tests/components/AppShell.spec.ts index 93b692705..fd33d95f7 100644 --- a/frontend/taskdeck-web/src/tests/components/AppShell.spec.ts +++ b/frontend/taskdeck-web/src/tests/components/AppShell.spec.ts @@ -6,6 +6,7 @@ import { useShellKeyboardHelp, type ShellKeyboardHelpControl, } from '../../composables/useShellKeyboardHelp' +import { useKeyboardShortcuts } from '../../composables/useKeyboardShortcuts' import type { FeatureFlags } from '../../types/feature-flags' /** @@ -22,6 +23,59 @@ const ShellHelpProbe = defineComponent({ }, }) +const boardProbe = reactive({ filterToggles: 0, addCardClicks: 0 }) + +/** + * The routed board's keyboard seam, at composable level (#2621). + * + * It installs the REAL `useKeyboardShortcuts` with BoardView's own `f` and `n` + * bindings and the same `[data-action="toggle-add-card"]` / + * `[data-action="add-card-input"]` DOM contract `createCardInSelectedColumn` + * drives, so the routing under test — AppShell's capture-phase window listener + * versus the board's bubble-phase one — is the shipped mechanism, not a mock. + * + * Fidelity limit: BoardView passes `enabled` predicates these bindings omit. + * Both predicates are true in the state this file exercises (a routed Legacy + * board with no Paper dialog open), which is exactly why the shell guard is + * the only thing standing between the modal and the board. + */ +const BoardKeyProbe = defineComponent({ + setup() { + useKeyboardShortcuts([ + { + key: 'f', + description: 'Toggle filter panel', + action: () => { + boardProbe.filterToggles += 1 + }, + }, + { + key: 'n', + description: 'New card in current column', + action: () => { + const column = document.querySelector('[data-column-id="column-1"]') + column?.querySelector('[data-action="toggle-add-card"]')?.click() + column?.querySelector('[data-action="add-card-input"]')?.focus() + }, + }, + ]) + + return () => h('div', { 'data-column-id': 'column-1' }, [ + h( + 'button', + { + 'data-action': 'toggle-add-card', + onClick: () => { + boardProbe.addCardClicks += 1 + }, + }, + 'Add card', + ), + h('textarea', { 'data-action': 'add-card-input' }), + ]) + }, +}) + const mockRouter = { push: vi.fn(), } @@ -150,6 +204,8 @@ describe('AppShell workspace navigation and command palette', () => { mockFeatureFlags.isEnabled = vi.fn((_flag: keyof FeatureFlags) => true) mockPaperTheme.isOn = false injectedShellHelp = null + boardProbe.filterToggles = 0 + boardProbe.addCardClicks = 0 }) afterEach(() => { @@ -443,6 +499,156 @@ describe('AppShell workspace navigation and command palette', () => { expect(mockRouter.push).not.toHaveBeenCalled() }) + /** + * The real routing path. A key pressed with focus outside a modal targets + * `document.body`, so the event runs AppShell's capture-phase window listener + * first and only then bubbles back to the board's window listener. Dispatching + * on `window` instead would put the event AT_TARGET, where capture and bubble + * listeners both run whatever propagation says, and prove nothing about the + * order the browser actually uses. + */ + function pressFromBody(key: string, init: KeyboardEventInit = {}) { + document.body.dispatchEvent(new KeyboardEvent('keydown', { key, bubbles: true, ...init })) + } + + function helpDialog(): HTMLElement | null { + return document.querySelector('[aria-label="Keyboard shortcuts"]') + } + + it('keeps board bare-letter keys away from the board while the help dialog is open', async () => { + mountedWrapper = mountShell(document.body, { RouterView: BoardKeyProbe }) + await waitForUi() + + pressFromBody('?') + await waitForUi() + const dialog = helpDialog() + expect(dialog).not.toBeNull() + + const dialogClose = dialog!.querySelector('button') as HTMLButtonElement + dialogClose.focus() + expect(dialog!.contains(document.activeElement)).toBe(true) + + pressFromBody('f') + pressFromBody('n') + await waitForUi() + + // `f` toggled the filter panel behind the modal and `n` clicked the column's + // add-card button and pulled focus into the composer, both from behind an + // open `aria-modal` dialog (#2621). + expect(boardProbe.filterToggles).toBe(0) + expect(boardProbe.addCardClicks).toBe(0) + expect(dialog!.contains(document.activeElement)).toBe(true) + }) + + it('keeps ? and Escape working on the help dialog with the board listener mounted', async () => { + mountedWrapper = mountShell(document.body, { RouterView: BoardKeyProbe }) + await waitForUi() + + pressFromBody('?') + await waitForUi() + expect(helpDialog()).not.toBeNull() + + // The surface that owns `?` still gets it, so the key stays a toggle. + pressFromBody('?') + await waitForUi() + expect(helpDialog()).toBeNull() + + pressFromBody('?') + await waitForUi() + expect(helpDialog()).not.toBeNull() + + pressFromBody('Escape') + await waitForUi() + expect(helpDialog()).toBeNull() + }) + + it('does not open the command palette over a modal it does not own', async () => { + mountedWrapper = mountShell(document.body) + const wrapper = mountedWrapper + + pressFromBody('C', { ctrlKey: true, shiftKey: true }) + await waitForUi() + expect(wrapper.find('[aria-label="Capture modal"]').exists()).toBe(true) + + pressFromBody('k', { ctrlKey: true }) + await waitForUi() + + expect(wrapper.find('[aria-label="Command palette"]').exists()).toBe(false) + }) + + it('still toggles the command palette closed with the key that opened it', async () => { + mountedWrapper = mountShell(document.body) + const wrapper = mountedWrapper + + pressFromBody('k', { ctrlKey: true }) + await waitForUi() + expect(wrapper.find('[aria-label="Command palette"]').exists()).toBe(true) + + pressFromBody('k', { ctrlKey: true }) + await waitForUi() + expect(wrapper.find('[aria-label="Command palette"]').exists()).toBe(false) + }) + + it('does not open quick capture over an active modal', async () => { + mountedWrapper = mountShell(document.body) + const wrapper = mountedWrapper + + pressFromBody('?') + await waitForUi() + expect(helpDialog()).not.toBeNull() + + pressFromBody('C', { ctrlKey: true, shiftKey: true }) + await waitForUi() + + expect(wrapper.find('[aria-label="Capture modal"]').exists()).toBe(false) + }) + + it('does not toggle the help dialog over a modal that owns the keyboard', async () => { + mountedWrapper = mountShell(document.body) + const wrapper = mountedWrapper + + pressFromBody('C', { ctrlKey: true, shiftKey: true }) + await waitForUi() + expect(wrapper.find('[aria-label="Capture modal"]').exists()).toBe(true) + + pressFromBody('?') + await waitForUi() + + expect(helpDialog()).toBeNull() + }) + + it('does not navigate when Shift is held with a bare-letter binding', async () => { + mountedWrapper = mountShell() + + // `strokeMatches` compares keys case-insensitively, so `Shift+H` arrives as + // `H` and used to navigate Home (#1968). + window.dispatchEvent(new KeyboardEvent('keydown', { key: 'H', shiftKey: true })) + window.dispatchEvent(new KeyboardEvent('keydown', { key: 'T', shiftKey: true })) + await waitForUi() + + expect(mockRouter.push).not.toHaveBeenCalled() + }) + + it('toggles the keyboard map when ? needs AltGr and leaves Alt letter combos alone', async () => { + mountedWrapper = mountShell() + const wrapper = mountedWrapper + + // AltGr reports `altKey: true`, and on Windows `ctrlKey: true` as well, so + // the strict modifier comparison made `?` unreachable on those layouts. + window.dispatchEvent(new KeyboardEvent('keydown', { key: '?', altKey: true })) + await waitForUi() + expect(wrapper.find('[aria-label="Keyboard shortcuts"]').exists()).toBe(true) + + window.dispatchEvent(new KeyboardEvent('keydown', { key: '?', altKey: true, ctrlKey: true })) + await waitForUi() + expect(wrapper.find('[aria-label="Keyboard shortcuts"]').exists()).toBe(false) + + // Alt over a letter binding is still a different stroke, not Home. + window.dispatchEvent(new KeyboardEvent('keydown', { key: 'h', altKey: true })) + await waitForUi() + expect(mockRouter.push).not.toHaveBeenCalled() + }) + it.each(['input', 'textarea', 'select', 'contenteditable'])( 'suppresses bare and chord navigation inside %s targets', async (kind) => { From 10b3f9a67218113aa2c1881f832cf1050554c4f2 Mon Sep 17 00:00:00 2001 From: Chris0Jeky Date: Sat, 5 Sep 2026 05:03:42 +0100 Subject: [PATCH 2/5] fix(shortcuts): keep board keys behind an open modal and tighten the shell guards AppShell now stops a keydown that no app-shell binding consumed while a keyboard-owning surface is active, so board bare-letter bindings can no longer run behind the help dialog (#2621). Escape and any target inside the surface are exempt: the escape stack and BoardView.closeOpenUi still close what they own, and a capture-phase stop would otherwise break typing inside modals. Each non-navigate action now says which surface owns its key, marked with data-shell-surface: ? toggles the help dialog it owns and does nothing over another modal, mod+k still closes the palette but no longer opens over one, and quick capture never stacks a second modal. strokeMatches moves into the shared ledger module and gains two rules: an undeclared shift now requires Shift to be up over a letter, so Shift+H stops navigating Home, and a layout-produced character ignores Alt plus the AltGr Ctrl, so ? stays reachable where the layout needs AltGr. The surface scan is now lazy per event, so a keystroke typed into a field no longer pays for the querySelectorAll and getComputedStyle sweep. Closes #2621. Refs #1968. --- .../components/paper/PaperCommandPalette.vue | 1 + .../paper/PaperShortcutsOverlay.vue | 1 + .../src/components/shell/AppShell.vue | 86 +++++++++++++++---- .../components/shell/ShellCommandPalette.vue | 1 + .../components/shell/ShellKeyboardHelp.vue | 1 + .../src/utils/keyboardShortcuts.ts | 50 +++++++++++ 6 files changed, 121 insertions(+), 19 deletions(-) diff --git a/frontend/taskdeck-web/src/components/paper/PaperCommandPalette.vue b/frontend/taskdeck-web/src/components/paper/PaperCommandPalette.vue index 75e5c983a..e5beb4cf5 100644 --- a/frontend/taskdeck-web/src/components/paper/PaperCommandPalette.vue +++ b/frontend/taskdeck-web/src/components/paper/PaperCommandPalette.vue @@ -231,6 +231,7 @@ watch(orderedItems, (items) => { role="dialog" aria-modal="true" aria-label="Command palette" + data-shell-surface="command-palette" @click.self="handleClose" @keydown.escape.prevent="handleClose" > diff --git a/frontend/taskdeck-web/src/components/paper/PaperShortcutsOverlay.vue b/frontend/taskdeck-web/src/components/paper/PaperShortcutsOverlay.vue index a35a25458..222935ab4 100644 --- a/frontend/taskdeck-web/src/components/paper/PaperShortcutsOverlay.vue +++ b/frontend/taskdeck-web/src/components/paper/PaperShortcutsOverlay.vue @@ -56,6 +56,7 @@ function onBackdropClick() { role="dialog" aria-modal="true" aria-labelledby="paper-shortcuts-title" + data-shell-surface="keyboard-help" @click.self="onBackdropClick" >
diff --git a/frontend/taskdeck-web/src/components/shell/AppShell.vue b/frontend/taskdeck-web/src/components/shell/AppShell.vue index 395ea78d0..0d9824da7 100644 --- a/frontend/taskdeck-web/src/components/shell/AppShell.vue +++ b/frontend/taskdeck-web/src/components/shell/AppShell.vue @@ -10,8 +10,9 @@ import { useViewportMode } from '../../composables/useViewportMode' import { provideShellKeyboardHelp } from '../../composables/useShellKeyboardHelp' import { APP_SHELL_SHORTCUT_BINDINGS, + strokeMatches, + type AppShellShortcutAction, type AppShellShortcutBinding, - type ShortcutStroke, } from '../../utils/keyboardShortcuts' import CaptureModal from '../common/CaptureModal.vue' import OfflineBanner from './OfflineBanner.vue' @@ -153,11 +154,11 @@ const KEYBOARD_OWNING_SURFACE_SELECTOR = [ '[aria-modal="true"]', ].join(', ') -function hasActiveKeyboardOwningSurface(): boolean { - if (typeof document === 'undefined') return false +function activeKeyboardOwningSurfaces(): HTMLElement[] { + if (typeof document === 'undefined') return [] return Array.from(document.querySelectorAll(KEYBOARD_OWNING_SURFACE_SELECTOR)) - .some((surface) => { + .filter((surface) => { if (!surface.isConnected) return false if (surface.closest('[hidden], [aria-hidden="true"], [inert]')) return false @@ -166,6 +167,20 @@ function hasActiveKeyboardOwningSurface(): boolean { }) } +/** + * True when every surface currently owning the keyboard is the shell's own + * surface for this action, which is what makes `?` and `mod+k` toggles rather + * than one-way openers: the help dialog owns `?`, the command palette owns + * `mod+k`, and neither owns the other's key (#1968). + * + * `navigate` and `quick-capture` name no surface, so an active surface always + * wins over them: nothing behind a modal should move the route, and quick + * capture would stack a second modal on the first (the #1959 class). + */ +function surfacesOwnAction(surfaces: readonly HTMLElement[], action: AppShellShortcutAction): boolean { + return surfaces.every((surface) => surface.dataset.shellSurface === action.type) +} + const CHORD_TIMEOUT_MS = 1_000 let pendingChord: AppShellShortcutBinding | null = null let chordTimer: ReturnType | null = null @@ -178,15 +193,6 @@ function clearPendingChord() { } } -function strokeMatches(event: KeyboardEvent, stroke: ShortcutStroke): boolean { - const modPressed = event.ctrlKey || event.metaKey - const shiftMatches = stroke.shift === undefined || stroke.shift === event.shiftKey - return event.key.toLowerCase() === stroke.key.toLowerCase() && - modPressed === Boolean(stroke.mod) && - event.altKey === Boolean(stroke.alt) && - shiftMatches -} - function consumeShortcut(event: KeyboardEvent) { event.preventDefault() // AppShell owns the workspace-level keys. Capture-phase handling means a @@ -215,6 +221,35 @@ function runAppShellShortcut(binding: AppShellShortcutBinding) { } } +/** + * Keep a key an active surface does not own from reaching the page behind it + * (#2621). + * + * With the help dialog open over a Legacy board, a plain `f` or `n` used to + * bubble past this listener to `BoardView`'s `useKeyboardShortcuts` window + * listener: `f` toggled the filter panel behind the modal and `n` clicked the + * column's add-card button and pulled focus out of the dialog. + * + * Two carve-outs keep this from taking more than it should: + * - Escape is never stopped. Board dialogs the escape stack does not carry + * (label manager, board settings, filter panel, column form) are closed by + * `BoardView.closeOpenUi` on the bubble, and stopping Escape here would + * strand them open. + * - A target inside the surface is left alone, because this listener runs in + * the capture phase, ahead of every handler the surface owns. Stopping + * there would break typing and arrow navigation inside modals. + * Text-entry targets never reach this, and never triggered board shortcuts in + * the first place -- `useKeyboardShortcuts` ignores them. + */ +function guardSurfaceFromPageShortcuts(event: KeyboardEvent, surfaces: readonly HTMLElement[]) { + if (event.key === 'Escape') return + + const target = event.target instanceof Node ? event.target : null + if (target && surfaces.some((surface) => surface.contains(target))) return + + event.stopPropagation() +} + function handleKeydown(event: KeyboardEvent) { if (event.isComposing) { clearPendingChord() @@ -222,15 +257,23 @@ function handleKeydown(event: KeyboardEvent) { } const textEntryTarget = isTextEntryTarget(event.target) - const keyboardOwningSurfaceActive = hasActiveKeyboardOwningSurface() - if (keyboardOwningSurfaceActive) clearPendingChord() + // Scanned at most once per event, and only once something actually needs the + // answer, so an ordinary keystroke typed into a field never pays for the + // `querySelectorAll` plus `getComputedStyle` sweep (#1968). + let surfaces: HTMLElement[] | null = null + const keyboardOwningSurfaces = () => (surfaces ??= activeKeyboardOwningSurfaces()) if (pendingChord) { const chord = pendingChord clearPendingChord() const nextStroke = chord.sequence[1] - if (!textEntryTarget && nextStroke && strokeMatches(event, nextStroke)) { + if ( + !textEntryTarget && + nextStroke && + strokeMatches(event, nextStroke) && + keyboardOwningSurfaces().length === 0 + ) { consumeShortcut(event) runAppShellShortcut(chord) return @@ -240,8 +283,8 @@ function handleKeydown(event: KeyboardEvent) { const direct = APP_SHELL_SHORTCUT_BINDINGS.find((binding) => binding.sequence.length === 1 && strokeMatches(event, binding.sequence[0]!) && - (binding.action.type !== 'navigate' || !keyboardOwningSurfaceActive) && - (!textEntryTarget || binding.allowInTextEntry === true), + (!textEntryTarget || binding.allowInTextEntry === true) && + surfacesOwnAction(keyboardOwningSurfaces(), binding.action), ) if (direct) { consumeShortcut(event) @@ -249,7 +292,12 @@ function handleKeydown(event: KeyboardEvent) { return } - if (textEntryTarget || keyboardOwningSurfaceActive) return + if (textEntryTarget) return + + if (keyboardOwningSurfaces().length > 0) { + guardSurfaceFromPageShortcuts(event, keyboardOwningSurfaces()) + return + } const chord = APP_SHELL_SHORTCUT_BINDINGS.find((binding) => binding.sequence.length > 1 && strokeMatches(event, binding.sequence[0]!), diff --git a/frontend/taskdeck-web/src/components/shell/ShellCommandPalette.vue b/frontend/taskdeck-web/src/components/shell/ShellCommandPalette.vue index bed68f612..7218a2074 100644 --- a/frontend/taskdeck-web/src/components/shell/ShellCommandPalette.vue +++ b/frontend/taskdeck-web/src/components/shell/ShellCommandPalette.vue @@ -208,6 +208,7 @@ watch(allPaletteItems, (items) => { role="dialog" aria-label="Command palette" aria-modal="true" + data-shell-surface="command-palette" @click.self="handleClose" @keydown.escape="handleClose" > diff --git a/frontend/taskdeck-web/src/components/shell/ShellKeyboardHelp.vue b/frontend/taskdeck-web/src/components/shell/ShellKeyboardHelp.vue index 9b9b4e563..a4b7c9f4d 100644 --- a/frontend/taskdeck-web/src/components/shell/ShellKeyboardHelp.vue +++ b/frontend/taskdeck-web/src/components/shell/ShellKeyboardHelp.vue @@ -40,6 +40,7 @@ const emit = defineEmits<{ role="dialog" aria-label="Keyboard shortcuts" aria-modal="true" + data-shell-surface="keyboard-help" @click.self="emit('close')" @keydown.escape="emit('close')" > diff --git a/frontend/taskdeck-web/src/utils/keyboardShortcuts.ts b/frontend/taskdeck-web/src/utils/keyboardShortcuts.ts index ac9601758..88a18d939 100644 --- a/frontend/taskdeck-web/src/utils/keyboardShortcuts.ts +++ b/frontend/taskdeck-web/src/utils/keyboardShortcuts.ts @@ -180,6 +180,56 @@ export function formatShortcut( .join(' / ') } +/** + * A stroke key the keyboard produces the same way whichever modifiers reach it. + * Letters are the case that needs the guard below: `Shift+H` arrives as `H`, + * which a case-insensitive comparison cannot tell apart from `h`. + */ +const LETTER_KEY = /^[a-z]$/i + +/** + * A printable character the LAYOUT produces, `?` being the one in the ledger. + * Which modifiers were held to reach it is a property of the layout, not of the + * shortcut: on a layout where `?` needs AltGr the browser reports `altKey`, and + * on Windows `ctrlKey` too, because AltGr is Ctrl+Alt there. + */ +function isLayoutProducedCharacter(key: string): boolean { + return key.length === 1 && !/[a-z0-9]/i.test(key) +} + +/** + * Match one keydown against one canonical stroke. + * + * Shift: a stroke that declares `shift` is exact. A stroke that does not + * declare it requires Shift to be UP over a letter, so `Shift+H` no longer + * navigates Home (#1968); over a layout-produced character it stays permissive, + * because Shift is usually how you type the character at all. + * + * Alt and mod: exact, except over a layout-produced character that declares + * neither. There Alt is ignored, and Ctrl is ignored while Alt is also down -- + * the AltGr signature -- so the `?` help key stays reachable on those layouts + * without loosening any ordinary Ctrl or Alt combination. + */ +export function strokeMatches(event: KeyboardEvent, stroke: ShortcutStroke): boolean { + if (event.key.toLowerCase() !== stroke.key.toLowerCase()) return false + + const layoutCharacter = !stroke.mod && !stroke.alt && isLayoutProducedCharacter(stroke.key) + + if (stroke.shift !== undefined) { + if (event.shiftKey !== stroke.shift) return false + } else if (LETTER_KEY.test(stroke.key) && event.shiftKey) { + return false + } + + if (!layoutCharacter) { + return (event.ctrlKey || event.metaKey) === Boolean(stroke.mod) && + event.altKey === Boolean(stroke.alt) + } + + const altGrPressed = event.ctrlKey && event.altKey + return altGrPressed || !(event.ctrlKey || event.metaKey) +} + /** * The handler owner is part of every displayed row. Adding an overlay entry * therefore requires naming the concrete runtime that owns it rather than From 059581f39126447099d8e781cd39b92f97bfe36c Mon Sep 17 00:00:00 2001 From: Chris0Jeky Date: Sat, 5 Sep 2026 05:05:51 +0100 Subject: [PATCH 3/5] test(shortcuts): assert the advertised strokes instead of the AppShell source text The Paper overlay spec asserted that the AppShell source contains APP_SHELL_SHORTCUT_BINDINGS.find. What that reached for is that an app-shell row on the surface is a key the shell really dispatches, which a grep cannot see. Each displayed app-shell row now runs the shipped matcher over the stroke it advertises, and the footer help key is checked on the Shift and AltGr layouts that have to reach it. keyboardShortcuts.spec.ts gains direct coverage of the matcher: Shift over a bare letter, an exact declared shift, exact mod and alt over letters, and the ? stroke under Shift, AltGr and a real Ctrl or Command chord. Refs #1968. --- .../paper/PaperShortcutsOverlay.spec.ts | 47 ++++++++++- .../src/tests/utils/keyboardShortcuts.spec.ts | 78 ++++++++++++++++++- 2 files changed, 123 insertions(+), 2 deletions(-) diff --git a/frontend/taskdeck-web/src/tests/components/paper/PaperShortcutsOverlay.spec.ts b/frontend/taskdeck-web/src/tests/components/paper/PaperShortcutsOverlay.spec.ts index b40594640..651625e2b 100644 --- a/frontend/taskdeck-web/src/tests/components/paper/PaperShortcutsOverlay.spec.ts +++ b/frontend/taskdeck-web/src/tests/components/paper/PaperShortcutsOverlay.spec.ts @@ -8,8 +8,10 @@ import boardViewSource from '../../../views/BoardView.vue?raw' import { APP_SHELL_SHORTCUT_BINDINGS, formatShortcut, + KEYBOARD_HELP_SHORTCUT, PAPER_SHORTCUT_GROUPS, SHORTCUT_HANDLER_CONTRACTS, + strokeMatches, type ShortcutHandlerOwner, } from '../../../utils/keyboardShortcuts' @@ -100,7 +102,50 @@ describe('PaperShortcutsOverlay', () => { expect(expectedRows.filter((row) => row.handlerOwner === 'app-shell').every( (row) => appShellIds.has(row.id), )).toBe(true) - expect(appShellSource).toContain('APP_SHELL_SHORTCUT_BINDINGS.find') + }) + + /** + * Replaces a `toContain('APP_SHELL_SHORTCUT_BINDINGS.find')` scan of the + * AppShell source (#1968). What that line was reaching for is that an + * app-shell row on this surface is a key the shell really dispatches, and a + * grep cannot see that. Running the shipped matcher over the stroke the row + * advertises can: a row whose declared stroke its own matcher rejects is + * exactly the dead affordance the ledger exists to prevent. + */ + it('advertises app-shell rows whose declared stroke the shipped matcher accepts', () => { + wrapper = mount(PaperShortcutsOverlay, { props: { visible: true }, attachTo: document.body }) + const displayedIds = new Set(Array.from( + teleportContent().querySelectorAll('[data-shortcut-id]'), + ).map((row) => row.dataset.shortcutId)) + + const displayedAppShellBindings = APP_SHELL_SHORTCUT_BINDINGS + .filter((binding) => displayedIds.has(binding.id)) + expect(displayedAppShellBindings.length).toBeGreaterThan(0) + + for (const binding of displayedAppShellBindings) { + for (const stroke of binding.sequence) { + const pressed = new KeyboardEvent('keydown', { + key: stroke.key, + ctrlKey: stroke.mod === true, + shiftKey: stroke.shift === true, + altKey: stroke.alt === true, + }) + expect({ id: binding.id, key: stroke.key, matched: strokeMatches(pressed, stroke) }) + .toEqual({ id: binding.id, key: stroke.key, matched: true }) + } + } + + // The overlay prints the help key in its footer rather than as a grouped + // row, and that key is the one the shell toggles this surface on. It has to + // survive the layouts that need Shift or AltGr to type it (#1968). + const root = teleportContent().querySelector('[data-paper-shortcuts]') as HTMLElement + expect(root.textContent).toContain(formatShortcut(KEYBOARD_HELP_SHORTCUT.descriptor)) + + const helpStroke = KEYBOARD_HELP_SHORTCUT.sequence[0]! + expect(strokeMatches(new KeyboardEvent('keydown', { key: '?', shiftKey: true }), helpStroke)) + .toBe(true) + expect(strokeMatches(new KeyboardEvent('keydown', { key: '?', altKey: true }), helpStroke)) + .toBe(true) }) it('documents the implemented Paper Board navigation and movement commands', () => { diff --git a/frontend/taskdeck-web/src/tests/utils/keyboardShortcuts.spec.ts b/frontend/taskdeck-web/src/tests/utils/keyboardShortcuts.spec.ts index 22a405e31..097d4b392 100644 --- a/frontend/taskdeck-web/src/tests/utils/keyboardShortcuts.spec.ts +++ b/frontend/taskdeck-web/src/tests/utils/keyboardShortcuts.spec.ts @@ -1,5 +1,10 @@ import { describe, expect, it } from 'vitest' -import { formatShortcut } from '../../utils/keyboardShortcuts' +import { + APP_SHELL_SHORTCUT_BINDINGS, + formatShortcut, + KEYBOARD_HELP_SHORTCUT, + strokeMatches, +} from '../../utils/keyboardShortcuts' describe('formatShortcut', () => { it('prefers userAgentData and renders Apple modifier glyphs', () => { @@ -62,3 +67,74 @@ describe('formatShortcut', () => { expect(formatShortcut('⌘⏎', { userAgentData: { platform: 'macOS' } })).toBe('⌘⏎') }) }) + +describe('strokeMatches', () => { + function keydown(init: KeyboardEventInit): KeyboardEvent { + return new KeyboardEvent('keydown', init) + } + + const homeStroke = { key: 'h' } as const + const paletteStroke = { key: 'k', mod: true } as const + const captureStroke = { key: 'c', mod: true, shift: true } as const + const helpStroke = KEYBOARD_HELP_SHORTCUT.sequence[0]! + + it('matches a bare letter stroke with no modifiers held', () => { + expect(strokeMatches(keydown({ key: 'h' }), homeStroke)).toBe(true) + }) + + it('does not match a bare letter stroke while Shift is held', () => { + // The comparison is case-insensitive, so `Shift+H` arrives as `H` and used + // to navigate Home (#1968). + expect(strokeMatches(keydown({ key: 'H', shiftKey: true }), homeStroke)).toBe(false) + expect(strokeMatches(keydown({ key: 'h', shiftKey: true }), homeStroke)).toBe(false) + expect(strokeMatches(keydown({ key: 'K', ctrlKey: true, shiftKey: true }), paletteStroke)) + .toBe(false) + }) + + it('keeps a declared shift exact in both directions', () => { + expect(strokeMatches(keydown({ key: 'C', ctrlKey: true, shiftKey: true }), captureStroke)) + .toBe(true) + expect(strokeMatches(keydown({ key: 'c', ctrlKey: true }), captureStroke)).toBe(false) + }) + + it('keeps mod and alt exact over letter strokes', () => { + expect(strokeMatches(keydown({ key: 'h', altKey: true }), homeStroke)).toBe(false) + expect(strokeMatches(keydown({ key: 'h', ctrlKey: true }), homeStroke)).toBe(false) + expect(strokeMatches(keydown({ key: 'k', ctrlKey: true }), paletteStroke)).toBe(true) + expect(strokeMatches(keydown({ key: 'k', metaKey: true }), paletteStroke)).toBe(true) + expect(strokeMatches(keydown({ key: 'k' }), paletteStroke)).toBe(false) + expect(strokeMatches(keydown({ key: 'k', ctrlKey: true, altKey: true }), paletteStroke)) + .toBe(false) + }) + + it('reaches the ? help stroke on layouts that need Shift or AltGr', () => { + expect(strokeMatches(keydown({ key: '?' }), helpStroke)).toBe(true) + // The common case: `?` is the shifted character, so Shift is always down. + expect(strokeMatches(keydown({ key: '?', shiftKey: true }), helpStroke)).toBe(true) + // AltGr reports altKey, and on Windows ctrlKey with it (#1968). + expect(strokeMatches(keydown({ key: '?', altKey: true }), helpStroke)).toBe(true) + expect(strokeMatches(keydown({ key: '?', altKey: true, ctrlKey: true }), helpStroke)).toBe(true) + }) + + it('still refuses a real Ctrl or Command chord over the ? help stroke', () => { + expect(strokeMatches(keydown({ key: '?', ctrlKey: true }), helpStroke)).toBe(false) + expect(strokeMatches(keydown({ key: '?', metaKey: true }), helpStroke)).toBe(false) + }) + + it('matches every app-shell binding against the stroke it advertises', () => { + expect(APP_SHELL_SHORTCUT_BINDINGS.length).toBeGreaterThan(0) + + for (const binding of APP_SHELL_SHORTCUT_BINDINGS) { + for (const stroke of binding.sequence) { + const event = keydown({ + key: stroke.key, + ctrlKey: stroke.mod === true, + shiftKey: stroke.shift === true, + altKey: stroke.alt === true, + }) + expect({ id: binding.id, key: stroke.key, matches: strokeMatches(event, stroke) }) + .toEqual({ id: binding.id, key: stroke.key, matches: true }) + } + } + }) +}) From 9087945c125883d3dd0de615eaa6ee01d127a608 Mon Sep 17 00:00:00 2001 From: Chris0Jeky Date: Sat, 5 Sep 2026 05:11:41 +0100 Subject: [PATCH 4/5] test(shortcuts): pin the surface scan out of the typing path Asserts that an ordinary keystroke into a field never reaches the querySelectorAll plus getComputedStyle sweep, and that mod+k, the one binding allowed inside text entry, still gets the scan because the surface state is what decides whether it may open. Red on the base implementation, which scanned twice for two typed keys. Refs #1968. --- .../src/tests/components/AppShell.spec.ts | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/frontend/taskdeck-web/src/tests/components/AppShell.spec.ts b/frontend/taskdeck-web/src/tests/components/AppShell.spec.ts index fd33d95f7..258e59297 100644 --- a/frontend/taskdeck-web/src/tests/components/AppShell.spec.ts +++ b/frontend/taskdeck-web/src/tests/components/AppShell.spec.ts @@ -617,6 +617,33 @@ describe('AppShell workspace navigation and command palette', () => { expect(helpDialog()).toBeNull() }) + it('does not scan the DOM for modal surfaces on an ordinary keystroke in a field', async () => { + mountedWrapper = mountShell(document.body) + const field = document.createElement('input') + document.body.appendChild(field) + + const querySelectorAll = vi.spyOn(document, 'querySelectorAll') + const surfaceScans = () => querySelectorAll.mock.calls + .filter(([selector]) => String(selector).includes('aria-modal')) + + field.dispatchEvent(new KeyboardEvent('keydown', { key: 'a', bubbles: true })) + field.dispatchEvent(new KeyboardEvent('keydown', { key: 'h', bubbles: true })) + await waitForUi() + + // Typing pays for `isTextEntryTarget` alone; the `querySelectorAll` plus + // `getComputedStyle` sweep is only reached once an answer is needed (#1968). + expect(surfaceScans()).toHaveLength(0) + + // mod+k is allowed inside text entry, so there the surface state decides + // whether it may open and the scan is the point. + field.dispatchEvent(new KeyboardEvent('keydown', { key: 'k', ctrlKey: true, bubbles: true })) + await waitForUi() + expect(surfaceScans().length).toBeGreaterThan(0) + + querySelectorAll.mockRestore() + field.remove() + }) + it('does not navigate when Shift is held with a bare-letter binding', async () => { mountedWrapper = mountShell() From 8eb4010d218fe1c2445f0b3d65b394e6dd89538b Mon Sep 17 00:00:00 2001 From: Chris0Jeky Date: Sat, 5 Sep 2026 05:39:06 +0100 Subject: [PATCH 5/5] fix(shortcuts): require modality before a surface owns the keyboard Round 2 review fixes for #2635. HIGH. KEYBOARD_OWNING_SURFACE_SELECTOR listed a bare [role="dialog"], and CardModal keeps that role in both presentations while setting aria-modal only outside the inspector. The Paper desktop card inspector is a sticky side panel that traps nothing, so it counted as a keyboard-owning surface: with the new ownership gate, ?, mod+k and mod+shift+c went dead for as long as a card was open for reading, and every non-Escape key pressed outside the panel was stopped. The selector now requires modality: dialog[open], [role="alertdialog"], [aria-modal="true"]. This is the 2026-08-29 #1968 MEDIUM on the same selector. LOW. Ownership asked that EVERY active surface be this action's surface, so two open shell surfaces left neither ? nor mod+k able to close its own surface. It now asks that this action's surface is present and that no surface outside the shell is. Stack order is deliberately not used: a Teleport places its anchor when the shell mounts, not when the surface opens, so document order is template order whatever the user opened first. MEDIUM. The two matcher loops built their event from the stroke's own fields and could not fail. The util spec now drives the matcher from the DESCRIPTOR, the string the help surfaces print, and asserts descriptor and sequence agree. The overlay spec couples each rendered chip to formatShortcut of its ledger row. AppShell.spec adds a behavioural table over APP_SHELL_SHORTCUT_BINDINGS that presses each advertised stroke and asserts the ledger action runs. MEDIUM. The escape-stack spec built its stack with keys the gate now refuses and passed only because an unattached mount hides every surface from document.querySelectorAll. It stacks by mouse and mounts attached, and the two other specs that press keys over an open surface now attach too, with the reason named on mountShell. Refs #1968, #2636. --- .../src/components/shell/AppShell.vue | 40 +++- .../src/tests/components/AppShell.spec.ts | 200 +++++++++++++++--- .../paper/PaperShortcutsOverlay.spec.ts | 27 ++- .../src/tests/utils/keyboardShortcuts.spec.ts | 48 ++++- 4 files changed, 255 insertions(+), 60 deletions(-) diff --git a/frontend/taskdeck-web/src/components/shell/AppShell.vue b/frontend/taskdeck-web/src/components/shell/AppShell.vue index 0d9824da7..58ddf4b6f 100644 --- a/frontend/taskdeck-web/src/components/shell/AppShell.vue +++ b/frontend/taskdeck-web/src/components/shell/AppShell.vue @@ -147,9 +147,22 @@ function isTextEntryTarget(target: EventTarget | null): boolean { return target.matches(selector) || target.closest(selector) !== null } +/** + * Only a surface that declares itself MODAL owns the keyboard (#1968). + * + * A bare `[role="dialog"]` is not enough, and matching it was a live defect: + * `CardModal` keeps `role="dialog"` in both presentations but sets + * `aria-modal` only outside the inspector, so the Paper desktop card inspector + * -- a sticky side panel that traps nothing and leaves the board usable -- + * counted as a keyboard-owning surface. That made `?`, `mod+k` and + * `mod+shift+c` dead for as long as a card was open for reading, and stopped + * every non-Escape key pressed outside the panel. + * + * `dialog[open]` and `[role="alertdialog"]` stay: a native open `` is + * modal when shown as one and an alertdialog is modal by definition. + */ const KEYBOARD_OWNING_SURFACE_SELECTOR = [ 'dialog[open]', - '[role="dialog"]', '[role="alertdialog"]', '[aria-modal="true"]', ].join(', ') @@ -168,17 +181,28 @@ function activeKeyboardOwningSurfaces(): HTMLElement[] { } /** - * True when every surface currently owning the keyboard is the shell's own - * surface for this action, which is what makes `?` and `mod+k` toggles rather - * than one-way openers: the help dialog owns `?`, the command palette owns - * `mod+k`, and neither owns the other's key (#1968). + * True when this action's own surface is among the active ones and every active + * surface belongs to the shell. That is what makes `?` and `mod+k` toggles + * rather than one-way openers: the help dialog owns `?`, the command palette + * owns `mod+k`, and neither opens over the other or over anything else (#1968). + * + * Deliberately not "the topmost surface owns it". Stack order is not readable + * here: both help twins and both palettes teleport to `body`, and a `` + * places its anchor when the SHELL mounts, not when the surface opens, so + * document order is AppShell's template order whatever the user opened first. + * Asking every surface instead would deadlock a stack -- open the help dialog, + * then the topbar Search control, and neither key could close its own surface + * again. * * `navigate` and `quick-capture` name no surface, so an active surface always * wins over them: nothing behind a modal should move the route, and quick * capture would stack a second modal on the first (the #1959 class). */ -function surfacesOwnAction(surfaces: readonly HTMLElement[], action: AppShellShortcutAction): boolean { - return surfaces.every((surface) => surface.dataset.shellSurface === action.type) +function shellSurfaceOwnsAction(surfaces: readonly HTMLElement[], action: AppShellShortcutAction): boolean { + if (surfaces.length === 0) return true + + return surfaces.some((surface) => surface.dataset.shellSurface === action.type) && + surfaces.every((surface) => surface.dataset.shellSurface !== undefined) } const CHORD_TIMEOUT_MS = 1_000 @@ -284,7 +308,7 @@ function handleKeydown(event: KeyboardEvent) { binding.sequence.length === 1 && strokeMatches(event, binding.sequence[0]!) && (!textEntryTarget || binding.allowInTextEntry === true) && - surfacesOwnAction(keyboardOwningSurfaces(), binding.action), + shellSurfaceOwnsAction(keyboardOwningSurfaces(), binding.action), ) if (direct) { consumeShortcut(event) diff --git a/frontend/taskdeck-web/src/tests/components/AppShell.spec.ts b/frontend/taskdeck-web/src/tests/components/AppShell.spec.ts index 258e59297..90de261f5 100644 --- a/frontend/taskdeck-web/src/tests/components/AppShell.spec.ts +++ b/frontend/taskdeck-web/src/tests/components/AppShell.spec.ts @@ -7,6 +7,7 @@ import { type ShellKeyboardHelpControl, } from '../../composables/useShellKeyboardHelp' import { useKeyboardShortcuts } from '../../composables/useKeyboardShortcuts' +import { APP_SHELL_SHORTCUT_BINDINGS } from '../../utils/keyboardShortcuts' import type { FeatureFlags } from '../../types/feature-flags' /** @@ -39,39 +40,72 @@ const boardProbe = reactive({ filterToggles: 0, addCardClicks: 0 }) * board with no Paper dialog open), which is exactly why the shell guard is * the only thing standing between the modal and the board. */ -const BoardKeyProbe = defineComponent({ - setup() { - useKeyboardShortcuts([ - { - key: 'f', - description: 'Toggle filter panel', - action: () => { - boardProbe.filterToggles += 1 - }, +function installBoardShortcuts() { + useKeyboardShortcuts([ + { + key: 'f', + description: 'Toggle filter panel', + action: () => { + boardProbe.filterToggles += 1 + }, + }, + { + key: 'n', + description: 'New card in current column', + action: () => { + const column = document.querySelector('[data-column-id="column-1"]') + column?.querySelector('[data-action="toggle-add-card"]')?.click() + column?.querySelector('[data-action="add-card-input"]')?.focus() }, + }, + ]) +} + +function boardColumnNode() { + return h('div', { 'data-column-id': 'column-1' }, [ + h( + 'button', { - key: 'n', - description: 'New card in current column', - action: () => { - const column = document.querySelector('[data-column-id="column-1"]') - column?.querySelector('[data-action="toggle-add-card"]')?.click() - column?.querySelector('[data-action="add-card-input"]')?.focus() + 'data-action': 'toggle-add-card', + onClick: () => { + boardProbe.addCardClicks += 1 }, }, - ]) + 'Add card', + ), + h('textarea', { 'data-action': 'add-card-input' }), + ]) +} + +const BoardKeyProbe = defineComponent({ + setup() { + installBoardShortcuts() + return () => boardColumnNode() + }, +}) - return () => h('div', { 'data-column-id': 'column-1' }, [ +/** + * The Paper desktop card inspector over its board (#2635 round 2). + * + * `CardModal.vue` keeps `role="dialog"` in both presentations and sets + * `aria-modal` only outside the inspector, so at desktop widths the open card + * is a sticky side panel that traps nothing: the board behind it stays usable + * and the shell keys have to keep working. + */ +const InspectorBoardProbe = defineComponent({ + setup() { + installBoardShortcuts() + return () => h('div', [ h( - 'button', + 'div', { - 'data-action': 'toggle-add-card', - onClick: () => { - boardProbe.addCardClicks += 1 - }, + role: 'dialog', + 'aria-label': 'Edit Card', + 'data-testid': 'card-inspector', }, - 'Add card', + [h('button', { 'data-testid': 'inspector-close' }, 'Close')], ), - h('textarea', { 'data-action': 'add-card-input' }), + boardColumnNode(), ]) }, }) @@ -153,6 +187,17 @@ vi.mock('../../composables/useCaptureQueueSync', () => ({ useCaptureQueueSync: () => ({ pendingCount: { value: 0 }, syncing: { value: false }, replayQueue: vi.fn(), registerBackgroundSync: vi.fn(), refreshCount: vi.fn() }), })) +/** + * `attachTo` is load-bearing for anything that touches the modal-ownership + * guard. Vue Test Utils only puts the component into the real document when it + * is given, and the guard answers `document.querySelectorAll`, so an unattached + * mount reports no active surface and defangs the gate entirely. Every spec + * below that opens a surface and then presses a key passes `document.body`. + * + * Teleport stays stubbed throughout. The guard does not read stack order -- a + * `` places its anchor when the shell mounts, not when the surface + * opens, so document order is AppShell's template order either way. + */ function mountShell(attachTo?: HTMLElement, extraStubs: Record = {}) { return mount(AppShell, { attachTo, @@ -540,6 +585,39 @@ describe('AppShell workspace navigation and command palette', () => { expect(dialog!.contains(document.activeElement)).toBe(true) }) + it('keeps every shell key live while a non-modal card inspector is open', async () => { + mountedWrapper = mountShell(document.body, { RouterView: InspectorBoardProbe }) + const wrapper = mountedWrapper + await waitForUi() + expect(document.querySelector('[data-testid="card-inspector"]')).not.toBeNull() + + pressFromBody('?') + await waitForUi() + expect(helpDialog()).not.toBeNull() + pressFromBody('?') + await waitForUi() + expect(helpDialog()).toBeNull() + + pressFromBody('k', { ctrlKey: true }) + await waitForUi() + expect(wrapper.find('[aria-label="Command palette"]').exists()).toBe(true) + pressFromBody('k', { ctrlKey: true }) + await waitForUi() + expect(wrapper.find('[aria-label="Command palette"]').exists()).toBe(false) + + pressFromBody('C', { ctrlKey: true, shiftKey: true }) + await waitForUi() + expect(wrapper.find('[aria-label="Capture modal"]').exists()).toBe(true) + await wrapper.get('.capture-close').trigger('click') + await waitForUi() + + // And the board behind the panel is still the board: a side panel that + // traps nothing does not take the board's keys away either. + pressFromBody('f') + await waitForUi() + expect(boardProbe.filterToggles).toBe(1) + }) + it('keeps ? and Escape working on the help dialog with the board listener mounted', async () => { mountedWrapper = mountShell(document.body, { RouterView: BoardKeyProbe }) await waitForUi() @@ -644,6 +722,61 @@ describe('AppShell workspace navigation and command palette', () => { field.remove() }) + it('lets each surface of a shell stack close itself with its own key', async () => { + // Both shell surfaces open at once, which only a mouse can reach now. Each + // key still closes the surface that owns it; requiring every active surface + // to be this action's surface would deadlock the stack instead. + mountedWrapper = mountShell(document.body) + const wrapper = mountedWrapper + + await wrapper.get('[aria-label="Keyboard shortcuts help"]').trigger('click') + await wrapper.get('[aria-label^="Open command palette"]').trigger('click') + await waitForUi() + expect(helpDialog()).not.toBeNull() + expect(wrapper.find('[aria-label="Command palette"]').exists()).toBe(true) + + pressFromBody('k', { ctrlKey: true }) + await waitForUi() + expect(wrapper.find('[aria-label="Command palette"]').exists()).toBe(false) + expect(helpDialog()).not.toBeNull() + + pressFromBody('?') + await waitForUi() + expect(helpDialog()).toBeNull() + }) + + it.each(APP_SHELL_SHORTCUT_BINDINGS.map((binding) => [binding.id, binding] as const))( + 'runs the ledger action the %s row advertises', + async (_id, binding) => { + mountedWrapper = mountShell(document.body) + const wrapper = mountedWrapper + + for (const stroke of binding.sequence) { + pressFromBody(stroke.key, { + ctrlKey: stroke.mod === true, + shiftKey: stroke.shift === true, + altKey: stroke.alt === true, + }) + } + await waitForUi() + + switch (binding.action.type) { + case 'navigate': + expect(mockRouter.push).toHaveBeenCalledWith(binding.action.path) + break + case 'command-palette': + expect(wrapper.find('[aria-label="Command palette"]').exists()).toBe(true) + break + case 'quick-capture': + expect(wrapper.find('[aria-label="Capture modal"]').exists()).toBe(true) + break + case 'keyboard-help': + expect(helpDialog()).not.toBeNull() + break + } + }, + ) + it('does not navigate when Shift is held with a bare-letter binding', async () => { mountedWrapper = mountShell() @@ -657,7 +790,9 @@ describe('AppShell workspace navigation and command palette', () => { }) it('toggles the keyboard map when ? needs AltGr and leaves Alt letter combos alone', async () => { - mountedWrapper = mountShell() + // Attached: the second press has to close the help dialog THROUGH the + // surface gate, which only sees surfaces that are really in the document. + mountedWrapper = mountShell(document.body) const wrapper = mountedWrapper // AltGr reports `altKey: true`, and on Windows `ctrlKey: true` as well, so @@ -744,14 +879,19 @@ describe('AppShell workspace navigation and command palette', () => { }) it('closes only the top-most escape surface first', async () => { - mountedWrapper = mountShell() + // Attached, because the surface gate answers `document.querySelectorAll`, + // and stacked by MOUSE: the gate deliberately refuses to open the palette + // over a modal it does not own, so a keyboard-built stack is no longer a + // reachable state. Clicking a background control still is, and the escape + // stack's ordering is what this asserts. + mountedWrapper = mountShell(document.body) const wrapper = mountedWrapper - window.dispatchEvent(new KeyboardEvent('keydown', { key: '?' })) + await wrapper.get('[aria-label="Keyboard shortcuts help"]').trigger('click') await waitForUi() expect(wrapper.find('[aria-label="Keyboard shortcuts"]').exists()).toBe(true) - window.dispatchEvent(new KeyboardEvent('keydown', { key: 'k', ctrlKey: true })) + await wrapper.get('[aria-label^="Open command palette"]').trigger('click') await waitForUi() expect(wrapper.find('[aria-label="Command palette"]').exists()).toBe(true) @@ -766,7 +906,9 @@ describe('AppShell workspace navigation and command palette', () => { }) it('opens the same help surface from the routed-view seam that ? opens', async () => { - mountedWrapper = mountShell(undefined, { RouterView: ShellHelpProbe }) + // Attached: every `?` after the first has to pass the surface gate, which + // only sees surfaces that are really in the document. + mountedWrapper = mountShell(document.body, { RouterView: ShellHelpProbe }) const wrapper = mountedWrapper await waitForUi() diff --git a/frontend/taskdeck-web/src/tests/components/paper/PaperShortcutsOverlay.spec.ts b/frontend/taskdeck-web/src/tests/components/paper/PaperShortcutsOverlay.spec.ts index 651625e2b..08d330e0a 100644 --- a/frontend/taskdeck-web/src/tests/components/paper/PaperShortcutsOverlay.spec.ts +++ b/frontend/taskdeck-web/src/tests/components/paper/PaperShortcutsOverlay.spec.ts @@ -112,27 +112,26 @@ describe('PaperShortcutsOverlay', () => { * advertises can: a row whose declared stroke its own matcher rejects is * exactly the dead affordance the ledger exists to prevent. */ - it('advertises app-shell rows whose declared stroke the shipped matcher accepts', () => { + it('prints each app-shell row as the key the shell dispatches it from', () => { wrapper = mount(PaperShortcutsOverlay, { props: { visible: true }, attachTo: document.body }) - const displayedIds = new Set(Array.from( + + const printed = new Map(Array.from( teleportContent().querySelectorAll('[data-shortcut-id]'), - ).map((row) => row.dataset.shortcutId)) + ).map((row) => [ + row.dataset.shortcutId, + row.querySelector('.paper-shortcuts-overlay__row-kbd')?.textContent?.trim(), + ])) const displayedAppShellBindings = APP_SHELL_SHORTCUT_BINDINGS - .filter((binding) => displayedIds.has(binding.id)) + .filter((binding) => printed.has(binding.id)) expect(displayedAppShellBindings.length).toBeGreaterThan(0) + // Couples the chip the user reads to the ledger row the shell dispatches: + // change one side and this reddens. Asserting the stroke against an event + // built from that same stroke could not. for (const binding of displayedAppShellBindings) { - for (const stroke of binding.sequence) { - const pressed = new KeyboardEvent('keydown', { - key: stroke.key, - ctrlKey: stroke.mod === true, - shiftKey: stroke.shift === true, - altKey: stroke.alt === true, - }) - expect({ id: binding.id, key: stroke.key, matched: strokeMatches(pressed, stroke) }) - .toEqual({ id: binding.id, key: stroke.key, matched: true }) - } + expect({ id: binding.id, chip: printed.get(binding.id) }) + .toEqual({ id: binding.id, chip: formatShortcut(binding.descriptor) }) } // The overlay prints the help key in its footer rather than as a grouped diff --git a/frontend/taskdeck-web/src/tests/utils/keyboardShortcuts.spec.ts b/frontend/taskdeck-web/src/tests/utils/keyboardShortcuts.spec.ts index 097d4b392..4d7e2f33e 100644 --- a/frontend/taskdeck-web/src/tests/utils/keyboardShortcuts.spec.ts +++ b/frontend/taskdeck-web/src/tests/utils/keyboardShortcuts.spec.ts @@ -121,20 +121,50 @@ describe('strokeMatches', () => { expect(strokeMatches(keydown({ key: '?', metaKey: true }), helpStroke)).toBe(false) }) - it('matches every app-shell binding against the stroke it advertises', () => { + /** + * Building the event out of the stroke's own fields would make this pass for + * any stroke at all. The descriptor is the independent side: it is the string + * the help surfaces PRINT, so pressing what the user is told to press has to + * reach the stroke the shell listens for. A row whose descriptor and sequence + * drift apart is the dead affordance the ledger exists to prevent. + */ + function strokesFromDescriptor(descriptor: string) { + return descriptor.trim().split(/\s+/).map((chord) => { + const tokens = chord.split('+').map((token) => token.toLowerCase()) + const modifiers = ['mod', 'shift', 'alt'] + return { + key: tokens.filter((token) => !modifiers.includes(token)).join('+'), + ...(tokens.includes('mod') ? { mod: true } : {}), + ...(tokens.includes('shift') ? { shift: true } : {}), + ...(tokens.includes('alt') ? { alt: true } : {}), + } + }) + } + + it('presses what each app-shell row prints and reaches the stroke it dispatches', () => { expect(APP_SHELL_SHORTCUT_BINDINGS.length).toBeGreaterThan(0) for (const binding of APP_SHELL_SHORTCUT_BINDINGS) { - for (const stroke of binding.sequence) { + const printed = strokesFromDescriptor(binding.descriptor) + const declared = binding.sequence.map((stroke) => ({ + key: stroke.key.toLowerCase(), + ...(stroke.mod ? { mod: true } : {}), + ...(stroke.shift ? { shift: true } : {}), + ...(stroke.alt ? { alt: true } : {}), + })) + expect({ id: binding.id, strokes: declared }).toEqual({ id: binding.id, strokes: printed }) + + binding.sequence.forEach((stroke, index) => { + const asPrinted = printed[index]! const event = keydown({ - key: stroke.key, - ctrlKey: stroke.mod === true, - shiftKey: stroke.shift === true, - altKey: stroke.alt === true, + key: asPrinted.key, + ctrlKey: asPrinted.mod === true, + shiftKey: asPrinted.shift === true, + altKey: asPrinted.alt === true, }) - expect({ id: binding.id, key: stroke.key, matches: strokeMatches(event, stroke) }) - .toEqual({ id: binding.id, key: stroke.key, matches: true }) - } + expect({ id: binding.id, step: index, reaches: strokeMatches(event, stroke) }) + .toEqual({ id: binding.id, step: index, reaches: true }) + }) } }) })