IBM-Design / icons

IBM Design Language Icons
http://www.ibm.com/design/language/resources/icon-library
Other
194 stars 41 forks source link

SVGs should be packaged with no fill color #20

Closed Moodycomputer closed 8 years ago

Moodycomputer commented 8 years ago

Icons in the .AI files right now have a black fill. when an SVG is exported with from these, they get stuck with a fill="#000000" attribute. This makes it impossible to style the colors of these icons through CSS on the web.

If the fill of the icons was switched over to registration, the fill attribute goes away and colors can be controlled through CSS.

barlock commented 8 years ago

I've never had a problem controlling the fill of these icons. If you just set a fill property it cascades down to the icon.

Moodycomputer commented 8 years ago

If fill is set on the icon as an attribute, it can't be controlled via CSS though.

Moodycomputer commented 8 years ago

For clarity, this use case is most prevalent when exporting from AI using the copy/paste method (copy icon from illustrator, paste in to text editor) where a lot of pruning is required either way. This may not be a common enough use case to be a major issue, but also isn't a large task. Small effort, small win?

barlock commented 8 years ago

Gotcha! prs welcome of course! I would rather spend cyclces updating a "build" to get from the .ai files to svgs and pngs to handle that case generally rather than take the effort to maintain the .ai files which seems harder to me. Might just be my developer talking sense I'm fairly bad at illustrator

kevinSuttle commented 8 years ago

Tangent: We probably should just have SVG files. Do we really need AI files?

kevinSuttle commented 8 years ago

@terracomma At least having the option to strip them out at build time would be good. I end up having to go back and do it manually.

kevinSuttle commented 8 years ago

FYI: I have a Sketch branch that uses extended svgo options to remove fills. https://github.com/IBM-Design/icons/tree/sketch

barlock commented 8 years ago

I'm confused, I took a random look at the svgs in the dist and they don't have fills. @kevinsuttle what is in that branch? Should it be merged into the build script?

barlock commented 8 years ago

It looks like a lot of the changes in there would be good for master, can you make a pr? I haven't researched all of the options you turned on

kevinSuttle commented 8 years ago

@mbarlock I'm still figuring it out. And @Moodycomputer I actually think that it's more flexible to set the fill to currentColor and inherit the color. https://github.com/svg/svgo/issues/561

kevinSuttle commented 8 years ago

Here we go! https://github.com/svg/svgo/pull/521

barlock commented 8 years ago

Awesome! I have that set by default in my style sheet. I do wonder if that is what we should be exporting by default though. What are other icon sets doing?

kevinSuttle commented 8 years ago

Who cares? Let's do the awesome thing. :)

seejamescode commented 8 years ago

Looks like this was resolved! Correct me if I am wrong.