Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

fix(svelte): Fixes excessive 'the hotkey ... has already been registered' messages#63679

Merged
fkling merged 1 commit into
mainfrom
fkling-srch-697-avoid-excessive-the-hotkey-esc-has-already-been-registered
Jul 6, 2024
Merged

fix(svelte): Fixes excessive 'the hotkey ... has already been registered' messages#63679
fkling merged 1 commit into
mainfrom
fkling-srch-697-avoid-excessive-the-hotkey-esc-has-already-been-registered

Conversation

@fkling

@fkling fkling commented Jul 6, 2024

Copy link
Copy Markdown
Contributor

Fixes srch-697

This commit makes two important changes:

  • The popover component will now only register the ESC key handler when it is open (und unregister when it closes). This was done by exposing a new function that creates a hotkey without registering it. The existing registerHotkey function simply wraps the new function so that existing callsites are not affected by this change.
  • The error message was turned into a warning and will now only be triggered in dev builds.

Test plan

Existing unit test, manual testing.

Before this change:
2024-07-06_20-48

After this change: No messages.

…red' messages

This commit makes two important changes:

- The popover component will now only register the ESC key handler when
  it is open (und unregister when it closes).
- The error message was turned into a warning and will now only be
  triggered in dev builds.
@fkling fkling requested a review from a team July 6, 2024 19:06
@fkling fkling self-assigned this Jul 6, 2024
@cla-bot cla-bot Bot added the cla-signed label Jul 6, 2024

@camdencheek camdencheek left a comment

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.

Thank you!

@fkling fkling merged commit 5f75345 into main Jul 6, 2024
@fkling fkling deleted the fkling-srch-697-avoid-excessive-the-hotkey-esc-has-already-been-registered branch July 6, 2024 20:26
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants