MultiCombobox: Fix async options to being able to be removed (#109473)
This commit is contained in:
@@ -117,6 +117,39 @@ describe('MultiCombobox', () => {
|
||||
expect(onChange).toHaveBeenNthCalledWith(3, [{ label: 'C', value: third }]);
|
||||
});
|
||||
|
||||
it('should allow for options to be deselected', async () => {
|
||||
// This test ensures that our fix for async options doesn't break sync options
|
||||
const options = [
|
||||
{ label: 'A', value: 'a' },
|
||||
{ label: 'B', value: 'b' },
|
||||
{ label: 'C', value: 'c' },
|
||||
];
|
||||
const onChange = jest.fn();
|
||||
|
||||
const ControlledMultiCombobox = (props: MultiComboboxProps<string>) => {
|
||||
const [value, setValue] = React.useState<string[]>(['a']);
|
||||
return (
|
||||
<MultiCombobox
|
||||
{...props}
|
||||
value={value}
|
||||
onChange={(val) => {
|
||||
setValue(val.map((v) => v.value));
|
||||
onChange(val);
|
||||
}}
|
||||
/>
|
||||
);
|
||||
};
|
||||
|
||||
render(<ControlledMultiCombobox options={options} value={[]} onChange={onChange} />);
|
||||
const input = screen.getByRole('combobox');
|
||||
await user.click(input);
|
||||
|
||||
// Click on option A to deselect it (it should be selected initially)
|
||||
await user.click(await screen.findByRole('option', { name: 'A' }));
|
||||
|
||||
expect(onChange).toHaveBeenCalledWith([]);
|
||||
});
|
||||
|
||||
it('should be able to render a value that is not in the options', async () => {
|
||||
const options = [
|
||||
{ label: 'A', value: 'a' },
|
||||
@@ -378,6 +411,76 @@ describe('MultiCombobox', () => {
|
||||
expect(asyncOptions).toHaveBeenCalledTimes(1);
|
||||
expect(asyncOptions).toHaveBeenCalledWith('abc');
|
||||
});
|
||||
|
||||
it('should allow deselecting items', async () => {
|
||||
const asyncOptions = jest.fn(() => Promise.resolve(simpleAsyncOptions));
|
||||
render(<MultiCombobox options={asyncOptions} value={['Option 1']} onChange={onChangeHandler} />);
|
||||
|
||||
const input = screen.getByRole('combobox');
|
||||
await user.click(input);
|
||||
|
||||
// Debounce
|
||||
await act(async () => jest.advanceTimersByTime(200));
|
||||
|
||||
// Click on Option 1 to deselect it (it should already be selected via value prop)
|
||||
const item = await screen.findByRole('option', { name: 'Option 1' });
|
||||
await user.click(item);
|
||||
|
||||
// Verify onChange was called to remove the item
|
||||
expect(onChangeHandler).toHaveBeenCalledWith([]);
|
||||
});
|
||||
|
||||
it('should allow deselecting items with async options using ComboboxOption value format', async () => {
|
||||
// This test reproduces the bug where deselection doesn't work with async options
|
||||
// when the value is passed as ComboboxOption objects
|
||||
const asyncOptionsData = [
|
||||
{ label: 'Integration A', value: 'a' },
|
||||
{ label: 'Integration B', value: 'b' },
|
||||
{ label: 'Integration C', value: 'c' },
|
||||
];
|
||||
|
||||
const asyncOptions = jest.fn(() => Promise.resolve(asyncOptionsData));
|
||||
|
||||
// Use a controlled component to simulate the user's scenario
|
||||
const ControlledComponent = () => {
|
||||
const [selectedValue, setSelectedValue] = React.useState<Array<ComboboxOption<string>>>([
|
||||
{ label: 'Integration A', value: 'a' },
|
||||
]);
|
||||
|
||||
return (
|
||||
<MultiCombobox
|
||||
options={asyncOptions}
|
||||
value={selectedValue}
|
||||
onChange={(options) => {
|
||||
onChangeHandler(options);
|
||||
setSelectedValue(options);
|
||||
}}
|
||||
/>
|
||||
);
|
||||
};
|
||||
|
||||
render(<ControlledComponent />);
|
||||
|
||||
const input = screen.getByRole('combobox');
|
||||
await user.click(input);
|
||||
|
||||
// Wait for async options to load
|
||||
await act(async () => jest.advanceTimersByTime(200));
|
||||
|
||||
// Integration A should be selected (shown as pill)
|
||||
const pillRemoveButton = screen.getByRole('button', { name: 'Remove Integration A' });
|
||||
expect(pillRemoveButton).toBeInTheDocument();
|
||||
|
||||
// Click on Integration A in the dropdown to deselect it
|
||||
const item = await screen.findByRole('option', { name: 'Integration A' });
|
||||
await user.click(item);
|
||||
|
||||
// Verify onChange was called to remove the item
|
||||
expect(onChangeHandler).toHaveBeenCalledWith([]);
|
||||
|
||||
// The pill should be removed
|
||||
expect(screen.queryByRole('button', { name: 'Remove Integration A' })).not.toBeInTheDocument();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -118,7 +118,7 @@ export const MultiCombobox = <T extends string | number>(props: MultiComboboxPro
|
||||
break;
|
||||
}
|
||||
},
|
||||
stateReducer: (state, actionAndChanges) => {
|
||||
stateReducer: (_state, actionAndChanges) => {
|
||||
const { changes } = actionAndChanges;
|
||||
return {
|
||||
...changes,
|
||||
@@ -220,7 +220,13 @@ export const MultiCombobox = <T extends string | number>(props: MultiComboboxPro
|
||||
}
|
||||
setSelectedItems(newSelectedItems);
|
||||
} else if (newSelectedItem && isOptionSelected(newSelectedItem)) {
|
||||
removeSelectedItem(newSelectedItem);
|
||||
// Find the actual selected item object that matches the clicked item by value
|
||||
// This is necessary because the clicked item (from async options) may be a different
|
||||
// object reference than the selected item, and useMultipleSelection uses object equality
|
||||
const itemToRemove = selectedItems.find((item) => item.value === newSelectedItem.value);
|
||||
if (itemToRemove) {
|
||||
removeSelectedItem(itemToRemove);
|
||||
}
|
||||
} else if (newSelectedItem) {
|
||||
addSelectedItem(newSelectedItem);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user