Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix multiple identical spritesheet icons not applying mutator #2178

Open
wants to merge 4 commits into
base: next
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions docs/pages/resources/changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ Components with the <sl-badge variant="warning" pill>Experimental</sl-badge> 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 `<sl-icon>` not applying the mutator when loading multiple icons of the same name from a spritesheet. [#2178]

## 2.17.0

- Added the `fixed-scroll-controls` attribute to `<sl-tab-group>` [#2128]
Expand Down
23 changes: 13 additions & 10 deletions src/components/icon/icon.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,16 +46,6 @@ export default class SlIcon extends ShoelaceElement {
<use part="use" href="${url}"></use>
</svg>`;

// 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;
}

Expand Down Expand Up @@ -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;
}

Expand Down
28 changes: 28 additions & 0 deletions src/components/icon/icon.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,34 @@ describe('<sl-icon>', () => {
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<HTMLDivElement>(html`
<div>
<sl-icon name="arrow-left" library="sprite"></sl-icon>
<sl-icon name="arrow-left" library="sprite"></sl-icon>
</div>
`);

const icons = [...el.querySelectorAll<SlIcon>('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}`,
Expand Down
Loading