setup jsx-a11y plugin #20
Conversation
|
Thanks @petrhanak for the PR! When we discussed this addition with @dannytce we came to a conclusion that we cannot use their recommended ruleset because they set all the rules to Could you please add the rules individually with their levels reduced down to |
|
fixed |
|
Waiting for additional review from @dannytce before merging. |
|
Idea: Would it make sense to split the rules into yet another configuration file instead of adding it to the optional.js ? Something like accessibility.js in the react family. What do you think @petrhanak @dannytce? |
environments/react/accessibility.js
Outdated
| // between options will trigger the onChange event in some browsers. Regardless, when a change | ||
| // of context results from an onBlur event or an onChange event, the user should be notified of | ||
| // the change unless it occurs below the currently focused element. | ||
| 'jsx-a11y/no-onchange': 'warn', |
There was a problem hiding this comment.
Let's have this one as an error, because it breaks Safari
| // Some HTML elements have native semantics that are implemented by the browser. This includes | ||
| // default/implicit ARIA roles. Setting an ARIA role that matches its default/implicit role is | ||
| // redundant since it is already set by the browser. | ||
| 'jsx-a11y/no-redundant-roles': 'warn', |
There was a problem hiding this comment.
Let's have this one as an error. It's a good practise to enforce developers to do that.
There was a problem hiding this comment.
If it's not absolutely terrible practice I would prefer keeping this at warn level. Best practices should not cause a failure in your test/deployment pipeline.
|
@dannytce Are you okay with the changes made and with my comment on your review? @petrhanak Some last things that need be done on this before we can (finally!) merge:
Thank you for your efforts on this! |
|
Thanks, merged! I will release this together with #22 (once that's merged, too). 🍻 🎉 👍 |
Fixes #18.