Skip to content

Add breadcrumbs to all layouts #4710

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

Merged
merged 13 commits into from
May 30, 2025

Conversation

romaricpascal
Copy link
Member

@romaricpascal romaricpascal commented May 23, 2025

This PR build on top of the commits from #4698 and implements breadcrumbs on all pages, not just the pages part of the main navigation.

Like in #4698, it:

  • adds a new global Nunjucks function, getAncestorPages, which is used to generate the breadcrumb navigation.
  • removes the theme name from above page titles (except for those in patterns) as they're now duplicating information in the breadcrumbs.

In addition, it tweaks the spacing so that:

  • the first item in the sidebar aligns as closely as possible with the breadcrumbs, regardless of whether it's a heading or just a link
  • the padding around the side pane is set up on the pane itself rather than each item within the pane to make things easier to maintain
  • the headings sit a little more closely to the breadcrumbs. @mia-allers-gds @hazalarpalikli can you confirm if it looks OK design-wise?

Thoughts

After discussion with the squad, we'd prefer that the breadcrumbs are consistently present on every page rather than there on some page and sometimes missing (proposed by #4698). They're consistently positioned before the <main> element in the following source order:

  1. header
  2. service navigation
  3. phase banner
  4. optional side-navigation
  5. breadcrumbs
  6. page content

The side-navigation (4) may be absent in some of the pages, but:

  • the overall order remains the same
  • placing the breadcrumbs directly after the phase banner wouldn't make sense from a content hierarchy point of view (breadcrumbs being related to the page itself, following them with the side-navigation which is related to the section the page belongs to would be a bit odd)

Visually, they'll be sometimes in the side pane, sometimes on the left of the page container, which is no different from the content. They're also consitently aligned and spaced with the page header. This makes us confident that we're not doing anything untowards regarding Consistent Navigation WCAG criterion. @selfthinker, it'd be great to get your confirmation on it, though 😊

querkmachine and others added 5 commits May 16, 2025 08:54
Copy link

netlify bot commented May 23, 2025

You can preview this change here:

Name Link
🔨 Latest commit 58b2e8b
🔍 Latest deploy log https://app.netlify.com/projects/govuk-design-system-preview/deploys/68398cd2a957710008126bbb
😎 Deploy Preview https://deploy-preview-4710--govuk-design-system-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

The fact that we needed to apply padding to both `app-breadcrumbs`
and `app-content` was a sign that the padding was not applied
to the right elements and should instead be applied to their parent.

This means that whatever gets added inside `app-side-pane__content`,
the right amount of space will surround it.
Make the spacing on single page layouts consistent with the spacing
on the side page pages.

This includes moving the `app-width-container` outside of `<main>`
so that the horizontal spacing is more easily applied to both
the breadcrumbs and `<main>`
@romaricpascal romaricpascal force-pushed the spike-breadcrumbs-all-pages branch from bf52b3a to 535e8ea Compare May 27, 2025 14:02
@romaricpascal romaricpascal changed the base branch from spike-breadcrumbs to spike-flat-mobile-nav May 27, 2025 16:42
@selfthinker
Copy link
Contributor

Regarding Consistent Navigation, because the breadcrumbs are always in the same relative order, that is fine and passes WCAG.

If the breadcrumbs being visually in a different spot could be a problem, either specifically for accessibility or generally for usability, I don't know. I think it makes logical sense to be where it is. But I don't know if it could confuse users.

Moves the vertical padding in the white area where the content is displayed
to a `app-main-wrapper` class. This ensures the spacing is consistent
between `layout-pane` and `layout-single-page` so the breadcrumbs end up
at the same level on all pages.

The padding is applied as its own class in a similar fashion that we have
`govuk-main-wrapper` in GOV.UK Frontend, but is made to match the spacing
designed for the `layout-pane`.

It also fixes the bottom padding having disappeared after the content
on the pages using `layout-single-page`.
Ports the code of #4368 for the rendering
of the subnavigation of each item of the Service Nav in a `<template>` element which will then
be used by JavaScript.

Co-Authored-By: [email protected]
Instantiate one component per section of the navigation to easily group the toggle button and subnavigation it reveals

Co-Authored-By: [email protected]
As the mobile navigation is only available when JavaScript is loaded,
and gets shown/hidden through the `hidden` HTML attribute, styles do not
need to be gated by a media query.

As the links live withing the Service Navigation, we can reuse its `govuk-service-navigation__link` styles for the interactive elements (subnavigation toggle and links)

Co-Authored-By: [email protected]
Include the fix that makes `<template>` tags correctly handled by `element-permitted-content`.

https://gitlab.com/html-validate/html-validate/-/issues/305
@36degrees 36degrees self-requested a review May 30, 2025 10:37
@romaricpascal romaricpascal merged commit c2d2682 into spike-flat-mobile-nav May 30, 2025
13 checks passed
@romaricpascal romaricpascal deleted the spike-breadcrumbs-all-pages branch May 30, 2025 10:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants