Change icon CSS classes to fill attribute

[rawdanowicz]

Hello all,
I've been using SVG icons in my portfolio and I encountered a problem with inconsistency of color filling. Most of the icons are properly styled with fill attribute but some of them use CSS classes .cls-1, .cls-2 and so on. It creates a scope problem and each next icon using CSS classes overdrives color of the previous ones.

A bad example (npm icon)

      <svg viewBox="0 0 128 128">
        <defs>
          <style>
            .cls-1 {
              fill: #cb3837;
            }
          </style>
        </defs>
        <title>Artboard 5</title>
        <path
          id="original-wordmark"
          class="cls-1"
          d="M2,38.5H126V82.21H64V89.5H36.44V82.21H2ZM8.89,74.93H22.67V53.07h6.89V74.93h6.89V45.79H8.89ZM43.33,45.79V82.21H57.11V74.93H70.89V45.79Zm13.78,7.29H64V67.64H57.11Zm20.67-7.29V74.93H91.56V53.07h6.89V74.93h6.89V53.07h6.89V74.93h6.89V45.79Z"
        ></path>
      </svg>

After improvement (npm icon)

      <svg role="img" viewBox="0 0 128 128">
        <path
          fill="#cb3837"
          d="M2,38.5H126V82.21H64V89.5H36.44V82.21H2ZM8.89,74.93H22.67V53.07h6.89V74.93h6.89V45.79H8.89ZM43.33,45.79V82.21H57.11V74.93H70.89V45.79Zm13.78,7.29H64V67.64H57.11Zm20.67-7.29V74.93H91.56V53.07h6.89V74.93h6.89V53.07h6.89V74.93h6.89V45.79Z"
        ></path>
      </svg>

My solution

• check all SVG icons one by one
• remove all of CSS Classes
• use fill attribute for color
• remove title attribute or change to matching one (optional)

I would be happy to fix that issue if there's a need for that.

[amacado]

Hello @rawdanowiczdev! You absolutely have a point here. Those "inline css classes" are not very pretty (and have some other problems too - which we have fixed in some workarounds). If you have the time feel free to improve them and create a pull request. I would highly appreciate it!

[rawdanowicz]

Thanks for your reply! I'll be glad to do so. Currently I have quite a lot of time so you can expect PR before the end of this year.

[Thomas-Boi]

Hey @amacado,

This is an interesting problem and I'm glad @rawdanowiczdev noticed it. I think we will need to check for this issue in future PRs, either manually or using a script.

What do you think? Should we just do it manually or using a script? Since GitHub allow us to modify files in a PR it might be easier and quicker to just do it manually. Although that means we must leave documentations for future maintainers regarding this.

[amacado]

Merged in #403 and released in v2.8.0 🚀 Tanks again @rawdanowiczdev