-
Notifications
You must be signed in to change notification settings - Fork 324
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
Update legend/label/heading examples to have unique names and ids #2441
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
govuk-design-system-ci
temporarily deployed
to
govuk-frontend-pr-2441
November 24, 2021 14:43
Inactive
govuk-design-system-ci
temporarily deployed
to
govuk-frontend-pr-2441
November 24, 2021 14:58
Inactive
vanitabarrett
force-pushed
the
update-label-legends-headings-examples
branch
from
November 25, 2021 09:57
f5da9b1
to
10105ba
Compare
govuk-design-system-ci
temporarily deployed
to
govuk-frontend-pr-2441
November 25, 2021 09:58
Inactive
vanitabarrett
force-pushed
the
update-label-legends-headings-examples
branch
from
November 25, 2021 10:21
10105ba
to
f8f81c9
Compare
govuk-design-system-ci
temporarily deployed
to
govuk-frontend-pr-2441
November 25, 2021 10:21
Inactive
For some reason the radio buttons are now pre-selected on |
govuk-design-system-ci
temporarily deployed
to
govuk-frontend-pr-2441
December 3, 2021 13:30
Inactive
lfdebrux
reviewed
Dec 3, 2021
Every example in the legend/label/heading examples has the same `name` and `idPrefix`. Having the same `name` on every example meant that the checkboxes and radio buttons looked like they were part of one question. For example, when testing with Voiceover and Safari, you would hear something like: "Yes, radio button, 1 of 60" for a simple yes/no question. The `idPrefix` is used to prefix the id's for the hints. If not present, the `name` is used instead. Because the `idPrefix` values were also identical, this led to hints being associated and read out for the wrong questions because there were multiple hints with the same `id`. This commit updates each `name` to give it a unique value. It also removes the `idPrefix`, which means the `id` will be generated from the `name`. This is ok in this case as the names are now all unique.
with the existing implementation, each text input in error had the same id. When testing with NVDA 2021.2 + Firefox, this meant that when a user tabbed to the text input, they would hear every single label with `for="input-with-error-message"`, e.g: ``` <h1> govuk-label--xl <h1> govuk-label--l <h1> govuk-label--m <h1> govuk-label--s <h1> No class <label> govuk-label--xl <label> govuk-label--l <label> govuk-label--m <label> govuk-label--s <label> No class edit has auto complete ``` This commit replaces the duplicate id's with unique ones to stop this happening
The `id` for text input examples were being set to a blank string. This was leading to text inputs having an `id` (and therefore an `aria-describedby`) starting with a dash, i.e: `id="-hint"`. Each text input also had the same `id`. This commit replaces the blank `id` and uses the same value as the `name` so each text input has a unique `id`.
The radio button examples had the 'No' radio pre-selected, but we don't recommend pre-selecting in our guidance and none of the other examples on this page have any preselected/pre-populated inputs. The `checked` attribute was already present in the code, but wasn't working properly because of the duplicate names/ids. Now that we fixed that, it had the unintentional effect of making the `checked` code work. This commit removes the `checked` attribute from the radio button examples.
vanitabarrett
force-pushed
the
update-label-legends-headings-examples
branch
from
December 3, 2021 15:21
714bece
to
8c3ede7
Compare
govuk-design-system-ci
temporarily deployed
to
govuk-frontend-pr-2441
December 3, 2021 15:22
Inactive
lfdebrux
approved these changes
Dec 6, 2021
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.
W3C validator reports no errors or warnings 🙌
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I came across this when using these examples to test #2083
The current examples for legends, labels and headings have the same
name
andid
(oridPrefix
). This meant for things using checkboxes and radios (for example), the checkboxes were appearing as one list of checkboxes for screenreader users, e.g: tickbox 2 of 60.It also meant that hints were being read incorrectly for some questions, because all hints on the page had the same
id
(e.g:example-hint
).This commit gives each question a unique name, and either removes the
idPrefix
so it falls back to thename
or gives each question a uniqueid
too.Each individual commit contains more details on the issues this was causing when testing with assistive technologies and the change that's been made to fix it.
https://govuk-frontend-pr-2441.herokuapp.com/examples/labels-legends-and-headings