Skip to content

Conversation

natergj
Copy link

@natergj natergj commented Mar 15, 2017

This allows for passing aria-, data- and role as props of Checkbox which will be included in the rendered output.

I need to be able to have aria-, data- and role attributes rendered into the HTML to provide accessibility and automation support. This explicitly calls out these attributes and passes them to the rendered html elements. aria-, data- and role are automatically supported in react components. https://facebook.github.io/react/docs/dom-elements.html#all-supported-html-attributes

If this PR is accepted, I will be making some more to add this support to other rc components.

passes aria-*, data-* and role props of Checkbox to the rendered output.
@coveralls
Copy link

Coverage Status

Changes Unknown when pulling b319e6b on natergj:support-rendering-of-data-attributes into ** on react-component:master**.

@paranoidjk
Copy link
Member

how about directly pass otherProps?

@natergj
Copy link
Author

natergj commented Mar 16, 2017

passing otherProps directly was my first thought, but I am using Ant Design checkbox which wraps the react-components checkbox. When I directly pass otherProps, I get React warnings about having both defaultChecked and checked as properties of <input />.

My other thought was to explicitly exclude the defaultChecked prop from otherProps if it exists, but that would only take care of a single error case. By explicitly calling out and including the aria-, data- and role props I can prevent react warnings about other invalid props on the <input />.

@paranoidjk
Copy link
Member

👍

@zandernelson
Copy link

this would be really great!! merge plz

also props like autoFocus and Id should be defined.

@chmontgomery
Copy link

@paranoidjk any update on this? Seems good to me!

@paranoidjk
Copy link
Member

npm owner ls rc-checkbox                                   
afc163 <afc163@gmail.com>
raohai <surgesoft@gmail.com>
sliuqin <sliuqin@gmail.com>
yiminghe <yiminghe@gmail.com>

ping @afc163

@afc163
Copy link
Member

afc163 commented Apr 13, 2017

Added

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants