Allow aria props

[wparrott-touchnet]

I saw that aria-label and aria-labelledby are accepted props to the component, but there are a number of others that are required based on a given element's state. It would be nice if:

1. All aria-* props are explicitly whitelisted, OR
2. The component used the ...restProps syntax to allow passing any unanticipated properties to the component without the need to whitelist them individually

The specific prop that I came here about is aria-checked, which is required for checkbox inputs (see https://dequeuniversity.com/rules/axe/3.1/aria-required-attr).

[markusenglund]

I am pretty certain aria-checked is not needed. React-switch uses an <input type="checkbox"> under the hood, so we should only need the regular checked attribute. aria-checked is meant for when you don't use the native checkbox element. See MDN.

[JaeYeopHan]

@markusenglund I agree with you! Then... How can I solve this issue?

I use this react-switch in my project. The lighthouse audit this issue in accessibility section. Could you give me advice?

[markusenglund]

Ah, it seems to be the role="switch" that creates problem in lighthouse. Their algorithm seems to detect any element with a role and checks if it has the required aria-* attributes.

The component works well with screen readers. Adding aria-checked to it will make no difference to what screen readers read out to users (as far as I can tell from testing with Voiceover). I suggest you ignore the warning, since there is no actual problem.

If you really need to pass the audit for some reason, I suggest you use patch-package to add aria-checked={checked} to the component in your project.

[JaeYeopHan]

@markusenglund Thanks! very helpful!

[psenger]

Hi @markusenglund thank you for such a wonderful widget.

I too need aria-* to pass through. If I may, let me explain. My switch, controls a Live Region. and I need to notify the SR that the given switch "aria-controls" that region. To get a better idea you can see this document ARIA_Live_Regions.

It looks like you are well versed on Aira, given your reply earlier. Maybe you can make a suggestion? If you agree, If I make the change, and PR, I think it would be best to simple pass all aria-* values and be done with it.

your thoughts?

[markusenglund]

Ok. Yeah, the best solution is probably to let people pass any prop they want straight to the input/checkbox element of the component. We could facilitate that by using the object ...spread syntax. I have been reluctant to do that because it adds a fair bit of size to the component after babel transpiles it, but it seems like it would be worth the cost.

[sag1v]

@markusenglund My use case is actually pass the required attribute to the input.

We can provide a dedicated object prop to be passed down to the underline input. something like

<Switch 
  checked={checked}
  inputProps={{required:true, 'aria-something': 'something'}} 
/>

And then react-switch will just spread this single object instead of the entire rest of props

<input {...inputProps} />

[markusenglund]

@sag1v Yup, I was thinking basically the same thing. The only question is if we should do a special inputProps prop or just spread the rest of the props on the input so lib users could pass them in like:

<Switch 
  checked={checked}
  required:true
  'aria-something': 'something'
/>

I'll take a look at it tomorrow.

[sag1v]

@markusenglund Though my suggestion was in favor of the explicit inputProps, i take it back. It's true, most of the time "explicitness" is better, but in this case i realize there are some major cons:

1. Extra prop to maintain for the library's API (any change may cause a breaking change)
2. Less intuitive, the lib users must read the docs to know about this prop.
3. Kind of awkward to pass these attributes to a JavaScript object instead of just writing them as element attribute

So spreading ...rest is actually superior in this case IMO.

Thanks for your time, if you want me to PR this i don't mind :)

[markusenglund]

Your points sound very reasonable. I'd absolutely appreciate a PR on this, although if I recall correctly it will take some changes to the Rollup config to make it happen. We need to use the babel object-spread plugin before the Buble compiler, since Buble doesn't support the rest-spread syntax at all.

[sag1v]

Yeah since babel 7 https://www.npmjs.com/package/@babel/plugin-proposal-object-rest-spread

[sag1v]

@markusenglund done with #43 :)

[markusenglund]

This was fixed in 5.0.0. Thanks @sag1v for the PR!