govuk-frontend icon indicating copy to clipboard operation
govuk-frontend copied to clipboard

Axe issue: "All page content must be contained by landmarks"

Open colinrotherham opened this issue 5 years ago • 17 comments

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

Components outside  main

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

Breadcrumbs component inside  main

Here's what we see in the axe sidebar when follow the examples (with “Back link” and “Phase banner” outside landmark regions):

Axe report

It would be great to see the research behind when to use a landmark or not.

Should this be logged under https://github.com/alphagov/govuk-frontend/issues/1280#issuecomment-509588851?

Thanks

colinrotherham avatar Oct 08 '19 12:10 colinrotherham

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:

  • the skip link is less useful if it does not skip the phase banner, back links and breadcrumbs
  • too many navigation landmarks on GOV.UK have been reported to be an issue so we intentionally have removed some navigation landmarks in order to make it clear what the main navigation is.

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.

NickColley avatar Oct 08 '19 13:10 NickColley

With the help from @36degrees @selfthinker I've pulled out some of the historical stuff:

It turns out some of our nav tags were redundant. They either added no extra meaning to the links they wrapped or were unnecessary as user agents got the same information from the links' context instead.

Among those that were needed, the navigational theme that grouped their contents was sometimes unclear.

The above came out of an accessibility review by Léonie Watson. In it she explained that having too many nav tags devalued their use for screen reader users. It was suggested that when multiple ones were necessary we could help those users find the links they needed by identifying them properly.

NickColley avatar Oct 08 '19 13:10 NickColley

@nickcolley This is exactly the sort of thing I was after! Thank you for the detective work 🕵

colinrotherham avatar Oct 08 '19 14:10 colinrotherham

I've added this to https://github.com/alphagov/govuk-frontend/issues/1280#issuecomment-509588851 as it can also seen on the GOV.UK Frontend Template. Thanks again for raising this @colinrotherham

hannalaakso avatar Oct 18 '19 12:10 hannalaakso

Looks like this has been resolved and added to documentation for people to find in the future so will close this one out, thanks :)

NickColley avatar Jan 28 '20 13:01 NickColley

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 <main> element, we have recently discussed whether it would make sense instead to extend the <header> to include them alongside the header component.

36degrees avatar Sep 14 '20 14:09 36degrees

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.

36degrees avatar Sep 14 '20 14:09 36degrees

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 avatar Oct 13 '20 15:10 robertparkinson

@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?

36degrees avatar Oct 13 '20 16:10 36degrees

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. https://github.com/alphagov/govuk-frontend/pull/1984

robertparkinson avatar Oct 14 '20 10:10 robertparkinson

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.

matthewmascord avatar Nov 19 '20 16:11 matthewmascord

With the help from @36degrees @selfthinker I've pulled out some of the historical stuff:

It turns out some of our nav tags were redundant. They either added no extra meaning to the links they wrapped or were unnecessary as user agents got the same information from the links' context instead. Among those that were needed, the navigational theme that grouped their contents was sometimes unclear. The above came out of an accessibility review by Léonie Watson. In it she explained that having too many nav tags devalued their use for screen reader users. It was suggested that when multiple ones were necessary we could help those users find the links they needed by identifying them properly.

Yes. It Is understandable the rationale of Leónie Watson, but a div title="breadcrumb" and a aria-current="page" at the last link (if I'm here why I need a link?) of the sequence is a good improve to screenreader users understand the structure of this element.

jorgeponto avatar Mar 01 '21 18:03 jorgeponto

skip-link still throws the same warning in Axe. i can't see any reference to this being tackled so far. any plans or have I missed something? Screenshot 2021-03-23 at 19 18 53

brunswickcomputing avatar Mar 23 '21 19:03 brunswickcomputing

We should consider the skip link component as part of this issue, but worth noting that Deque themselves say:

It is best practice to contain all content excepting skip links, within distinct regions such as the header, nav, main, and footer.

36degrees avatar May 20 '21 08:05 36degrees

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.

vanitabarrett avatar Sep 01 '21 16:09 vanitabarrett

We revisited this very briefly in a discussion amongst some of the devs today. The takeaways:

  • Whilst this is something worth fixing for the sake of suppressing warnings and saving dev teams time, we are skeptical that it's an actual issue for users so it is still low priority
  • We're going to run this past our accessibility specialist to verify that skepticism
  • This could be solved by a long term idea to not couple the header component to the header landmark which itself could potentially go into v5

owenatgov avatar Sep 26 '22 15:09 owenatgov

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.

selfthinker avatar Sep 11 '24 10:09 selfthinker