search

wmira / react-icons-kit

React Svg Icons
https://react-icons-kit.vercel.app/
MIT License
370 stars 44 forks source link

Don't discard fill=none #73

Closed lyleunderwood closed 3 years ago

lyleunderwood commented 3 years ago

Is there a reason that the fill attribute is discarded in SvgIcon?

Some icons are using fill=none without an accompanying stroke (I'm not sure why):

https://github.com/google/material-design-icons/blob/ab12f16d050ecb1886b606f08825d24b30acafea/src/av/video_label/materialicons/24px.svg#L1

The current logic in SvgIcon only allows fill=none if there is an accompanying stroke, meaning these icons are not rendered correctly:

image

This seems like it is now the standard for material design icons, so it should probably be supported. I made the minimal possible change here in an attempt to resolve the immediate issue, is there a more appropriate fix that presents itself? I'm unclear on why the code was written this way in the first place.

kamikazebr commented 3 years ago

Using the npm package its normal, but when i use the repo modified by me, its error occours. will use your patch in my fork repo, ty @lyleunderwood

lyleunderwood commented 3 years ago

@kamikazebr That's because the change to react-icons-kit that updates it to use the version 4 material design icons has not been published yet, it's only on master. This seems to be a change in how the material design icons in v4 were produced.

wmira commented 3 years ago

guys sorry I will pick up the PR's

On Tue, Jun 8, 2021, 1:30 AM Lyle Underwood @.***> wrote:

@kamikazebr https://github.com/kamikazebr That's because the change to react-icons-kit that updates it to use the version 4 material design icons has not been published yet, it's only on master. This seems to be a change in how the material design icons in v4 were produced.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/wmira/react-icons-kit/pull/73#issuecomment-856123999, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADSMECUG6MJYHHSD4IPN4LTRT63BANCNFSM453MEFVA .

wmira commented 3 years ago

@lyleunderwood sorry it took a while. there was some issue rendering this new md-4 and I needed to find out why. I think its because they've added quite a bit of scheme to the icons.