Skip to content

Add ToggleEvent.source #11186

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

Merged
merged 9 commits into from
Jun 2, 2025
Merged

Add ToggleEvent.source #11186

merged 9 commits into from
Jun 2, 2025

Conversation

josepharhar
Copy link
Contributor

@josepharhar josepharhar commented Apr 2, 2025

This adds a new Element attribute to ToggleEvent called source. The ToggleEvent's source attribute is set to the element which invoked the element which the ToggleEvent is being fired on. Sources can be set up as a parameter passed to element.showPopover(), an element with the popovertarget attribute, or an element with the commandfor attribute.

Fixes #9111

(See WHATWG Working Mode: Changes for more details.)


/form-control-infrastructure.html ( diff )
/form-elements.html ( diff )
/infrastructure.html ( diff )
/interaction.html ( diff )
/interactive-elements.html ( diff )
/popover.html ( diff )

This adds a new Element attribute to ToggleEvent called invoker. The
ToggleEvent's invoker attribute is set to the element which invoked the
element which the ToggleEvent is being fired on. Invokers can be set up
as a parameter passed to element.showPopover(), an element with the
popovertarget attribute, or an element with the commandfor attribute.

Fixes whatwg#9111
@domenic
Copy link
Member

domenic commented Apr 3, 2025

I haven't kept up with all the command discussions, but didn't we decide never to use the word "invoker" in the public APIs? (I've been a bit disappointed to see it popping up in documentation given that decision, but that's a separate thing...)

I thought we settled on source in CommandEvent.

@nt1m
Copy link
Member

nt1m commented Apr 3, 2025

source is also what's used for the imperative API: showPopover({source: ...})

@josepharhar josepharhar changed the title Add ToggleEvent.invoker Add ToggleEvent.source Apr 3, 2025
@josepharhar
Copy link
Contributor Author

Thank you both! I renamed it from invoker to source.

@annevk annevk added needs implementer interest Moving the issue forward requires implementers to express interest needs tests Moving the issue forward requires someone to write tests labels Apr 4, 2025
Copy link
Contributor

@keithamus keithamus left a comment

Choose a reason for hiding this comment

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

Generally this looks good, and I think it is well motivated. I've done what I'll call a "pseudo editorial review pass" and by my eyes this has no editorial issues.

Some meta commentary that's perhaps worth pointing out: details elements toggling, and dialog.show()/showModal(), all dispatch toggle events where the source will be null. I think that's a little odd, and maybe worth discussing, but I don't see it as problematic to change in a follow-up, or possibly never. For example it might be nice to later add {source} as an argument to show()/showModal().

@josepharhar
Copy link
Contributor Author

Thanks Keith!

If command invokers make it possible to declaratively toggle a details element in the future, we could make the command attribute invoker element get set as the source of the toggle event. We could also use the summary element as Luke mentioned in the issue thread if that seems valuable, hopefully that's not too hard to spec.

Adding a source argument to show()/showModal() makes sense, but since that argument already does more for popovers I'm not sure that this change alone justifies adding that as a parameter to those methods. If an invoker/source argument is ever added though, we should add it as the source of the toggle event.

Copy link
Member

@domenic domenic left a comment

Choose a reason for hiding this comment

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

Generally looks good, but I think we can do the events betterNope, we can't, I was wrong.

Also, I haven't yet done a final check that all updated algorithms match all their call sites. It looks likely that you got them all, but if you want to double-check that could save us some time.

@josepharhar
Copy link
Contributor Author

Also, I haven't yet done a final check that all updated algorithms match all their call sites. It looks likely that you got them all, but if you want to double-check that could save us some time.

I just double checked and fixed two of them

@annevk annevk added addition/proposal New features or enhancements topic: events and removed needs implementer interest Moving the issue forward requires implementers to express interest needs tests Moving the issue forward requires someone to write tests labels May 29, 2025
@domenic domenic added the topic: dialog The <dialog> element label Jun 2, 2025
@domenic domenic added the topic: popover The popover attribute and friends label Jun 2, 2025
@domenic domenic merged commit 889a58b into whatwg:main Jun 2, 2025
2 checks passed
aarongable pushed a commit to chromium/chromium that referenced this pull request Jun 3, 2025
I recently added this to the spec PR after incorporating spec changes
for the addition of dialog.requestClose:
whatwg/html#11186

Bug: 408018828
Change-Id: Ia6a3a2560349760219a863dba814c5addfa60165
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6598554
Reviewed-by: Mason Freed <[email protected]>
Commit-Queue: Joey Arhar <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1468857}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Jun 3, 2025
I recently added this to the spec PR after incorporating spec changes
for the addition of dialog.requestClose:
whatwg/html#11186

Bug: 4080188
Change-Id: Ia6a3a2560349760219a863dba814c5addfa60165
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6598554
Reviewed-by: Mason Freed <[email protected]>
Commit-Queue: Joey Arhar <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1468857}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Jun 3, 2025
I recently added this to the spec PR after incorporating spec changes
for the addition of dialog.requestClose:
whatwg/html#11186

Bug: 4080188
Change-Id: Ia6a3a2560349760219a863dba814c5addfa60165
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6598554
Reviewed-by: Mason Freed <[email protected]>
Commit-Queue: Joey Arhar <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1468857}
aarongable pushed a commit to chromium/chromium that referenced this pull request Jun 4, 2025
The spec PR has been merged, so these WPTs should no longer be marked as
tentative: whatwg/html#11186

Bug: 408018828
Change-Id: I6ff9da8ab80551cdb4cae3cfca3cb60872806dfa
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6621684
Reviewed-by: Luke <[email protected]>
Commit-Queue: Luke <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1469501}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Jun 4, 2025
The spec PR has been merged, so these WPTs should no longer be marked as
tentative: whatwg/html#11186

Bug: 4080188
Change-Id: I6ff9da8ab80551cdb4cae3cfca3cb60872806dfa
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6621684
Reviewed-by: Luke <[email protected]>
Commit-Queue: Luke <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1469501}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Jun 4, 2025
The spec PR has been merged, so these WPTs should no longer be marked as
tentative: whatwg/html#11186

Bug: 4080188
Change-Id: I6ff9da8ab80551cdb4cae3cfca3cb60872806dfa
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6621684
Reviewed-by: Luke <[email protected]>
Commit-Queue: Luke <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1469501}
Gingeh added a commit to Gingeh/ladybird that referenced this pull request Jun 4, 2025
Gingeh added a commit to Gingeh/ladybird that referenced this pull request Jun 4, 2025
Gingeh added a commit to Gingeh/ladybird that referenced this pull request Jun 4, 2025
Gingeh added a commit to Gingeh/ladybird that referenced this pull request Jun 5, 2025
Gingeh added a commit to Gingeh/ladybird that referenced this pull request Jun 5, 2025
Gingeh added a commit to Gingeh/ladybird that referenced this pull request Jun 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
addition/proposal New features or enhancements topic: dialog The <dialog> element topic: events topic: popover The popover attribute and friends
Development

Successfully merging this pull request may close these issues.

Popover does not know what triggered it
5 participants