diff --git a/packages/keystrokes/readme.md b/packages/keystrokes/readme.md
index db22166..29c14d5 100644
--- a/packages/keystrokes/readme.md
+++ b/packages/keystrokes/readme.md
@@ -15,11 +15,20 @@
-
-
-
` key aliases
+
+Every key event carries `aliases: ['@' + event.code]`, so a binding can name a
+physical key rather than the character it produces. This is what makes a shortcut
+on a punctuation key survive a keyboard-layout change. Related upstream requests:
+[#33](https://github.com/RobertWHurst/Keystrokes/issues/33),
+[#58](https://github.com/RobertWHurst/Keystrokes/issues/58). *Upstreamable.*
+
+### 2. Longest combo wins
+
+When a keypress completes several combos at once, `_handleKeyPress` executes only
+those whose `sequenceLength` is the largest among them, and `_handleKeyRelease`
+releases them via `_keyCombosPressedByKey`.
+
+The motivation is that shortcuts are frequently sub-combos of one another: in
+WEBKNOSSOS `b` sets a branch point while `control + k, b` switches to the brush
+tool. Without this, completing the longer combo also fires the shorter one. Only
+the most specific match should win. *Offered upstream as a PR.*
+
+Invariant to preserve: **a longer combo suppresses shorter ones for exactly as
+long as it is physically held, and not one event longer.** Because suppression is
+global, a combo that stays marked as pressed disables every shorter combo for the
+rest of the session — which is what made this a source of hard-to-diagnose bugs.
+The recovery path for that is `releaseAllKeys()`, via `forceRelease` in
+`key-combo-state.ts`.
+
+### 3. Keys are tracked by physical key, not by label
+
+`KeyEvent.identity` (set to `event.code` by the browser bindings) is the id used
+for `_activeKeyPresses`, `_activeKeyMap` and `_keyCombosPressedByKey`.
+
+`event.key` cannot serve that purpose because the label depends on the modifiers
+held at the time: pressing shift+slash reports `?` on keydown and, if shift is
+released first, `/` on keyup. Tracking by label therefore strands the `?` entry
+forever — and since combo matching is positional, one stranded key stops *every*
+single-key combo from matching until the page reloads.
+
+Also covers [#64](https://github.com/RobertWHurst/Keystrokes/issues/64)
+(`Cannot read properties of undefined (reading 'toLowerCase')`): a keydown with
+no `key` is ignored, while a keyup with no `key` is still resolved through
+`identity` and released under the label seen at keydown. Note the patch suggested
+in that issue — returning early from both handlers — is unsafe: skipping a keyup
+is exactly what strands a key. *Upstreamable.*
+
+### 4. Partially typed combos expire
+
+`sequenceTimeout` governs the whole sequence, not just the release window. Once
+`control + k` has been typed and released, upstream leaves the combo armed
+indefinitely, so whichever single key is pressed next — however much later —
+completes it instead of firing its own handler. See `_sequenceAdvancedAt` in
+`key-combo-state.ts`. *Upstreamable.*
+
+### 5. `releaseAllKeys()`
+
+Public on both the instance and the module, and used internally whenever the
+environment goes inactive. Consumers need a supported way to resynchronise after
+keyups that never arrived — a page regaining focus, or a modal that stopped
+propagation of key events. Related: [#49](https://github.com/RobertWHurst/Keystrokes/issues/49).
+*Upstreamable.*
+
+### 6. Broader recovery in the browser bindings
+
+- `activeKeyEvents` is keyed by `event.code` rather than `event.key`, so it does
+ not leak an entry every time a key's label changes mid-press.
+- Held keys are released on `pagehide` and on `visibilitychange` (hidden) as well
+ as `blur`, since `blur` does not cover every way a page stops receiving keys.
+- Keydowns that cannot have a matching keyup are ignored: `isComposing`, and the
+ `Process` / `Unidentified` placeholders emitted by IMEs and some autofill
+ implementations. Keyups are **never** filtered — they only remove state, and
+ dropping one is what strands a key. Likely also covers
+ [#50](https://github.com/RobertWHurst/Keystrokes/issues/50) and
+ [#55](https://github.com/RobertWHurst/Keystrokes/issues/55). *Upstreamable.*
+
+### Known behaviour, deliberately unchanged
+
+Combo matching is positional: each unit is only searched in a window starting at
+the current offset into the held-key list. As a result, **while any unrelated key
+is held, no single-key combo matches at all** — holding space and tapping `b`
+does nothing. This is intended for WEBKNOSSOS. If it ever needs revisiting, see
+`NOTE[held-key-blocks-single-key-combos]` in `key-combo-state.ts`.
+
+## Releasing this fork
+
+Downstream projects install the built artifacts from the orphan `dist/keystrokes`
+branch, not from npm:
+
+```json
+"@rwh/keystrokes": "github:scalableminds/Keystrokes#dist/keystrokes"
+```
+
+A separate branch is needed because Yarn and npm resolve a git dependency by
+looking for `package.json` at the repository root, and this repo is a pnpm
+monorepo whose root package is `@rwh/keystrokes-monorepo`.
+
+### Releasing a merged change
+
+1. Make your `src/` changes on a feature branch and open a PR against `main`.
+ CI runs lint and tests on pull requests.
+2. Once it is merged, run `scripts/update-dist.sh` from `main` (requires `pnpm`).
+ It builds the package and force-pushes `package.json` + `dist/` to
+ `dist/keystrokes`.
+3. In the downstream project, run `yarn install` and commit the lockfile change.
+
+Step 3 is not optional: the lockfile pins a commit hash on a force-pushed branch,
+so without refreshing it the consumer keeps the old build and the change silently
+does not ship.
+
+### Getting a change reviewed downstream before it is merged
+
+`dist/keystrokes` is what *every* consumer resolves, so publishing unreviewed work
+there ships it to anyone who runs `yarn install`. `update-dist.sh` refuses to
+publish it from a branch other than `main` for that reason (override with
+`ALLOW_UNMERGED=1` if you really mean to).
+
+Publish the review build to its own dist branch instead, and pin the commit the
+script prints:
+
+```sh
+./scripts/update-dist.sh dist/my-feature
+```
+
+```json
+"@rwh/keystrokes": "github:scalableminds/Keystrokes#"
+```
+
+Pin the commit rather than the branch name — dist branches are force-pushed, so a
+branch reference can change under a reviewer mid-review.
+
+When the source PR merges, publish to `dist/keystrokes` as usual and point the
+downstream project back at `#dist/keystrokes` before merging it.
+
## Help Welcome
-If you want to support this project by throwing be some coffee money It's
+If you want to support this project by throwing me some coffee money It's
greatly appreciated.
[](https://github.com/sponsors/RobertWHurst)
diff --git a/packages/keystrokes/src/browser-bindings.ts b/packages/keystrokes/src/browser-bindings.ts
index e8d2000..edbe8f4 100644
--- a/packages/keystrokes/src/browser-bindings.ts
+++ b/packages/keystrokes/src/browser-bindings.ts
@@ -80,12 +80,24 @@ const maybeHandleMacOsCommandKeyReleased = (event: KeyboardEvent) => {
const activeKeyEvents = new Map()
+// Keyed by the physical key rather than the reported label as this not stable. See README.
+const keyEventIdentity = (event: KeyboardEvent) => event.code || event.key
+
+// Composition keydowns (IME) and the placeholder keys some autofill
+// implementations emit never receive a matching keyup, so tracking them could
+// only ever strand a key. Note we never filter keyups: those can only ever
+// remove state, and dropping one is what strands a key in the first place.
+const cannotBeReleased = (event: KeyboardEvent) =>
+ event.isComposing === true ||
+ event.key === 'Process' ||
+ event.key === 'Unidentified'
+
const addActiveKeyEvent = (event: KeyboardEvent) => {
- activeKeyEvents.set(event.key, event)
+ activeKeyEvents.set(keyEventIdentity(event), event)
}
const removeActiveKeyEvent = (event: KeyboardEvent) => {
- activeKeyEvents.delete(event.key)
+ activeKeyEvents.delete(keyEventIdentity(event))
}
const dispatchKeyUpForAllActiveKeys = () => {
@@ -118,8 +130,24 @@ export const browserOnInactiveBinder: OnActiveEventBinder = (handler) => {
handler()
}
+ // blur does not cover every way a page can stop receiving key events, and any
+ // keyup delivered elsewhere in the meantime is lost.
+ const visibilityHandler = () => {
+ const doc = getDoc() as Partial
+ if (doc.visibilityState === 'hidden') handlerWrapper()
+ }
+
addEventListener('blur', handlerWrapper)
- return () => removeEventListener('blur', handlerWrapper)
+ addEventListener('pagehide', handlerWrapper)
+ // visibilitychange is fired at the document, but it bubbles, so listening on
+ // the window keeps the document listener order untouched.
+ addEventListener('visibilitychange', visibilityHandler)
+
+ return () => {
+ removeEventListener('blur', handlerWrapper)
+ removeEventListener('pagehide', handlerWrapper)
+ removeEventListener('visibilitychange', visibilityHandler)
+ }
} catch {}
}
@@ -129,12 +157,15 @@ export const browserOnKeyPressedBinder: OnKeyEventBinder<
> = (handler) => {
try {
const handlerWrapper = (e: KeyboardEvent) => {
+ if (cannotBeReleased(e)) return
+
addActiveKeyEvent(e)
maybeHandleMacOsCommandKeyPressed(e)
handler({
key: e.key,
aliases: [`@${e.code}`],
+ identity: e.code || undefined,
originalEvent: e,
composedPath: () => e.composedPath(),
preventDefault: () => e.preventDefault(),
@@ -157,6 +188,7 @@ export const browserOnKeyReleasedBinder: OnKeyEventBinder<
handler({
key: e.key,
aliases: [`@${e.code}`],
+ identity: e.code || undefined,
originalEvent: e,
composedPath: () => e.composedPath(),
preventDefault: () => e.preventDefault(),
diff --git a/packages/keystrokes/src/handler-state.ts b/packages/keystrokes/src/handler-state.ts
index d9db721..221ae56 100644
--- a/packages/keystrokes/src/handler-state.ts
+++ b/packages/keystrokes/src/handler-state.ts
@@ -1,6 +1,12 @@
export type KeyEvent = KeyEventProps & {
key: string
aliases?: string[]
+ // A stable id for the physical key, used to track which keys are held. `key`
+ // cannot serve that purpose because the label a browser reports depends on the
+ // modifiers held at the time: shift+slash reports "?" on keydown and, if shift
+ // is released first, "/" on keyup. The browser bindings pass event.code here.
+ // Falls back to `key` when absent, which keeps non-browser bindings working.
+ identity?: string
originalEvent?: OriginalEvent
}
diff --git a/packages/keystrokes/src/index.ts b/packages/keystrokes/src/index.ts
index 989beb0..a3009dd 100644
--- a/packages/keystrokes/src/index.ts
+++ b/packages/keystrokes/src/index.ts
@@ -73,6 +73,10 @@ export const checkKey: typeof globalKeystrokes.checkKey = (...args) =>
export const checkKeyCombo: typeof globalKeystrokes.checkKeyCombo = (...args) =>
getGlobalKeystrokes().checkKeyCombo(...args)
+export const releaseAllKeys: typeof globalKeystrokes.releaseAllKeys = (
+ ...args
+) => getGlobalKeystrokes().releaseAllKeys(...args)
+
export const normalizeKeyCombo = KeyComboState.normalizeKeyCombo
export const stringifyKeyCombo = KeyComboState.stringifyKeyCombo
export const parseKeyCombo = KeyComboState.parseKeyCombo
diff --git a/packages/keystrokes/src/key-combo-state.ts b/packages/keystrokes/src/key-combo-state.ts
index e70c5fb..f3a2725 100644
--- a/packages/keystrokes/src/key-combo-state.ts
+++ b/packages/keystrokes/src/key-combo-state.ts
@@ -130,7 +130,7 @@ export class KeyComboState {
}
get sequenceLength() {
- return this._parsedKeyCombo.length;
+ return this._parsedKeyCombo.length
}
private _normalizedKeyCombo: string
@@ -145,6 +145,7 @@ export class KeyComboState {
>
private _movingToNextSequenceAt: number
private _sequenceIndex: number
+ private _sequenceAdvancedAt: number
private _unitIndex: number
private _lastActiveKeyPresses: KeyPress[][]
private _lastActiveKeyCount: number
@@ -167,12 +168,50 @@ export class KeyComboState {
this._keyComboEventMapper = keyComboEventMapper
this._movingToNextSequenceAt = 0
this._sequenceIndex = 0
+ this._sequenceAdvancedAt = 0
this._unitIndex = 0
this._lastActiveKeyPresses = []
this._lastActiveKeyCount = 0
this._isPressedWithFinalUnit = null
}
+ /**
+ * Releases the combo if it is pressed and abandons any progress through its
+ * sequence.
+ *
+ * Both halves matter. A combo waiting part way through its sequence — "control
+ * + k" typed but not yet completed — is armed without being pressed, so nothing
+ * in the normal release path touches it, and it would otherwise swallow whatever
+ * key is pressed next, however much later.
+ */
+ forceRelease(event?: KeyEvent) {
+ if (this._isPressedWithFinalUnit) {
+ // By this point updateState has already run and cleared _lastActiveKeyPresses,
+ // so the release event is built from the event that triggered the recovery.
+ if (event) {
+ this._handlerState.executeReleased(
+ this._wrapEvent(this._lastActiveKeyPresses, {
+ key: event.key,
+ aliases: new Set(event.aliases),
+ identity: event.identity,
+ event,
+ }),
+ )
+ }
+ this._isPressedWithFinalUnit = null
+ }
+
+ this._resetProgress()
+ }
+
+ private _resetProgress() {
+ this._movingToNextSequenceAt = 0
+ this._sequenceIndex = 0
+ this._sequenceAdvancedAt = 0
+ this._unitIndex = 0
+ this._lastActiveKeyPresses.length = 0
+ }
+
isOwnHandler(
handler: Handler<
KeyComboEvent
@@ -220,15 +259,8 @@ export class KeyComboState {
const hasReleasedKeys = activeKeysCount < this._lastActiveKeyCount
this._lastActiveKeyCount = activeKeysCount
- const sequence = this._parsedKeyCombo[this._sequenceIndex]
- const previousUnits = sequence.slice(0, this._unitIndex)
- const remainingUnits = sequence.slice(this._unitIndex)
-
const reset = () => {
- this._movingToNextSequenceAt = 0
- this._sequenceIndex = 0
- this._unitIndex = 0
- this._lastActiveKeyPresses.length = 0
+ this._resetProgress()
// In the case of key combos that are used by checkKeyCombo, we need to
// clear the final unit for it because the executeReleased will not be
@@ -238,6 +270,21 @@ export class KeyComboState {
}
}
+ // A partially typed combo must not stay armed indefinitely. Without this, once
+ // the user has typed "control + k" the combo waits forever, and whichever
+ // single key they press next — minutes later — completes it instead of firing
+ // its own handler.
+ if (
+ this._sequenceAdvancedAt !== 0 &&
+ this._sequenceAdvancedAt + sequenceTimeout < Date.now()
+ ) {
+ reset()
+ }
+
+ const sequence = this._parsedKeyCombo[this._sequenceIndex]
+ const previousUnits = sequence.slice(0, this._unitIndex)
+ const remainingUnits = sequence.slice(this._unitIndex)
+
let activeKeyIndex = 0
// if we do not have new keys pressed, and we are not advancing to the next
@@ -250,10 +297,18 @@ export class KeyComboState {
if (activeKeysCount !== 0) return
this._movingToNextSequenceAt = 0
this._sequenceIndex += 1
+ this._sequenceAdvancedAt = Date.now()
this._unitIndex = 0
return
}
+ // NOTE[held-key-blocks-single-key-combos]: matching below is positional — each
+ // unit is only searched in the window starting at activeKeyIndex. Consequence:
+ // while any unrelated key is held, no single key combo matches at all. This is
+ // intended for now (`b` must not fire while space is held). If it is ever
+ // deemed unwanted, this is the place to change: scan the whole activeKeyPresses
+ // list and track which entries have already been consumed.
+
// go through each each previous unit. If any are no longer pressed then
// we reset to the beginning of the combo.
for (const previousUnit of previousUnits) {
@@ -331,6 +386,9 @@ export class KeyComboState {
// Setting the final unit marks the combo as active. It also allows for
// something to match key repeat against.
this._isPressedWithFinalUnit = new Set(sequence[sequence.length - 1])
+
+ // The combo is complete, so it is no longer partially typed.
+ this._sequenceAdvancedAt = 0
}
_wrapEvent(
diff --git a/packages/keystrokes/src/keystrokes.ts b/packages/keystrokes/src/keystrokes.ts
index 81f3c97..e69eaad 100644
--- a/packages/keystrokes/src/keystrokes.ts
+++ b/packages/keystrokes/src/keystrokes.ts
@@ -28,6 +28,8 @@ export type KeyComboEventMapper<
export type KeyPress = {
key: string
aliases: Set
+ // Stable id of the physical key; see KeyEvent#identity.
+ identity?: string
event: KeyEvent
}
@@ -288,6 +290,9 @@ export class Keystrokes<
})
const unbindInactive = this._onInactiveBinder(() => {
this._isActive = false
+ // Any keyup that happens while we are not receiving events is lost, so
+ // treat losing focus as releasing everything.
+ this.releaseAllKeys()
})
const unbindKeyPressed = this._onKeyPressedBinder((e) => {
this._handleKeyPress(e)
@@ -308,6 +313,33 @@ export class Keystrokes<
this._unbinder?.()
}
+ /**
+ * Releases every key currently believed to be held and abandons any progress
+ * through a key combo. Call this whenever keyups may have been missed — the
+ * page regaining focus, or a modal that stopped propagation of key events.
+ *
+ * Without a way to do this a single missed keyup is unrecoverable: a stranded
+ * key stops all single key combos from matching, and a combo left marked as
+ * pressed keeps suppressing shorter combos (see the readme).
+ */
+ releaseAllKeys() {
+ const heldKeyPresses = [...this._activeKeyPresses]
+ for (const keyPress of heldKeyPresses)
+ this._handleKeyRelease(keyPress.event)
+
+ // Whatever is still marked pressed had its keyup swallowed, so it will never
+ // be released through the normal path.
+ const fallbackEvent = heldKeyPresses[heldKeyPresses.length - 1]?.event
+ for (const keyComboState of this._keyComboStatesArray) {
+ keyComboState.forceRelease(fallbackEvent)
+ }
+
+ this._activeKeyPresses.length = 0
+ this._activeKeyMap.clear()
+ this._keyCombosPressedByKey.clear()
+ this._updateKeyComboStates()
+ }
+
private _ensureCachedKeyComboState(keyCombo: string) {
keyCombo = KeyComboState.normalizeKeyCombo(keyCombo)
if (!this._watchedKeyComboStates[keyCombo]) {
@@ -321,22 +353,37 @@ export class Keystrokes<
return keyComboState
}
- private _handleKeyPress(event: KeyEvent) {
- if (!this._isActive) return
-
- event = {
+ private _normalizeEvent(event: KeyEvent) {
+ const normalized = {
...event,
key: event.key.toLowerCase(),
aliases: event.aliases?.map((a) => a.toLowerCase()) ?? [],
}
- const remappedKey = this._keyRemap[event.key]
- if (remappedKey) event.key = remappedKey
- for (let i = 0; i < event.aliases!.length; i += 1) {
- const remappedAlias = this._keyRemap[event.aliases![i]]
- if (remappedAlias) event.aliases![i] = remappedAlias
+ const remappedKey = this._keyRemap[normalized.key]
+ if (remappedKey) normalized.key = remappedKey
+ for (let i = 0; i < normalized.aliases.length; i += 1) {
+ const remappedAlias = this._keyRemap[normalized.aliases[i]]
+ if (remappedAlias) normalized.aliases[i] = remappedAlias
}
+ return normalized
+ }
+
+ private _identityOf(event: KeyEvent) {
+ return event.identity ?? event.key
+ }
+
+ private _handleKeyPress(event: KeyEvent) {
+ if (!this._isActive) return
+
+ // Some events arrive without a key at all. There is nothing to track, and
+ // normalizing would throw.
+ // https://github.com/RobertWHurst/Keystrokes/issues/64
+ if (event.key == null) return
+
+ event = this._normalizeEvent(event)
+
const keyPressHandlerStates = this._handlerStates[event.key]
if (keyPressHandlerStates) {
for (const s of keyPressHandlerStates) s.executePressed(event)
@@ -348,16 +395,23 @@ export class Keystrokes<
}
}
- const existingKeypress = this._activeKeyMap.get(event.key)
+ const identity = this._identityOf(event)
+ const existingKeypress = this._activeKeyMap.get(identity)
if (existingKeypress) {
+ // The same physical key. Its label may have changed because a modifier was
+ // pressed or released while it was held, so refresh the entry rather than
+ // adding a second one that could never be released.
+ existingKeypress.key = event.key
+ existingKeypress.aliases = new Set(event.aliases)
existingKeypress.event = event
} else {
const keypress = {
key: event.key,
aliases: new Set(event.aliases),
+ identity,
event,
}
- this._activeKeyMap.set(event.key, keypress)
+ this._activeKeyMap.set(identity, keypress)
this._activeKeyPresses.push(keypress)
}
@@ -385,24 +439,36 @@ export class Keystrokes<
}
if (activatedCombos.length > 0) {
- this._keyCombosPressedByKey.set(event.key, activatedCombos)
+ this._keyCombosPressedByKey.set(identity, activatedCombos)
}
}
}
private _handleKeyRelease(event: KeyEvent) {
- event = {
- ...event,
- key: event.key.toLowerCase(),
- aliases: event.aliases?.map((a) => a.toLowerCase()) ?? [],
+ // A keyup can arrive without a key. Unlike a keydown we must not simply bail:
+ // the matching keydown may well have been tracked, and dropping its release is
+ // precisely what strands a key. Recover the label seen at keydown instead.
+ if (event.key == null) {
+ const tracked =
+ event.identity != null
+ ? this._activeKeyMap.get(event.identity)
+ : undefined
+ if (!tracked) return
+ event = { ...event, key: tracked.key, aliases: [...tracked.aliases] }
+ } else {
+ event = this._normalizeEvent(event)
}
- const remappedKey = this._keyRemap[event.key]
- if (remappedKey) event.key = remappedKey
- if (event.aliases) {
- for (let i = 0; i < event.aliases.length; i += 1) {
- const remappedAlias = this._keyRemap[event.aliases[i]]
- if (remappedAlias) event.aliases[i] = remappedAlias
+ const identity = this._identityOf(event)
+ const activeKeyPress = this._activeKeyMap.get(identity)
+
+ // Release under the label reported at keydown. Handlers and combo states
+ // matched on that label, so releasing under a different one leaves them stuck.
+ if (activeKeyPress && activeKeyPress.key !== event.key) {
+ event = {
+ ...event,
+ key: activeKeyPress.key,
+ aliases: [...activeKeyPress.aliases],
}
}
@@ -417,10 +483,10 @@ export class Keystrokes<
}
}
- if (this._activeKeyMap.has(event.key)) {
- this._activeKeyMap.delete(event.key)
+ if (this._activeKeyMap.has(identity)) {
+ this._activeKeyMap.delete(identity)
for (let i = 0; i < this._activeKeyPresses.length; i += 1) {
- if (this._activeKeyPresses[i].key === event.key) {
+ if (this._identityOfKeyPress(this._activeKeyPresses[i]) === identity) {
this._activeKeyPresses.splice(i, 1)
i -= 1
break
@@ -431,15 +497,21 @@ export class Keystrokes<
this._tryReleaseSelfReleasingKeys()
this._updateKeyComboStates()
- const activatedCombos = this._keyCombosPressedByKey.get(event.key)
+ const activatedCombos = this._keyCombosPressedByKey.get(identity)
if (activatedCombos) {
for (const combo of activatedCombos) {
combo.executeReleased(event)
}
- this._keyCombosPressedByKey.delete(event.key)
+ this._keyCombosPressedByKey.delete(identity)
}
}
+ private _identityOfKeyPress(
+ keyPress: KeyPress,
+ ) {
+ return keyPress.identity ?? keyPress.key
+ }
+
private _updateKeyComboStates() {
for (const keyComboState of this._keyComboStatesArray)
keyComboState.updateState(this._activeKeyPresses, this.sequenceTimeout)
diff --git a/packages/keystrokes/src/tests/browser-bindings.spec.ts b/packages/keystrokes/src/tests/browser-bindings.spec.ts
index bb04bf0..f67fcaa 100644
--- a/packages/keystrokes/src/tests/browser-bindings.spec.ts
+++ b/packages/keystrokes/src/tests/browser-bindings.spec.ts
@@ -98,6 +98,94 @@ describe('browserOnInactiveBinder(handler) -> void', () => {
})
})
+describe('/stale key state/', () => {
+ const getHandlers = () => {
+ const winAddEventListenerStub = vi.fn()
+ const docAddEventListenerStub = vi.fn()
+ const dispatchEventStub = vi.fn()
+ vi.stubGlobal('addEventListener', winAddEventListenerStub)
+ vi.spyOn(document, 'addEventListener').mockImplementation(
+ docAddEventListenerStub,
+ )
+ vi.spyOn(document, 'dispatchEvent').mockImplementation(dispatchEventStub)
+
+ browserOnInactiveBinder(vi.fn())
+ browserOnKeyPressedBinder(vi.fn())
+ browserOnKeyReleasedBinder(vi.fn())
+
+ return {
+ dispatchEventStub,
+ inactive: winAddEventListenerStub.mock.calls[0][1],
+ keyPressed: docAddEventListenerStub.mock.calls[0][1],
+ keyReleased: docAddEventListenerStub.mock.calls[1][1],
+ listenedWindowEvents: winAddEventListenerStub.mock.calls.map((c) => c[0]),
+ }
+ }
+
+ it('tracks pressed keys by code, so a changed key label cannot strand one', () => {
+ const { inactive, keyPressed, keyReleased, dispatchEventStub } =
+ getHandlers()
+
+ // keydown reports "?" but the keyup reports "/" — same physical key.
+ keyPressed(new KeyboardEvent('keydown', { key: '?', code: 'Slash' }))
+ keyReleased(new KeyboardEvent('keyup', { key: '/', code: 'Slash' }))
+ inactive()
+
+ // Nothing is held, so losing focus must not replay a synthetic keyup.
+ expect(dispatchEventStub).toBeCalledTimes(0)
+
+ vi.unstubAllGlobals()
+ })
+
+ it('replays the key reported at keydown when releasing on focus loss', () => {
+ const { inactive, keyPressed, dispatchEventStub } = getHandlers()
+
+ keyPressed(new KeyboardEvent('keydown', { key: '?', code: 'Slash' }))
+ inactive()
+
+ expect(dispatchEventStub).toBeCalledTimes(1)
+ const synthetic = dispatchEventStub.mock.calls[0][0]
+ expect(synthetic.type).toBe('keyup')
+ expect(synthetic.key).toBe('?')
+ expect(synthetic.code).toBe('Slash')
+
+ vi.unstubAllGlobals()
+ })
+
+ it('also releases held keys when the page is hidden or unloaded', () => {
+ const { listenedWindowEvents } = getHandlers()
+
+ expect(listenedWindowEvents).toContain('blur')
+ expect(listenedWindowEvents).toContain('pagehide')
+ // visibilitychange is fired at the document but bubbles, so a window
+ // listener sees it without disturbing the document listener order.
+ expect(listenedWindowEvents).toContain('visibilitychange')
+
+ vi.unstubAllGlobals()
+ })
+
+ it('ignores keydowns that cannot have a matching keyup', () => {
+ const handlerStub = vi.fn()
+ const docAddEventListenerStub = vi.fn()
+ vi.spyOn(document, 'addEventListener').mockImplementation(
+ docAddEventListenerStub,
+ )
+
+ browserOnKeyPressedBinder(handlerStub)
+ const keyPressed = docAddEventListenerStub.mock.calls[0][1]
+
+ // IME composition and autofill emit keydowns with no reliable keyup, so
+ // tracking them can only ever strand a key.
+ keyPressed({ key: 'a', code: 'KeyA', isComposing: true })
+ keyPressed({ key: 'Process', code: 'KeyA' })
+ keyPressed({ key: 'Unidentified', code: 'KeyA' })
+
+ expect(handlerStub).toBeCalledTimes(0)
+
+ vi.unstubAllGlobals()
+ })
+})
+
describe('browserOnKeyPressedBinder(handler) -> void', () => {
it('correctly binds the given handler to document keydown', () => {
const addEventListenerStub = vi.fn()
diff --git a/packages/keystrokes/src/tests/keystrokes.spec.ts b/packages/keystrokes/src/tests/keystrokes.spec.ts
index b26761e..8dc3713 100644
--- a/packages/keystrokes/src/tests/keystrokes.spec.ts
+++ b/packages/keystrokes/src/tests/keystrokes.spec.ts
@@ -688,3 +688,234 @@ describe('new Keystrokes(options)', () => {
})
})
})
+
+// See the "differences from upstream" section of the readme.
+describe('/stale key state/', () => {
+ // Builds an event the way the browser bindings do: `identity` is the physical
+ // key (event.code) and `key` is the label, which changes with modifier state.
+ const ev = (key: string, code: string) =>
+ ({ key, aliases: [`@${code}`], identity: code }) as any
+
+ describe('key identity', () => {
+ it('leaves no phantom key when keydown and keyup report different keys', () => {
+ const keystrokes = createTestKeystrokes()
+ const bPressed = vi.fn()
+ keystrokes.bindKeyCombo('b', { onPressed: bPressed })
+
+ // Typing "?" and releasing shift before the slash key: the keydown reports
+ // "?" while the keyup reports "/".
+ keystrokes.press(ev('shift', 'ShiftLeft'))
+ keystrokes.press(ev('?', 'Slash'))
+ keystrokes.release(ev('shift', 'ShiftLeft'))
+ keystrokes.release(ev('/', 'Slash'))
+
+ expect(keystrokes.pressedKeys).toEqual([])
+
+ // A phantom key parked at the head of the active key presses would stop
+ // every single key combo from matching, for the rest of the session.
+ keystrokes.press(ev('b', 'KeyB'))
+ expect(bPressed).toBeCalledTimes(1)
+ })
+
+ it('releases a dead key whose keyup reports the composed character', () => {
+ const keystrokes = createTestKeystrokes()
+
+ // macOS option+e reports "Dead" on keydown and "e" on keyup.
+ keystrokes.press(ev('Dead', 'KeyE'))
+ keystrokes.release(ev('e', 'KeyE'))
+
+ expect(keystrokes.pressedKeys).toEqual([])
+ })
+
+ it('replaces rather than duplicates state when the key label changes mid press', () => {
+ const keystrokes = createTestKeystrokes()
+
+ keystrokes.press(ev('?', 'Slash'))
+ keystrokes.press(ev('/', 'Slash'))
+
+ expect(keystrokes.pressedKeys).toEqual(['/'])
+
+ keystrokes.release(ev('/', 'Slash'))
+
+ expect(keystrokes.pressedKeys).toEqual([])
+ })
+
+ // https://github.com/RobertWHurst/Keystrokes/issues/64
+ it('ignores a keydown carrying no key rather than throwing', () => {
+ const keystrokes = createTestKeystrokes()
+
+ expect(() => keystrokes.press({ identity: 'KeyB' } as any)).not.toThrow()
+ expect(keystrokes.pressedKeys).toEqual([])
+ })
+
+ // The obvious fix for the issue above — bailing out of both handlers — would
+ // strand the key here, which is the phantom bug all over again.
+ it('still releases a key when the keyup carries no key', () => {
+ const keystrokes = createTestKeystrokes()
+
+ keystrokes.press(ev('b', 'KeyB'))
+ expect(() =>
+ keystrokes.release({ identity: 'KeyB' } as any),
+ ).not.toThrow()
+
+ expect(keystrokes.pressedKeys).toEqual([])
+ })
+ })
+
+ describe('longest combo wins', () => {
+ const bindChordAndPlainKey = () => {
+ const keystrokes = createTestKeystrokes()
+ const chordPressed = vi.fn()
+ const chordReleased = vi.fn()
+ const plainPressed = vi.fn()
+ keystrokes.bindKeyCombo('control + k, b', {
+ onPressed: chordPressed,
+ onReleased: chordReleased,
+ })
+ keystrokes.bindKeyCombo('b', { onPressed: plainPressed })
+ return { keystrokes, chordPressed, chordReleased, plainPressed }
+ }
+
+ const typeChord = (
+ keystrokes: ReturnType['keystrokes'],
+ ) => {
+ keystrokes.press(ev('control', 'ControlLeft'))
+ keystrokes.press(ev('k', 'KeyK'))
+ keystrokes.release(ev('k', 'KeyK'))
+ keystrokes.release(ev('control', 'ControlLeft'))
+ keystrokes.press(ev('b', 'KeyB'))
+ }
+
+ it('suppresses the shorter combo while the longer combo is held', () => {
+ const { keystrokes, chordPressed, plainPressed } = bindChordAndPlainKey()
+
+ typeChord(keystrokes)
+
+ expect(chordPressed).toBeCalledTimes(1)
+ expect(plainPressed).toBeCalledTimes(0)
+ })
+
+ it('stops suppressing the shorter combo once the longer combo is released', () => {
+ const { keystrokes, chordReleased, plainPressed } = bindChordAndPlainKey()
+
+ typeChord(keystrokes)
+ keystrokes.release(ev('b', 'KeyB'))
+
+ expect(chordReleased).toBeCalledTimes(1)
+ expect(keystrokes.checkKeyCombo('control + k, b')).toBe(false)
+
+ keystrokes.press(ev('b', 'KeyB'))
+ expect(plainPressed).toBeCalledTimes(1)
+ })
+
+ it('recovers when the keyup completing the longer combo is never delivered', () => {
+ const { keystrokes, chordReleased, plainPressed } = bindChordAndPlainKey()
+
+ typeChord(keystrokes)
+ // The keyup for the final `b` is swallowed by the browser. Without a
+ // recovery path the chord stays pressed forever and, because it is longer,
+ // permanently suppresses every single key combo.
+ keystrokes.releaseAllKeys()
+
+ expect(keystrokes.pressedKeys).toEqual([])
+ expect(chordReleased).toBeCalledTimes(1)
+ expect(keystrokes.checkKeyCombo('control + k, b')).toBe(false)
+
+ keystrokes.press(ev('b', 'KeyB'))
+ expect(plainPressed).toBeCalledTimes(1)
+ })
+
+ // Documents current behaviour rather than a requirement: matching is
+ // positional, so an unrelated held key blocks single key combos. Intended for
+ // now — see NOTE[held-key-blocks-single-key-combos] in key-combo-state.ts.
+ it('does not fire a single key combo while an unrelated key is held', () => {
+ const keystrokes = createTestKeystrokes()
+ const bPressed = vi.fn()
+ keystrokes.bindKeyCombo('b', { onPressed: bPressed })
+
+ keystrokes.press(ev(' ', 'Space'))
+ keystrokes.press(ev('b', 'KeyB'))
+
+ expect(bPressed).toBeCalledTimes(0)
+ })
+ })
+
+ describe('#releaseAllKeys()', () => {
+ it('releases keys that are genuinely held', () => {
+ const keystrokes = createTestKeystrokes()
+ const released = vi.fn()
+ keystrokes.bindKeyCombo('b', { onReleased: released })
+
+ keystrokes.press(ev('b', 'KeyB'))
+ keystrokes.releaseAllKeys()
+
+ expect(keystrokes.pressedKeys).toEqual([])
+ expect(released).toBeCalledTimes(1)
+ })
+
+ it('resets a combo left part way through its sequence', () => {
+ const keystrokes = createTestKeystrokes()
+ const chordPressed = vi.fn()
+ const plainPressed = vi.fn()
+ keystrokes.bindKeyCombo('control + k, m', { onPressed: chordPressed })
+ keystrokes.bindKeyCombo('m', { onPressed: plainPressed })
+
+ keystrokes.press(ev('control', 'ControlLeft'))
+ keystrokes.press(ev('k', 'KeyK'))
+ keystrokes.release(ev('k', 'KeyK'))
+ keystrokes.release(ev('control', 'ControlLeft'))
+ keystrokes.releaseAllKeys()
+
+ keystrokes.press(ev('m', 'KeyM'))
+
+ expect(chordPressed).toBeCalledTimes(0)
+ expect(plainPressed).toBeCalledTimes(1)
+ })
+ })
+
+ describe('partially typed combos', () => {
+ const setup = () => {
+ const keystrokes = createTestKeystrokes()
+ const chordPressed = vi.fn()
+ const plainPressed = vi.fn()
+ keystrokes.bindKeyCombo('control + k, m', { onPressed: chordPressed })
+ keystrokes.bindKeyCombo('m', { onPressed: plainPressed })
+ keystrokes.press(ev('control', 'ControlLeft'))
+ keystrokes.press(ev('k', 'KeyK'))
+ keystrokes.release(ev('k', 'KeyK'))
+ keystrokes.release(ev('control', 'ControlLeft'))
+ return { keystrokes, chordPressed, plainPressed }
+ }
+
+ it('completes the combo within the sequence timeout', () => {
+ vi.useFakeTimers()
+ try {
+ const { keystrokes, chordPressed, plainPressed } = setup()
+
+ keystrokes.press(ev('m', 'KeyM'))
+
+ expect(chordPressed).toBeCalledTimes(1)
+ expect(plainPressed).toBeCalledTimes(0)
+ } finally {
+ vi.useRealTimers()
+ }
+ })
+
+ it('expires the combo once the sequence timeout has passed', () => {
+ vi.useFakeTimers()
+ try {
+ const { keystrokes, chordPressed, plainPressed } = setup()
+
+ vi.advanceTimersByTime(keystrokes.sequenceTimeout + 1)
+ keystrokes.press(ev('m', 'KeyM'))
+
+ // Otherwise a stray control+k swallows whichever single key is pressed
+ // next, however many minutes later that happens to be.
+ expect(chordPressed).toBeCalledTimes(0)
+ expect(plainPressed).toBeCalledTimes(1)
+ } finally {
+ vi.useRealTimers()
+ }
+ })
+ })
+})
diff --git a/readme.md b/readme.md
index 31d2b3e..29c14d5 100644
--- a/readme.md
+++ b/readme.md
@@ -17,6 +17,18 @@
+> [!IMPORTANT]
+> This is the **scalableminds fork** of Keystrokes, consumed by WEBKNOSSOS. It
+> diverges from upstream in several ways — see
+> [scalableminds fork: differences from upstream](#scalableminds-fork-differences-from-upstream)
+> before changing anything in `src/`, and read
+> [Releasing this fork](#releasing-this-fork) before expecting a change to reach
+> a downstream project.
+>
+> Note that `packages/keystrokes/readme.md` is a **build artifact** — the package
+> build copies this file over it — so fork documentation belongs here, in the
+> monorepo root readme, and nowhere else.
+
__If you encounter a bug please [report it][bug-report].__
Keystrokes as a quick and easy to use library for binding functions to keys
@@ -495,6 +507,143 @@ const keystrokes = new Keystrokes({
keystrokes.bindKey(...)
```
+## scalableminds fork: differences from upstream
+
+Everything below is specific to this fork and is **not** in
+`RobertWHurst/Keystrokes`. Keep this list current when changing `src/`, so the
+next person does not have to reconstruct it from `git log`.
+
+### 1. `@` key aliases
+
+Every key event carries `aliases: ['@' + event.code]`, so a binding can name a
+physical key rather than the character it produces. This is what makes a shortcut
+on a punctuation key survive a keyboard-layout change. Related upstream requests:
+[#33](https://github.com/RobertWHurst/Keystrokes/issues/33),
+[#58](https://github.com/RobertWHurst/Keystrokes/issues/58). *Upstreamable.*
+
+### 2. Longest combo wins
+
+When a keypress completes several combos at once, `_handleKeyPress` executes only
+those whose `sequenceLength` is the largest among them, and `_handleKeyRelease`
+releases them via `_keyCombosPressedByKey`.
+
+The motivation is that shortcuts are frequently sub-combos of one another: in
+WEBKNOSSOS `b` sets a branch point while `control + k, b` switches to the brush
+tool. Without this, completing the longer combo also fires the shorter one. Only
+the most specific match should win. *Offered upstream as a PR.*
+
+Invariant to preserve: **a longer combo suppresses shorter ones for exactly as
+long as it is physically held, and not one event longer.** Because suppression is
+global, a combo that stays marked as pressed disables every shorter combo for the
+rest of the session — which is what made this a source of hard-to-diagnose bugs.
+The recovery path for that is `releaseAllKeys()`, via `forceRelease` in
+`key-combo-state.ts`.
+
+### 3. Keys are tracked by physical key, not by label
+
+`KeyEvent.identity` (set to `event.code` by the browser bindings) is the id used
+for `_activeKeyPresses`, `_activeKeyMap` and `_keyCombosPressedByKey`.
+
+`event.key` cannot serve that purpose because the label depends on the modifiers
+held at the time: pressing shift+slash reports `?` on keydown and, if shift is
+released first, `/` on keyup. Tracking by label therefore strands the `?` entry
+forever — and since combo matching is positional, one stranded key stops *every*
+single-key combo from matching until the page reloads.
+
+Also covers [#64](https://github.com/RobertWHurst/Keystrokes/issues/64)
+(`Cannot read properties of undefined (reading 'toLowerCase')`): a keydown with
+no `key` is ignored, while a keyup with no `key` is still resolved through
+`identity` and released under the label seen at keydown. Note the patch suggested
+in that issue — returning early from both handlers — is unsafe: skipping a keyup
+is exactly what strands a key. *Upstreamable.*
+
+### 4. Partially typed combos expire
+
+`sequenceTimeout` governs the whole sequence, not just the release window. Once
+`control + k` has been typed and released, upstream leaves the combo armed
+indefinitely, so whichever single key is pressed next — however much later —
+completes it instead of firing its own handler. See `_sequenceAdvancedAt` in
+`key-combo-state.ts`. *Upstreamable.*
+
+### 5. `releaseAllKeys()`
+
+Public on both the instance and the module, and used internally whenever the
+environment goes inactive. Consumers need a supported way to resynchronise after
+keyups that never arrived — a page regaining focus, or a modal that stopped
+propagation of key events. Related: [#49](https://github.com/RobertWHurst/Keystrokes/issues/49).
+*Upstreamable.*
+
+### 6. Broader recovery in the browser bindings
+
+- `activeKeyEvents` is keyed by `event.code` rather than `event.key`, so it does
+ not leak an entry every time a key's label changes mid-press.
+- Held keys are released on `pagehide` and on `visibilitychange` (hidden) as well
+ as `blur`, since `blur` does not cover every way a page stops receiving keys.
+- Keydowns that cannot have a matching keyup are ignored: `isComposing`, and the
+ `Process` / `Unidentified` placeholders emitted by IMEs and some autofill
+ implementations. Keyups are **never** filtered — they only remove state, and
+ dropping one is what strands a key. Likely also covers
+ [#50](https://github.com/RobertWHurst/Keystrokes/issues/50) and
+ [#55](https://github.com/RobertWHurst/Keystrokes/issues/55). *Upstreamable.*
+
+### Known behaviour, deliberately unchanged
+
+Combo matching is positional: each unit is only searched in a window starting at
+the current offset into the held-key list. As a result, **while any unrelated key
+is held, no single-key combo matches at all** — holding space and tapping `b`
+does nothing. This is intended for WEBKNOSSOS. If it ever needs revisiting, see
+`NOTE[held-key-blocks-single-key-combos]` in `key-combo-state.ts`.
+
+## Releasing this fork
+
+Downstream projects install the built artifacts from the orphan `dist/keystrokes`
+branch, not from npm:
+
+```json
+"@rwh/keystrokes": "github:scalableminds/Keystrokes#dist/keystrokes"
+```
+
+A separate branch is needed because Yarn and npm resolve a git dependency by
+looking for `package.json` at the repository root, and this repo is a pnpm
+monorepo whose root package is `@rwh/keystrokes-monorepo`.
+
+### Releasing a merged change
+
+1. Make your `src/` changes on a feature branch and open a PR against `main`.
+ CI runs lint and tests on pull requests.
+2. Once it is merged, run `scripts/update-dist.sh` from `main` (requires `pnpm`).
+ It builds the package and force-pushes `package.json` + `dist/` to
+ `dist/keystrokes`.
+3. In the downstream project, run `yarn install` and commit the lockfile change.
+
+Step 3 is not optional: the lockfile pins a commit hash on a force-pushed branch,
+so without refreshing it the consumer keeps the old build and the change silently
+does not ship.
+
+### Getting a change reviewed downstream before it is merged
+
+`dist/keystrokes` is what *every* consumer resolves, so publishing unreviewed work
+there ships it to anyone who runs `yarn install`. `update-dist.sh` refuses to
+publish it from a branch other than `main` for that reason (override with
+`ALLOW_UNMERGED=1` if you really mean to).
+
+Publish the review build to its own dist branch instead, and pin the commit the
+script prints:
+
+```sh
+./scripts/update-dist.sh dist/my-feature
+```
+
+```json
+"@rwh/keystrokes": "github:scalableminds/Keystrokes#"
+```
+
+Pin the commit rather than the branch name — dist branches are force-pushed, so a
+branch reference can change under a reviewer mid-review.
+
+When the source PR merges, publish to `dist/keystrokes` as usual and point the
+downstream project back at `#dist/keystrokes` before merging it.
+
## Help Welcome
If you want to support this project by throwing me some coffee money It's
diff --git a/scripts/update-dist.sh b/scripts/update-dist.sh
index 42063e4..d348a45 100755
--- a/scripts/update-dist.sh
+++ b/scripts/update-dist.sh
@@ -1,11 +1,18 @@
#!/usr/bin/env bash
#
-# update-dist.sh — build @rwh/keystrokes and publish the artifacts to the
-# `dist/keystrokes` branch so downstream projects can install the package
-# directly from this GitHub fork via:
+# update-dist.sh — build @rwh/keystrokes and publish the artifacts to a dist
+# branch, so downstream projects can install the package directly from this
+# GitHub fork via:
#
# "@rwh/keystrokes": "github:scalableminds/Keystrokes#dist/keystrokes"
#
+# Usage:
+# ./scripts/update-dist.sh [dist-branch]
+#
+# ./scripts/update-dist.sh # publish to dist/keystrokes (release)
+# ./scripts/update-dist.sh dist/my-feature # publish to a review branch
+# ALLOW_UNMERGED=1 ./scripts/update-dist.sh # publish a release from a non-main branch
+#
# WHY a separate dist branch?
# This repo is a pnpm monorepo. The root package.json belongs to the monorepo
# itself ("@rwh/keystrokes-monorepo"), not to the keystrokes package. Yarn
@@ -17,16 +24,54 @@
# that holds only the files a consumer needs: package.json + dist/. It has
# a flat structure with no monorepo overhead, so Yarn resolves it correctly.
#
-# WORKFLOW:
-# 1. Make your changes to packages/keystrokes/src/ on any source branch.
-# 2. Run this script to rebuild and update the dist branch.
-# 3. The downstream project just runs `yarn install` — no extra steps.
+# RELEASE WORKFLOW:
+# 1. Make your changes to packages/keystrokes/src/ on a feature branch.
+# 2. Open a PR against main and get it reviewed.
+# 3. After it merges, run this script from main.
+# 4. In the downstream project run `yarn install` and commit the lockfile.
+#
+# REVIEW WORKFLOW (change not merged yet):
+# dist/keystrokes is what every consumer resolves, so publishing unreviewed
+# work there ships it to anyone who runs `yarn install`. Publish to a separate
+# dist branch instead and point the downstream PR at the commit this script
+# prints:
+#
+# ./scripts/update-dist.sh dist/my-feature
+# # then in the downstream package.json, for the duration of the review:
+# # "@rwh/keystrokes": "github:scalableminds/Keystrokes#"
+#
+# Pin the commit rather than the branch name: dist branches are force-pushed,
+# so a branch reference can change under a reviewer mid-review.
+#
+# Once the source PR is merged, publish to dist/keystrokes as usual and point
+# the downstream project back at "#dist/keystrokes".
#
set -euo pipefail
-DIST_BRANCH="dist/keystrokes"
+DEFAULT_DIST_BRANCH="dist/keystrokes"
+DIST_BRANCH="${1:-$DEFAULT_DIST_BRANCH}"
REPO_ROOT="$(git rev-parse --show-toplevel)"
PKG_DIR="$REPO_ROOT/packages/keystrokes"
+CURRENT_BRANCH="$(git rev-parse --abbrev-ref HEAD)"
+
+# Guard against shipping unreviewed source to the branch all consumers resolve.
+if [ "$DIST_BRANCH" = "$DEFAULT_DIST_BRANCH" ] &&
+ [ "$CURRENT_BRANCH" != "main" ] &&
+ [ "${ALLOW_UNMERGED:-0}" != "1" ]; then
+ cat >&2 <