docusaurus icon indicating copy to clipboard operation
docusaurus copied to clipboard

a11y: light & dark mode checkbox doesn't announce state switches

Open JoshuaKGoldberg opened this issue 2 years ago • 16 comments

Have you read the Contributing Guidelines on issues?

Prerequisites

  • [X] I'm using the latest version of Docusaurus.
  • [X] I have tried the npm run clear or yarn 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

  1. Activate a screenreader such as NVDA or VoiceOver
  2. Tab over to the light/dark mode toggle
  3. 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.

JoshuaKGoldberg avatar Jun 23 '22 14:06 JoshuaKGoldberg

I'm not sure how we are to fix this. aria-pressed definitely doesn't seem like a fit. Would aria-live="assertive" help?

Josh-Cena avatar Jun 23 '22 14:06 Josh-Cena

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.

BenDMyers avatar Jun 23 '22 14:06 BenDMyers

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 avatar Jun 23 '22 14:06 Josh-Cena

@Josh-Cena I would like to work on this issue if no one else is already working on it. Thanks!

noobyogi0010 avatar Jul 30 '22 07:07 noobyogi0010

@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 avatar Jul 30 '22 07:07 Josh-Cena

@Josh-Cena Okay, I'll get started with it. Thanks!

noobyogi0010 avatar Jul 30 '22 08:07 noobyogi0010

@Josh-Cena Will something like this be a good fix for this issue? (The red underlined text) a11yDocusaurusRef

I used aria-pressed and set it as false for dark mode and true for light mode.

noobyogi0010 avatar Aug 11 '22 06:08 noobyogi0010

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 avatar Oct 12 '22 17:10 slorber

@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 avatar Oct 16 '22 17:10 mturoci

@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.

slorber avatar Oct 19 '22 13:10 slorber

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.

dawei-wang avatar Dec 20 '22 03:12 dawei-wang

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"

slorber avatar Dec 22 '22 18:12 slorber

By moving away, I mean focusing the screen reader on another part of the screen. By region, I meant the light/dark mode button.

dawei-wang avatar Dec 22 '22 19:12 dawei-wang

Here's a video. I hope this makes it clearer:

https://user-images.githubusercontent.com/11358567/209221266-121c645a-6930-4988-a895-5033e438e52f.mp4

dawei-wang avatar Dec 22 '22 20:12 dawei-wang

Is this issue still open?

Vishrut19 avatar May 18 '23 04:05 Vishrut19