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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Review the registration of event listeners #1

Closed
dvago opened this issue Apr 20, 2022 · 1 comment
Closed

Review the registration of event listeners #1

dvago opened this issue Apr 20, 2022 · 1 comment

Comments

@dvago
Copy link

dvago commented Apr 20, 2022

馃憢馃徎 Zach

I randomly found your project via the Frontend Focus newsletter and playing around with your demos I realised you are generating X event listeners for each component within the page.

You might want to review the way your web component registers event listeners and perhaps check if the listener already exists to avoid bloating the whatever interface uses this component:

Screenshot 2022-04-20 at 18 27 09

@zachleat
Copy link
Owner

zachleat commented Oct 7, 2022

Finally had a look, I鈥檓 assuming this is the demo page right?

Just to take stock, the component binds the following events for each <details-utils> element:

  • document-level mousedown (click out to close)
  • keypress (click out to close)
  • document-level keydown (esc to close)

And then for each <details> inside of <details-utils>:

  • toggle (toggle document class)
  • click (animate)

2 events for click to close, 1 event for esc to close, 1 event for toggle class, and 1 for animate. If you don鈥檛 use these features, the event listeners are not assigned. There doesn鈥檛 seem to be any duplicate event listeners or re-initialization happening here?

Seems okay to me, but happy to listen to more feedback!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants