mozilla / FirefoxColor

Theming demo for Firefox Quantum and beyond
https://color.firefox.com
Mozilla Public License 2.0
461 stars 95 forks source link

Add jsx-a11y ESLint rules? #26

Open pdehaan opened 6 years ago

pdehaan commented 6 years ago

I randomly tried adding the eslint-plugin-jsx-a11y plugin to our ESLint config, extended the "plugin:jsx-a11y/recommended" rules and it returned 18 errors. Not sure if we want to take a look and possibly add it to our config:

$ npm run lint

> themesrfun@0.0.17 lint /Users/pdehaan/dev/github/mozilla/ThemesRFun
> eslint --color .

/Users/pdehaan/dev/github/mozilla/ThemesRFun/src/web/lib/components/BrowserPreview/index.js
   33:5   error  Visible, non-interactive elements with click handlers must have at least one keyboard listener  jsx-a11y/click-events-have-key-events
   33:5   error  Static HTML elements with event handlers require a role                                         jsx-a11y/no-static-element-interactions
   41:5   error  Visible, non-interactive elements with click handlers must have at least one keyboard listener  jsx-a11y/click-events-have-key-events
   41:5   error  Non-interactive elements should not be assigned mouse or keyboard event listeners               jsx-a11y/no-noninteractive-element-interactions
   49:7   error  Visible, non-interactive elements with click handlers must have at least one keyboard listener  jsx-a11y/click-events-have-key-events
   49:7   error  Non-interactive elements should not be assigned mouse or keyboard event listeners               jsx-a11y/no-noninteractive-element-interactions
   67:7   error  Visible, non-interactive elements with click handlers must have at least one keyboard listener  jsx-a11y/click-events-have-key-events
   67:7   error  Non-interactive elements should not be assigned mouse or keyboard event listeners               jsx-a11y/no-noninteractive-element-interactions
   79:7   error  Visible, non-interactive elements with click handlers must have at least one keyboard listener  jsx-a11y/click-events-have-key-events
   79:7   error  Static HTML elements with event handlers require a role                                         jsx-a11y/no-static-element-interactions
   89:9   error  Visible, non-interactive elements with click handlers must have at least one keyboard listener  jsx-a11y/click-events-have-key-events
   89:9   error  Static HTML elements with event handlers require a role                                         jsx-a11y/no-static-element-interactions
  102:11  error  Visible, non-interactive elements with click handlers must have at least one keyboard listener  jsx-a11y/click-events-have-key-events
  102:11  error  Static HTML elements with event handlers require a role                                         jsx-a11y/no-static-element-interactions

/Users/pdehaan/dev/github/mozilla/ThemesRFun/src/web/lib/components/ThemeBackgroundPicker/index.js
  14:5  error  Visible, non-interactive elements with click handlers must have at least one keyboard listener  jsx-a11y/click-events-have-key-events
  14:5  error  Static HTML elements with event handlers require a role                                         jsx-a11y/no-static-element-interactions

/Users/pdehaan/dev/github/mozilla/ThemesRFun/src/web/lib/components/ThemeColorsEditor/index.js
  25:15  error  Visible, non-interactive elements with click handlers must have at least one keyboard listener  jsx-a11y/click-events-have-key-events
  25:15  error  Non-interactive elements should not be assigned mouse or keyboard event listeners               jsx-a11y/no-noninteractive-element-interactions

✖ 18 problems (18 errors, 0 warnings)

npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! themesrfun@0.0.17 lint: `eslint --color .`
npm ERR! Exit status 1
diff --git a/.eslintrc.yaml b/.eslintrc.yaml
index 2b0df99..c4a961b 100644
--- a/.eslintrc.yaml
+++ b/.eslintrc.yaml
@@ -5,13 +5,18 @@ env:
   webextensions: true

 extends:
-- eslint:recommended
-- airbnb-base
-- plugin:react/recommended
+  - eslint:recommended
+  - airbnb-base
+  - plugin:react/recommended
+  - plugin:jsx-a11y/recommended

 parserOptions:
   ecmaVersion: 6

+plugins:
+  - jsx-a11y
+  - react
+
 root: true

And on that note, not sure if we want to drop the airbnb-base rules in favor of the plugin:mozilla/recommended rules or not.

env:
  browser: true
  es6: true
  node: true
  webextensions: true

extends:
  - eslint:recommended
  - plugin:jsx-a11y/recommended
  - plugin:mozilla/recommended
  - plugin:react/recommended

parserOptions:
  ecmaVersion: 6
  sourceType: module

plugins:
  - jsx-a11y
  - mozilla
  - react

root: true

rules:
  import/no-named-as-default: off
  import/prefer-default-export: off
  react/prop-types: off

  quotes: [error, single]
lmorchard commented 6 years ago

Hmm, I wonder if this can help get us to satisfying #47

ghost commented 6 years ago

Is this issue still open ?

pdehaan commented 6 years ago

@csd713 I don't see jsx-a11y in package.json or the .eslintrc config file, so it should still be up for grabs if you want to take a poke at a11y rules.

devcer commented 5 years ago

Hey, I still see few linting errors, Can I work on this?

caitmuenster commented 5 years ago

Hey @devcer, sorry for the late reply! We currently don't have anyone who can review and approve a patch. :/ If that changes in the near future, we'll open this back up for contributors!