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

Work out whether to include aria-labelledby on error summary and notification banner #1954

Closed
3 tasks
hannalaakso opened this issue Sep 15, 2020 · 1 comment
Closed
3 tasks
Assignees
Labels
accessibility documentation User requests new documentation or improvements to existing documentation error summary 🕔 hours A well understood issue which we expect to take less than a day to resolve. notification banner

Comments

@hannalaakso
Copy link
Member

hannalaakso commented Sep 15, 2020

What

@selfthinker has flagged that aria-labelledby on the error summary is redundant as the error summary is not a landmark. This article by the Paciello Group seems to also support that.

EDIT: Removing the aria-labelledby completely changes the behaviour in VoiceOver (macOS) so we need to do further testing to understand what's going on.

Why

If we're using the attribute incorrectly and unnecessarily we should remove it. We can then also remove the corresponding id from the <h2> in the error summary.

Further detail

It seems that aria-labelledby was originally added together with role="group" as discussed here:

The aria-labelledby attribute is there to ensure the heading in the summary is associated with the summary box when using role="group". So when focus is set to the error summary, the heading is announced.

However we have since swapped group for alert which seems to make aria-labelledby redundant.

We should test that the error summary continues to work correctly with the assistive technologies we support if aria-labelledby is removed.

Who needs to know about this

Developers, Mark as this will be a breaking change at least for fixtures

Done when

  • We've tested whether removing the attribute has any adverse effects
  • If appropriate, we've removed the attribute from the error summary and the id from the header
  • If appropriate, we've documented this in the release note for breaking changes for fixtures (I'm assuming that we wouldn't document this as a breaking change as such but it's something we could discuss doing for 4.0)
@kellylee-gds kellylee-gds added breaking change 🕔 hours A well understood issue which we expect to take less than a day to resolve. and removed awaiting triage Needs triaging by team breaking change labels Sep 21, 2020
@m-green m-green added the documentation User requests new documentation or improvements to existing documentation label Sep 24, 2020
@36degrees 36degrees changed the title Remove aria-labelledby from error summary Work out whether to include aria-labelledby on error summary and notification banner Nov 17, 2020
@36degrees 36degrees self-assigned this Nov 17, 2020
@36degrees
Copy link
Contributor

36degrees commented Dec 4, 2020

After testing in various different assistive technologies, we found that removing aria-labelledby did affect the way that the notification banner / error summary were presented, especially with VoiceOver on macOS and TalkBack on Android:

With aria-labelledby Without aria-labelledby
Voiceover / Safari (macOS Catalina) SOMETIMES:

main, There is a problem, alert. You are currently in an alert.

OR:

There is a problem There is a problem, Enter your passport number Enter your expiry date. You are currently in an alert.
Sometimes:
There is a problem enter your passport number enter your expiry date. You are currently on a heading level 2

OR:
"There is a problem. You are currently on a heading level 2"

OR:
Nothing is read out. (However, the speech viewer sometimes showed one of the above)
Talkback / Chrome 86 (Android 10) "There is a problem, alert"

Sometimes misses off the start by announcing the page load percentage
Sometimes: "Enter your expiry date"

Sometimes: "Alert"

Always seems to miss off the start by announcing the page load percentage

Full results from testing (Google Sheet)

In the short term, we decided not to remove aria-labelledbyfrom the error summary, and instead to add aria-labelledby to notification banners to make them consistent with the error summary.

However, we'll explore whether there are ways to improve the screen reader experience for notification banners and the error summary in the future, especially as testing has highlighted how inconsistently they seem to be announced.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accessibility documentation User requests new documentation or improvements to existing documentation error summary 🕔 hours A well understood issue which we expect to take less than a day to resolve. notification banner
Projects
Development

No branches or pull requests

4 participants