From f379815f307f03c8e42fca3b598e657716d34cd8 Mon Sep 17 00:00:00 2001 From: Ryan Parrish Date: Fri, 8 Nov 2024 03:12:24 -0700 Subject: [PATCH] MWPW-140452 - Icon authoring in milo using the federal repo and individual SVG assets (#2986) * updated features/icons to pull from federal and allow the icons set to be sharepoint authorable * bettter icon-spacing handling * preload federated.js instead of non used icons.svg on util decorateIcon() * Updated preload federated as script type not fetch * Minor clean up based on pr feedback * remove console * addressed some issues found w/ icons not rending in tables due to race condition w/ decorateIcon() * lana instead of console * no 100% height * coverage * full coverage * minor cleanup * minor cleanup * preload first section icons, spread syntax * no cons * Performace refactor - loadIcons to hold most related functionallity. Preloaded inView icons and defered others till postSectionLoad. * async decTooltips and refactor to not chain func calls * fix: icon alignment in georouting modal's button * move render blocking code to utils * fix import of test method * remove condition that causes error when no icon is in view * remove redundant link load * remove extra federated.js import --------- Co-authored-by: Saugat Malla Co-authored-by: Saugat Malla --- libs/blocks/text/text.css | 7 +- libs/features/georoutingv2/georoutingv2.css | 1 + libs/features/georoutingv2/georoutingv2.js | 2 +- libs/features/icons/icons.js | 118 ++++++++++++++++---- libs/styles/styles.css | 39 +++++-- libs/utils/utils.js | 50 +++++---- test/features/icons/icons.test.js | 61 +++++----- test/features/icons/mocks/body.html | 4 +- 8 files changed, 198 insertions(+), 84 deletions(-) diff --git a/libs/blocks/text/text.css b/libs/blocks/text/text.css index 6242f50132..f8cbefd365 100644 --- a/libs/blocks/text/text.css +++ b/libs/blocks/text/text.css @@ -100,7 +100,7 @@ position: relative; } -.text-block .icon-list-item .icon.margin-right:not(.margin-left) { /* target first node only */ +.text-block .icon-list-item .icon.node-index-first { position: absolute; inset: 0 100% auto auto; } @@ -122,7 +122,6 @@ .text-block .icon-area { display: flex; - column-gap: var(--spacing-xs); } .text-block p.icon-area { /* NOT tags with icons in them */ @@ -218,10 +217,6 @@ max-width: unset; } -.text-block .icon-area.con-button { - column-gap: unset; -} - .text-block .icon-area picture { line-height: 0em; height: inherit; /* Safari + FF bug fix */ diff --git a/libs/features/georoutingv2/georoutingv2.css b/libs/features/georoutingv2/georoutingv2.css index 255ba3f301..09df2cba8b 100644 --- a/libs/features/georoutingv2/georoutingv2.css +++ b/libs/features/georoutingv2/georoutingv2.css @@ -83,6 +83,7 @@ } .dialog-modal.locale-modal-v2 span.icon { + display: inline; vertical-align: middle; } diff --git a/libs/features/georoutingv2/georoutingv2.js b/libs/features/georoutingv2/georoutingv2.js index 7213696f63..236e9b685a 100644 --- a/libs/features/georoutingv2/georoutingv2.js +++ b/libs/features/georoutingv2/georoutingv2.js @@ -194,7 +194,7 @@ function buildContent(currentPage, locale, geoData, locales) { { once: true }, ); img.src = `${config.miloLibs || config.codeRoot}/img/georouting/${flagFile}`; - const span = createTag('span', { class: 'icon margin-inline-end' }, img); + const span = createTag('span', { class: 'icon node-index-first' }, img); const mainAction = createTag('a', { class: 'con-button blue button-l', lang, role: 'button', 'aria-haspopup': !!locales, 'aria-expanded': false, href: '#', }, span); diff --git a/libs/features/icons/icons.js b/libs/features/icons/icons.js index 975c0625dc..a39ff61e06 100644 --- a/libs/features/icons/icons.js +++ b/libs/features/icons/icons.js @@ -1,5 +1,9 @@ +import { getFederatedContentRoot } from '../../utils/federated.js'; +import { loadLink, loadStyle } from '../../utils/utils.js'; + let fetchedIcons; let fetched = false; +const federalIcons = {}; async function getSVGsfromFile(path) { /* c8 ignore next */ @@ -22,6 +26,7 @@ async function getSVGsfromFile(path) { return miloIcons; } +// TODO: remove after all consumers have stopped calling this method // eslint-disable-next-line no-async-promise-executor export const fetchIcons = (config) => new Promise(async (resolve) => { /* c8 ignore next */ @@ -34,10 +39,10 @@ export const fetchIcons = (config) => new Promise(async (resolve) => { resolve(fetchedIcons); }); -function decorateToolTip(icon) { +async function decorateToolTip(icon) { const wrapper = icon.closest('em'); - wrapper.className = 'tooltip-wrapper'; if (!wrapper) return; + wrapper.className = 'tooltip-wrapper'; const conf = wrapper.textContent.split('|'); // Text is the last part of a tooltip const content = conf.pop().trim(); @@ -45,30 +50,101 @@ function decorateToolTip(icon) { icon.dataset.tooltip = content; // Position is the next to last part of a tooltip const place = conf.pop()?.trim().toLowerCase() || 'right'; - icon.className = `icon icon-info milo-tooltip ${place}`; + const defaultIcon = 'info-outline'; + icon.className = `icon icon-${defaultIcon} milo-tooltip ${place}`; + icon.dataset.name = defaultIcon; wrapper.parentElement.replaceChild(icon, wrapper); } -export default async function loadIcons(icons, config) { - const iconSVGs = await fetchIcons(config); - if (!iconSVGs) return; +export function getIconData(icon) { + const fedRoot = getFederatedContentRoot(); + const name = [...icon.classList].find((c) => c.startsWith('icon-'))?.substring(5); + const path = `${fedRoot}/federal/assets/icons/svgs/${name}.svg`; + return { path, name }; +} + +function preloadInViewIconResources(config) { + const { base } = config; + loadStyle(`${base}/features/icons/icons.css`); +} + +const preloadInViewIcons = async (icons = []) => icons.forEach((icon) => { + const { path } = getIconData(icon); + loadLink(path, { rel: 'preload', as: 'fetch', crossorigin: 'anonymous' }); +}); + +function filterDuplicatedIcons(icons) { + if (!icons.length) return []; + const uniqueIconKeys = new Set(); + const uniqueIcons = []; + for (const icon of icons) { + const key = [...icon.classList].find((c) => c.startsWith('icon-'))?.substring(5); + if (!uniqueIconKeys.has(key)) { + uniqueIconKeys.add(key); + uniqueIcons.push(icon); + } + } + return uniqueIcons; +} + +export async function decorateIcons(area, icons, config) { + if (!icons.length) return; + const uniqueIcons = filterDuplicatedIcons(icons); + if (!uniqueIcons.length) return; + preloadInViewIcons(uniqueIcons); + preloadInViewIconResources(config); + icons.forEach((icon) => { + const iconName = [...icon.classList].find((c) => c.startsWith('icon-'))?.substring(5); + if (!iconName) return; + icon.dataset.name = iconName; + }); +} + +export default async function loadIcons(icons) { + const fedRoot = getFederatedContentRoot(); + const iconRequests = []; + const iconsToFetch = new Map(); + icons.forEach(async (icon) => { - const { classList } = icon; - if (classList.contains('icon-tooltip')) decorateToolTip(icon); - const iconName = icon.classList[1].replace('icon-', ''); - const existingIcon = icon.querySelector('svg'); - if (!iconSVGs[iconName] || existingIcon) return; + const isToolTip = icon.classList.contains('icon-tooltip'); + if (isToolTip) decorateToolTip(icon); + const iconName = icon.dataset.name; + if (icon.dataset.svgInjected || !iconName) return; + if (!federalIcons[iconName] && !iconsToFetch.has(iconName)) { + const url = `${fedRoot}/federal/assets/icons/svgs/${iconName}.svg`; + iconsToFetch.set(iconName, fetch(url) + .then(async (res) => { + if (!res.ok) throw new Error(`Failed to fetch SVG for ${iconName}: ${res.statusText}`); + const text = await res.text(); + const parser = new DOMParser(); + const svgDoc = parser.parseFromString(text, 'image/svg+xml'); + const svgElement = svgDoc.querySelector('svg'); + if (!svgElement) { + window.lana?.log(`No SVG element found in fetched content for ${iconName}`); + return; + } + const svgClone = svgElement.cloneNode(true); + svgClone.classList.add('icon-milo', `icon-milo-${iconName}`); + federalIcons[iconName] = svgClone; + }) + /* c8 ignore next 3 */ + .catch((error) => { + window.lana?.log(`Error fetching SVG for ${iconName}:`, error); + })); + } + iconRequests.push(iconsToFetch.get(iconName)); const parent = icon.parentElement; - if (parent.childNodes.length > 1) { - if (parent.lastChild === icon) { - icon.classList.add('margin-inline-start'); - } else if (parent.firstChild === icon) { - icon.classList.add('margin-inline-end'); - if (parent.parentElement.tagName === 'LI') parent.parentElement.classList.add('icon-list-item'); - } else { - icon.classList.add('margin-inline-start', 'margin-inline-end'); - } + if (parent && parent.parentElement.tagName === 'LI') parent.parentElement.classList.add('icon-list-item'); + }); + + await Promise.all(iconRequests); + + icons.forEach((icon) => { + const iconName = icon.dataset.name; + if (iconName && federalIcons[iconName] && !icon.dataset.svgInjected) { + const svgClone = federalIcons[iconName].cloneNode(true); + icon.appendChild(svgClone); + icon.dataset.svgInjected = 'true'; } - icon.insertAdjacentHTML('afterbegin', iconSVGs[iconName].outerHTML); }); } diff --git a/libs/styles/styles.css b/libs/styles/styles.css index febdd75f5c..1bf6c4bb08 100644 --- a/libs/styles/styles.css +++ b/libs/styles/styles.css @@ -128,6 +128,7 @@ --icon-size-s: 32px; --icon-size-xs: 24px; --icon-size-xxs: 16px; + --icon-spacing: 8px; /* z-index */ --above-all: 9000; /* Used for page tools that overlay page content */ @@ -349,6 +350,7 @@ line-height: 20px; min-height: 21px; padding: 7px 18px 8px; + --icon-spacing: 12px; } .xl-button .con-button, @@ -358,6 +360,17 @@ line-height: 24px; min-height: 28px; padding: 10px 24px 8px; + --icon-spacing: 14px; +} + +.xxl-button .con-button, +.con-button.button-xxl { + border-radius: 30px; + font-size: 22px; + line-height: 27px; + min-height: 27px; + padding: 14px 30px 15px; + --icon-spacing: 14px; } .xxl-button .con-button, @@ -555,19 +568,23 @@ div[data-failed="true"]::before { color: var(--color-gray-300); } -span.icon.margin-right { margin-right: 8px; } - -span.icon.margin-left { margin-left: 8px; } - -span.icon.margin-inline-end { margin-inline-end: 8px; } - -span.icon.margin-inline-start { margin-inline-start: 8px; } +span.icon { + width: 1em; + display: inline-block; + margin-inline: var(--icon-spacing); +} -.button-l .con-button span.icon.margin-left, -.con-button.button-l span.icon.margin-left { margin-left: 12px; } +span.icon.node-index-first { margin-inline-start: unset; } +span.icon.node-index-middle { margin-inline: var(--icon-spacing); } +span.icon.node-index-last { margin-inline-end: unset; } +span.icon.node-index-only { margin-inline: unset; } -.button-xl .con-button span.icon.margin-left, -.con-button.button-xl span.icon.margin-left { margin-left: 14px; } +span.icon svg { + height: 1em; + position: relative; + top: .1em; + width: auto; +} /* Con Block Utils */ .con-block.xs-spacing { padding: var(--spacing-xs) 0; } diff --git a/libs/utils/utils.js b/libs/utils/utils.js index baf303c781..c29960c7de 100644 --- a/libs/utils/utils.js +++ b/libs/utils/utils.js @@ -311,7 +311,7 @@ export function localizeLink( const isLocalizedLink = path.startsWith(`/${LANGSTORE}`) || path.startsWith(`/${PREVIEW}`) || Object.keys(locales).some((loc) => loc !== '' && (path.startsWith(`/${loc}/`) - || path.endsWith(`/${loc}`))); + || path.endsWith(`/${loc}`))); if (isLocalizedLink) return processedHref; const urlPath = `${locale.prefix}${path}${url.search}${hash}`; return relative ? urlPath : `${url.origin}${urlPath}`; @@ -762,7 +762,7 @@ function decorateHeader() { } header.className = headerMeta || 'global-navigation'; const metadataConfig = getMetadata('breadcrumbs')?.toLowerCase() - || getConfig().breadcrumbs; + || getConfig().breadcrumbs; if (metadataConfig === 'off') return; const baseBreadcrumbs = getMetadata('breadcrumbs-base')?.length; @@ -775,16 +775,6 @@ function decorateHeader() { if (promo?.length) header.classList.add('has-promo'); } -async function decorateIcons(area, config) { - const icons = area.querySelectorAll('span.icon'); - if (icons.length === 0) return; - const { base } = config; - loadStyle(`${base}/features/icons/icons.css`); - loadLink(`${base}/img/icons/icons.svg`, { rel: 'preload', as: 'fetch', crossorigin: 'anonymous' }); - const { default: loadIcons } = await import('../features/icons/icons.js'); - await loadIcons(icons, config); -} - export async function customFetch({ resource, withCacheRules }) { const options = {}; if (withCacheRules) { @@ -824,8 +814,8 @@ async function decoratePlaceholders(area, config) { area.dataset.hasPlaceholders = 'true'; const placeholderPath = `${config.locale?.contentRoot}/placeholders.json`; placeholderRequest = placeholderRequest - || customFetch({ resource: placeholderPath, withCacheRules: true }) - .catch(() => ({})); + || customFetch({ resource: placeholderPath, withCacheRules: true }) + .catch(() => ({})); const { decoratePlaceholderArea } = await import('../features/placeholders.js'); await decoratePlaceholderArea({ placeholderPath, placeholderRequest, nodes }); } @@ -1195,9 +1185,8 @@ function decorateDocumentExtras() { decorateHeader(); } -async function documentPostSectionLoading(config) { +async function documentPostSectionLoading(area, config) { decorateFooterPromo(); - const appendage = getMetadata('title-append'); if (appendage) { import('../features/title-append/title-append.js').then((module) => module.default(appendage)); @@ -1261,6 +1250,18 @@ async function resolveInlineFrags(section) { section.preloadLinks = newlyDecoratedSection.preloadLinks; } +export function setIconsIndexClass(icons) { + [...icons].forEach((icon) => { + const parent = icon.parentNode; + const children = parent.childNodes; + const nodeIndex = [...children].indexOf.call(children, icon); + let indexClass = (nodeIndex === children.length - 1) ? 'last' : 'middle'; + if (nodeIndex === 0) indexClass = 'first'; + if (children.length === 1) indexClass = 'only'; + icon.classList.add(`node-index-${indexClass}`); + }); +} + async function processSection(section, config, isDoc) { await resolveInlineFrags(section); const firstSection = section.el.dataset.idx === '0'; @@ -1268,7 +1269,6 @@ async function processSection(section, config, isDoc) { preloadBlockResources(section.preloadLinks); await Promise.all([ decoratePlaceholders(section.el, config), - decorateIcons(section.el, config), ]); const loadBlocks = [...stylePromises]; if (section.preloadLinks.length) { @@ -1301,6 +1301,11 @@ export async function loadArea(area = document) { decorateDocumentExtras(); } + const allIcons = area.querySelectorAll('span.icon'); + if (allIcons.length) { + setIconsIndexClass(allIcons); + } + const sections = decorateSections(area, isDoc); const areaBlocks = []; @@ -1313,13 +1318,20 @@ export async function loadArea(area = document) { }); } + if (allIcons.length) { + const { default: loadIcons, decorateIcons } = await import('../features/icons/icons.js'); + await decorateIcons(area, allIcons, config); + await loadIcons(allIcons); + } + const currentHash = window.location.hash; if (currentHash) { scrollToHashedElement(currentHash); } - if (isDoc) await documentPostSectionLoading(config); - + if (isDoc) { + await documentPostSectionLoading(area, config); + } await loadDeferred(area, areaBlocks, config); } diff --git a/test/features/icons/icons.test.js b/test/features/icons/icons.test.js index cd355a0aff..cb3cb994db 100644 --- a/test/features/icons/icons.test.js +++ b/test/features/icons/icons.test.js @@ -1,45 +1,56 @@ import { readFile } from '@web/test-runner-commands'; import { expect } from '@esm-bundle/chai'; -import { stub } from 'sinon'; -import { setConfig, getConfig, createTag } from '../../../libs/utils/utils.js'; +import sinon, { stub } from 'sinon'; +import { waitForElement } from '../../helpers/waitfor.js'; -const { default: loadIcons } = await import('../../../libs/features/icons/icons.js'); - -const codeRoot = '/libs'; -const conf = { codeRoot }; -setConfig(conf); -const config = getConfig(); +const { default: loadIcons, getIconData } = await import('../../../libs/features/icons/icons.js'); +const { setIconsIndexClass } = await import('../../../libs/utils/utils.js'); +const mockRes = ({ payload, status = 200, ok = true } = {}) => new Promise((resolve) => { + resolve({ + status, + ok, + json: () => payload, + text: () => payload, + }); +}); document.body.innerHTML = await readFile({ path: './mocks/body.html' }); let icons; +const svgEx = ` + + +`; describe('Icon Suppprt', () => { - let paramsGetStub; - - before(() => { - paramsGetStub = stub(URLSearchParams.prototype, 'get'); - paramsGetStub.withArgs('cache').returns('off'); + beforeEach(() => { + stub(window, 'fetch').callsFake(() => mockRes({})); }); - after(() => { - paramsGetStub.restore(); + afterEach(() => { + sinon.restore(); }); - before(async () => { + it('Replaces span.icon', async () => { + const payload = svgEx; + window.fetch.returns(mockRes({ payload })); + icons = document.querySelectorAll('span.icon'); - await loadIcons(icons, config); - await loadIcons(icons, config); // Test duplicate icon not created if run twice - }); + icons.forEach((icon) => { + const { name } = getIconData(icon); + icon.dataset.name = name; + }); + await loadIcons(icons); - it('Fetches successfully with cache control enabled', async () => { - const otherIcons = [createTag('span', { class: 'icon icon-play' })]; - await loadIcons(otherIcons, config); + const selector = await waitForElement('span.icon svg'); + expect(selector).to.exist; }); - it('Replaces span.icon', async () => { - const selector = icons[0].querySelector(':scope svg'); - expect(selector).to.exist; + it('Sets icon index class', async () => { + icons = document.querySelectorAll('span.icon'); + setIconsIndexClass(icons); + const secondIconHasIndexClass = icons[2].classList.contains('node-index-last'); + expect(secondIconHasIndexClass).to.be.true; }); it('No duplicate icon', async () => { diff --git a/test/features/icons/mocks/body.html b/test/features/icons/mocks/body.html index 586013bf4f..2d908a81ca 100644 --- a/test/features/icons/mocks/body.html +++ b/test/features/icons/mocks/body.html @@ -1,4 +1,6 @@ -
+
+ +