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 missing dialog features #4

Merged
merged 16 commits into from
Mar 21, 2023
Merged

Conversation

viniciusmuller
Copy link
Collaborator

@viniciusmuller viniciusmuller commented Mar 20, 2023

Closes #2

@viniciusmuller
Copy link
Collaborator Author

Based on Radix's Dialog, it uses the referenced ARIA guide to implement accessibility.

I think we need to take a better look at this part:

Because marking a dialog modal by setting aria-modal to true can prevent users of some assistive technologies from perceiving content outside the dialog, users of those technologies will experience severe negative ramifications if a dialog is marked modal but does not behave as a modal for other users. So, mark a dialog modal only when both:

  • Application code prevents all users from interacting in any way with content outside of it.
  • Visual styling obscures the content outside of it.

We should find a way to detect if we're inside a backdrop and static dialog, and then set aria-model=true.

@thiagomajesk
Copy link
Owner

thiagomajesk commented Mar 20, 2023

This is the checklist we have so far (with respective references):

Update: Added some commits for you to test, specially this little refactor in the portal logic: b1815eb.

BTW, choose to left aria-modal="true" by default, as I think dialogs are mostly gonna be used for this purpose. I don't think we should think about this that much, since most implementations I've seen (like Headless UI for instance) does this. Also, after looking at Reakit docs, it became clear to me that there's no consensus on how non-modal dialogs should behave: https://reakit.io/docs/dialog/#non-modal-dialogs. If users don't want to use dialogs as modals they are free to override the defaults 👍.

@viniciusmuller viniciusmuller force-pushed the add-missing-dialog-features branch 5 times, most recently from af6bfef to 8633342 Compare March 21, 2023 15:22
@thiagomajesk thiagomajesk merged commit 33278a4 into master Mar 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Missing dialog features
2 participants