Add ability to customize fill color

[pgom]

Following the proposed solution detailed on https://github.com/vault-development/react-native-svg-uri/issues/86, this PR allows to defined a custom fill value for svg elements without any defined fill attribute

[hitmanmcc]

It would be cool to have this merged, we need this as well.

[matc4]

I have a doubt whit this change, can cause that some elements that should not have fill attr, get colored, what do you think?

[pgom]

@matc4 for that cases, I believe that not setting the fill prop would serve, right?

[matc4]

@pgom its possible that you have an SVG with 10 elements, 5 with the fill attr, and 5 without.
Actually, only the 5 elements with fill attr get color changed. Whit this PR 10 elements will be colored whit fill attr.

This will be the desirable behaviour in all cases?, It is possible that some of that elements must remain without the fill prop?

I'm not sure if this is a possible scenario

[pgom]

I see what you mean @matc4. We are using this mainly to display icons and they are intented to have a single color. But without this change, I'm unable to change the color of the svg. Any suggestions? Maybe adding a new flag to control this behavior? Something like icon={true|false}?

[egid]

@matc4 do you have a suggested alternate solution? Because right now all SVGs seem to render black, ignoring fill (which is listed in the readme as a supported prop).

This is a bug with a working solution; please consider releasing a version with support for fill.

From the readme:

fill	Color		Overrides all fill attributes of the svg file

The documented behavior sounds exactly like the thing you are concerned might happen with this fix.

[matc4]

Hello @egid , the documentation says that "overrides" the fill attr of the svg file, but this PR "adds" fill attr to svg elements that previously they did not have, is not the same.

I understand the problem that you have in your use case, but my concern, is if this PR will "break" for other uses cases.

I don't have a solution yet. i'm trying to think possible solutions

[egid]

Got it. I misunderstood what was happening under the hood and have gone back and re-read.

imo, if an SVG contains no fill attributes (which seems to be our problem with black SVGs, and likely what is hitting @pgom and @hitmanmcc), we should apply the specified fill to all paths. If the SVG already contains fills, then perhaps we only override.

The icon={true} syntax also seems reasonable, or perhaps something less specific like fillAll={true}.

[pgom]

I like fillAll better, actually 👍. @matc4 wdyt? I can change this PR to include that logic.

[matc4]

@pgom Yes, i think that 'fillAll' is more representative.

[pgom]

@matc4 please take a look at the latest changes.

[egid]

Any chance we can get this merged sooner than later?

[egid]

Please :D

[ilya-vaikutis]

Please also add corresponding type definitions.

[pgom]

What do you mean @ilya-vaikutis?

[ilya-vaikutis]

@pgom, new property should be also described in TypeScript type definition file (index.d.ts).

[pgom]

Added @ilya-vaikutis

[pgom]

@matc4 did you take the change to review this? 😄

[egid]

Just following up - it'd be great if this could merge soonish.

[egid]

Looks like this project is pretty dead. Does anybody have an alternative? Should we just fork this and maintain it locally?

[matc4]

Hello @egid , i will merge all the pending pull request, and i will need help to test the final state of the library, so we can make a new release

[egid]

If you publish a prerelease version, I know that we will be happy to install it and test it out.

[egid]

Any status changes?

[egid]

Excellent! Thank you.

[egid]

Should we be pinning directly to github master to test this out, or will y'all be releasing a prerelease version?

[egid]

@matc4 any plans for a release any time soon?