Fix useHotkeySequence resets progress when modifier key is pressed before a key combo e.g. zA (Vim-like hotkeys)#74
Merged
KevinVandy merged 6 commits intoTanStack:mainfrom Mar 22, 2026
Conversation
klaskow
changed the title
📝 WalkthroughWalkthroughSequenceManager now ignores modifier-only KeyboardEvents during sequence processing, preventing modifier presses (e.g., Shift) between steps from advancing or resetting combo progress. Changes include implementation update, tests, docs reference adjustments, examples, and a changeset entry. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
2 tasks
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/hotkeys/tests/sequence-manager.test.ts (1)
516-549: Good coverage for the reported bug; add one regression case for modifier-only sequence steps.Please add a case like
['Shift', 'C']to ensure modifier-only key steps still progress (and don’t get swallowed globally).🧪 Suggested test addition
describe('modifier key handling', () => { + it('should allow modifier-only step when explicitly part of the sequence', () => { + const manager = SequenceManager.getInstance() + const callback = vi.fn() + + manager.register(['Shift', 'C'], callback) + + document.dispatchEvent( + new KeyboardEvent('keydown', { key: 'Shift', bubbles: true }), + ) + document.dispatchEvent( + new KeyboardEvent('keydown', { key: 'c', bubbles: true }), + ) + + expect(callback).toHaveBeenCalledTimes(1) + }) + it('should not reset sequence when modifier key is pressed between steps', () => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/hotkeys/tests/sequence-manager.test.ts` around lines 516 - 549, Add a regression test that registers a sequence containing a modifier-only step (e.g., SequenceManager.getInstance().register(['Shift', 'C'], callback)) and asserts the sequence still progresses when the modifier key is pressed alone then the non-modifier step is pressed; specifically, simulate dispatching a 'Shift' keydown (modifier-only step) then a 'c' keydown with shiftKey: true and assert callback was called once, and include a negative case where an unrelated key between steps prevents the callback (use dispatchKey helper and the existing describe 'modifier key handling' block to place the test near register and dispatchKey usages).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.changeset/lovely-boats-grin.md:
- Line 2: The release type in the changeset entry "'@tanstack/hotkeys': minor"
is too high for a bug-fix; update that entry to use "patch" instead of "minor"
so the changeset reads "'@tanstack/hotkeys': patch" to reflect a patch-level
release.
---
Nitpick comments:
In `@packages/hotkeys/tests/sequence-manager.test.ts`:
- Around line 516-549: Add a regression test that registers a sequence
containing a modifier-only step (e.g.,
SequenceManager.getInstance().register(['Shift', 'C'], callback)) and asserts
the sequence still progresses when the modifier key is pressed alone then the
non-modifier step is pressed; specifically, simulate dispatching a 'Shift'
keydown (modifier-only step) then a 'c' keydown with shiftKey: true and assert
callback was called once, and include a negative case where an unrelated key
between steps prevents the callback (use dispatchKey helper and the existing
describe 'modifier key handling' block to place the test near register and
dispatchKey usages).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: cd9a2baa-24e2-491b-86c8-78878623dfb4
📒 Files selected for processing (3)
.changeset/lovely-boats-grin.mdpackages/hotkeys/src/sequence-manager.tspackages/hotkeys/tests/sequence-manager.test.ts
@tanstack/angular-hotkeys
@tanstack/hotkeys
@tanstack/hotkeys-devtools
@tanstack/preact-hotkeys
@tanstack/preact-hotkeys-devtools
@tanstack/react-hotkeys
@tanstack/react-hotkeys-devtools
@tanstack/solid-hotkeys
@tanstack/solid-hotkeys-devtools
@tanstack/svelte-hotkeys
@tanstack/vue-hotkeys
@tanstack/vue-hotkeys-devtools
commit: |
KevinVandy
approved these changes
Mar 22, 2026
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@examples/angular/injectHotkeySequence/src/app/app.component.html`:
- Around line 117-119: The `<pre>` snippet contains unescaped '>' characters in
the arrow functions which violates HTMLHint's spec-char-escape rule; update the
two occurrences inside the snippet that call injectHotkeySequence (the lines
referencing injectHotkeySequence(['C', 'I', 'W'], () => changeInnerWord()) and
injectHotkeySequence(['Shift+R', 'Shift+T'], () => doSomething())) by replacing
each '>' in the arrow `=>` with `>` so the HTML is properly escaped.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: afeaf62e-4d16-4d87-a5b7-2d7cccb15828
📒 Files selected for processing (7)
examples/angular/injectHotkeySequence/src/app/app.component.htmlexamples/angular/injectHotkeySequence/src/app/app.component.tsexamples/preact/useHotkeySequence/src/index.tsxexamples/react/useHotkeySequence/src/index.tsxexamples/solid/createHotkeySequence/src/index.tsxexamples/svelte/create-hotkey-sequence/src/App.svelteexamples/vue/useHotkeySequence/src/App.vue
✅ Files skipped from review due to trivial changes (1)
- examples/angular/injectHotkeySequence/src/app/app.component.ts
Comment on lines
+117
to
+119
| injectHotkeySequence(['C', 'I', 'W'], () => changeInnerWord()) | ||
|
|
||
| injectHotkeySequence(['Shift+R', 'Shift+T'], () => doSomething())</pre |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
git ls-files | grep -E "examples/angular/injectHotkeySequence/src/app/app.component.html"Repository: TanStack/hotkeys
Length of output: 124
🏁 Script executed:
if [ -f "examples/angular/injectHotkeySequence/src/app/app.component.html" ]; then
wc -l "examples/angular/injectHotkeySequence/src/app/app.component.html"
fiRepository: TanStack/hotkeys
Length of output: 128
🏁 Script executed:
if [ -f "examples/angular/injectHotkeySequence/src/app/app.component.html" ]; then
sed -n '110,125p' "examples/angular/injectHotkeySequence/src/app/app.component.html"
fiRepository: TanStack/hotkeys
Length of output: 456
🏁 Script executed:
find . -name ".htmlhintrc" -o -name "htmlhint*" -o -name ".htmlhintignore" 2>/dev/null | head -20Repository: TanStack/hotkeys
Length of output: 73
🏁 Script executed:
rg "htmlhint\|spec-char-escape" --type json --type js --type yaml -A 3 | head -50Repository: TanStack/hotkeys
Length of output: 42
🏁 Script executed:
cat .htmlhintrcRepository: TanStack/hotkeys
Length of output: 213
Escape > in the <pre> usage snippet to satisfy HTMLHint linting.
The code contains unescaped > characters within => operators inside <pre> tags, which violates the spec-char-escape rule enabled in .htmlhintrc. Replace > with > in both lines:
Suggested fix
-injectHotkeySequence(['C', 'I', 'W'], () => changeInnerWord())
+injectHotkeySequence(['C', 'I', 'W'], () => changeInnerWord())
-injectHotkeySequence(['Shift+R', 'Shift+T'], () => doSomething())
+injectHotkeySequence(['Shift+R', 'Shift+T'], () => doSomething())📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| injectHotkeySequence(['C', 'I', 'W'], () => changeInnerWord()) | |
| injectHotkeySequence(['Shift+R', 'Shift+T'], () => doSomething())</pre | |
| injectHotkeySequence(['C', 'I', 'W'], () => changeInnerWord()) | |
| injectHotkeySequence(['Shift+R', 'Shift+T'], () => doSomething())</pre |
🧰 Tools
🪛 HTMLHint (1.9.2)
[error] 117-117: Special characters must be escaped : [ > ].
(spec-char-escape)
[error] 119-119: Special characters must be escaped : [ > ].
(spec-char-escape)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@examples/angular/injectHotkeySequence/src/app/app.component.html` around
lines 117 - 119, The `<pre>` snippet contains unescaped '>' characters in the
arrow functions which violates HTMLHint's spec-char-escape rule; update the two
occurrences inside the snippet that call injectHotkeySequence (the lines
referencing injectHotkeySequence(['C', 'I', 'W'], () => changeInnerWord()) and
injectHotkeySequence(['Shift+R', 'Shift+T'], () => doSomething())) by replacing
each '>' in the arrow `=>` with `>` so the HTML is properly escaped.
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🎯 Changes
✅ Checklist
pnpm run test:pr.🚀 Release Impact
Summary by CodeRabbit
Bug Fixes
Tests
Documentation
New Features
Chores