search

alphagov / govuk_elements

❗️GOV.UK Elements is deprecated, and will only receive major bug fixes and security patches.
https://govuk-elements.herokuapp.com/
MIT License
227 stars 90 forks source link

Change error summary ARIA role to "alert" #511

Closed selfthinker closed 7 years ago

selfthinker commented 7 years ago

What problem does the pull request solve?

Currently neither VoiceOver on iOS or macOS nor TalkBack on Android announce that there is an error when submitting a form with errors. Although we focus on the error summary box, it doesn't have the desired effect. In the iOS case it's because of one browser bug and in the macOS case because of another browser bug. At least the iOS and Android issue can be fixed by adding the ARIA role="alert" to the error summary box.

I am not sure what the effect will be of removing the group role. It didn't seem to have an effect when I tested this but I would like to have someone else's opinion on this.

How has this been tested?

Changing this doesn't change the behaviour (postively or negatively) in JAWS, NVDA or VO on macOS, but it does mean that ZoomText is not announcing anything. As VO on iOS is used much more often by blind people than ZoomText (ZoomText is used more by visually impaired people who would be able to see the big red alert box), this seems to be a sensible sacrifice.

What type of change is it?

Has the documentation been updated?

36degrees commented 7 years ago

Makes sense to me. I'm not an expert on roles by any means, but from a quick Google I don't think we're losing anything important by removing the group role.

hannalaakso commented 7 years ago

I'm guessing the role role "group" was meant to group related elements (h1, p, ul) together, just looking at form_error_radio_show_errors.html. But W3C say role "group" should be used for form controls so I think the group role here is not doing anything, unless there are form controls inside any of the divs which I couldn't see.

selfthinker commented 7 years ago

I wonder if the aria-labelledby has any symbiotic relationship with the role=group and if it can be accessed the same way without it?

@36degrees, the copy change makes sense, I will change it when I am back.

gemmaleigh commented 7 years ago

I had a look for the reason why role="group" was added, I'm afraid can't find the history of it. I can only assume it was added so that assistive technologies know that it's a container for a group of related content. https://www.w3.org/TR/wai-aria/roles#group

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.

I'd be keen to know if the JavaScript which is setting focus to the error summary box can also be removed?

The documentation for role="alert" implies there isn't a need to also set focus to the summary box: https://www.w3.org/TR/wai-aria/roles#alert

Neither authors nor user agents are required to set or manage focus to them in order for them to be processed.

hannalaakso commented 7 years ago

I've finally got a working machine so can do some testing on this. I'll pick this up tomorrow.

selfthinker commented 7 years ago

@gemmaleigh, I have tested lots of variations, but I have not tested what happens if we don't set the focus. Good question. I would guess it's still needed due to inconsistent screen reader support but I can make sure and test it.

@hannalaakso, I can introduce you to our AT laptops and show you how to test in our various screen readers etc.

hannalaakso commented 7 years ago

This claims that NVDA + Edge doesn't support role="alert". I guess we need to do some testing.

I did test removing the JS that sets focus and using role="alert" and VoiceOver on MacOS 10.11.6 announces it correctly. I was testing in form_error_multiple.html.

Thanks @selfthinker have slacked you separately re:intro 👍

selfthinker commented 7 years ago

I don't think NVDA officially supports Edge yet. (Struggling to find evidence of that, though.) So, I think we can ignore that.

selfthinker commented 7 years ago

I can currently not fully test this without the focus as our laptops with JAWS etc on them have connection issues. I have just tested this in NVDA and Firefox, though, and that does definitely not work properly without the focus. It jumps straight to the first input field (after reading the page title) and skips the error summary box completely. Another reason why the focus is still useful is mobile devices. It helps move the box into the viewport!

selfthinker commented 7 years ago

I have asked Leonie for feedback on this and she responded:

We used the group role to set an explicit role on the div element (without which the aria-labelledby attribute won't work). More on this here: https://www.paciellogroup.com/blog/2017/07/short-note-on-aria-label-aria-labelledby-and-aria-describedby/

If the alert role is used instead it will achieve the same thing, so no problem with switching it over.

I haven't made the copy change @36degrees requested yet because I'm waiting for #509 to be merged as I know there will be conflicts.

selfthinker commented 7 years ago

I have just made the copy changes. Although #509 isn't merged yet, this one could be merged before it. I guess it doesn't really matter which one is the one that is causing the other to conflict.