[MWPW-135217] Use correct approach for decorateLinks in Gnav (#1211)

* [MWPW-135217] Use decorateLinks before decorating Gnav * [MWPW-135217] Adjust gnav dropdown path after decorateLinks
adobecom · Sep 6, 2023 · 9cbdcaf · 9cbdcaf
1 parent 993f9d2
commit 9cbdcaf
Show file tree

Hide file tree

Showing 17 changed files with 510 additions and 110 deletions.
diff --git a/libs/blocks/global-footer/global-footer.js b/libs/blocks/global-footer/global-footer.js
@@ -68,6 +68,16 @@ class Footer {
 
     // TODO: log to LANA if Footer content could not be found
     if (!this.body) return;
+    // TODO: revisit region picker and social links decoration logic
+    const regionAnchor = this.body.querySelector('.region-selector a');
+    if (regionAnchor?.href) {
+      regionAnchor.setAttribute('href', `${regionAnchor.getAttribute('href')}#_dnt#_dnb`);
+    }
+    const socialLinks = document.querySelectorAll('.social a');
+    socialLinks.forEach((socialLink) => {
+      socialLink.setAttribute('href', `${socialLink.getAttribute('href')}#_dnb`);
+    });
+    decorateLinks(this.body);
 
     // Order is important, decorateFooter makes use of elements
     // which have already been created in previous steps
@@ -264,9 +274,8 @@ class Footer {
       const link = socialBlock.querySelector(`a[href*="${platform}"]`);
       if (!link) return;
 
-      // Add '#_dnb' to the 'href' value, since certain social media platforms are also blocks
       const iconElem = toFragment`<li class="feds-social-item">
-          <a href="${link.href}#_dnb" class="feds-social-link" aria-label="${platform}">
+          <a href="${link.href}" class="feds-social-link" aria-label="${platform}" target="_blank">
             <svg xmlns="http://www.w3.org/2000/svg" class="feds-social-icon" alt="${platform} logo">
               <use href="#footer-icon-${platform}" />
             </svg>
@@ -326,8 +335,6 @@ class Footer {
         </div>
       </div>`;
 
-    decorateLinks(this.elements.footer);
-
     return this.elements.footer;
   };
 }

diff --git a/libs/blocks/global-navigation/base.css b/libs/blocks/global-navigation/base.css
@@ -34,7 +34,9 @@
   display: flex;
   align-items: center;
   padding: 12px;
+  border: none;
   color: var(--feds-color-link--light);
+  font: inherit;
   white-space: nowrap;
   flex-shrink: 0;
 }

diff --git a/libs/blocks/global-navigation/features/breadcrumbs/breadcrumbs.js b/libs/blocks/global-navigation/features/breadcrumbs/breadcrumbs.js
@@ -1,4 +1,4 @@
-import { getMetadata, localizeLink } from '../../../../utils/utils.js';
+import { getMetadata } from '../../../../utils/utils.js';
 import { toFragment, lanaLog } from '../../utilities/utilities.js';
 
 const metadata = {
@@ -35,12 +35,6 @@ const setBreadcrumbSEO = (breadcrumbs) => {
   document.head.append(script);
 };
 
-const localizeEntries = (anchor) => {
-  if (!anchor) return;
-  const href = anchor.getAttribute('href');
-  anchor.setAttribute('href', localizeLink(href));
-};
-
 const createBreadcrumbs = (element) => {
   if (!element) return null;
   const ul = element.querySelector('ul');
@@ -58,12 +52,10 @@ const createBreadcrumbs = (element) => {
     .split(',')
     .map((item) => item.trim()) || [];
 
-  ul.querySelectorAll('li').forEach(
-    (li) => {
-      if (hiddenEntries.includes(li.innerText?.toLowerCase().trim())) return li.remove();
-      return localizeEntries(li.querySelector('a'));
-    },
-  );
+  ul.querySelectorAll('li').forEach((li) => {
+    if (hiddenEntries.includes(li.innerText?.toLowerCase().trim())) li.remove();
+  });
+
   const breadcrumbs = toFragment`
     <div class="feds-breadcrumbs-wrapper">
       <nav class="feds-breadcrumbs" aria-label="Breadcrumb">${ul}</nav>

diff --git a/libs/blocks/global-navigation/global-navigation.js b/libs/blocks/global-navigation/global-navigation.js
@@ -3,8 +3,7 @@ import {
   getConfig,
   getMetadata,
   loadIms,
-  localizeLink,
-  decorateSVG,
+  decorateLinks,
 } from '../../utils/utils.js';
 import {
   toFragment,
@@ -131,18 +130,6 @@ const getBrandImage = (image) => {
   // Return the default Adobe logo if an image is not available
   if (!image) return CONFIG.icons.company;
 
-  try {
-    // Try to decorate image as SVG
-    const decoratedSvg = decorateSVG(image);
-    // 'decorateSVG' might return the original element if decoration fails
-    // or the picture wrapped in an anchor element in certain cases
-    const svg = decoratedSvg instanceof HTMLPictureElement
-      ? decoratedSvg : decoratedSvg.querySelector('picture');
-    if (svg) return svg;
-  } catch (e) {
-    // continue execution
-  }
-
   // Try to decorate image as PNG, JPG or JPEG
   const imgText = image?.textContent || '';
   const [source, alt] = imgText.split('|');
@@ -181,6 +168,7 @@ class Gnav {
 
     this.el = el;
     this.body = body;
+    decorateLinks(this.body);
     this.elements = {};
   }
 
@@ -445,7 +433,7 @@ class Gnav {
     if (!rawBlock) return '';
 
     // Get all non-image links
-    const imgRegex = /(\.png|\.svg|\.jpg|\.jpeg)/;
+    const imgRegex = /(\.png|\.jpg|\.jpeg)/;
     const blockLinks = [...rawBlock.querySelectorAll('a')];
     const link = blockLinks.find((blockLink) => !imgRegex.test(blockLink.href)
       && !imgRegex.test(blockLink.textContent));
@@ -459,24 +447,27 @@ class Gnav {
     if (!renderImage && !renderLabel) return '';
 
     // Create image element
-    let imageEl = '';
+    const getImageEl = () => {
+      const svgImg = rawBlock.querySelector('picture img[src$=".svg"]');
+      if (svgImg) return svgImg;
 
-    if (renderImage) {
       const image = blockLinks.find((blockLink) => imgRegex.test(blockLink.href)
         || imgRegex.test(blockLink.textContent));
-      imageEl = toFragment`<span class="${classPrefix}-image">${getBrandImage(image)}</span>`;
-    }
+      return getBrandImage(image);
+    };
 
-    // Create label element
-    let labelEl = '';
+    const imageEl = renderImage
+      ? toFragment`<span class="${classPrefix}-image">${getImageEl()}</span>`
+      : '';
 
-    if (renderLabel) {
-      labelEl = toFragment`<span class="${classPrefix}-label">${link.textContent}</span>`;
-    }
+    // Create label element
+    const labelEl = renderLabel
+      ? toFragment`<span class="${classPrefix}-label">${link.textContent}</span>`
+      : '';
 
     // Create final template
     const decoratedElem = toFragment`
-      <a href="${localizeLink(link.getAttribute('href'))}" class="${classPrefix}" daa-ll="${analyticsValue}">
+      <a href="${link.href}" class="${classPrefix}" daa-ll="${analyticsValue}">
         ${imageEl}
         ${labelEl}
       </a>`;
@@ -573,16 +564,14 @@ class Gnav {
     switch (itemType) {
       case 'syncDropdownTrigger':
       case 'asyncDropdownTrigger': {
-        const dropdownTrigger = toFragment`<a
-          href="#"
+        const dropdownTrigger = toFragment`<button
           class="feds-navLink feds-navLink--hoverCaret"
-          role="button"
           aria-expanded="false"
           aria-haspopup="true"
           daa-ll="${getAnalyticsValue(item.textContent, index + 1)}"
           daa-lh="header|Open">
             ${item.textContent.trim()}
-          </a>`;
+          </button>`;
 
         const isSectionMenu = item.closest('.section') instanceof HTMLElement;
         const tag = isSectionMenu ? 'section' : 'div';
@@ -618,16 +607,12 @@ class Gnav {
           </div>`;
       case 'link': {
         const linkElem = item.querySelector('a');
-        const navLink = toFragment`<a
-          href="${localizeLink(linkElem.href)}"
-          class="feds-navLink"
-          daa-ll="${getAnalyticsValue(linkElem.textContent, index + 1)}">
-            ${linkElem.textContent.trim()}
-          </a>`;
+        linkElem.className = 'feds-navLink';
+        linkElem.setAttribute('daa-ll', getAnalyticsValue(linkElem.textContent, index + 1));
 
         const linkTemplate = toFragment`
           <div class="feds-navItem">
-            ${navLink}
+            ${linkElem}
           </div>`;
         return linkTemplate;
       }
@@ -645,8 +630,12 @@ class Gnav {
   decorateBreadcrumbs = async () => {
     if (!this.el.classList.contains('has-breadcrumbs')) return null;
     if (this.elements.breadcrumbsWrapper) return this.elements.breadcrumbsWrapper;
+    const breadcrumbsElem = this.el.querySelector('.breadcrumbs');
+    if (!breadcrumbsElem) return null;
+    // Breadcrumbs are not initially part of the nav, need to decorate the links
+    decorateLinks(breadcrumbsElem);
     const createBreadcrumbs = await loadBlock('../features/breadcrumbs/breadcrumbs.js');
-    this.elements.breadcrumbsWrapper = await createBreadcrumbs(this.el.querySelector('.breadcrumbs'));
+    this.elements.breadcrumbsWrapper = await createBreadcrumbs(breadcrumbsElem);
     return this.elements.breadcrumbsWrapper;
   };
 

diff --git a/libs/blocks/global-navigation/utilities/menu/menu.js b/libs/blocks/global-navigation/utilities/menu/menu.js
@@ -9,7 +9,7 @@ import {
   trigger,
   isDesktop,
 } from '../utilities.js';
-import { decorateLinks, localizeLink } from '../../../../utils/utils.js';
+import { decorateLinks } from '../../../../utils/utils.js';
 import { replaceText } from '../../../../features/placeholders.js';
 
 const decorateHeadline = (elem) => {
@@ -74,6 +74,7 @@ const decorateLinkGroup = (elem, index) => {
       ${imageElem}
       ${contentElem}
     </a>`;
+  if (link?.target) linkGroup.target = link.target;
 
   return linkGroup;
 };
@@ -172,7 +173,6 @@ const decoratePromo = (elem, index) => {
 };
 
 const decorateColumns = async ({ content, separatorTagName = 'H5' } = {}) => {
-  decorateLinks(content);
   const hasMultipleColumns = content.children.length > 1;
 
   // The resulting template structure should follow these rules:
@@ -264,8 +264,8 @@ const decorateMenu = (config) => logErrorFor(async () => {
   if (config.type === 'asyncDropdownTrigger') {
     const pathElement = config.item.querySelector('a');
     if (!(pathElement instanceof HTMLElement)) return;
-    const path = localizeLink(pathElement.href);
-    const res = await fetch(`${path}.plain.html`);
+    const path = pathElement.href.replace(/(\.html$|$)/, '.plain.html');
+    const res = await fetch(path);
     if (res.status !== 200) return;
     const content = await res.text();
     const parsedContent = await replaceText(content, getFedsPlaceholderConfig(), /{{(.*?)}}/g, 'feds');
@@ -275,6 +275,8 @@ const decorateMenu = (config) => logErrorFor(async () => {
         </div>
       </div>`;
 
+    // Content has been fetched dynamically, need to decorate links
+    decorateLinks(menuTemplate);
     await decorateColumns({ content: menuTemplate.firstElementChild });
     config.template.classList.add('feds-navItem--megaMenu');
   }

diff --git a/libs/blocks/global-navigation/utilities/utilities.js b/libs/blocks/global-navigation/utilities/utilities.js
@@ -110,14 +110,13 @@ export async function loadDecorateMenu() {
 export function decorateCta({ elem, type = 'primaryCta', index } = {}) {
   const modifier = type === 'secondaryCta' ? 'secondary' : 'primary';
 
+  const clone = elem.cloneNode(true);
+  clone.className = `feds-cta feds-cta--${modifier}`;
+  clone.setAttribute('daa-ll', getAnalyticsValue(clone.textContent, index));
+
   return toFragment`
     <div class="feds-cta-wrapper">
-      <a
-        href="${elem.href}"
-        class="feds-cta feds-cta--${modifier}"
-        daa-ll="${getAnalyticsValue(elem.textContent, index)}">
-          ${elem.textContent}
-      </a>
+      ${clone}
     </div>`;
 }
 

diff --git a/test/blocks/global-navigation/features/breadcrumbs.test.js b/test/blocks/global-navigation/features/breadcrumbs.test.js
@@ -2,8 +2,7 @@ import { expect } from '@esm-bundle/chai';
 import { stub } from 'sinon';
 import breadcrumbs from '../../../../libs/blocks/global-navigation/features/breadcrumbs/breadcrumbs.js';
 import { toFragment } from '../../../../libs/blocks/global-navigation/utilities/utilities.js';
-import { mockRes } from '../test-utilities.js';
-import { setConfig } from '../../../../libs/utils/utils.js';
+import { mockRes, createFullGlobalNavigation } from '../test-utilities.js';
 
 export const breadcrumbMock = () => toFragment`
   <div class="breadcrumbs">
@@ -131,19 +130,19 @@ describe('breadcrumbs', () => {
   });
 
   it('should localize breadcrumb links', async () => {
-    setConfig({
+    const customConfig = {
       locales: {
         '': { ietf: 'en-US' },
         fi: { ietf: 'fi-FI' },
       },
       pathname: '/fi/',
       prodDomains: 'http://localhost:2000',
-    });
+    };
 
-    await breadcrumbs(breadcrumbMock());
-    const script = document.querySelector('script');
+    const breadcrumbsEl = breadcrumbMock();
+    await createFullGlobalNavigation({ breadcrumbsEl, customConfig });
+    const script = document.querySelector('script[type="application/ld+json"]');
     expect(script).to.exist;
-    expect(script.type).to.equal('application/ld+json');
     expect(JSON.parse(script.innerHTML)).to.deep.equal(
       {
         '@context': 'https://schema.org',
@@ -159,7 +158,7 @@ describe('breadcrumbs', () => {
             '@type': 'ListItem',
             position: 2,
             name: 'Future Releases',
-            item: 'http://localhost:2000/fi/', // <-- localised prod domain
+            item: 'http://localhost:2000/fi/', // <-- localized prod domain
           },
           {
             '@type': 'ListItem',

diff --git a/test/blocks/global-navigation/global-navigation-with-html-extension.test.html b/test/blocks/global-navigation/global-navigation-with-html-extension.test.html
@@ -0,0 +1,20 @@
+<html>
+  <head></head>
+  <body></body>
+  <script type="module">
+    import { runTests } from '@web/test-runner-mocha';
+    import { readFile } from '@web/test-runner-commands';
+    import { expect } from '@esm-bundle/chai';
+    import { createFullGlobalNavigation } from './test-utilities.js';
+
+    runTests(() => {
+      describe('Gnav on page with .html extension', () => {
+        it('uses the correct section menu path', async () => {
+          await createFullGlobalNavigation();
+          const sectionDropdown = document.querySelector('section.feds-navItem--section .feds-popup:not(:empty)');
+          expect(sectionDropdown).to.exist;
+        });
+      });
+    });
+  </script>
+</html>
diff --git a/test/blocks/global-navigation/global-navigation.test.js b/test/blocks/global-navigation/global-navigation.test.js
@@ -722,7 +722,7 @@ describe('global navigation', () => {
         signIn.click();
 
         const signInDropdown = document.querySelector(selectors.signInDropdown);
-        const dropdownSignIn = signInDropdown.querySelector('[href="https://adobe.com?sign-in=true"]');
+        const dropdownSignIn = signInDropdown.querySelector('[href$="?sign-in=true"]');
 
         window.adobeIMS = { signIn: sinon.spy() };
 
@@ -806,7 +806,7 @@ describe('global navigation', () => {
         signIn.click();
 
         const signInDropdown = document.querySelector(selectors.signInDropdown);
-        const dropdownSignIn = signInDropdown.querySelector('[href="https://adobe.com?sign-in=true"]');
+        const dropdownSignIn = signInDropdown.querySelector('[href$="?sign-in=true"]');
 
         window.adobeIMS = { signIn: sinon.spy() };