resolvetosavelives / healthicons

A collection of open source icons for public health projects.
https://healthicons.org
MIT License
648 stars 49 forks source link

Cannot change icons' colour or their background colour #156

Closed troccoli closed 1 year ago

troccoli commented 2 years ago

I have found an issue that I think is worth looking at. I think it's related to #150 and possibly #151.

Let's say I have the following HTML

<div style="color: blue; background-color: yellow">
    <svg width="48" height="48" viewBox="0 0 48 48" fill="none" xmlns="http://www.w3.org/2000/svg">
        <rect width="48" height="48" fill="white"/>
        <path d="M30.5 9.5C30.5 11.433 28.933 13 27 13C25.067 13 23.5 11.433 23.5 9.5C23.5 7.567 25.067 6 27 6C28.933 6 30.5 7.567 30.5 9.5Z" fill="#333333"/>
        <path d="M24.6295 32.3726L25.9938 32.2748C25.9979 32.1837 26 32.0921 26 32C26 28.6863 23.3137 26 20 26C19.1147 26 18.2743 26.1917 17.5178 26.5359C17.5607 27.6864 17.6798 28.6119 17.8577 29.1063C19.2218 32.8971 24.6295 32.3726 24.6295 32.3726Z" fill="#333333"/>
        <path fill-rule="evenodd" clip-rule="evenodd" d="M38.3387 37.4031C39.2385 39.4773 36.1367 41.027 34.9055 39.1674C33.6743 37.3077 30.6199 31.9434 30.6199 31.9434L30 31.9878L30 32C30 37.5228 25.5228 42 20 42C14.4772 42 10 37.5228 10 32C10 27.7085 12.7033 24.0483 16.5 22.6296V16.75L13.9 14.8L15.1 13.2L18.5 15.75V17.9478C18.7523 17.1634 19.0573 16.4825 19.4204 15.9697C21.4804 13.0611 21.8829 12.8465 24.1559 13.0611C26.429 13.2757 30.2647 20.8572 30.2647 20.8572C30.2647 20.8572 35.0949 22.0016 36.9418 22.6453C38.7886 23.289 37.6284 25.8162 36.4445 25.7685C35.2607 25.7208 28.773 25.0771 27.897 24.5765C27.0209 24.0758 25.5529 21.6678 25.5529 21.6678L25.4086 23.5875C26.531 24.3105 27.4993 25.2521 28.2534 26.352C30.0556 26.5087 32.1041 26.6745 32.7035 26.6745C33.84 26.6745 34.1005 27.4613 34.1005 27.4613C34.1005 27.4613 37.439 35.3289 38.3387 37.4031ZM25.3973 26.0949C25.3426 26.0449 25.2873 25.9957 25.2313 25.9472C23.8287 24.734 22.0001 24 20 24C19.135 24 18.302 24.1373 17.5217 24.3913C14.3165 25.4345 12 28.4467 12 32C12 36.4183 15.5817 40 20 40C24.4183 40 28 36.4183 28 32C28 29.6617 26.9968 27.5576 25.3973 26.0949Z" fill="#333333"/>
    </svg>
</div>

I get Screenshot 2022-02-27 at 10 19 22

However, I would expect the wheelcahir to be blue in a yellow box, since I change the colours of the container <div>.

The issue is that the fill colour is fixed in the SVG.

I propose to remove the fill="#333333" that sets the colour to (a shade of) black on all the <path> tag, replace fill="none" on the <svg> tag with fill="currentColor", and finally replace fill="white" in the <rect> tag with fill="none".

This would result in the following

Screenshot 2022-02-27 at 10 27 08

There may be other tags in the various SVGs that use different fills, but the idea is the same: we shold have fill="currentColor" on the <svg> tag, we should not have any fill that sets a specific colour, and we may have fill="none" on some tags inside the SVG when appropriate.

troccoli commented 2 years ago

Is this a valid issue? Shall I submit a PR?

ampodobas commented 2 years ago

@troccoli This would be a neat addition if the opacity and hex value for each color can be set. Font Awesome's duotone icons (https://fontawesome.com/docs/web/style/duotone) implemented that flexibility quite well.

sgarrity commented 1 year ago

After an extensive 10 minutes of research, it looks like Figma isn't capable of handling smarter colors like currentColor, none, or the CSS Custom Properties (aka CSS Variables) approach that @ampodobas mentioned is in use by Font Awesome.

I did a quick spot check and it looks like the current SVGs are consistent with the colors, with the only instances of fill or stroke that I found being one of the following:

fill="white"
fill="none"
fill="black"
fill="#333333"
stroke="#333333"

It should be relatively "easy" to do run add a script that does some search/replacing, assuming the semantics hold up across all icons (that #333333 should always be currentColor, for example).

I did a quick experiment and it looks like if we replace #333333 with currentColor, most apps will just treat that as black (I tried Illustrator, Firefox, and Figma). This seems like a fine default. Then, if you use the file in a context with custom CSS (or even just embedded in an HTML document with a different font color, the icon will automagically take on that color.

Are there any drawbacks to this approach we may not be thinking of?

troccoli commented 1 year ago

I am the maintainer of the troccoli/blade-health-icons package, which was how I found out the issue.

I run the following in a shell script before addind the SVG to my packages

for FILE in $RESOURCES/*; do
  # Fix the svg to allow changing their colour
  sed -i '' 's/fill="none"/fill="currentColor"/g' $FILE
  sed -i '' 's/fill="white"/fill="none"/g' $FILE
  sed -i '' 's/fill="#333333"//g' $FILE
done

I'm not sure why I completely remove the #333333 colour, but it seems to work for me. I haven't checked all icons though, but I haven't received any issues on my package either.

nburka commented 1 year ago

I like @troccoli's use of sed. We could add something like that to the yarn update-icons command.

nburka commented 1 year ago

@sgarrity: I see in @troccoli's example above that the replacements are:

Is that what we should be doing here? Or is it:

nburka commented 1 year ago

I have the code implemented to handle the search/replace. I just need to know what commands we actually need.

nburka commented 1 year ago

I have updated all of the icons and merged that to master.

There's now an extra yarn remove-svg-backgrounds command which will be automatically run when you run yarn update-icons which we always do when publishing new icons anyway.

That will automatically clean up all of the colors and add currentColor where appropriate.

Thanks @troccoli for filing this!!

troccoli commented 1 year ago

Thank you @nburka for fixing it.