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

Only register click handler once #1480

Merged
merged 6 commits into from
Oct 11, 2022
Merged

Only register click handler once #1480

merged 6 commits into from
Oct 11, 2022

Conversation

koddsson
Copy link
Contributor

Description

The <modal-dialog> registers a global event handler each time it connects to the DOM. We actually just want one global event handler so that we don't open or close multiple dialogs when clicking around on the page.

This PR makes sure we only register the event handler once by using a function reference instead of a inlined function for the click handler.

Closes #1366 (again :)

koddsson and others added 4 commits October 11, 2022 11:20
Co-authored-by: Keith Cirkel <keithamus@users.noreply.github.com>
Co-authored-by: Keith Cirkel <keithamus@users.noreply.github.com>
Co-authored-by: Keith Cirkel <keithamus@users.noreply.github.com>
Co-authored-by: Keith Cirkel <keithamus@users.noreply.github.com>
@koddsson koddsson requested review from a team and jonrohan October 11, 2022 14:07
@changeset-bot
Copy link

changeset-bot bot commented Oct 11, 2022

🦋 Changeset detected

Latest commit: dae2bc7

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@primer/view-components Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Base automatically changed from update-eslint-plugin-github to main October 11, 2022 14:11
@koddsson koddsson temporarily deployed to github-pages October 11, 2022 14:15 Inactive
@@ -83,5 +84,6 @@
"tslib": "^2.4.0",
"typescript": "^4.7.4"
},
"prettier": "@github/prettier-config"
"prettier": "@github/prettier-config",
"browserslist": "extends @github/browserslist-config"
Copy link
Member

@jonrohan jonrohan Oct 11, 2022

Choose a reason for hiding this comment

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

There's a browserslistrc file I added recently, could you delete that in favor of this one?

Copy link
Member

Choose a reason for hiding this comment

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

Edit: saw that you did 😂

@koddsson koddsson merged commit c764cee into main Oct 11, 2022
@koddsson koddsson deleted the make-nested-dialogs-work branch October 11, 2022 15:55
@primer-css primer-css mentioned this pull request Oct 11, 2022
jonrohan pushed a commit that referenced this pull request Oct 11, 2022
* Update `eslint-plugin-github`

Co-authored-by: Keith Cirkel <keithamus@users.noreply.github.com>

* Use `@github/browserlist-config`

Co-authored-by: Keith Cirkel <keithamus@users.noreply.github.com>

* Update caniuse-lite db

Co-authored-by: Keith Cirkel <keithamus@users.noreply.github.com>

* Fix shadowed variable

Co-authored-by: Keith Cirkel <keithamus@users.noreply.github.com>

* Only register click handler once

* changeset

Co-authored-by: Keith Cirkel <keithamus@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
javascript Pull requests that update Javascript code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Nested dialogs are dismissed immediately
2 participants