fix(commons/aria): allow aria-required on radio and checkbox roles

[KittyGiraudel]

This pull-request closes #540.

[CLAassistant]

All committers have signed the CLA.

[WilcoFiers]

I think we should have this for checkbox as well. Can you also add one or two tests?

[KittyGiraudel]

I wasn‘t sure if you wanted it for checkbox. Happy to add it. Not sure how to add the tests though. Care to put me on the right track?

[WilcoFiers]

Have a look at some of these:

• https://github.com/dequelabs/axe-core/blob/develop/test/checks/aria/required-attr.js
• https://github.com/dequelabs/axe-core/blob/develop/test/checks/aria/valid-attr-value.js
• https://github.com/dequelabs/axe-core/blob/develop/test/checks/aria/errormessage.js

[KittyGiraudel]

I run through these files. I’m not sure what kind of test you would like for allowing aria-required attribute on radio and checkbox roles. I didn’t find a suite I could extend with that kind of check.

Do you test all allowed attributes somewhere maybe?

[WilcoFiers]

We're not testing every attribute, no. But we are adding tests for every PR so that we can prevent regression on new issues. On second thought, maybe adding it as an integration tests is better:

https://github.com/dequelabs/axe-core/blob/develop/test/integration/rules/aria-allowed-attr/passes.html
https://github.com/dequelabs/axe-core/blob/develop/test/integration/rules/aria-allowed-attr/passes.json

Does that help?

[KittyGiraudel]

I just committed something that I thought could be useful. Is it enough or would you like me to check integration tests as well?

Edit: I’m going to add integration tests as well.

[WilcoFiers]

Looks good to me. There was one comment from Dylan in the original issue. @dylanb Any objections to doing this?