Responding to PR feedback

Author: jessebeach

__mocks__/genInteractives.js

@@ -152,7 +152,11 @@ export function genInteractiveElements () {
 }
 
 export function genInteractiveRoleElements () {
-  return interactiveRoles.map(
+  return [
+    ...interactiveRoles,
+    'button article',
+    'fakerole button article',
+  ].map(
     value => JSXElementMock('div', [
       JSXAttributeMock('role', value)
     ])
@@ -175,7 +179,11 @@ export function genNonInteractiveElements () {
 }
 
 export function genNonInteractiveRoleElements () {
-  return nonInteractiveRoles.map(
+  return [
+    ...nonInteractiveRoles,
+    'article button',
+    'fakerole article button',
+  ].map(
     value => JSXElementMock('div', [
       JSXAttributeMock('role', value)
     ])

__tests__/src/util/isAbstractRole-test.js

@@ -8,7 +8,7 @@ import {
   genNonAbstractRoleElements,
 } from '../../../__mocks__/genInteractives';
 
-describe('isInteractiveRole', () => {
+describe('isAbstractRole', () => {
   describe('JSX Components (no tagName)', () => {
     it('should NOT identify them as abstract role elements', () => {
       expect(isAbstractRole(undefined, []))

docs/rules/interactive-supports-focus.md

@@ -6,7 +6,7 @@ Elements with an interactive role and interaction handlers (mouse or key press)
 
 ### Case: This element is a stand-alone control like a button, a link or a form element
 
-Add this prop to your component:
+Add the `tabIndex` property to your component. A value of zero indicates that this element can be tabbed to.
 
 ```
 <div
@@ -26,7 +26,7 @@ An element that can be tabbed to is said to be in the _tab ring_.
 One item in a group should have a tabindex of zero, the rest should have a tabindex of -1. A value of zero makes the element _tabbable_. A value of -1 makes the element _focusable_.
 
 ```
-<div role="men">
+<div role="menu">
   <div role="menuitem" tabIndex="0">Open</div>
   <div role="menuitem" tabIndex="-1">Save</div>
   <div role="menuitem" tabIndex="-1">Close</div>
@@ -41,7 +41,7 @@ If your element is catching bubbled click or key events from descendant elements
 
 ```
 <div
-  onClick="onClickHandler"
+  onClick={onClickHandler}
   role="presentation">
   <button>Save</button>
 </div>

docs/rules/no-interactive-element-to-noninteractive-role.md

@@ -41,4 +41,23 @@ Put the content inside your interactive element.
 
 ## Rule details
 
-This rule takes no arguments.
+The recommended options for this rule allow the `tr` element to be given a role of `presentation` (or its semantic equivalent `none`). Under normal circumstances, an element with an interactive role should not be semantically neutralized with `presentation` (or `none`).
+
+Options are provided as an object keyed by HTML element name; the value is an array of interactive roles that are allowed on the specified element.
+
+```
+{
+  'no-interactive-element-to-noninteractive-role': [
+    'error',
+    {
+      tr: ['none', 'presentation'],
+    },
+  ]
+}
+```
+
+Under the recommended options, the following code is valid. It would be invalid under the strict rules.
+
+```
+<tr role="presentation" />
+```

docs/rules/no-noninteractive-element-to-interactive-role.md

@@ -45,4 +45,24 @@ Or wrap the content inside your interactive element.
 
 ## Rule details
 
-This rule takes no arguments.
+The recommended options for this rule allow several common interactive roles to be applied to a non-interactive element. The options are provided as an object keyed by HTML element name; the value is an array of interactive roles that are allowed on the specified element.
+
+```
+{
+  'no-noninteractive-element-to-interactive-role': [
+  'error',
+  {
+    ul: ['listbox', 'menu', 'menubar', 'radiogroup', 'tablist', 'tree', 'treegrid'],
+    ol: ['listbox', 'menu', 'menubar', 'radiogroup', 'tablist', 'tree', 'treegrid'],
+    li: ['menuitem', 'option', 'row', 'tab', 'treeitem'],
+    table: ['grid'],
+    td: ['gridcell'],
+  },
+}
+```
+
+Under the recommended options, the following code is valid. It would be invalid under the strict rules.
+
+```
+<ul role="menu" />
+```

src/rules/no-interactive-element-to-noninteractive-role.js

@@ -16,6 +16,7 @@ import {
   elementType,
   getProp,
   getLiteralPropValue,
+  propName,
 } from 'jsx-ast-utils';
 import type { JSXIdentifier } from 'ast-types-flow';
 import isInteractiveElement from '../util/isInteractiveElement';
@@ -48,7 +49,7 @@ module.exports = {
       JSXAttribute: (
         attribute: ESLintJSXAttribute,
       ) => {
-        const attributeName: JSXIdentifier = attribute.name.name;
+        const attributeName: JSXIdentifier = propName(attribute);
         if (attributeName !== 'role') {
           return;
         }

src/rules/no-static-element-interactions.js

@@ -72,7 +72,7 @@ module.exports = {
         || isNonInteractiveRole(type, attributes)
         || isAbstractRole(type, attributes)
       ) {
-        // This rule has no opinion about abtract roles.
+        // This rule has no opinion about abstract roles.
         return;
       }
 

src/util/isInteractiveRole.js

@@ -1,12 +1,13 @@
 import {
-  roles,
+  roles as rolesMap,
 } from 'aria-query';
 import type { Node } from 'ast-types-flow';
 import { getProp, getLiteralPropValue } from 'jsx-ast-utils';
 
-const interactiveRoles = [...roles.keys()]
-  .filter(name => !roles.get(name).abstract)
-  .filter(name => roles.get(name).superClass.some(
+const roles = [...rolesMap.keys()];
+const interactiveRoles = roles
+  .filter(name => !rolesMap.get(name).abstract)
+  .filter(name => rolesMap.get(name).superClass.some(
     klasses => klasses.includes('widget')),
   );
 
@@ -39,10 +40,18 @@ const isInteractiveRole = (
 
   let isInteractive = false;
   const normalizedValues = String(value).toLowerCase().split(' ');
-  if (normalizedValues.length > 0) {
-    // The last role value is a series takes precedence.
-    const val = normalizedValues[normalizedValues.length - 1];
-    isInteractive = interactiveRoles.indexOf(val) > -1;
+  const validRoles = normalizedValues.reduce((
+    accumulator: Array<string>,
+    name: string,
+  ) => {
+    if (roles.includes(name)) {
+      accumulator.push(name);
+    }
+    return accumulator;
+  }, []);
+  if (validRoles.length > 0) {
+    // The first role value is a series takes precedence.
+    isInteractive = interactiveRoles.includes(validRoles[0]);
   }
 
   return isInteractive;

src/util/isNonInteractiveRole.js

@@ -4,18 +4,17 @@
 
 import {
   dom,
-  roles,
+  roles as rolesMap,
 } from 'aria-query';
 import type { Node } from 'ast-types-flow';
 import { getProp, getLiteralPropValue } from 'jsx-ast-utils';
 
-const nonInteractiveRoles = new Set(
-  [...roles.keys()]
-    .filter(name => !roles.get(name).abstract)
-    .filter(name => !roles.get(name).superClass.some(
-      klasses => klasses.includes('widget')),
-    ),
-);
+const roles = [...rolesMap.keys()];
+const nonInteractiveRoles = roles
+  .filter(name => !rolesMap.get(name).abstract)
+  .filter(name => !rolesMap.get(name).superClass.some(
+    klasses => klasses.includes('widget')),
+  );
 
 /**
  * Returns boolean indicating whether the given element has a role
@@ -46,7 +45,24 @@ const isNonInteractiveRole = (
   }
 
   const role = getLiteralPropValue(getProp(attributes, 'role'));
-  return nonInteractiveRoles.has(role);
+
+  let isNonInteractive = false;
+  const normalizedValues = String(role).toLowerCase().split(' ');
+  const validRoles = normalizedValues.reduce((
+    accumulator: Array<string>,
+    name: string,
+  ) => {
+    if (roles.includes(name)) {
+      accumulator.push(name);
+    }
+    return accumulator;
+  }, []);
+  if (validRoles.length > 0) {
+    // The first role value is a series takes precedence.
+    isNonInteractive = nonInteractiveRoles.includes(validRoles[0]);
+  }
+
+  return isNonInteractive;
 };
 
 export default isNonInteractiveRole;