webalys-hq / streamlinehq-npm

The deprecated Streamline NPM package, which will be deleted on 1st November 2022.
https://medium.com/streamline-icons/how-streamline-narrows-down-its-focus-eac6fdb5c6f2
24 stars 5 forks source link

Hardcoded 'fill' attribute presents styling problems if not using CSS #15

Closed sparkpunk closed 3 years ago

sparkpunk commented 3 years ago

Overview

When building with React/Material-UI, we rely on theme overrides to do a majority of our custom styling, and avoid as much extra CSS as possible. As such, the fill attribute on each icon forces us to decode each icon's data string in order to manipulate its color.

Suggestion

It'd be great if the fill attribute was a configuration option (default to true), so that we could pass the color as props to the icon element.

Example

import { ReactComponent as StreamlineIcon } from '@streamline/path/to/icon.svg';

<StreamlineIcon fill="#f0f" />
cbrwizard commented 3 years ago

Good question. Would using currentColor in every svg's stroke and fill suffice? From my understanding it will be black if a parent doesn't have a color set.

Update: no, at the same time some svgs are colorful, eg in https://app.streamlinehq.com/illustrations/illustrations-multicolor. If we set currentColor everywhere, they will be black completely and users will be forced to colorize them.

Right now images are just .svg files so we don't want to complicate them by turning them into components with props. Going that way means that we'll have to create wrappers for each framework. We're a small team and we cannot afford this complexity.

The only option I see here is to create and distribute separate versions of svg files where each color is replaced with currentColor. Do you see any other options?

sparkpunk commented 3 years ago

If an SVG doesn't have a fill attribute set on it, the browser will use #000 to paint them by default. I don't think there needs to be a componentization of the SVGs, but in removing fill from each path (only on families where there is no added stroke definition or duotone presentation, like Streamline Bold), it should be more flexible for a wider range of use cases. Here's an example:

Screenshot: https://cln.sh/hz6CnN

import { ReactComponent as TractorFill } from './path/to/Tractor-fill.svg';
import { ReactComponent as TractorNoFill } from './path/to/Tractor-nofill.svg';

<TractorFill />
<TractorNoFill />
<TractorNoFill fill="red" />

Edit: sorry, I just saw the last part of your response. Yes, I think substituting #000000 for currentColor is also a good solution!

stigvanbrabant commented 3 years ago

Having the same issue using this with Vite, i solved it by overwriting the fill attribute inside an svg loader with currentColor for now but its not ideal. I think substituting the hardcoded color to currentColor would be a good solution.

shilpa-sujatha commented 3 years ago

Streamine-regular category cannot be imported as a component this way import { ReactComponent as StreamlineIcon } from '@streamline/path/to/icon.svg';, how do we add color to icons from this category? I was rendering them in an img tag but that doesn't allow any customizations on color, could someone provide any insights?

certainlyakey commented 3 years ago

We definitely need predefined fills and strokes removed from SVG files, in order to be able to freely change them with CSS. In the previous version (where there were separate framework-dependent wrappers) we at least could do that by passing currentColor to either fill or stroke props. Now we can't do that — SVG code doesn't have any props, and it's not possible to override a color attribute set on a path from its SVG root tag.

BTW please consider converting all the icons to use fill only and not stroke. Having to guess once you want to recolor an icon where we should target fills and where strokes is really inconvenient.

lucasrusso95 commented 3 years ago

@certainlyakey How can i work with color change in menu when the link is active ?

cbrwizard commented 3 years ago

We have recently recreated all assets and released a new NPM 3.0.1 version https://github.com/webalys-hq/streamlinehq-npm/releases/tag/3.0.1, please install it, fetch newest images and check if it helps you. Now colors in one-colored families are replaced by currentColor and for them you can pass stroke/fill to the svg tag itself.

cbrwizard commented 3 years ago

Closing this issue since I believe it's fixed. Feel free to respond if you think otherwise