docusaurus
docusaurus copied to clipboard
a11y: light & dark mode checkbox doesn't announce state switches
Have you read the Contributing Guidelines on issues?
- [X] I have read the Contributing Guidelines on issues.
Prerequisites
- [X] I'm using the latest version of Docusaurus.
- [X] I have tried the
npm run clear
oryarn clear
command. - [X] I have tried
rm -rf node_modules yarn.lock package-lock.json
and re-installing packages. - [X] I have tried creating a repro with https://new.docusaurus.io.
- [X] I have read the console error message carefully (if applicable).
Description
Following up from #6665: The color mode toggle now does correctly indicate what its state is with its label. But, when you activate it while using a screenreader, nothing narrates that it switched.
Discussed on stream with @BenDMyers on Twitch: https://www.twitch.tv/videos/1498962341.
Reproducible demo
https://typescript-eslint.io
Steps to reproduce
- Activate a screenreader such as NVDA or VoiceOver
- Tab over to the light/dark mode toggle
- Press Enter to trigger it
Expected behavior
There should be a narration, such as what's provided by aria-pressed
.
See more here: https://sarahmhigley.com/writing/playing-with-state
Actual behavior
No narration.
Your environment
- Public source code: https://github.com/typescript-eslint/typescript-eslint
- Public site URL: https://typescript-eslint.io/
- Docusaurus version used: 2.0.0-beta.21
- Environment name and version (e.g. Chrome 89, Node.js 16.4): Chrome 102
- Operating system and version (e.g. Ubuntu 20.04.2 LTS): Windows 10
Self-service
- [X] I'd be willing to fix this bug myself.
I'm not sure how we are to fix this. aria-pressed
definitely doesn't seem like a fit. Would aria-live="assertive"
help?
aria-pressed
could be a fit if the toggle button's name were something like "Enable dark mode" — that way, as you toggle dark mode, you'd get announcements along the lines of "Enable dark mode, pressed" or "Enable dark mode, not pressed." Since aria-pressed
is an ARIA state attribute, it should be reliably announced when changed, unlike the accessible name as the linked Sarah Higley article points out.
The button itself probably shouldn't receive aria-live
, since you'll have a mismatch of roles. If Docusaurus already has a mechanism for aria-live
announcements, that could work.
Yes, but our button is light/dark instead of dark/not dark, so pretty sure that's not aria-pressed
. I'm just not sure how to force the screen reader to announce the change in button title.
@Josh-Cena I would like to work on this issue if no one else is already working on it. Thanks!
@noobyogi0010 Since it hasn't had any activities in the last month, you can assume no-one is working on it and just go ahead.
@Josh-Cena Okay, I'll get started with it. Thanks!
@Josh-Cena
Will something like this be a good fix for this issue?
(The red underlined text)
I used aria-pressed
and set it as false for dark mode and true for light mode.
I think there's an issue with our accessibility fix.
When we navigate from one page to another, triggering a global layout unmount/remount (ie navigating from homepage to docs or to blog), the dark mode switch will announce itself on navigation.
We should probably only make it announce after having been pressed once no?
@slorber after a bit of investigation I found out that aria-live
regions are reannounced every time the React component is remounted (inserted into the DOM) which makes sense.
The question is why is the ThemeToggle button reinserted on the navigation.
From my quick look into the code, it seems like the whole page is wrapped within react-router. The problem with this approach is that it tears down everything and builds an entirely new tree from the ground up - remounts. This includes layout, nav, and footer despite the content being the only one that needs a real remount. The rest of the components would be fine with just an update. Wrapping just the content part within the react-router would not only solve the issue that you mentioned but could also prevent a lot of bugs in the future + is a nice perf improvement.
Note that the above proposal is a bit naive and doesn't account for any hidden problems that may have led to the current architecture which I would like to learn more about.
@mturoci the unmount/remount problem is a known problem (https://github.com/facebook/docusaurus/issues/2891) and not so simple to solve in the short term. I'm more looking for a solution considering we won't be able to fix this deeper problem immediately.
The layout should not automatically be applied to all Docusauurus routes: some sites need to have pages with a different layout. And there can also be multiple layout levels (see docs or blog). We should have support for nested routes (like Remix, and soon Next.js have).
I think only adding aria-live=true
after the first toggle happens should be good enough temporarily. Good first issue for anyone willing to contribute. Open a PR directly.
Another problem with the accessibility fix is that the screen reader will continue to announce that the page is in dark mode or light mode for five or six times if the user does not move away from the light/dark mode button. I am able to reduce the announcements to two times if I remove aria-live="polite" from the web inspector. I expected the states to be announced one time on change only.
Another problem with the accessibility fix is that the screen reader will continue to announce that the page is in dark mode or light mode for five or six times if the user does not move away from the region. I am able to reduce the announcements to two times if I remove aria-live="polite" from the web inspector. I expected the states to be announced one time on change only.
I'm not sure to understand. By chance are you able to record a video or something that could make it clearer?
What do you mean by "move away from the region"
By moving away, I mean focusing the screen reader on another part of the screen. By region, I meant the light/dark mode button.
Here's a video. I hope this makes it clearer:
https://user-images.githubusercontent.com/11358567/209221266-121c645a-6930-4988-a895-5033e438e52f.mp4
Is this issue still open?