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

Use generic div element for tabspanel #1746

Merged
merged 2 commits into from
Feb 26, 2020

Conversation

NickColley
Copy link
Contributor

Sections when they have an accessible name create landmarks.

The use of region landmarks should be limited, as too many landmarks can dilute their usefulness in quickly navigating to important areas of a page. If you find there are many landmarks, especially generic regions existing in a single page, then the page’s structure should be reexamined.

Also, when the component has not be enhanced these sections do not have
accessible names so it means they are the equivalent of a div already.

When they're enhanced they're given a role of tabpanel, so they're no
longer a section.

Given this I'm not sure it was ever useful and by making them generic
divs we can avoid HTML validators warning about this component.

Closes #1339

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-1746 February 25, 2020 10:17 Inactive
@NickColley NickColley added this to Needs review in Design System Sprint Board via automation Feb 25, 2020
@NickColley NickColley added this to the [NEXT] milestone Feb 25, 2020
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-1746 February 25, 2020 10:18 Inactive
Copy link
Member

@hannalaakso hannalaakso left a comment

Choose a reason for hiding this comment

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

Looks good 👍

We had a chat about whether we should inform users about this in the release notes in our next breaking release but since there is no change in behaviour we don't think it's necessary to do that.

Sections when they have an accessible name create landmarks.

> The use of region landmarks should be limited, as too many landmarks can dilute their usefulness in quickly navigating to important areas of a page. If you find there are many landmarks, especially generic regions existing in a single page, then the page’s structure should be reexamined.

- https://www.scottohara.me/blog/2018/03/03/landmarks.html

Also, when the component has not be enhanced these sections do not have
accessible names so it means they are the equivalent of a div already.

When they're enhanced they're given a role of tabpanel, so they're no
longer a section.

Given this I'm not sure it was ever useful and by making them generic
divs we can avoid HTML validators warning about this component.

Closes #1339
@NickColley NickColley force-pushed the tabs-use-divs-instead-of-sections branch from 72068e3 to 0ddb915 Compare February 26, 2020 13:17
@NickColley NickColley merged commit 51f0112 into master Feb 26, 2020
Design System Sprint Board automation moved this from Needs review to Done Feb 26, 2020
@NickColley NickColley deleted the tabs-use-divs-instead-of-sections branch February 26, 2020 13:20
@alex-ju
Copy link
Contributor

alex-ju commented Feb 26, 2020

As I remember, on the initial implementation of tabs component a (potentially visually hidden) heading was meant to be automatically generated for each section acting as identifier (as exemplified in the Design System) – hence passing the HTML validation.

@hannalaakso hannalaakso modified the milestones: Next, v3.6.0 Mar 3, 2020
@36degrees 36degrees mentioned this pull request Mar 6, 2020
andymantell added a commit to surevine/govuk-react-jsx that referenced this pull request Mar 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

Consider making tabs <section> into <div>
4 participants