Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

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

wants to merge 1 commit into from

Conversation

peterpeterparker
Copy link
Contributor

@peterpeterparker peterpeterparker commented Mar 15, 2020

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.

@louisiscoding
Copy link

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

@leifriksheim
Copy link

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

@mrlund
Copy link

mrlund commented Apr 24, 2020

@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
Copy link

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
Copy link
Contributor Author

peterpeterparker commented May 28, 2020

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

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

@jan-grosser
Copy link

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
Copy link
Contributor Author

peterpeterparker commented Aug 18, 2020

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
Copy link
Contributor

Closed in 98fa89e

@peterpeterparker peterpeterparker deleted the fix-fill-stroke-currentColor branch October 6, 2020 17:06
@peterpeterparker
Copy link
Contributor Author

Yeaaaaaaah awesome! Thx @adamdbradley 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants