-
-
Notifications
You must be signed in to change notification settings - Fork 230
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’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: add eslint jsx a11y package #4118
chore: add eslint jsx a11y package #4118
Conversation
✅ Deploy Preview for oss-insights ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Compliance Checks
Thank you for your Pull Request! We have run several checks on this pull request in order to make sure it's suitable for merging into this project. The results are listed in the following section.
Watched Files
This pull request modifies specific files that require careful review by the maintainers.
Files Matched
- npm-shrinkwrap.json
- package.json
✅ Deploy Preview for design-insights ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
@FatumaA, thanks for the PR! This will be a great first step in sorting out some accessibility issues mentioned in the associated issue. For the new rules, there's no need to explicitly set all the rules to warn. Only, set the rules to warn that currently cause the build to fail. I believe most of the issues are From there, as discussed we can tackle those issues one by one, and once they're all sorted, we can remove all the rules that were set to warn so they can error by default, i.e. #4110 |
I've removed the rules that did not cause errors when running |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this @FatumaA! Looking forward to making the app more accessible!
Looks like the build is failing because of lint errors still.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like some more rules need warnings still to get the build to pass.
Co-authored-by: Nick Taylor <[email protected]>
I should have run the build locally 🤦🏾♀️ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Description
This PR adds the eslint jsx a11y plugin to the project.
Related Tickets & Documents
Fixes #2264
Mobile & Desktop Screenshots/Recordings
N/A
Steps to QA
npx eslint .
at root of projectTier (staff will fill in)
[optional] What gif best describes this PR or how it makes you feel?