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(combobox): ensure listbox optionn ids in groups are always unique #893

Merged
merged 2 commits into from
Oct 28, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
96 changes: 30 additions & 66 deletions packages/paste-core/components/combobox/__tests__/index.spec.tsx
Original file line number Diff line number Diff line change
@@ -1,15 +1,11 @@
import * as React from 'react';
import _ from 'lodash';
import {axe} from 'jest-axe';
import {useUIDSeed} from '@twilio-paste/uid-library';
import {render, screen, fireEvent} from '@testing-library/react';
import {Label} from '@twilio-paste/label';
import {HelpText} from '@twilio-paste/help-text';
import {InputElement} from '@twilio-paste/input';
import {Button} from '@twilio-paste/button';
import {CloseIcon} from '@twilio-paste/icons/esm/CloseIcon';
import {Box} from '@twilio-paste/box';
import {useCombobox, Combobox, ComboboxInputWrapper, ComboboxListbox, ComboboxListboxOption} from '../src';
import {useCombobox, Combobox} from '../src';
import {ComboboxProps} from '../src/types';

const items = ['Alert', 'Anchor', 'Button', 'Card', 'Heading', 'List', 'Modal', 'Paragraph'];
Expand Down Expand Up @@ -64,54 +60,6 @@ const ComboboxMock: React.FC<{}> = () => {
);
};

const ComboboxPartsMock: React.FC<{}> = () => {
const {
getComboboxProps,
getInputProps,
getItemProps,
getLabelProps,
getMenuProps,
getToggleButtonProps,
highlightedIndex,
isOpen,
selectedItem,
} = useCombobox({items});
const seed = useUIDSeed();
return (
<>
<Label htmlFor="example-textbox" {...getLabelProps()} id="example-label" data-testid="label">
Choose a component:
</Label>
<ComboboxInputWrapper {...getComboboxProps({role: 'combobox'})} aria-owns="example-combobox-menu">
<InputElement
type="text"
{...getToggleButtonProps({tabIndex: 0})}
{...getInputProps()}
value={selectedItem || ''}
// id="example-textbox"
data-testid="textbox"
aria-controls="example-combobox-menu"
aria-describedby="example-helptext"
aria-labelledby="example-label"
/>
</ComboboxInputWrapper>
<ComboboxListbox {...getMenuProps()} aria-labelledby="example-label">
{isOpen &&
items.map((item, index) => (
<ComboboxListboxOption
{...getItemProps({item, index})}
highlighted={highlightedIndex === index}
key={seed(item)}
>
{item}
</ComboboxListboxOption>
))}
</ComboboxListbox>
<HelpText id="example-helptext">This is the help text</HelpText>
</>
);
};

const GroupedMockCombobox: React.FC<{groupLabelTemplate?: ComboboxProps['groupLabelTemplate']}> = ({
groupLabelTemplate,
}) => {
Expand All @@ -124,7 +72,7 @@ const GroupedMockCombobox: React.FC<{groupLabelTemplate?: ComboboxProps['groupLa
helpText="This is group"
groupLabelTemplate={groupLabelTemplate}
optionTemplate={(item: any) => <div>{item.label}</div>}
itemToString={item => (item ? item.label : null)}
itemToString={item => (item && typeof item !== 'string' ? item.label : null)}
/>
);
};
Expand Down Expand Up @@ -191,31 +139,39 @@ describe('Combobox ', () => {
});

it('should render a combobox with aria attributes', () => {
render(<ComboboxPartsMock />);
render(<ComboboxMock />);
const renderedCombobox = screen.getByRole('combobox');
expect(renderedCombobox.getAttribute('aria-haspopup')).toEqual('listbox');
expect(renderedCombobox.getAttribute('aria-owns')).toEqual('example-combobox-menu');
expect(renderedCombobox.getAttribute('aria-expanded')).toEqual('false');
expect(renderedCombobox.getAttribute('aria-owns')).toEqual(screen.getByRole('listbox').id);
expect(renderedCombobox.getAttribute('aria-expanded')).toEqual('true');
});

it('should render a textbox with aria attributes', () => {
render(<ComboboxPartsMock />);
render(<ComboboxMock />);
const renderedCombobox = screen.getByRole('textbox');
expect(renderedCombobox.getAttribute('aria-controls')).toEqual('example-combobox-menu');
expect(renderedCombobox.getAttribute('aria-labelledby')).toEqual('example-label');
expect(renderedCombobox.getAttribute('aria-describedby')).toEqual('example-helptext');
expect(renderedCombobox.getAttribute('aria-controls')).toEqual(screen.getByRole('listbox').id);
expect(renderedCombobox.getAttribute('aria-labelledby')).toEqual(document.querySelector('label').id);
expect(renderedCombobox.getAttribute('aria-describedby')).not.toEqual('');
});

it('should render a list with aria attributes', () => {
render(<ComboboxPartsMock />);
render(<ComboboxMock />);
const renderedCombobox = screen.getByRole('listbox');
expect(renderedCombobox.getAttribute('aria-labelledby')).toEqual('example-label');
expect(renderedCombobox.getAttribute('aria-labelledby')).toEqual(document.querySelector('label').id);
});

it('should render a list with unique option ids', () => {
render(<ComboboxMock />);
const renderedOptions = screen.getAllByRole('option');
const optionIDs = renderedOptions.map(option => option.id);
const uniqueIDs = _.uniq(optionIDs);
expect(uniqueIDs.length).toEqual(optionIDs.length);
});

it('should render a label for that matches the input id', () => {
render(<ComboboxPartsMock />);
const renderedLabel = screen.getByTestId('label');
const renderedTextbox = screen.getByTestId('textbox');
render(<ComboboxMock />);
const renderedLabel = document.querySelector('label');
const renderedTextbox = screen.getByRole('textbox');
expect(renderedLabel.getAttribute('for')).toEqual(renderedTextbox.getAttribute('id'));
});
});
Expand Down Expand Up @@ -243,6 +199,14 @@ describe('Combobox ', () => {
expect(renderedListbox.querySelectorAll('[role="listbox"] > [role="option"]').length).toEqual(1);
});

it('should render a listbox with groups of options that contains no duplicate ids', () => {
render(<GroupedMockCombobox />);
const renderedOptions = screen.getAllByRole('option');
const optionIDs = renderedOptions.map(option => option.id);
const uniqueIDs = _.uniq(optionIDs);
expect(uniqueIDs.length).toEqual(optionIDs.length);
});

it('should render a custom group label', () => {
render(<GroupedMockCombobox groupLabelTemplate={groupName => <span>hi {groupName}</span>} />);
const renderedGroups = screen.getAllByRole('group');
Expand Down
2 changes: 1 addition & 1 deletion packages/paste-core/components/combobox/src/Combobox.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ const GroupedItems: React.FC<GroupItemsProps> = ({
return (
<Item
item={item}
index={index}
index={UIDSeed(`${groupedItemKey}-${index}`)}
key={UIDSeed(`${groupedItemKey}-${index}`)}
getItemProps={getItemProps}
highlightedIndex={highlightedIndex}
Expand Down
2 changes: 1 addition & 1 deletion packages/paste-core/components/combobox/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ export interface ComboboxProps extends Omit<InputProps, 'id' | 'type' | 'value'>

export interface ItemProps extends Pick<ComboboxProps, 'optionTemplate'> {
item: Item;
index: number;
index: number | string;
getItemProps: any;
highlightedIndex: UseComboboxPrimitiveState<Item>['highlightedIndex'];
inGroup?: boolean;
Expand Down