Skip to content

Commit

Permalink
Enable rules from eslint-plugin-jsx-a11y in `@khanacademy/eslint-co…
Browse files Browse the repository at this point in the history
…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
  • Loading branch information
beaesguerra authored Feb 11, 2025
1 parent 046f326 commit cd91b93
Show file tree
Hide file tree
Showing 6 changed files with 977 additions and 118 deletions.
5 changes: 5 additions & 0 deletions .changeset/quiet-snails-cheer.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@khanacademy/eslint-config": minor
---

Enable accessibility linting rules from `eslint-plugin-jsx-a11y` in `@khanacademy/eslint-config/a11y`
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@
"eslint-plugin-import": "^2.28.1",
"eslint-plugin-jest": "28.9.0",
"eslint-plugin-jsdoc": "^48.1.0",
"eslint-plugin-jsx-a11y": "^6.7.1",
"eslint-plugin-jsx-a11y": "^6.10.2",
"eslint-plugin-monorepo": "^0.3.2",
"eslint-plugin-prettier": "^5.0.1",
"eslint-plugin-promise": "^6.1.1",
Expand Down
6 changes: 2 additions & 4 deletions packages/eslint-config-khan/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ Shared Khan Academy eslint configuration.
- Update your .eslintrc.js file to:
- extend `"@khanacademy"`
- include settings for `"import/resolver"`
- For accessibility linting rules, extend `@khanacademy/eslint-config/a11y`

For monorepos the `"import/resolver"` settings will look like this:

Expand All @@ -30,10 +31,7 @@ For monorepos the `"import/resolver"` settings will look like this:
},
```

For regulard repos, the settings will look like this:

```
For monorepos the `"import/resolver"` settings will look like this:
For regular repos, the settings will look like this:

```
settings: {
Expand Down
133 changes: 132 additions & 1 deletion packages/eslint-config-khan/a11y.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,19 @@
/* eslint-disable import/no-commonjs */
const ERROR = "error";
const OFF = "off";

/**
* Eslint config for enabling lint rules related to accessibility. This config
* is intended to encourage the development of frontend interfaces with
* accessibility in mind.
*
* Note: Other types of accessibility tests (tooling, manual testing)
* are still useful since linting can only detect some issues from static code.
*/
module.exports = {
parser: "@typescript-eslint/parser",
plugins: ["@khanacademy"],
extends: ["plugin:jsx-a11y/strict"],
plugins: ["@khanacademy", "jsx-a11y"],
env: {
browser: true,
es6: true,
Expand All @@ -16,8 +26,129 @@ module.exports = {
"@typescript-eslint/parser": [".ts", ".tsx"],
},
"import/extensions": [".js", ".jsx", ".ts", ".tsx"],
"jsx-a11y": {
polymorphicPropName: "tag",
// These are the WB components that have a `tag` prop to assign the
// underlying html element. Intentionally allowing these components
// since other components could be using "tag" as a prop name for
// a different purpose.
polymorphicAllowList: [
"BodyMonospace",
"BodySerifBlock",
"Body",
"Caption",
"Footnote",
"HeadingLarge",
"HeadingMedium",
"HeadingSmall",
"HeadingXSmall",
"LabelLarge",
"LabelMedium",
"LabelSmall",
"LabelXSmall",
"Tagline",
"Text",
"Title",
"View",
],
components: {
// Mapping for common WB components
Link: "a",
Button: "button",
IconButton: "button",
TextArea: "textarea",
TextField: "input",
// SingleSelect/MultiSelect components don't have an equivalent
// html element, the closest is a `select` element. This allows
// for labels to be assosciated with the control for the
// jsx-a11y/label-has-associated-control rule.
SingleSelect: "select",
MultiSelect: "select",

// Mapping for common wrappers for html elements when we can use `addStyle`
StyledA: "a",
StyledButton: "button",
StyledImg: "img",
StyledSvg: "svg",
StyledUl: "ul",
StyledOl: "ol",
StyledLi: "li",
StyledSpan: "span",
StyledDiv: "div",
StyledSection: "section",
StyledHeader: "header",
StyledFooter: "footer",
StyledBlockquote: "blockquote",
StyledForm: "form",
StyledOutput: "output",
StyledIframe: "iframe",
StyledHr: "hr",
StyledP: "p",
StyledFieldset: "fieldset",
StyledLegend: "legend",
StyledCaption: "caption",
StyledPre: "pre",
StyledSup: "sup",
StyledMark: "mark",
StyledTable: "table",
StyledTr: "tr",
StyledTd: "td",
StyledTh: "th",
StyledDl: "dl",
StyledDt: "dt",
StyledDd: "dd",
},
attributes: {
for: ["htmlFor", "for"],
},
},
},
rules: {
"@khanacademy/aphrodite-add-style-variable-name": ERROR,

/** jsx-a11y **/

/** jsx-a11y: Rules to explictly enable that are not part of the strict rules **/
"jsx-a11y/lang": ERROR,
"jsx-a11y/no-aria-hidden-on-focusable": ERROR,
"jsx-a11y/anchor-ambiguous-text": ERROR,
// Explicitly enable accessible emoji rule so that emojis are used intentionally.
// Note: The plugin marks this rule as deprecated since browsers and assistive
// tech have advanced since the rule was introduced.
"jsx-a11y/accessible-emoji": ERROR,

/** jsx-a11y: Rules to explicitly disable **/
// Disabled since using autofocus could be valid depending on the context
"jsx-a11y/no-autofocus": OFF,
// Disabled since setting role is sometimes valid, especially on flexible
// custom components
"jsx-a11y/prefer-tag-over-role": OFF,
// Disabled because results from rule are not reliable. Would be more
// helpful to check associated labels on rendered output
"jsx-a11y/control-has-associated-label": OFF,

/** jsx-a11y: Rules with configuration options **/
"jsx-a11y/anchor-is-valid": [
ERROR,
// This makes it so Link components using the `to` prop is valid
// https://github.com/jsx-eslint/eslint-plugin-jsx-a11y/issues/340#issuecomment-338424908
{components: ["Link"], specialLink: ["to"]},
],
"jsx-a11y/no-noninteractive-tabindex": [
ERROR,
// It is recommended to allow this rule on elements with role=tabpanel
// https://github.com/jsx-eslint/eslint-plugin-jsx-a11y/blob/e5dda96f9c021c524a05d9b6b209a2389828ffb3/docs/rules/no-noninteractive-tabindex.md#rule-options
{
tags: [],
roles: ["tabpanel"],
},
],
"jsx-a11y/label-has-associated-control": [
ERROR,
{
// Increase depth to support cases where there are nested elements within a label
depth: 3,
},
],
},
};
2 changes: 1 addition & 1 deletion packages/eslint-config-khan/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
"eslint-config-prettier": "^8.3.0",
"eslint-import-resolver-typescript": "^3.5.3",
"eslint-plugin-import": "^2.23.4",
"eslint-plugin-jsx-a11y": "^6.4.1",
"eslint-plugin-jsx-a11y": "^6.10.2",
"eslint-plugin-prettier": "^3.4.0",
"eslint-plugin-react": "^7.24.0"
},
Expand Down
Loading

0 comments on commit cd91b93

Please sign in to comment.