Templarian / MaterialDesign-React

Dist for Material Design Icons React Component for JS/TypeScript
https://materialdesignicons.com
MIT License
139 stars 20 forks source link

fix: ensure currentColor is used as default fill #41

Closed davidjb closed 4 years ago

davidjb commented 4 years ago

Previously, the default props for Icon set color as null, which meant the previous change to set the default as fill: currentColor in ec665ec wasn't effective and the inline style attribute remained empty.

This also updates the unit tests to confirm the value is passed through to the underlying path.style.fill.

The demo at https://templarian.github.io/@mdi/react/ has an example that looks like this, when rendered:

<span style="fill: pink;"><svg viewBox="0 0 24 24" role="presentation"><path d="[snip]"></path></svg></span>

The icon was appearing as pink, but only the fill style was being applied outside the icon. By comparison, the path needs to look like <path d="[snip] style="fill: currentColor"> and the outer <span> changed to <span style="color: pink;"> to the text colour is picked up (CSS ref)

Corresponding demo change PR at https://github.com/Templarian/MaterialDesign-React-Demo/pull/6

Templarian commented 4 years ago

You are correct! I'll look through this PR tonight and merge it. Will merge the demo also and push a new version.

Thanks! I really need to not rush things like this. Not sure what I was thinking.

Templarian commented 4 years ago

@davidjb Thanks again, bumped the package to v1.4 and released the demo with latest also.

Let me know if the package is missing anything. Busy working on a Web Component version and hopefully Angular to match.

davidjb commented 4 years ago

Fantastic, thanks @Templarian! Upgraded to v1.4 and it's working well -- I've been able to clean up all my inline colouring of icons.