Skip to content

Commit 770eaad

Browse files
authored
Merge pull request #124 from jessebeach/util-isInteractiveRole
Explicitly distinguish interactive from non-interactive elements and roles
2 parents 8b6d82a + 7f0f06a commit 770eaad

14 files changed

+720
-112
lines changed

.babelrc

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
11
{
2-
"presets": ["es2015"]
2+
"presets": ["es2015"],
3+
"plugins": [
4+
"transform-object-rest-spread"
5+
]
36
}

__mocks__/JSXElementMock.js

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
export default function JSXElementMock (tagName, attributes) {
2+
return {
3+
type: 'JSXElement',
4+
openingElement: {
5+
type: 'JSXOpeningElement',
6+
name: {
7+
type: 'JSXIdentifier',
8+
name: tagName
9+
},
10+
attributes,
11+
},
12+
};
13+
}

__mocks__/genInteractives.js

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
import JSXAttributeMock from './JSXAttributeMock';
2+
import JSXElementMock from './JSXElementMock';
3+
import DOMElements from '../src/util/attributes/DOM.json';
4+
import roles from '../src/util/attributes/role.json';
5+
6+
const pureInteractiveElements = Object.keys(DOMElements)
7+
.filter(name => DOMElements[name].interactive === true)
8+
.reduce((interactiveElements, name) => {
9+
interactiveElements[name] = [];
10+
return interactiveElements;
11+
}, {});
12+
13+
const interactiveElementsMap = {
14+
...pureInteractiveElements,
15+
a: [
16+
{prop: 'href', value: '#'}
17+
],
18+
area: [
19+
{prop: 'href', value: '#'}
20+
],
21+
input: [
22+
{prop: 'type', value: 'text'}
23+
],
24+
};
25+
26+
const pureNonInteractiveElementsMap = Object.keys(DOMElements)
27+
.filter(name => !DOMElements[name].interactive)
28+
.reduce((nonInteractiveElements, name) => {
29+
nonInteractiveElements[name] = [];
30+
return nonInteractiveElements;
31+
}, {});
32+
33+
const nonInteractiveElementsMap = {
34+
...pureNonInteractiveElementsMap,
35+
input: [
36+
{prop: 'type', value: 'hidden'}
37+
],
38+
};
39+
40+
const interactiveRoles = Object.keys(roles).filter(
41+
role => roles[role].interactive === true
42+
);
43+
44+
const nonInteractiveRoles = Object.keys(roles).filter(
45+
role => roles[role].interactive === false
46+
);
47+
48+
export function genInteractiveElements () {
49+
return Object.keys(interactiveElementsMap)
50+
.map(name => {
51+
const attributes = interactiveElementsMap[name].map(
52+
({prop, value}) => JSXAttributeMock(prop, value)
53+
);
54+
return JSXElementMock(name, attributes);
55+
});
56+
}
57+
58+
export function genInteractiveRoleElements () {
59+
return interactiveRoles.map(
60+
value => JSXElementMock('div', [
61+
JSXAttributeMock('role', value)
62+
])
63+
);
64+
}
65+
66+
export function genNonInteractiveElements () {
67+
return Object.keys(nonInteractiveElementsMap)
68+
.map(name => {
69+
const attributes = nonInteractiveElementsMap[name].map(
70+
({prop, value}) => JSXAttributeMock(prop, value)
71+
);
72+
return JSXElementMock(name, attributes);
73+
});
74+
}
75+
76+
export function genNonInteractiveRoleElements () {
77+
return nonInteractiveRoles.map(
78+
value => JSXElementMock('div', [
79+
JSXAttributeMock('role', value)
80+
])
81+
);
82+
}

__tests__/src/rules/onclick-has-focus-test.js

Lines changed: 88 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,10 @@ const parserOptions = {
2525
const ruleTester = new RuleTester();
2626

2727
const expectedError = {
28-
message: 'Elements with onClick handlers must be focusable. ' +
29-
'Either set the tabIndex property to a valid value (usually 0), ' +
30-
'or use an element type which is inherently focusable such as `button`.',
28+
message: 'An non-interactive element with an onClick handler and an ' +
29+
'interactive role must be focusable. Either set the tabIndex property to ' +
30+
'a valid value (usually 0) or use an element type which is inherently ' +
31+
'focusable such as `button`.',
3132
type: 'JSXOpeningElement',
3233
};
3334

@@ -43,94 +44,159 @@ ruleTester.run('onclick-has-focus', rule, {
4344
{ code: '<div aria-hidden={1 <= 2} onClick={() => void 0} />', parserOptions },
4445
{ code: '<div aria-hidden={2 > 1} onClick={() => void 0} />', parserOptions },
4546
{ code: '<div aria-hidden={2 >= 1} onClick={() => void 0} />', parserOptions },
47+
{ code: '<div onClick={() => void 0} />;', parserOptions },
48+
{ code: '<div onClick={() => void 0} tabIndex={undefined} />;', parserOptions },
49+
{ code: '<div onClick={() => void 0} tabIndex="bad" />;', parserOptions },
50+
{ code: '<div onClick={() => void 0} role={undefined} />;', parserOptions },
51+
{ code: '<div role="section" onClick={() => void 0} />', parserOptions },
52+
{ code: '<div onClick={() => void 0} aria-hidden={false} />;', parserOptions },
53+
{ code: '<div onClick={() => void 0} {...props} />;', parserOptions },
4654
{ code: '<input type="text" onClick={() => void 0} />', parserOptions },
4755
{ code: '<input type="hidden" onClick={() => void 0} tabIndex="-1" />', parserOptions },
4856
{ code: '<input type="hidden" onClick={() => void 0} tabIndex={-1} />', parserOptions },
4957
{ code: '<input onClick={() => void 0} />', parserOptions },
58+
{ code: '<input onClick={() => void 0} role="combobox" />', parserOptions },
5059
{ code: '<button onClick={() => void 0} className="foo" />', parserOptions },
5160
{ code: '<option onClick={() => void 0} className="foo" />', parserOptions },
5261
{ code: '<select onClick={() => void 0} className="foo" />', parserOptions },
5362
{ code: '<area href="#" onClick={() => void 0} className="foo" />', parserOptions },
63+
{ code: '<area onClick={() => void 0} className="foo" />', parserOptions },
5464
{ code: '<textarea onClick={() => void 0} className="foo" />', parserOptions },
65+
{ code: '<a onClick="showNextPage();">Next page</a>', parserOptions },
66+
{ code: '<a onClick="showNextPage();" tabIndex={undefined}>Next page</a>', parserOptions },
67+
{ code: '<a onClick="showNextPage();" tabIndex="bad">Next page</a>', parserOptions },
68+
{ code: '<a onClick={() => void 0} />', parserOptions },
5569
{ code: '<a tabIndex="0" onClick={() => void 0} />', parserOptions },
5670
{ code: '<a tabIndex={dynamicTabIndex} onClick={() => void 0} />', parserOptions },
5771
{ code: '<a tabIndex={0} onClick={() => void 0} />', parserOptions },
5872
{ code: '<a role="button" href="#" onClick={() => void 0} />', parserOptions },
5973
{ code: '<a onClick={() => void 0} href="http://x.y.z" />', parserOptions },
6074
{ code: '<a onClick={() => void 0} href="http://x.y.z" tabIndex="0" />', parserOptions },
6175
{ code: '<a onClick={() => void 0} href="http://x.y.z" tabIndex={0} />', parserOptions },
76+
{ code: '<a onClick={() => void 0} href="http://x.y.z" role="button" />', parserOptions },
6277
{ code: '<TestComponent onClick={doFoo} />', parserOptions },
6378
{ code: '<input onClick={() => void 0} type="hidden" />;', parserOptions },
79+
{ code: '<span onClick="submitForm();">Submit</span>', errors: [expectedError], parserOptions },
80+
{ code: '<span onClick="submitForm();" tabIndex={undefined}>Submit</span>', parserOptions },
81+
{ code: '<span onClick="submitForm();" tabIndex="bad">Submit</span>', parserOptions },
6482
{ code: '<span onClick="doSomething();" tabIndex="0">Click me!</span>', parserOptions },
6583
{ code: '<span onClick="doSomething();" tabIndex={0}>Click me!</span>', parserOptions },
6684
{ code: '<span onClick="doSomething();" tabIndex="-1">Click me too!</span>', parserOptions },
6785
{
6886
code: '<a href="javascript:void(0);" onClick="doSomething();">Click ALL the things!</a>',
6987
parserOptions,
7088
},
89+
{ code: '<section onClick={() => void 0} />;', parserOptions },
90+
{ code: '<main onClick={() => void 0} />;', parserOptions },
91+
{ code: '<article onClick={() => void 0} />;', parserOptions },
92+
{ code: '<header onClick={() => void 0} />;', parserOptions },
93+
{ code: '<footer onClick={() => void 0} />;', parserOptions },
94+
{ code: '<div role="button" tabIndex="0" onClick={() => void 0} />', parserOptions },
95+
{ code: '<div role="checkbox" tabIndex="0" onClick={() => void 0} />', parserOptions },
96+
{ code: '<div role="link" tabIndex="0" onClick={() => void 0} />', parserOptions },
97+
{ code: '<div role="menuitem" tabIndex="0" onClick={() => void 0} />', parserOptions },
98+
{ code: '<div role="menuitemcheckbox" tabIndex="0" onClick={() => void 0} />', parserOptions },
99+
{ code: '<div role="menuitemradio" tabIndex="0" onClick={() => void 0} />', parserOptions },
100+
{ code: '<div role="option" tabIndex="0" onClick={() => void 0} />', parserOptions },
101+
{ code: '<div role="radio" tabIndex="0" onClick={() => void 0} />', parserOptions },
102+
{ code: '<div role="spinbutton" tabIndex="0" onClick={() => void 0} />', parserOptions },
103+
{ code: '<div role="switch" tabIndex="0" onClick={() => void 0} />', parserOptions },
104+
{ code: '<div role="tab" tabIndex="0" onClick={() => void 0} />', parserOptions },
105+
{ code: '<div role="textbox" tabIndex="0" onClick={() => void 0} />', parserOptions },
71106
{ code: '<Foo.Bar onClick={() => void 0} aria-hidden={false} />;', parserOptions },
72107
{ code: '<Input onClick={() => void 0} type="hidden" />;', parserOptions },
73108
],
74109

75110
invalid: [
76-
{ code: '<span onClick="submitForm();">Submit</span>', errors: [expectedError], parserOptions },
77111
{
78-
code: '<span onClick="submitForm();" tabIndex={undefined}>Submit</span>',
112+
code: '<span role="button" onClick={() => void 0} />',
113+
errors: [expectedError],
114+
parserOptions,
115+
},
116+
{
117+
code: '<a role="button" onClick={() => void 0} />',
118+
errors: [expectedError],
119+
parserOptions,
120+
},
121+
{
122+
code: '<div role="button" onClick={() => void 0} />',
123+
errors: [expectedError],
124+
parserOptions,
125+
},
126+
{
127+
code: '<div role="checkbox" onClick={() => void 0} />',
128+
errors: [expectedError],
129+
parserOptions,
130+
},
131+
{
132+
code: '<div role="link" onClick={() => void 0} />',
133+
errors: [expectedError],
134+
parserOptions,
135+
},
136+
{
137+
code: '<div role="gridcell" onClick={() => void 0} />',
138+
errors: [expectedError],
139+
parserOptions,
140+
},
141+
{
142+
code: '<div role="menuitem" onClick={() => void 0} />',
143+
errors: [expectedError],
144+
parserOptions,
145+
},
146+
{
147+
code: '<div role="menuitemcheckbox" onClick={() => void 0} />',
148+
errors: [expectedError],
149+
parserOptions,
150+
},
151+
{
152+
code: '<div role="menuitemradio" onClick={() => void 0} />',
79153
errors: [expectedError],
80154
parserOptions,
81155
},
82156
{
83-
code: '<span onClick="submitForm();" tabIndex="bad">Submit</span>',
157+
code: '<div role="option" onClick={() => void 0} />',
84158
errors: [expectedError],
85159
parserOptions,
86160
},
87-
{ code: '<a onClick="showNextPage();">Next page</a>', errors: [expectedError], parserOptions },
88161
{
89-
code: '<a onClick="showNextPage();" tabIndex={undefined}>Next page</a>',
162+
code: '<div role="radio" onClick={() => void 0} />',
90163
errors: [expectedError],
91164
parserOptions,
92165
},
93166
{
94-
code: '<a onClick="showNextPage();" tabIndex="bad">Next page</a>',
167+
code: '<div role="searchbox" onClick={() => void 0} />',
95168
errors: [expectedError],
96169
parserOptions,
97170
},
98171
{
99-
code: '<a onClick={() => void 0} />',
172+
code: '<div role="slider" onClick={() => void 0} />',
100173
errors: [expectedError],
101174
parserOptions,
102175
},
103176
{
104-
code: '<area onClick={() => void 0} className="foo" />',
177+
code: '<div role="spinbutton" onClick={() => void 0} />',
105178
errors: [expectedError],
106179
parserOptions,
107180
},
108-
{ code: '<div onClick={() => void 0} />;', errors: [expectedError], parserOptions },
109181
{
110-
code: '<div onClick={() => void 0} tabIndex={undefined} />;',
182+
code: '<div role="switch" onClick={() => void 0} />',
111183
errors: [expectedError],
112184
parserOptions,
113185
},
114186
{
115-
code: '<div onClick={() => void 0} tabIndex="bad" />;',
187+
code: '<div role="tab" onClick={() => void 0} />',
116188
errors: [expectedError],
117189
parserOptions,
118190
},
119191
{
120-
code: '<div onClick={() => void 0} role={undefined} />;',
192+
code: '<div role="textbox" onClick={() => void 0} />',
121193
errors: [expectedError],
122194
parserOptions,
123195
},
124196
{
125-
code: '<div onClick={() => void 0} aria-hidden={false} />;',
197+
code: '<div role="treeitem" onClick={() => void 0} />',
126198
errors: [expectedError],
127199
parserOptions,
128200
},
129-
{ code: '<div onClick={() => void 0} {...props} />;', errors: [expectedError], parserOptions },
130-
{ code: '<section onClick={() => void 0} />;', errors: [expectedError], parserOptions },
131-
{ code: '<main onClick={() => void 0} />;', errors: [expectedError], parserOptions },
132-
{ code: '<article onClick={() => void 0} />;', errors: [expectedError], parserOptions },
133-
{ code: '<header onClick={() => void 0} />;', errors: [expectedError], parserOptions },
134-
{ code: '<footer onClick={() => void 0} />;', errors: [expectedError], parserOptions },
135201
],
136202
});
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
/* eslint-env mocha */
2+
import expect from 'expect';
3+
import { elementType } from 'jsx-ast-utils';
4+
import isInteractiveElement from '../../../src/util/isInteractiveElement';
5+
import {
6+
genInteractiveElements,
7+
genNonInteractiveElements,
8+
} from '../../../__mocks__/genInteractives';
9+
10+
describe('isInteractiveElement', () => {
11+
describe('JSX Components (no tagName)', () => {
12+
it('should identify them as interactive elements', () => {
13+
expect(isInteractiveElement(undefined, []))
14+
.toBe(true);
15+
});
16+
});
17+
describe('non-interactive elements', () => {
18+
genNonInteractiveElements().forEach(
19+
({ openingElement }) => {
20+
it(`should not identify \`${openingElement.name.name}\` as an interactive element`, () => {
21+
expect(isInteractiveElement(
22+
elementType(openingElement),
23+
openingElement.attributes,
24+
)).toBe(false);
25+
});
26+
},
27+
);
28+
});
29+
describe('interactive elements', () => {
30+
genInteractiveElements().forEach(
31+
({ openingElement }) => {
32+
it(`should identify \`${openingElement.name.name}\` as an interactive element`, () => {
33+
expect(isInteractiveElement(
34+
elementType(openingElement),
35+
openingElement.attributes,
36+
)).toBe(true);
37+
});
38+
},
39+
);
40+
});
41+
});
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
/* eslint-env mocha */
2+
import expect from 'expect';
3+
import { elementType, getProp, getLiteralPropValue } from 'jsx-ast-utils';
4+
import isInteractiveRole from '../../../src/util/isInteractiveRole';
5+
import {
6+
genInteractiveRoleElements,
7+
genNonInteractiveRoleElements,
8+
} from '../../../__mocks__/genInteractives';
9+
10+
describe('isInteractiveRole', () => {
11+
describe('JSX Components (no tagName)', () => {
12+
it('should identify them as interactive role elements', () => {
13+
expect(isInteractiveRole(undefined, []))
14+
.toBe(true);
15+
});
16+
});
17+
describe('elements with a non-interactive role', () => {
18+
genNonInteractiveRoleElements().forEach(
19+
({ openingElement }) => {
20+
const attributes = openingElement.attributes;
21+
const role = getLiteralPropValue(getProp(attributes, 'role')).toLowerCase();
22+
it(`should not identify \`${role}\` as an interactive role element`, () => {
23+
expect(isInteractiveRole(
24+
elementType(openingElement),
25+
attributes,
26+
)).toBe(false);
27+
});
28+
},
29+
);
30+
});
31+
describe('elements without a role', () => {
32+
it('should not identify them as interactive role elements', () => {
33+
expect(isInteractiveRole('div', [])).toBe(false);
34+
});
35+
});
36+
describe('elements with an interactive role', () => {
37+
genInteractiveRoleElements().forEach(
38+
({ openingElement }) => {
39+
const attributes = openingElement.attributes;
40+
const role = getLiteralPropValue(getProp(attributes, 'role')).toLowerCase();
41+
it(`should identify \`${role}\` as an interactive role element`, () => {
42+
expect(isInteractiveRole(
43+
elementType(openingElement),
44+
attributes,
45+
)).toBe(true);
46+
});
47+
},
48+
);
49+
});
50+
});

0 commit comments

Comments
 (0)