search

cascornelissen / svg-spritemap-webpack-plugin

SVG spritemap plugin for webpack
MIT License
211 stars 51 forks source link

Allow for attributes on root svg element #70

Closed noelheesen closed 5 years ago

noelheesen commented 5 years ago

Hello, I have the wish to set an id attribute on the root element of the generated sprite. I'm currently in-lining the generated sprite and this allows for easy targeting from CSS.

cascornelissen commented 5 years ago

Isn't it possible to wrap the spritemap in another element with a specific id/class?

<div id="spritemap">{{ load_spritemap() }}</div>

I'm hesitant to allow changing the (attributes on the) root SVG because either 1) someone will submit a new PR to also allow other attributes or 2) we'll have to come up with a way to dynamically add attributes which will make it way harder to prevent backwards incompatible changes as we have no idea which attributes might be present on the generated SVG element.

noelheesen commented 5 years ago

It's possible but requires dirty code. I'm reading the rawAsset from the compilation and apply it to preview-head.html for storybook so it get's in-lined. I'll have to modify a plugin for this to allow asset modification before appending (just for this asset).

I've set it up so attributes could be dynamically applied but didn't implement it yet since I agree that it make things way more complex. On the other hand, making it dynamic gives control (and responsibility) to the user.

cascornelissen commented 5 years ago

Solving it in the plugin would IMHO require a "dirty" (hard to maintain) solution as well, doesn't matter if we go for option 1 or 2 in my previous comment.

I understand that this solves an issue in your specific use-case but I don't think that issue should be solved by the plugin as it's unrelated to the spritemap.

If we would be able to come up with a way that allows for variable attributes on the root SVG (to solve issue 1) while keeping the plugin itself maintainable (issue 2), since we don't want to worry about every possible root attribute when releasing a new version, we might be able to solve this... What would your take on this be?

stale[bot] commented 5 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.