Improve accessibility of the Warning component in the block editor #67433

sarthaknagoshe2002 · 2024-11-29T18:16:09Z

Accessibility Fix: #62737

What?

This PR improves the Warning component in the block editor by enhancing its accessibility.

Why?

Currently, the Warning component only provides a visual cue without notifying screen readers, which limits accessibility. These changes ensure that the component is accessible to all users.

How?

Added ARIA role="alert" and aria-live="assertive" to the Warning component for screen reader announcements.
Added aria-describedby="block-editor-warning-message" to the Warning actions.
Added tabIndex="0", so that the Warning can be accessed using tabs. This helps to navigate inside the warning.

Testing Instructions

Add a More block to a post and copy it using the keyboard shortcut or the Options menu.
Paste the copied block elsewhere in the post.
Observe the Warning message appears with improved design and triggers a screen reader announcement.

github-actions · 2024-11-29T18:16:24Z

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: sarthaknagoshe2002 <[email protected]>
Co-authored-by: afercia <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

afercia · 2024-12-17T08:56:29Z

@sarthaknagoshe2002 thanks for your PR.

Good catch for adding the tabindex="0" which makes possible to navigate into the Warning with the keyboard.

Unfortunately, testing with Safari and VoiceOver the Warning isn't announced. I guess because the editor sets focus on the newly inserted block. See in the screenshot below: focus is on the doubled 'More' block:

I think a potential way to fix this would be setting focus on the Warning when it renders by using useEffect and useRef.

I'll leave a few comments in the review as well.
Worth noting there's a merge conflict because it appears this branch doesn't incorporate recent changes to the Warning component introduced in #67675

afercia · 2024-12-17T08:59:35Z

packages/block-editor/src/components/warning/index.js

+			<div
+				className={ clsx( className, 'block-editor-warning' ) }
+				role="alert"
+				aria-live="assertive"


role alert inherently works as an assertive live region so that aria-live="assertive" isn't necessary. aria-live doesn;t work well for elements that appear in the DOM anyways.

afercia · 2024-12-17T09:00:24Z

packages/block-editor/src/components/warning/index.js

-						<div className="block-editor-warning__actions">
+						<div
+							className="block-editor-warning__actions"
+							aria-describedby="block-editor-warning-message"


Not sure this aria-describedby is necessary. It doesn't point to any ID anyways, please remove it.

afercia

Left some comments

sarthaknagoshe2002 · 2024-12-17T09:08:45Z

I think a potential way to fix this would be setting focus on the Warning when it renders by using useEffect and useRef.

It seems to work fine in Chrome on macOS using the default screen reader. I also tested it on Safari, and it’s a bit buggy—I noticed it works intermittently. I’m currently investigating the issue and exploring possible solutions.

sarthaknagoshe2002 · 2024-12-17T18:28:08Z

@afercia I have now fixed things to make it work in safari.
Also, I have implemented the changes suggested by you.

Chrome	Safari

Let me know this this requires any more changes. If not then I'll update the tests too. 😇

sarthaknagoshe2002 · 2024-12-17T18:30:33Z

Also, the git hook seems to have modified the linting of some tsconfig.json file.
let me know if we should keep those changes too.

afercia · 2024-12-18T08:00:09Z

packages/block-editor/src/components/warning/index.js

+		if ( null !== alertRef.current ) {
+			alertRef.current.focus();
+		}
+	}, [] );


Question: can this be simplified? I'm not sure there's the need to set the initial ref to null and then checking for it. To my understanding the optional chaining operator ?. would make this much more concise. See an example here:

gutenberg/packages/block-library/src/video/tracks-editor.js

Lines 182 to 186 in e4649fe

const dropdownPopoverRef = useRef();

useEffect( () => {

dropdownPopoverRef.current?.focus();

}, [ trackBeingEdited ] );

afercia · 2024-12-18T08:01:02Z

packages/upload-media/tsconfig.json

@@ -8,13 +8,13 @@
 	},
 	"include": [ "src/**/*" ],
 	"references": [
-	  { "path": "../api-fetch" },


changes to this file should be reverted, probably a result of merge conflict or something

The pre-commit Git hook is enforcing linting even after reverting changes. Are there alternative solutions?

I submitted a quick PR #68083 to fix it separately.
Once that is merged, please rebase this PR.

afercia · 2024-12-18T08:02:51Z

packages/block-editor/src/components/warning/index.js

-					<p className="block-editor-warning__message">
+					<p
+						className="block-editor-warning__message"
+						ref={ alertRef }


I think the reg should be set on the container with tabIndex="0", otherwise moving focus will fail?

afercia · 2024-12-18T08:27:10Z

let me know if we should keep those changes too.

@sarthaknagoshe2002 yes I think those changes should be reverted.
Thanks for working on this, I left some new comments.

As a last step, when everything is done, I guess the snapshot of the failing test should be updated.

npm run test:unit -- --updateSnapshot --testPathPattern packages/block-editor/src/components/warning/test/index.js

afercia · 2024-12-18T08:42:01Z

It's worth noting that all Warnings will receive focus when they render, for example the 'invalid content' Warning and other ones. There may be edge cases but I think it's OK as default behavior.

Enhance accessibility of the Warning component

9a7deff

sarthaknagoshe2002 requested a review from ellatrix as a code owner November 29, 2024 18:16

afercia reviewed Dec 17, 2024

View reviewed changes

afercia requested changes Dec 17, 2024

View reviewed changes

Fix: resolve merge conflicts & ensure working in safari

68559e8

sarthaknagoshe2002 requested a review from afercia December 17, 2024 20:54

afercia reviewed Dec 18, 2024

View reviewed changes

afercia mentioned this pull request Dec 18, 2024

Add new private upload-media package #66290

Merged

sarthaknagoshe2002 added 2 commits December 18, 2024 15:01

Merge branch 'WordPress:trunk' into fix/issue-62737

5c53329

Merge branch 'WordPress:trunk' into fix/issue-62737

be3b30a

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve accessibility of the Warning component in the block editor #67433

Improve accessibility of the Warning component in the block editor #67433

sarthaknagoshe2002 commented Nov 29, 2024 •

edited

Loading

github-actions bot commented Nov 29, 2024 •

edited

Loading

afercia commented Dec 17, 2024

afercia Dec 17, 2024

afercia Dec 17, 2024

afercia left a comment

sarthaknagoshe2002 commented Dec 17, 2024

sarthaknagoshe2002 commented Dec 17, 2024 •

edited

Loading

sarthaknagoshe2002 commented Dec 17, 2024

afercia Dec 18, 2024

afercia Dec 18, 2024

sarthaknagoshe2002 Dec 18, 2024

afercia Dec 18, 2024

afercia Dec 18, 2024

afercia commented Dec 18, 2024

afercia commented Dec 18, 2024

	const dropdownPopoverRef = useRef();

	useEffect( () => {
	dropdownPopoverRef.current?.focus();
	}, [ trackBeingEdited ] );

Improve accessibility of the Warning component in the block editor #67433

Are you sure you want to change the base?

Improve accessibility of the Warning component in the block editor #67433

Conversation

sarthaknagoshe2002 commented Nov 29, 2024 • edited Loading

What?

Why?

How?

Testing Instructions

github-actions bot commented Nov 29, 2024 • edited Loading

afercia commented Dec 17, 2024

afercia Dec 17, 2024

Choose a reason for hiding this comment

afercia Dec 17, 2024

Choose a reason for hiding this comment

afercia left a comment

Choose a reason for hiding this comment

sarthaknagoshe2002 commented Dec 17, 2024

sarthaknagoshe2002 commented Dec 17, 2024 • edited Loading

sarthaknagoshe2002 commented Dec 17, 2024

afercia Dec 18, 2024

Choose a reason for hiding this comment

afercia Dec 18, 2024

Choose a reason for hiding this comment

sarthaknagoshe2002 Dec 18, 2024

Choose a reason for hiding this comment

afercia Dec 18, 2024

Choose a reason for hiding this comment

afercia Dec 18, 2024

Choose a reason for hiding this comment

afercia commented Dec 18, 2024

afercia commented Dec 18, 2024

sarthaknagoshe2002 commented Nov 29, 2024 •

edited

Loading

github-actions bot commented Nov 29, 2024 •

edited

Loading

sarthaknagoshe2002 commented Dec 17, 2024 •

edited

Loading