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

Re-use icons #185

Closed
royduin opened this issue Feb 17, 2022 · 4 comments
Closed

Re-use icons #185

royduin opened this issue Feb 17, 2022 · 4 comments

Comments

@royduin
Copy link
Contributor

royduin commented Feb 17, 2022

Currently when an icon is used multiple times the SVG will be in the source multiple times which isn't very efficient. This can be solved by bundeling everything. For example at the current Laracon website: https://laracon.net, at the bottom:

<svg hidden>
    <defs>
        <g id="icon:camera-off"><path.... /></g>
        <g id="icon:live"><path.... /></g>
        <g id="icon:microphone-off"><path.... /></g>
        ...
    </defs>
</svg>

And these are used with:

<svg class="..." viewBox="0 0 24 24"><use href="#icon:live"></use></svg>

Thoughts? It would be nice if there was a config option to enable this and push all svg's like the @pushOnce directive behavior. Would something like this be accepted as PR? If I can find some time I'll see what I can do; otherwise a nice feature request.

@driesvints
Copy link
Member

Heya, there's a long explanation in #50 why I decided not to include sprite sheet support for this library. I'm undecided if I'd ever bring this back namely because I'm not convinced it solves an actual issue. I don't think that many SVG icons actually cause an efficiency problem when loading a web page.

For example, the https://blade-ui-kit.com/blade-icons overview loads with only 33KB while showing lots of icons. I think that's still pretty low.

@royduin
Copy link
Contributor Author

royduin commented Feb 17, 2022

Lighthouse doesn't like it. DOM size and tree depth, see: https://web.dev/dom-size/#how-the-lighthouse-dom-size-audit-fails, in general duplication isn't very nice and it cleans-up the source making it more readable.

@driesvints
Copy link
Member

Hmm I do see your point. It's just that I'm a tad reluctant to bring it back as it'd be quite a bit of code for me to maintain. I'm torn really.

Maybe, if you're up for it, you can work on a PR already? Worst case you'd be maintaining a fork of Blade Icons if you want this if I didn't merge it?

Think most code I removed can still be reused so that'd be a good starting point: fc99ec6

@royduin
Copy link
Contributor Author

royduin commented Feb 17, 2022

Let's take the Paypal logo as an example: https://upload.wikimedia.org/wikipedia/commons/b/b5/PayPal.svg, this SVG has 10x a path. So let's say we've a product listing with 50 products where the logo is displayed in; the DOM get's filled with duplication very quickly and we've 500 dom nodes, Lighthouse starts with warning at 800 nodes. And seeing 50 times the same big SVG in the source code isn't very clean and easy to read.

Of course we could put the svg url in an image tag or handle it with css but then things are getting mixed.

When I find myself some time I'll give it a try, thanks! 💯

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants