fix(shortcuts): track active sequences explicitly to prevent misfires
Replace the global sequenceBuffer (which only tracked key count) with per-candidate ActiveSequence tracking. This fixes two bugs: - Sequences with different prefixes could misfire once any sequence started, since only buffer length was checked, not which sequence was being matched. - Single-key shortcuts could fire during an in-progress sequence, preempting sequence completion depending on binding iteration order.
This commit is contained in:
parent
f8763d812e
commit
da9cb87448
|
|
@ -157,8 +157,6 @@
|
|||
"workbox-cli": "7.4.0"
|
||||
},
|
||||
"pnpm": {
|
||||
"patchedDependencies": {
|
||||
},
|
||||
"onlyBuiltDependencies": [
|
||||
"@parcel/watcher",
|
||||
"@sentry/cli",
|
||||
|
|
|
|||
|
|
@ -1,7 +1,7 @@
|
|||
import {describe, it, expect, vi} from 'vitest'
|
||||
import {describe, it, expect, vi, beforeEach, afterEach} from 'vitest'
|
||||
|
||||
import * as appleDevice from '@/helpers/isAppleDevice'
|
||||
import {parseKey, matchesKey, eventToShortcutString, isFormField} from './shortcut'
|
||||
import {parseKey, matchesKey, eventToShortcutString, isFormField, install, uninstall} from './shortcut'
|
||||
|
||||
// Helper to create a partial KeyboardEvent with sensible defaults
|
||||
function makeEvent(overrides: Partial<KeyboardEvent> = {}): KeyboardEvent {
|
||||
|
|
@ -328,3 +328,101 @@ describe('isFormField', () => {
|
|||
expect(isFormField(null)).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
// --- install / uninstall integration tests ---
|
||||
|
||||
function fireKeydown(code: string, mods: Partial<Pick<KeyboardEvent, 'ctrlKey' | 'altKey' | 'shiftKey' | 'metaKey'>> = {}) {
|
||||
const event = new KeyboardEvent('keydown', {
|
||||
code,
|
||||
key: code,
|
||||
bubbles: true,
|
||||
cancelable: true,
|
||||
...mods,
|
||||
})
|
||||
document.dispatchEvent(event)
|
||||
return event
|
||||
}
|
||||
|
||||
describe('install / uninstall (sequence handling)', () => {
|
||||
let elA: HTMLElement
|
||||
let elB: HTMLElement
|
||||
let clickA: ReturnType<typeof vi.fn>
|
||||
let clickB: ReturnType<typeof vi.fn>
|
||||
|
||||
beforeEach(() => {
|
||||
elA = document.createElement('button')
|
||||
elB = document.createElement('button')
|
||||
clickA = vi.fn()
|
||||
clickB = vi.fn()
|
||||
elA.addEventListener('click', clickA)
|
||||
elB.addEventListener('click', clickB)
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
uninstall(elA)
|
||||
uninstall(elB)
|
||||
})
|
||||
|
||||
it('should not fire single-key shortcut while a sequence is in progress', () => {
|
||||
// KeyA is standalone, KeyG KeyA is a sequence
|
||||
install(elA, 'KeyA')
|
||||
install(elB, 'KeyG KeyA')
|
||||
|
||||
fireKeydown('KeyG') // starts sequence
|
||||
fireKeydown('KeyA') // should complete sequence, NOT fire standalone
|
||||
|
||||
expect(clickB).toHaveBeenCalledTimes(1)
|
||||
expect(clickA).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('should fire single-key shortcut when no sequence is in progress', () => {
|
||||
install(elA, 'KeyA')
|
||||
install(elB, 'KeyG KeyO')
|
||||
|
||||
fireKeydown('KeyA')
|
||||
|
||||
expect(clickA).toHaveBeenCalledTimes(1)
|
||||
expect(clickB).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('should not misfire sequences with different prefixes', () => {
|
||||
// KeyG KeyO and KeyH KeyO — pressing KeyH then KeyO should not fire KeyG KeyO
|
||||
install(elA, 'KeyG KeyO')
|
||||
install(elB, 'KeyH KeyO')
|
||||
|
||||
fireKeydown('KeyH')
|
||||
fireKeydown('KeyO')
|
||||
|
||||
expect(clickB).toHaveBeenCalledTimes(1)
|
||||
expect(clickA).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('should reset and allow single-key shortcuts after sequence timeout', async () => {
|
||||
install(elA, 'KeyA')
|
||||
install(elB, 'KeyG KeyA')
|
||||
|
||||
fireKeydown('KeyG') // starts sequence
|
||||
|
||||
// Wait for timeout (1500ms)
|
||||
await new Promise(r => setTimeout(r, 1600))
|
||||
|
||||
fireKeydown('KeyA') // sequence timed out, should fire standalone
|
||||
|
||||
expect(clickA).toHaveBeenCalledTimes(1)
|
||||
expect(clickB).not.toHaveBeenCalled()
|
||||
}, 3000)
|
||||
|
||||
it('should reset when a non-matching key is pressed during a sequence', () => {
|
||||
install(elA, 'KeyA')
|
||||
install(elB, 'KeyG KeyO')
|
||||
|
||||
fireKeydown('KeyG') // starts sequence
|
||||
fireKeydown('KeyX') // doesn't match any active sequence — resets
|
||||
|
||||
// Now KeyA should work as standalone
|
||||
fireKeydown('KeyA')
|
||||
|
||||
expect(clickA).toHaveBeenCalledTimes(1)
|
||||
expect(clickB).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -95,11 +95,18 @@ interface Binding {
|
|||
|
||||
const bindings = new Set<Binding>()
|
||||
const elementBindings = new WeakMap<HTMLElement, Binding>()
|
||||
let sequenceBuffer: string[] = []
|
||||
|
||||
interface ActiveSequence {
|
||||
binding: Binding
|
||||
sequenceIndex: number
|
||||
step: number
|
||||
}
|
||||
|
||||
let activeSequences: ActiveSequence[] = []
|
||||
let sequenceTimer: ReturnType<typeof setTimeout> | null = null
|
||||
|
||||
function resetSequence() {
|
||||
sequenceBuffer = []
|
||||
activeSequences = []
|
||||
if (sequenceTimer !== null) {
|
||||
clearTimeout(sequenceTimer)
|
||||
sequenceTimer = null
|
||||
|
|
@ -115,45 +122,62 @@ function globalKeydownHandler(event: KeyboardEvent) {
|
|||
if (target?.shadowRoot) return
|
||||
if (isFormField(target)) return
|
||||
|
||||
for (const binding of bindings) {
|
||||
for (const sequence of binding.keys) {
|
||||
if (sequence.length === 1) {
|
||||
// Single-key shortcut
|
||||
if (matchesKey(event, sequence[0])) {
|
||||
// If sequences are in progress, only advance those — skip single-key shortcuts
|
||||
if (activeSequences.length > 0) {
|
||||
const stillActive: ActiveSequence[] = []
|
||||
|
||||
for (const active of activeSequences) {
|
||||
const sequence = active.binding.keys[active.sequenceIndex]
|
||||
if (matchesKey(event, sequence[active.step])) {
|
||||
if (active.step + 1 === sequence.length) {
|
||||
// Sequence complete
|
||||
event.preventDefault()
|
||||
binding.el.click()
|
||||
active.binding.el.click()
|
||||
resetSequence()
|
||||
return
|
||||
}
|
||||
} else {
|
||||
// Sequence shortcut (e.g. 'KeyG KeyO')
|
||||
const stepIndex = sequenceBuffer.length
|
||||
if (stepIndex < sequence.length && matchesKey(event, sequence[stepIndex])) {
|
||||
sequenceBuffer.push(event.code)
|
||||
stillActive.push({...active, step: active.step + 1})
|
||||
}
|
||||
}
|
||||
|
||||
if (sequenceTimer !== null) {
|
||||
clearTimeout(sequenceTimer)
|
||||
}
|
||||
sequenceTimer = setTimeout(resetSequence, SEQUENCE_TIMEOUT)
|
||||
if (stillActive.length > 0) {
|
||||
activeSequences = stillActive
|
||||
if (sequenceTimer !== null) clearTimeout(sequenceTimer)
|
||||
sequenceTimer = setTimeout(resetSequence, SEQUENCE_TIMEOUT)
|
||||
event.preventDefault()
|
||||
return
|
||||
}
|
||||
|
||||
if (sequenceBuffer.length === sequence.length) {
|
||||
event.preventDefault()
|
||||
binding.el.click()
|
||||
resetSequence()
|
||||
return
|
||||
}
|
||||
// No active sequences matched this key — reset and fall through
|
||||
resetSequence()
|
||||
}
|
||||
|
||||
// Partial match — consume the event
|
||||
event.preventDefault()
|
||||
return
|
||||
}
|
||||
// Check single-key shortcuts
|
||||
for (const binding of bindings) {
|
||||
for (const sequence of binding.keys) {
|
||||
if (sequence.length === 1 && matchesKey(event, sequence[0])) {
|
||||
event.preventDefault()
|
||||
binding.el.click()
|
||||
return
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// No match for any sequence step — reset
|
||||
if (sequenceBuffer.length > 0) {
|
||||
resetSequence()
|
||||
// Try to start new sequences
|
||||
const newActive: ActiveSequence[] = []
|
||||
for (const binding of bindings) {
|
||||
for (let i = 0; i < binding.keys.length; i++) {
|
||||
const sequence = binding.keys[i]
|
||||
if (sequence.length > 1 && matchesKey(event, sequence[0])) {
|
||||
newActive.push({binding, sequenceIndex: i, step: 1})
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if (newActive.length > 0) {
|
||||
activeSequences = newActive
|
||||
sequenceTimer = setTimeout(resetSequence, SEQUENCE_TIMEOUT)
|
||||
event.preventDefault()
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue