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

Add Service Navigation component, add new Mobile Navigation component #4368

Draft
wants to merge 9 commits into
base: main
Choose a base branch
from
24 changes: 12 additions & 12 deletions config/navigation.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,28 +5,28 @@
*/
const config = [
{
label: 'Get started',
url: 'get-started'
text: 'Get started',
href: 'get-started'
},
{
label: 'Styles',
url: 'styles'
text: 'Styles',
href: 'styles'
},
{
label: 'Components',
url: 'components'
text: 'Components',
href: 'components'
},
{
label: 'Patterns',
url: 'patterns'
text: 'Patterns',
href: 'patterns'
},
{
label: 'Community',
url: 'community'
text: 'Community',
href: 'community'
},
{
label: 'Accessibility',
url: 'accessibility'
text: 'Accessibility',
href: 'accessibility'
}
]

Expand Down
8 changes: 5 additions & 3 deletions lib/navigation.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ module.exports = (config) => (files, metalsmith, done) => {
for (const item of items) {
// Match navigation item child directories
// (for example, ['components/breadcrumbs/index.html', 'components/checkboxes/index.html', ...])
const itemPaths = metalsmith.match(`${item.url}/*/index.html`, paths)
const itemPaths = metalsmith.match(`${item.href}/*/index.html`, paths)

// No sub items required for this path
if (!itemPaths.length) {
Expand All @@ -47,8 +47,8 @@ module.exports = (config) => (files, metalsmith, done) => {

// Add subitem to navigation
item.items.push({
url: dirname(itemPath),
label: frontmatter.title,
href: `/${dirname(itemPath)}`,
text: frontmatter.title,
order: frontmatter.order,
theme: frontmatter.theme,

Expand All @@ -65,6 +65,8 @@ module.exports = (config) => (files, metalsmith, done) => {

// Sort navigation sub items using 'order' (optional)
item.items?.sort((a, b) => compare(a.order, b.order))

item.href = `/${item.href}`
}

// Add navigation to global variables
Expand Down
15 changes: 11 additions & 4 deletions src/javascripts/application.mjs
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
/* eslint-disable no-new */

import { createAll, Button, NotificationBanner, SkipLink } from 'govuk-frontend'
import {
createAll,
Button,
NotificationBanner,
ServiceNavigation,
SkipLink
} from 'govuk-frontend'

import { loadAnalytics } from './components/analytics.mjs'
import BackToTop from './components/back-to-top.mjs'
Expand All @@ -14,7 +20,7 @@ import CookiesPage from './components/cookies-page.mjs'
import Copy from './components/copy.mjs'
import EmbedCard from './components/embed-card.mjs'
import ExampleFrame from './components/example-frame.mjs'
import Navigation from './components/navigation.mjs'
import MobileNavigation from './components/mobile-navigation.mjs'
import OptionsTable from './components/options-table.mjs'
import ScrollContainer from './components/scroll-container.mjs'
import Search from './components/search.mjs'
Expand All @@ -23,6 +29,7 @@ import AppTabs from './components/tabs.mjs'
// Initialise GOV.UK Frontend
createAll(Button)
createAll(NotificationBanner)
createAll(ServiceNavigation)
createAll(SkipLink)

// Cookies and analytics
Expand All @@ -47,8 +54,8 @@ createAll(AppTabs)
createAll(Copy)
new OptionsTable()

// Initialise mobile navigation
createAll(Navigation)
// Initialise mobile navigation (again)
createAll(MobileNavigation)

// Initialise scrollable container handling
createAll(ScrollContainer)
Expand Down
111 changes: 111 additions & 0 deletions src/javascripts/components/mobile-navigation.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
import { Component } from 'govuk-frontend'

/**
* Mobile Navigation enhancement for Service Navigation component
*/
class MobileNavigation extends Component {
static moduleName = 'app-mobile-navigation'

/**
* @param {Element} $root - HTML element
*/
constructor($root) {
super($root)

this.templates = this.$root.querySelectorAll(
'.app-mobile-navigation__template'
)
this.links = this.$root.querySelectorAll('a')
Copy link
Member

Choose a reason for hiding this comment

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

issue This has a risk to collect elements more broadly than we intend, if one day we add some content to the navigation through slots, for example. Would be good to limit this to links from the actual navigation using their class from the Service Navigation (unless we can set up our own .js- prefixed class).


Array.from(this.templates).forEach((template) => {
const templateClone = template.content.cloneNode(true)
let link

if (template.parentNode.tagName === 'A') {
link = template.parentNode
link.removeChild(template)
} else {
link = template.parentNode.parentNode
template.parentNode.removeChild(template)
}

const button = document.createElement('button')
button.classList.add('govuk-service-navigation__link')
button.classList.add('app-mobile-navigation__toggle-button')
button.setAttribute(
'aria-expanded',
String(
link.parentNode.classList.contains(
'govuk-service-navigation__item--active'
)
)
)
button.textContent = link.textContent

link.insertAdjacentElement('afterend', templateClone.firstElementChild)
link.insertAdjacentElement('afterend', button)
})

const currentLink = Array.from(
this.$root.querySelectorAll(
`.app-navigation__subnav-item a[href="${window.location.pathname.slice(0, -1)}"]`
)
).pop()

if (currentLink) {
currentLink.classList.add('app-mobile-navigation__link--active')
}

// A global const for storing a matchMedia instance which we'll use to detect when a screen size change happens
// Set the matchMedia to the govuk-frontend tablet breakpoint

const breakPoint = getComputedStyle(
document.documentElement
).getPropertyValue('--govuk-frontend-breakpoint-tablet')

this.mql = window.matchMedia(`(min-width: ${breakPoint})`)

// MediaQueryList.addEventListener isn't supported by Safari < 14 so we need
// to be able to fall back to the deprecated MediaQueryList.addListener
if ('addEventListener' in this.mql) {
this.mql.addEventListener('change', () => this.setHiddenStates())
} else {
// @ts-expect-error Property 'addListener' does not exist
this.mql.addListener(() => this.setHiddenStates())
}

this.setHiddenStates()
this.setEventListener()
}

/**
* Set up event delegation for button clicks
*/
setEventListener() {
this.$root.addEventListener(
'click',
(e) => {
if (e.target.tagName === 'BUTTON') {
e.target.setAttribute(
'aria-expanded',
!(e.target.getAttribute('aria-expanded') === 'true')
)
}
},
{ bubbles: true }
Comment on lines +94 to +95
Copy link
Member

Choose a reason for hiding this comment

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

issue bubbles is not one of the options of addEventListener

Suggested change
},
{ bubbles: true }
}

)
}

/**
* Hide links if viewport is below tablet
*/
setHiddenStates() {
if (!this.mql.matches) {
this.links.forEach((a) => a.setAttribute('hidden', ''))
} else {
this.links.forEach((a) => a.removeAttribute('hidden'))
}
}
}

export default MobileNavigation
69 changes: 69 additions & 0 deletions src/stylesheets/components/_mobile-navigation.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
.app-mobile-navigation__theme {
@include govuk-typography-common;
position: relative; // this is to get around the artificial click area generated by the :after of the parent button
margin: 0;
padding: 0;
color: govuk-colour("dark-grey");
// Font is defined as a hard 19px so
// it does not re-size on mobile viewport
font-size: 19px;
font-size: govuk-px-to-rem(19px);
font-weight: normal;
}

.app-mobile-navigation .govuk-service-navigation__item--active {
@include govuk-media-query($until: tablet) {
border-color: transparent;
}
}

.app-mobile-navigation__link--active {
@include govuk-media-query($until: tablet) {
border-left: 5px solid rgb(26.1, 100.8, 165.6);
}
}

.app-mobile-navigation__toggle-button {
@include govuk-media-query($from: tablet) {
display: none;
}
position: relative;
padding: 0;
border: 0;

background: none;
font-size: inherit;
}

.app-mobile-navigation__toggle-button::after {
content: "";
box-sizing: border-box;
display: block;
position: absolute;
right: govuk-px-to-rem(-14px);
bottom: govuk-px-to-rem(10px);
width: govuk-px-to-rem(6px);
height: govuk-px-to-rem(6px);
border-top: govuk-px-to-rem(2px) solid;
border-right: govuk-px-to-rem(2px) solid;
Comment on lines +47 to +48
Copy link
Member

Choose a reason for hiding this comment

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

Solid way to have the chevron visible for people using high contrast mode 🙌🏻

}

.app-mobile-navigation__toggle-button[aria-expanded="false"]::after {
transform: rotate(135deg);
}

.app-mobile-navigation__toggle-button[aria-expanded="false"] ~ .app-navigation__list {
display: none;
}
Comment on lines +55 to +57
Copy link
Member

Choose a reason for hiding this comment

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

We may be able to let that selector go when we re-implement the current navigation feature for using hidden to hide elements when CSS fails to load.


.app-mobile-navigation__toggle-button[aria-expanded="true"]::after {
transform: rotate(-45deg);
}
Comment on lines +59 to +61
Copy link
Member

Choose a reason for hiding this comment

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

suggestion This can likely be the default state of the button and save a selector.


.app-mobile-navigation__toggle-button[aria-expanded="true"] ~ .app-navigation__list {
@include govuk-media-query($until: tablet) {
display: block;
}

display: none;
}
Comment on lines +63 to +69
Copy link
Member

Choose a reason for hiding this comment

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

suggestion Hiding the navigation list and toggle would gain at being regrouped in a single block of code and a single media query for easier maintainability rather than split in their own selector, in the lines of:

Suggested change
.app-mobile-navigation__toggle-button[aria-expanded="true"] ~ .app-navigation__list {
@include govuk-media-query($until: tablet) {
display: block;
}
display: none;
}
@include govuk-media-query($from: tablet) {
.app-mobile-navigation__toggle-button,
.app-navigation__list {
display: none;
}
}

This would allow the file to describe first how the different parts look, then when they're visible or not. That media query may also not be necessary once the JavaScript handles using hidden to ensure elements are not visible if CSS did not load.

10 changes: 10 additions & 0 deletions src/stylesheets/components/_navigation.scss
Original file line number Diff line number Diff line change
@@ -1,5 +1,15 @@
$navigation-height: 50px;

.app-navigation__list-toggle {
@include govuk-media-query($from: tablet) {
display: none;
}
}

.js-app-navigation__list--hidden {
display: none;
}

.app-navigation {
border-bottom: 1px solid $govuk-border-colour;
background-color: $app-light-grey;
Expand Down
13 changes: 12 additions & 1 deletion src/stylesheets/main.scss
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ $govuk-new-typography-scale: true;
@import "govuk/components/panel";
@import "govuk/components/phase-banner";
@import "govuk/components/radios";
@import "govuk/components/service-navigation";
@import "govuk/components/skip-link";
@import "govuk/components/summary-list";
@import "govuk/components/table";
Expand Down Expand Up @@ -58,6 +59,7 @@ $app-code-color: #d13118;
@import "components/highlight";
@import "components/image-card";
@import "components/masthead";
@import "components/mobile-navigation";
@import "components/navigation";
@import "components/options";
@import "components/page-navigation";
Expand All @@ -68,8 +70,17 @@ $app-code-color: #d13118;
@import "components/tabs";
@import "components/table";

.app-nav-subitems-mobile {
@include govuk-media-query($from: tablet) {
display: none;
}

background-color: govuk-colour("mid-grey");
}

// We don't change the global width container width so that examples are the current width.
.app-width-container {
.app-width-container,
.govuk-service-navigation .govuk-width-container {
Comment on lines +82 to +83
Copy link
Member

Choose a reason for hiding this comment

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

This may be an indication that we may want to update the CSS variable in this stylesheet and add a custom width container to the examples 🤔 This is something we can explore in a future PR, though 😊

@include govuk-width-container(1100px);
}

Expand Down
20 changes: 20 additions & 0 deletions views/partials/_mobile-navigation.njk
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
{% macro mobileNavigation(params) %}
{% if params.items %}
<ul class="app-navigation__list">
{% for theme, items in params.items | groupby("theme") %}
{% if theme != 'undefined' %}
<li class="app-navigation__subnav-item">
<h3 class="app-mobile-navigation__theme" >{{ theme }}</h3>
</li>
{% endif %}
{% for item in items %}
<li class="app-navigation__subnav-item">
<a class="govuk-link govuk-link--no-visited-state govuk-link--no-underline" href="{{ item.href }}">
{{ item.text }}
</a>
</li>
{% endfor %}
{% endfor %}
</ul>
{% endif %}
{% endmacro %}
Loading
Loading