Merge pull request #210 from jessebeach/no-noninteractive-tabindex

No noninteractive tabindex

Author: jessebeach

README.md

@@ -116,6 +116,7 @@ You can also enable all the recommended rules at once. Add `plugin:jsx-a11y/reco
 - [no-interactive-element-to-noninteractive-role](docs/rules/no-interactive-element-to-noninteractive-role.md): Interactive elements should not be assigned non-interactive roles.
 - [no-noninteractive-element-interactions](docs/rules/no-noninteractive-element-interactions.md): Non-interactive elements should not be assigned mouse or keyboard event listeners.
 - [no-noninteractive-element-to-interactive-role](docs/rules/no-noninteractive-element-to-interactive-role.md): Non-interactive elements should not be assigned interactive roles.
+- [no-noninteractive-tabindex](docs/rules/no-noninteractive-tabindex.md): `tabIndex` should only be declared on interactive elements.
 - [no-onchange](docs/rules/no-onchange.md): Enforce usage of `onBlur` over `onChange` on select menus for accessibility.
 - [no-redundant-roles](docs/rules/no-redundant-roles.md): Enforce explicit role property is not the same as implicit/default role property on element.
 - [no-static-element-interactions](docs/rules/no-static-element-interactions.md): Enforce that non-interactive, visible elements (such as `<div>`) that have click handlers use the role attribute.

__tests__/src/rules/no-noninteractive-tabindex-test.js

@@ -0,0 +1,75 @@
+/* eslint-env jest */
+/**
+ * @fileoverview Disallow tabindex on static and noninteractive elements
+ * @author jessebeach
+ */
+
+// -----------------------------------------------------------------------------
+// Requirements
+// -----------------------------------------------------------------------------
+
+import { RuleTester } from 'eslint';
+import { configs } from '../../../src/index';
+import parserOptionsMapper from '../../__util__/parserOptionsMapper';
+import rule from '../../../src/rules/no-noninteractive-tabindex';
+import ruleOptionsMapperFactory from '../../__util__/ruleOptionsMapperFactory';
+
+// -----------------------------------------------------------------------------
+// Tests
+// -----------------------------------------------------------------------------
+
+const ruleTester = new RuleTester();
+
+const ruleName = 'no-noninteractive-tabindex';
+
+const expectedError = {
+  message: '`tabIndex` should only be declared on interactive elements.',
+  type: 'JSXAttribute',
+};
+
+const alwaysValid = [
+  { code: '<MyButton tabIndex={0} />' },
+  { code: '<button />' },
+  { code: '<button tabIndex="0" />' },
+  { code: '<button tabIndex={0} />' },
+  { code: '<div />' },
+  { code: '<div tabIndex="-1" />' },
+  { code: '<div role="button" tabIndex="0" />' },
+  { code: '<div role="article" tabIndex="-1" />' },
+  { code: '<article tabIndex="-1" />' },
+];
+
+const neverValid = [
+  { code: '<div tabIndex="0" />', errors: [expectedError] },
+  { code: '<div role="article" tabIndex="0" />', errors: [expectedError] },
+  { code: '<article tabIndex="0" />', errors: [expectedError] },
+  { code: '<article tabIndex={0} />', errors: [expectedError] },
+];
+
+const recommendedOptions = (
+  configs.recommended.rules[`jsx-a11y/${ruleName}`][1] || {}
+);
+
+ruleTester.run(`${ruleName}:recommended`, rule, {
+  valid: [
+    ...alwaysValid,
+    { code: '<div role="tabpanel" tabIndex="0" />' },
+  ]
+    .map(ruleOptionsMapperFactory(recommendedOptions))
+    .map(parserOptionsMapper),
+  invalid: [
+    ...neverValid,
+  ]
+    .map(ruleOptionsMapperFactory(recommendedOptions))
+    .map(parserOptionsMapper),
+});
+
+ruleTester.run(`${ruleName}:strict`, rule, {
+  valid: [
+    ...alwaysValid,
+  ].map(parserOptionsMapper),
+  invalid: [
+    ...neverValid,
+    { code: '<div role="tabpanel" tabIndex="0" />', errors: [expectedError] },
+  ].map(parserOptionsMapper),
+});

docs/rules/no-noninteractive-tabindex.md

@@ -0,0 +1,87 @@
+# no-noninteractive-tabindex
+
+Tab key navigation should be limited to elements on the page that can be interacted with. Thus it is not necessary to add a tabindex to items in an unordered list, for example, to make them navigable through assistive technology. These applications already afford page traversal mechanisms based on the HTML of the page. Generally, we should try to reduce the size of the page's tab ring rather than increasing it.
+
+## How do I resolve this error?
+
+### Case: I am using an `<a>` tag. Isn't that interactive?
+
+The `<a>` tag is tricky. Consider the following:
+
+```
+<a>Edit</a>
+<a href="#">Edit</a>
+<a role="button">Edit</a>
+```
+
+The bare `<a>` tag is an _anchor_. It has no semantic AX API mapping in either ARIA or the AXObject model. It's as meaningful as `<div>`, which is to say it has no meaning. An `<a>` tag with an `href` attribute has an inherent role of `link`. An `<a>` tag with an explicit role obtains the designated role. In the example above, this role is `button`.
+
+### Case: I am using "semantic" HTML. Isn't that interactive?
+
+If we take a step back into the field of linguistics for a moment, let's consider what it means for something to be "semantic". Nothing, in and of itself, has meaning. Meaning is constructed through dialogue. A speaker intends a meaning and a listener/observer interprets a meaning. Each participant constructs their own meaning through dialogue. There is no intrinsic or isolated meaning outside of interaction. Thus, we must ask, given that we have a "speaker" who communicates via "semantic" HTML, who is listening/observing?
+
+In our case, the observer is the Accessibility (AX) API. Browsers interpret HTML (inflected at times by ARIA) to construct a meaning (AX Tree) of the page. Whatever the semantic HTML intends has only the force of suggestion to the AX API. Therefore, we have inconsistencies. For example, there is not yet an ARIA role for `text` or `label` and thus no way to change a `<label>` into plain text or a `<span>` into a label via ARIA. '<div>' has an AXObject correpondant `DivRole`, but no such object maps to `<span>`.
+
+What this lint rule endeavors to do is apply the AX API understanding of the semantics of an HTML document back onto your code. The concept of interactivity boils down to whether a user can do something with the indicated or focused component.
+
+Common interactive roles include:
+
+  1. `button`
+  1. `link`
+  1. `checkbox`
+  1. `menuitem`
+  1. `menuitemcheckbox`
+  1. `menuitemradio`
+  1. `option`
+  1. `radio`
+  1. `searchbox`
+  1. `switch`
+  1. `textbox`
+
+Endeavor to limit tabbable elements to those that a user can act upon.
+
+### Case: Shouldn't I add a tabindex so that users can navigate to this item?
+
+It is not necessary to put a tabindex on an `<article>`, for instance or on `<li>` items; assistive technologies provide affordances to users to find and traverse these containers. Most elements that require a tabindex -- `<a href>`, `<button>`, `<input>`, `<textarea>` -- have it already.
+
+Your application might require an exception to this rule in the case of an element that captures incoming tab traversal for a composite widget. In that case, turn off this rule on a per instance basis. This is an uncommon case.
+
+### References
+
+  1. [Fundamental Keyboard Navigation Conventions](https://www.w3.org/TR/wai-aria-practices-1.1/#kbd_generalnav)
+
+## Rule details
+
+The recommended options for this rule allow `tabIndex` on elements with the noninteractive `tabpanel` role. Adding `tabIndex` to a tabpanel is a recommended practice in some instances.
+
+```javascript
+'jsx-a11y/no-noninteractive-tabindex': [
+  'error',
+  {
+    tags: [],
+    roles: ['tabpanel'],
+  },
+]
+```
+
+### Succeed
+```jsx
+<div />
+<MyButton tabIndex={0} />
+<button />
+<button tabIndex="0" />
+<button tabIndex={0} />
+<div />
+<div tabIndex="-1" />
+<div role="button" tabIndex="0" />
+<div role="article" tabIndex="-1" />
+<article tabIndex="-1" />
+```
+
+### Fail
+```jsx
+<div tabIndex="0" />
+<div role="article" tabIndex="0" />
+<article tabIndex="0" />
+<article tabIndex={0} />
+```

src/index.js

@@ -27,6 +27,7 @@ module.exports = {
     'no-interactive-element-to-noninteractive-role': require('./rules/no-interactive-element-to-noninteractive-role'),
     'no-noninteractive-element-interactions': require('./rules/no-noninteractive-element-interactions'),
     'no-noninteractive-element-to-interactive-role': require('./rules/no-noninteractive-element-to-interactive-role'),
+    'no-noninteractive-tabindex': require('./rules/no-noninteractive-tabindex'),
     'no-onchange': require('./rules/no-onchange'),
     'no-redundant-roles': require('./rules/no-redundant-roles'),
     'no-static-element-interactions': require('./rules/no-static-element-interactions'),
@@ -100,7 +101,13 @@ module.exports = {
             td: ['gridcell'],
           },
         ],
-
+        'jsx-a11y/no-noninteractive-tabindex': [
+          'error',
+          {
+            tags: [],
+            roles: ['tabpanel'],
+          },
+        ],
         'jsx-a11y/no-onchange': 'error',
         'jsx-a11y/no-redundant-roles': 'error',
         'jsx-a11y/no-static-element-interactions': 'warn',
@@ -141,6 +148,7 @@ module.exports = {
         'jsx-a11y/no-interactive-element-to-noninteractive-role': 'error',
         'jsx-a11y/no-noninteractive-element-interactions': 'error',
         'jsx-a11y/no-noninteractive-element-to-interactive-role': 'error',
+        'jsx-a11y/no-noninteractive-tabindex': 'error',
         'jsx-a11y/no-onchange': 'error',
         'jsx-a11y/no-redundant-roles': 'error',
         'jsx-a11y/no-static-element-interactions': 'warn',

src/rules/no-noninteractive-tabindex.js

@@ -0,0 +1,96 @@
+/**
+ * @fileoverview Disallow tabindex on static and noninteractive elements
+ * @author jessebeach
+ * @flow
+ */
+
+// ----------------------------------------------------------------------------
+// Rule Definition
+// ----------------------------------------------------------------------------
+
+import {
+  dom,
+} from 'aria-query';
+import {
+  elementType,
+  getProp,
+  getLiteralPropValue,
+} from 'jsx-ast-utils';
+import isInteractiveElement from '../util/isInteractiveElement';
+import isInteractiveRole from '../util/isInteractiveRole';
+import { generateObjSchema, arraySchema } from '../util/schemas';
+import getTabIndex from '../util/getTabIndex';
+
+const errorMessage =
+  '`tabIndex` should only be declared on interactive elements.';
+
+const schema = generateObjSchema({
+  roles: {
+    ...arraySchema,
+    description: 'An array of ARIA roles',
+  },
+  tags: {
+    ...arraySchema,
+    description: 'An array of HTML tag names',
+  },
+});
+
+module.exports = {
+  meta: {
+    docs: {},
+    schema: [schema],
+  },
+
+  create: (context: ESLintContext) => {
+    const options = context.options;
+    return {
+      JSXOpeningElement: (
+          node: JSXOpeningElement,
+        ) => {
+        const type = elementType(node);
+        const attributes = node.attributes;
+        const tabIndexProp = getProp(attributes, 'tabIndex');
+        const tabIndex = getTabIndex(tabIndexProp);
+        // Early return;
+        if (typeof tabIndex === 'undefined') {
+          return;
+        }
+        const role = getLiteralPropValue(
+          getProp(node.attributes, 'role'),
+        );
+
+
+        if (!dom.has(type)) {
+            // Do not test higher level JSX components, as we do not know what
+            // low-level DOM element this maps to.
+          return;
+        }
+        // Allow for configuration overrides.
+        const {
+          tags,
+          roles,
+        } = (options[0] || {});
+        if (
+          (tags && tags.includes(type))
+          || (roles && roles.includes(role))
+        ) {
+          return;
+        }
+        if (
+          isInteractiveElement(type, attributes)
+          || isInteractiveRole(type, attributes)
+        ) {
+          return;
+        }
+        if (
+          tabIndex >= 0
+        ) {
+          context.report({
+            node: tabIndexProp,
+            message: errorMessage,
+          });
+        }
+      },
+    };
+  },
+};

src/util/schemas.js

@@ -6,7 +6,6 @@ export const arraySchema = {
   items: {
     type: 'string',
   },
-  minItems: 1,
   uniqueItems: true,
   additionalItems: false,
 };