[FEATURE REQUEST] Automatically convert svg styles to attributes

[Snailedlt]

I have searched through the issues and didn't find my problem.

• Confirm

Problem

Upon fixing some stuff (#1276) causing a bug in the react-devicons project, I noticed that the process of converting SVG styles to attributes can be automated.

Some projects using devicons, like react-devicons will fail if styles are used in the SVG icons. This has led to several bugfixes. See #1169

Here's a screenshot of the issue:

Possible Solution

SVGO has a neat script (convertStyleToAttrs.js) to go through and convert all SVG styles into attributes. We could leverage this script to automate the process with one of our bots (maybe make a new one?).

This would be help ensure the SVG's work in the react-devicons project and potentially future projects with similar solutions.

Additional information

No response

[Snailedlt]

@devicons/supporter @devicons/ecosystem-react What do you guys think of this feature?

[maltejur]

Sounds great but somebody would have to implement it.

[maltejur]

Alternatively SVGO could also be directly integrated into react-devicons.

[maltejur]

Upon looking a bit further into it I think that convertStyleToAttrs.js does something entirely different. It converts a single style attribute on one element to multiple independent attributes on the same element. But our problem is with style elements, not attributes.

[maltejur]

This is what we need: https://github.com/svg/svgo/blob/main/plugins/removeStyleElement.js
But it just removes the style element entirely

[maltejur]

I've gone ahead and added removeStyleElement.js to react-devicons: devicons/react-devicons#14

[Snailedlt]

@maltejur Yeah looks like you're right!

I found what we need though. Instead of just removing the style element, we want to convert it to inline style attributes. This can be done with this script: https://github.com/svg/svgo/blob/main/plugins/inlineStyles.js

Description of what it does here: https://github.com/svg/svgo#built-in-plugins

Plugin	Description	Default
inlineStyles	move and merge styles from <style> elements to element style attributes	enabled

[Snailedlt]

I've gone ahead and added removeStyleElement.js to react-devicons: devicons/react-devicons#14

Great, but like I mentioned above, I think what we need is actually the inlineStyles.js script

[BenSouchet]

Hi, yes @Snailedlt is right, removing the style in svg elements will break lots of SVG icons.

The conversion from this inline style:

style="fill-rule:evenodd;clip-rule:evenodd;"

To this HTML attributes styling:

fill-rule="evenodd" clip-rule="evenodd"

is required to preserve / not break icons.

Personally, when I add new icons I always use the style HTML attributes, but some old icons still have inline style="".

[Snailedlt]

@BenSouchet
I think you're misunderstanding the issue here (just like I was earlier). You're talking about the style attribute, and not the style element, which is where the problem lies.

Style attribute looks like this:

<path style={{"fill-rule:evenodd;clip-rule:evenodd;"}} d="some path here"

Style element looks like this:

<style id="someId" type="text/css">
.someStyle{"fill-rule:evenodd;clip-rule:evenodd;"}
</style>

Now the first (Style attribute) is okey. Looks dirty, but it doesn't cause any errors. The 2nd (Style element) however is what's causing the error.

@maltejur ended up fixing the error by first running inlineStyles.js to inline any styles that are possible to inline, and then running removeStyleElement.js afterwards to remove the style element. Note that the change was implemented in the react-devicons repo instead of i the devicons repo since that's where the bug occured.

@BenSouchet
We could consider adding this to this repo too, but then we'd have to make sure it doesn't cause nay other errors. We should also consider (like you say) converting the inline style attribute into separate attributes with the convertStyleToAttrs.js script, but that's a different discussion, so feel free to open a new issue or discussion for that.

[BenSouchet]

@Snailedlt Ah okay my bad 🙂

I opened an issue but it's not a priority, nor a vital element.