search

danawoodman / react-fontawesome

A React Font Awesome component.
https://www.npmjs.com/package/react-fontawesome
MIT License
667 stars 72 forks source link

Filter which props are passed into the underlying span #8

Closed ide closed 8 years ago

ide commented 8 years ago

Previously the JSX splat syntax was used to pass this.props in its entirety into the underlying <span> component. This resulted in invalid HTML markup because the name prop was rendered in the markup, and according to the W3 HTML checker it's invalid to have a name prop on a <span> in most contexts.

So instead, we split out the FontAwesome-specific props from the rest of them, and pass only the non-FontAwesome props down to the <span>.

Test Plan: Added a regression test

ide commented 8 years ago

cc @danawoodman @amsardesai could one of you guys take a look at these commits when you get a chance? Thanks =)

danawoodman commented 8 years ago

Thanks again @ide I will review these all shortly and get back to you, sorry for the delay (messages missed my inbox!)

danawoodman commented 8 years ago

Thanks @ide, merged! 👍 😃

ide commented 8 years ago

@danawoodman thanks for the review! Do you think you could publish a new version to npm sometime?

danawoodman commented 8 years ago

@ide this in the newly published v1.0.0 on npm!

ide commented 8 years ago

Thanks :)

danawoodman commented 8 years ago

No, thank you! I welcome any more additions/changes you have 😸