-
Notifications
You must be signed in to change notification settings - Fork 213
Add ToggleEvent source #3605
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
Add ToggleEvent source #3605
Conversation
ddbeck
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for starting this, @tunetheweb!
| @@ -0,0 +1,6 @@ | |||
| name: Popover ToggleEvent source | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I understand correctly, the toggle event fires for <details>, <dialog>, and popovers. But I'm not sure how to understand the spec with the source property. Is the property only for popovers? I think the answer to this makes a big difference for how we frame this feature.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thinking on this a little more, can you point to an example of how this might be used? The MDN reference page example feels a little contrived and I'm trying to figure out what a developer might do with this that they could not without this property.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I understand correctly, the toggle event fires for
<details>,<dialog>, and popovers. But I'm not sure how to understand the spec with the source property. Is the property only for popovers? I think the answer to this makes a big difference for how we frame this feature.
Oh you're right it's for more than just Popovers (though that's a big use case). Updated to drop the "Popover" from the feature name.
And here's an example of how it might be used:
https://chrome.dev/css-wrapped-2025/#toggleevent-source
(which coincidently is why we want to add this feature so we can add a nice Baseline widget to this page 😁)
ddbeck
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the link to the Wrapped page—something about the "responsible" element cleared this up for me. Some little things to fix (most importantly, this feature's ID), but this is close to merging. Thank you!
Co-authored-by: Daniel D. Beck <[email protected]>
ddbeck
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
Fixes #3604
Has quite different support than the rest of Toggle Event so added it as a new feature: