-
Notifications
You must be signed in to change notification settings - Fork 331
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
Axe issue: "All page content must be contained by landmarks" #1604
Comments
Just to clarify, the 'govuk-frontend-review' application is not intended to be guidance and contains intentionally legacy and therefore out of date patterns for test the 'compatibility mode'. So that's the reason why one of the examples in there is wrong. In the official guidance on the Design System website we intentionally do not have the phase banner, back links, or breadcrumbs in the 'main' element. This is for two reasons I know of:
I don't have the original research for the latter decision, but will ask around and see if I can dig it up. Will leave this in triage for us to decide as a team what direction we want to take, thanks a lot for raising this I'm sure others will run into this too. |
With the help from @36degrees @selfthinker I've pulled out some of the historical stuff:
|
@NickColley This is exactly the sort of thing I was after! Thank you for the detective work 🕵 |
I've added this to #1280 (comment) as it can also seen on the GOV.UK Frontend Template. Thanks again for raising this @colinrotherham |
Looks like this has been resolved and added to documentation for people to find in the future so will close this one out, thanks :) |
Re-opening this in favour of #1949 as this has more context. Whilst we previously decided against moving the phase banner, back link or breadcrumbs into the |
We could also revisit wrapping the breadcrumbs and back link in navigation landmarks, taking into consideration https://insidegovuk.blog.gov.uk/2013/07/03/rethinking-navigation/ and validating with user research. |
I have come across this issue recently in our code and have worked up a potentional solution for the phase banner being moved into the header. How do I go about getting rights to push a PR for discussion? |
@robertparkinson we do not grant write permissions to users outside of the team, but we're more than happy for you to raise a pull request from a fork. It does sound like the change you're suggesting would be fairly substantial (and breaking), so it would need to be managed carefully. To avoid any wasted effort on your part, are you able to describe the change that you're planning on proposing first? |
Non breaking implementation so low impact on existing users. See alphagov#1604 for details
Thanks @36degrees I've opened a PR. Would love your feedback and have just focussed on the phase banner as that should be an easier conversation than surroundign the back button with nav tags. #1984 |
Having the ability to add additional content before the HEADER closing tag would definitely be a step forward. This would be for content that is semantically part of the header for screen reader purposes, for example, a departmental banner, but visually under the grey bar. Allowing this would mean screenreader users can get to the main content quicker and avoid Aria difficulties around having multiple banner landmarks on the same page. |
As discussed in the V4 scoping session, this needs more investigation to find out what the recommended implementation is and ideally test with users before we make the change live. We should also talk to GOV.UK and find out what changes they're planning to the header component (and if we need to make any changes as a result), as that may affect this work. |
This adds an exclusion to accessibility rules for the phase banner, as axe was failing tests for the following rule: "All page content must be contained by landmarks". This is due to the banner being outside `<header>` and outside `<main>`. This is a [known issue](alphagov/govuk-frontend#1604) with the govuk-frontend component, but as the "region" rule is not a requirement for WCAG, it should be ok to ignore for the time being on this banner. I initially tried moving the banner down into `<main>` but this didn't look good.
This adds an exclusion to accessibility rules for the phase banner, as axe was failing tests for the following rule: "All page content must be contained by landmarks". This is due to the banner being outside `<header>` and outside `<main>`. This is a [known issue](alphagov/govuk-frontend#1604) with the govuk-frontend component, but as the "region" rule is not a requirement for WCAG, it should be ok to ignore for the time being on this banner. I initially tried moving the banner down into `<main>` but this didn't look good.
We revisited this very briefly in a discussion amongst some of the devs today. The takeaways:
|
According to UKGovernmentBEIS/regulated-professions-register@33be548 this fails axe tests because of the rule "All page content must be contained by landmarks". It's unclear how to get cypress-axe to tell you the rule that's been violated, but if I run the integration tests in UI mode, I can verify that the error highlights the phase banner Again according to that commit, this is due to the banner being outside `<header>` and `<main>` elements. Apparently this is a [known issue][1] with the component, but as the region rule is not a requirement for WCAG, it should be okay to ignore for now [1]: alphagov/govuk-frontend#1604
Per UKGovernmentBEIS/regulated-professions-register@33be548 this fails axe tests because of the rule "All page content must be contained by landmarks" It's unclear how the rules that were violated were identified in that PR, since it doesn't appear to add any logging of these. Here we've added the logging, and when running the tests in the Cypress UI, we can see that clicking on the error highlights the phase banner Again according to that PR, the violation is due to the banner being outside `<header>` and `<main>` elements. Apparently this is a [known issue][1] with the component, but as the region rule is not a requirement for WCAG, it should be okay to ignore for now [1]: alphagov/govuk-frontend#1604
Per UKGovernmentBEIS/regulated-professions-register@33be548 this fails axe tests because of the rule "All page content must be contained by landmarks" It's unclear how the rules that were violated were identified in that PR, since it doesn't appear to add any logging of these. Here we've added the logging, and when running the tests in the Cypress UI, we can see that clicking on the error highlights the phase banner Again according to that PR, the violation is due to the banner being outside `<header>` and `<main>` elements. Apparently this is a [known issue][1] with the component, but as the region rule is not a requirement for WCAG, it should be okay to ignore for now [1]: alphagov/govuk-frontend#1604
Per UKGovernmentBEIS/regulated-professions-register@33be548 this fails axe tests because of the rule "All page content must be contained by landmarks" It's unclear how the rules that were violated were identified in that PR, since it doesn't appear to add any logging of these. Here we've added the logging, and when running the tests in the Cypress UI, we can see that clicking on the error highlights the phase banner Again according to that PR, the violation is due to the banner being outside `<header>` and `<main>` elements. Apparently this is a [known issue][1] with the component, but as the region rule is not a requirement for WCAG, it should be okay to ignore for now [1]: alphagov/govuk-frontend#1604
Per UKGovernmentBEIS/regulated-professions-register@33be548 this fails axe tests because of the rule "All page content must be contained by landmarks" It's unclear how the rules that were violated were identified in that PR, since it doesn't appear to add any logging of these. Here we've added the logging, and when running the tests in the Cypress UI, we can see that clicking on the error highlights the phase banner Again according to that PR, the violation is due to the banner being outside `<header>` and `<main>` elements. Apparently this is a [known issue][1] with the component, but as the region rule is not a requirement for WCAG, it should be okay to ignore for now [1]: alphagov/govuk-frontend#1604
Per UKGovernmentBEIS/regulated-professions-register@33be548 this fails axe tests because of the rule "All page content must be contained by landmarks" It's unclear how the rules that were violated were identified in that PR, since it doesn't appear to add any logging of these. Here we've added the logging, and when running the tests in the Cypress UI, we can see that clicking on the error highlights the phase banner Again according to that PR, the violation is due to the banner being outside `<header>` and `<main>` elements. Apparently this is a [known issue][1] with the component, but as the region rule is not a requirement for WCAG, it should be okay to ignore for now [1]: alphagov/govuk-frontend#1604
Per UKGovernmentBEIS/regulated-professions-register@33be548 this fails axe tests because of the rule "All page content must be contained by landmarks" It's unclear how the rules that were violated were identified in that PR, since it doesn't appear to add any logging of these. Here we've added the logging, and when running the tests in the Cypress UI, we can see that clicking on the error highlights the phase banner Again according to that PR, the violation is due to the banner being outside `<header>` and `<main>` elements. Apparently this is a [known issue][1] with the component, but as the region rule is not a requirement for WCAG, it should be okay to ignore for now [1]: alphagov/govuk-frontend#1604
Per [this Regulated Professions Register commit][1] this fails axe tests because of the rule "All page content must be contained by landmarks" It's unclear how the rules that were violated were identified in that PR, since it doesn't appear to add any logging of these. Here we've added the logging, and when running the tests in the Cypress UI, we can see that clicking on the error highlights the phase banner Again according to that PR, the violation is due to the banner being outside `<header>` and `<main>` elements. Apparently this is a [known issue][2] with the component, but as the region rule is not a requirement for WCAG, it should be okay to ignore for now The approach here differs to that PR - it uses `cy.configureAxe`, which feels like a good place to put config, rather than passing a selector-based exception to `cy.checkA11y`. The exception is more specific: a selector and a specific rule, so that other rules are still checked. I'm not sure if this is possibly with `cy.checkA11y` [1]: UKGovernmentBEIS/regulated-professions-register@33be548 [2]: alphagov/govuk-frontend#1604 use configure instead
Accessibility: All page content should be contained by landmarks Reference: alphagov/govuk-frontend#1604 - re-enabled the AXE rule `region` - apply role attribute to phase banner and navigation, language switcher grids
Accessibility: All page content should be contained by landmarks Reference: alphagov/govuk-frontend#1604 - re-enabled the AXE rule `region` - apply role attribute to phase banner and navigation, language switcher grids
It's worth noting that Axe changed this from a "must" and flagging it as a WCAG fail to a "should" and now flags it as "best practice". See more on what Deque say about the issue from Axe. |
Hello 👋
We’ve noticed for a while that prototype kit and GOV.UK Frontend examples keep the “Back link” and “Phase banner” outside landmark regions.
i.e. They’re not in a
<header>
,<footer>
or<main>
.This throws up moderate issues in axe:
https://dequeuniversity.com/rules/axe/3.3/region?application=AxeChrome
Components not in
<main>
or<header>
This example shows showing both components outside regions:
http://govuk-frontend-review.herokuapp.com/full-page-examples/upload-your-photo
But the "Breadcrumbs" component is in
<main>
This example shows breadcrumbs inside a region:
http://govuk-frontend-review.herokuapp.com/full-page-examples/renew-driving-licence
Here's what we see in the axe sidebar when follow the examples (with “Back link” and “Phase banner” outside landmark regions):
It would be great to see the research behind when to use a landmark or not.
Should this be logged under #1280 (comment)?
Thanks
The text was updated successfully, but these errors were encountered: