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

Add heading level option to label component called headingLevel #5240

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

aliuk2012
Copy link
Contributor

This optional setting only works if the label is a page heading i.e isPageHeading is set to true.

A HMPPS service had an external accessibility audit raised the following point on a few of it pages:

The form labels are styled to look like headings and stand out visually. While sighted users can identify headings based on larger font size and bold formatting, screen reader users rely on proper HTML markup to understand the structure of a page.

Screen reader users sometimes jump between the headings of the page to gain a better understanding of the structure and content on the page. The WebAIM screen reader survey has found that headings are the number one means by which screen reader users get a sense of a complex page. When headings are not available programmatically to users, it affects the time and effort needed to understand how the content is presented on the page. Furthermore, this creates a discrepancy between information available to a sighted individual and users relying on assistive technology.

They suggested something like

<h2>
    <label class="govuk-label govuk-label--m" for="myInputField">
        This is a h2 label
     </label>
</h2>

The problem is that the existing label component:

  1. It does support custom html, but that would place the headings inside the label means Inline elements would contain block level elements which is not valid
  2. Using isPageHeading, uses a hard coded h1 so cannot use that options

These changes are backwards compatible by setting the heading level to 1.

This optional setting only works if the label is a page heading i.e `isPageHeading` is set to true.

A HMPPS service had an external accessibility audit raised the following point on a few of it pages:

> The form labels are styled to look like headings and stand out visually. While sighted users can identify headings based on larger font size and bold formatting, screen reader users rely on proper HTML markup to understand the structure of a page.

> Screen reader users sometimes jump between the headings of the page to gain a better understanding of the structure and content on the page. The [WebAIM screen reader survey](https://webaim.org/projects/) has found that [headings are the number one means by which screen reader users get a sense of a complex page](https://webaim.org/projects/screenreadersurvey9/#finding). When headings are not available programmatically to users, it affects the time and effort needed to understand how the content is presented on the page. Furthermore, this creates a discrepancy between information available to a sighted individual and users relying on assistive technology.

They suggested something like
```
<h2>
    <label class="govuk-label govuk-label--m" for="myInputField">
        This is a h2 label
     </label>
</h2>
```

The problem is that the existing label component:

1. It does support custom html, but that would place the headings inside the label means Inline elements would contain block level elements which is not valid
2. Using isPageHeading, uses a hard coded h1 so cannot use that options

These changes are backwards compatible by setting the heading level to 1.
@romaricpascal
Copy link
Member

Hi Alistair!

Thanks for the suggestion, this feels useful, but before moving further with it, we'll likely want some more input from our accessibility specialist and research to better understand when and when not form labels need wrapping in headings so we could issue the appropriate guidance with the option.

Releasing the option would also have an effect on our Fieldset component which has a similar isPageHeading option for its legend as the Label.

In terms of API, it may be preferable to have it replace the isPageHeading option altogether, which would let people set a single option to control the heading level (isPageHeading being an equivalent to headingLevel=1 while being deprecated until its removal).

As an aside for when we look further into this, I've tracked the introduction of the isPageHeading option to this pull request which may contain extra context.

@aliuk2012
Copy link
Contributor Author

Hi @romaricpascal

Thanks for your replying. A very good point about fieldset component would need to behave in exactly the same way. Do you want me to make those changes as part of this PR?

I agree, replacing isPageHeading might be a better option but I was trying to suggest a smaller change that wouldn't break existing functionality and mean that the change would have to wait to be part of a major version bump.

It terms of this change, I'm not 100% clear on what is required of me to move this forward. Happy to help out and contribute as much as I can.

@romaricpascal
Copy link
Member

@aliuk2012 I think the next step before going any further is to figure when/when not to make field labels headings from an accessibility standpoint. Would you or your organisation have any research around when/when not to make field labels headings? @selfthinker are you aware if it's something that's ever been looked into at GDS (seems the original PR only added <h1>, but that might be due to focusing on 'one question per page')?

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

Successfully merging this pull request may close these issues.

2 participants