fix(Notifications): improve screen-reader support #1790
Conversation
|
|
||
| return ( | ||
| <StyledClose ref={ref} hue={hue} aria-label={ariaLabel} {...props}> | ||
| <StyledClose ref={ref} hue={hue} aria-label={ariaLabel} {...props} aria-hidden> |
There was a problem hiding this comment.
This is a controversial quality-of-life enhancement. With the assumption that most notifications are auto-dismissed and because we are against focus stealing, it's unlikely screen-reader users will be able to use the Close button. Therefore, I think it's better to remove it from the a11y tree to prevent its announcement.
Additionally, I could first check that the autoDismiss is true before applying aria-hidden.
Thoughts? 🤔
There was a problem hiding this comment.
This doesn't feel right to me. Garden's approach to accessibility is a pragmatic one. If a close exists, there shouldn't be harm in announcing it. I'm also not aware of this being flagged during a11y audit – so best not to touch it.
There was a problem hiding this comment.
My only concern is your wording: "most notifications" - implying there are some notifications that may still receive focus, right?
EDIT: oop JZ commented before me.
There was a problem hiding this comment.
Sounds good. I'll revert.
There was a problem hiding this comment.
Narrator is not part of Garden's testing matrix. We follow Zendesk's recommended screen readers and browsers – which includes Safari + VO. What is the affect on that combo?
| </StyledIcon> | ||
| )} | ||
| return ( | ||
| <StyledNotification ref={ref} type={props.type} isFloating {...props} role="alert"> |
There was a problem hiding this comment.
Please retain the existing role override so that consumers can continue to cater to their specific needs.
There was a problem hiding this comment.
From my testing, any role other than alert will not trigger an announcement on JAWS or NVDA. Leaving it up to the user could create an opportunity for an A11Y bug. Can Notification be used outside of a "toast" pattern?
There was a problem hiding this comment.
Can
Notificationbe used outside of a "toast" pattern?
By definition, yes. Conversely, ToastProvider can receive other types of content. Let's stick with Garden's approach of a) doing the right thing, b) giving the user control over the corner cases.
|
|
||
| return ( | ||
| <StyledClose ref={ref} hue={hue} aria-label={ariaLabel} {...props}> | ||
| <StyledClose ref={ref} hue={hue} aria-label={ariaLabel} {...props} aria-hidden> |
There was a problem hiding this comment.
This doesn't feel right to me. Garden's approach to accessibility is a pragmatic one. If a close exists, there shouldn't be harm in announcing it. I'm also not aware of this being flagged during a11y audit – so best not to touch it.
This reverts commit 7d4c031.
Thanks for the helpful link. I'll test that combo. 👍 |
| </StyledIcon> | ||
| )} | ||
| return ( | ||
| <StyledNotification ref={ref} type={props.type} isFloating role="alert" {...props}> |
There was a problem hiding this comment.
| <StyledNotification ref={ref} type={props.type} isFloating role="alert" {...props}> | |
| <StyledNotification ref={ref} type={type} isFloating role="alert" {...props}> |
Not exactly related to this PR, but should type be structured out in the parameters above so that it doesn't (re)spread?
Description
Nofitication's role toalertto improve compatibility with JAWS, Narrator, and NVDA.aria-hiddento Notification'sClosebutton to improve UX.Detail
I've tested the following combinations using the latest version of the browser and screen-reader:
Two methods were compared:
statusoralertis applied toNotification. When the Notification incrementally mounts, the SR will read out loud its content.aria-liveregion": This region has anaria-liveattribute set to eitherpoliteorassertiveand is mounted by theToastProvideron load. Changes to its content will be announced incrementally by setting the attributearia-atomictofalse.Conclusion
There is no perfect solution but
role="alert"onNotificationseems best.TL;DR
The "Persistent
aria-liveregion" offers wider SR compatibility with witharia-live=politebut completely fails with Chrome + Narrator. Additionally, the assertive behavior of JAWS and NVDA remainspoliteregardless ofaria-livesetting.Setting
role="alert"onNotificationprovides more consistent results. Most SR's exceptNarratorwould announce the notification with an alert prefix or a chime (VoiceOver) before reading its content. That helps users better distinguish such events but at the cost of the occasional announcement interruption.Checklist
👌 design updates will be Garden Designer approved (add the designer as a reviewer)npm start)⬅️ renders as expected with reversed (RTL) direction🤘 renders as expected with Bedrock CSS (?bedrock)