diff --git a/packages/grafana-ui/src/components/Combobox/MultiCombobox.test.tsx b/packages/grafana-ui/src/components/Combobox/MultiCombobox.test.tsx index f92371b6e3b..4d3c1e0abee 100644 --- a/packages/grafana-ui/src/components/Combobox/MultiCombobox.test.tsx +++ b/packages/grafana-ui/src/components/Combobox/MultiCombobox.test.tsx @@ -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) => { + const [value, setValue] = React.useState(['a']); + return ( + { + setValue(val.map((v) => v.value)); + onChange(val); + }} + /> + ); + }; + + render(); + 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(); + + 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>>([ + { label: 'Integration A', value: 'a' }, + ]); + + return ( + { + onChangeHandler(options); + setSelectedValue(options); + }} + /> + ); + }; + + render(); + + 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(); + }); }); }); diff --git a/packages/grafana-ui/src/components/Combobox/MultiCombobox.tsx b/packages/grafana-ui/src/components/Combobox/MultiCombobox.tsx index bd9a65b705c..e6a3723191c 100644 --- a/packages/grafana-ui/src/components/Combobox/MultiCombobox.tsx +++ b/packages/grafana-ui/src/components/Combobox/MultiCombobox.tsx @@ -118,7 +118,7 @@ export const MultiCombobox = (props: MultiComboboxPro break; } }, - stateReducer: (state, actionAndChanges) => { + stateReducer: (_state, actionAndChanges) => { const { changes } = actionAndChanges; return { ...changes, @@ -220,7 +220,13 @@ export const MultiCombobox = (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); }