Templarian / MaterialDesign-React

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

Icon prop types do not allow assignment of onClick #36

Closed jhlav closed 4 years ago

jhlav commented 4 years ago

Using Typescript, an error is thrown when using an onClick prop on the Icon, such as for switching between two icon states. I'm using this for a password field which has two different icons depending on whether the password is shown.

image

I'm making a PR with the necessary changes to fix this. It is no longer showing the error after changing locally.

Templarian commented 4 years ago

This was intentional left off. We did not want to encourage the use of event handlers on presentation components.

Equivalent libraries for web components, vue, and angular that match these props and using this as the basis for others to follow. The idea is to keep these as simple as possible and consistent.

Templarian commented 4 years ago

Leaving this opened for now, but putting click events on an icon like your showing is probably not accessible to screen readers. You'll need to use a button.

Templarian commented 4 years ago

By the way @jhlav published v1.3.0 to NPM with the other updates. 😃

https://templarian.github.io/@mdi/react/ Demo was updated also to include currentColor support.

jhlav commented 4 years ago

This was intentional left off. We did not want to encourage the use of event handlers on presentation components.

I see - learned something new about accessibility. The library I'm using this with is react-md - the TextField component. It does not expose an onClick prop for the password input's icon button (PasswordButton component), so I will instead focus my efforts there to implement a better solution.

I hope others doing something similar will learn from this issue as well.

Thanks for publishing the new version and quick response!