improvement(a11y): add aria-live attribute to month name #2123
Conversation
…der users are read the current
|
Is there an existing test or story that covers this? It’d be great to avoid regressions. |
|
@ljharb there isn't but I'd be happy to add an assertion for the attribute in the unit test! |
|
In the scenario where there are two months showing at once, what would the desired behavior be? I'm not 100% positive what the behavior is for different screen readers when 2 aria-live regions announce at the same time (if it queues or if one reads over the other). Should this be something that's configurable? Or should the be something where you could make more of a status update announcement (eg. "January loaded" or "January and February showing") that exists in a separate live region that can be cleared out after some time period? |
|
@backwardok that's a great question! I just tested it with the macOS VoiceOver screen reader and with |
|
@nathansh I'm not personally aware of any visually hidden text utilities in this repo (but I also don't regularly contribute to this repo so I don't know if there is one or not). Localization/translation I'd imagine would be similar to how other phrases are handled, but I think maybe for now this is probably a good start, and if there's additional feedback for improvement, things can iterate from there! |
src/components/CalendarMonth.jsx
Outdated
| @@ -203,7 +203,7 @@ class CalendarMonth extends React.PureComponent { | |||
| isVisible, | |||
| }) | |||
| ) : ( | |||
| <strong> | |||
| <strong aria-live="polite"> | |||
There was a problem hiding this comment.
Recommend also adding aria-atomic="true" so that the whole phrase gets read out
There was a problem hiding this comment.
@backwardok ooo thanks for the recommendation! I just added that as well :)
|
@backwardok are there any other pieces needed from my end? 🙂 |
|
@nathansh general code change is fine to me! Though it looks like CI is failing? Maybe need to rebase or something |
|
@backwardok very strange! I just rebased with the latest master and the branch is up to date 🤔 I just took a peak at the issues travis and coveralls are reporting, and they're all with existing code. Is it possible to maybe give CI a kick to try again? |
|
@nathansh Kicked off all the failing jobs again for you - hopefully they pass this time 🤞 |
|
hmm.. looks like they're still failing. @ljharb do you know what's going on with the CI checks? |
|
Hi @backwardok! Super sorry for being a nudge, just wanted to check in and see if there's anything I can do to help this one? |
|
@ljharb any insights here on the failing CI checks? |
|
@backwardok no, i'm not really sure - it's only failing in react 16. We might need an enzyme update, and some test tweaks, but it's tough for me to figure it out since this repo has a PR review requirement, and I'm not an admin. |
|
@nathansh unfortunately I'm not super familiar with this code base specifically (mostly just chime in on a11y questions) and it seems like even in the main branch, CI is failing. If you're interested in digging into it at all, that would be highly appreciated! Otherwise I'm not sure exactly when folks will be able to get to it 😕 |
|
@nathansh i will be able to get to this within the next month; hopefully the PR can stay open until then. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2123 +/- ##
==========================================
+ Coverage 79.56% 80.02% +0.46%
==========================================
Files 57 57
Lines 2285 2303 +18
Branches 635 636 +1
==========================================
+ Hits 1818 1843 +25
+ Misses 274 271 -3
+ Partials 193 189 -4 ☔ View full report in Codecov by Sentry. |
|
@ljharb I wouldn't mind closing this PR if it's no longer helpful/relevant/going to merge! |
|
It’s still helpful and will be merged eventually :-) thanks for your patience |
|
Closing as it's been open since 2021 and I can't imagine it has value anymore |
|
I think it still has value, delay notwithstanding :-) |
|
@ljharb do you know if there's any intent to address the underlying issue with what's failing? are all PRs failing at this point? |
|
@backwardok yes, i believe all PRs are failing, and I definitely have on my (sadly long) list to fix that on this repo, and rebase and hopefully land outstanding PRs. |
This PR addresses an issue that surfaced in a user testing session with Fable. Our test involved date selection tasks with a screenreader user.
In our testing session, I asked the screenreader user to select 2 months from today, but since the screenreader didn't read out the name of the months when changing, he had to actually count the number of months he'd gone forward. This was especially frustrating for him when he needed to go ahead 8 months.
In this solution, which was recommended by our tester, the name of the month is read out, allowing for a much easier date selection experience for screenreader users.