Skip to content

fix(nav): Use capture-phase listener so Cmd+K works when inputs have focus#111443

Open
jshchnz wants to merge 1 commit intomasterfrom
fix/cmd-k-capture-phase
Open

fix(nav): Use capture-phase listener so Cmd+K works when inputs have focus#111443
jshchnz wants to merge 1 commit intomasterfrom
fix/cmd-k-capture-phase

Conversation

@jshchnz
Copy link
Copy Markdown
Member

@jshchnz jshchnz commented Mar 24, 2026

Summary

  • Adds a useCapture option to useHotkeys that registers the keydown listener on the capture phase
  • Components like the search query builder call stopPropagation() on keydown events, which prevents shortcuts from reaching the document-level bubble listener. Capture-phase listeners fire before any component can stop propagation.
  • Applies useCapture: true and includeInputs: true to the Cmd+K command palette shortcut

Test plan

  • Focus on the search query builder input on the issues page, press Cmd+K → palette opens
  • Focus on any other text input, press Cmd+K → palette opens
  • Existing hotkeys without useCapture continue to work as before
  • New unit tests pass for capture-phase registration and isolation

…focus

Components like the search query builder call stopPropagation() on
keydown events, preventing them from reaching the document-level
listener in useHotkeys. This adds a useCapture option to useHotkeys
that registers the listener on the capture phase, firing before any
child component can stop propagation.
@jshchnz jshchnz requested review from a team as code owners March 24, 2026 19:13
@github-actions github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Mar 24, 2026
Copy link
Copy Markdown
Contributor

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

expect(callback).toHaveBeenCalled();
});

it('registers on capture phase with useCapture', () => {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Test mock overwrites bubble handler with capture handler

High Severity

The test mock for document.addEventListener stores callbacks by event name (events[event] = callback), so only one callback survives per event. Since useHotkeys now registers two 'keydown' listeners (bubble then capture), the capture handler overwrites the bubble handler in events. All existing tests that invoke events.keydown!(evt) with non-useCapture hotkeys now silently run the capture handler, which skips those hotkeys due to the !!hotkey.useCapture !== capture guard. This breaks every pre-existing test case that expects a bubble-phase callback to fire.

Additional Locations (1)
Fix in Cursor Fix in Web

@@ -42,6 +42,8 @@ function UserAndOrganizationNavigation() {
: [
{
match: ['command+shift+p', 'command+k', 'ctrl+shift+p', 'ctrl+k'],
includeInputs: true,
useCapture: true,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can we start with just includeInputs for now?

@JonasBa
Copy link
Copy Markdown
Member

JonasBa commented Mar 25, 2026

@josh we can close this, the changes in #111540 will take care of this

@getsantry
Copy link
Copy Markdown
Contributor

getsantry bot commented Apr 16, 2026

This issue has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you remove the label Waiting for: Community, I will leave it alone ... forever!


"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

@getsantry getsantry bot added the Stale label Apr 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Frontend Automatically applied to PRs that change frontend components Stale

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants