oliverfindl / vue-svg-inline-plugin

Vue plugin for inline replacement of SVG images with actual content of SVG files.
MIT License
33 stars 3 forks source link

SVG Sprites doesn't work since the viewBox attribute is missing #3

Closed madebyfabian closed 3 years ago

madebyfabian commented 3 years ago

Hey there, first of all, thank you very much for this plugin! Currently, I believe it's the only one for vue3, which is great. But what's really a pain is that I can't use the sprite mode. The reason I want to use it is since every icon ever needed will make a http request to it. So it's not cached or anything, which leads in thousands of HTTP requests for a relatively simple list of cards.

To avoid this, I wanted to use the sprite mode with <img v-svg-inline.sprite .... But then, the viewBox attribute is not applied to the tag where the icon is finally used. This results in unusable behaviour, since a viewBox attribute is absolutely needed on both sites (both on in sprite & on in the UI). It leads to icons not properly resizing.

Here is an example of how it behaves

image

A fix (for me) is to add the viewBox property manually:

image image


Is there a way you could fix this? Or is there a reason why viewBox is not getting applied to the in UI?

oliverfindl commented 3 years ago

Hello,

plugin should work with both Vue\@2 and with Vue\@3. It worked on my examples when I last published this plugin. I always test it on all examples before publishing. So if you have some issues, please provide some dummy repo, where this issue would be reproducible.

For your first issue with sprites. It's fetching all SVGs as they appear on screen (with lazy loading you can postpone SVGs under fold). Once SVG has been fetched, it can be cached into local storage, so next request won't fetch them again. As for sprites, I didn't test this behavior, so it could be broken in this scenario. I didn't want to implement blocking fetching process, because it could make UI jump around.

For your second issue with viewBox attribute, I can add some new option for transfer/clone selected attributes if sprite directive is present.

If you have more questions, feel free to ask here.

Thanks.

madebyfabian commented 3 years ago

Hello Oliver, thanks for your quick answer!

This sounds very great so far. Until you implemented this, I would make a PR adding a note for this viewBox behaviour into the README, that users must define this manually for now. If that's okay for you.

oliverfindl commented 3 years ago

Hello,

I've published new version which fixes this issue. Check out new attributes.clone option. Unfortunately, this solution is not ideal and is somewhat taxing on performance, but correct solution is not possible without some major code refactoring.

Tested it successfully on all examples.

I also backported it into vue-svg-inline-loader package.

Thanks.

madebyfabian commented 3 years ago

Thank you very much! Works like a charm, at least in my case.

oliverfindl commented 3 years ago

Glad to hear.