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

Dispatching Toggle Events for Dialog open/close #1005

Open
1 task done
keithamus opened this issue Oct 17, 2024 · 11 comments
Open
1 task done

Dispatching Toggle Events for Dialog open/close #1005

keithamus opened this issue Oct 17, 2024 · 11 comments

Comments

@keithamus
Copy link

keithamus commented Oct 17, 2024

こんにちは TAG-さん!

I'm requesting a TAG review of Dispatching Toggle Events for Dialog open/close.

It is useful for web authors do determine when their <dialog> elements open and close. popover already has ToggleEvent which is dispatched when a popover opens or closes, but <dialog> does not. The current way to detect when a <dialog> opens is to register a mutation observer to check for open, however, this is quite a lot of work where an event would be easier.

I propose we add dispatching of ToggleEvent for <dialog> also. To be explicit: when show or showModal is called, <dialog> should dispatch a ToggleEvent with newState=open. When a dialog is closed (via form or button or close signal) it should dispatch a ToggleEvent with newState=closed.

Further details:

  • I have reviewed the TAG's Web Platform Design Principles
  • Relevant time constraints or deadlines: Ideally in the next 2 weeks so we can continue to ship in Firefox.
  • The group where the work on this specification is currently being done: whatwg
  • The group where standardization of this work is intended to be done (if different from the current group): whatwg
  • Major unresolved issues with or opposition to this specification: None
  • This work is being funded by: GitHub

You should also know that...

I've enabled this in Firefox nightly targeting 133, it is currently flagged in Chromium, but I've raised an I2S (https://groups.google.com/a/chromium.org/g/blink-dev/c/PxluqSXWXD4/m/7q3qwklWAAAJ) and I'm aiming to ship in M132.

@dandclark
Copy link

(Not a TAG opinion)

I'm curious -- after this, do you also plan to work on adding beforetoggle to <details> (which you'd noted as missing in whatwg/html#9743), or do you know if anyone plans to pick that up? Once <dialog> has these events it seems like beforetoggle on <details> is the last remaining place where that developers might be surprised to find one of them missing.

In any case, dispatching beforetoggle/toggle for <dialog> looks like a nice step towards consistency and I'm glad to see it moving towards cross-browser adoption.

@keithamus
Copy link
Author

Yes I’d be happy to also work on <details>.

@jyasskin
Copy link
Contributor

We discussed this in a breakout today:

The two types ("before" and "on") of event seem like very useful things to have.

Why is this "toggle" and not separate "open" and "close" events? The platform is not entirely consistent here, but these elements likely need distinct actions for the two cases. We don't see why this should be a single event type. (We'd also like to see a path toward harmonization of events across the platform, but that's a larger project.)

We also agree with Dan about <details> and the opportunity for future work. That's not in scope here, but we encourage you to look into it.

@keithamus
Copy link
Author

Having the separate event would likely lead to setting up 2 event listeners instead of one for some fairly common tasks, but I can see why it might be nicer for others.

The design was borrowed from popovers to keep symmetry, which perhaps themselves borrowed from the details toggle event (which doesn’t have the new/oldstate properties because it can be observed via the open attribute).

@jyasskin
Copy link
Contributor

I think it would help to see the common cases where you'd need 2 event listeners with mostly the same code. It does make sense to align the 3 kinds of elements, but maybe that should be done by adding open and close to popover (which we should file against HTML and not just discuss in this issue).

@martinthomson
Copy link
Contributor

My intuition and experience with using these suggests that opening and closing <details> and <dialog> has very different reactions in a lot of cases. I would expect to see "toggle" handlers that have if (e.newState) {} else {} at the top level. I can see commonality in "open"/"close" handlers, but that seems less common and it can be handled with el.onopen = handleToggle.bind(true); el.onclose = handleToggle.bind(false) or similar. Choosing requires an understanding of how it might be used in practice.

... or you could ship both, but that seems like a premature commitment to waste.

@josepharhar
Copy link

We could go back on beforetoggle/toggle and replace them with beforeopen/beforeclose/open/close, but this was decided quite a long time ago for popover in openui (and whatwg?), and would require a fairly large deprecation process since its shipped in all browsers now.

I'm not sure exactly where that discussion took place but I could find it if needed.

@plinss plinss added this to the 2024-11-04-week milestone Nov 4, 2024
@jyasskin
Copy link
Contributor

jyasskin commented Nov 4, 2024

@josepharhar I'd love to see the popover discussion that settled on toggle events instead of open/close. The TAG can be useful in identifying likely inconsistencies with either the rest of the platform or common developer needs, but if the domain experts have already examined the exact question we want to raise, I think we should usually accept their decision.

@keithamus
Copy link
Author

FWIW it would not be as simple as open/close; you'd need beforeopen/beforeclose also.

This was discussed in 2021 as part of OpenUI (https://www.w3.org/2021/05/13-openui-minutes.html#t01) where Robert pointed out the precedent for event naming around before; (e.g. beforeinput or beforeunload). This meeting settled on beforeshow/beforehide. At some point this was tweaked to open/closed (though I can't find that thread).

So in 2022 whatwg/html#8386 was raised which proposed the various beforeopen/afteropen/beforeclosed/afterclosed. Domenic asked why we need a set of events for this, and pointed out that toggle could be re-used for the after* events: whatwg/html#8386 (comment). Perhaps during a meeting it was resolved to also use beforetoggle; as the comment here suggests: whatwg/html#8386 (comment).

I'm sure @josepharhar could find more threads exposing this level of discussion but this is what I could find.

@jyasskin
Copy link
Contributor

jyasskin commented Nov 4, 2024

@keithamus Perfect, thanks! It looks like the idea of unifying the *open and *close events into a single event started in @mfreed7's openui/open-ui#607 (comment) in 2022, with Mason echoing the concern here that "It takes more work to write the event handler" (although the potential confusion is gone in the current event design with newState). @domenic endorsed the unification with "I think it will be less work for some use cases, e.g. event handlers that mainly want to sync the state change with some other part of the page and thus would have to listen to both events anyway." @domenic, do you still think moving from *open/*close to *toggle event was the right choice?

As a personal, non-TAG, opinion I think https://w3ctag.github.io/design-principles/#consistency indicates that dialog, details, and popover should have the same set of event names for this purpose, even if they're not the most ergonomic set for the most common uses. We could try to switch all three over to the open/close event set (for which we'd need to file an issue in HTML rather than here), if removing an if statement justifies that work. Someone from the TAG will reply again if we develop any group opinions.

@keithamus
Copy link
Author

We could try to switch all three over to the open/close event set

Another consideration here is that while <dialog> and <details> use open/closed as part of their vernacular, popover uses show/hide. So does it make more sense to have <dialog> and <details> to using open/close events while popover has the asymmetrical show/hide events?

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

7 participants