Extend initial a11y config for eslint #2459
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
🦋 Changeset detectedLatest commit: 65a281f The changes in this PR will be included in the next version bump. This PR includes changesets to release 27 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Size Change: -20 B (-0.02%) Total Size: 97.4 kB
ℹ️ View Unchanged
|
A new build was pushed to Chromatic! 🚀https://5e1bf4b385e3fb0020b7073c-gpqylfgtde.chromatic.com/ Chromatic results:
|
|
Note: This PR uses the |
GeraldRequired Reviewers
Don't want to be involved in this pull request? Comment |
npm Snapshot: Published🎉 Good news!! We've packaged up the latest commit from this PR (eb50a4d) and published all packages with changesets to npm. You can install the packages in webapp by running: ./services/static/dev/tools/deploy_wonder_blocks.js --tag="PR2459"Packages can also be installed manually by running: pnpm add @khanacademy/wonder-blocks-<package-name>@PR2459 |
… address aphrodite-add-style-variable-name linting rule
|
The pnpm changes are in |
jandrade
approved these changes
Feb 6, 2025
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2459 +/- ##
============================
============================
Continue to review full report in Codecov by Sentry.
|
beaesguerra
added a commit
to Khan/wonder-stuff
that referenced
this pull request
Feb 11, 2025
…nfig/a11y` (#1118) ## Summary: This work is part of the implementation of [ADR#781 Enabling more lint rules for accessibility](https://khanacademy.atlassian.net/wiki/x/IoBVyg) These changes includes: - Updating the `eslint-plugin-jsx-a11y` dependency - Configuring rules and settings (including custom component mapping, attribute mapping, polymorphic components) for `eslint-plugin-jsx-a11y` based on the [proposed rules and settings in the ADR](https://khanacademy.atlassian.net/wiki/spaces/ENG/pages/3394600994/ADR+781+Enabling+more+lint+rules+for+accessibility#Proposed-customization-and-overrides-for-the-plugin) and the [POC](https://github.com/Khan/webapp/pull/26840/files#diff-20e55c78ab86b0eb87c6f756500b87f451b52fab251d6f327ce572ec88e8ae3b) - Updating the README to include instructions for using `@khanacademy/eslint-config/a11y` Issue: FEI-1133 Implementation Plan: - #1114 Set up eslint a11y config and enable aphrodite-add-style-variable-name - Use this new config in WB, perseus, webapp and address lint errors from the aphrodite-add-style-variable-name rule - Khan/wonder-blocks#2459 - Khan/perseus#2193 - Khan/webapp#29212 - (this PR) Update the a11y.js config with the config based on the accessibility linting rules ADR - Use the updated config in projects and address existing errors by disabling the rules per line. Teams can address existing errors as they work in the area and new errors can be prevented with these lint rules! ## Test plan: Testing the new config locally: - Run yarn pack in `packages/eslint-config-khan`. This will create a `.tgz` file in the directory - In another project, install the package: ex: `yarn add -D -W /Users/beaesguerra/khan/wonder-stuff/packages/eslint-config-khan/khanacademy-eslint-config-v5.1.0.tgz` - Note: you might need to remove the node_modules first to make sure it installed the correct package. You can check by checking `node_modules/@khanacademy/eslint-config/a11y.js` in the project to see if the new config is there - Make sure the project already extends `@khanacademy/eslint-config/a11y` in the project's eslint config file - Run `yarn lint`/`pnpm lint` in the project and restart the ESLint server. It should show errors from the `jsx-a11y` plugin Note: Before merging and releasing these changes, I'll test these changes locally with the different projects and evaluate the errors to make sure the config changes are still relevant and helpful! Author: beaesguerra Reviewers: beaesguerra, marcysutton, kevinb-khan Required Reviewers: Approved By: marcysutton, kevinb-khan Checks: ✅ codecov/project, ✅ Test (macos-latest, 20.x), ✅ CodeQL, ✅ Lint, typecheck, and coverage check (ubuntu-latest, 20.x), ✅ Prime node_modules cache for primary configuration (ubuntu-latest, 20.x), ✅ Analyze (javascript), ✅ gerald, ⏭️ dependabot Pull Request URL: #1118
beaesguerra
added a commit
that referenced
this pull request
Feb 12, 2025
## Summary: This work is part of the implementation of [ADR#781 Enabling more lint rules for accessibility](https://khanacademy.atlassian.net/wiki/x/IoBVyg) These changes include: - Updating `@khanacademy/eslint-config` to the latest version to include new jsx-a11y configuration and settings - Addressed existing errors by adding `eslint-disable-next-line` - View the PR commits for a breakdown of errors by rule - Used `npx eslint-interactive@10.8.0 .` to easily disable lint errors per rule - New potential a11y issues can be prevented with these lint rules! Note: In a follow up PR, I'll address the low hanging issues in WB and create tickets for more complex issues that were found A breakdown of the existing jsx-a11y errors: <img width="1027" alt="wb-linting-errors-before" src="https://github.com/user-attachments/assets/cbe4be7f-6868-48af-8474-bad0297cf8b2" /> Implementation Plan: - Khan/wonder-stuff#1114 Set up eslint a11y config and enable aphrodite-add-style-variable-name - Use this new config in WB, perseus, webapp and address lint errors from the aphrodite-add-style-variable-name rule - #2459 - Khan/perseus#2193 - Khan/webapp#29212 - Khan/wonder-stuff#1118 Update the eslint-config/a11y config with the config based on the accessibility linting rules ADR - (includes this PR) Use the updated config in projects and address existing errors by disabling the rules per line. Issue: FEI-1133 ## Test plan: - Run `pnpm lint` and make sure there are no linting errors Author: beaesguerra Reviewers: beaesguerra, jandrade Required Reviewers: Approved By: jandrade Checks: ✅ Chromatic - Get results on regular PRs (ubuntu-latest, 20.x), ✅ Lint / Lint (ubuntu-latest, 20.x), ✅ Test / Test (ubuntu-latest, 20.x, 2/2), ✅ Test / Test (ubuntu-latest, 20.x, 1/2), ✅ Check build sizes (ubuntu-latest, 20.x), ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Chromatic - Build and test on regular PRs / chromatic (ubuntu-latest, 20.x), ✅ Prime node_modules cache for primary configuration (ubuntu-latest, 20.x), ⏭️ Chromatic - Skip on Release PR (changesets), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ gerald, ⏭️ dependabot Pull Request URL: #2471
beaesguerra
added a commit
to Khan/perseus
that referenced
this pull request
Feb 13, 2025
## Summary: This work is part of the implementation of [ADR#781 Enabling more lint rules for accessibility](https://khanacademy.atlassian.net/wiki/x/IoBVyg) These changes include: - Updating `@khanacademy/eslint-config` to the latest version to include new jsx-a11y configuration and settings - Updating `esling-plugin-jsx-a11y` to the latest version so it is up to date - Addressed existing errors by adding `eslint-disable-next-line` - View the PR commits for a breakdown of errors by rule - Used `npx eslint-interactive@10.8.0 .` to disable lint errors per rule - Provided custom component mapping for jsx-a11y in the eslint config since Perseus has custom components New potential a11y issues can be prevented with these lint rules and existing errors can be addressed over time by the team! The lint error messages have a link to the [rule docs](https://github.com/jsx-eslint/eslint-plugin-jsx-a11y/tree/main/docs/rules) for more details! A breakdown of the existing jsx-a11y errors: <img width="1019" alt="perseus-linting-errors-before" src="https://github.com/user-attachments/assets/8191e551-4147-43cb-bbcf-79b0e5684c26" /> ### Implementation Plan: - Khan/wonder-stuff#1114 Set up eslint a11y config and enable aphrodite-add-style-variable-name - Use this new config in WB, perseus, webapp and address lint errors from the aphrodite-add-style-variable-name rule - Khan/wonder-blocks#2459 - #2193 - Khan/webapp#29212 - Khan/wonder-stuff#1118 Update the eslint-config/a11y config with the config based on the accessibility linting rules ADR - (includes this PR) Use the updated config in projects and address existing errors by disabling the rules per line. Issue: FEI-1133 ## Test plan: - Run `yarn lint` and make sure there are no linting errors Author: beaesguerra Reviewers: catandthemachines, beaesguerra, #frontend-infra-web Required Reviewers: Approved By: catandthemachines Checks: ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x) Pull Request URL: #2232
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary:
This work is part of the implementation of ADR#781 Enabling more lint rules for accessibility
These changes includes:
@khanacademy/eslint-config@khanacademy/eslint-config/a11yconfig in project's eslint configaphrodite-add-style-variable-namerule from@khanacademy/eslint-plugin. This is the first step to adding more a11y linting rules so that the naming convention for html elements is more predictable for custom component mapping foreslint-plugin-jsx-a11y(see rule docs for more details).@khanacademy/eslint-config/a11ywill be updated in another PR to include config foreslint-plugin-jsx-a11yIssue: FEI-6050
Implementation Plan:
Test plan:
pnpm lintand make sure there are no linting errors.