From e82c905ec8d69fbb42862429ded1215c71f539c1 Mon Sep 17 00:00:00 2001 From: Vjacheslav Trushkin Date: Sat, 8 Oct 2022 13:38:53 +0300 Subject: [PATCH] fix: better way to detect previously rendered child node in web component --- iconify-icon/icon/src/render/icon.ts | 18 ++++++++----- iconify-icon/icon/src/render/style.ts | 21 +++++++++++---- iconify-icon/icon/src/tests/helpers.ts | 7 +++++ iconify-icon/icon/tests/component-api-test.ts | 13 ++++----- iconify-icon/icon/tests/component-test.ts | 27 ++++++++++--------- iconify-icon/icon/tests/render-icon-test.ts | 11 ++++---- iconify-icon/icon/tests/render-style-test.ts | 17 +++++++++--- 7 files changed, 76 insertions(+), 38 deletions(-) diff --git a/iconify-icon/icon/src/render/icon.ts b/iconify-icon/icon/src/render/icon.ts index 7e31d3e..d524883 100644 --- a/iconify-icon/icon/src/render/icon.ts +++ b/iconify-icon/icon/src/render/icon.ts @@ -37,16 +37,22 @@ export function renderIcon(parent: Element | ShadowRoot, state: RenderedState) { } // Set element - // Assumes first node is a style node created with updateStyle() - if (parent.childNodes.length > 1) { - const lastChild = parent.lastChild as HTMLElement; - if (node.tagName === 'SPAN' && lastChild.tagName === node.tagName) { + const oldNode = Array.from(parent.childNodes).find((node) => { + const tag = + (node as HTMLElement).tagName && + (node as HTMLElement).tagName.toUpperCase(); + return tag === 'SPAN' || tag === 'SVG'; + }) as HTMLElement | undefined; + if (oldNode) { + // Replace old element + if (node.tagName === 'SPAN' && oldNode.tagName === node.tagName) { // Swap style instead of whole node - lastChild.setAttribute('style', node.getAttribute('style')); + oldNode.setAttribute('style', node.getAttribute('style')); } else { - parent.replaceChild(node, lastChild); + parent.replaceChild(node, oldNode); } } else { + // Add new element parent.appendChild(node); } } diff --git a/iconify-icon/icon/src/render/style.ts b/iconify-icon/icon/src/render/style.ts index 2e0370b..cef6ea4 100644 --- a/iconify-icon/icon/src/render/style.ts +++ b/iconify-icon/icon/src/render/style.ts @@ -1,16 +1,27 @@ +/** + * Attribute to add + */ +const nodeAttr = 'data-style'; + /** * Add/update style node */ export function updateStyle(parent: Element | ShadowRoot, inline: boolean) { // Get node, create if needed - let style = parent.firstChild; - if (!style) { - style = document.createElement('style'); - parent.appendChild(style); + let styleNode = Array.from(parent.childNodes).find( + (node) => + (node as HTMLElement).hasAttribute && + (node as HTMLElement).hasAttribute(nodeAttr) + ) as HTMLElement | undefined; + + if (!styleNode) { + styleNode = document.createElement('style'); + styleNode.setAttribute(nodeAttr, nodeAttr); + parent.appendChild(styleNode); } // Update content - style.textContent = + styleNode.textContent = ':host{display:inline-block;vertical-align:' + (inline ? '-0.125em' : '0') + '}span,svg{display:block}'; diff --git a/iconify-icon/icon/src/tests/helpers.ts b/iconify-icon/icon/src/tests/helpers.ts index 810427a..efe88b4 100644 --- a/iconify-icon/icon/src/tests/helpers.ts +++ b/iconify-icon/icon/src/tests/helpers.ts @@ -3,6 +3,13 @@ import { mockAPIModule, mockAPIData } from '@iconify/core/lib/api/modules/mock'; import { addAPIProvider } from '@iconify/core/lib/api/config'; import { setAPIModule } from '@iconify/core/lib/api/modules'; +/** + * ` + `${styleOpeningTag}${expectedBlock}` ); expect(node.status).toBe('loading'); @@ -78,7 +79,7 @@ describe('Testing icon component with API', () => { // Should not have sent query to API yet expect(sendQuery).toBeUndefined(); expect(node._shadowRoot.innerHTML).toBe( - `` + `${styleOpeningTag}${expectedBlock}` ); expect(node.status).toBe('loading'); @@ -97,7 +98,7 @@ describe('Testing icon component with API', () => { const blankSVG = ''; expect(node._shadowRoot.innerHTML).toBe( - `${blankSVG}` + `${styleOpeningTag}${expectedBlock}${blankSVG}` ); expect(node.status).toBe('rendered'); }); @@ -121,7 +122,7 @@ describe('Testing icon component with API', () => { // Should be empty expect(node._shadowRoot.innerHTML).toBe( - `` + `${styleOpeningTag}${expectedBlock}` ); expect(node.status).toBe('loading'); @@ -147,7 +148,7 @@ describe('Testing icon component with API', () => { // Should not have sent query to API yet expect(node._shadowRoot.innerHTML).toBe( - `` + `${styleOpeningTag}${expectedBlock}` ); expect(node.status).toBe('loading'); @@ -157,7 +158,7 @@ describe('Testing icon component with API', () => { // Should fail to render expect(node._shadowRoot.innerHTML).toBe( - `` + `${styleOpeningTag}${expectedBlock}` ); expect(node.status).toBe('failed'); }); diff --git a/iconify-icon/icon/tests/component-test.ts b/iconify-icon/icon/tests/component-test.ts index 7fc74ab..46169e8 100644 --- a/iconify-icon/icon/tests/component-test.ts +++ b/iconify-icon/icon/tests/component-test.ts @@ -4,6 +4,7 @@ import { expectedInline, setupDOM, nextTick, + styleOpeningTag, } from '../src/tests/helpers'; import { defineIconifyIcon, IconifyIconHTMLElement } from '../src/component'; import type { IconState } from '../src/state'; @@ -73,7 +74,7 @@ describe('Testing icon component', () => { // Should be empty expect(node._shadowRoot.innerHTML).toBe( - `` + `${styleOpeningTag}${expectedBlock}` ); expect(node.status).toBe('loading'); @@ -96,7 +97,7 @@ describe('Testing icon component', () => { // Should still be empty: waiting for next tick expect(node._shadowRoot.innerHTML).toBe( - `` + `${styleOpeningTag}${expectedBlock}` ); expect(node.status).toBe('loading'); await nextTick(); @@ -105,7 +106,7 @@ describe('Testing icon component', () => { const blankSVG = ''; expect(node._shadowRoot.innerHTML).toBe( - `${blankSVG}` + `${styleOpeningTag}${expectedBlock}${blankSVG}` ); expect(node.status).toBe('rendered'); @@ -119,7 +120,7 @@ describe('Testing icon component', () => { expect(node.getAttribute('inline')).toBe('true'); expect(node._shadowRoot.innerHTML).toBe( - `${blankSVG}` + `${styleOpeningTag}${expectedInline}${blankSVG}` ); expect(node.status).toBe('rendered'); }); @@ -145,7 +146,7 @@ describe('Testing icon component', () => { // Should be empty with block style expect(node._shadowRoot.innerHTML).toBe( - `` + `${styleOpeningTag}${expectedBlock}` ); // Check inline @@ -157,7 +158,7 @@ describe('Testing icon component', () => { node.inline = true; expect(node._shadowRoot.innerHTML).toBe( - `` + `${styleOpeningTag}${expectedInline}` ); expect(node.inline).toBe(true); expect(node.hasAttribute('inline')).toBe(true); @@ -167,7 +168,7 @@ describe('Testing icon component', () => { node.removeAttribute('inline'); expect(node._shadowRoot.innerHTML).toBe( - `` + `${styleOpeningTag}${expectedBlock}` ); expect(node.inline).toBe(false); expect(node.hasAttribute('inline')).toBe(false); @@ -177,7 +178,7 @@ describe('Testing icon component', () => { node.setAttribute('inline', 'inline'); expect(node._shadowRoot.innerHTML).toBe( - `` + `${styleOpeningTag}${expectedInline}` ); expect(node.inline).toBe(true); expect(node.hasAttribute('inline')).toBe(true); @@ -229,7 +230,7 @@ describe('Testing icon component', () => { ""; const html1 = node._shadowRoot.innerHTML; expect(html1.replace(/-- [0-9]+ --/, '-- --')).toBe( - `${renderedIconWithComment}` + `${styleOpeningTag}${expectedBlock}${renderedIconWithComment}` ); // Restart animation, test icon again @@ -238,7 +239,7 @@ describe('Testing icon component', () => { const html2 = node._shadowRoot.innerHTML; expect(html2).not.toBe(html1); expect(html2.replace(/-- [0-9]+ --/, '-- --')).toBe( - `${renderedIconWithComment}` + `${styleOpeningTag}${expectedBlock}${renderedIconWithComment}` ); expect(node.status).toBe('rendered'); @@ -252,7 +253,7 @@ describe('Testing icon component', () => { const html3 = node._shadowRoot.innerHTML; expect(html3.replace(/-- [0-9]+ --/, '-- --')).toBe( - `${renderedIconWithComment}` + `${styleOpeningTag}${expectedBlock}${renderedIconWithComment}` ); expect(html3).not.toBe(html1); expect(html3).not.toBe(html2); @@ -306,7 +307,9 @@ describe('Testing icon component', () => { const html1 = node._shadowRoot.innerHTML; const svg1 = node._shadowRoot.lastChild as SVGSVGElement; const setCurrentTimeSupported = !!svg1.setCurrentTime; - expect(html1).toBe(`${renderedIcon}`); + expect(html1).toBe( + `${styleOpeningTag}${expectedBlock}${renderedIcon}` + ); expect(svg1.outerHTML).toBe(renderedIcon); // Restart animation, test icon again diff --git a/iconify-icon/icon/tests/render-icon-test.ts b/iconify-icon/icon/tests/render-icon-test.ts index c090843..69b4d08 100644 --- a/iconify-icon/icon/tests/render-icon-test.ts +++ b/iconify-icon/icon/tests/render-icon-test.ts @@ -4,6 +4,7 @@ import { expectedBlock, expectedInline, setupDOM, + styleOpeningTag, } from '../src/tests/helpers'; import { updateStyle } from '../src/render/style'; import { renderIcon } from '../src/render/icon'; @@ -39,7 +40,7 @@ describe('Testing rendering loaded icon', () => { // Test HTML expect(node.innerHTML).toBe( - `` + `${styleOpeningTag}${expectedBlock}` ); // Replace icon content @@ -65,7 +66,7 @@ describe('Testing rendering loaded icon', () => { // Test HTML expect(node.innerHTML).toBe( - `` + `${styleOpeningTag}${expectedBlock}` ); }); @@ -97,7 +98,7 @@ describe('Testing rendering loaded icon', () => { // Test HTML expect(node.innerHTML).toBe( - `` + `${styleOpeningTag}${expectedBlock}` ); }); @@ -128,7 +129,7 @@ describe('Testing rendering loaded icon', () => { // Test HTML expect(node.innerHTML).toBe( - `` + `${styleOpeningTag}${expectedInline}` ); // Change mode to background, add some customisations @@ -151,7 +152,7 @@ describe('Testing rendering loaded icon', () => { // Test HTML expect(node.innerHTML).toBe( - `` + `${styleOpeningTag}${expectedInline}` ); }); }); diff --git a/iconify-icon/icon/tests/render-style-test.ts b/iconify-icon/icon/tests/render-style-test.ts index 344a88a..cffe278 100644 --- a/iconify-icon/icon/tests/render-style-test.ts +++ b/iconify-icon/icon/tests/render-style-test.ts @@ -4,6 +4,7 @@ import { expectedBlock, expectedInline, setupDOM, + styleOpeningTag, } from '../src/tests/helpers'; describe('Testing rendering style', () => { @@ -18,18 +19,26 @@ describe('Testing rendering style', () => { // Add style to empty parent updateStyle(node, false); - expect(node.innerHTML).toBe(''); + expect(node.innerHTML).toBe( + styleOpeningTag + expectedBlock + '' + ); // Change inline mode updateStyle(node, true); - expect(node.innerHTML).toBe(''); + expect(node.innerHTML).toBe( + styleOpeningTag + expectedInline + '' + ); // Do not change anything updateStyle(node, true); - expect(node.innerHTML).toBe(''); + expect(node.innerHTML).toBe( + styleOpeningTag + expectedInline + '' + ); // Change to block updateStyle(node, false); - expect(node.innerHTML).toBe(''); + expect(node.innerHTML).toBe( + styleOpeningTag + expectedBlock + '' + ); }); });