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

Add native dialog element support #13807

Open
4 of 10 tasks
tay1orjones opened this issue May 15, 2023 · 9 comments
Open
4 of 10 tasks

Add native dialog element support #13807

tay1orjones opened this issue May 15, 2023 · 9 comments
Labels
planning: umbrella Umbrella issues, surfaced in Projects views proposal: open This request has gone through triaging. We're determining whether we take this on or not. type: a11y ♿ type: enhancement 💡
Milestone

Comments

@tay1orjones
Copy link
Member

tay1orjones commented May 15, 2023

Context

An initial exploration and delivery of a Dialog component was done a while ago:

This experimental work was never exported or shown in storybook but still lives in the codebase. Most of the spec of that work stream was focused on managing focus and handling interactions such as escape closing the dialog.

The current state

Since then, there has been some progress on the native dialog element spec and much of this focus and interaction behavior is now provided natively by browsers.

Proposal

This issue is to propose considering using the native dialog element in place of the existing implementation that uses the FocusScope primitive.

Depending on what we consider to be "must have" functionality of a Dialog, it might be that the bulk of the requirements might be met by the dialog element. It might be a simple wrapper around that element is all we need.

Pieces solved in #15926

Preview Give feedback

Explore using native Dialog within the system

Preview Give feedback
  1. role: dev 🤖 type: enhancement 💡 type: infrastructure 🤖
@mbgower
Copy link

mbgower commented May 17, 2023

Some considerations for the assessment:

  • granularity for placing focus when dialog appears (on dialog element, on controls inside dialog)
  • effect on scrolling inside dialog, depending on focus placement
  • keyboard retention in modal dialog
  • naming/title for the dialog

In addition, for the non-modal version of the dialog, the following should be assessed, to discover potential issues meeting WCAG 2.2's Focus Not Obscured (Minimum) requirement as well as other existing requirements and conventions on focus handling, prior point of regard, focus indicators, etc. :

  • dismissal mechanisms
  • persistence on loss of focus
  • positioning in DOM

@sstrubberg sstrubberg moved this from Triage to Icebox in Roadmap Jul 10, 2023
@tay1orjones
Copy link
Member Author

tay1orjones commented Jul 10, 2023

As part of #14156 @guidari and I tested the native dialog functionality as a potential fix but couldn't get the focus wrap behavior to work at all. I'm not sure if it's behind a flag for now or perhaps chrome/firefox hasn't implemented it yet fully?

Either way I think we're blocked until we can confirm the new native dialog behavior functions as expected in all supported browsers.

@sstrubberg sstrubberg added the proposal: open This request has gone through triaging. We're determining whether we take this on or not. label Sep 25, 2023
@tay1orjones
Copy link
Member Author

We'll also need to evaluate and document how other top layer elements interact with a dialog. Initial assumptions/questions I have:

  • Portals will not work as expected. Elements rendered via portals will always render behind the dialog.
  • What is the behavior of our Popover (Tooltip, Toggletip, etc) - do these render above or below the native dialog when placed within it?
  • Does a Popover with autoAlign work as intended, is it the same as non-autoAlign popovers?
  • If an element within the dialog uses the native Popover API, which element sits on top?
  • If an element using the native Popover API renders a dialog, which element sits on top?

@lee-chase
Copy link
Member

@tay1orjones it would appear that completely trapping focus is not desired. Tabbing to browser/environment controls is desirable, page content controls are blocked.

whatwg/html#8339 (comment)

@lee-chase
Copy link
Member

Use of the the native dialog component

https://codepen.io/lee-chase/pen/QWPbRLp/9c8c3f23b42a395dc519839e12f5d647

@lee-chase
Copy link
Member

Unfortunately the transition behavior to allow-discrete is not well supported and would need script

https://caniuse.com/?search=transition-behavior

@lee-chase
Copy link
Member

Experimented and created this based on the native dialog.
#15926

@tay1orjones tay1orjones changed the title Consider using native dialog element Add native dialog element support Aug 1, 2024
@tay1orjones
Copy link
Member Author

I've updated this issue with continued direction now that the initial exploration has been merged in. Logical next step would be to refactor modal behind a feature flag to use Dialog

Unfortunately the transition behavior to allow-discrete is not well supported and would need script

In #15926 we landed on not trying to polyfill this. Since it doesn't break anything other than the animation, we can consider it a case of progressive enhancement. It works in chrome/edge now and should work in safari this fall since it's already in the TP. Firefox users just won't see the animation until it's implemented there.

@tay1orjones tay1orjones added planning: umbrella Umbrella issues, surfaced in Projects views and removed status: blocked 🙅‍♀️ labels Aug 1, 2024
@sstrubberg sstrubberg moved this from Later 🧊 to Next ➡ in Roadmap Nov 12, 2024
@sstrubberg
Copy link
Member

sstrubberg commented Dec 5, 2024

Testing, VRT
Rollout strategy
Flags
Modal, Composed Modal
Tearsheets, Side Panels(?), NotificationsPanel, Interstitial, TagSet.
Stacking in Tearsheets makes more sense in Dialogs than in Modals.
Check in with @lee-chase

Might need design to weigh in: method by with we animate the modal in and out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
planning: umbrella Umbrella issues, surfaced in Projects views proposal: open This request has gone through triaging. We're determining whether we take this on or not. type: a11y ♿ type: enhancement 💡
Projects
Status: Next ➡
Development

No branches or pull requests

4 participants