scope no-onchange to select menu elements (#61)

Fixes #55

Author: octatone

README.md

@@ -84,7 +84,7 @@ Then configure the rules you want to use under the rules section.
 - [mouse-events-have-key-events](docs/rules/mouse-events-have-key-events.md): Enforce that `onMouseOver`/`onMouseOut` are accompanied by `onFocus`/`onBlur` for keyboard-only users.
 - [no-access-key](docs/rules/no-access-key.md): Enforce that the `accessKey` prop is not used on any element to avoid complications with keyboard commands used by a screenreader.
 - [no-marquee](docs/rules/no-marquee.md): Enforce `<marquee>` elements are not used.
-- [no-onchange](docs/rules/no-onchange.md): Enforce that `onBlur` is used instead of `onChange`.
+- [no-onchange](docs/rules/no-onchange.md): Enforce usage of `onBlur` over `onChange` on select menus for accessibility.
 - [onclick-has-focus](docs/rules/onclick-has-focus.md): Enforce that elements with `onClick` handlers must be focusable.
 - [onclick-has-role](docs/rules/onclick-has-role.md): Enforce that non-interactive, visible elements (such as `<div>`) that have click handlers use the role attribute.
 - [role-has-required-aria-props](docs/rules/role-has-required-aria-props.md): Enforce that elements with ARIA roles must have all required attributes for that role.

docs/rules/no-onchange.md

@@ -1,6 +1,6 @@
 # no-onchange
 
-Enforce usage of `onBlur` over/in parallel with `onChange` for accessibility. `onBlur` **should** be used instead of `onChange`, unless absolutely necessary and it causes no negative consequences for keyboard only or screen reader users. `onBlur` is a more declarative action by the user: for instance in a dropdown, using the arrow keys to toggle between options will trigger the `onChange` event in some browsers. Regardless, when a change of context results from an `onBlur` event or an `onChange` event, the user should be notified of the change unless it occurs below the currently focused element.
+Enforce usage of `onBlur` over/in parallel with `onChange` on select menu elements for accessibility. `onBlur` **should** be used instead of `onChange`, unless absolutely necessary and it causes no negative consequences for keyboard only or screen reader users. `onBlur` is a more declarative action by the user: for instance in a dropdown, using the arrow keys to toggle between options will trigger the `onChange` event in some browsers. Regardless, when a change of context results from an `onBlur` event or an `onChange` event, the user should be notified of the change unless it occurs below the currently focused element.
 
 #### References
 1. [onChange Event Accessibility Issues](http://cita.disability.uiuc.edu/html-best-practices/auto/onchange.php)
@@ -12,11 +12,16 @@ This rule takes no arguments.
 
 ### Succeed
 ```jsx
-<input onBlur={updateModel} />
-<input onBlur={handleOnBlur} onChange={handleOnChange} />
+<select onBlur={updateModel}>
+  <option/>
+</select>
+
+<select>
+  <option onBlur={handleOnBlur} onChange={handleOnChange} />
+</select>
 ```
 
 ### Fail
 ```jsx
-<input onChange={updateModel} />
+<select onChange={updateModel} />
 ```

src/rules/no-onchange.js

@@ -7,14 +7,25 @@
 // Rule Definition
 // ----------------------------------------------------------------------------
 
-import { getProp } from 'jsx-ast-utils';
+import { getProp, elementType } from 'jsx-ast-utils';
 
 const errorMessage = 'onBlur must be used instead of onchange, ' +
   'unless absolutely necessary and it causes no negative consequences ' +
   'for keyboard only or screen reader users.';
 
+const applicableTypes = [
+  'select',
+  'option',
+];
+
 module.exports = context => ({
   JSXOpeningElement: node => {
+    const nodeType = elementType(node);
+
+    if (applicableTypes.indexOf(nodeType) === -1) {
+      return;
+    }
+
     const onChange = getProp(node.attributes, 'onChange');
     const hasOnBlur = getProp(node.attributes, 'onBlur') !== undefined;
 

tests/src/rules/no-onchange.js

@@ -1,5 +1,5 @@
 /**
- * @fileoverview Enforce usage of onBlur over onChange for accessibility.
+ * @fileoverview Enforce usage of onBlur over onChange on select menus for accessibility.
  * @author Ethan Cohen
  */
 
@@ -32,16 +32,21 @@ const expectedError = {
 
 ruleTester.run('no-onchange', rule, {
   valid: [
-    { code: '<div onBlur={() => {}} />;', parserOptions },
-    { code: '<div onBlur={handleOnBlur} />;', parserOptions },
-    { code: '<div />;', parserOptions },
-    { code: '<div onBlur={() => {}} onChange={() => {}} />;', parserOptions },
-    { code: '<div {...props} />', parserOptions },
+    { code: '<select onBlur={() => {}} />;', parserOptions },
+    { code: '<select onBlur={handleOnBlur} />;', parserOptions },
+    { code: '<option />;', parserOptions },
+    { code: '<option onBlur={() => {}} onChange={() => {}} />;', parserOptions },
+    { code: '<option {...props} />', parserOptions },
+    { code: '<input onChange={() => {}} />;', parserOptions },
+    { code: '<input onChange={handleOnChange} />;', parserOptions },
+    { code: '<input />;', parserOptions },
+    { code: '<input onChange={() => {}} onChange={() => {}} />;', parserOptions },
+    { code: '<input {...props} />', parserOptions },
   ],
   invalid: [
-    { code: '<div onChange={() => {}} />;', errors: [expectedError], parserOptions },
-    { code: '<div onChange={handleOnChange} />;', errors: [expectedError], parserOptions },
-    { code: '<input onChange={() => {}} />', errors: [expectedError], parserOptions },
-    { code: '<input onChange={() => {}} {...props} />', errors: [expectedError], parserOptions },
+    { code: '<select onChange={() => {}} />;', errors: [expectedError], parserOptions },
+    { code: '<select onChange={handleOnChange} />;', errors: [expectedError], parserOptions },
+    { code: '<option onChange={() => {}} />', errors: [expectedError], parserOptions },
+    { code: '<option onChange={() => {}} {...props} />', errors: [expectedError], parserOptions },
   ],
 });