Skip to content

Commit

Permalink
feat: update Listbox and Option to use menu and menuitemcheckbox sema…
Browse files Browse the repository at this point in the history 
…ntics for multiselect (#25905)
  • Loading branch information
@smhigley
smhigley authored Dec 7, 2022
1 parent ac32a5c commit e7ee1ef
Show file tree
Hide file tree
Showing 7 changed files with 70 additions and 36 deletions.
7 changes: 7 additions & 0 deletions change/@fluentui-react-combobox-701e29a2-ac8a-4be4-bfa7-44fb3466a96f.json
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "prerelease",
"comment": "feat: update Listbox and Option to use menu and menuitemcheckbox semantics for multiselect",
"packageName": "@fluentui/react-combobox",
"email": "[email protected]",
"dependentChangeType": "patch"
}
24 changes: 12 additions & 12 deletions packages/react-components/react-combobox/src/components/Combobox/Combobox.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -273,9 +273,9 @@ describe('Combobox', () => {
</Combobox>,
);

expect(getByTestId('green').getAttribute('aria-selected')).toEqual('true');
expect(getByTestId('red').getAttribute('aria-selected')).toEqual('true');
expect(getByTestId('blue').getAttribute('aria-selected')).toEqual('false');
expect(getByTestId('green').getAttribute('aria-checked')).toEqual('true');
expect(getByTestId('red').getAttribute('aria-checked')).toEqual('true');
expect(getByTestId('blue').getAttribute('aria-checked')).toEqual('false');
});

it('should set defaultSelectedOptions based on Option `value`', () => {
Expand All @@ -291,8 +291,8 @@ describe('Combobox', () => {
</Combobox>,
);

expect(getByTestId('green').getAttribute('aria-selected')).toEqual('true');
expect(getByTestId('blue').getAttribute('aria-selected')).toEqual('true');
expect(getByTestId('green').getAttribute('aria-checked')).toEqual('true');
expect(getByTestId('blue').getAttribute('aria-checked')).toEqual('true');
});

it('should set selectedOptions', () => {
Expand Down Expand Up @@ -322,9 +322,9 @@ describe('Combobox', () => {
</Combobox>,
);

expect(getByTestId('red').getAttribute('aria-selected')).toEqual('true');
expect(getByTestId('blue').getAttribute('aria-selected')).toEqual('true');
expect(getByTestId('green').getAttribute('aria-selected')).toEqual('false');
expect(getByTestId('red').getAttribute('aria-checked')).toEqual('true');
expect(getByTestId('blue').getAttribute('aria-checked')).toEqual('true');
expect(getByTestId('green').getAttribute('aria-checked')).toEqual('false');
});

it('should change defaultSelectedOptions on click', () => {
Expand Down Expand Up @@ -419,9 +419,9 @@ describe('Combobox', () => {

userEvent.click(getByText('Green'));

expect(getByText('Red', { selector: '[role=option]' }).getAttribute('aria-selected')).toEqual('true');
expect(getByText('Green').getAttribute('aria-selected')).toEqual('true');
expect(getByText('Blue').getAttribute('aria-selected')).toEqual('false');
expect(getByText('Red', { selector: '[role=menuitemcheckbox]' }).getAttribute('aria-checked')).toEqual('true');
expect(getByText('Green').getAttribute('aria-checked')).toEqual('true');
expect(getByText('Blue').getAttribute('aria-checked')).toEqual('false');
});

it('stays open on click for multiselect', () => {
Expand All @@ -435,7 +435,7 @@ describe('Combobox', () => {

userEvent.click(getByText('Green'));

expect(getByRole('listbox')).not.toBeNull();
expect(getByRole('menu')).not.toBeNull();
});

it('should respect value over selected options', () => {
Expand Down
24 changes: 12 additions & 12 deletions packages/react-components/react-combobox/src/components/Dropdown/Dropdown.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -209,9 +209,9 @@ describe('Dropdown', () => {
</Dropdown>,
);

expect(getByTestId('green').getAttribute('aria-selected')).toEqual('true');
expect(getByTestId('red').getAttribute('aria-selected')).toEqual('true');
expect(getByTestId('blue').getAttribute('aria-selected')).toEqual('false');
expect(getByTestId('green').getAttribute('aria-checked')).toEqual('true');
expect(getByTestId('red').getAttribute('aria-checked')).toEqual('true');
expect(getByTestId('blue').getAttribute('aria-checked')).toEqual('false');
});

it('should set defaultSelectedOptions based on Option `value`', () => {
Expand All @@ -227,8 +227,8 @@ describe('Dropdown', () => {
</Dropdown>,
);

expect(getByTestId('green').getAttribute('aria-selected')).toEqual('true');
expect(getByTestId('blue').getAttribute('aria-selected')).toEqual('true');
expect(getByTestId('green').getAttribute('aria-checked')).toEqual('true');
expect(getByTestId('blue').getAttribute('aria-checked')).toEqual('true');
});

it('should set selectedOptions', () => {
Expand Down Expand Up @@ -258,9 +258,9 @@ describe('Dropdown', () => {
</Dropdown>,
);

expect(getByTestId('red').getAttribute('aria-selected')).toEqual('true');
expect(getByTestId('blue').getAttribute('aria-selected')).toEqual('true');
expect(getByTestId('green').getAttribute('aria-selected')).toEqual('false');
expect(getByTestId('red').getAttribute('aria-checked')).toEqual('true');
expect(getByTestId('blue').getAttribute('aria-checked')).toEqual('true');
expect(getByTestId('green').getAttribute('aria-checked')).toEqual('false');
});

it('should change defaultSelectedOptions on click', () => {
Expand Down Expand Up @@ -355,9 +355,9 @@ describe('Dropdown', () => {

fireEvent.click(getByText('Green'));

expect(getByText('Red', { selector: '[role=option]' }).getAttribute('aria-selected')).toEqual('true');
expect(getByText('Green').getAttribute('aria-selected')).toEqual('true');
expect(getByText('Blue').getAttribute('aria-selected')).toEqual('false');
expect(getByText('Red', { selector: '[role=menuitemcheckbox]' }).getAttribute('aria-checked')).toEqual('true');
expect(getByText('Green').getAttribute('aria-checked')).toEqual('true');
expect(getByText('Blue').getAttribute('aria-checked')).toEqual('false');
});

it('calls onOptionSelect with correct data for single-select', () => {
Expand Down Expand Up @@ -443,7 +443,7 @@ describe('Dropdown', () => {

fireEvent.click(getByText('Green'));

expect(getByRole('listbox')).not.toBeNull();
expect(getByRole('menu')).not.toBeNull();
});

it('should respect value over selected options', () => {
Expand Down
34 changes: 29 additions & 5 deletions packages/react-components/react-combobox/src/components/Listbox/Listbox.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,30 @@ describe('Listbox', () => {
expect(result.container).toMatchSnapshot();
});

it('uses lisbox/option semantics for single-select', () => {
const { getAllByRole } = render(
<Listbox>
<Option>Red</Option>
<Option>Green</Option>
<Option>Blue</Option>
</Listbox>,
);
expect(getAllByRole('listbox').length).toEqual(1);
expect(getAllByRole('option').length).toEqual(3);
});

it('uses menu/menuitemcheckbox semantics for multi-select', () => {
const { getAllByRole } = render(
<Listbox multiselect>
<Option>Red</Option>
<Option>Green</Option>
<Option>Blue</Option>
</Listbox>,
);
expect(getAllByRole('menu').length).toEqual(1);
expect(getAllByRole('menuitemcheckbox').length).toEqual(3);
});

/* Moving activeOption */
it('should set active option on click', () => {
const { getByTestId } = render(
Expand Down Expand Up @@ -158,9 +182,9 @@ describe('Listbox', () => {
</Listbox>,
);

expect(getByText('Green').getAttribute('aria-selected')).toEqual('true');
expect(getByText('Red').getAttribute('aria-selected')).toEqual('true');
expect(getByText('Blue').getAttribute('aria-selected')).toEqual('false');
expect(getByText('Green').getAttribute('aria-checked')).toEqual('true');
expect(getByText('Red').getAttribute('aria-checked')).toEqual('true');
expect(getByText('Blue').getAttribute('aria-checked')).toEqual('false');
});

it('should set selectedOptions', () => {
Expand Down Expand Up @@ -201,8 +225,8 @@ describe('Listbox', () => {

fireEvent.click(getByText('Red'));

expect(getByText('Green').getAttribute('aria-selected')).toEqual('true');
expect(getByText('Red').getAttribute('aria-selected')).toEqual('true');
expect(getByText('Green').getAttribute('aria-checked')).toEqual('true');
expect(getByText('Red').getAttribute('aria-checked')).toEqual('true');
});

it('should fire onChange when an option is selected', () => {
Expand Down
2 changes: 1 addition & 1 deletion packages/react-components/react-combobox/src/components/Listbox/useListbox.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ export const useListbox_unstable = (props: ListboxProps, ref: React.Ref<HTMLElem
},
root: getNativeElementProps('div', {
ref,
role: 'listbox',
role: multiselect ? 'menu' : 'listbox',
'aria-activedescendant': hasComboboxContext ? undefined : activeOption?.id,
'aria-multiselectable': multiselect,
tabIndex: 0,
Expand Down
8 changes: 4 additions & 4 deletions .../react-components/react-combobox/src/components/Option/__snapshots__/Option.test.tsx.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,10 @@
exports[`Option renders a default multi-select state 1`] = `
<div>
<div
aria-selected="false"
aria-checked="false"
class="fui-Option"
id="fluent-option1"
role="option"
role="menuitemcheckbox"
>
<span
aria-hidden="true"
Expand Down Expand Up @@ -67,10 +67,10 @@ exports[`Option renders a default single-select state 1`] = `
exports[`Option renders a selected multi-select state 1`] = `
<div>
<div
aria-selected="true"
aria-checked="true"
class="fui-Option"
id="fluent-option1"
role="option"
role="menuitemcheckbox"
>
<span
aria-hidden="true"
Expand Down
7 changes: 5 additions & 2 deletions packages/react-components/react-combobox/src/components/Option/useOption.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,17 +108,20 @@ export const useOption_unstable = (props: OptionProps, ref: React.Ref<HTMLElemen
}
}, [id, optionData, registerOption]);

const semanticProps = multiselect
? { role: 'menuitemcheckbox', 'aria-checked': selected }
: { role: 'option', 'aria-selected': selected };

return {
components: {
root: 'div',
checkIcon: 'span',
},
root: getNativeElementProps('div', {
ref: useMergedRefs(ref, optionRef),
role: 'option',
'aria-disabled': disabled ? 'true' : undefined,
'aria-selected': `${selected}`,
id,
...semanticProps,
...props,
onClick,
}),
Expand Down

0 comments on commit e7ee1ef

Please sign in to comment.