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

PLATUI-1550: fix hmrc-header mobile dropdown behaviour #207

Merged
merged 1 commit into from
Feb 4, 2022

Conversation

oscarduignan
Copy link
Contributor

This has been broken for any service using hmrc-frontend with a version of govuk-frontend after version 3 where they changed the code that initialises the behaviour of govuk-header (which hmrc-header depends on) to require data-module attributes to use a "govuk-" prefix

Fix was to add the prefix to the data-module attribute. I've done some searching and it seems like there are possibly only two services using navigation items with the hmrcHeader (at least via play-frontend-hmrc) so it might be that's why this bug managed to go undetected for a while, because few hmrc services were using the navigation items in the hmrcHeader.

I've added a visual regression test to catch any issues with this in the future.

Did a search for other uses of data-module attributes that could have been missed and nothing came up:

madetech@oscars-laptop hmrc-frontend % rg 'data-module="'
src/all.js

8:  const $AccountMenuSelector = '[data-module="hmrc-account-menu"]';
18:  const $UserResearchBanner = document.querySelector('[data-module="hmrc-user-research-banner"]');
23:  const $CharacterCounts = document.querySelectorAll('[data-module="hmrc-character-count"]');

src/components/header/template.njk
7:  <div class="govuk-header hmrc-header {{ params.classes if params.classes }} {{ 'hmrc-header--with-additional-navigation' if params.signOutHref or params.languageToggle }}" data-module="govuk-header"

src/components/account-menu/template.test.js
20:      const $nav = $('[data-module="hmrc-account-menu"]');
39:      const $nav = $('[data-module="hmrc-account-menu"]');
60:      const $nav = $('[data-module="hmrc-account-menu"]');

src/components/account-menu/template.njk
5:<nav id="secondary-nav" class="hmrc-account-menu" aria-label="Account" data-module="hmrc-account-menu">

src/components/character-count/template.njk
5:<div class="hmrc-character-count" data-module="hmrc-character-count" data-language="{{ language }}"

src/components/user-research-banner/template.njk
1:<div class="hmrc-user-research-banner" data-module="hmrc-user-research-banner">

@oscarduignan oscarduignan merged commit 090da11 into main Feb 4, 2022
@oscarduignan oscarduignan deleted the PLATUI-1550-hmrc-header-mobile-dropdown branch February 4, 2022 15:26
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.

2 participants