Improve accessibility of the Warning component in the block editor

[sarthaknagoshe2002]

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

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

[github-actions]

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]

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

Left some comments

[sarthaknagoshe2002]

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]

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

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]

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]

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.

[sarthaknagoshe2002]

@afercia I believe this is now ready to be merged.

[afercia]

Looks good to me, thanks @sarthaknagoshe2002 !

[afercia]

@WordPress/gutenberg-core as mentioned earlier, with this change all Warnings will receive focus when they render. There may be edge cases but I think it's OK as default behavior and I'm going to merge this PR. Just wanted to be sure everyone is aware of this change in case there are edge cases to address in follow-ups.

[t-hamano]

I think having this component always receive focus when rendered can cause other accessibility issues.

For example, create a pattern that already has broken blocks. When that pattern is rendered via a site editor or similar, the Warning component in the pattern will be focused, causing the current navigation focus to be lost. And once focus is moved into an iframe, it cannot be moved out of it:

ec4af8494f02912743de2b5d683b4af2.mp4

Another concern is that when I open a post with multiple broken blocks, the last Warning component gets focus immediately. I would like to confirm if this is an issue or not.

[afercia]

@t-hamano thanks for your feedback. To me, moving focus is the most sensible default, especially when the Warning contains action buttons. Keyboard users will benefit from being able to immediately access the actions.

causing the current navigation focus to be lost

Yes I see your point. Should Warnings be rendered in the Patterns preview card in the first place? I didn't know of this behavior but to me it looks pretty weird. That is a preview, it should not contain interactive elements and they're so small anyways that I'd argue they're not usable at all. Screenshot:

when I open a post with multiple broken blocks, the last Warning component gets focus immediately.

Ideally, there should be only one Warning at a time. If the editor shows multiple warnings at the same time, they should also be all announced to screen reader users to provide an equivalent information. However, implementing a way to announce all warnings at the same time could be potentially very noisy and confusing. I'm really not sure how to address this potential edge case.

[afercia]

I'll create a separate issue for the Warnings in the preview cards.

[Mamaduka]

I agree with @t-hamano that the Warning component shouldn't autofocus because of rendering issues. We should revert it.

Why?

• The "focus because of render" is a tricky pattern in React; it's hard to get right, and because of that, it should be avoided.
• Multiple blocks can render a Warning component since they block a specific. The last one to render will steal the focus.
• These are caused by invalid or broken content when a post is rendered, not by user action. So they can be handled later.

TL;DR, let's avoid making similar changes to components that can be rendered multiple times and are widely used by block editors.

[getdave]

I would also be in favour of reverting this. Autofocus on render is very likely to cause a number of knock on problems.

The other changes to make the warnings perceivable and more prominent to assistive tech seem like good improvements however.

[afercia]

I'm not opposed to a revertl. However:

The other changes to make the warnings perceivable and more prominent to assistive tech seem like good improvements however.

• The other improvements don't work without the focus being set on the Warning.
• The Warning may provide (and in most of the cases it does) action buttons. It's pointless and non-accessible rendering action buttons if keyboard users have to tab dozens and dozens of times to navigate to them.
• The Warning is a sort of 'alert' message. To make it usable for everyone it should be semantically an alert and should be announced by assistive tech and be easily navigable to with keyboard. Ideally, as with all alerts, there should be only one Warning rendered at a time.
• For accessibility, it's not uncommon to move focus to alerts and warning messages. It's a pretty common pattern. rather then removing this behavior we should find a way to make it solid and reliable.

Overall, the usage of the Warning is highly problematic and all these points should be addressed. If anyone can think at better ways to address these points, that would be very welcome.

[Mamaduka]

You can say that these warnings are mostly part of the content/canvas. Let's say you opened a documentation page with multiple warnings for experimental/unstable APIs. What are the expectations here? Do each of these warning messages try and move focus on itself, or are they just read as you get closer to actual experimental/unstable sections?

rather then removing this behavior we should find a way to make it solid and reliable.

Ideally, we would have caught this broken behavior before merging this PR, but we are humans and sometimes miss things.

I'll include a full revert in my PR (#68133). Let's reopen the issue and continue working on a solution.

[afercia]

Was thinking at a potential different approach, I will comment on the issue and reopen it.

[Mamaduka]

Thanks, @afercia!

Meanwhile, my revert PR is ready to be merged - #68133.

[sarthaknagoshe2002]

Since the focus was causing issues, we could have simply removed that part, as the warning was being announced by the screen reader in other browsers (except Safari) without the focus. This way, the rest of the functionality remains untouched.