github action to ensure svg fulfills requirements regarding css classes/inline css

[amacado]

As proposed in #403 and #400 we would like to introduce a new requirement for adding SVG icons to this project:

SVGs are not allowed to have classes for defining properties but instead enforcing inline css to declare styling. This might not considered best practice in general but in this case it provides a cleaner look of your icons and solving decleration issues which can occur when classes are beeing used.

Bad example (no longer allowed)

      <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>

Improved example (what the rule should enforce)

      <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>

Since this can be checked automated it would improve the reviewing process when this rule would be checked by a gihub action on every pull request.

[Thomas-Boi]

Hello,

I agree. I think we can add an extra step or a parallel job in the peek script to ensure that all svgs follow our guideline. We should also check for the view box as well. We had an issue with the view box not being "0 0 128 128" before.

[Thomas-Boi]

Taken from #456 @amacado:

Each .svg file contains one version of an icon in a 0 0 128 128 viewbox.
https://github.com/devicons/devicon/blob/master/CONTRIBUTING.md#svg-standards


Some examples:
https://raw.githubusercontent.com/devicons/devicon/master/icons/yunohost/yunohost-plain.svg
https://raw.githubusercontent.com/devicons/devicon/master/icons/cucumber/cucumber-plain-wordmark.svg

We should run a small script to check that all existing icons match our guidelines.

[Thomas-Boi]

I can run a small script to check that the current icons match our guidelines. During this time, I think we should refrain from merging new icons until:

-The upload issue is fixed (seems like it is for now)
-Create a script that checks for viewbox (I'll get on it)

[Thomas-Boi]

So I made a script that we can later use as part of the peek script. Here is its result:

The script will fail if anything is found, which save us time from uploading to Icomoon.

I think I can fix the icons shown above if possible and commit the peek script change in one PR.

[Thomas-Boi]

Hey @amacado ,

I notice that you want to remove the style="enable-background:new 0 0 128 128;" and x="0px" y="0px" as seen here. Why would you want to remove them and if the svgs shouldn't have these, should I add a check in the peek script as well?

[amacado]

Cool! :) @Thomas-Boi you're contributing so much useful additions! Feel free to correct the issues regarding the viewbox. We could add this script as github action to run on each pull_request event (not only on the bot:peek command). What do you think?

I notice that you want to remove the style="enable-background:new 0 0 128 128;" and x="0px" y="0px" as seen here. Why would you want to remove them and if the svgs shouldn't have these, should I add a check in the peek script as well?

Mh, I think this is more related to "optimisation", same for removing SVG comments like in
https://raw.githubusercontent.com/devicons/devicon/master/icons/cucumber/cucumber-plain-wordmark.svg

Those are superfluous and therefore can be removed. enable-background is even marked as depreceated (https://developer.mozilla.org/en-US/docs/Web/SVG/Attribute/enable-background) so I suggested to remove it.