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

fix: turn section into live region #306 #436

Merged
merged 3 commits into from
Nov 2, 2024

Conversation

tricinel
Copy link
Contributor

@tricinel tricinel commented Jun 7, 2024

Issue: Older screen readers don't announce individual toasts

li elements are not allowed to have the role="status" tag as per MDN.

Fixes #306

What has been done:

  • Removed aria-live, aria-atomic and role tags from the <li> element (the Toast)
  • Added aria-live, aria-relevant and aria-atomic to the containing <section> element
  • Removed the important key from the ToastT type

Screenshots/Videos:

Screen.Recording.2024-03-20.at.09.24.41.mov

Visually, not changed. Tests all pass. Build passes.

Notes

This is be a breaking change because we remove the important key from the ToastT type, meaning that consumers will get a type error if they've been using the key so far. Happy to discuss another solution to this, if any.

Copy link

vercel bot commented Jun 7, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
sonner ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 2, 2024 4:58pm

@tricinel
Copy link
Contributor Author

tricinel commented Jun 7, 2024

As a proposal, we could have a way to deprecate the important key in the ToastT interface. And maybe "announce" it for the next few minor versions. Meaning, we keep it but it does nothing so we can warn users that it will disappear and they have time to prepare.

@tricinel
Copy link
Contributor Author

Hm, I might need to update from main before it can be merged. I'll take care of that.

@andrewbarnesweb
Copy link
Contributor

Is there any update on this PR? I could do with it as we have automated accessibility tests which it fails for... And for users 🙂

@tricinel
Copy link
Contributor Author

I kept merging main into this, but I don't think it'll ever actually get merged, so I gave up :(

@tonyfarney
Copy link

tonyfarney commented Oct 24, 2024

@tricinel Why do you think it will not be merged?

@tricinel
Copy link
Contributor Author

There are plenty of PRs unmerged 🤷

@emilkowalski emilkowalski merged commit 971bfe1 into emilkowalski:main Nov 2, 2024
2 of 3 checks passed
@emilkowalski
Copy link
Owner

Thanks, merged. My guess is that not a lot of people use important, so it's ok to merge.

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.

BUG: Invalid WAI-ARIA setup preventing screen readers from announcing new toasts
5 participants