From 74e3beabd06db23e36e480fff6662fb330e1cf89 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Laura=20Fern=C3=A1ndez?= Date: Mon, 3 Feb 2025 11:41:54 +0100 Subject: [PATCH] MultiCombobox: add `CustomValue` as an option (#99815) --- .betterer.results | 6 ++-- .../src/components/Combobox/Combobox.test.tsx | 17 ++++++---- .../src/components/Combobox/Combobox.tsx | 8 ++--- .../Combobox/MultiCombobox.test.tsx | 34 +++++++++++++++++++ .../src/components/Combobox/MultiCombobox.tsx | 19 ++++++++--- .../src/components/Combobox/filter.ts | 3 -- .../src/components/Combobox/useOptions.ts | 34 ++++++++++++++++--- public/locales/en-US/grafana.json | 2 +- public/locales/pseudo-LOCALE/grafana.json | 2 +- 9 files changed, 97 insertions(+), 28 deletions(-) diff --git a/.betterer.results b/.betterer.results index 160b522a8c2..06a027a4207 100644 --- a/.betterer.results +++ b/.betterer.results @@ -538,8 +538,7 @@ exports[`better eslint`] = { [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "1"] ], "packages/grafana-ui/src/components/Combobox/Combobox.tsx:5381": [ - [0, 0, 0, "Do not use any type assertions.", "0"], - [0, 0, 0, "Do not use any type assertions.", "1"] + [0, 0, 0, "Do not use any type assertions.", "0"] ], "packages/grafana-ui/src/components/Combobox/MultiCombobox.tsx:5381": [ [0, 0, 0, "Do not use any type assertions.", "0"] @@ -547,6 +546,9 @@ exports[`better eslint`] = { "packages/grafana-ui/src/components/Combobox/ValuePill.tsx:5381": [ [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "0"] ], + "packages/grafana-ui/src/components/Combobox/useOptions.ts:5381": [ + [0, 0, 0, "Do not use any type assertions.", "0"] + ], "packages/grafana-ui/src/components/ConfirmModal/ConfirmContent.tsx:5381": [ [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "0"] ], diff --git a/packages/grafana-ui/src/components/Combobox/Combobox.test.tsx b/packages/grafana-ui/src/components/Combobox/Combobox.test.tsx index 7787e38d00c..7ae3f1c5ec5 100644 --- a/packages/grafana-ui/src/components/Combobox/Combobox.test.tsx +++ b/packages/grafana-ui/src/components/Combobox/Combobox.test.tsx @@ -237,11 +237,11 @@ describe('Combobox', () => { const onChangeHandler = jest.fn(); render(); const input = screen.getByRole('combobox'); - await userEvent.type(input, 'custom value'); + await userEvent.type(input, 'Use custom value'); await userEvent.keyboard('{Enter}'); - expect(screen.getByDisplayValue('custom value')).toBeInTheDocument(); - expect(onChangeHandler).toHaveBeenCalledWith(expect.objectContaining({ value: 'custom value' })); + expect(screen.getByDisplayValue('Use custom value')).toBeInTheDocument(); + expect(onChangeHandler).toHaveBeenCalledWith(expect.objectContaining({ value: 'Use custom value' })); }); it('should provide custom string when all options are numbers', async () => { @@ -256,10 +256,10 @@ describe('Combobox', () => { render(); const input = screen.getByRole('combobox'); - await userEvent.type(input, 'custom value'); + await userEvent.type(input, 'Use custom value'); await userEvent.keyboard('{Enter}'); - expect(screen.getByDisplayValue('custom value')).toBeInTheDocument(); + expect(screen.getByDisplayValue('Use custom value')).toBeInTheDocument(); expect(typeof onChangeHandler.mock.calls[0][0].value === 'string').toBeTruthy(); expect(typeof onChangeHandler.mock.calls[0][0].value === 'number').toBeFalsy(); @@ -411,9 +411,12 @@ describe('Combobox', () => { jest.advanceTimersByTime(500); // Custom value while typing }); - const customItem = screen.queryByRole('option', { name: 'Custom value: fir' }); - + const customItem = screen.getByRole('option'); + const customValue = customItem.getElementsByTagName('span')[0].textContent; + const customDescription = customItem.getElementsByTagName('span')[1].textContent; expect(customItem).toBeInTheDocument(); + expect(customValue).toBe('fir'); + expect(customDescription).toBe('Use custom value'); }); it('should display message when there is an error loading async options', async () => { diff --git a/packages/grafana-ui/src/components/Combobox/Combobox.tsx b/packages/grafana-ui/src/components/Combobox/Combobox.tsx index cf59c4910f8..4c2f6a97b63 100644 --- a/packages/grafana-ui/src/components/Combobox/Combobox.tsx +++ b/packages/grafana-ui/src/components/Combobox/Combobox.tsx @@ -135,12 +135,10 @@ export const Combobox = (props: ComboboxProps) => if (!optionMatchingInput) { const customValueOption = { - label: t('combobox.custom-value.label', 'Custom value: ') + inputValue, + label: inputValue, // Type casting needed to make this work when T is a number - value: inputValue as unknown as T, - /* TODO: Add this back when we do support descriptions and have need for it - description: t('combobox.custom-value.create', 'Create custom value'), - */ + value: inputValue as T, + description: t('combobox.custom-value.description', 'Use custom value'), }; itemsToSet = items.slice(0); diff --git a/packages/grafana-ui/src/components/Combobox/MultiCombobox.test.tsx b/packages/grafana-ui/src/components/Combobox/MultiCombobox.test.tsx index fa1274b5666..bee10de6783 100644 --- a/packages/grafana-ui/src/components/Combobox/MultiCombobox.test.tsx +++ b/packages/grafana-ui/src/components/Combobox/MultiCombobox.test.tsx @@ -128,6 +128,40 @@ describe('MultiCombobox', () => { expect(await screen.findByText('d')).toBeInTheDocument(); }); + it('should be able to set custom value', async () => { + const options = [ + { label: 'A', value: 'a' }, + { label: 'B', value: 'b' }, + { label: 'C', value: 'c' }, + ]; + const onChange = jest.fn(); + render(); + const input = screen.getByRole('combobox'); + await user.click(input); + await user.type(input, 'D'); + await user.keyboard('{arrowdown}{enter}'); + expect(onChange).toHaveBeenCalledWith([{ label: 'D', value: 'D', description: 'Use custom value' }]); + }); + + it('should be able to add custom value to the selected options', async () => { + const options = [ + { label: 'A', value: 'a' }, + { label: 'B', value: 'b' }, + { label: 'C', value: 'c' }, + ]; + const onChange = jest.fn(); + render(); + const input = screen.getByRole('combobox'); + await user.click(input); + await user.type(input, 'D'); + await user.keyboard('{arrowdown}{enter}'); + expect(onChange).toHaveBeenCalledWith([ + { value: 'a' }, + { value: 'c' }, + { label: 'D', value: 'D', description: 'Use custom value' }, + ]); + }); + it('should remove value when clicking on the close icon of the pill', async () => { const options = [ { label: 'A', value: 'a' }, diff --git a/packages/grafana-ui/src/components/Combobox/MultiCombobox.tsx b/packages/grafana-ui/src/components/Combobox/MultiCombobox.tsx index a2d5063b096..849e2a26271 100644 --- a/packages/grafana-ui/src/components/Combobox/MultiCombobox.tsx +++ b/packages/grafana-ui/src/components/Combobox/MultiCombobox.tsx @@ -37,8 +37,19 @@ interface MultiComboboxBaseProps extends Omit = MultiComboboxBaseProps & AutoSizeConditionals; export const MultiCombobox = (props: MultiComboboxProps) => { - const { placeholder, onChange, value, width, enableAllOption, invalid, disabled, minWidth, maxWidth, isClearable } = - props; + const { + placeholder, + onChange, + value, + width, + enableAllOption, + invalid, + disabled, + minWidth, + maxWidth, + isClearable, + createCustomValue = false, + } = props; const styles = useStyles2(getComboboxStyles); const [inputValue, setInputValue] = useState(''); @@ -55,7 +66,7 @@ export const MultiCombobox = (props: MultiComboboxPro }, [inputValue]); // Handle async options and the 'All' option - const { options: baseOptions, updateOptions, asyncLoading } = useOptions(props.options); + const { options: baseOptions, updateOptions, asyncLoading } = useOptions(props.options, createCustomValue); const options = useMemo(() => { // Only add the 'All' option if there's more than 1 option const addAllOption = enableAllOption && baseOptions.length > 1; @@ -202,14 +213,12 @@ export const MultiCombobox = (props: MultiComboboxPro const filteredSet = new Set(realOptions.map((item) => item.value)); newSelectedItems = selectedItems.filter((item) => !filteredSet.has(item.value)); } - setSelectedItems(newSelectedItems); } else if (newSelectedItem && isOptionSelected(newSelectedItem)) { removeSelectedItem(newSelectedItem); } else if (newSelectedItem) { addSelectedItem(newSelectedItem); } - break; case useCombobox.stateChangeTypes.InputChange: setInputValue(newInputValue ?? ''); diff --git a/packages/grafana-ui/src/components/Combobox/filter.ts b/packages/grafana-ui/src/components/Combobox/filter.ts index cb7a14d86fe..a03cbec475c 100644 --- a/packages/grafana-ui/src/components/Combobox/filter.ts +++ b/packages/grafana-ui/src/components/Combobox/filter.ts @@ -20,9 +20,6 @@ export function itemToString(item?: ComboboxOption if (item == null) { return ''; } - if (item.label?.startsWith('Custom value: ')) { - return item.value.toString(); - } return item.label ?? item.value.toString(); } diff --git a/packages/grafana-ui/src/components/Combobox/useOptions.ts b/packages/grafana-ui/src/components/Combobox/useOptions.ts index 504f3584e1d..a66e60a2747 100644 --- a/packages/grafana-ui/src/components/Combobox/useOptions.ts +++ b/packages/grafana-ui/src/components/Combobox/useOptions.ts @@ -1,6 +1,8 @@ import { debounce } from 'lodash'; import { useState, useCallback, useMemo } from 'react'; +import { t } from '../../utils/i18n'; + import { itemFilter } from './filter'; import { ComboboxOption } from './types'; import { StaleResultError, useLatestAsyncCall } from './useLatestAsyncCall'; @@ -20,7 +22,7 @@ const asyncNoop = () => Promise.resolve([]); * - function to call when user types (to filter, or call async fn) * - loading and error states */ -export function useOptions(rawOptions: AsyncOptions) { +export function useOptions(rawOptions: AsyncOptions, createCustomValue: boolean) { const isAsync = typeof rawOptions === 'function'; const loadOptions = useLatestAsyncCall(isAsync ? rawOptions : asyncNoop); @@ -56,6 +58,27 @@ export function useOptions(rawOptions: AsyncOptions>) => { + let currentOptions: Array> = opts; + if (createCustomValue && userTypedSearch) { + const customValueExists = opts.some((opt) => opt.value === userTypedSearch); + if (!customValueExists) { + currentOptions = [ + { + label: userTypedSearch, + value: userTypedSearch as T, + description: t('combobox.custom-value.description', 'Use custom value'), + }, + ...currentOptions, + ]; + } + } + return currentOptions; + }, + [createCustomValue, userTypedSearch] + ); + const updateOptions = useCallback( (inputValue: string) => { if (!isAsync) { @@ -71,12 +94,15 @@ export function useOptions(rawOptions: AsyncOptions { + let currentOptions = []; if (isAsync) { - return asyncOptions; + currentOptions = addCustomValue(asyncOptions); } else { - return rawOptions.filter(itemFilter(userTypedSearch)); + currentOptions = addCustomValue(rawOptions.filter(itemFilter(userTypedSearch))); } - }, [rawOptions, asyncOptions, isAsync, userTypedSearch]); + + return currentOptions; + }, [isAsync, addCustomValue, asyncOptions, rawOptions, userTypedSearch]); return { options: finalOptions, updateOptions, asyncLoading, asyncError }; } diff --git a/public/locales/en-US/grafana.json b/public/locales/en-US/grafana.json index 9f7da371ab5..92612e85853 100644 --- a/public/locales/en-US/grafana.json +++ b/public/locales/en-US/grafana.json @@ -709,7 +709,7 @@ "title": "Clear value" }, "custom-value": { - "label": "Custom value: " + "description": "Use custom value" }, "options": { "no-found": "No options found." diff --git a/public/locales/pseudo-LOCALE/grafana.json b/public/locales/pseudo-LOCALE/grafana.json index b2558bd0edb..b64424a9734 100644 --- a/public/locales/pseudo-LOCALE/grafana.json +++ b/public/locales/pseudo-LOCALE/grafana.json @@ -709,7 +709,7 @@ "title": "Cľęäř väľūę" }, "custom-value": { - "label": "Cūşŧőm väľūę: " + "description": "Ůşę čūşŧőm väľūę" }, "options": { "no-found": "Ńő őpŧįőʼnş ƒőūʼnđ."