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

The icon library mutator is only applied to the first of multiple identical icons #2161

Closed
niboan opened this issue Sep 9, 2024 · 5 comments · Fixed by #2178
Closed

The icon library mutator is only applied to the first of multiple identical icons #2161

niboan opened this issue Sep 9, 2024 · 5 comments · Fixed by #2178
Assignees
Labels
bug Things that aren't working right in the library.

Comments

@niboan
Copy link

niboan commented Sep 9, 2024

Describe the bug

When using SVG sprite sheets while having multiple instances of the same icon in the page, the mutator function is only applied to the first instance.

Might be related to #1925.

To Reproduce

Use SVG sprite sheets with a mutator, and place multiple instances of the same icon

registerIconLibrary('default', {
    ...
    mutator: svg => svg.setAttribute('fill', 'currentColor'),
    spriteSheet: true
});
<div>
    <sl-icon name="fas-folder-open"></sl-icon>
    <sl-icon name="fas-folder-open"></sl-icon>
    <sl-icon name="fas-folder"></sl-icon>
    <sl-icon name="fas-folder"></sl-icon>
</div>

The first of each icon type has the mutator applied, the second has not, see screen shot.

Demo

The CodePen for https://shoelace.style/components/icon#customize-the-default-library-to-use-svg-sprites doesn't work, probably because of CSP, so I can only reproduce it locally.

Screenshots

image

Browser / OS

  • OS: Windows
  • Browser: Chrome, Firefox
  • Browser version: latest
@niboan niboan added the bug Things that aren't working right in the library. label Sep 9, 2024
@claviska claviska assigned claviska and KonnorRogers and unassigned claviska Sep 9, 2024
@KonnorRogers
Copy link
Collaborator

Thanks for the report!

Fix here: #2178

@niboan
Copy link
Author

niboan commented Sep 23, 2024

Nice, thanks!

Too bad it didn't make it to the 2.17.0 release, though, we were hoping to have the fix in our next release.
Will there be a 2.17.1?

@KonnorRogers
Copy link
Collaborator

@niboan looks like we have a few other bug fixes pending, so i'll check with @claviska , but I think we could reasonably get out a 2.17.1 pretty shortly.

@KonnorRogers
Copy link
Collaborator

@niboan we released 2.17.1 today which should have this fix.

@niboan
Copy link
Author

niboan commented Sep 24, 2024

Wow, that was fast, thanks a lot!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Things that aren't working right in the library.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants