html icon indicating copy to clipboard operation
html copied to clipboard

Add light dismiss functionality to `<dialog>`

Open mfreed7 opened this issue 2 years ago • 53 comments
trafficstars

One of the nice features of the Popover API is its light dismiss behavior. In several of the demos of Popover that I've seen, developers are doing something like this:

<button popovertarget=foo>Click me</button>
<dialog popover id=foo>I'm a dialog!</dialog>
<style>
dialog[popover]::backdrop {
  background-color: black;
}
</style>

Using <dialog> with a popover attribute is perfectly fine semantically here, since the content represents a dialog. However, this pattern is being used almost entirely because of the features provided by the Popover API which are missing from the <dialog> element itself. Note the usage of ::backdrop to obscure the backdrop entirely. That indicates that this really is meant to be a modal dialog, because the intent is to focus attention only on the dialog and keep the user from "seeing" the rest of the page. However, popovers aren't modal and as such they don't inert the rest of the page. So in the above example, keyboard users are free to tab-navigate to other content they can't see. Mouse users are free to click "through" the opaque background onto unseen elements. Generally, it'd be better if this was a plain old modal <dialog> and not a popover.

To get around this usage pattern, let's bring the missing functionality to <dialog>. https://github.com/whatwg/html/issues/3567 discusses one of those behaviors, namely declarative invocation of <dialog>. In this issue, I'd like to propose a mechanism to add light dismiss to <dialog>s.

Proposal (subject to bikeshedding):

<dialog lightdismiss> I'm a light dismiss dialog </dialog>

With the lightdismiss attribute present, clicking outside the dialog, or hitting ESC (or other close signals) will have the same affect as calling dialog.close().

Note one nuance, which is different from popover: since there's no concept of "nested" dialogs, if more than one dialog is open at a time, only the topmost (most recently opened) dialog will be closed on each light dismiss action. So if three dialogs are open and the user clicks outside all three of them, only the topmost dialog will close. Generally, nested dialogs is an anti-pattern, but even so, this feels the most natural to me anyway.

mfreed7 avatar Jun 01 '23 18:06 mfreed7

So if three dialogs are open and the user clicks outside all three of them, only the topmost dialog will close.

I think this is what I'd expect to happen anyway, so that's good.

tabatkins avatar Jun 01 '23 20:06 tabatkins

Note one nuance, which is different from popover: since there's no concept of "nested" dialogs

Can you expand on this? Intuitively, there's definitely a concept of nesting, e.g. a dialog opened because of an action taken inside another open dialog. I guess popover's notion of nesting is different than this, and that's how you're using the term "nesting"?

So if three dialogs are open and the user clicks outside all three of them, only the topmost dialog will close.

I agree this result does seem reasonable.

domenic avatar Jun 03 '23 02:06 domenic

Supporting light dismiss for dialog seems nice. I wonder if we're able to make it work by default, or does that break content that implement their own light dismiss?

zcorpan avatar Jun 05 '23 14:06 zcorpan

Note one nuance, which is different from popover: since there's no concept of "nested" dialogs

Can you expand on this? Intuitively, there's definitely a concept of nesting, e.g. a dialog opened because of an action taken inside another open dialog. I guess popover's notion of nesting is different than this, and that's how you're using the term "nesting"?

Sure. Dialogs can certainly be "stacked" like this, but different from Popover, there's no concept of "nested" popovers. The difference is mainly that Popover has complicated rules about how they're related. In the case of dialog, one dialog can open another dialog, but we don't have logic to connect them to each other. But I think that's ok: dialogs usually aren't used in a "nested" fashion, and when they are, the second dialog should be considered a standalone modal dialog. So one dialog should be dismissed at a time. Example: dialog 1 is "Do you want to save this file?", and on a "no" click, a second dialog shows "Are you sure??" which might cover dialog 1. Clicking outside should just light dismiss the topmost one, and leave the original modal visible.

Supporting light dismiss for dialog seems nice. I wonder if we're able to make it work by default, or does that break content that implement their own light dismiss?

I'm guessing that's very non-web-compatible. Since the existing dialog requires an explicit close, I bet there are dialogs that don't want to be light dismissed. A light dismiss dialog might not even be the majority use case for modal dialogs: I could see several types of modal dialog that really want explicit action. Like "Choose A or B" - cancel is not an option.

mfreed7 avatar Jun 05 '23 16:06 mfreed7

agreed. light dismiss for modal dialogs should be an opt in, not a default,

scottaohara avatar Jun 05 '23 16:06 scottaohara

+100 we need this feature. And combined with https://github.com/whatwg/html/issues/3567 I think this will solve a lot of the issues right now with popover vs. dialog choices.

una avatar Jun 15 '23 15:06 una

One comment that came up in discussion was that the light dismiss behavior should differ from Popover in one respect: if the user clicks outside the dialog to light-dismiss it, that click should not "fall through" to the element underneath the backdrop. That might come for free depending on how we spec/implement this, and how it interacts with inert, but I think it's important to call out so we get that behavior right.

mfreed7 avatar Jun 15 '23 15:06 mfreed7

See https://github.com/openui/open-ui/issues/834 for a quick summary of the TPAC discussion just now, and a new question to discuss the name of the attribute.

mfreed7 avatar Sep 14 '23 14:09 mfreed7

Apologies if this is mentioned in the TPAC discussion or elsewhere that I've missed but I was under the impression the new proposed close watcher concept had a scenario where the nested dialogs could in fact all be closed at once I think it was if they were created without user activation they'd be grouped. I think it's worth maintaining this behaviour for the lightdismiss attribute.

lukewarlow avatar Sep 14 '23 18:09 lukewarlow

Apologies if this is mentioned in the TPAC discussion or elsewhere that I've missed but I was under the impression the new proposed close watcher concept had a scenario where the nested dialogs could in fact all be closed at once I think it was if they were created without user activation they'd be grouped. I think it's worth maintaining this behaviour for the lightdismiss attribute.

This is automatic for all "close requests", either popup or dialog or CloseWatcher. It's actually separate from the "light dismiss" behavior, which is triggered when clicking or touching outside of the dialog/popover.

In today's TPAC discussion, we called the Esc key behavior "heavy dismiss" (which all dialogs have), to contrast it with the clicking-outside behavior "light dismiss" (which only <dialog lightdismiss>, or whatever the better name is, would have).

Note that the HTML spec that got merged is a bit confused about this. https://html.spec.whatwg.org/#popover-light-dismiss lists the Esc key behavior under "canceling popovers"; that triggers "hide popover", not "light dismiss open popovers", which is correct. But the Note says that Esc is a subset of light dismiss, and the "canceling popovers" behavior is under the "Popover light dismiss" heading, so the non-normative aspects are confusing. I will add a fix to that to #9462, to make it clear that they are separate.

domenic avatar Sep 14 '23 21:09 domenic

Ah yeah that makes sense. I guess in that case my question should be. Should that behaviour be matched by this lightdismiss definition. If you have multiple dialogs opened without user activation should their light dismiss group such that when the ::backdrop is clicked they all close?

It might be odd if escape or back swipe closes them all but you have to manually click out of all of them?

lukewarlow avatar Sep 14 '23 21:09 lukewarlow

Made a demo page with a "polyfill" https://demo.lukewarlow.dev/lightdismiss/

lukewarlow avatar Oct 02 '23 17:10 lukewarlow

In the poll, many web developers are suggesting enumerated variants like closetrigger="backdrop delay blur etc".

At first I thought this was not a good idea, because we wanted dialogs to have a baseline close trigger of a close request. So the idea was just to add something that says "also give me close-when-clicking-on-backdrop". For which a boolean attribute name was most helpful.

However, in https://bugs.chromium.org/p/chromium/issues/detail?id=1504283 I found out that there are developers using <dialog> today which do not want the dialog to respond to close requests at all. In retrospect, this makes some sense; some dialogs are truly in-your-face important and don't want to be easily dismissed.

So now I think a model something like the following might work?

  • closetrigger="" (no automatic close trigger, can only be closed programatically)
  • closetrigger="closerequest" (closes on a close request, i.e. Esc key / back gesture. Default behavior.)
  • closetrigger="backdropclick" (closes when clicking on the backdrop)
  • closetrigger="closerequest backdropclock" (closes on either)

I'm not 100% satisfied with this API design, because closetrigger="" is a weird way of saying "no close trigger". (But if we added an explicit token, e.g. none, then what does closetrigger="none closerequest" do?) And the default being a specific token is also a bit weird. Maybe there's a better precedent somewhere in the attribute index that avoids these weirdnesses? But it's at least a starting point.

(Regarding naming: as stated before, I think the name needs to include close. And, I quite like connecting up to the backdrop concept, which is exposed through ::backdrop. But I'm not certain on details like closetrigger="" vs. closeon="", or backdropclick vs. backdrop, or closerequest vs. request.)

domenic avatar Nov 22 '23 03:11 domenic

I would want to avoid click in the name because it ties it to a specific input type.

I also would want to avoid closeon because it's too similar to onclose.

How about closes=""?

Then can do closes="closerequest backdrop"

closetrigger(s) I think is also fine.

While the empty list might be confusing I think it's probably fine (compared to any alternativs I can think of)?

lukewarlow avatar Nov 22 '23 15:11 lukewarlow

I agree with @lukewarlow on avoiding closeon for the reason he stated... I think that's a strong argument against.

I think closes is appealingly short but also seems kinda wrong way around... it makes it sounds like the dialog is doing the closing of some other things, rather than respecting various close triggers. It's more like closers than closes if that makes sense... Idk, I guess closetrigger is ... accurate? But I kind of hate it.

bkardell avatar Nov 22 '23 15:11 bkardell

closers is quite nice and short.

lukewarlow avatar Nov 22 '23 15:11 lukewarlow

(But if we added an explicit token, e.g. none, then what does closetrigger="none closerequest" do?) And the default being a specific token is also a bit weird. Maybe there's a better precedent somewhere in the attribute index that avoids these weirdnesses? But it's at least a starting point.

I think an explicit closetrigger=none would be ideal, and closetrigger=closerequest being the default seems also fine. Precedent exists for mutually exclusive sets of DOMTokens in the iframe sandbox attribute (whether or not that's a good design pattern and/or one we want to propagate is up for debate):

The allow-top-navigation and allow-top-navigation-by-user-activation keywords must not both be specified, as doing so is redundant; only allow-top-navigation will have an effect in such non-conformant markup.

Similarly, the allow-top-navigation-to-custom-protocols keyword must not be specified if either allow-top-navigation or allow-popups are specified, as doing so is redundant.

I will say I'd estimate the order of popularity of these would be:

  • closetrigger="backdrop closerequest"
  • closetrigger="none"
  • closetrigger="closerequest"

Having the most desirable be the most boilerplate is not the best.

keithamus avatar Nov 22 '23 15:11 keithamus

Agree re closeon, not very keen on closers or closes, they seem fairly vague and therefore not intuitive. I like closetrigger, agreed with Brian it's accurate, and it's clear what to expect as the value.

hidde avatar Nov 22 '23 15:11 hidde

I'm not sure about "backdrop" either. This isn't how popovers work, and I'd like to keep the same "click/tap outside" behavior that popover has. For example, clicking outside this dialog should still close it, even though you're not clicking on the backdrop:

<dialog closetrigger="backdrop">
<style>
dialog::backdrop {
  width:10px;
  height:10px;
}
</style>

same with this:

<dialog closetrigger="outside">
<style>
dialog::backdrop {
  pointer-events:none;
}
</style>

Generally, as I mentioned here, I really don't want to start adding other ways to close the dialog. We spent considerable time ruling out "dismiss-on-scroll" or "dismiss-on-blur", so I'd hope we don't add those as values. They will be easy footguns for most people. For the same avoidance-of-footguns reason, I don't see any use case for a dialog that closes when you click/tap outside it, but does not close when you hit ESC.

So there really are just three values/cases, right?

  1. No automatic close at all (including ESC)
  2. Just ESC closes the dialog (current behavior)
  3. ESC or clicking/tapping outside the dialog closes it

How about:

  1. closetrigger=none
  2. closetrigger=closerequest
  3. closetrigger=any

mfreed7 avatar Nov 22 '23 18:11 mfreed7

Has closedby="…" been considered as a name?

js-choi avatar Nov 23 '23 02:11 js-choi

I'm not sure about "backdrop" either. This isn't how popovers work, and I'd like to keep the same "click/tap outside" behavior that popover has. For example, clicking outside this dialog should still close it, even though you're not clicking on the backdrop:

Interesting point. I agree backdrop is technically not correct in this way. I still think it might be a reasonable way for developers to think about it, because in the default case clicking outside = clicking the backdrop. But if you think it's too misleading, let's go with outside or something of that sort.

I don't see any use case for a dialog that closes when you click/tap outside it, but does not close when you hit ESC.

This is a good point, and is probably right. However I've already been surprised once by learning that developers sometimes want dialogs that don't respond to close requests. So if anyone disagrees, or can find a dialog of this sort in the web or native platforms, that'd be very helpful info!

How about:

LGTM. (And avoids us having to choose between backdrop and outside!)

The following possible tweaks come to mind, but I think I prefer your proposal as-is. I just want to write them down in case others have strong feelings.

  • closetrigger=request instead of closetrigger=closerequest
  • closetrigger=all instead of closetrigger=any
  • Different names than closetrigger, e.g. closedby, autoclose, closers, etc.

domenic avatar Nov 23 '23 02:11 domenic

Am I correct in thinking that escape doesn't actually close a non-modal (non-popover) dialog currently. Is that gonna impact on the serialising of the default?

lukewarlow avatar Nov 23 '23 12:11 lukewarlow

LGTM. (And avoids us having to choose between backdrop and outside!)

Thanks - glad you agree.

The following possible tweaks come to mind, but I think I prefer your proposal as-is. I just want to write them down in case others have strong feelings.

  • closetrigger=request instead of closetrigger=closerequest

At first I liked this one better, but it does make you wonder what kind of "request". Whereas "closerequest" is explicit.

  • closetrigger=all instead of closetrigger=any

I don't really like this one, because it implies you need all of the triggers to close.

  • Different names than closetrigger, e.g. closedby, autoclose, closers, etc.

I kind of like closedby - shorter and to the point. autoclose isn't bad either. I think perhaps I'll bring this question back to the poll to see where people stand?

Am I correct in thinking that escape doesn't actually close a non-modal (non-popover) dialog currently. Is that gonna impact on the serialising of the default?

You are correct that non-modal dialogs don't get closed by ESC. But I don't think there's a serialization problem here - if a dialog doesn't wear closetrigger, then it won't get serialized. Right? And I would assume the IDL for closeTrigger would return null.

mfreed7 avatar Nov 28 '23 17:11 mfreed7

I created a couple more polls for naming:

https://github.com/openui/open-ui/discussions/960 https://github.com/openui/open-ui/discussions/961

mfreed7 avatar Nov 28 '23 17:11 mfreed7

Am I correct in thinking that escape doesn't actually close a non-modal (non-popover) dialog currently. Is that gonna impact on the serialising of the default?

You are correct that non-modal dialogs don't get closed by ESC. But I don't think there's a serialization problem here - if a dialog doesn't wear closetrigger, then it won't get serialized. Right? And I would assume the IDL for closeTrigger would return null.

I'm a little confused on what the proposal is for modeless (non-modal) dialogs. I know we're vaguely thinking of deprecating them (https://github.com/whatwg/html/issues/9376) and their semantics and behavior are unclear anyway (https://github.com/whatwg/html/issues/7707). But while they still exist, we need to have a coherent proposal.

I feel like there are two possibilities?

  1. closetrigger="" has no impact on the behavior of modeless dialogs.
    • If desired, we can reflect this in the IDL, so that if the dialog is open and in the modeless state, dialogEl.closeTrigger starts returning null. That seems weird to me though.
  2. closetrigger="" does impact the behavior of modeless dialogs.
    • It basically behaves the same as with modal dialogs, however the misssing value default / invalid value default behavior is different. For modeless, the default is "none", whereas for modal, the default is "closerequest".

If we choose (1), it's possible the name of this attribute should include modal, e.g. modalclosetrigger="". But I'd be OK omitting it; I think on balance the verbosity is not worth it.

(2) seems pretty nice and natural. The downside, I guess, is that it contributes further to the confusing nature of modeless dialogs, given them even more capabilities.

There may be other possibilities too, that I'm not thinking of.

domenic avatar Nov 29 '23 01:11 domenic

There is also a complication about what <dialog popover closetrigger=""> does.

keithamus avatar Nov 29 '23 09:11 keithamus

Should closetrigger allow you to control popovers too?

Manual popovers require JS currently but that might be heavy handed if you just wanted close request but not click outside?

Don't want to over complicate things though.

lukewarlow avatar Nov 29 '23 11:11 lukewarlow

(2) seems pretty nice and natural. The downside, I guess, is that it contributes further to the confusing nature of modeless dialogs, given them even more capabilities.

I'm inclined to agree with you that (2) seems to be the most natural. It keeps the existing behavior for non-modal dialogs unless closetrigger is added, at which point it gives parity with modal dialog behavior. Even though we're hoping to maybe deprecate non-modal dialog, I don't think it's a problem to keep parity on this feature in the meantime. Is there precedent for a missing value default that depends on state? Or do you see any issue with adding that, if not?

There is also a complication about what

does.

Here, I think the behavior is pretty clear. If the dialog is open as a dialog, closetrigger adds light dismiss behaviors. If it's open as a popover, then the light dismiss behavior is already there and shouldn't be modified by closetrigger. This is equivalent to the statement that the open attribute on <dialog popover open> doesn't change any popover behavior, it applies just to the dialog open as a dialog. Said another way, I don't think popovers that are dialogs should get special behavior - they should just get the normal popover behavior that any other element gets.

mfreed7 avatar Nov 29 '23 16:11 mfreed7

Is there precedent for a missing value default that depends on state? Or do you see any issue with adding that, if not?

There is precedent; in fact there are three separate types of precedent :cry:.

1A: have the states be { none, closerequest, any, auto }, with an invalid value default and missing value default of auto, but no actual keyword for triggering the auto behavior. (The keywords are just { none, closerequest, any }.) Do the reflection "limited to only known values". In this version, whenever you are in the auto state (including when closetrigger="asdf"), you get dialogEl.closeTrigger === "". This is the model used by dir=""and shadowrootmode="".

1B: similar to 1A, but also declare the IDL attribute as nullable. Then instead of the empty string when in the auto state, you get null. This is the model used by popover="" and crossorigin="".

2: The other would be to have the states and keywords both match, as { none, closerequest, any }, and then do custom reflection. This reflection would allow dialogEl.closeTrigger to always return the "actual" close trigger currently in play, changing over time I guess so that by default closetrigger="asdf" maps to dialogEl.closeTrigger === "none", and then after dialogEl.showModal(), it starts returning dialogEl.closeTrigger === "closerequest". Some version of this custom-reflection model is used by translate="", hidden="", contenteditable="", spellcheck="", autocapitalize="", draggable=""

I think 1A is probably better than 1B because we reserve 1B for cases where the empty string is valid (<div popover> and <img crossorigin>). Whereas for closetrigger="" the empty string is not valid.

Between 1A and 2, I lean toward 1A. Although it's kind of nice that in 2 you get some insight into the "actual" close triggers, it's a bit strange how it causes dialogEl.closeTrigger to change over time. But I don't feel strongly.

I recall @annevk having stronger opinions on this, so I'd like his take.

domenic avatar Nov 30 '23 03:11 domenic

Thanks for the thorough description of the options!

Between 1A and 2, I lean toward 1A. Although it's kind of nice that in 2 you get some insight into the "actual" close triggers, it's a bit strange how it causes dialogEl.closeTrigger to change over time. But I don't feel strongly.

Any of the options sound ok to me, but 1A does sound the best overall. Option 2 sounds somewhat odd, and the developer can already check dialog.matches(':modal') to see what the behavior would be if dialog.getAttribute('closetrigger')===null. So +1 to option 1A.

mfreed7 avatar Dec 04 '23 17:12 mfreed7