Skip to content
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

Setting color through fill prop not working properly on 0.14.1 #64

Closed
ptfly opened this issue Nov 19, 2019 · 6 comments · Fixed by #67
Closed

Setting color through fill prop not working properly on 0.14.1 #64

ptfly opened this issue Nov 19, 2019 · 6 comments · Fixed by #67
Labels
Bug 🐛 Something isn't working

Comments

@ptfly
Copy link

ptfly commented Nov 19, 2019

Using fill on svgs containing groups <g> with fill="none" and actual fill color set deeper in the tree doesn't work. In order to make it work, I have to set the fill color on the top most group.

This will work:

<svg width="11px" height="13px" viewBox="0 0 11 13" version="1.1" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
    <g id="Page-1" stroke="none" stroke-width="1" fill="#000000" fill-rule="evenodd">
        <path d="M10.260805,9.55......" fill="#000000"></path>
    </g>
</svg>

But this wont:

<svg width="11px" height="13px" viewBox="0 0 11 13" version="1.1" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
    <g id="Page-1" stroke="none" stroke-width="1" fill="none" fill-rule="evenodd">
        <path d="M10.260805,9.55......" fill="#000000"></path>
    </g>
</svg>

On 0.13 its is working fine! Is there any breaking change which is not mentioned?

@kristerkari
Copy link
Owner

kristerkari commented Nov 19, 2019

Is there any breaking change which is not mentioned?

Yeah 0.14 enabled SVGO, which in theory does not mean breaking changes, but in reality it did break things.

@kristerkari
Copy link
Owner

@ptfly Could you please provide a full example to reproduce the bug? I tried to reproduce the bug, but wasn't able to do it because probably I'm not doing exactly what you are doing.

@ptfly
Copy link
Author

ptfly commented Nov 25, 2019

There is nothing more then the code above. I just removed the path for readability sake. Could be my SVGs, but they are quite standard. Here is an example icon:

<svg width="14px" height="10px" viewBox="0 0 14 10" version="1.1" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
    <g id="Symbols" stroke="none" stroke-width="1" fill="none" fill-rule="evenodd">
        <g id="dropdown" transform="translate(-270.000000, -17.000000)" fill="#000000">
            <path d="M277.731744,18.0625377 C277.680272,18.0241534 277.625577,17.9902943 277.568269,17.9613392 C277.507675,17.9134899 277.442485,17.8717683 277.373656,17.8367871 C277.30272,17.8140644 277.229709,17.7984191 277.15569,17.79008 C277.014165,17.7431734 276.861283,17.7431734 276.719758,17.79008 C276.645739,17.7984191 276.572728,17.8140644 276.501792,17.8367871 C276.432963,17.8717683 276.367773,17.9134899 276.307179,17.9613392 C276.249871,17.9902943 276.195175,18.0241534 276.143704,18.0625377 L270.305325,24.290142 C269.874353,24.762213 269.903048,25.4930642 270.369707,25.9298913 C270.836366,26.3667183 271.567519,26.3471402 272.010132,25.8859656 L277,20.5691484 L281.989868,25.8859656 C282.432481,26.3471402 283.163634,26.3667183 283.630293,25.9298913 C284.096952,25.4930642 284.125647,24.762213 283.694675,24.290142 L277.731744,18.0625377 Z" id="Shape" transform="translate(277.000000, 22.000000) scale(1, -1) translate(-277.000000, -22.000000) "></path>
        </g>
    </g>
</svg>

I'm using them like that:

const Icon = {
    AngleDown:require('../../assets/svg/angle-down.svg').default,
    ....
};

Then
<Icon.AngleDown fill={color}/>

@kristerkari
Copy link
Owner

Alright, I'll try to reproduce with that example.

@kristerkari
Copy link
Owner

@ptfly I managed to reproduce the bug, so I released a fix in 0.14.3. Could you please try it out and see if it fixes the issue for you?

@ptfly
Copy link
Author

ptfly commented Nov 28, 2019

Yes, now it looks fine! Thanks for the fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug 🐛 Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants