Skip to content

Commit

Permalink
feat(component): auto-highlight first matching option in Select and M…
Browse files Browse the repository at this point in the history
…ultiSelect (#818)

* feat(component): auto hightlight first matching option in Select

* feat(component): add suggestion
  • Loading branch information
MariaJose authored May 11, 2022
1 parent bf264b6 commit 76eeeb7
Show file tree
Hide file tree
Showing 5 changed files with 137 additions and 15 deletions.
14 changes: 13 additions & 1 deletion packages/big-design/src/components/MultiSelect/MultiSelect.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -90,12 +90,22 @@ export const MultiSelect = typedMemo(
setInputValue('');
}, [selectedOptions]);

const getFirstMatchingOptionIndex = (filteredOptions: (SelectOption<T> | SelectAction)[]) => {
return filteredOptions.findIndex((option) => !option.disabled);
};

const handleSetInputValue = ({
inputValue,
isOpen,
}: Partial<UseComboboxState<SelectOption<T> | SelectAction | null>>) => {
if (filterable && isOpen === true) {
setFilteredOptions(filterOptions(inputValue));
const newFilteredOptions = filterOptions(inputValue);
const firstMatchingOptionIndex = getFirstMatchingOptionIndex(newFilteredOptions);

setFilteredOptions(newFilteredOptions);

// Auto highlight first matching option
setHighlightedIndex(firstMatchingOptionIndex);
}

setInputValue(inputValue || '');
Expand Down Expand Up @@ -219,8 +229,10 @@ export const MultiSelect = typedMemo(
highlightedIndex,
isOpen,
openMenu,
setHighlightedIndex,
} = useCombobox({
id: multiSelectUniqueId,
initialHighlightedIndex: 0,
inputId: id,
inputValue,
itemToString: (option) => (option ? option.content : ''),
Expand Down
68 changes: 57 additions & 11 deletions packages/big-design/src/components/MultiSelect/spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -325,8 +325,6 @@ test('up/down arrows should change select item selection', async () => {

const options = await screen.findAllByRole('option');

fireEvent.keyDown(input, { key: 'ArrowDown' });

expect(options[0].getAttribute('aria-selected')).toBe('true');
expect(input.getAttribute('aria-activedescendant')).toEqual(options[0].id);

Expand Down Expand Up @@ -370,7 +368,7 @@ test('end should select last select item', async () => {
const options = await screen.findAllByRole('option');

fireEvent.keyDown(input, { key: 'ArrowDown' });
expect(options[0].getAttribute('aria-selected')).toBe('true');
expect(options[1].getAttribute('aria-selected')).toBe('true');

fireEvent.keyDown(input, { key: 'End' });

Expand All @@ -389,7 +387,7 @@ test('enter should trigger onOptionsChange', async () => {
await fireEvent.keyDown(input, { key: 'Enter' });
});

expect(onChange).toHaveBeenCalledWith([mockOptions[1].value], [mockOptions[1]]);
expect(onChange).toHaveBeenCalledWith([mockOptions[0].value], [mockOptions[0]]);
});

test('clicking on select options should trigger onOptionsChange', async () => {
Expand Down Expand Up @@ -784,8 +782,8 @@ test('multiselect should be able to select multiple options', async () => {
});

expect(onChange).toHaveBeenCalledWith(
[mockOptions[0].value, mockOptions[1].value, mockOptions[2].value],
[mockOptions[0], mockOptions[1], mockOptions[2]],
[mockOptions[0].value, mockOptions[1].value, mockOptions[3].value],
[mockOptions[0], mockOptions[1], mockOptions[3]],
);
});

Expand All @@ -800,7 +798,7 @@ test('multiselect should be able to deselect options', async () => {
await fireEvent.keyDown(inputs[0], { key: 'Enter' });
});

expect(onChange).toHaveBeenCalledWith([mockOptions[1].value], [mockOptions[1]]);
expect(onChange).toHaveBeenCalledWith([mockOptions[0].value], [mockOptions[0]]);
});

test('multiselect options should immediately rerender when prop changes', async () => {
Expand Down Expand Up @@ -930,17 +928,18 @@ test('group labels should be skipped when using keyboard to navigate options', a
const options = await screen.findAllByRole('option');

expect(options.length).toBe(6);
expect(options[0].getAttribute('aria-selected')).toBe('true');

fireEvent.keyDown(input, { key: 'ArrowDown' });

expect(options[0].getAttribute('aria-selected')).toBe('true');
expect(input.getAttribute('aria-activedescendant')).toEqual(options[0].id);
expect(options[1].getAttribute('aria-selected')).toBe('true');
expect(input.getAttribute('aria-activedescendant')).toEqual(options[1].id);

fireEvent.keyDown(input, { key: 'ArrowDown' });
fireEvent.keyDown(input, { key: 'ArrowDown' });

expect(options[2].getAttribute('aria-selected')).toBe('true');
expect(input.getAttribute('aria-activedescendant')).toEqual(options[2].id);
expect(options[3].getAttribute('aria-selected')).toBe('true');
expect(input.getAttribute('aria-activedescendant')).toEqual(options[3].id);
});

test('group labels should still render when filtering options', async () => {
Expand All @@ -960,6 +959,53 @@ test('group labels should still render when filtering options', async () => {
expect(label2).toBeInTheDocument();
});

test('autoselects first matching option when filtering', async () => {
render(MultiSelectMock);

const input = await screen.findByTestId('multi-select');

fireEvent.change(input, { target: { value: 'm' } });

const options = await screen.findAllByRole('option');

expect(options.length).toBe(2);
expect(options[0].getAttribute('aria-selected')).toBe('true');
expect(options[1].getAttribute('aria-selected')).toBe('false');
});

test('does not autoselect first matching option when it is disabled', async () => {
render(MultiSelectMock);

const input = screen.getByTestId('multi-select');

fireEvent.change(input, { target: { value: 'f' } });

const options = await screen.findAllByRole('option');

expect(options.length).toBe(2);
expect(options[0].getAttribute('aria-selected')).toBe('false');
expect(options[1].getAttribute('aria-selected')).toBe('true');
});

test('after clearing the input value, first option is always selected', async () => {
render(MultiSelectMock);

const input = screen.getByTestId('multi-select');

fireEvent.change(input, { target: { value: 'Can' } });

const canadaOption = await screen.findByRole('option', { name: 'Canada' });

expect(canadaOption.getAttribute('aria-selected')).toBe('true');

fireEvent.change(input, { target: { value: '' } });

const options = await screen.findAllByRole('option');

expect(options.length).toBe(6);
expect(options[0].getAttribute('aria-selected')).toBe('true');
});

test('select option should supports description', async () => {
render(MultiSelectWithOptionsDescriptions);

Expand Down
19 changes: 18 additions & 1 deletion packages/big-design/src/components/Select/Select.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,10 @@ export const Select = typedMemo(
// Need to set select options if options prop changes
useEffect(() => setFilteredOptions(flattenedOptions), [flattenedOptions]);

const getFirstMatchingOptionIndex = (filteredOptions: (SelectOption<T> | SelectAction)[]) => {
return filteredOptions.findIndex((option) => !option.disabled);
};

const handleOnSelectedItemChange = (changes: Partial<UseComboboxState<SelectOption<T> | SelectAction | null>>) => {
if (action && changes.selectedItem && changes.selectedItem.content === action.content) {
action.onActionClick(inputValue || null);
Expand All @@ -94,7 +98,19 @@ export const Select = typedMemo(
}: Partial<UseComboboxState<SelectOption<T> | SelectAction | null>>) => {
// Filter only when List is open
if (filterable && isOpen === true) {
setFilteredOptions(filterOptions(inputValue));
const newFilteredOptions = filterOptions(inputValue);
const firstMatchingOptionIndex = getFirstMatchingOptionIndex(newFilteredOptions);

setFilteredOptions(newFilteredOptions);

// Auto highlight first matching option
if (inputValue !== '') {
setHighlightedIndex(firstMatchingOptionIndex);
} else if (selectedItem) {
const selectedItemIndex = flattenedOptions.indexOf(selectedItem);

setHighlightedIndex(selectedItemIndex);
}
}

setInputValue(inputValue || '');
Expand Down Expand Up @@ -147,6 +163,7 @@ export const Select = typedMemo(
isOpen,
openMenu,
selectedItem,
setHighlightedIndex,
} = useCombobox({
id: selectUniqueId,
inputId: id,
Expand Down
48 changes: 48 additions & 0 deletions packages/big-design/src/components/Select/spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -347,6 +347,54 @@ test('select items should be filterable', async () => {
expect(options.length).toBe(2);
});

test('autoselects first matching option when filtering', async () => {
render(SelectMock);

const input = screen.getByTestId('select');

fireEvent.change(input, { target: { value: 'm' } });

const options = await screen.findAllByRole('option');

expect(options.length).toBe(2);
expect(options[0].getAttribute('aria-selected')).toBe('true');
expect(options[1].getAttribute('aria-selected')).toBe('false');
});

test('does not autoselect first matching option when it is disabled', async () => {
render(SelectMock);

const input = screen.getByTestId('select');

fireEvent.change(input, { target: { value: 'f' } });

const options = await screen.findAllByRole('option');

expect(options.length).toBe(2);
expect(options[0].getAttribute('aria-selected')).toBe('false');
expect(options[1].getAttribute('aria-selected')).toBe('true');
});

test('previous option remains selected after clearing the input value', async () => {
render(SelectMock);

const input = screen.getByTestId('select');

fireEvent.change(input, { target: { value: 'ca' } });

const canadaOption = await screen.findByRole('option', { name: 'Canada' });

expect(canadaOption.getAttribute('aria-selected')).toBe('true');

fireEvent.change(input, { target: { value: '' } });

const options = await screen.findAllByRole('option');
const mexicoOption = await screen.findByRole('option', { name: 'Mexico' });

expect(options.length).toBe(6);
expect(mexicoOption.getAttribute('aria-selected')).toBe('true');
});

test('select options should immediately rerender when prop changes', async () => {
const { rerender } = render(<Select onOptionChange={onChange} options={mockOptions} />);

Expand Down
3 changes: 1 addition & 2 deletions packages/docs/pages/multi-select.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -409,7 +409,7 @@ const MultiSelectPage = () => {
onActionClick: () => null,
}}
label="Select"
onOptionsChange={() => null}
onOptionsChange={handleChange}
options={[
{ value: 1, content: 'Option #1', description: 'Description for option #1' },
{
Expand All @@ -422,7 +422,6 @@ const MultiSelectPage = () => {
{ value: 4, content: 'Option #4' },
{ value: 5, content: 'Option #5' },
]}
onChange={handleChange}
placeholder="Choose option"
required
value={value}
Expand Down

0 comments on commit 76eeeb7

Please sign in to comment.