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

allow heading content within a legend element #5090

Merged
merged 10 commits into from
Feb 19, 2020

Conversation

scottaohara
Copy link
Collaborator

@scottaohara scottaohara commented Nov 15, 2019

This PR updates the legend’s content model to allow for heading content. This update uses the summary element’s content model as the reference for how I think this should be indicated in the spec.

If accepted, this closes #4990

Thanks


/form-elements.html ( diff )
/indices.html ( diff )
/interactive-elements.html ( diff )
/sections.html ( diff )

This PR updates the `legend`’s content model to allow for heading content, using the `summary` element’s content model as the template for how I think this should be indicated in the spec.

If accepted, this closes whatwg#4990

Thanks
@annevk
Copy link
Member

annevk commented Nov 15, 2019

https://www.accessibility-developer-guide.com/examples/forms/grouping-with-headings/ suggests this has quite special semantics in its conclusion. In that the level is not announced. Does that need to be reflected somehow?

And when should you use a normal legend vs a heading legend?

@annevk annevk added the accessibility Affects accessibility label Nov 15, 2019
@scottaohara
Copy link
Collaborator Author

scottaohara commented Nov 15, 2019

suggests this has quite special semantics in its conclusion

the way the conclusion is written is not entirely accurate / robust in exactly how this works. Because yes, the heading's semantics are not announced when a screen reader navigates into a grouping via form elements quick key or via tab key.... but that's not unique to headings. Any element within a legend will have it's role dropped when the accessible name of the group is announced.

if navigating by heading elements, there is no issue save for a support gap with Firefox and JAWS. VO + Safari, NVDA + FF/Chrome and JAWS + Chrome all allow for navigating to the headings and will announce the heading role as expected.

So per your question:

Does that need to be reflected somehow?

no. it is reflected correctly depending on navigation method, and works as expected save for the one mentioned gap i found in quickly re-testing.

And when should you use a normal legend vs a heading legend?

when a section of a form should be exposed in the document outline, which is beneficial in allowing easier access to it for screen reader users / people who have plugins that allow them to navigate a document by headings. Additionally it's useful when a document itself is largely form related and having a separate heading and legend would create duplicative information (see link to gov.uk issue below).

@stevefaulkner's link in 4990 to the original discussion w3c/html#724 had a good initial use case/reasoning by @DavidMacDonald, agreement at the time by @sideshowbarker, and the thread contains various links to issues from gov.uk where they are using this pattern in practice, and are opting to ignore the current error from the validator due to this working well for them and their users.

Copy link

@SelenIT SelenIT left a comment

Choose a reason for hiding this comment

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

LGTM

@annevk
Copy link
Member

annevk commented Nov 25, 2019

@scottaohara looking at https://github.com/alphagov/govuk_elements/pull/507/files suggests that only allowing a heading element is not enough to make those templates conforming, unless there's some non-obvious post-processing. E.g., some of the legend elements there have a heading element followed by a span element.

@scottaohara
Copy link
Collaborator Author

@annevk ah yes. that's an oversight on my part, thank you.

The intent was to allow a heading as a child of a legend, but to indicate it wouldn't be ideal to have multiple headings.

With that in mind, would one of the following work for you?

Noting they're both allowed, but not being restrictive:

Phrasing content and heading content.

or rephrasing my original text:

Either: phrasing content.
Or: phrasing content and one element of heading content.

Thank you

@annevk annevk added document conformance impacts documentation Used by documentation communities, such as MDN, to track changes that impact documentation labels Nov 25, 2019
@annevk
Copy link
Member

annevk commented Nov 25, 2019

I'd expect something like "Phrasing content, optionally intermixed with heading content" (not sure restricting to a single element makes sense if you allow hgroup). I guess the other question is whether the summary element should similarly change.

@scottaohara
Copy link
Collaborator Author

I've adjusted the PR to match the wording you provided, thank you.

Regarding updating summary, would you want that done in this PR, or discuss in a separate issue?

sideshowbarker
sideshowbarker previously approved these changes Nov 26, 2019
@sideshowbarker
Copy link
Member

Note that the original discussion at w3c/html#724 was about allowing each legend element to contain either heading content or phrasing content, but not both — not intermixed.

looking at https://github.com/alphagov/govuk_elements/pull/507/files suggests that only allowing a heading element is not enough to make those templates conforming, unless there's some non-obvious post-processing. E.g., some of the legend elements there have a heading element followed by a span element.

If @scottaohara and others who’ve looked in detail at that site and related cases are confident there’s an accessibility win in allowing a legend element to contain an intermixing of heading-content children and phrasing-content children — rather than an either-or that allows each legend to only have one or the other — then yeah, "Phrasing content, optionally intermixed with heading content" is the right wording. Otherwise, I’d suggest we restrict it to being either-or.

I guess the other question is whether the summary element should similarly change.

Yeah, since we’re allowing hgroup in summary, then I think the summary content model should be changed to match what’s in the patch here for legend ("Phrasing content, optionally intermixed with heading content").

Regarding updating summary, would you want that done in this PR, or discuss in a separate issue?

IMHO, we should include the summary change here. That way, somebody reading through the change history later will see that these were changed together, with the same rationale.

as per the update to `legend`, update `summary` to allow phrasing content optionally intermixed with heading content.
@scottaohara
Copy link
Collaborator Author

Thank you both for your feedback here. I've updated the PR now to adjust the content model for summary to match the updated model for legend.

@annevk
Copy link
Member

annevk commented Nov 26, 2019

I was going to land this as Allow heading content within legend and summary elements but I realized something:

  1. We need to update h1-6 and hgroup to state that they can be used "Where heading content is expected."
  2. The indexes need updating.
  3. Maybe we should add an example?

@36degrees
Copy link
Contributor

Hi folks,

Exciting to see this change going through.

I just wanted to flag that in a recent audit of the GOV.UK Design System some bugs with JAWS and ZoomText were reported when the heading was placed inside a legend.

alphagov/govuk-frontend#1605

As discussed in the issue, this appears to affect JAWS 2018 and above – JAWS 17/18 which we previously tested with when we made the change work fine. We've reported those issues to Freedom Scientific. We haven't yet been able to reproduce the issue reported in ZoomText.

We still believe that allowing headings inside legends is the right approach. Hopefully having this in the WHATWG HTML spec will help to get things fixed when we're reporting bugs with the vendors.

@scottaohara
Copy link
Collaborator Author

hi @36degrees
yes i noticed the JAWS issue as well and mentioned in the related issue to this PR. I've passed along the information and a bug with JAWS will be filed.

@sideshowbarker sideshowbarker self-requested a review January 31, 2020 12:10
@sideshowbarker sideshowbarker dismissed their stale review January 31, 2020 12:11

further changed needed per #5090 (comment)

@sideshowbarker
Copy link
Member

@scottaohara I think this PR is waiting on the further changes needed that @annevk listed in #5090 (comment)

@scottaohara
Copy link
Collaborator Author

@sideshowbarker @annevk, apologies. it's been on my to do list. I'll try to get it done within the next few days.

@scottaohara
Copy link
Collaborator Author

scottaohara commented Feb 13, 2020

@annevk & @sideshowbarker i believe i've addressed the bullets. i added an example to the end of the other fieldset examples. if you think that'd be best somewhere else, or if you notice anything i've missed/would like additional updates on, just let me know.

thanks

Copy link
Member

@annevk annevk left a comment

Choose a reason for hiding this comment

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

This seems mostly good, but the indices are still not quite right. I suspect we should also update the summary listing there while we're changing this as it currently doesn't mention heading content at all.

source Show resolved Hide resolved
source Show resolved Hide resolved
@scottaohara
Copy link
Collaborator Author

thanks @annevk. updated per your comments and added heading content to the summary index.

Copy link
Member

@annevk annevk left a comment

Choose a reason for hiding this comment

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

Thanks, I think this is good now, but it could really use a fresh pair of eyes.

Copy link
Member

@domenic domenic left a comment

Choose a reason for hiding this comment

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

This mostly looks good to me, with one comment. @sideshowbarker's eyes would definitely be helpful from the conformance-checker perspective, to see if there's any updates we missed...

source Show resolved Hide resolved
Copy link
Member

@sideshowbarker sideshowbarker left a comment

Choose a reason for hiding this comment

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

LGTM

@annevk annevk merged commit db55877 into whatwg:master Feb 19, 2020
@annevk
Copy link
Member

annevk commented Feb 19, 2020

Thanks @scottaohara!

@scottaohara scottaohara deleted the 4990_allow_heading_content_in_legend branch February 19, 2020 13:56
chris-gds pushed a commit to alphagov/govuk_publishing_components that referenced this pull request Oct 2, 2021
When a radio input is used on a page a heading may not be required for accessibility.  

A legend however would be required.  This adds this option as well as providing visual styling options should the the legend need to have different sizing options.  

There is also an update to include a possible error state and clarification of language when as heading is used to indicate this is nested inside a legend[1]

There are some on-going accessibility considerations regarding a heading nested inside a legend [2]

[1]whatwg/html#5090 
[2]alphagov/govuk-frontend#1605
chris-gds pushed a commit to alphagov/feedback that referenced this pull request Oct 2, 2021
GOV.UK Contact page was flagged as having a confusing heading structure. 

"Contact GOV.UK" is h1 
"What's it to do with?" is a h2 however
"What are the details?, Do you want a reply?" are legends

The ideal solution would be to restructure this page to ask "one question per page" as per the Design System's Guidance[1]

Also it was discovered that nested headings inside legends has been added to the HTML spec, in part, by work from the Design System team[3][4]

However from an accessibility POV - from this update - there seem to be some additional considerations to take into account with JAWS not recognising headings while nested.[5]

One balance, the accessibility team decided to remove the h2 on the page.  This is then more inline with Design System guidance and resolves the confusing structure.

[1]https://design-system.service.gov.uk/patterns/question-pages/#start-by-asking-one-question-per-page

[2]https://design-system.service.gov.uk/get-started/labels-legends-headings/

[3]w3c/html#724
[4]whatwg/html#5090
[5]alphagov/govuk-frontend#1605
chris-gds pushed a commit to alphagov/govuk_publishing_components that referenced this pull request Oct 4, 2021
When a radio input is used on a page a heading may not be required for accessibility.  

A legend however would be required.  This adds this option as well as providing visual styling options should the the legend need to have different sizing options.  

There is also an update to include a possible error state and clarification of language when as heading is used to indicate this is nested inside a legend[1]

There are some on-going accessibility considerations regarding a heading nested inside a legend [2]

Add tests 
- for adding just the legend
- the legend but styled.

[1]whatwg/html#5090 
[2]alphagov/govuk-frontend#1605
Update testing
chris-gds pushed a commit to alphagov/feedback that referenced this pull request Oct 4, 2021
GOV.UK Contact page was flagged as having a confusing heading structure. 

"Contact GOV.UK" is h1 
"What's it to do with?" is a h2 however
"What are the details?, Do you want a reply?" are legends

The ideal solution would be to restructure this page to ask "one question per page" as per the Design System's Guidance[1]

Also it was discovered that nested headings inside legends has been added to the HTML spec, in part, by work from the Design System team[3][4]

However from an accessibility POV - from this update - there seem to be some additional considerations to take into account with JAWS not recognising headings while nested.[5]

One balance, the accessibility team decided to remove the h2 on the page.  This is then more inline with Design System guidance and resolves the confusing structure.

[1]https://design-system.service.gov.uk/patterns/question-pages/#start-by-asking-one-question-per-page

[2]https://design-system.service.gov.uk/get-started/labels-legends-headings/

[3]w3c/html#724
[4]whatwg/html#5090
[5]alphagov/govuk-frontend#1605
chris-gds pushed a commit to alphagov/govuk_publishing_components that referenced this pull request Oct 4, 2021
When a radio input is used on a page a heading may not be required for accessibility.  

A legend however would be required.  This adds this option as well as providing visual styling options should the the legend need to have different sizing options.  

There is also an update to include a possible error state and clarification of language when as heading is used to indicate this is nested inside a legend[1]

There are some on-going accessibility considerations regarding a heading nested inside a legend [2]

Add tests 
- for adding just the legend
- the legend but styled.

[1]whatwg/html#5090 
[2]alphagov/govuk-frontend#1605

Update CHANGELOG


Revert "Add legend option for radio input"

This reverts commit bedaefe4894f91b4d4a71a355b60f5ca9e744eda.

Add additional examples

- Add new examples to show the radio without a heading but with a legend alone.  When the heading_level is set to 0 this defaults to a span

- Adding additional tests to reference this

- Slight adjustments to language
a


a
chris-gds pushed a commit to alphagov/feedback that referenced this pull request Oct 4, 2021
GOV.UK Contact page was flagged as having a confusing heading structure. 

"Contact GOV.UK" is h1 
"What's it to do with?" is a h2 however
"What are the details?, Do you want a reply?" are legends

The ideal solution would be to restructure this page to ask "one question per page" as per the Design System's Guidance[1]

Also it was discovered that nested headings inside legends has been added to the HTML spec, in part, by work from the Design System team[3][4]

However from an accessibility POV - from this update - there seem to be some additional considerations to take into account with JAWS not recognising headings while nested.[5]

One balance, the accessibility team decided to remove the h2 on the page.  This is then more inline with Design System guidance and resolves the confusing structure.

[1]https://design-system.service.gov.uk/patterns/question-pages/#start-by-asking-one-question-per-page

[2]https://design-system.service.gov.uk/get-started/labels-legends-headings/

[3]w3c/html#724
[4]whatwg/html#5090
[5]alphagov/govuk-frontend#1605
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accessibility Affects accessibility document conformance impacts documentation Used by documentation communities, such as MDN, to track changes that impact documentation
Development

Successfully merging this pull request may close these issues.

Adopt the allowance of headings inside legend from W3C HTML5.2?
6 participants