github / details-dialog-element

A modal dialog that's opened with <details>.

<details-dialog> element

A modal dialog opened with a <details> button.

DEPRECATION WARNING

This web component has been deprecated. There are a number of accessibility concerns with this approach and so we no longer recommend using this component.

Accessibility and Usability Concerns

• Semantically, using a details-summary pattern for a dialog solution can be confusing for screen reader users.
• If the user performs a "find in page" operation on a website using details-dialog elements, the content on those elements will appear when they shouldn't.
• Opening the dialog does not disable scrolling on the underlying page.

GitHub are moving towards using a dialog Primer View Component which enforces certain aspects of the design (such as always having a close button and a title).

Installation

Available on npm as @github/details-dialog-element.

$ npm install --save @github/details-dialog-element

Usage

Script

Import as ES modules:

import '@github/details-dialog-element'

Include with a script tag:

<script type="module" src="./node_modules/@github/details-dialog-element/dist/index.js">

Markup

<details>
  <summary>Open dialog</summary>
  <details-dialog>
    Modal content
    <button type="button" data-close-dialog>Close</button>
  </details-dialog>
</details>

Deferred loading

Dialog content can be loaded from a server by embedding an [<include-fragment>][fragment] element.

<details>
  <summary>Robots</summary>
  <details-dialog src="/robots" preload>
    <include-fragment>Loading…</include-fragment>
  </details-dialog>
</details>

The src attribute value is copied to the <include-fragment> the first time the <details> button is toggled open, which starts the server fetch.

If the preload attribute is present, hovering over the <details> element will trigger the server fetch.

Events

details-dialog-close

details-dialog-close event is fired from <details-dialog> when a request to close the dialog is made from

• pressing escape,
• submitting a form[method="dialog"]
• clicking on summary, form button[formmethod="dialog"], [data-close-dialog], or
• dialog.toggle(false)

This event bubbles, and can be canceled to keep the dialog open.

document.addEventListener('details-dialog-close', function(event) {
  if (!confirm('Are you sure?')) {
    event.preventDefault()
  }
})

Browser support

Browsers without native custom element support require a polyfill.

• Chrome
• Firefox
• Safari
• Microsoft Edge

License

Distributed under the MIT license. See LICENSE for details.

details-dialog-element's Issues

Post-close event to manage focus

Hello friends 👋

I am working in a system that uses websockets to process live updates and replace chunks of the DOM. In order for this live update to be successful, the element being replaced cannot have any interactions.

Because details-dialog is managing focus after it is closed, I get stuck in this workflow with an activeElement:

1. Click "Close" button
2. details-dialog opens and user confirms the behavior is desired by clicking another "Close" button
3. Form is submitted
4. details-dialog closes
5. Live update fails to replace the "Close" button from step 1 fails because details-dialog has called focus() on that button

Normally I would call blur() here, but we are removing blur() from the code base where possible to improve accessibility.

🌮 ty for your help!!

INPUT_SELECTOR limits use of custom buttons

Consider:

<details id="update-dialog" hidden>
  <summary>New Version</summary>
  <details-dialog>
    <h1>New Version Available!</h1>
    <p>Reload the Page?</p>
    <mwc-button id="dialog-reload">OK</mwc-button>
    <mwc-button data-close-dialog>Cancel</mwc-button>
  </details-dialog>
</details>

This will fail entirely, since the requisite inputs are not found when the element boots up.

By adding mwc-button to the INPUT_SELECTOR, expected behaviour is restored.

I recommend adding a special attribute to the list, like so:

<details id="update-dialog" hidden>
  <summary>New Version</summary>
  <details-dialog>
    <h1>New Version Available!</h1>
    <p>Reload the Page?</p>
    <mwc-button data-dialog-input id="dialog-reload">OK</mwc-button>
    <mwc-button data-dialog-input data-close-dialog>Cancel</mwc-button>
  </details-dialog>
</details>

Test for managing focus is failing

https://github.com/github/details-dialog-element/runs/5240152268?check_suite_focus=true

This test appears to have been failing since https://github.com/github/details-dialog-element/actions/runs/1766435305 but it looks like older runs may have been failing silently as tests were not running. See for example this run with missing logs https://github.com/github/details-dialog-element/runs/3652343581?check_suite_focus=true

Should Dialog let contents overflow outside?

When an element inside <details-dialog> is longer than the dialog, it results in a scroll on the dialog which isn't ideal:

overflow-auto.mov

This can be reproduced in the Dialog with <include-fragment> example: https://github.github.io/details-dialog-element/example/index.html

Screen.Recording.2022-03-16.at.2.36.28.PM.mov

The cause can be narrowed down to a overflow:auto index.css#L12

Removing that line, does make it behave nicely. But, I'm not sure what would break instead OR if there are any tradeoffs.

without-overflow-auto.mov

More screenshots via @dipree:

Quirks with dialog trigger "button"s

Per the manner in which the summary element is exposed to Safari's Webkit (announced as "summary" but no matching ARIA role) and Blink (announced as and reported as a "DisclosureTriangle"role) these "buttons" are not discoverable when navigating by form controls or button elements with VoiceOver on macOS or iOS.

Additionally, due to poor support for aria-haspopup="dialog" these elements will be exposed as popup buttons for "menus", which, depending on the screen reader will inform a user certain keys will become available to them, which is not true.

I would suggest that role=button is added to the summary elements when JavaScript is enabled to better ensure these elements which are visually presented as buttons are also discoverable as the buttons they are meant to be.

Additionally, I would suggest that aria-haspopup=dialog is removed until it receives better support.

CSS class for disabling escape key?

CSS class for disabling escape key?

Add example for usage in React application

It would be nice to see an example for how to utilize this library from within a TypeScript React application. How do you import a custom component like this?

Open a dialog from JavaScript

I.e. kindly make dialog() from the internal GitHub repository public.

Thank you, this element is awesome!

Opt out (or in?) to close button?

First of all, this is rad! I'm experimenting with it in Primer land, though, and the .dd-close-button that it adds automatically is giving me grief. Could we make it possible to opt out of that button if we're providing one with the data-close-dialog attribute, or maybe just check to see if that selector is found before creating it?

Option to preventScroll on focus

I’m running into a problem where the calls to focus() are jumping the scroll around on my page. Specifically in Chrome with the dialogs inside of a sticky header. Changing .focus() to .focus({ preventScroll: true }) fixes the issue.

I’d love to have a prevent-scroll attribute on the <details-dialog> to opt into this behavior.

Dialog with include-fragment is not working

If I go to the examples page and click the dialog with <include-fragment> it is not loading content.html, I have tried in both Chrome and FF.

The reason why it is not working is that the attributeChangedCallback is called before connectedCallback, this results in a early return in attributeChangedCallback because the element is not yet connected and therefore the parentNode of the details-dialog element is NULL. Because of this the events that call loadIncludeFragment are never added.

I don't know if this is the desired callback lifecycle of custom elements, as this is the first time I've dived into them. To be honest it seems weird that the attributeChangedCallback is called at all, when no attributes have changed. I would have expected only the connectedCallback to be called when the elements are inserted into the DOM and then attributeChangedCallback to be called when attributes are changed.

I can make a PR that solves the issue. I would extract the body of attributeChangedCallback into a function and then call that function in both attributeChangedCallback and connectedCallback. Please let me know what you think.

Cut a new version?

Hey @muan 👋 😸

Can we cut a new version and update this in github/github? I think we are getting errors on production because we don't have the !window.customElements.get('details-dialog') guard in the version we are running.

details-dialog-element/index.js

Lines 98 to 101 in 9901ae6

	if (!window.customElements.get('details-dialog')) {
	window.HTMLDetailsDialogElement = DetailsDialogElement
	window.customElements.define('details-dialog', DetailsDialogElement)
	}

I can also do it but I wasn't sure since this seems to be using the new github registry 📦

Tab trapping doesn't know elements are hidden by child <details>

Demonstrated here: https://html-is.glitch.me/throwaway/dialog.html

We'd try to focus on details-dialog > details > input and it silently fails. It'd be good if there's a way to know if .focus() failed and move on to the next element if so.

Relevant to http://github.com/whatwg/html/issues/4464.

"You can click anywhere to dismiss without JS" is not anywhere

Just a follow up that, in the demo for Bare, shows up a message saying "But you can click anywhere to dismiss without JS.".

In fact, if you click on this text, isn't dismissed. 😋

Initial focus should be set to the first focusable element

details-dialog-element/src/index.ts

Lines 10 to 17 in 59902d7

	function autofocus(el: DetailsDialogElement): void {
	let autofocusElement = Array.from(el.querySelectorAll<HTMLElement>('[autofocus]')).filter(focusable)[0]
	if (!autofocusElement) {
	autofocusElement = el
	el.setAttribute('tabindex', '-1')
	}
	autofocusElement.focus()
	}

Initial focus should be set to the first focusable element instead of the dialog element.

@jscholes: Placing focus on the close button is the correct behaviour if the dialog has an accessible name via aria-label or aria-labelledby, which every dialog should. So a screen reader will announce the name, the role of dialog, then the close button.

The focus priority should be as follows:

1. element with [autofocus]
2. first focusable element if it's in the beginning of the dialog (usually the close button)
3. first element

Q: Selection of static type checker

I think this is a good example of custom element composition. I was just wondering, why was flow chosen over typescript for example?