-
Notifications
You must be signed in to change notification settings - Fork 334
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
Move error message and hint out of labels and legends #681
Conversation
{% from "label/macro.njk" import govukLabel %} | ||
|
||
{#- If an id 'prefix' is not passed, fall back to using the name attribute | ||
instead. We need this for error messages and hints as well -#} | ||
{% set idPrefix = params.idPrefix if params.idPrefix else params.name %} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is name
always unique?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm, doesn't require it to be.
if you had two sets of checkbox groups on the page (nationality, religion) then data for all of them would be submitted as nationality=<value>
even if the second set was about religion. We can't solve this (the same would occur with idPrefix)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Possibly not, but there's only so much we can do to help the user here. I don't think there's any parameter we could use that's guaranteed to be unique.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we have a test that checks that if you use both hint and error message that you get an aria-describedby
that contains both, and in the right order?
src/hint/hint.njk
Outdated
text: "It’s on your National Insurance card, benefit letter, payslip or P60. | ||
For example, ‘QQ 12 34 56 C’.", | ||
id: "hint-id", | ||
name: "hint-name" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is name
used?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nope. good catch 👍
fixed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we consider writing accessibility acceptance criteria for this?
42bf950
to
572dedc
Compare
Great and thorough work, well done! One thing I already mentioned to Oli... There are generally three cases to test not just two.
I believe you haven't tested the latter. It might make a difference or it might not. But I think it makes sense to check that as well, especially regarding where the |
Good call @selfthinker! In my naivety, I thought any input, regardless its type, will be treated similarly by the ATs. @36degrees and I did a round of tests today on the date input example, and it turns out it's not the case with JAWS - which for the date input example doesn't announce the hint and error message when landing on the input fields, despite the fact that they are being referred in the describedby attribute. We'll think about workarounds for JAWS to be able to pick them up. Update: adding a redundant |
@NickColley tests to make sure aria-describedby contains both the hint and the error message and in the right order might be a good addition. Also, accessibility criteria should be added, but I'm not sure will get into this PR until we define generic criterias. I would thumb up your comment, but I can't since it's a review :) |
There's nothing stopping us from adding additional information right now. https://github.com/alphagov/govuk-frontend/search?utf8=%E2%9C%93&q=accessibilityCriteria&type= Would be good to have this since this is fairly nuanced. |
I've pushed a change that explicitly adds the Adding the group role to the fieldset component would make the output overly verbose for radio buttons or checkboxes - which are otherwise working fine already. We've also made a first stab at some accessibility criteria and added tests to ensure that hint and error message are both associated correctly when they are both present. |
We do this because otherwise JAWS does not announce the description for a fieldset comprised of text inputs, but adding the role to the fieldset always makes the output overly verbose for radio buttons or checkboxes.
Also reorganise tests to group tests relating to hints, error messages, and hints and error messages together.
Also reorganise tests to group tests relating to hints, error messages, and hints and error messages together.
22e1da3
to
c0d8f6f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
top quality work 🥇
We haven't been able to make Voiceover on macOS read the error message and hint when focussing the field. It doesn't read the description for the fieldset. We've filed a bug with Apple as we don't think this is correct. However, the error message is available as part of the error summary component (which acts as an alert, so should always be announced to the user), and both the hint and error message can be accessed by 'going out of' the input (ctrl-option-shift + up-arrow) which focuses the fieldset. This change also gives us a number of benefits. It:
With that in mind, we're going to merge this anyway. |
This PR moves error messages and hints out of legends and labels, instead associating them with the input using
aria-describedby
.https://govuk-frontend-review-pr-681.herokuapp.com/
https://trello.com/c/6JHtnTfC
Co-authored with @36degrees.
Background
For question pages that follow the 'one-thing-per-page' format, the question being asked functions as both the label for the question and the H1 for the page.
This was previously achieved by putting the question in the H1 but duplicating it within a visually hidden label or legend. This was functional, but resulted in the question being read twice by screen readers. Anika did a lot of research into this and made changes to Elements to remove the duplication.
The way this currently works is:
<h1>
inside the legend of a fieldset (<legend><h1>Question</h1></legend>
); or<label>
in an<h1>
(<h1><label>Question</label></h1>
)The reason that the nesting differs is that fieldsets allow 'phrasing content and headings' as permitted content (since w3c/html#724) but labels only allow for 'phrasing content', so a heading inside a label is technically invalid.
There are a few issues that we would like to address to make this approach work correctly in GOV.UK Frontend:
fieldset
, because of the inconsistency in the markup.Having explored a number of options (including questioning whether the HTML spec should allow headings inside labels), we've settled on moving the hint and error message outside of the label / legend and associating them with
aria-describedby
as has been previously suggested - it resolves the first two issues and makes it considerably easier to tackle the third.Unfortunately Voiceover on macOS will not announce the hint or error message when focussing the field. This is not ideal, but every approach (including the current approach of nesting them inside the label) had issues in at least one combination of technologies, so we have opted to go with the approach that seemed the most 'semantically correct', which is to treat the error message and hints as part of the 'description' of the field as per the ARIA spec:
We will file an issue with Apple if we can not find it already recorded as a bug.We have filed a bug report with Apple for this issue.
We considered using different approaches for different scenarios (for example, only using
aria-describedby
if thelegend
orlabel
sits inside a heading, or using different implementations depending on whether we are dealing with alegend
or alabel
). However, we concluded that there were a number of benefits to having a consistent approach, as:Detailed changes
/tools/exports
tohelpers/clearfix
in order to pass the Individual components should compile individual SCSS files without throwing exceptionsTo be done in future PRs:
Appendix: Results of testing with assistive technologies
Current approach (nesting error and hint inside of legend or label)
Input with
<label>
❗️When the
<label>
is nested inside a heading, the hint and error message also become part of the heading.❗️It is difficult to make the relationship between the question and the hint and error message consistent with the
fieldset
, because of the inconsistency in the markup.✅ Voiceover + Safari (Mac OS X El Capitan)
Voiceover announces:
✅ Voiceover + Safari (iOS 11)
Voiceover announces:
✅ Voiceover + Safari (iOS 10)
Voiceover announces:
✅ Voiceover + Safari (iOS 9)
Voiceover announces:
✅ NVDA + Firefox
NVDA announces:
✅ JAWS18 + IE11
JAWS announces:
Fieldset with
<legend>
Very strange issue in JAWS18 where the hint and error message are not announced. Other than that, this works OK – because the heading is inside the legend (unlike label) the hint and error message can sit outside of the page heading. (Update: This appears to have been fixed in JAWS 2018)
✅ Voiceover + Safari (Mac OS X El Capitan)
Voiceover announces:
✅ Voiceover + Safari (iOS 11)
When entering the fieldset (e.g. navigating backwards through the document)", Voiceover announces:
Then, for each input:
✅ Voiceover + Safari (iOS 10)
When entering the fieldset (e.g. navigating backwards through the document)", Voiceover announces:
Then, for each input:
✅ Voiceover + Safari (iOS 9)
For each input, Voiceover announces:
iOS 9 does not seem to announce
<fieldset>
legends for inputs.✅ NVDA + Firefox
NVDA announces:
❗️ JAWS18 + IE11
JAWS announces:
Hint and error message are not announced despite being part of the legend. Absolutely no idea what's going on here. Tested this with the current implementation in Elements and the same thing happens.
Associating error and hint with
aria-describedby
Seems like the 'correct' implementation semantically. Some slight quirks in the way this is announced, e.g. JAWS puts the description between "Edit" and "Type in text" which seems disjointed.
For legends, we should associate the hint and error message with the fieldset, not the individual inputs.
Input with
<label>
✅ Voiceover + Safari (Mac OS X El Capitan)
Voiceover announces:
✅ Voiceover + Safari (iOS 11)
Voiceover announces:
✅ Voiceover + Safari (iOS 10)
Voiceover announces:
✅ Voiceover + Safari (iOS 9)
Voiceover announces:
✅ NVDA + Firefox
NVDA announces:
✅ JAWS18 + IE11
JAWS announces:
(includes JAWS commands)
Fieldset with
<legend>
,aria-describedby
on each<input>
✅ Voiceover + Safari (Mac OS X El Capitan)
Voiceover announces:
This includes changing your last name or spelling your name differently. Choose yes or no. You are currently on a radio button, 1 of 2, inside of a group. To select this option…
❗️ Voiceover + Safari (iOS 11)
When entering the fieldset (e.g. navigating backwards through the document)", Voiceover announces:
Then, for each input:
Hint and error message are always announced when focussing the input, but the legend is only announced when entering the fieldset. This behaviour may be confusing as the hint is unlikely to make sense without the question being announced. It's also overly verbose.
❗️ Voiceover + Safari (iOS 10)
When entering the fieldset (e.g. navigating backwards through the document)", Voiceover announces:
Then, for each input:
Hint and error message are always announced when focussing the input, but the legend is only announced when entering the fieldset. This behaviour may be confusing as the hint is unlikely to make sense without the question being announced. It's also overly verbose.
❗️ Voiceover + Safari (iOS 9)
For each input, Voiceover announces:
iOS 9 does not seem to announce
<fieldset>
legends for inputs.✅ NVDA + Firefox
NVDA announces:
There is a minor bug here (reported) where no is announced as number, but it is a side effect of the ordering.
✅ JAWS18 + IE11
JAWS announces:
Fieldset with
<legend>
,aria-describedby
on the<fieldset>
❗️ Voiceover + Safari (Mac OS X El Capitan)
Voiceover announces:
The error message and hint are not announced.
✅ Voiceover + Safari (iOS 11)
When entering the fieldset (e.g. navigating backwards through the document)", Voiceover announces:
Then, for each input:
✅ Voiceover + Safari (iOS 10)
When entering the fieldset (e.g. navigating backwards through the document)", Voiceover announces:
Then, for each input:
❗️ Voiceover + Safari (iOS 9)
For each input, Voiceover announces:
iOS 9 does not seem to announce
<fieldset>
legends for inputs.✅ NVDA + Firefox
NVDA announces:
There is a minor bug here where no is announced as number, but it is a side effect of the ordering.
✅ JAWS18 + IE11
JAWS announces:
Associating error and hint with
aria-labelledby
According to the ARIA spec, a label should be concise, where a description is intended to provide more verbose information. This suggests that aria-labelledby is not the right implementation for the hint and error message, as they do not label the field and cannot really be considered 'concise'.
Input with
<label>
✅ Voiceover + Safari (Mac OS X El Capitan)
Voiceover announces:
❗️ Voiceover + Safari (iOS 11)
Voiceover announces:
A bug in iOS means that if a label is associated with a field, that label will override any aria defined label.
❗️ Voiceover + Safari (iOS 10)
Voiceover announces:
A bug in iOS means that if a label is associated with a field, that label will override any aria defined label.
❗️ Voiceover + Safari (iOS 9)
Voiceover announces:
A bug in iOS means that if a label is associated with a field, that label will override any aria defined label.
✅ NVDA + Firefox
NVDA announces:
✅ JAWS18 + IE11
JAWS announces:
(includes JAWS commands)
Fieldset with
<legend>
✅ Voiceover + Safari (Mac OS X El Capitan)
Voiceover announces:
✅ Voiceover + Safari (iOS 11)
When entering the fieldset (e.g. navigating backwards through the document)", Voiceover announces:
Then, for each input:
✅️ Voiceover + Safari (iOS 10)
When entering the fieldset (e.g. navigating backwards through the document)", Voiceover announces:
Then, for each input:
❗️ Voiceover + Safari (iOS 9)
For each input, Voiceover announces:
iOS 9 does not seem to announce
<fieldset>
legends for inputs.✅ NVDA + Firefox
NVDA announces:
✅ JAWS18 + IE11
JAWS announces: