red-hat-design-system icon indicating copy to clipboard operation
red-hat-design-system copied to clipboard

feat(dialog): use native HTML dialog element

Open adamjohnson opened this issue 1 year ago â€ĸ 5 comments

What I did

  1. Change internal structure of rh-dialog to use the native HTML <dialog> element.
  2. Overhaul much of the CSS.
  3. Change how we handle #overlay to use the CSS native ::backdrop psuedo-element.
  4. Left notes about deprecating #overlay
  5. Added the accessible-label property for when users want to manually label their modals + added docs.
  6. Modal now has logic to decide when to use accessible-label, aria-labelledby + IDREF, and the aria-label of the trigger element.
  7. Improved layout of modal contents when scrollbars are present
  8. Increased the touch target of the close modal button.
  9. Trap focus inside the modal for improved accessibility (WIP).

Testing Instructions

  1. Visit the DP demo.
  2. Add all kinds of content to the modal header, body, and footer, see if you can break it.
  3. See if focus trapping works with various different interactive elements (or none at all).

To Do's

  • [x] Fix Shift + Tab focus trapping
  • [ ] Fix focus trap with Tab in video modal with YouTube embed
  • [ ] Update/improve changeset
  • [ ] Test across different browsers and AT
  • [x] Look into why trigger/rh-button displays two buttons (one without a label) in the DP but not locally

Notes to Reviewers

Some docs on the Guidelines page are incorrect as they have the modal body scrolling independently from the header and footer. That needs updated.

WIP right now. Come back later for a more accurate description. 😇

Closes #1242 and #1238.

adamjohnson avatar Sep 13 '24 21:09 adamjohnson

đŸĻ‹ Changeset detected

Latest commit: a8df42a08251cb7f325ec18214e0645031c86673

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@rhds/elements Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

changeset-bot[bot] avatar Sep 13 '24 21:09 changeset-bot[bot]

Deploy Preview for red-hat-design-system ready!

Name Link
Latest commit a8df42a08251cb7f325ec18214e0645031c86673
Latest deploy log https://app.netlify.com/sites/red-hat-design-system/deploys/66fffc042cb6e600089aeddd
Deploy Preview https://deploy-preview-1865--red-hat-design-system.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

netlify[bot] avatar Sep 13 '24 21:09 netlify[bot]

Size Change: +785 B (+0.43%)

Total Size: 184 kB

Filename Size Change
./elements/rh-dialog/rh-dialog.js 5.57 kB +785 B (+16.42%) âš ī¸
â„šī¸ View Unchanged
Filename Size
./elements.js 473 B
./elements/rh-accordion/context.js 162 B
./elements/rh-accordion/rh-accordion-header.js 2.77 kB
./elements/rh-accordion/rh-accordion-panel.js 1.37 kB
./elements/rh-accordion/rh-accordion.js 3.17 kB
./elements/rh-alert/rh-alert.js 3.93 kB
./elements/rh-audio-player/rh-audio-player-about.js 1.85 kB
./elements/rh-audio-player/rh-audio-player-rate-stepper.js 1.85 kB
./elements/rh-audio-player/rh-audio-player-scrolling-text-overflow.js 1.53 kB
./elements/rh-audio-player/rh-audio-player-subscribe.js 1.43 kB
./elements/rh-audio-player/rh-audio-player.js 13.2 kB
./elements/rh-audio-player/rh-cue.js 2 kB
./elements/rh-audio-player/rh-transcript.js 2.75 kB
./elements/rh-avatar/random-pattern-controller.js 2.72 kB
./elements/rh-avatar/rh-avatar.js 2.9 kB
./elements/rh-back-to-top/rh-back-to-top.js 2.1 kB
./elements/rh-badge/rh-badge.js 1.1 kB
./elements/rh-blockquote/rh-blockquote.js 1.4 kB
./elements/rh-breadcrumb/rh-breadcrumb.js 1.5 kB
./elements/rh-button/rh-button.js 4.24 kB
./elements/rh-card/rh-card.js 3.48 kB
./elements/rh-code-block/prism.css.js 725 B
./elements/rh-code-block/prism.js 556 B
./elements/rh-code-block/rh-code-block.js 6.93 kB
./elements/rh-cta/rh-cta.js 3.89 kB
./elements/rh-dialog/yt-api.js 617 B
./elements/rh-footer/rh-footer-block.js 766 B
./elements/rh-footer/rh-footer-copyright.js 362 B
./elements/rh-footer/rh-footer-links.js 1.17 kB
./elements/rh-footer/rh-footer-social-link.js 964 B
./elements/rh-footer/rh-footer-universal.js 4.05 kB
./elements/rh-footer/rh-footer.js 5.01 kB
./elements/rh-health-index/rh-health-index.js 2.35 kB
./elements/rh-icon/rh-icon.js 2.36 kB
./elements/rh-icon/ssr.js 181 B
./elements/rh-menu/rh-menu.js 1.29 kB
./elements/rh-navigation-secondary/rh-navigation-secondary-dropdown.js 2.47 kB
./elements/rh-navigation-secondary/rh-navigation-secondary-menu-section.js 1.35 kB
./elements/rh-navigation-secondary/rh-navigation-secondary-menu.js 1.75 kB
./elements/rh-navigation-secondary/rh-navigation-secondary-overlay.js 571 B
./elements/rh-navigation-secondary/rh-navigation-secondary.js 5.27 kB
./elements/rh-navigation-secondary/test/fixtures.js 769 B
./elements/rh-pagination/rh-pagination.js 5.46 kB
./elements/rh-site-status/rh-site-status.js 3.08 kB
./elements/rh-skip-link/rh-skip-link.js 1.24 kB
./elements/rh-spinner/rh-spinner.js 1.43 kB
./elements/rh-stat/rh-stat.js 2.2 kB
./elements/rh-subnav/rh-subnav.js 2.7 kB
./elements/rh-surface/rh-surface.js 1.11 kB
./elements/rh-surface/test/elements.js 423 B
./elements/rh-switch/rh-switch.js 2.92 kB
./elements/rh-table/rh-sort-button.js 1.49 kB
./elements/rh-table/rh-table.js 3.17 kB
./elements/rh-tabs/context.js 160 B
./elements/rh-tabs/rh-tab-panel.js 1.14 kB
./elements/rh-tabs/rh-tab.js 2.93 kB
./elements/rh-tabs/rh-tabs.js 3.68 kB
./elements/rh-tag/rh-tag.js 2.84 kB
./elements/rh-tile/rh-tile-group.js 1.81 kB
./elements/rh-tile/rh-tile.js 5.16 kB
./elements/rh-timestamp/rh-timestamp.js 983 B
./elements/rh-tooltip/rh-tooltip.js 2.24 kB
./elements/rh-video-embed/rh-video-embed.js 4.6 kB
./lib/context/color/consumer.js 1.23 kB
./lib/context/color/controller.js 947 B
./lib/context/color/provider.js 2.08 kB
./lib/context/event.js 593 B
./lib/context/headings/consumer.js 722 B
./lib/context/headings/controller.js 1.12 kB
./lib/context/headings/provider.js 1.24 kB
./lib/DirController.js 565 B
./lib/elements/rh-context-demo/rh-context-demo.js 1.28 kB
./lib/elements/rh-context-picker/rh-context-picker.js 2.21 kB
./lib/environment.js 4.09 kB
./lib/functions.js 175 B
./lib/I18nController.js 1.38 kB
./lib/ScreenSizeController.js 849 B
./react/rh-accordion/rh-accordion-header.js 199 B
./react/rh-accordion/rh-accordion-panel.js 185 B
./react/rh-accordion/rh-accordion.js 215 B
./react/rh-alert/rh-alert.js 184 B
./react/rh-audio-player/rh-audio-player-about.js 191 B
./react/rh-audio-player/rh-audio-player-rate-stepper.js 213 B
./react/rh-audio-player/rh-audio-player-scrolling-text-overflow.js 214 B
./react/rh-audio-player/rh-audio-player-subscribe.js 196 B
./react/rh-audio-player/rh-audio-player.js 183 B
./react/rh-audio-player/rh-cue.js 195 B
./react/rh-audio-player/rh-transcript.js 207 B
./react/rh-avatar/rh-avatar.js 173 B
./react/rh-back-to-top/rh-back-to-top.js 183 B
./react/rh-badge/rh-badge.js 174 B
./react/rh-blockquote/rh-blockquote.js 179 B
./react/rh-breadcrumb/rh-breadcrumb.js 179 B
./react/rh-button/rh-button.js 174 B
./react/rh-card/rh-card.js 172 B
./react/rh-code-block/rh-code-block.js 181 B
./react/rh-cta/rh-cta.js 170 B
./react/rh-dialog/rh-dialog.js 203 B
./react/rh-footer/rh-footer-block.js 184 B
./react/rh-footer/rh-footer-copyright.js 187 B
./react/rh-footer/rh-footer-links.js 185 B
./react/rh-footer/rh-footer-social-link.js 193 B
./react/rh-footer/rh-footer-universal.js 188 B
./react/rh-footer/rh-footer.js 174 B
./react/rh-health-index/rh-health-index.js 184 B
./react/rh-icon/rh-icon.js 202 B
./react/rh-menu/rh-menu.js 173 B
./react/rh-navigation-secondary/rh-navigation-secondary-dropdown.js 217 B
./react/rh-navigation-secondary/rh-navigation-secondary-menu-section.js 205 B
./react/rh-navigation-secondary/rh-navigation-secondary-menu.js 199 B
./react/rh-navigation-secondary/rh-navigation-secondary-overlay.js 201 B
./react/rh-navigation-secondary/rh-navigation-secondary.js 213 B
./react/rh-pagination/rh-pagination.js 178 B
./react/rh-site-status/rh-site-status.js 181 B
./react/rh-skip-link/rh-skip-link.js 181 B
./react/rh-spinner/rh-spinner.js 175 B
./react/rh-stat/rh-stat.js 171 B
./react/rh-subnav/rh-subnav.js 175 B
./react/rh-surface/rh-surface.js 175 B
./react/rh-switch/rh-switch.js 185 B
./react/rh-table/rh-sort-button.js 213 B
./react/rh-table/rh-table.js 174 B
./react/rh-tabs/rh-tab-panel.js 181 B
./react/rh-tabs/rh-tab.js 187 B
./react/rh-tabs/rh-tabs.js 174 B
./react/rh-tag/rh-tag.js 182 B
./react/rh-tile/rh-tile-group.js 183 B
./react/rh-tile/rh-tile.js 194 B
./react/rh-timestamp/rh-timestamp.js 176 B
./react/rh-tooltip/rh-tooltip.js 175 B
./react/rh-video-embed/rh-video-embed.js 227 B

compressed-size-action

github-actions[bot] avatar Sep 17 '24 17:09 github-actions[bot]

Current status

With a lot of help from @zeroedin, he and I were able to get Shift + Tab mostly working for <rh-dialog>. Check out the DP.

Setbacks

Just when we thought we had it cracked, we added an <rh-tabs> component to the body of the <rh-dialog> and found that it did not trap focus as intended if an <rh-*> element is the last focusable item in the dialog.

Possible solutions

We suspect that we'd have to add each focusable <rh-*> element to the focusable selectors list and possibly add delegatesFocus to each of those elements. We'd then have to maintain this focusable selectors list.

OR we could explore implementing a function like this that recursively traverses the Shadow DOM looking for focusable elements. Good to know we're not the first people to run into complexities with web components and focus traps.

OR we could add one of the off the shelf solutions as a dependency.

If anyone has strong feelings about any of these solutions, weigh in!

Video dialog + YouTube

Note that, on a video dialog with a YouTube embed, Shift + Tab works, but Tab does not trap focus. DP Video dialog demo

NB: Right now on uxdot for a dialog + YouTube embed, focus trapping works with Tab but not Shift + Tab. 🔁 uxdot production video demo

APG Guidelines vs. browser implementations

It's also worth noting that when a <dialog> is open, it does not allow other elements on the page to be focused. APG guidelines explicitly define the required keyboard behavior; however, the native <dialog> element does not follow this guidance. Accessibility practitioners are split as to which behavior is preferrable.

adamjohnson avatar Sep 19 '24 03:09 adamjohnson

Will look into moving <rh-surface> inside the dialog. 👍

If we're using <dialog>, why do we still need kitty's focus trap javascript? Should we just call showModal by default?

We are calling showModal() on line 370. APG keyboard interaction guidelines clearly state:

  • Tab:
    • Moves focus to the next tabbable element inside the dialog.
    • If focus is on the last tabbable element inside the dialog, moves focus to the first tabbable element inside the dialog.
  • Shift + Tab:
    • Moves focus to the previous tabbable element inside the dialog.
    • If focus is on the first tabbable element inside the dialog, moves focus to the last tabbable element inside the dialog.
  • Escape: Closes the dialog.

Again, browser's have implemented <dialog> to allow Tab access to things like other tabs and the URL bar instead of truly focus trapping the dialog.

adamjohnson avatar Sep 19 '24 13:09 adamjohnson

Closing this PR due to merge conflicts. Continue this discussion in #2078.

adamjohnson avatar Dec 06 '24 19:12 adamjohnson