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

Should we deprecate dialog.show()? #9376

Open
mfreed7 opened this issue Jun 2, 2023 · 7 comments
Open

Should we deprecate dialog.show()? #9376

mfreed7 opened this issue Jun 2, 2023 · 7 comments
Labels
removal/deprecation Removing or deprecating a feature topic: dialog The <dialog> element.

Comments

@mfreed7
Copy link
Collaborator

mfreed7 commented Jun 2, 2023

Now that the Popover API has landed, I'm not sure what the use case for non-modal <dialog> is. Comparing the two options:

  1. <dialog open> I'm a non-modal dialog </dialog>
  2. <dialog popover> I'm a non-modal dialog </dialog>

Both represent dialogs, and both are intended to be non-modal. Neither, therefore, inert the rest of the page. However, the popover version comes with several very nice-to-have features that I believe most developers prefer:

  1. <dialog popover> is displayed in the top-layer, so you don't have to do tricks to make sure it's on top of things and non-transformed/clipped.
  2. <dialog popover> comes with an easy declarative way to trigger, via <button popovertarget=foo>.
  3. <dialog popover> comes with light dismiss behavior, so clicking outside or hitting ESC closes it. (<dialog popover=manual> can be used if light dismiss is undesirable.)
  4. <dialog popover> comes with the beforetoggle event, which fires both when showing and hiding the dialog.

Given the advantages of the popover version, and no disadvantages that I can see, I'm not sure why developers should ever choose the non-popover version, when creating a non-modal dialog.

I think we should therefore deprecate dialog.show() and simplify things for developers. The guidance would then be:

  1. For modal dialogs, use <dialog>.showModal().
  2. For non-modal dialogs, use <dialog popover>.

(See #3567 and #9373 for efforts to bring more of the popover goodness to modal dialog.)

@Link2Twenty
Copy link

In current implementations what does this do?

<dialog id="example" popover>
  Content
</dialog>
document.querySelector("#example").showPopover()

If it just works do we need to deprecate dialog.show()?

@mfreed7
Copy link
Collaborator Author

mfreed7 commented Jun 2, 2023

If it just works do we need to deprecate dialog.show()?

Yes, that just works. And that's why I think we should deprecate dialog.show() since dialog.showPopover() is strictly better.

@keithamus keithamus added the removal/deprecation Removing or deprecating a feature label Jun 2, 2023
@domenic
Copy link
Member

domenic commented Jun 3, 2023

I think the biggest question mark here is whether you actually always want non-modal dialog things in top layer.

Non-modal dialogs in the old-school Windows/Mac sense are pretty rare these days. But, they were definitely not in the top layer: e.g., clicking on the main window would cause them to end up behind the main window.

Modern uses of "non-modal dialogs" seem pretty unlike old-school ones, and I'm still somewhat fuzzy on what things we'd want to classify as non-modal dialogs. But looking at the cases under "Consider Nonmodal Dialogs Instead" from https://www.nngroup.com/articles/modal-nonmodal-dialog/ , some examples might be "sign up for my newsletter", cookie banners, or Gmail's compose window.

I don't think those things should be in the top layer? In particular, consider how they should interact with things like a modal dialog that was opened before those items were shown, or if you want to drag-and-drop something from your Gmail inbox to your Gmail compose window. They should be below the modal dialog, or below the being-dragged item. Right?

(I'll also note that probably none of those want light-dismiss. Even if as users we might wish for the first two to be light-dismissable...)

This kind of investigation is what I was hoping people would dig into with #4633... finding all the things that could go in top layer, and creating a classification system for them.

@keithamus
Copy link
Contributor

@domenic for GitHub’s use case we consider non modal dialogs stuff like the labels, assignees, reviewers panels, or the panel that opens when you click “Review changes” in a PR. These should have a role=dialog (or use the <dialog> element) but we want them to appear above all page content, even floating banners like the notification header widget. <dialog popover> fits our use case perfectly, and better than a non modal <dialog> precisely because of the benefits of top layer. These dialogs also need light dismiss, which dialog doesn’t offer but popover does. In short- for our use case there’s zero reason to use non modal dialog over <dialog popover> (or modal dialogs).

@mbgower
Copy link

mbgower commented Jul 27, 2023

Given the advantages of the popover version, and no disadvantages that I can see, I'm not sure why developers should ever choose the non-popover version, when creating a non-modal dialog.

"non-modal dialog" can mean a lot of different things these days, but the way ARIA uses the term is something that retains a closed tab ring (i.e, tab order loops within that overlay), while allowing some method of keyboard release to other parts of the screen (I've seen F6 used).

So, I see possible uses for things like palettes and toolbars in rich composition/authoring tool situations. Thus a user has an ability to move between discrete regions of a page within which each contains the tab ring.

Whether or not dialog is the proper category for such 'region' uses is a different discussion. But my main caution is that what is here described as 'light dismiss' behaviour seems to be contestable in some quarters as representing what 'non-modal' truly conveys.

Here's the pertinent part from the APG

Like non-modal dialogs, modal dialogs contain their tab sequence. That is, Tab and Shift + Tab do not move focus outside the dialog. However, unlike most non-modal dialogs, modal dialogs do not provide means for moving keyboard focus outside the dialog window without closing the dialog.

@scottaohara
Copy link
Collaborator

@mbgower understood your points, re: a closed tab ring - there's this issue #7707 - where this commentary would be more applicable.

re: light dismiss, that's something people do now with JS for modal/non-modal dialogs - where a mouse click or tap outside of the dialog can close it. but this issue isn't saying that people would have to do that, because popover=manual could be used instead which does not have the light dismiss behavior as a default.

@lukewarlow
Copy link
Member

lukewarlow commented Nov 9, 2023

If the intention is to unify non-modal dialogs I think the current issue, that's alluded to in your code example, is there's no declarative way to initially open a popover.

If we have to keep <dialog open> for that use case, is removing .show() useful? Could it potentially be more confusing?

Especially because you could just do setAttribute('open', '') and get the show equivalent. It wouldn't simplify the html spec much at all? If that's one of the desired outcomes?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
removal/deprecation Removing or deprecating a feature topic: dialog The <dialog> element.
Development

No branches or pull requests

7 participants