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 Dialog .show() close all popovers? #10708

Open
keithamus opened this issue Oct 17, 2024 · 2 comments
Open

Should Dialog .show() close all popovers? #10708

keithamus opened this issue Oct 17, 2024 · 2 comments
Labels
needs implementer interest Moving the issue forward requires implementers to express interest needs tests Moving the issue forward requires someone to write tests topic: dialog The <dialog> element topic: popover The popover attribute and friends

Comments

@keithamus
Copy link
Contributor

What is the issue with the HTML Standard?

Dialog's show method: https://html.spec.whatwg.org/multipage/interactive-elements.html#dom-dialog-show

  1. If this has an open attribute and the is modal flag of this is false, then return.
  2. If this has an open attribute, then throw an "InvalidStateError" DOMException.
  3. Add an open attribute to this, whose value is the empty string.
  4. Set this's previously focused element to the focused element.
  5. Let hideUntil be the result of running topmost popover ancestor given this, null, and false.
  6. If hideUntil is null, then set hideUntil to this's node document.
  7. Run hide all popovers until given hideUntil, false, and true.
  8. Run the dialog focusing steps given this.

Step 6 & 7 show that a dialog, not shown as modal, will close all popovers.

Dialog's show() method also does not check for connectedness. This means in an active document, the following script can cause all popovers to hide:

document.createElement('dialog').show()

This seems strange as there's no observable change to UI being performed by the Dialog, and yet it is closing all popovers as a side effect.

Perhaps between step 3 and 4 we should add a check like: If this is not connected, then return.?

/cc @josepharhar

@keithamus keithamus added needs implementer interest Moving the issue forward requires implementers to express interest needs tests Moving the issue forward requires someone to write tests topic: dialog The <dialog> element topic: popover The popover attribute and friends labels Oct 17, 2024
@smaug----
Copy link

I'd expect consistency with showModal() which has the connected check.

@domenic
Copy link
Member

domenic commented Oct 18, 2024

We could roll this into #10705

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs implementer interest Moving the issue forward requires implementers to express interest needs tests Moving the issue forward requires someone to write tests topic: dialog The <dialog> element topic: popover The popover attribute and friends
Development

No branches or pull requests

3 participants