ionic-team / ionicons

Premium hand-crafted icons built by Ionic, for Ionic apps and web apps everywhere 🌎
http://ionicons.com
MIT License
17.4k stars 2.06k forks source link

fix(#784): use currentColor for fill and stroke properties #805

Closed peterpeterparker closed 3 years ago

peterpeterparker commented 4 years ago

most icons have a stroke:#000; color fixed to black and two have a fill:#010101; color fixed to grey.

this PR replace these fixed colors with currentColor in order to inherit the parent color.

LouisJS commented 4 years ago

Please make it happens. The library is just not usable without modifying the hardstuck black color

leifriksheim commented 4 years ago

This would be awesome. It's the only reason why I can't use ionicons.

mrlund commented 4 years ago

@adamdbradley it seems a bunch of us are stuck on this issue (understandable, since changing icon color is pretty common). Any chance you would be able to let us know if you think it'll be merged, or if there's some problem with the suggested approach?

vladshcherbin commented 4 years ago

Can someone merge this? 🙏

It's unbelievable to have inline stroke color from icon creators with 7+ years experience. Even on official website icons don't have hardcoded color.

cc @adamdbradley

peterpeterparker commented 4 years ago

I discussed my PR a while ago and I think it can't be merged so easily as most of the icons are generated and used across different pipelines, what I did understand and remember. That's probably why it takes more time than as it looks.

Peege151 commented 4 years ago

Any update on this, you guys? No conflicts and this problem that makes this library unusable for many...

jan-grosser commented 4 years ago

To everyone having this issue:

Using <script src="https://unpkg.com/ionicons@5.0.0/dist/ionicons.js"></script> like described in the readme.md logs the following error:

[ionicons] Deprecated script, please remove: <script src="https://unpkg.com/ionicons@5.0.0/dist/ionicons.js"></script>
To improve performance it is recommended to set the differential scripts in the head as follows:
<script type="module" src="https://unpkg.com/ionicons@5.0.0/dist/ionicons/ionicons.esm.js"></script>
<script nomodule="" src="https://unpkg.com/ionicons@5.0.0/dist/ionicons/ionicons.js"></script>

Using these scripts allows you to set the color of outlined icons.

Just replace <script src="https://unpkg.com/ionicons@5.0.0/dist/ionicons.js"></script> with

<script type="module" src="https://unpkg.com/ionicons@5.0.0/dist/ionicons/ionicons.esm.js"></script>
<script nomodule="" src="https://unpkg.com/ionicons@5.0.0/dist/ionicons/ionicons.js"></script>
peterpeterparker commented 3 years ago

This issue with this PR is the following: updating the source icons is kind of a conflicts with the actual build pipeline.

It works currently as following:

Icons in source folder can contain style, such as stroke:#000;, because they are often added or merged without prior processing. Basically straight from the design tools such as Photoshop.

To overcome the issue and clean the icons, there is a build script which optimize ("clean") the svg before saving them in the dist folder.

That's why there is no issue if the icons are consumed from a bundle ("from npm") but only if downloaded.

Cleaning the source icons or creating a script to do so is nice, but won't ensure in the future that there will be one new icons which will contain a style.

Therefore, I am thinking that moreover than a script and updating all current icons, maybe it would actually need a GitHub action/robot which check any PR for such style or process icons and clean them in PR.

But, spontaneously, maybe the easier option is to close this PR and "just" to enhance the ionicons website so that icons downloaded from the website do not contains these style, basically enhancing the existing encodeSvg.

The download can either happens from GitHub or from the ionicons website but doing so, at least from the most important channel (my guess), the icons' style would be delivered cleaned.

What do you think about it?

adamdbradley commented 3 years ago

Closed in https://github.com/ionic-team/ionicons/commit/98fa89ea5731f546f2bc88f53818a3f304d4195a

peterpeterparker commented 3 years ago

Yeaaaaaaah awesome! Thx @adamdbradley 🙏