Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Fix] no-redundant-roles: Refine implicit role of select to include combobox scenarios #1027

Merged
merged 1 commit into from
Oct 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 20 additions & 1 deletion __tests__/src/rules/no-redundant-roles-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,31 @@ const alwaysValid = [
{ code: '<MyComponent role="button" />' },
{ code: '<button role={`${foo}button`} />' },
{ code: '<Button role={`${foo}button`} />', settings: componentsSettings },
{ code: '<select role="menu"><option>1</option><option>2</option></select>' },
{ code: '<select role="menu" size={2}><option>1</option><option>2</option></select>' },
{ code: '<select role="menu" multiple><option>1</option><option>2</option></select>' },
];

const neverValid = [
{ code: '<button role="button" />', errors: [expectedError('button', 'button')] },
{ code: '<body role="DOCUMENT" />', errors: [expectedError('body', 'document')] },
// button - treated as button by default
{ code: '<button role="button" />', errors: [expectedError('button', 'button')] },
{ code: '<Button role="button" />', settings: componentsSettings, errors: [expectedError('button', 'button')] },
// select - treated as combobox by default
{ code: '<select role="combobox"><option>1</option><option>2</option></select>', errors: [expectedError('select', 'combobox')] },
{ code: '<select role="combobox" size="" />', errors: [expectedError('select', 'combobox')] },
{ code: '<select role="combobox" size={1} />', errors: [expectedError('select', 'combobox')] },
{ code: '<select role="combobox" size="1" />', errors: [expectedError('select', 'combobox')] },
{ code: '<select role="combobox" size={null}></select>', errors: [expectedError('select', 'combobox')] },
{ code: '<select role="combobox" size={undefined}></select>', errors: [expectedError('select', 'combobox')] },
{ code: '<select role="combobox" multiple={undefined}></select>', errors: [expectedError('select', 'combobox')] },
{ code: '<select role="combobox" multiple={false}></select>', errors: [expectedError('select', 'combobox')] },
{ code: '<select role="combobox" multiple=""></select>', errors: [expectedError('select', 'combobox')] },
// select - treated as listbox when multiple OR size > 1
{ code: '<select role="listbox" size="3" />', errors: [expectedError('select', 'listbox')] },
{ code: '<select role="listbox" size={2} />', errors: [expectedError('select', 'listbox')] },
{ code: '<select role="listbox" multiple><option>1</option><option>2</option></select>', errors: [expectedError('select', 'listbox')] },
ljharb marked this conversation as resolved.
Show resolved Hide resolved
{ code: '<select role="listbox" multiple={true}></select>', errors: [expectedError('select', 'listbox')] },
];

ruleTester.run(`${ruleName}:recommended`, rule, {
Expand Down
158 changes: 158 additions & 0 deletions __tests__/src/util/implicitRoles/select-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,158 @@
import test from 'tape';

import JSXAttributeMock from '../../../../__mocks__/JSXAttributeMock';
import getImplicitRoleForSelect from '../../../../src/util/implicitRoles/select';

test('isAbstractRole', (t) => {
t.test('works for combobox', (st) => {
st.equal(
getImplicitRoleForSelect([]),
'combobox',
'defaults to combobox',
);

st.equal(
getImplicitRoleForSelect([JSXAttributeMock('multiple', null)]),
'combobox',
'is combobox when multiple attribute is set to not be present',
);

st.equal(
getImplicitRoleForSelect([JSXAttributeMock('multiple', undefined)]),
'combobox',
'is combobox when multiple attribute is set to not be present',
);

st.equal(
getImplicitRoleForSelect([JSXAttributeMock('multiple', false)]),
'combobox',
'is combobox when multiple attribute is set to boolean false',
);

st.equal(
getImplicitRoleForSelect([JSXAttributeMock('multiple', '')]),
'combobox',
'is listbox when multiple attribute is falsey (empty string)',
);

st.equal(
getImplicitRoleForSelect([JSXAttributeMock('size', '1')]),
'combobox',
'is combobox when size is not greater than 1',
);

st.equal(
getImplicitRoleForSelect([JSXAttributeMock('size', 1)]),
'combobox',
'is combobox when size is not greater than 1',
);

st.equal(
getImplicitRoleForSelect([JSXAttributeMock('size', 0)]),
'combobox',
'is combobox when size is not greater than 1',
);

st.equal(
getImplicitRoleForSelect([JSXAttributeMock('size', '0')]),
'combobox',
'is combobox when size is not greater than 1',
);

st.equal(
getImplicitRoleForSelect([JSXAttributeMock('size', '-1')]),
'combobox',
'is combobox when size is not greater than 1',
);

st.equal(
getImplicitRoleForSelect([JSXAttributeMock('size', '')]),
'combobox',
'is combobox when size is a valid number',
);

st.equal(
getImplicitRoleForSelect([JSXAttributeMock('size', 'true')]),
'combobox',
'is combobox when size is a valid number',
);

st.equal(
getImplicitRoleForSelect([JSXAttributeMock('size', true)]),
'combobox',
'is combobox when size is a valid number',
);

st.equal(
getImplicitRoleForSelect([JSXAttributeMock('size', NaN)]),
'combobox',
'is combobox when size is a valid number',
);

st.equal(
getImplicitRoleForSelect([JSXAttributeMock('size', '')]),
'combobox',
'is combobox when size is a valid number',
);

st.equal(
getImplicitRoleForSelect([JSXAttributeMock('size', undefined)]),
'combobox',
'is combobox when size is a valid number',
);

st.equal(
getImplicitRoleForSelect([JSXAttributeMock('size', false)]),
'combobox',
'is combobox when size is a valid number',
);

st.end();
});

t.test('works for listbox based on multiple attribute', (st) => {
st.equal(
getImplicitRoleForSelect([JSXAttributeMock('multiple', true)]),
'listbox',
'is listbox when multiple is boolean true',
);

st.equal(
getImplicitRoleForSelect([JSXAttributeMock('multiple', 'multiple')]),
'listbox',
'is listbox when multiple is truthy (string)',
);

st.equal(
getImplicitRoleForSelect([JSXAttributeMock('multiple', 'true')]),
'listbox',
'is listbox when multiple is truthy (string) - React will warn about this',
);

st.end();
});

t.test('works for listbox based on size attribute', (st) => {
st.equal(
getImplicitRoleForSelect([JSXAttributeMock('size', 2)]),
'listbox',
'is listbox when size is greater than 1',
);

st.equal(
getImplicitRoleForSelect([JSXAttributeMock('size', '3')]),
'listbox',
'is listbox when size is greater than 1',
);

st.equal(
getImplicitRoleForSelect([JSXAttributeMock('size', 40)]),
'listbox',
'is listbox when size is greater than 1',
);

st.end();
});

t.end();
});
18 changes: 15 additions & 3 deletions src/util/implicitRoles/select.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,18 @@
import { getProp, getLiteralPropValue } from 'jsx-ast-utils';

/**
* Returns the implicit role for a select tag.
* Returns the implicit role for a select tag depending on attributes.
*
* @see https://www.w3.org/TR/html-aria/#el-select
*/
export default function getImplicitRoleForSelect() {
return 'listbox';
export default function getImplicitRoleForSelect(attributes) {
const multiple = getProp(attributes, 'multiple');
if (multiple && getLiteralPropValue(multiple)) {
return 'listbox';
}

const size = getProp(attributes, 'size');
const sizeValue = size && getLiteralPropValue(size);

return sizeValue > 1 ? 'listbox' : 'combobox';
}