Skip to content

Commit

Permalink
fix(combobox): require prop now works as intended (#3450)
Browse files Browse the repository at this point in the history
  • Loading branch information
TheSisb authored Aug 30, 2023
1 parent 91f78b1 commit e6ebb11
Show file tree
Hide file tree
Showing 4 changed files with 90 additions and 29 deletions.
6 changes: 6 additions & 0 deletions .changeset/great-lemons-itch.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@twilio-paste/combobox': patch
'@twilio-paste/core': patch
---

[MultiselectCombobox] fix bug where passing `required` would block form submit events. `required` now only blocks the submit event if no items are selected rather than when the input box is blank.
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import * as React from 'react';
import {render, screen, fireEvent} from '@testing-library/react';
import type {RenderOptions} from '@testing-library/react';
import {Theme} from '@twilio-paste/theme';
import {Form} from '@twilio-paste/form';
import filter from 'lodash/filter';
import uniq from 'lodash/uniq';

Expand Down Expand Up @@ -80,24 +81,30 @@ function getFilteredGroupedItems(inputValue: string): GroupedItem[] {
return filter(groupedItems, (item: GroupedItem) => item.label.toLowerCase().includes(lowerCasedInputValue));
}

const onSubmitMock = jest.fn();
const onKeyDownMock = jest.fn();

const GroupedMultiselectComboboxMock: React.FC<Partial<MultiselectComboboxProps>> = (props) => {
const [inputValue, setInputValue] = React.useState('');
const filteredItems = React.useMemo(() => getFilteredGroupedItems(inputValue), [inputValue]);

return (
<MultiselectCombobox
groupItemsBy="group"
items={filteredItems}
itemToString={(item: GroupedItem) => (item ? item.label : '')}
onInputValueChange={({inputValue: newInputValue = ''}) => {
setInputValue(newInputValue);
}}
labelText="Choose a component:"
helpText="This is group"
initialIsOpen
optionTemplate={(item: GroupedItem) => <div>{item.label}</div>}
{...props}
/>
<Form onSubmit={onSubmitMock}>
<MultiselectCombobox
selectedItemsLabelText="Selected Paste components"
groupItemsBy="group"
items={filteredItems}
itemToString={(item: GroupedItem) => (item ? item.label : '')}
onInputValueChange={({inputValue: newInputValue = ''}) => {
setInputValue(newInputValue);
}}
labelText="Choose a component:"
helpText="This is group"
initialIsOpen
optionTemplate={(item: GroupedItem) => <div>{item.label}</div>}
{...props}
/>
</Form>
);
};

Expand Down Expand Up @@ -130,7 +137,7 @@ describe('MultiselectCombobox', () => {
expect(renderedTextbox.getAttribute('aria-controls')).toEqual(dropdownListbox.id);
expect(renderedTextbox.getAttribute('aria-describedby')).not.toEqual('');
expect(renderedTextbox.getAttribute('aria-labelledby')).toEqual(document.querySelector('label')!.id);
expect(renderedTextbox.getAttribute('required')).toEqual('');
expect(renderedTextbox.getAttribute('required')).toEqual(null);

// unique option ids
const renderedOptions = screen.getAllByRole('option');
Expand Down Expand Up @@ -167,6 +174,27 @@ describe('MultiselectCombobox', () => {
fireEvent.click(renderedOptions[0]);
expect(renderedTextbox.getAttribute('value')).toEqual('');
});

it('should handle required correctly', async () => {
render(<MultiselectComboboxMock initialIsOpen={false} required onKeyDown={onKeyDownMock} />, {
wrapper: ThemeWrapper,
});

// Open the combobox
const renderedCombobox = screen.getByRole('combobox');
fireEvent.click(renderedCombobox);

// Focus the textbox
const renderedTextbox = screen.getByRole('textbox');
renderedTextbox.focus();

expect(onKeyDownMock).toHaveBeenCalledTimes(0);
fireEvent.keyDown(renderedTextbox, {key: 'Enter', code: 'Enter'});
expect(onKeyDownMock).toHaveBeenCalledTimes(1);

// No form submit
expect(onSubmitMock).toHaveBeenCalledTimes(0);
});
});

describe('Groups', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,24 @@ export const MultiselectCombobox = React.forwardRef<HTMLInputElement, Multiselec
[items, selectedItems, highlightedIndex]
);

/*
* If we press the enter key in a form, it will attempt to submit the form
* but if this is a required field and there are no selected items, we don't want
* to submit the form
*/

const {onKeyDown} = props;
const handleKeyDown = React.useCallback(
(event: React.KeyboardEvent<HTMLInputElement>) => {
if (event.code === 'Enter' && required && selectedItems.length === 0) {
// Don't submit the form
event.preventDefault();
}
onKeyDown?.(event);
},
[required, selectedItems, onKeyDown]
);

// FIX: a11y issue where `aria-expanded` isn't being set until the dropdown opens the very first time
const comboboxProps = getComboboxProps({
disabled,
Expand Down Expand Up @@ -332,9 +350,9 @@ export const MultiselectCombobox = React.forwardRef<HTMLInputElement, Multiselec
// we spread props into `getInputProps` so that Downshift handles events correctly
{...getInputProps({
...getDropdownProps({ref, preventKeyAction: isOpen}),
disabled,
required,
...props,
disabled,
onKeyDown: handleKeyDown,
})}
aria-describedby={helpTextId}
element={`${element}_ELEMENT`}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {MediaObject, MediaFigure, MediaBody} from '@twilio-paste/media-object';
import {InformationIcon} from '@twilio-paste/icons/esm/InformationIcon';
import {AttachIcon} from '@twilio-paste/icons/esm/AttachIcon';
import filter from 'lodash/filter';
import {Form} from '@twilio-paste/form';
import {Modal, ModalBody, ModalHeader, ModalHeading} from '@twilio-paste/modal';
import {Button} from '@twilio-paste/button';
import {useUID} from '@twilio-paste/uid-library';
Expand Down Expand Up @@ -186,21 +187,29 @@ export const MultiselectComboboxRequired = (): React.ReactNode => {
const filteredItems = React.useMemo(() => getFilteredItems(inputValue), [inputValue]);

return (
<MultiselectCombobox
required
labelText="Choose a Paste Component"
selectedItemsLabelText="Selected Paste components"
helpText="Paste components are the building blocks of your product UI."
items={filteredItems}
initialSelectedItems={['Alert', 'Anchor']}
onInputValueChange={({inputValue: newInputValue = ''}) => {
setInputValue(newInputValue);
}}
onSelectedItemsChange={(selectedItems: string[]) => {
<Form
onSubmit={(event) => {
event.preventDefault();
// eslint-disable-next-line no-console
console.log(selectedItems);
console.log('The form was submit');
}}
/>
>
<MultiselectCombobox
required
labelText="Choose a Paste Component"
selectedItemsLabelText="Selected Paste components"
helpText="Paste components are the building blocks of your product UI."
items={filteredItems}
initialSelectedItems={['Alert', 'Anchor']}
onInputValueChange={({inputValue: newInputValue = ''}) => {
setInputValue(newInputValue);
}}
onSelectedItemsChange={(selectedItems: string[]) => {
// eslint-disable-next-line no-console
console.log(selectedItems);
}}
/>
</Form>
);
};
MultiselectComboboxRequired.storyName = 'Basic - Required';
Expand Down

0 comments on commit e6ebb11

Please sign in to comment.