From edb69ad7e080f22487a76caccbd4d7b1e2b03f07 Mon Sep 17 00:00:00 2001 From: Konnor Rogers Date: Mon, 23 Sep 2024 13:37:57 -0400 Subject: [PATCH] fix multiple identical spritesheet icons not applying mutator (#2178) * fix a bug in multiple identical spritesheet icons * add changelog entry * add changelog entry * prettier --------- Co-authored-by: Cory LaViska --- docs/pages/resources/changelog.md | 3 ++- src/components/icon/icon.component.ts | 23 ++++++++++++---------- src/components/icon/icon.test.ts | 28 +++++++++++++++++++++++++++ 3 files changed, 43 insertions(+), 11 deletions(-) diff --git a/docs/pages/resources/changelog.md b/docs/pages/resources/changelog.md index 15766a05..56a982cc 100644 --- a/docs/pages/resources/changelog.md +++ b/docs/pages/resources/changelog.md @@ -14,8 +14,9 @@ New versions of Shoelace are released as-needed and generally occur when a criti ## Next -- Improved performance of `` by waiting for the active state before spinning up the positioning library [#2179] +- Fixed a bug in `` not applying the mutator when loading multiple icons of the same name from a spritesheet. [#2178] - Fixed a bug in `` that made the suffix slot collide with the clear button [#2145] +- Improved performance of `` by waiting for the active state before spinning up the positioning library [#2179] ## 2.17.0 diff --git a/src/components/icon/icon.component.ts b/src/components/icon/icon.component.ts index 7b2878dc..f35ebb67 100644 --- a/src/components/icon/icon.component.ts +++ b/src/components/icon/icon.component.ts @@ -46,16 +46,6 @@ export default class SlIcon extends ShoelaceElement { `; - // Using a templateResult requires the SVG to be written to the DOM first before we can grab the SVGElement - // to be passed to the library's mutator function. - await this.updateComplete; - - const svg = this.shadowRoot!.querySelector("[part='svg']")!; - - if (typeof library.mutator === 'function') { - library.mutator(svg as SVGElement); - } - return this.svg; } @@ -185,6 +175,19 @@ export default class SlIcon extends ShoelaceElement { if (isTemplateResult(svg)) { this.svg = svg; + + if (library) { + // Using a templateResult requires the SVG to be written to the DOM first before we can grab the SVGElement + // to be passed to the library's mutator function. + await this.updateComplete; + + const shadowSVG = this.shadowRoot!.querySelector("[part='svg']")!; + + if (typeof library.mutator === 'function' && shadowSVG) { + library.mutator(shadowSVG as SVGElement); + } + } + return; } diff --git a/src/components/icon/icon.test.ts b/src/components/icon/icon.test.ts index 0361c83d..53417c1a 100644 --- a/src/components/icon/icon.test.ts +++ b/src/components/icon/icon.test.ts @@ -185,6 +185,34 @@ describe('', () => { expect(rect?.width).to.be.greaterThan(0); }); + // https://github.com/shoelace-style/shoelace/issues/2161 + it('Should apply mutator to multiple identical spritesheet icons', async () => { + registerIconLibrary('sprite', { + resolver: name => `/docs/assets/images/sprite.svg#${name}`, + mutator: svg => svg.setAttribute('fill', 'pink'), + spriteSheet: true + }); + + const el = await fixture(html` +
+ + +
+ `); + + const icons = [...el.querySelectorAll('sl-icon')]; + + await Promise.allSettled(icons.map(el => elementUpdated(el))); + + // This is kind of hacky...but with no way to check "load", we just use a timeout + await aTimeout(1000); + const icon1 = icons[0]; + const icon2 = icons[1]; + + expect(icon1.shadowRoot?.querySelector('svg')?.getAttribute('fill')).to.equal('pink'); + expect(icon2.shadowRoot?.querySelector('svg')?.getAttribute('fill')).to.equal('pink'); + }); + it('Should render nothing if the sprite hash is wrong', async () => { registerIconLibrary('sprite', { resolver: name => `/docs/assets/images/sprite.svg#${name}`,