kiwilan / nuxt-svg-transformer

Nuxt 3 module. Transform SVG to inject dynamically into Vue component, type included. Powered by unplugin.
https://github.com/kiwilan/unplugin-svg-transformer
MIT License
27 stars 6 forks source link

Clear only the outermost sizes #2

Closed FarhanShares closed 1 year ago

FarhanShares commented 1 year ago

Currently, setting the clearSize: true in the config clears all the height & width attributes that may be present on a SVG file,

For that reason the following:

<svg height="100" width="200" ...>
    <rect height="4" width="4" ...></rect>
</svg>

Will be transformed into:

<svg ...>
    <rect ...></rect>
</svg>

As a result, the rect will not be shown (it can happen on other elements as well) & it'll result in unexpected output. Clearing the nested height & width attributes might not be necessary. I think only applying it to the parent or outermost element is sufficient, otherwise it is currently affecting some SVGs that I've tested with.

Also, the default value for clearSize is false written in the in the doc / readme, however it is actually behaving like the value is true.

FarhanShares commented 1 year ago

I can send a PR for that. Even we can have another config, e.g. clearNestedSize: true | false to also preserve the existing behaviour.

ewilan-riviere commented 1 year ago

Do you think clearSize should be true by default? I can set it to false by default, I agree it can generate some bugs.

The extra option clearNestedSize should be a very insteresting feature, yes. I will check if a system like that can be added easily ;)

FarhanShares commented 1 year ago

Maybe we can turn off clearSize by default and It's job is to just clearing the outermost sizes (i.e. from parent <svg> tag only & not it's children)

We can have clearNestedSize, with a default value of false as well. Which will just clear the sizes for the children tags (e.g. <rect>, <cx>, <circle> etc) of the parent . And of course, it'll not touch the parent tag, just as the name implies.

On the other hand, we can go for another approach if that much granular control is not necessary, clearSize: 'all' | 'parent' | 'none' and it'll either clear all sizes or only the sizes from the parent element.

ewilan-riviere commented 1 year ago

The last option is really wonderful! I set this type to clearClasses, clearSize and clearStyles to allow customization, these options are set to none by default. Thanks for this excellent proposition. ;)

I publish 0.0.5 version, tell if it's okay with this version!

FarhanShares commented 1 year ago

Cool. Thanks for updating it. How about having a naming consistency on the config properties too? E.g. either singular or plural.

Or

Though doesn't matter much, it's more about aesthetics.

ewilan-riviere commented 1 year ago

You are right, I publish a version with singular name options ;)