Skip to content
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.

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

Don't add unused keydown listeners #7218

Merged
merged 1 commit into from
Dec 20, 2024
Merged

Don't add unused keydown listeners #7218

merged 1 commit into from
Dec 20, 2024

Conversation

gaearon
Copy link
Collaborator

@gaearon gaearon commented Dec 20, 2024

Web-only.

We have hundreds of these listeners at any moment in time but usually they're not doing anything:

Screenshot 2024-12-20 at 17 15 21

They're only relevant when open is true, as seen in the code of the handlers.

So let's attach these listeners only if open is true.

Test Plan

Place console.log into the listeners. Verify they run on every keystroke across the app.

Apply the fix.

Verify they no longer run.

Verify the hashtag link web dropdown menu still responds to "Escape".

@arcalinea arcalinea temporarily deployed to rm-open - social-app PR #7218 December 20, 2024 17:23 — with Render Destroyed
@gaearon gaearon changed the title Don't add unused remove listeners Don't add unused keydown listeners Dec 20, 2024
@arcalinea arcalinea temporarily deployed to rm-open - social-app PR #7218 December 20, 2024 17:23 — with Render Destroyed
Copy link
Member

@estrattonbailey estrattonbailey left a comment

Choose a reason for hiding this comment

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

Tested, nice one 👍

Copy link

Old size New size Diff
6.84 MB 6.84 MB 13 B (0.00%)

@gaearon gaearon merged commit 8a3dfcb into main Dec 20, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants