-
-
Notifications
You must be signed in to change notification settings - Fork 146
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
Comments
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. |
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. |
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 |
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! 💯 |
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:
And these are used with:
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.The text was updated successfully, but these errors were encountered: