[refs #398] Move JavaScript variables to be locally scoped

Globally scoped variables were causing issues with JavaScript frameworks such as Angular, so they have been moved to be locally scoped within functions. This could be improved as we have to duplicate variables.
nhsuk · Feb 22, 2019 · 9bdd950 · 9bdd950
1 parent bc23d52
commit 9bdd950
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 17 deletions.
diff --git a/packages/components/feedback-banner/feedback-banner.js b/packages/components/feedback-banner/feedback-banner.js
@@ -11,17 +11,15 @@
  * feedbackBanner.init(3000);
  */
 
-const banner = document.querySelector('#nhsuk-feedback-banner');
-const bannerCloseButton = document.querySelector('#nhsuk-feedback-banner-close');
-const footer = document.querySelector('#nhsuk-footer');
-
 function showBanner() {
+  const banner = document.querySelector('#nhsuk-feedback-banner');
   if (banner) {
     banner.style.display = 'block';
   }
 }
 
 function hideBanner() {
+  const banner = document.querySelector('#nhsuk-feedback-banner');
   if (banner) {
     banner.style.display = 'none';
   }
@@ -41,6 +39,8 @@ function isScrolledIntoView(el) {
 function unstickBanner() {
   let didScroll = false;
   let timer = false;
+  const footer = document.querySelector('#nhsuk-footer');
+  const banner = document.querySelector('#nhsuk-feedback-banner');
   // set a timer when scrolling, so as not to be constantly calling the
   // isScrolledIntoView function and spiking CPU, to check when the footer
   // comes in to view, to make the banner not sticky but position it in the
@@ -66,12 +66,14 @@ function handleBannerDisplay(delay) {
 }
 
 function handleBannerClose() {
+  const bannerCloseButton = document.querySelector('#nhsuk-feedback-banner-close');
   if (bannerCloseButton) {
     bannerCloseButton.addEventListener('click', hideBanner);
   }
 }
 
 function handleBannerSticky() {
+  const banner = document.querySelector('#nhsuk-feedback-banner');
   if (banner) {
     window.addEventListener('scroll', unstickBanner);
   }

diff --git a/packages/components/header/header.js b/packages/components/header/header.js
@@ -21,25 +21,29 @@ function toggleClass(ele, class1) {
 }
 
 /* Search toggle */
-
-const searchToggleButton = document.querySelector('#toggle-search');
-const searchClose = document.querySelector('#close-search');
-const searchContainer = document.querySelector('#wrap-search');
-const menuSearchContainer = document.querySelector('#content-header');
-
 function toggleSearch(e) {
+  const searchToggleButton = document.querySelector('#toggle-search');
+  const searchContainer = document.querySelector('#wrap-search');
+  const menuSearchContainer = document.querySelector('#content-header');
+
   e.preventDefault();
+  e.stopImmediatePropagation();
+
   if (searchToggleButton.hasAttribute('aria-expanded')) {
     searchToggleButton.removeAttribute('aria-expanded');
   } else {
     searchToggleButton.setAttribute('aria-expanded', 'true');
   }
+
   toggleClass(searchToggleButton, 'is-active');
   toggleClass(searchContainer, 'js-show');
   toggleClass(menuSearchContainer, 'js-show');
 }
 
 function handleSearchToggle() {
+  const searchToggleButton = document.querySelector('#toggle-search');
+  const searchClose = document.querySelector('#close-search');
+
   if (searchToggleButton) {
     searchToggleButton.addEventListener('click', toggleSearch);
   }
@@ -50,22 +54,27 @@ function handleSearchToggle() {
 
 /* Menu toggle */
 
-const menuToggleButton = document.querySelector('#toggle-menu');
-const menuClose = document.querySelector('#close-menu');
-const nav = document.querySelector('#header-navigation');
-
 function toggleMenu(e) {
+  const menuToggleButton = document.querySelector('#toggle-menu');
+  const nav = document.querySelector('#header-navigation');
+
   e.preventDefault();
+  e.stopImmediatePropagation();
+
   if (menuToggleButton.hasAttribute('aria-expanded')) {
     menuToggleButton.removeAttribute('aria-expanded');
   } else {
     menuToggleButton.setAttribute('aria-expanded', 'true');
   }
+
   toggleClass(menuToggleButton, 'is-active');
   toggleClass(nav, 'js-show');
 }
 
 function handleMenuToggle() {
+  const menuToggleButton = document.querySelector('#toggle-menu');
+  const menuClose = document.querySelector('#close-menu');
+
   if (menuToggleButton) {
     menuToggleButton.addEventListener('click', toggleMenu);
   }

diff --git a/packages/components/skip-link/skip-link.js b/packages/components/skip-link/skip-link.js
@@ -8,19 +8,20 @@
  * then removes it when focus is off it.
  */
 
-const skipLinkElement = document.querySelector('.nhsuk-skip-link');
-const headingElement = document.getElementsByTagName('H1')[0];
-
 function addFocus() {
+  const headingElement = document.getElementsByTagName('H1')[0];
   headingElement.setAttribute('tabIndex', '-1');
   headingElement.focus();
 }
 
 function removeFocus() {
+  const headingElement = document.getElementsByTagName('H1')[0];
   headingElement.removeAttribute('tabIndex');
 }
 
 function handleSkipLink() {
+  const skipLinkElement = document.querySelector('.nhsuk-skip-link');
+  const headingElement = document.getElementsByTagName('H1')[0];
   if (skipLinkElement && headingElement) {
     skipLinkElement.addEventListener('click', e => { /* eslint-disable-line arrow-parens */
       e.preventDefault();
@@ -30,6 +31,8 @@ function handleSkipLink() {
 }
 
 function handleHeader() {
+  const skipLinkElement = document.querySelector('.nhsuk-skip-link');
+  const headingElement = document.getElementsByTagName('H1')[0];
   if (skipLinkElement && headingElement) {
     headingElement.addEventListener('blur', e => { /* eslint-disable-line arrow-parens */
       e.preventDefault();