From 57365f0a9e38f2595c8f0e6be9c8b305005dcc75 Mon Sep 17 00:00:00 2001 From: Robert Bogos Date: Mon, 10 Mar 2025 16:23:05 +0200 Subject: [PATCH 01/11] federal icons --- libs/blocks/library-config/library-config.css | 8 +- libs/blocks/library-config/library-config.js | 13 +- libs/blocks/library-config/lists/icons.js | 46 ++--- libs/blocks/table/table.css | 1 + libs/blocks/text/text.css | 7 +- libs/features/georoutingv2/georoutingv2.js | 2 +- libs/features/icons/icons.js | 105 ++++++++--- libs/styles/styles.css | 34 ++-- libs/utils/utils.js | 1 - test/features/icons/icons.test.js | 168 ++++++++++++++---- test/features/icons/mocks/body.html | 4 +- 11 files changed, 276 insertions(+), 113 deletions(-) diff --git a/libs/blocks/library-config/library-config.css b/libs/blocks/library-config/library-config.css index 121f79c95a..fea7df1d69 100644 --- a/libs/blocks/library-config/library-config.css +++ b/libs/blocks/library-config/library-config.css @@ -206,7 +206,8 @@ input.sk-library-search-input:focus { * Margin height equal to search bar height */ .sk-library ul.con-blocks-list.inset, -.sk-library ul.con-templates-list.inset { +.sk-library ul.con-templates-list.inset, +.sk-library ul.con-icons-list.inset { margin-bottom: 39px; } @@ -244,6 +245,7 @@ input.sk-library-search-input:focus { } .sk-library .block-group.is-hidden, +.sk-library .icon-item.is-hidden, .con-templates-list .template.is-hidden { display: none; } @@ -290,6 +292,10 @@ input.sk-library-search-input:focus { padding: 0 12px; } +.sk-library li.icon-item img.icon-fed { + filter: invert(100%); +} + .sk-library li.template { display: flex; align-items: stretch; diff --git a/libs/blocks/library-config/library-config.js b/libs/blocks/library-config/library-config.js index 6d1d6c586a..ce2b33ac0a 100644 --- a/libs/blocks/library-config/library-config.js +++ b/libs/blocks/library-config/library-config.js @@ -17,9 +17,9 @@ async function loadPlaceholders(content, list) { placeholders(content, list); } -async function loadIcons(content, list) { +async function loadIcons({ content, list, query }) { const { default: icons } = await import('./lists/icons.js'); - icons(content, list); + icons(content, list, query); } async function loadAssets(content, list) { @@ -56,6 +56,9 @@ function addSearch({ content, list, type }) { case 'templates': loadTemplates({ content, list, query, type }); break; + case 'icons': + loadIcons({ content, list, query }); + break; default: } }); @@ -71,6 +74,9 @@ function addSearch({ content, list, type }) { case 'templates': loadTemplates({ content, list, query, type }); break; + case 'icons': + loadIcons({ content, list, query }); + break; default: } }); @@ -98,7 +104,8 @@ async function loadList(type, content, list) { loadPlaceholders(content, list); break; case 'icons': - loadIcons(content, list); + addSearch({ content, list, type }); + loadIcons({ content, list }); break; case 'assets': loadAssets(content, list); diff --git a/libs/blocks/library-config/lists/icons.js b/libs/blocks/library-config/lists/icons.js index 62b4966295..9707a95e09 100644 --- a/libs/blocks/library-config/lists/icons.js +++ b/libs/blocks/library-config/lists/icons.js @@ -1,25 +1,29 @@ -import { fetchIcons } from '../../../features/icons/icons.js'; -import { createTag, getConfig } from '../../../utils/utils.js'; +import { fetchIconList } from '../../../features/icons/icons.js'; +import { createTag } from '../../../utils/utils.js'; import createCopy from '../library-utils.js'; -export default async function iconList(content, list) { - const config = getConfig(); - const icons = await fetchIcons(config); - Object.keys(icons).forEach((key) => { - const icon = createTag('span', { class: `icon icon-${key}` }, icons[key]); - const titleText = createTag('p', { class: 'item-title' }, key); - const title = createTag('li', { class: 'icon-item' }, icon); - title.append(titleText); - const copy = createTag('button', { class: 'copy' }); - copy.id = `${key}-icon-copy`; - copy.addEventListener('click', (e) => { - e.target.classList.add('copied'); - setTimeout(() => { e.target.classList.remove('copied'); }, 3000); - const formatted = `:${key}:`; - const blob = new Blob([formatted], { type: 'text/plain' }); - createCopy(blob); +export default async function iconList(content, list, query) { + const fedIconList = await fetchIconList(content[0].path); + list.innerHTML = ''; + if (fedIconList.length) { + [...fedIconList].forEach((icon) => { + const svg = createTag('span', { class: `icon icon-${icon.name}` }, createTag('img', { class: `icon-${icon.name}-img icon-fed`, src: `${icon.url}`, width: '18px' })); + const titleText = createTag('p', { class: 'item-title' }, icon.name); + const title = createTag('li', { class: 'icon-item' }, svg); + if (query && !icon.name.includes(query)) title.classList.add('is-hidden'); + title.append(titleText); + const copy = createTag('button', { class: 'copy' }); + copy.id = `${icon.name}-icon-copy`; + copy.addEventListener('click', (e) => { + e.target.classList.add('copied'); + setTimeout(() => { e.target.classList.remove('copied'); }, 3000); + const formatted = `:${icon.name}:`; + const blob = new Blob([formatted], { type: 'text/plain' }); + createCopy(blob); + window.hlx?.rum.sampleRUM('click', { source: e.target }); + }); + title.append(copy); + list.append(title); }); - title.append(copy); - list.append(title); - }); + } } diff --git a/libs/blocks/table/table.css b/libs/blocks/table/table.css index cc1a4c81a6..b8ea2e2815 100644 --- a/libs/blocks/table/table.css +++ b/libs/blocks/table/table.css @@ -361,6 +361,7 @@ cursor: pointer; rotate: -90deg; background-size: contain; + margin-inline: unset; } [dir="rtl"] .table .icon.expand { diff --git a/libs/blocks/text/text.css b/libs/blocks/text/text.css index 685c9fbbd0..e20fdbb1dd 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:not(:has(span.icon)) { display: flex; - column-gap: var(--spacing-xs); } .text-block p.icon-area { /* NOT tags with icons in them */ @@ -217,10 +216,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.js b/libs/features/georoutingv2/georoutingv2.js index d6c0ad8683..26f5e46c67 100644 --- a/libs/features/georoutingv2/georoutingv2.js +++ b/libs/features/georoutingv2/georoutingv2.js @@ -196,7 +196,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 eb7f74a214..17fa5bdec6 100644 --- a/libs/features/icons/icons.js +++ b/libs/features/icons/icons.js @@ -1,5 +1,10 @@ +import { lanaLog } from '../../blocks/global-navigation/utilities/utilities.js'; +import { getFederatedContentRoot } from '../../utils/federated.js'; +import { getConfig } from '../../utils/utils.js'; + let fetchedIcons; let fetched = false; +const federalIcons = {}; async function getSVGsfromFile(path) { /* c8 ignore next */ @@ -22,6 +27,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,13 +40,20 @@ export const fetchIcons = (config) => new Promise(async (resolve) => { resolve(fetchedIcons); }); -function decorateToolTip(icon, iconName) { - const hasTooltip = icon.closest('em')?.textContent.includes('|') && [...icon.classList].some((cls) => cls.includes('tooltip')); - if (!hasTooltip) return; +export function fetchIconList(url) { + return fetch(url) + .then((resp) => resp.json()) + .then((json) => json.content.data) + .catch(() => { + lanaLog({ message: 'Failed to fetch iconList', tags: 'icons', errorType: 'error' }); + return []; + }); +} +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(); @@ -48,35 +61,71 @@ function decorateToolTip(icon, iconName) { 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-${iconName} milo-tooltip ${place}`; - icon.setAttribute('tabindex', '0'); - icon.setAttribute('aria-label', content); - icon.setAttribute('role', 'button'); + icon.className = `icon icon-info-outline milo-tooltip ${place}`; + [['tabindex', '0'], ['aria-label', content], ['role', 'button']].forEach(([attr, value]) => { + icon.setAttribute(attr, value); + }); wrapper.parentElement.replaceChild(icon, wrapper); } -export default async function loadIcons(icons, config) { - const iconSVGs = await fetchIcons(config); - if (!iconSVGs) return; - icons.forEach(async (icon) => { - const iconNameInitial = icon.classList[1].replace('icon-', ''); - let iconName = iconNameInitial === 'tooltip' ? 'info' : iconNameInitial; - if (iconNameInitial.includes('tooltip-')) iconName = iconNameInitial.replace(/tooltip-/, ''); - decorateToolTip(icon, iconName); +export function setNodeIndexClass(icon) { + const children = icon.parentNode.childNodes; + const nodeIndex = Array.prototype.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}`); +} - const existingIcon = icon.querySelector('svg'); - if (!iconSVGs[iconName] || existingIcon) return; +export default async function loadIcons(icons) { + const fedRoot = getFederatedContentRoot(); + const iconRequests = []; + const iconsToFetch = new Map(); + icons.forEach((icon) => { + setNodeIndexClass(icon); + if (icon.classList.contains('icon-tooltip')) decorateToolTip(icon); + const iconName = [...icon.classList].find((c) => c.startsWith('icon-'))?.substring(5); + if (icon.dataset.svgInjected || !iconName) return; + if (!federalIcons[iconName] && !iconsToFetch.has(iconName)) { + iconsToFetch.set(iconName, fetch(`${fedRoot}/federal/assets/icons/svgs/${iconName}.svg`) + .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 svgElement = parser.parseFromString(text, 'image/svg+xml').querySelector('svg'); + if (!svgElement) { + lanaLog({ message: `No SVG element found in fetched content for ${iconName}`, tags: 'icons', errorType: 'error' }); + return; + } + svgElement.classList.add('icon-milo', `icon-milo-${iconName}`); + federalIcons[iconName] = svgElement; + }) + .catch(async (e) => { + lanaLog({ message: `Error fetching federal SVG for ${iconName}, falling back to Milo icon`, e, tags: 'icons', errorType: 'error' }); + // Fallback to Milo icons + if (!fetchedIcons) { + const { miloLibs, codeRoot } = getConfig(); + const base = miloLibs || codeRoot; + fetchedIcons = await getSVGsfromFile(`${base}/img/icons/icons.svg`); + } + if (fetchedIcons?.[iconName]) { + federalIcons[iconName] = fetchedIcons[iconName].cloneNode(true); + return; + } + lanaLog({ message: `No fallback Milo icon found for ${iconName}`, e, tags: 'icons', errorType: '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.classList].find((c) => c.startsWith('icon-'))?.substring(5); + 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 d150683058..b5258600ae 100644 --- a/libs/styles/styles.css +++ b/libs/styles/styles.css @@ -129,6 +129,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 */ @@ -356,6 +357,7 @@ line-height: 20px; min-height: 21px; padding: 7px 18px 8px; + --icon-spacing: 12px; } .xl-button .con-button, @@ -365,6 +367,7 @@ line-height: 24px; min-height: 28px; padding: 10px 24px 8px; + --icon-spacing: 14px; } .xxl-button .con-button, @@ -374,6 +377,7 @@ line-height: 27px; min-height: 27px; padding: 14px 30px 15px; + --icon-spacing: 14px; } .con-button.button-justified { @@ -566,23 +570,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.milo-tooltip.margin-inline-end { margin-inline-end: 3px; } - -span.icon.milo-tooltip.margin-inline-start { margin-inline-start: 3px; } +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 4af6d67231..4fb1d33182 100644 --- a/libs/utils/utils.js +++ b/libs/utils/utils.js @@ -896,7 +896,6 @@ async function decorateIcons(area, config) { 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); } diff --git a/test/features/icons/icons.test.js b/test/features/icons/icons.test.js index 669551a807..27ead5c4d6 100644 --- a/test/features/icons/icons.test.js +++ b/test/features/icons/icons.test.js @@ -1,45 +1,88 @@ import { readFile } from '@web/test-runner-commands'; import { expect } from '@esm-bundle/chai'; -import { stub } from 'sinon'; +import sinon from 'sinon'; import { setConfig, getConfig, createTag } from '../../../libs/utils/utils.js'; const { default: loadIcons } = await import('../../../libs/features/icons/icons.js'); -const codeRoot = '/libs'; -const conf = { codeRoot }; -setConfig(conf); +setConfig({ codeRoot: '/libs' }); const config = getConfig(); - -document.body.innerHTML = await readFile({ path: './mocks/body.html' }); - let icons; +let fetchStub; +let paramsGetStub; describe('Icon Support', () => { - let paramsGetStub; - - before(() => { - paramsGetStub = stub(URLSearchParams.prototype, 'get'); + beforeEach(async () => { + document.body.innerHTML = await readFile({ path: './mocks/body.html' }); + paramsGetStub = sinon.stub(URLSearchParams.prototype, 'get'); paramsGetStub.withArgs('cache').returns('off'); + fetchStub = sinon.stub(window, 'fetch'); + fetchStub.resolves({ + ok: true, + text: () => Promise.resolve(` + + + + `), + }); + window.lana.log = sinon.spy(); + icons = document.querySelectorAll('span.icon'); }); - after(() => { - paramsGetStub.restore(); + afterEach(() => { + document.body.innerHTML = ''; + sinon.restore(); }); - before(async () => { - document.body.innerHTML = await readFile({ path: './mocks/body.html' }); - icons = document.querySelectorAll('span.icon'); - await loadIcons(icons, config); + it('Falls back to Milo icons when federal icon fetch fails', async () => { + const icon = createTag('span', { class: 'icon icon-play' }); + document.body.appendChild(icon); + + fetchStub.callsFake((url) => { + if (url.includes('/federal/')) return Promise.reject(new Error('Failed to fetch federal icon')); + + return Promise.resolve({ + ok: true, + text: () => Promise.resolve(` + + + + + + `), + }); + }); + + await loadIcons([icon], config); + expect(icon.querySelector('svg')).to.exist; + }); + + it('Handles fetchIconList', async () => { + const { fetchIconList } = await import('../../../libs/features/icons/icons.js'); + + fetchStub.resolves({ + ok: true, + json: () => Promise.resolve({ content: { data: ['icon1', 'icon2'] } }), + }); + + const result = await fetchIconList('https://example.com/icons'); + expect(result).to.deep.equal(['icon1', 'icon2']); + + fetchStub.rejects(new Error('Network error')); + await fetchIconList('https://example.com/icons'); + expect(window.lana.log.calledOnce).to.be.true; + const lanaCallArguments = window.lana.log.getCall(0).args; + expect(lanaCallArguments[0]).to.include('Failed to fetch iconList'); + expect(lanaCallArguments[1].tags).to.equal('icons'); + expect(lanaCallArguments[1].errorType).to.equal('error'); }); - it('Handles tooltip- prefix correctly', async () => { + it('Handles tooltip prefix correctly', async () => { const tooltipIcon = createTag('span', { class: 'icon icon-tooltip-info' }); + document.body.appendChild(tooltipIcon); await loadIcons([tooltipIcon], config); - const svgIcon = tooltipIcon.querySelector(':scope svg'); - expect(svgIcon).to.exist; - - const iconName = tooltipIcon.classList[1].replace('icon-', '').replace(/tooltip-/, ''); - expect(iconName).to.equal('info'); + expect(tooltipIcon.querySelector(':scope svg')).to.exist; + expect(tooltipIcon.classList[1].replace('icon-tooltip-', '')).to.equal('info'); }); it('Fetches successfully with cache control enabled', async () => { @@ -50,7 +93,8 @@ describe('Icon Support', () => { expect(otherIcons[0].querySelector('svg')).to.exist; }); - it('Renders an SVG after loading the icons', () => { + it('Renders an SVG after loading the icons', async () => { + await loadIcons(icons, config); const selector = icons[0].querySelector(':scope svg'); expect(selector).to.exist; }); @@ -61,31 +105,84 @@ describe('Icon Support', () => { expect(svgs.length).to.equal(1); }); - it('Creates default tooltip (right-aligned)', () => { + it('Creates default tooltip (right-aligned)', async () => { + await loadIcons(icons, config); const tooltip = document.querySelector('.milo-tooltip.right'); expect(tooltip).to.exist; expect(tooltip.dataset.tooltip).to.equal('This is my tooltip text.'); }); - it('Creates top tooltip', () => { + it('Creates top tooltip', async () => { + const tooltipWrapper = createTag('em', {}, 'top|This is my tooltip text.'); + const tooltipIcon = createTag('span', { class: 'icon icon-tooltip' }); + tooltipWrapper.appendChild(tooltipIcon); + document.body.appendChild(tooltipWrapper); + + await loadIcons([tooltipIcon], config); const tooltip = document.querySelector('.milo-tooltip.top'); expect(tooltip).to.exist; }); + + it('Assigns correct node index classes', async () => { + const container = createTag('div'); + const icon1 = createTag('span', { class: 'icon icon-play' }); + const icon2 = createTag('span', { class: 'icon icon-play' }); + const icon3 = createTag('span', { class: 'icon icon-play' }); + + [icon1, icon2, icon3].forEach((i) => container.appendChild(i)); + document.body.appendChild(container); + + await loadIcons([icon1, icon2, icon3], config); + + expect(icon1.classList.contains('node-index-first')).to.be.true; + expect(icon2.classList.contains('node-index-middle')).to.be.true; + expect(icon3.classList.contains('node-index-last')).to.be.true; + }); + + it('Handles single icon node index class', async () => { + const container = createTag('div'); + const icon = createTag('span', { class: 'icon icon-play' }); + container.appendChild(icon); + document.body.appendChild(container); + + await loadIcons([icon], config); + expect(icon.classList.contains('node-index-only')).to.be.true; + }); + + it('Handles invalid SVG response', async () => { + const icon = createTag('span', { class: 'icon icon-invalid' }); + document.body.appendChild(icon); + + fetchStub.resolves({ + ok: true, + text: () => Promise.resolve('Definitely a SVG'), + }); + + await loadIcons([icon], config); + expect(window.lana.log.calledOnce).to.be.true; + const lanaCallArguments = window.lana.log.getCall(0).args; + expect(lanaCallArguments[0]).to.include('No SVG element found in fetched content for invalid'); + expect(lanaCallArguments[1].tags).to.equal('icons'); + expect(lanaCallArguments[1].errorType).to.equal('error'); + }); }); describe('Tooltip', () => { let iconTooltip; let wrapper; let normalIcon; + let container; beforeEach(() => { + container = createTag('div'); iconTooltip = createTag('span', { class: 'icon icon-tooltip' }); wrapper = createTag('em', {}, 'top|This is a tooltip text.'); wrapper.appendChild(iconTooltip); - document.body.appendChild(wrapper); + container.appendChild(wrapper); normalIcon = createTag('span', { class: 'icon icon-play' }); - document.body.appendChild(normalIcon); + container.appendChild(normalIcon); + document.body.appendChild(container); }); afterEach(() => { @@ -111,7 +208,6 @@ describe('Tooltip', () => { }); it('Tooltip should become visible on focus', async () => { - document.body.appendChild(wrapper); await loadIcons([iconTooltip], config); const tooltip = document.querySelector('.milo-tooltip'); expect(tooltip).to.exist; @@ -120,11 +216,11 @@ describe('Tooltip', () => { expect(tooltip.className).to.contain('top'); }); - it('Creates a tooltip without default alignment (left)', () => { + it('Creates a tooltip without default alignment (left)', async () => { const customTooltip = createTag('span', { class: 'icon icon-tooltip milo-tooltip left', 'data-tooltip': 'Left-aligned tooltip' }); - document.body.appendChild(customTooltip); + container.appendChild(customTooltip); - loadIcons([customTooltip], config); + await loadIcons([customTooltip], config); const tooltip = document.querySelector('.milo-tooltip.left'); expect(tooltip).to.exist; @@ -140,23 +236,23 @@ describe('Tooltip', () => { it('Should assign correct default icon class and name', async () => { await loadIcons([iconTooltip], config); - expect(iconTooltip.classList.contains('icon-info')).to.be.true; + expect(iconTooltip.classList.contains('icon-info-outline')).to.be.true; expect(iconTooltip.dataset.name).to.be.undefined; }); it('Should not assign default icon class if already defined', async () => { const customIcon = createTag('span', { class: 'icon icon-warning' }); - document.body.appendChild(customIcon); + container.appendChild(customIcon); await loadIcons([customIcon], config); expect(customIcon.classList.contains('icon-warning')).to.be.true; - expect(customIcon.classList.contains('icon-info')).to.be.false; + expect(customIcon.classList.contains('icon-info-outline')).to.be.false; }); it('Should not add tooltip if data-tooltip is missing', async () => { const noTooltipIcon = createTag('span', { class: 'icon icon-tooltip' }); - document.body.appendChild(noTooltipIcon); + container.appendChild(noTooltipIcon); await loadIcons([noTooltipIcon], config); 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 @@ -
+
+ +
From 9fcaf54ff284832b528b0a303495cade0372f7a5 Mon Sep 17 00:00:00 2001 From: Robert Bogos Date: Mon, 10 Mar 2025 17:39:39 +0200 Subject: [PATCH 02/11] cache fed icons for sidekick --- libs/blocks/library-config/lists/icons.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/libs/blocks/library-config/lists/icons.js b/libs/blocks/library-config/lists/icons.js index 9707a95e09..2b51fc22ad 100644 --- a/libs/blocks/library-config/lists/icons.js +++ b/libs/blocks/library-config/lists/icons.js @@ -2,8 +2,10 @@ import { fetchIconList } from '../../../features/icons/icons.js'; import { createTag } from '../../../utils/utils.js'; import createCopy from '../library-utils.js'; +let fedIconList; + export default async function iconList(content, list, query) { - const fedIconList = await fetchIconList(content[0].path); + if (!fedIconList) fedIconList = await fetchIconList(content[0].path); list.innerHTML = ''; if (fedIconList.length) { [...fedIconList].forEach((icon) => { From d30a8b0c9a843fb1bd06c107139a577dc3751a58 Mon Sep 17 00:00:00 2001 From: Robert Bogos Date: Tue, 18 Mar 2025 16:29:24 +0200 Subject: [PATCH 03/11] hotfix geo modal icon --- libs/styles/styles.css | 2 -- 1 file changed, 2 deletions(-) diff --git a/libs/styles/styles.css b/libs/styles/styles.css index b5258600ae..ec52bd00d2 100644 --- a/libs/styles/styles.css +++ b/libs/styles/styles.css @@ -571,8 +571,6 @@ div[data-failed="true"]::before { } span.icon { - width: 1em; - display: inline-block; margin-inline: var(--icon-spacing); } From 3389f578002884749f9d06d9a71eee0016f0cad2 Mon Sep 17 00:00:00 2001 From: Robert Bogos Date: Wed, 19 Mar 2025 10:18:21 +0200 Subject: [PATCH 04/11] refactored iconList function --- libs/blocks/library-config/lists/icons.js | 26 +++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/libs/blocks/library-config/lists/icons.js b/libs/blocks/library-config/lists/icons.js index 2b51fc22ad..8f132ae4e9 100644 --- a/libs/blocks/library-config/lists/icons.js +++ b/libs/blocks/library-config/lists/icons.js @@ -3,16 +3,18 @@ import { createTag } from '../../../utils/utils.js'; import createCopy from '../library-utils.js'; let fedIconList; +const iconElements = new Map(); export default async function iconList(content, list, query) { - if (!fedIconList) fedIconList = await fetchIconList(content[0].path); - list.innerHTML = ''; - if (fedIconList.length) { - [...fedIconList].forEach((icon) => { + if (!fedIconList) { + fedIconList = await fetchIconList(content[0].path); + if (!fedIconList?.length) { + throw new Error('No icons returned from fetchIconList'); + } + fedIconList.forEach((icon) => { const svg = createTag('span', { class: `icon icon-${icon.name}` }, createTag('img', { class: `icon-${icon.name}-img icon-fed`, src: `${icon.url}`, width: '18px' })); const titleText = createTag('p', { class: 'item-title' }, icon.name); const title = createTag('li', { class: 'icon-item' }, svg); - if (query && !icon.name.includes(query)) title.classList.add('is-hidden'); title.append(titleText); const copy = createTag('button', { class: 'copy' }); copy.id = `${icon.name}-icon-copy`; @@ -25,7 +27,19 @@ export default async function iconList(content, list, query) { window.hlx?.rum.sampleRUM('click', { source: e.target }); }); title.append(copy); - list.append(title); + + iconElements.set(icon.name, title); }); } + + list.replaceChildren(); + + iconElements.forEach((element, name) => { + if (!query || name.includes(query)) { + element.classList.remove('is-hidden'); + } else { + element.classList.add('is-hidden'); + } + list.appendChild(element); + }); } From f5a7768a3a98c09a355606153ae59bcf181c2951 Mon Sep 17 00:00:00 2001 From: Robert Bogos Date: Wed, 19 Mar 2025 11:04:39 +0200 Subject: [PATCH 05/11] hotfix --- libs/features/icons/icons.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/features/icons/icons.js b/libs/features/icons/icons.js index 17fa5bdec6..53203ec8e5 100644 --- a/libs/features/icons/icons.js +++ b/libs/features/icons/icons.js @@ -119,7 +119,7 @@ export default async function loadIcons(icons) { const parent = icon.parentElement; if (parent && parent.parentElement.tagName === 'LI') parent.parentElement.classList.add('icon-list-item'); }); - await Promise.all(iconRequests); + await Promise.allSettled(iconRequests); icons.forEach((icon) => { const iconName = [...icon.classList].find((c) => c.startsWith('icon-'))?.substring(5); if (iconName && federalIcons[iconName] && !icon.dataset.svgInjected) { From f1fa896ea2e57ad7c3c4c389ff7f439bc264f53b Mon Sep 17 00:00:00 2001 From: Robert Bogos Date: Wed, 19 Mar 2025 11:15:55 +0200 Subject: [PATCH 06/11] removed unnecessary CSS --- libs/styles/styles.css | 7 ------- 1 file changed, 7 deletions(-) diff --git a/libs/styles/styles.css b/libs/styles/styles.css index ec52bd00d2..fe279e0f72 100644 --- a/libs/styles/styles.css +++ b/libs/styles/styles.css @@ -579,13 +579,6 @@ 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; } -span.icon svg { - height: 1em; - position: relative; - top: .1em; - width: auto; -} - /* Con Block Utils */ .con-block.xs-spacing { padding: var(--spacing-xs) 0; } .con-block.s-spacing { padding: var(--spacing-s) 0; } From a478af3564b08584809130b6ecd5844d8db7942c Mon Sep 17 00:00:00 2001 From: Robert Bogos Date: Thu, 12 Jun 2025 15:56:20 +0300 Subject: [PATCH 07/11] fixed race condition when falling back to milo icons --- libs/features/icons/icons.js | 211 ++++++++++++++++++++--------------- 1 file changed, 122 insertions(+), 89 deletions(-) diff --git a/libs/features/icons/icons.js b/libs/features/icons/icons.js index 53203ec8e5..556193149b 100644 --- a/libs/features/icons/icons.js +++ b/libs/features/icons/icons.js @@ -2,21 +2,53 @@ import { lanaLog } from '../../blocks/global-navigation/utilities/utilities.js'; import { getFederatedContentRoot } from '../../utils/federated.js'; import { getConfig } from '../../utils/utils.js'; -let fetchedIcons; -let fetched = false; -const federalIcons = {}; +const iconCache = new Map(); +let miloIconsPromise; + +function setNodeIndexClass(icon) { + const children = icon.parentNode.childNodes; + const nodeIndex = Array.prototype.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}`); +} + +function decorateToolTip(icon) { + const wrapper = icon.closest('em'); + if (!wrapper) return; + + wrapper.className = 'tooltip-wrapper'; + const conf = wrapper.textContent.split('|'); + const content = conf.pop()?.trim(); + if (!content) return; + + icon.dataset.tooltip = content; + const place = conf.pop()?.trim().toLowerCase() || 'right'; + icon.className = `icon icon-info-outline milo-tooltip ${place}`; + + [['tabindex', '0'], ['aria-label', content], ['role', 'button']].forEach(([attr, value]) => { + icon.setAttribute(attr, value); + }); + + wrapper.parentElement.replaceChild(icon, wrapper); +} + +function getIconNameFromElement(element) { + return [...element.classList].find((c) => c.startsWith('icon-'))?.substring(5); +} async function getSVGsfromFile(path) { - /* c8 ignore next */ if (!path) return null; const resp = await fetch(path); - /* c8 ignore next */ if (!resp.ok) return null; + const miloIcons = {}; const text = await resp.text(); const parser = new DOMParser(); const parsedText = parser.parseFromString(text, 'image/svg+xml'); const symbols = parsedText.querySelectorAll('symbol'); + symbols.forEach((symbol) => { const svg = document.createElementNS('http://www.w3.org/2000/svg', 'svg'); while (symbol.firstChild) svg.appendChild(symbol.firstChild); @@ -24,108 +56,109 @@ async function getSVGsfromFile(path) { svg.classList.add('icon-milo', `icon-milo-${svg.id}`); miloIcons[svg.id] = svg; }); + 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 */ - if (!fetched) { - const { miloLibs, codeRoot } = config; - const base = miloLibs || codeRoot; - fetchedIcons = await getSVGsfromFile(`${base}/img/icons/icons.svg`); - fetched = true; - } - resolve(fetchedIcons); -}); +async function fetchAndParseSVG(url, iconName) { + const response = await fetch(url); + if (!response.ok) throw new Error(`Failed to fetch SVG for ${iconName}: ${response.statusText}`); -export function fetchIconList(url) { - return fetch(url) - .then((resp) => resp.json()) - .then((json) => json.content.data) - .catch(() => { - lanaLog({ message: 'Failed to fetch iconList', tags: 'icons', errorType: 'error' }); - return []; + const text = await response.text(); + const parser = new DOMParser(); + const svgElement = parser.parseFromString(text, 'image/svg+xml').querySelector('svg'); + + if (!svgElement) throw new Error(`No SVG element found in fetched content for ${iconName}`); + + svgElement.classList.add('icon-milo', `icon-milo-${iconName}`); + return svgElement; +} + +async function fetchFederalIcon(iconName) { + const fedRoot = getFederatedContentRoot(); + const url = `${fedRoot}/federal/assets/icons/svgs/${iconName}.svg`; + + try { + const svgElement = await fetchAndParseSVG(url, iconName); + iconCache.set(iconName, svgElement); + return svgElement; + } catch (error) { + lanaLog({ + message: `Error fetching federal SVG for ${iconName}, falling back to Milo icon`, + error, + tags: 'icons', + errorType: 'error', }); + return null; + } } -async function decorateToolTip(icon) { - const wrapper = icon.closest('em'); - 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(); - if (!content) return; - 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-outline milo-tooltip ${place}`; - [['tabindex', '0'], ['aria-label', content], ['role', 'button']].forEach(([attr, value]) => { - icon.setAttribute(attr, value); +async function fetchMiloIcon(iconName) { + if (!miloIconsPromise) { + const { miloLibs, codeRoot } = getConfig(); + const base = miloLibs || codeRoot; + miloIconsPromise = getSVGsfromFile(`${base}/img/icons/icons.svg`); + } + + const miloIcons = await miloIconsPromise; + if (miloIcons?.[iconName]) { + const icon = miloIcons[iconName].cloneNode(true); + iconCache.set(iconName, icon); + return icon; + } + + lanaLog({ + message: `No fallback Milo icon found for ${iconName}`, + tags: 'icons', + errorType: 'error', }); - wrapper.parentElement.replaceChild(icon, wrapper); + return null; } -export function setNodeIndexClass(icon) { - const children = icon.parentNode.childNodes; - const nodeIndex = Array.prototype.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 getIcon(iconName) { + if (iconCache.has(iconName)) return iconCache.get(iconName); + + const federalIcon = await fetchFederalIcon(iconName); + if (federalIcon) return federalIcon; + + return fetchMiloIcon(iconName); } export default async function loadIcons(icons) { - const fedRoot = getFederatedContentRoot(); - const iconRequests = []; - const iconsToFetch = new Map(); - icons.forEach((icon) => { + const iconPromises = [...icons].map(async (icon) => { setNodeIndexClass(icon); + if (icon.classList.contains('icon-tooltip')) decorateToolTip(icon); - const iconName = [...icon.classList].find((c) => c.startsWith('icon-'))?.substring(5); + + const iconName = getIconNameFromElement(icon); if (icon.dataset.svgInjected || !iconName) return; - if (!federalIcons[iconName] && !iconsToFetch.has(iconName)) { - iconsToFetch.set(iconName, fetch(`${fedRoot}/federal/assets/icons/svgs/${iconName}.svg`) - .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 svgElement = parser.parseFromString(text, 'image/svg+xml').querySelector('svg'); - if (!svgElement) { - lanaLog({ message: `No SVG element found in fetched content for ${iconName}`, tags: 'icons', errorType: 'error' }); - return; - } - svgElement.classList.add('icon-milo', `icon-milo-${iconName}`); - federalIcons[iconName] = svgElement; - }) - .catch(async (e) => { - lanaLog({ message: `Error fetching federal SVG for ${iconName}, falling back to Milo icon`, e, tags: 'icons', errorType: 'error' }); - // Fallback to Milo icons - if (!fetchedIcons) { - const { miloLibs, codeRoot } = getConfig(); - const base = miloLibs || codeRoot; - fetchedIcons = await getSVGsfromFile(`${base}/img/icons/icons.svg`); - } - if (fetchedIcons?.[iconName]) { - federalIcons[iconName] = fetchedIcons[iconName].cloneNode(true); - return; - } - lanaLog({ message: `No fallback Milo icon found for ${iconName}`, e, tags: 'icons', errorType: 'error' }); - })); - } - iconRequests.push(iconsToFetch.get(iconName)); - const parent = icon.parentElement; - if (parent && parent.parentElement.tagName === 'LI') parent.parentElement.classList.add('icon-list-item'); - }); - await Promise.allSettled(iconRequests); - icons.forEach((icon) => { - const iconName = [...icon.classList].find((c) => c.startsWith('icon-'))?.substring(5); - if (iconName && federalIcons[iconName] && !icon.dataset.svgInjected) { - const svgClone = federalIcons[iconName].cloneNode(true); + + const svgElement = await getIcon(iconName); + if (svgElement && !icon.dataset.svgInjected) { + const svgClone = svgElement.cloneNode(true); icon.appendChild(svgClone); icon.dataset.svgInjected = 'true'; + + const parent = icon.parentElement; + if (parent?.parentElement?.tagName === 'LI') parent.parentElement.classList.add('icon-list-item'); } }); + + await Promise.allSettled(iconPromises); +} + +export const fetchIcons = (config) => { + const { miloLibs, codeRoot } = config; + const base = miloLibs || codeRoot; + return getSVGsfromFile(`${base}/img/icons/icons.svg`); +}; + +export function fetchIconList(url) { + return fetch(url) + .then((resp) => resp.json()) + .then((json) => json.content.data) + .catch(() => { + lanaLog({ message: 'Failed to fetch iconList', tags: 'icons', errorType: 'error' }); + return []; + }); } From 9c21b3134e1d653e9ea31d75c195c5e7517c2033 Mon Sep 17 00:00:00 2001 From: Robert Bogos Date: Thu, 19 Jun 2025 15:25:23 +0300 Subject: [PATCH 08/11] addressed feedback --- libs/blocks/library-config/lists/icons.js | 10 ++-------- libs/styles/styles.css | 3 --- 2 files changed, 2 insertions(+), 11 deletions(-) diff --git a/libs/blocks/library-config/lists/icons.js b/libs/blocks/library-config/lists/icons.js index 8f132ae4e9..ea5abe5ccd 100644 --- a/libs/blocks/library-config/lists/icons.js +++ b/libs/blocks/library-config/lists/icons.js @@ -8,9 +8,7 @@ const iconElements = new Map(); export default async function iconList(content, list, query) { if (!fedIconList) { fedIconList = await fetchIconList(content[0].path); - if (!fedIconList?.length) { - throw new Error('No icons returned from fetchIconList'); - } + if (!fedIconList?.length) throw new Error('No icons returned from fetchIconList'); fedIconList.forEach((icon) => { const svg = createTag('span', { class: `icon icon-${icon.name}` }, createTag('img', { class: `icon-${icon.name}-img icon-fed`, src: `${icon.url}`, width: '18px' })); const titleText = createTag('p', { class: 'item-title' }, icon.name); @@ -35,11 +33,7 @@ export default async function iconList(content, list, query) { list.replaceChildren(); iconElements.forEach((element, name) => { - if (!query || name.includes(query)) { - element.classList.remove('is-hidden'); - } else { - element.classList.add('is-hidden'); - } + element.classList.toggle('is-hidden', query && !name.includes(query)); list.appendChild(element); }); } diff --git a/libs/styles/styles.css b/libs/styles/styles.css index 09463b7868..cc74c36525 100644 --- a/libs/styles/styles.css +++ b/libs/styles/styles.css @@ -359,7 +359,6 @@ line-height: 20px; min-height: 21px; padding: 7px 18px 8px; - --icon-spacing: 12px; } .xl-button .con-button, @@ -369,7 +368,6 @@ line-height: 24px; min-height: 28px; padding: 10px 24px 8px; - --icon-spacing: 14px; } .xxl-button .con-button, @@ -379,7 +377,6 @@ line-height: 27px; min-height: 27px; padding: 14px 30px 15px; - --icon-spacing: 14px; } .con-button.button-justified { From 405eaaa794086eaed27f89334aa9c6fb99c36edc Mon Sep 17 00:00:00 2001 From: Robert Bogos Date: Thu, 19 Jun 2025 16:03:34 +0300 Subject: [PATCH 09/11] fixed unit tests --- test/features/icons/icons.test.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/features/icons/icons.test.js b/test/features/icons/icons.test.js index 27d95381bb..71ef5dcecc 100644 --- a/test/features/icons/icons.test.js +++ b/test/features/icons/icons.test.js @@ -159,9 +159,9 @@ describe('Icon Support', () => { }); await loadIcons([icon], config); - expect(window.lana.log.calledOnce).to.be.true; - const lanaCallArguments = window.lana.log.getCall(0).args; - expect(lanaCallArguments[0]).to.include('No SVG element found in fetched content for invalid'); + expect(window.lana.log.getCalls().length > 0).to.be.true; + const lanaCallArguments = window.lana.log.getCall(1).args; + expect(lanaCallArguments[0]).to.include('No fallback Milo icon found for invalid'); expect(lanaCallArguments[1].tags).to.equal('icons'); expect(lanaCallArguments[1].errorType).to.equal('error'); }); From ae77f4b30e4f0f447c0d242175bc7c5a580aa273 Mon Sep 17 00:00:00 2001 From: Robert Bogos Date: Tue, 8 Jul 2025 15:17:25 +0300 Subject: [PATCH 10/11] addressed QA feedback --- libs/blocks/table/table.css | 1 - libs/blocks/text/text.css | 7 ++++- libs/features/georoutingv2/georoutingv2.js | 2 +- libs/features/icons/icons.js | 31 +++++++++++++--------- libs/styles/styles.css | 29 +++++++++++--------- test/features/icons/icons.test.js | 4 +-- 6 files changed, 45 insertions(+), 29 deletions(-) diff --git a/libs/blocks/table/table.css b/libs/blocks/table/table.css index 7cb536e7cf..ecf7f398ed 100644 --- a/libs/blocks/table/table.css +++ b/libs/blocks/table/table.css @@ -375,7 +375,6 @@ cursor: pointer; rotate: -90deg; background-size: contain; - margin-inline: unset; } [dir="rtl"] .table .icon.expand { diff --git a/libs/blocks/text/text.css b/libs/blocks/text/text.css index c85b390dfc..109f9c372e 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.node-index-first { +.text-block .icon-list-item .icon.margin-right:not(.margin-left) { /* target first node only */ position: absolute; inset: 0 100% auto auto; } @@ -122,6 +122,7 @@ .text-block .icon-area:not(:has(span.icon)) { display: flex; + column-gap: var(--spacing-xs); } .text-block p.icon-area { /* NOT tags with icons in them */ @@ -216,6 +217,10 @@ 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.js b/libs/features/georoutingv2/georoutingv2.js index 0fe66fa055..1bc81c01b0 100644 --- a/libs/features/georoutingv2/georoutingv2.js +++ b/libs/features/georoutingv2/georoutingv2.js @@ -236,7 +236,7 @@ function buildContent(currentPage, locale, geoData, locales) { { once: true }, ); img.src = `${config.miloLibs || config.codeRoot}/img/georouting/${flagFile}`; - const span = createTag('span', { class: 'icon node-index-first' }, img); + const span = createTag('span', { class: 'icon margin-inline-end' }, 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 2d10c2345f..3c501d918e 100644 --- a/libs/features/icons/icons.js +++ b/libs/features/icons/icons.js @@ -33,10 +33,11 @@ function addTooltipListeners() { }); } -function decorateToolTip(icon) { - const wrapper = icon.closest('em'); - if (!wrapper) return; +function decorateToolTip(icon, iconName) { + const hasTooltip = icon.closest('em')?.textContent.includes('|') && [...icon.classList].some((cls) => cls.includes('tooltip')); + if (!hasTooltip) return; + const wrapper = icon.closest('em'); wrapper.className = 'tooltip-wrapper'; const conf = wrapper.textContent.split('|'); const content = conf.pop()?.trim(); @@ -44,7 +45,7 @@ function decorateToolTip(icon) { icon.dataset.tooltip = content; const place = conf.pop()?.trim().toLowerCase() || 'right'; - icon.className = `icon icon-info-outline milo-tooltip ${place}`; + icon.className = `icon icon-${iconName} milo-tooltip ${place}`; [['tabindex', '0'], ['aria-label', content], ['role', 'button']].forEach(([attr, value]) => { icon.setAttribute(attr, value); @@ -54,10 +55,6 @@ function decorateToolTip(icon) { if (!tooltipListenersAdded) addTooltipListeners(); } -function getIconNameFromElement(element) { - return [...element.classList].find((c) => c.startsWith('icon-'))?.substring(5); -} - async function getSVGsfromFile(path) { if (!path) return null; const resp = await fetch(path); @@ -148,9 +145,10 @@ export default async function loadIcons(icons) { const iconPromises = [...icons].map(async (icon) => { setNodeIndexClass(icon); - if (icon.classList.contains('icon-tooltip')) decorateToolTip(icon); - - const iconName = getIconNameFromElement(icon); + const iconNameInitial = icon.classList[1].replace('icon-', ''); + let iconName = iconNameInitial === 'tooltip' ? 'info' : iconNameInitial; + if (iconNameInitial.includes('tooltip-')) iconName = iconNameInitial.replace(/tooltip-/, ''); + decorateToolTip(icon, iconName); if (icon.dataset.svgInjected || !iconName) return; const svgElement = await getIcon(iconName); @@ -160,7 +158,16 @@ export default async function loadIcons(icons) { icon.dataset.svgInjected = 'true'; const parent = icon.parentElement; - if (parent?.parentElement?.tagName === 'LI') parent.parentElement.classList.add('icon-list-item'); + 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'); + } + } } }); diff --git a/libs/styles/styles.css b/libs/styles/styles.css index cc74c36525..d3f91484ba 100644 --- a/libs/styles/styles.css +++ b/libs/styles/styles.css @@ -1,6 +1,6 @@ /*** Variables ***/ - :root { +:root { --global-height-nav: 64px; --feds-height-nav: 63px; --global-height-breadcrumbs: 33px; @@ -131,10 +131,6 @@ --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 */ } :root:lang(ar) { @@ -569,14 +565,23 @@ div[data-failed="true"]::before { color: var(--color-gray-300); } -span.icon { - margin-inline: var(--icon-spacing); -} +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.milo-tooltip.margin-inline-end { margin-inline-end: 3px; } + +span.icon.milo-tooltip.margin-inline-start { margin-inline-start: 3px; } + +.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; } /* Con Block Utils */ .con-block.xxs-spacing { padding: var(--spacing-xxs) 0; } diff --git a/test/features/icons/icons.test.js b/test/features/icons/icons.test.js index 71ef5dcecc..0f508cb4cc 100644 --- a/test/features/icons/icons.test.js +++ b/test/features/icons/icons.test.js @@ -236,7 +236,7 @@ describe('Tooltip', () => { it('Should assign correct default icon class and name', async () => { await loadIcons([iconTooltip], config); - expect(iconTooltip.classList.contains('icon-info-outline')).to.be.true; + expect(iconTooltip.classList.contains('icon-info')).to.be.true; expect(iconTooltip.dataset.name).to.be.undefined; }); @@ -247,7 +247,7 @@ describe('Tooltip', () => { await loadIcons([customIcon], config); expect(customIcon.classList.contains('icon-warning')).to.be.true; - expect(customIcon.classList.contains('icon-info-outline')).to.be.false; + expect(customIcon.classList.contains('icon-info')).to.be.false; }); it('Should not add tooltip if data-tooltip is missing', async () => { From e83c8478308961041a8312ff5941900409f8859e Mon Sep 17 00:00:00 2001 From: Robert Bogos Date: Tue, 8 Jul 2025 15:58:56 +0300 Subject: [PATCH 11/11] removed node index classes --- libs/features/icons/icons.js | 11 ----------- test/features/icons/icons.test.js | 26 -------------------------- 2 files changed, 37 deletions(-) diff --git a/libs/features/icons/icons.js b/libs/features/icons/icons.js index 3c501d918e..1aa816bb60 100644 --- a/libs/features/icons/icons.js +++ b/libs/features/icons/icons.js @@ -5,15 +5,6 @@ import { getConfig } from '../../utils/utils.js'; const iconCache = new Map(); let miloIconsPromise; -function setNodeIndexClass(icon) { - const children = icon.parentNode.childNodes; - const nodeIndex = Array.prototype.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}`); -} - let tooltipListenersAdded = false; function addTooltipListeners() { tooltipListenersAdded = true; @@ -143,8 +134,6 @@ async function getIcon(iconName) { export default async function loadIcons(icons) { const iconPromises = [...icons].map(async (icon) => { - setNodeIndexClass(icon); - const iconNameInitial = icon.classList[1].replace('icon-', ''); let iconName = iconNameInitial === 'tooltip' ? 'info' : iconNameInitial; if (iconNameInitial.includes('tooltip-')) iconName = iconNameInitial.replace(/tooltip-/, ''); diff --git a/test/features/icons/icons.test.js b/test/features/icons/icons.test.js index 0f508cb4cc..da4ff76c3e 100644 --- a/test/features/icons/icons.test.js +++ b/test/features/icons/icons.test.js @@ -123,32 +123,6 @@ describe('Icon Support', () => { expect(tooltip).to.exist; }); - it('Assigns correct node index classes', async () => { - const container = createTag('div'); - const icon1 = createTag('span', { class: 'icon icon-play' }); - const icon2 = createTag('span', { class: 'icon icon-play' }); - const icon3 = createTag('span', { class: 'icon icon-play' }); - - [icon1, icon2, icon3].forEach((i) => container.appendChild(i)); - document.body.appendChild(container); - - await loadIcons([icon1, icon2, icon3], config); - - expect(icon1.classList.contains('node-index-first')).to.be.true; - expect(icon2.classList.contains('node-index-middle')).to.be.true; - expect(icon3.classList.contains('node-index-last')).to.be.true; - }); - - it('Handles single icon node index class', async () => { - const container = createTag('div'); - const icon = createTag('span', { class: 'icon icon-play' }); - container.appendChild(icon); - document.body.appendChild(container); - - await loadIcons([icon], config); - expect(icon.classList.contains('node-index-only')).to.be.true; - }); - it('Handles invalid SVG response', async () => { const icon = createTag('span', { class: 'icon icon-invalid' }); document.body.appendChild(icon);