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

De-duplicate legends/headings #507

Merged
merged 1 commit into from
Jul 3, 2017
Merged

De-duplicate legends/headings #507

merged 1 commit into from
Jul 3, 2017

Conversation

selfthinker
Copy link
Contributor

@selfthinker selfthinker commented Jun 28, 2017

What problem does the pull request solve?

When we have one thing per page, our current pattern is to let the h1 be the question and then repeat but visually hide it in the legend (or label). That means screen readers will read the question twice, which is not a proper barrier but annoying and gets raised as an issue again and again, e.g. in #320 or on the cross-government accessibility mailinglist.

This changes that by moving the h1 into the legend. That creates the following issues:

  • It is currently invalid. But it will be valid in HTML 5.2 and the W3C validator passes it fine. (See the reason for the change.) As such it might cause issues with existing validators run during CI. Does anyone know if this will be a problem for them?
  • Because the h1 is moved further down into the code inside other elements, some systems/frameworks might make it difficult to do. Does anyone already know they would have problems with that? We could add some guidance that using the old way would be fine in that case (i.e. visually hiding the legend).
  • When the label is in an h1 (see below), then also having the hint in there could potentially be too much. Although that will more be a problem for examples that use potentially unnecessary long hints. And please note, that is not a problem for the legend, only for the label as another element for the hint can exist alongside the h1.

What else could solve this?

I tested some other ways of fixing this issue, some of which have been suggested by the cross-government accessibility community in the thread mentioned above:

  • Visually hide the heading instead of the label: It doesn't solve the problem
  • Remove the legend and use aria-labelledby instead: It solves the problem (except JAWS and legend combination) but feels wrong, 7 out of 10 testing tools fail this, although all tested AT is fine with it
  • Remove heading and use role=heading instead: It solves the problem but feels even more wrong, the h1 is too essential to lose (even if it's there for AT, it would be missing for other things, e.g. search engines), 4 out of 10 testing tools fail this, although all tested AT is fine with it

How has this been tested?

Browsers

I tested this in various old and new browsers via BrowserStack and it's not causing any issues anywhere. I have also built a local version with the question being a label for a text input and the label being in an h1. That didn't cause an issue either.
Neither has had any visual effect in any of the tested browsers.

Screen readers

I tested in a couple of screen readers:
JAWS with IE11, NVDA in Firefox and VoiceOver on iOS read everything once (instead of twice) when "down-arrowing". VoiceOver on macOS reads everything twice (instead of three times) and reads the heading after it announced the group.

Accessibility testing tools

I also wanted to make sure that accessibility testing tools wouldn't report any unnecessary issues, so tested this in aXe, Tenon, AChecker, WAVE, FAE, EIII, HTML_CodeSniffer, SortSite, Google A11y Dev Tools and Asqatasun. And only EIII came back with one fail for the legend ("The name of the form control is not set correctly"). As I mentioned above, all the other viable solutions failed way more times than this solution.

How to test yourself

If you want to test this solution yourself, I'd suggest to not use this branch because it only includes one single type of implementation, i.e. a single question for radio buttons with a legend. I have tested all the difference scenarios I could think of, including when the question is for a text input, i.e. it would need to be in a label not in a legend. Although I have tested that locally within a version of the prototype kit with GOV.UK branding and Elements styles etc, I have created three separate unstyled JSbin version, so you can test just the markup independently:

What is potentially missing? / Open questions

  • I wonder if we should add more examples? Although there are lots of examples, they are only of one type of implementation, the other two types are missing.
  • Do we need to add to the documentation that falling back to visually hiding the legend is acceptable in case someone has problems implementing this solution?

What type of change is it?

  • Bug fix (non-breaking change which fixes an issue)

Has the documentation been updated?

  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.

When we have one thing per page, our current pattern is
to let the h1 be the question and then repeat but visually hide it
in the legend (or label).
That means screen readers will read the question twice,
which is not a proper barrier but annoying and gets raised as an issue
again and again, e.g. in #320 or on the cross-government accessibility mailinglist
and from accessibility auditors.

This changes that by moving the h1 into the legend.
That is currently invalid but will be valid in HTML 5.2
[https://www.w3.org/TR/html52/sec-forms.html#the-legend-element]
and the W3C validator passes it fine.
Also see the reason for the change: w3c/html#724.
@gemmaleigh gemmaleigh temporarily deployed to govuk-elements-review-pr-507 June 28, 2017 14:32 Inactive
Where should we send your new passport?
<legend>
<h1 class="heading-large">
Where should we send your new passport?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this still need the line-break that was in the h1?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have removed it because I didn't understand why it was there in the first place. I think it's generally bad practice to have <br>s in a heading. I tried to find out via git blame why it was there but got lost trying to get before October 2014.

BTW, without this change, it currently looks broken on mobile because of that br:
br-in-h1

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I just found it. It was there from the beginning of the checkbox examples in bc2c6b4, so there is no further explanation as it was part of a bigger thing. Maybe @gemmaleigh knows more?

@fofr
Copy link
Contributor

fofr commented Jun 28, 2017

Presumably in a circumstance where the H1 and the legend text differ we'd still advocate having both. Not sure what would justify the difference though.

I've assumed this, but wanted to double check: Does the output still look identical?

This is a really nice PR that answers all of the questions I had about this pattern. 👌 💯

@selfthinker
Copy link
Contributor Author

In a "one thing per page" pattern, the h1 and the legend should be the same. If you have more things per page, then they should obviously be different as the h1 should reflect what the page is about and the legend should be the question for each group of things. In that case they would both be visible, so would not be the same pattern that this is about (where originally the legend was visually hidden). We have other examples where both the h1 and the legend are visible and they're both different.

Yes, the output sill looks identical. That's what I meant when I wrote

Neither has had any visual effect in any of the tested browsers.

@LJWatson
Copy link

This solution tests well with Jaws in Chrome, Firefox and IE, and with NVDA in Chrome, Firefox, IE and Edge (although the legend itself isn't recognised by NVDA in Edge regardless of whether its in a heading or not).

@jbuller
Copy link

jbuller commented Jun 29, 2017

Let me challenge: Why does the H1 have to match the question?
Usually the H1 should (closely) match the page title attribute (not including the service and GOV.UK names)?
The title and H1 can be for example 'Your nationality' and the question can be more detailed.
That keeps the non-question style of the titles and H1s the same across the service(s).
I just worry a bit about sowing potential for confusion in to the patterns with exceptions to consistency for both design teams and users.

@LJWatson
Copy link

If the title and h1 reflect each other, I think that's ok. So if you have <title>Your nationality...</title>, and <h1><legend>What is your nationality</legend></h1>, I think that's ok.

@joelanman
Copy link
Contributor

@jbuller the H1 does not always have to match the question, that's not being proposed. However in many cases it does match, due to the 'one thing per page' pattern. If you look at Register to vote you can see this in action.

@stevenaproctor
Copy link

stevenaproctor commented Jun 30, 2017

What happens if there is no fieldset and legend but a visually hidden label and a visible form field? Would the h1 go inside the label?

Current code:

<h1>What is your postcode?</h1>
<label class="visuallyhidden">What is your postcode?</label>
<input>

New code:

<label><h1>What is your postcode?</h1></label>
<input>...

Is this correct?

Also, for page titles and h1s, HMRC's guidance is they should match. See hmrc/design-patterns#90

@selfthinker
Copy link
Contributor Author

If I understand @jbuller correctly, I think he questions the pattern itself to have only the question visible not this code interpretation of it?
For example, if you have more than one thing per page, like your address, the h1 (and half of the title) would be "Your address" and all the inputs would have their own label/legend. If you wanted to keep it consistent, you could keep both h1 and legend/label visible for one thing per page, e.g. "Your account" as the h1 (and half of the title) and "Do you already have an account?" as the legend.
While I think that would make the pattern easier for developers and more consistent, I believe the current pattern (which would have both the h1 inside the legend and half the title as "Do you already have an account?") has been tested multiple times vs the other one and having duplicate information on the page proved less successful. But designers like @joelanman will know much more about previous user tests and can explain it better.

@stevenaproctor, that's nearly correct, see the code I linked above to Label in heading. So with your example it would be

<h1><label>What is your postcode?</label></h1>
<input>...

The h1 inside a label would be invalid HTML, so it needs to be the other way around.

@stevenaproctor
Copy link

Thanks @selfthinker

@davehaigh
Copy link

top and bottom margins aren't being applied to block elements (e.g. h1) inside legends on Safari. Adding display: inline-block; to the h1 solves this and the margins are then applied.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants