dispatch toggleevents for dialog open/close #10091

keithamus · 2024-01-25T16:35:42Z

This attempts to fix #9733 by specifying that:

a Dialog's show() steps should dispatch a beforetoggle cancellable event
followed by a queuing a non-cancellable toggle event.
a Dialog's showModal() steps should dispatch a beforetoggle cancellable event
followed by a queuing non-cancellable toggle event.
the close the dialog steps should disaptch a non-cancellable beforetoggle event
followed by queuing a non-cancellable toggle event.

At least two implementers are interested (and none opposed):
- Chromium: Tentatively interested
- Gecko: interested. Dialog BeforeToggle/Toggle events mozilla/standards-positions#1101
Tests are written and can be reviewed and commented upon at:
- Implement toggle events for dialog elements web-platform-tests/wpt#44208
Implementation bugs are filed:
- Chromium: https://bugs.chromium.org/p/chromium/issues/detail?id=1521813
- Gecko: https://bugzilla.mozilla.org/show_bug.cgi?id=1876762
- WebKit: https://bugs.webkit.org/show_bug.cgi?id=268160
- Deno (only for timers, structured clone, base64 utils, channel messaging, module resolution, web workers, and web storage): …
- Node.js (only for timers, structured clone, base64 utils, channel messaging, and module resolution): …
MDN issue is filed: …
The top of this comment includes a clear commit message to use.

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

josepharhar · 2024-01-26T23:11:02Z

I think this needs the "toggle task tracker" concept used in details elements in order to coalesce async toggle events: https://html.spec.whatwg.org/multipage/interactive-elements.html#queue-a-details-toggle-event-task

If you do this and open+close the dialog element synchronously , there should be one "toggle" event with both oldState and newState set to "closed":

dialog.showModal();
dialog.close();

josepharhar · 2024-01-29T18:14:00Z

Consider chrome as tentatively interested. I will file an intent to ship to get an official position when this gets a bit more attention from the other browsers, but I don't anticipate objections

smaug---- · 2024-01-29T22:44:54Z

Also Gecko is interested assuming the pr follows what was discussed at TPAC.

josepharhar · 2024-02-08T20:47:16Z

Tests can be added to the OP: web-platform-tests/wpt#44208

marco-prontera · 2024-04-05T09:24:21Z

Hi, this implementation could help us improve a popup, such as a CMP, for better integration by utilizing native browser APIs, specifically the Popover API, to make everything more robust. This will enable us to further improve the INP impact, as described in my case study here: https://web.dev/case-studies/pubconsent-inp?hl=en. Since the CMP is a predominant component on the web, I believe this kind of implementation should be supported as much as possible. I hope it helps. Thank you

keithamus · 2024-09-26T18:47:46Z

@annevk would you have time to review this or otherwise delegate it out to another editor for review?

annevk

Maybe @nt1m or @josepharhar is interested in reviewing this?

annevk · 2024-09-29T00:18:49Z

source

+   <li><p>Set <var>element</var>'s <span>dialog toggle task tracker</span> to a struct with <span
+   data-x="toggle-task-task">task</span> set to the just-queued <span
+   data-x="concept-task">task</span> and <span data-x="toggle-task-old-state">old state</span> set
+   to <var>oldState</var>.</p></li>


Just-queued is a lil vague. But I guess we have no alternative?

I cribbed this from the popover part of the spec which does the same thing. Happy to look at alternatives.

Yeah if all the task queueing algorithms returned the task they create then this would be better. Maybe that could be done as a follow up and we could fix up this part and the identical popover one?

source

whatwg/html#10091 This also slightly refactors FireToggleEvent to allow to to accomodate both popovers and now also dialogs Differential Revision: https://phabricator.services.mozilla.com/D225449 bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1876762 gecko-commit: a8b51403fa4f1dd6d936826f2a5b14c670426f3d gecko-reviewers: smaug

…e r=smaug whatwg/html#10091 This also slightly refactors FireToggleEvent to allow to to accomodate both popovers and now also dialogs Differential Revision: https://phabricator.services.mozilla.com/D225449

whatwg/html#10091 This also slightly refactors FireToggleEvent to allow to to accomodate both popovers and now also dialogs Differential Revision: https://phabricator.services.mozilla.com/D225449 bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1876762 gecko-commit: a8b51403fa4f1dd6d936826f2a5b14c670426f3d gecko-reviewers: smaug

…e r=smaug whatwg/html#10091 This also slightly refactors FireToggleEvent to allow to to accomodate both popovers and now also dialogs Differential Revision: https://phabricator.services.mozilla.com/D225449 UltraBlame original commit: a8b51403fa4f1dd6d936826f2a5b14c670426f3d

josepharhar · 2024-10-16T16:22:06Z

source

@@ -61030,6 +61101,7 @@ interface <dfn interface>HTMLDialogElement</dfn> : <span>HTMLElement</span> {
   document</span>.</p></li>

   <li><p>Run the <span>dialog focusing steps</span> given <span>this</span>.</p></li>
+


Remove this newline?

josepharhar · 2024-10-16T16:22:52Z

lgtm, I don't see any differences between this spec and the chromium implementation

…e r=smaug whatwg/html#10091 This also slightly refactors FireToggleEvent to allow to to accomodate both popovers and now also dialogs Differential Revision: https://phabricator.services.mozilla.com/D225449

dispatch toggleevents for dialog open/close

87a626d

keithamus mentioned this pull request Jan 27, 2024

Implement toggleevents for dialog open/close WebKit/WebKit#23332

Open

keithamus added 4 commits January 29, 2024 10:18

add dialog toggle task tracker

895ed4d

use dialog toggle event tasks for toggle events

adbfe07

queue a dialog toggle event task on close

210dcab

fix queued toggle event on close

eb2056c

josepharhar mentioned this pull request Jan 29, 2024

Add ToggleEvents to <dialog>.showModal() #9733

Open

keithamus marked this pull request as ready for review February 8, 2024 21:31

davatron5000 mentioned this pull request Jun 4, 2024

chore(web-components): refactor fluent-dialog and add new fluent-dialog-body component microsoft/fluentui#31512

Merged

keithamus requested a review from annevk September 26, 2024 18:47

annevk reviewed Sep 29, 2024

View reviewed changes

smaug---- reviewed Oct 13, 2024

View reviewed changes

source Outdated Show resolved Hide resolved

smaug---- reviewed Oct 13, 2024

View reviewed changes

source Show resolved Hide resolved

smaug---- reviewed Oct 13, 2024

View reviewed changes

source Show resolved Hide resolved

keithamus added 7 commits October 13, 2024 20:35

s/on the//

8e69a0b

remove comma

cd96aeb

s/toggle/beforetoggle/

84c0f1f

return early if dialog closed after beforetoggle fired

9fb2ac3

return early if dialog opened after beforetoggle fired

920ebd8

return early if dialog opened after beforetoggle fired

b21ed65

reorder queued toggle events just after beforetoggle, for clarity

0b2b8c8

moz-wptsync-bot mentioned this pull request Oct 14, 2024

[Gecko Bug 1876762] Implement togggle events for Dialog show/showModal/close web-platform-tests/wpt#48601

Merged

This was referenced Oct 16, 2024

Dialog BeforeToggle/Toggle events mozilla/standards-positions#1101

Open

dispatch toggleevents for dialog open/close WebKit/standards-positions#418

Open

josepharhar reviewed Oct 16, 2024

View reviewed changes

josepharhar approved these changes Oct 16, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

dispatch toggleevents for dialog open/close #10091

dispatch toggleevents for dialog open/close #10091

keithamus commented Jan 25, 2024 •

edited by pr-preview bot

Loading

josepharhar commented Jan 26, 2024

josepharhar commented Jan 29, 2024

smaug---- commented Jan 29, 2024

josepharhar commented Feb 8, 2024

marco-prontera commented Apr 5, 2024

keithamus commented Sep 26, 2024

annevk left a comment

annevk Sep 29, 2024

keithamus Oct 13, 2024 •

edited

Loading

josepharhar Oct 16, 2024

josepharhar Oct 16, 2024

josepharhar commented Oct 16, 2024

		@@ -61030,6 +61101,7 @@ interface <dfn interface>HTMLDialogElement</dfn> : <span>HTMLElement</span> {
		document</span>.</p></li>

		<li><p>Run the <span>dialog focusing steps</span> given <span>this</span>.</p></li>

dispatch toggleevents for dialog open/close #10091

Are you sure you want to change the base?

dispatch toggleevents for dialog open/close #10091

Conversation

keithamus commented Jan 25, 2024 • edited by pr-preview bot Loading

josepharhar commented Jan 26, 2024

josepharhar commented Jan 29, 2024

smaug---- commented Jan 29, 2024

josepharhar commented Feb 8, 2024

marco-prontera commented Apr 5, 2024

keithamus commented Sep 26, 2024

annevk left a comment

Choose a reason for hiding this comment

annevk Sep 29, 2024

Choose a reason for hiding this comment

keithamus Oct 13, 2024 • edited Loading

Choose a reason for hiding this comment

josepharhar Oct 16, 2024

Choose a reason for hiding this comment

josepharhar Oct 16, 2024

Choose a reason for hiding this comment

josepharhar commented Oct 16, 2024

keithamus commented Jan 25, 2024 •

edited by pr-preview bot

Loading

keithamus Oct 13, 2024 •

edited

Loading