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

Consistent handling of aria-describedby #55362

Open
jeryj opened this issue Oct 13, 2023 · 5 comments
Open

Consistent handling of aria-describedby #55362

jeryj opened this issue Oct 13, 2023 · 5 comments
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Type] Enhancement A suggestion for improvement.

Comments

@jeryj
Copy link
Contributor

jeryj commented Oct 13, 2023

What problem does this address?

aria-describedby's usage is inconsistent in the codebase, and different areas use different methods. A lot use the<VisuallyHidden> component with an instanceID to implement it, like:

import { useInstanceId } from '@wordpress/compose';

const descriptionId = useInstanceId(
	ComponentName,
	'component-name__description'
);

const somethingExtra = 'variable';

<Button
	aria-describedby={ descriptionId }
	{ ...props }
/>
<VisuallyHidden id={ descriptionId }>
	I'm a description with a potential {somethingExtra}.
</VisuallyHidden>

The issue with <VisuallyHidden> is that the description is announced twice. In the above example, a screen reader would announce the description once on the button, and once after it within the VisuallyHidden content. This is better than nothing, but only announcing it once is far preferable.

aria-describedby content can be hidden via display: none or visibility: hidden as well, which allows the description to be read when the virtual cursor is focused on the item with the aria-description, but won't have access to read the element that contains the description. That's a technical and confusing way of saying: it'll only read it once, and at the right time.

To make things harder, unfortunately, we can't rely on using display: none or visibility: hidden alone. While that should work, it doesn't for some reason. We think it may be due to rerenders, but we're not sure yet.

What is your proposed solution?

My initial, unexplored idea, is to handle it similar to modals. Have one div wrapper at the end of the content that contains all of the aria-description elements. Something that renders like

<div class="aria-descriptions">
  <div id="component-name__description">
	<span>I'm a description with a potential {somethingExtra}.</span>
  </div>
   <div id="another-component-name__description">
	<span>Another description with a potential {somethingExtra}.</span>
  </div>
</div>

A good question here is, "why <span>s? Well, if we rerender the div itself, it will remount the div, potentially making it so the description doesn't get announced. But if we can only rerender the content inside the div, maybe it'll work. Whatever the solution is, ideally we can make it easier to add aria-descriptions across Gutenberg, and also make them work in a consistent way that allows us to easily patch any necessary updates.

@jeryj jeryj added the [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). label Oct 13, 2023
@jeryj
Copy link
Contributor Author

jeryj commented Oct 13, 2023

First, I think it'd be useful to make an isolated test case that sees what methods work well. Is rerendering even the issue here?

@alexstine
Copy link
Contributor

I agree. Can we start off with something really simple?

  1. Add aria-describedby to the list view toggle.
  2. Add the description with display: none; and the matching ID attribute.
  3. If it works, this confirms the re-rendering bug. This button should not re-render.

I tested it with the below code and it works fine.

<html>
	<head>
		<title>Test</title>
	</head>
	<body>
		<h1>Test</h1>
		<br />
		<span id="some-id" style="display: none;">Description goes here</span>
		<button type="button" aria-expanded="false" aria-haspopup="menu" aria-describedby="some-id">Options</button>
		<br />
	</body>
</html>

Even with the ID appearing directly above the aria-describedby in the DOM, it still works.

@alexstine
Copy link
Contributor

I also tried this version. The bug has to happen during re-render because the click function successfully changes the text and it's announced. I think a good first step might be to avoid re-rendering the entire element and only the text inside. Could probably do that with useEffect. I guess if the whole element re-renders, it breaks screen readers somehow.

<html>
	<head>
		<title>Test</title>
	</head>
	<body>
		<h1>Test</h1>
		<br />
		<span id="some-id" style="display: none;">Description goes here</span>
		<button type="button" aria-expanded="false" aria-haspopup="menu" aria-describedby="some-id" id="some-button">Options</button>
		<br />
		<script type="text/javascript">
			const button = document.getElementById( 'some-button' );
			button.addEventListener( 'click', function() {
				const description = document.getElementById( 'some-id' );
				description.textContent = 'New Description';
			} );
		</script>
	</body>
</html>

@jordesign jordesign added the [Type] Enhancement A suggestion for improvement. label Oct 15, 2023
@jeryj
Copy link
Contributor Author

jeryj commented Oct 16, 2023

I created some test cases as well. The bug, to me, appears when:

  • On a button
  • Clicking the button causes a rerender
  • Focus remains on the button
  • The button description isn't announced

Then, I can tab off of the button, tab back to the updated button, and the description is there. Is this the same behavior you experience on my test case, @alexstine?

@jeryj
Copy link
Contributor Author

jeryj commented Oct 20, 2023

Updated test case:

Significant finding: Safari + VoiceOver will not announce updated descriptions unless the ID of the description and aria-describedBy change. I'm curious how NVDA handles this. Does updating the ID of the description and aria-describedBy allow it to access the updated description? cc @alexstine

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

No branches or pull requests

3 participants