Fix header navigation item allocation on resize

[colinrotherham]

Description

This PR fixes #1226 and adds 2x commits from #1058 as follows:

1. Wrap header navigation items when JavaScript is not enabled
2. Add space between header navigation items using padding

Most helpful was 2) where CSS gap is removed

Checklist

• Tested against our testing policy (Resolution, Browser & Accessibility)
• Follows our coding standards and style guide
• CHANGELOG entry

[paulrobertlloyd]

This all looks good to me, some nice tidy up within the JS for this component too.

Take it this PR also ticks off updating the mocked navigation HTML used in the component test to actually represent the markup now being used as well?

[MatMoore]

I skipped over some of the CSS details, but the javascript changes look good to me. Extracting methods for showing/hiding the menu bar is a nice improvement 👍🏻

Have a couple of minor suggestions inline, but the approach seems good.

[colinrotherham]

Take it this PR also ticks off updating the mocked navigation HTML used in the component test to actually represent the markup now being used as well?

@paulrobertlloyd Yeah it's the same HTML now, did I see that somewhere?

I'd like to import Nunjucks in future and render the actual template so it's always correct 😮

[colinrotherham]

Thanks @paulrobertlloyd @MatMoore I've pushed up some feedback, ready for review again

[MatMoore]

👍🏻

[anandamaryon1]

Not sure whether it's really an issue, but in testing this I observe that the number of items shown varies between first load and any subsequent resizing at 768px wide.

For example, if I resize from wider down to 768, I see the More button:

But if I refresh / load up on 768 wide, I see all the nav items at once, without the More button:

Not really a usability issue but maybe a symptom of some breakpoints not quite aligning?

[colinrotherham]

Not sure whether it's really an issue, but in testing this I observe that the number of items shown varies between first load and any subsequent resizing at 768px wide.

Hey @anandamaryon1 thanks for spotting another bug 😆

The loop in this.calculateBreakPoints() had these lines:

if (item.element.parentElement === this.mobileMenu) {
  return
}

Which means:

• On the 1st run, we move overflow items to the mobile menu
• On the 2nd run, we skip offset calculations for overflow items

But uniquely at 769px we adjust the spacing, so if we hadn't skipped we'd realise there was enough room

I've pushed up a fix

Other bug fixes and removals

I've also spotted a few other things so pushed:

• Remove unused orientation change handler
• Remove unused js-show from README example
• Remove unused this.mobileMenuCloseButton
• Remove unnecessary focus on menu close
• Fix escape key removeEventListener() and others
• Fix menu button arrow colour on focus

With the tests updated to check the expected item counts at each width