Skip to content
12 changes: 12 additions & 0 deletions docs/plans/2026-09-25-shortcut-recorder-release.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
# Shortcut recorder releases capture on outside click and window blur (#1272)

## Evidence
`CommandKeybindingsRow.tsx`: while recording, a capture-phase window keydown listener calls preventDefault and stopPropagation on every key. It releases only on Escape or a finished chord. `keybindingFromEvent` accepts a bare letter.

Sequence: click Add, then click the Settings search box, then type `t`. The key is swallowed and saved as that command's shortcut. The sibling recorders (`HotkeyInput`, `MouseButtonInput`) already release on an outside mousedown and on blur.

## Change
While recording, a mousedown outside the recorder's own button, or a window blur, ends the recording. This matches HotkeyInput.

## Tests
A renderer test on the real component with the real store. The first test clicks elsewhere and asserts that recording ended and that a typed `t` is neither swallowed nor saved. The second blurs the window. Both are red on main.
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
import { fireEvent, render, screen } from '@testing-library/react'
import { afterEach, expect, it } from 'vitest'

import { useAppStore } from '@renderer/app-state/store'
import { CommandKeybindingsRow } from './CommandKeybindingsRow'

// #1272 (C4 hunt): the recorder captured every key on the window until
// Escape or a finished chord. Clicking elsewhere (the Settings search box) or
// leaving the window did not release it, so the next bare letter typed was
// swallowed AND saved as that command's shortcut.
const original = useAppStore.getState()
afterEach(() => { useAppStore.setState(original, true) })

function startRecording() {
render(<CommandKeybindingsRow />)
const add = screen.getAllByRole('button', { name: 'Add' })[0]!
fireEvent.click(add)
expect(screen.getByRole('button', { name: 'Press keys… (Esc)' })).toBeInTheDocument()
}

it('stops recording when the user clicks somewhere else', () => {
startRecording()
const elsewhere = document.createElement('input')
document.body.appendChild(elsewhere)
fireEvent.mouseDown(elsewhere)
expect(screen.queryByRole('button', { name: 'Press keys… (Esc)' })).toBeNull()
// The next key is ordinary typing again: not swallowed, not saved.
const before = JSON.stringify(useAppStore.getState().settings.commandKeybindingOverrides ?? {})
const typed = fireEvent.keyDown(elsewhere, { key: 't', code: 'KeyT' })
expect(typed).toBe(true)
expect(JSON.stringify(useAppStore.getState().settings.commandKeybindingOverrides ?? {})).toBe(before)
elsewhere.remove()
})

it('stops recording when the window loses focus', () => {
startRecording()
fireEvent.blur(window)
expect(screen.queryByRole('button', { name: 'Press keys… (Esc)' })).toBeNull()
})

// #1308 review: the edges of the release rule.
// The scroller is the target for both its scrollbar and its own flex gaps;
// only the scrollbar, past the client box, keeps recording (#1308 round 2).
function listGeometry(): HTMLElement {
const list = document.querySelector<HTMLElement>('[data-shortcut-list]')!
list.getBoundingClientRect = () => ({ left: 0, top: 0, right: 400, bottom: 420, width: 400, height: 420, x: 0, y: 0, toJSON: () => ({}) })
Object.defineProperty(list, 'clientWidth', { configurable: true, value: 388 })
Object.defineProperty(list, 'clientHeight', { configurable: true, value: 420 })
return list
}

it('keeps recording while the user drags the list scrollbar', () => {
startRecording()
fireEvent.mouseDown(listGeometry(), { clientX: 395, clientY: 100 })
expect(screen.getByRole('button', { name: 'Press keys… (Esc)' })).toBeInTheDocument()
})

it('ends recording on a click in the gap between category blocks', () => {
startRecording()
fireEvent.mouseDown(listGeometry(), { clientX: 120, clientY: 20 })
expect(screen.queryByRole('button', { name: 'Press keys… (Esc)' })).toBeNull()
})

it('ends recording on a click on another row', () => {
startRecording()
const row = document.querySelector('[data-shortcut-list] [data-shortcut-recorder]')!.parentElement!.parentElement!
fireEvent.mouseDown(row)
expect(screen.queryByRole('button', { name: 'Press keys… (Esc)' })).toBeNull()
})

it('lets the recorder button toggle recording off itself', () => {
startRecording()
const recorder = screen.getByRole('button', { name: 'Press keys… (Esc)' })
fireEvent.mouseDown(recorder)
fireEvent.click(recorder)
expect(screen.queryByRole('button', { name: 'Press keys… (Esc)' })).toBeNull()
})

it('ends recording even when the clicked control stops propagation', () => {
startRecording()
const stubborn = document.createElement('button')
stubborn.addEventListener('mousedown', event => event.stopPropagation())
document.body.appendChild(stubborn)
fireEvent.mouseDown(stubborn)
expect(screen.queryByRole('button', { name: 'Press keys… (Esc)' })).toBeNull()
stubborn.remove()
})

51 changes: 50 additions & 1 deletion src/renderer/src/features/settings/ui/CommandKeybindingsRow.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -395,6 +395,39 @@ export function CommandKeybindingsRow() {
return () => window.removeEventListener('keydown', onKeyDown, true)
}, [capturingFor, contextForCommandId, effective, effectiveAsDefaults, settings.dictationShortcut, commit])

// The recording owns every key, so it must end the moment the user's
// attention does (#1272, C4 hunt): a click anywhere but the recorder's own
// button, or the window losing focus. Escape alone was not enough, because
// clicking the Settings search box and typing "t" silently bound "T" to the
// command. The outside-click half matches HotkeyInput and MouseButtonInput;
// ending on blur is stricter than HotkeyInput (which only clears held
// modifiers), because a recording left armed across an app switch is exactly
// how a stray key got saved.
//
// Grabbing the LIST's scrollbar is not leaving (#1308 review B): the list is
// 134 rows in a 420 px scroller, and a mousedown on its scrollbar targets the
// scroller element itself, which is not inside any recorder button. The
// scroller is also the target for its own flex gaps (8 px strips between
// category blocks, #1308 round 2), so only a press PAST its client box, on
// the scrollbar, is exempt; a gap or a row still ends recording.
// Capture phase, so a control that stops propagation cannot keep it armed.
useEffect(() => {
if (!capturingFor) return
const release = () => setCapturingFor(null)
const onMouseDown = (event: MouseEvent) => {
const target = event.target as Element | null
if (target?.closest?.('[data-shortcut-recorder]')) return
if (target instanceof HTMLElement && target.matches('[data-shortcut-list]') && onScrollbar(target, event)) return
release()
}
window.addEventListener('mousedown', onMouseDown, true)
window.addEventListener('blur', release)
return () => {
window.removeEventListener('mousedown', onMouseDown, true)
window.removeEventListener('blur', release)
}
}, [capturingFor])

/**
* The ONLY override path. Removes the binding from every command that owns
* it and installs it on the requester, in ONE settings write — so the store
Expand Down Expand Up @@ -455,7 +488,9 @@ export function CommandKeybindingsRow() {
</div>
) : null}

<div className="flex max-h-[420px] flex-col gap-2 overflow-auto">
{/* data-shortcut-list: a mousedown ON this element is its scrollbar,
which must not end a recording (#1308 review B). */}
<div data-shortcut-list="" className="flex max-h-[420px] flex-col gap-2 overflow-auto">
{/* Column header lives INSIDE the scroll container, and sticks.
Outside it, the header sits in a box that is not narrowed by the
scrollbar while the rows below it are — so on any platform with
Expand Down Expand Up @@ -506,6 +541,9 @@ export function CommandKeybindingsRow() {
)}

<button
// Clicks on the recorder itself toggle it (below) and must
// not count as "clicked elsewhere".
data-shortcut-recorder=""
onClick={() => {
setConflict(null)
setCapturingFor(capturingFor === row.id ? null : row.id)
Expand Down Expand Up @@ -642,3 +680,14 @@ function commandLabel(commandId: string): string {
if (!command) return commandId
return typeof command.title === 'function' ? commandId : command.title
}

/** Did this mousedown land on the element's own scrollbar? The scrollbar sits
* outside the client box (clientWidth/clientHeight exclude it), so a press
* past either edge of the client area is on it. */
function onScrollbar(element: HTMLElement, event: MouseEvent): boolean {
const rect = element.getBoundingClientRect()
const x = event.clientX - rect.left - element.clientLeft
const y = event.clientY - rect.top - element.clientTop
return x >= element.clientWidth || y >= element.clientHeight
}

Loading