From c66b9e437ef896d397d3491e75f03993037b3bcd Mon Sep 17 00:00:00 2001 From: konnorrogers Date: Fri, 20 Sep 2024 14:16:36 -0400 Subject: [PATCH 1/4] fix a bug in multiple identical spritesheet icons --- src/components/icon/icon.component.ts | 23 ++++++++++++---------- src/components/icon/icon.test.ts | 28 +++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 10 deletions(-) diff --git a/src/components/icon/icon.component.ts b/src/components/icon/icon.component.ts index 7b2878dc46..f35ebb6768 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 0361c83d9b..db7e49d726 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}`, From 5fad0d43e28e69de2ef1345045a6497517b5a8c9 Mon Sep 17 00:00:00 2001 From: konnorrogers Date: Fri, 20 Sep 2024 14:17:18 -0400 Subject: [PATCH 2/4] add changelog entry --- docs/pages/resources/changelog.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/docs/pages/resources/changelog.md b/docs/pages/resources/changelog.md index 142de763c1..14e4de990b 100644 --- a/docs/pages/resources/changelog.md +++ b/docs/pages/resources/changelog.md @@ -12,6 +12,10 @@ Components with the Experimental bad New versions of Shoelace are released as-needed and generally occur when a critical mass of changes have accumulated. At any time, you can see what's coming in the next release by visiting [next.shoelace.style](https://next.shoelace.style). +## Next + +- Fixed a bug in `` not applying the mutator when loading multiple icons of the same name from a spritesheet. [#] + ## 2.17.0 - Added the `fixed-scroll-controls` attribute to `` [#2128] @@ -1776,4 +1780,4 @@ The following pages demonstrate why this change was necessary. ## 2.0.0-beta.1 -- Initial release +- Initial release \ No newline at end of file From d1b692e8436f2d25fba03ac2a70b5e6976ea5b16 Mon Sep 17 00:00:00 2001 From: konnorrogers Date: Fri, 20 Sep 2024 14:18:03 -0400 Subject: [PATCH 3/4] add changelog entry --- docs/pages/resources/changelog.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/pages/resources/changelog.md b/docs/pages/resources/changelog.md index 14e4de990b..6867e9ab74 100644 --- a/docs/pages/resources/changelog.md +++ b/docs/pages/resources/changelog.md @@ -14,7 +14,7 @@ New versions of Shoelace are released as-needed and generally occur when a criti ## Next -- Fixed a bug in `` not applying the mutator when loading multiple icons of the same name from a spritesheet. [#] +- Fixed a bug in `` not applying the mutator when loading multiple icons of the same name from a spritesheet. [#2178] ## 2.17.0 From ed137251651c6fb23487c3da564a153e30ea1a62 Mon Sep 17 00:00:00 2001 From: konnorrogers Date: Fri, 20 Sep 2024 14:18:17 -0400 Subject: [PATCH 4/4] prettier --- docs/pages/resources/changelog.md | 2 +- src/components/icon/icon.test.ts | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/docs/pages/resources/changelog.md b/docs/pages/resources/changelog.md index 6867e9ab74..ec79ad0878 100644 --- a/docs/pages/resources/changelog.md +++ b/docs/pages/resources/changelog.md @@ -1780,4 +1780,4 @@ The following pages demonstrate why this change was necessary. ## 2.0.0-beta.1 -- Initial release \ No newline at end of file +- Initial release diff --git a/src/components/icon/icon.test.ts b/src/components/icon/icon.test.ts index db7e49d726..53417c1a0a 100644 --- a/src/components/icon/icon.test.ts +++ b/src/components/icon/icon.test.ts @@ -200,17 +200,17 @@ describe('', () => { `); - const icons = [...el.querySelectorAll("sl-icon")] + const icons = [...el.querySelectorAll('sl-icon')]; - await Promise.allSettled(icons.map((el) => elementUpdated(el))); + 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] + 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") + 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 () => {