Skip to content

Swarm Fix: [bug][alpha] Which Key (WhichKey): keydown with repeat === true extends the pending chord and can cancel the sequence #37721

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
hinzwilliam52-ship-it wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Swarm Fix: [bug][alpha] Which Key (WhichKey): keydown with repeat === true extends the pending chord and can cancel the sequence #37721

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions FIX_PROPOSAL.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
To fix the issue, you need to add a check for `event.repeat` in the `keydown` event listener. If `event.repeat` is `true`, you should ignore the event and not append the keystroke to the sequence.

Here is the exact code fix:

```typescript
// In WhichKeyContext.tsx
window.addEventListener('keydown', (event) => {
// ...
if (event.repeat) return; // Add this line to ignore repeat events
const newPrefix = [...currentSequence.keystrokes, keystroke];
// ...
});
```
Comment on lines +5 to +13
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

MDN KeyboardEvent repeat property documentation and browser support

💡 Result:

The MDN documentation for the KeyboardEvent.repeat property states that it is a read-only boolean property of the KeyboardEvent interface. It returns true if the given key is being held down such that it is automatically repeating. Syntax: var repeat = event.repeat; Value: A boolean value. Specification: Defined in UI Events # dom-keyboardevent-repeat. Browser support (as of 2026, based on CanIUse data): - Chrome: Full support since version 32 (partial before), full in recent versions (137+). - Firefox: Full support since 28. - Safari: Full support since 10.1 (desktop), 10.3 (iOS). - Edge: Full support since 79 (and IE 9-11 supported). - Opera: Full support since recent versions. - Mobile browsers generally follow desktop counterparts, with full support in recent versions. The feature has broad support across modern browsers since around 2015-2017, with no support in very old versions (e.g., Chrome <32, Firefox <28). MDN notes it as baseline compatible since 2025 for latest browsers.

Citations:

The proposed code snippet has several concerns.

  1. Event listener management: Using window.addEventListener directly in a React component typically requires proper setup in useEffect with cleanup to prevent memory leaks:
Recommended pattern 
useEffect(() => {
  const handleKeydown = (event: KeyboardEvent) => {
    if (event.repeat) return;
    // ... rest of handler
  };
  
  window.addEventListener('keydown', handleKeydown);
  return () => window.removeEventListener('keydown', handleKeydown);
}, [/* dependencies */]);

  1. Misleading variable name: newPrefix on line 10 represents a keystroke sequence, not a prefix, which is confusing terminology.

  2. Incomplete snippet: The // ... placeholders make it difficult to verify the fix works correctly in context.

Note: event.repeat is a valid and well-supported property for detecting auto-repeated key presses across all modern browsers (Chrome 32+, Firefox 28+, Safari 10.1+, Edge 79+).

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@FIX_PROPOSAL.md` around lines 5 - 13, Move the raw window.addEventListener
usage in WhichKeyContext.tsx into a useEffect hook that registers a named
handleKeydown function and removes it in the cleanup return; inside
handleKeydown check event.repeat early to ignore auto-repeats, then compute the
keystroke sequence (rename the variable newPrefix to newSequence or
keystrokeSequence to reflect its purpose) and perform the existing sequence
logic; ensure useEffect dependency array includes any state/props used (e.g.,
currentSequence) and replace the `// ...` placeholders with the full handler
logic so the handler works correctly in context.


This will prevent the repeat events from appending to the sequence and canceling the chord. Instead, the sequence will remain open for the next deliberate key press.
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Add tests to verify the expected behavior.

The proposal describes the expected behavior but doesn't include any tests to verify that:

  • Repeated keydown events are properly ignored
  • The sequence remains open for the next deliberate key press
  • The fix doesn't prevent legitimate multi-key chords from working
  • Edge cases are handled correctly

Would you like me to generate a test suite that covers these scenarios, or open an issue to track adding comprehensive tests for this fix?

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@FIX_PROPOSAL.md` at line 15, Add a test suite exercising the key-chord
handling to verify repeated keydown events are ignored and the sequence remains
open for the next deliberate press: write tests targeting the KeyChordHandler
(or SequenceManager) and its handleKeyDown behavior to simulate repeated keydown
with event.repeat=true and assert the sequence does not append repeats, simulate
legitimate multi-key chords to assert chords still form, and include edge-case
tests (rapid repeated presses, modifier keys, and keyup without prior keydown)
to ensure no regressions; use the public API/tests helpers that call
handleKeyDown/handleKeyUp (or processKeyEvent) and assert internal
sequence/state and emitted chord events.