Skip to content

Commit d13725d

Browse files
lb-ljharb
lb-
authored and
ljharb
committed
[Fix] no-redundant-roles: Refine implicit role of select to include combobox scenarios
Encode implicit roles for `select` elements based on roles defined in https://www.w3.org/TR/html-aria/#el-select - `select` (with a multiple attribute or a size attribute having value greater than 1) will have the implicit role 'listbox' - `select` (with NO multiple attribute and NO size attribute having value greater than 1) will have the implicit role 'combobox' Fixes #949
1 parent 068608b commit d13725d

File tree

3 files changed

+193
-4
lines changed

3 files changed

+193
-4
lines changed

‎__tests__/src/rules/no-redundant-roles-test.js

+20-1
Original file line numberDiff line numberDiff line change
@@ -41,12 +41,31 @@ const alwaysValid = [
4141
{ code: '<MyComponent role="button" />' },
4242
{ code: '<button role={`${foo}button`} />' },
4343
{ code: '<Button role={`${foo}button`} />', settings: componentsSettings },
44+
{ code: '<select role="menu"><option>1</option><option>2</option></select>' },
45+
{ code: '<select role="menu" size={2}><option>1</option><option>2</option></select>' },
46+
{ code: '<select role="menu" multiple><option>1</option><option>2</option></select>' },
4447
];
4548

4649
const neverValid = [
47-
{ code: '<button role="button" />', errors: [expectedError('button', 'button')] },
4850
{ code: '<body role="DOCUMENT" />', errors: [expectedError('body', 'document')] },
51+
// button - treated as button by default
52+
{ code: '<button role="button" />', errors: [expectedError('button', 'button')] },
4953
{ code: '<Button role="button" />', settings: componentsSettings, errors: [expectedError('button', 'button')] },
54+
// select - treated as combobox by default
55+
{ code: '<select role="combobox"><option>1</option><option>2</option></select>', errors: [expectedError('select', 'combobox')] },
56+
{ code: '<select role="combobox" size="" />', errors: [expectedError('select', 'combobox')] },
57+
{ code: '<select role="combobox" size={1} />', errors: [expectedError('select', 'combobox')] },
58+
{ code: '<select role="combobox" size="1" />', errors: [expectedError('select', 'combobox')] },
59+
{ code: '<select role="combobox" size={null}></select>', errors: [expectedError('select', 'combobox')] },
60+
{ code: '<select role="combobox" size={undefined}></select>', errors: [expectedError('select', 'combobox')] },
61+
{ code: '<select role="combobox" multiple={undefined}></select>', errors: [expectedError('select', 'combobox')] },
62+
{ code: '<select role="combobox" multiple={false}></select>', errors: [expectedError('select', 'combobox')] },
63+
{ code: '<select role="combobox" multiple=""></select>', errors: [expectedError('select', 'combobox')] },
64+
// select - treated as listbox when multiple OR size > 1
65+
{ code: '<select role="listbox" size="3" />', errors: [expectedError('select', 'listbox')] },
66+
{ code: '<select role="listbox" size={2} />', errors: [expectedError('select', 'listbox')] },
67+
{ code: '<select role="listbox" multiple><option>1</option><option>2</option></select>', errors: [expectedError('select', 'listbox')] },
68+
{ code: '<select role="listbox" multiple={true}></select>', errors: [expectedError('select', 'listbox')] },
5069
];
5170

5271
ruleTester.run(`${ruleName}:recommended`, rule, {

‎__tests__/src/util/implicitRoles/select-test.js

+158
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,158 @@
1+
import test from 'tape';
2+
3+
import JSXAttributeMock from '../../../../__mocks__/JSXAttributeMock';
4+
import getImplicitRoleForSelect from '../../../../src/util/implicitRoles/select';
5+
6+
test('isAbstractRole', (t) => {
7+
t.test('works for combobox', (st) => {
8+
st.equal(
9+
getImplicitRoleForSelect([]),
10+
'combobox',
11+
'defaults to combobox',
12+
);
13+
14+
st.equal(
15+
getImplicitRoleForSelect([JSXAttributeMock('multiple', null)]),
16+
'combobox',
17+
'is combobox when multiple attribute is set to not be present',
18+
);
19+
20+
st.equal(
21+
getImplicitRoleForSelect([JSXAttributeMock('multiple', undefined)]),
22+
'combobox',
23+
'is combobox when multiple attribute is set to not be present',
24+
);
25+
26+
st.equal(
27+
getImplicitRoleForSelect([JSXAttributeMock('multiple', false)]),
28+
'combobox',
29+
'is combobox when multiple attribute is set to boolean false',
30+
);
31+
32+
st.equal(
33+
getImplicitRoleForSelect([JSXAttributeMock('multiple', '')]),
34+
'combobox',
35+
'is listbox when multiple attribute is falsey (empty string)',
36+
);
37+
38+
st.equal(
39+
getImplicitRoleForSelect([JSXAttributeMock('size', '1')]),
40+
'combobox',
41+
'is combobox when size is not greater than 1',
42+
);
43+
44+
st.equal(
45+
getImplicitRoleForSelect([JSXAttributeMock('size', 1)]),
46+
'combobox',
47+
'is combobox when size is not greater than 1',
48+
);
49+
50+
st.equal(
51+
getImplicitRoleForSelect([JSXAttributeMock('size', 0)]),
52+
'combobox',
53+
'is combobox when size is not greater than 1',
54+
);
55+
56+
st.equal(
57+
getImplicitRoleForSelect([JSXAttributeMock('size', '0')]),
58+
'combobox',
59+
'is combobox when size is not greater than 1',
60+
);
61+
62+
st.equal(
63+
getImplicitRoleForSelect([JSXAttributeMock('size', '-1')]),
64+
'combobox',
65+
'is combobox when size is not greater than 1',
66+
);
67+
68+
st.equal(
69+
getImplicitRoleForSelect([JSXAttributeMock('size', '')]),
70+
'combobox',
71+
'is combobox when size is a valid number',
72+
);
73+
74+
st.equal(
75+
getImplicitRoleForSelect([JSXAttributeMock('size', 'true')]),
76+
'combobox',
77+
'is combobox when size is a valid number',
78+
);
79+
80+
st.equal(
81+
getImplicitRoleForSelect([JSXAttributeMock('size', true)]),
82+
'combobox',
83+
'is combobox when size is a valid number',
84+
);
85+
86+
st.equal(
87+
getImplicitRoleForSelect([JSXAttributeMock('size', NaN)]),
88+
'combobox',
89+
'is combobox when size is a valid number',
90+
);
91+
92+
st.equal(
93+
getImplicitRoleForSelect([JSXAttributeMock('size', '')]),
94+
'combobox',
95+
'is combobox when size is a valid number',
96+
);
97+
98+
st.equal(
99+
getImplicitRoleForSelect([JSXAttributeMock('size', undefined)]),
100+
'combobox',
101+
'is combobox when size is a valid number',
102+
);
103+
104+
st.equal(
105+
getImplicitRoleForSelect([JSXAttributeMock('size', false)]),
106+
'combobox',
107+
'is combobox when size is a valid number',
108+
);
109+
110+
st.end();
111+
});
112+
113+
t.test('works for listbox based on multiple attribute', (st) => {
114+
st.equal(
115+
getImplicitRoleForSelect([JSXAttributeMock('multiple', true)]),
116+
'listbox',
117+
'is listbox when multiple is boolean true',
118+
);
119+
120+
st.equal(
121+
getImplicitRoleForSelect([JSXAttributeMock('multiple', 'multiple')]),
122+
'listbox',
123+
'is listbox when multiple is truthy (string)',
124+
);
125+
126+
st.equal(
127+
getImplicitRoleForSelect([JSXAttributeMock('multiple', 'true')]),
128+
'listbox',
129+
'is listbox when multiple is truthy (string) - React will warn about this',
130+
);
131+
132+
st.end();
133+
});
134+
135+
t.test('works for listbox based on size attribute', (st) => {
136+
st.equal(
137+
getImplicitRoleForSelect([JSXAttributeMock('size', 2)]),
138+
'listbox',
139+
'is listbox when size is greater than 1',
140+
);
141+
142+
st.equal(
143+
getImplicitRoleForSelect([JSXAttributeMock('size', '3')]),
144+
'listbox',
145+
'is listbox when size is greater than 1',
146+
);
147+
148+
st.equal(
149+
getImplicitRoleForSelect([JSXAttributeMock('size', 40)]),
150+
'listbox',
151+
'is listbox when size is greater than 1',
152+
);
153+
154+
st.end();
155+
});
156+
157+
t.end();
158+
});

‎src/util/implicitRoles/select.js

+15-3
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,18 @@
1+
import { getProp, getLiteralPropValue } from 'jsx-ast-utils';
2+
13
/**
2-
* Returns the implicit role for a select tag.
4+
* Returns the implicit role for a select tag depending on attributes.
5+
*
6+
* @see https://www.w3.org/TR/html-aria/#el-select
37
*/
4-
export default function getImplicitRoleForSelect() {
5-
return 'listbox';
8+
export default function getImplicitRoleForSelect(attributes) {
9+
const multiple = getProp(attributes, 'multiple');
10+
if (multiple && getLiteralPropValue(multiple)) {
11+
return 'listbox';
12+
}
13+
14+
const size = getProp(attributes, 'size');
15+
const sizeValue = size && getLiteralPropValue(size);
16+
17+
return sizeValue > 1 ? 'listbox' : 'combobox';
618
}

0 commit comments

Comments
 (0)