Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add jsx-a11y plugin #170

Merged
merged 3 commits into from
Mar 25, 2021
Merged

feat: add jsx-a11y plugin #170

merged 3 commits into from
Mar 25, 2021

Conversation

artemruts
Copy link
Contributor

@artemruts artemruts commented Nov 23, 2020

Description

Adds ESLint accessibility plugin so it's easy to do the right thing.

@quinnkeast @felixfbecker do we keep recommended rules? We need to be careful introducing this, all of the default rules are error and may block CI/CD.

Right now our codebase has ~111 a11y errors (yarn all:eslint | grep jsx-a11y | wc -l).

Edit:

It's pretty cool we're running this config against the codebase in CI 馃憤 鉂わ笍

@felixfbecker
Copy link
Contributor

Since there are so many violations, we would need a PR ready that fixes the violations in the main codebase before merging this one.

Alternatively, something we often do is add rules as warnings, then promote them to errors once every instance is fixed. However in reality often they stay warnings forever, so if possible there can be value in fixing violations in one swoop. But warnings are still better than no warnings :)

@artemruts
Copy link
Contributor Author

Since there are so many violations, we would need a PR ready that fixes the violations in the main codebase before merging this one.

Alternatively, something we often do is add rules as warnings, then promote them to errors once every instance is fixed. However in reality often they stay warnings forever, so if possible there can be value in fixing violations in one swoop. But warnings are still better than no warnings :)

Yeah, I agree that if we introduce them as warnings it'll take a long time. I'll just try to fix them and test very page where I can 馃憤

@umpox umpox merged commit 5e5a7ea into master Mar 25, 2021
@umpox umpox deleted the adds-eslint-a11y branch March 25, 2021 11:19
@sourcegraph-buildkite
Copy link
Collaborator

馃帀 This PR is included in version 0.23.0 馃帀

The release is available on:

Your semantic-release bot 馃摝馃殌

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

4 participants