[v5] Fix VoiceOver access to hidden Service Navigation menu button

packages/govuk-frontend/src/govuk/components/service-navigation/service-navigation.mjs

@@ -49,12 +49,13 @@ export class ServiceNavigation extends Component {
       return this
     }
 
-    const menuId = $menuButton.getAttribute('aria-controls')
+    const menuId = $menuButton.getAttribute('data-aria-controls')
+
     if (!menuId) {
       throw new ElementError({
         component: ServiceNavigation,
         identifier:
-          'Navigation button (`<button class="govuk-js-service-navigation-toggle">`) attribute (`aria-controls`)'
+          'Navigation button (`<button class="govuk-js-service-navigation-toggle">`) attribute (`data-aria-controls`)'
       })
     }
 
@@ -69,6 +70,7 @@ export class ServiceNavigation extends Component {
 
     this.$menu = $menu
     this.$menuButton = $menuButton
+    this.$menuButton.setAttribute('aria-controls', this.$menu.id)
 
     this.setupResponsiveChecks()
 
@@ -125,9 +127,9 @@ export class ServiceNavigation extends Component {
 
     if (this.mql.matches) {
       this.$menu.removeAttribute('hidden')
-      this.$menuButton.setAttribute('hidden', '')
+      setAttributes(this.$menuButton, attributesForHidingButton)
     } else {
-      this.$menuButton.removeAttribute('hidden')
+      removeAttributes(this.$menuButton, Object.keys(attributesForHidingButton))
       this.$menuButton.setAttribute('aria-expanded', this.menuIsOpen.toString())
 
       if (this.menuIsOpen) {
@@ -156,3 +158,41 @@ export class ServiceNavigation extends Component {
    */
   static moduleName = 'govuk-service-navigation'
 }
+
+/**
+ * Collection of attributes that needs setting on a `<button>`
+ * to fully hide it, both visually and from screen-readers,
+ * and prevent its activation while hidden
+ */
+const attributesForHidingButton = {
+  hidden: '',
+  // Prevent activating the button with JavaScript APIs while hidden
+  disabled: '',
+  // Fix button still appearing in VoiceOver's form control's menu despite being hidden
+  // https://bugs.webkit.org/show_bug.cgi?id=300899
+  'aria-hidden': 'true'
+}
+
+/**
+ * Sets a group of attributes on the given element
+ *
+ * @param {Element} $element - The element to set the attribute on
+ * @param {{[attributeName: string]: string}} attributes - The attributes to set
+ */
+function setAttributes($element, attributes) {
+  for (const attributeName in attributes) {
+    $element.setAttribute(attributeName, attributes[attributeName])
+  }
+}
+
+/**
+ * Removes a list of attributes from the given element
+ *
+ * @param {Element} $element - The element to remove the attributes from
+ * @param {string[]} attributeNames - The names of the attributes to remove
+ */
+function removeAttributes($element, attributeNames) {
+  for (const attributeName of attributeNames) {
+    $element.removeAttribute(attributeName)
+  }
+}

packages/govuk-frontend/src/govuk/components/service-navigation/service-navigation.puppeteer.test.js

@@ -109,7 +109,9 @@ describe('/components/service-navigation', () => {
         await expect(
           render(page, 'service-navigation', examples.default, {
             beforeInitialisation($root, { selector }) {
-              $root.querySelector(selector).removeAttribute('aria-controls')
+              $root
+                .querySelector(selector)
+                .removeAttribute('data-aria-controls')
             },
             context: {
               selector: toggleButtonSelector
@@ -119,7 +121,7 @@ describe('/components/service-navigation', () => {
           cause: {
             name: 'ElementError',
             message:
-              'govuk-service-navigation: Navigation button (`<button class="govuk-js-service-navigation-toggle">`) attribute (`aria-controls`) not found'
+              'govuk-service-navigation: Navigation button (`<button class="govuk-js-service-navigation-toggle">`) attribute (`data-aria-controls`) not found'
           }
         })
       })
@@ -167,6 +169,15 @@ describe('/components/service-navigation', () => {
         expect(buttonHiddenAttribute).toBeFalsy()
       })
 
+      it('does not add an `aria-hidden` attribute to the button for Voice Over', async () => {
+        const buttonHiddenAttribute = await page.$eval(
+          toggleButtonSelector,
+          (el) => el.hasAttribute('aria-hidden')
+        )
+
+        expect(buttonHiddenAttribute).toBeFalsy()
+      })
+
       it('renders the toggle button with `aria-expanded` set to false', async () => {
         const toggleExpandedAttribute = await page.$eval(
           toggleButtonSelector,
@@ -175,6 +186,15 @@ describe('/components/service-navigation', () => {
 
         expect(toggleExpandedAttribute).toBe('false')
       })
+
+      it('adds the `aria-controls` attribute to the toggle', async () => {
+        const toggleExpandedAttribute = await page.$eval(
+          toggleButtonSelector,
+          (el) => el.getAttribute('aria-controls')
+        )
+
+        expect(toggleExpandedAttribute).toBe('navigation')
+      })
     })
 
     describe('when toggle button is clicked once', () => {
@@ -229,5 +249,97 @@ describe('/components/service-navigation', () => {
         expect(toggleExpandedAttribute).toBe('false')
       })
     })
+
+    describe('on wide viewports', () => {
+      beforeAll(async () => {
+        // First let's reset the viewport to a desktop one
+        await page.setViewport({
+          width: 1280,
+          height: 720
+        })
+      })
+
+      afterAll(async () => {
+        // After tests have run reset teh viewport to a phone
+        await page.emulate(iPhone)
+      })
+
+      describe('on page load', () => {
+        beforeAll(async () => {
+          await render(page, 'service-navigation', examples.default)
+        })
+
+        it('shows the navigation', async () => {
+          const navigationHiddenAttribute = await page.$eval(
+            navigationSelector,
+            (el) => el.hasAttribute('hidden')
+          )
+
+          expect(navigationHiddenAttribute).toBeFalsy()
+        })
+
+        it('keeps the toggle button hidden', async () => {
+          const buttonHiddenAttribute = await page.$eval(
+            toggleButtonSelector,
+            (el) => el.hasAttribute('hidden')
+          )
+
+          expect(buttonHiddenAttribute).toBeTruthy()
+        })
+
+        it('adds an `aria-hidden` attribute for Voice Over', async () => {
+          const buttonHiddenAttribute = await page.$eval(
+            toggleButtonSelector,
+            (el) => el.getAttribute('aria-hidden')
+          )
+
+          expect(buttonHiddenAttribute).toBe('true')
+        })
+      })
+
+      describe('when page is resized to a narrow viewport', () => {
+        beforeAll(async () => {
+          await render(page, 'service-navigation', examples.default)
+
+          // Once the page is loaded resize the viewport to a narrow one
+          // Only set the width and height to avoid the page getting wiped
+          await page.setViewport({
+            width: iPhone.viewport.width,
+            height: iPhone.viewport.height
+          })
+
+          // Wait that the page got updated after the resize,
+          // as sometimes the tests run too early
+          await page.waitForSelector(`${navigationSelector}[hidden]`)
+        })
+
+        it('hides the navigation', async () => {
+          const navigationHiddenAttribute = await page.$eval(
+            `${navigationSelector}[hidden]`,
+            (el) => el.hasAttribute('hidden')
+          )
+
+          expect(navigationHiddenAttribute).toBeTruthy()
+        })
+
+        it('reveals the toggle button', async () => {
+          const buttonHiddenAttribute = await page.$eval(
+            toggleButtonSelector,
+            (el) => el.hasAttribute('hidden')
+          )
+
+          expect(buttonHiddenAttribute).toBeFalsy()
+        })
+
+        it('removes the `aria-hidden` attribute on the toggle', async () => {
+          const buttonHiddenAttribute = await page.$eval(
+            toggleButtonSelector,
+            (el) => el.hasAttribute('aria-hidden')
+          )
+
+          expect(buttonHiddenAttribute).toBeFalsy()
+        })
+      })
+    })
   })
 })

packages/govuk-frontend/src/govuk/components/service-navigation/template.njk

@@ -37,7 +37,7 @@ data-module="govuk-service-navigation"
       {% if navigationItems | length or params.slots.navigationStart or params.slots.navigationEnd %}
         <nav aria-label="{{ params.navigationLabel | default(menuButtonText, true) }}" class="govuk-service-navigation__wrapper {%- if params.navigationClasses %} {{ params.navigationClasses }}{% endif %}">
           {% if collapseNavigationOnMobile %}
-          <button type="button" class="govuk-service-navigation__toggle govuk-js-service-navigation-toggle" aria-controls="{{ navigationId }}" {%- if params.menuButtonLabel and params.menuButtonLabel != menuButtonText %} aria-label="{{ params.menuButtonLabel }}"{% endif %} hidden>
+          <button type="button" class="govuk-service-navigation__toggle govuk-js-service-navigation-toggle" data-aria-controls="{{ navigationId }}" {%- if params.menuButtonLabel and params.menuButtonLabel != menuButtonText %} aria-label="{{ params.menuButtonLabel }}"{% endif %} hidden>
             {{ menuButtonText }}
           </button>
           {% endif %}

packages/govuk-frontend/src/govuk/components/service-navigation/template.test.js

@@ -85,7 +85,7 @@ describe('Service Navigation', () => {
       const navId = $nav.attr('id')
 
       expect(navId).toBe('navigation')
-      expect($navToggle.attr('aria-controls')).toBe(navId)
+      expect($navToggle.attr('data-aria-controls')).toBe(navId)
     })
 
     it('omits empty items from the navigation', () => {
@@ -158,7 +158,7 @@ describe('Service Navigation', () => {
         const navId = $nav.attr('id')
 
         expect(navId).toBe('main-nav')
-        expect($navToggle.attr('aria-controls')).toBe(navId)
+        expect($navToggle.attr('data-aria-controls')).toBe(navId)
       })
     })