From b3b044b54ba9ef6c4006a1b15f62a05013dd9c68 Mon Sep 17 00:00:00 2001 From: Joao Silva <100691367+JoaoSilvaGrafana@users.noreply.github.com> Date: Mon, 20 Jan 2025 10:32:09 +0000 Subject: [PATCH] MultiCombobox: Add "All" option (#98377) * MultiCombobox: Add All option * Translate * Add prop to show All option * Change variable name * betterer update * Extract variable * Update packages/grafana-ui/src/components/Combobox/MultiCombobox.tsx Co-authored-by: Tobias Skarhed <1438972+tskarhed@users.noreply.github.com> * Return All item * Update checkboxes * Add filtering functionality * Handle deduplication when selecting already selected items * Performance improvements when selecting and modifying all items * Handle bug with isOpen for tests to pass * Small fixes * Add filtered phrase * Address PR feedback * Reset okg/services from main * Reset from main * Restore main * Add counter to filtered all * Fix OptionListItem * Hide all when there are no results * Refactor to use useMemo instead * Fix comments * Remove useEffect --------- Co-authored-by: Tobias Skarhed <1438972+tskarhed@users.noreply.github.com> Co-authored-by: Tobias Skarhed --- .betterer.results | 3 + .../Combobox/MultiCombobox.internal.story.tsx | 10 +- .../Combobox/MultiCombobox.test.tsx | 50 ++++++- .../src/components/Combobox/MultiCombobox.tsx | 133 ++++++++++++------ .../components/Combobox/OptionListItem.tsx | 10 +- .../src/components/Combobox/filter.ts | 4 +- .../Combobox/getMultiComboboxStyles.ts | 7 +- .../components/Combobox/useMeasureMulti.ts | 5 +- public/locales/en-US/grafana.json | 6 + public/locales/pseudo-LOCALE/grafana.json | 6 + 10 files changed, 172 insertions(+), 62 deletions(-) diff --git a/.betterer.results b/.betterer.results index 74e1cff93f2..10cd3e30550 100644 --- a/.betterer.results +++ b/.betterer.results @@ -541,6 +541,9 @@ exports[`better eslint`] = { [0, 0, 0, "Do not use any type assertions.", "0"], [0, 0, 0, "Do not use any type assertions.", "1"] ], + "packages/grafana-ui/src/components/Combobox/MultiCombobox.tsx:5381": [ + [0, 0, 0, "Do not use any type assertions.", "0"] + ], "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"] ], diff --git a/packages/grafana-ui/src/components/Combobox/MultiCombobox.internal.story.tsx b/packages/grafana-ui/src/components/Combobox/MultiCombobox.internal.story.tsx index f40e9f62bdd..3af44f61d3c 100644 --- a/packages/grafana-ui/src/components/Combobox/MultiCombobox.internal.story.tsx +++ b/packages/grafana-ui/src/components/Combobox/MultiCombobox.internal.story.tsx @@ -13,11 +13,11 @@ const meta: Meta = { const commonArgs = { options: [ - { label: 'Option 1', value: 'option1' }, - { label: 'Option 2', value: 'option2' }, - { label: 'Option 3', value: 'option3' }, - { label: 'Option 4', value: 'option4' }, - { label: 'Option 5', value: 'option5' }, + { label: 'wasd - 1', value: 'option1' }, + { label: 'wasd - 2', value: 'option2' }, + { label: 'wasd - 3', value: 'option3' }, + { label: 'asdf - 1', value: 'option4' }, + { label: 'asdf - 2', value: 'option5' }, ], value: ['option2'], placeholder: 'Select multiple options...', diff --git a/packages/grafana-ui/src/components/Combobox/MultiCombobox.test.tsx b/packages/grafana-ui/src/components/Combobox/MultiCombobox.test.tsx index 2f44fd1dd11..70be3c4148d 100644 --- a/packages/grafana-ui/src/components/Combobox/MultiCombobox.test.tsx +++ b/packages/grafana-ui/src/components/Combobox/MultiCombobox.test.tsx @@ -36,8 +36,8 @@ describe('MultiCombobox', () => { const input = screen.getByRole('combobox'); user.click(input); expect(await screen.findByText('A')).toBeInTheDocument(); - expect(await screen.findByText('B')).toBeInTheDocument(); - expect(await screen.findByText('C')).toBeInTheDocument(); + expect(screen.getByText('B')).toBeInTheDocument(); + expect(screen.getByText('C')).toBeInTheDocument(); }); it('should render with value', () => { @@ -85,6 +85,7 @@ describe('MultiCombobox', () => { /> ); }; + render(); const input = screen.getByRole('combobox'); await user.click(input); @@ -111,4 +112,49 @@ describe('MultiCombobox', () => { await user.click(screen.getByRole('combobox')); expect(await screen.findByText('d')).toBeInTheDocument(); }); + + describe('all option', () => { + it('should render all option', async () => { + const options = [ + { label: 'A', value: 'a' }, + { label: 'B', value: 'b' }, + { label: 'C', value: 'c' }, + ]; + render(); + const input = screen.getByRole('combobox'); + await user.click(input); + expect(await screen.findByRole('option', { name: 'All' })).toBeInTheDocument(); + }); + + it('should select all option', 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.click(await screen.findByText('All')); + + expect(onChange).toHaveBeenCalledWith(['a', 'b', 'c']); + }); + + it('should deselect all option', 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.click(await screen.findByRole('option', { name: 'All' })); + expect(onChange).toHaveBeenCalledWith([]); + }); + }); }); diff --git a/packages/grafana-ui/src/components/Combobox/MultiCombobox.tsx b/packages/grafana-ui/src/components/Combobox/MultiCombobox.tsx index 81abe9f6a00..bb8b9934472 100644 --- a/packages/grafana-ui/src/components/Combobox/MultiCombobox.tsx +++ b/packages/grafana-ui/src/components/Combobox/MultiCombobox.tsx @@ -1,9 +1,10 @@ import { cx } from '@emotion/css'; import { useVirtualizer } from '@tanstack/react-virtual'; import { useCombobox, useMultipleSelection } from 'downshift'; -import { useCallback, useEffect, useMemo, useState } from 'react'; +import { useCallback, useMemo, useState } from 'react'; import { useStyles2 } from '../../themes'; +import { t } from '../../utils/i18n'; import { Checkbox } from '../Forms/Checkbox'; import { Box } from '../Layout/Box/Box'; import { Stack } from '../Layout/Stack/Stack'; @@ -20,17 +21,20 @@ import { itemFilter, itemToString } from './filter'; import { getComboboxStyles, MENU_OPTION_HEIGHT, MENU_OPTION_HEIGHT_DESCRIPTION } from './getComboboxStyles'; import { getMultiComboboxStyles } from './getMultiComboboxStyles'; import { useComboboxFloat } from './useComboboxFloat'; -import { useMeasureMulti } from './useMeasureMulti'; +import { MAX_SHOWN_ITEMS, useMeasureMulti } from './useMeasureMulti'; + +export const ALL_OPTION_VALUE = '__GRAFANA_INTERNAL_MULTICOMBOBOX_ALL_OPTION__'; interface MultiComboboxBaseProps extends Omit, 'value' | 'onChange'> { value?: T[] | Array>; onChange: (items?: T[]) => void; + enableAllOption?: boolean; } export type MultiComboboxProps = MultiComboboxBaseProps & AutoSizeConditionals; export const MultiCombobox = (props: MultiComboboxProps) => { - const { options, placeholder, onChange, value, width, invalid, loading, disabled } = props; + const { options, placeholder, onChange, value, width, enableAllOption, invalid, loading, disabled } = props; const isAsync = typeof options === 'function'; const selectedItems = useMemo(() => { @@ -45,15 +49,30 @@ export const MultiCombobox = (props: MultiComboboxPro const styles = useStyles2(getComboboxStyles); const [inputValue, setInputValue] = useState(''); - const [baseItems, baseSetItems] = useState(isAsync ? [] : options); + const allOptionItem = useMemo(() => { + return { + label: + inputValue === '' + ? t('multicombobox.all.title', 'All') + : t('multicombobox.all.title-filtered', 'All (filtered)'), + // Type casting needed to make this work when T is a number + value: ALL_OPTION_VALUE, + } as ComboboxOption; + }, [inputValue]); - const items = useMemo(() => baseItems.filter(itemFilter(inputValue)), [baseItems, inputValue]); + const baseItems = useMemo(() => { + return isAsync ? [] : enableAllOption ? [allOptionItem, ...options] : options; + }, [options, enableAllOption, allOptionItem, isAsync]); - // TODO: Improve this with async - useEffect(() => { - baseSetItems(isAsync ? [] : options); - }, [options, isAsync]); + const items = useMemo(() => { + const newItems = baseItems.filter(itemFilter(inputValue)); + if (enableAllOption && newItems.length === 1 && newItems[0] === allOptionItem) { + return []; + } + + return newItems; + }, [baseItems, inputValue, enableAllOption, allOptionItem]); const [isOpen, setIsOpen] = useState(false); const { inputRef: containerRef, floatingRef, floatStyles, scrollRef } = useComboboxFloat(items, isOpen); @@ -125,6 +144,25 @@ export const MultiCombobox = (props: MultiComboboxPro switch (type) { case useCombobox.stateChangeTypes.InputKeyDownEnter: case useCombobox.stateChangeTypes.ItemClick: + // Handle All functionality + if (newSelectedItem?.value === ALL_OPTION_VALUE) { + const allFilteredSelected = selectedItems.length === items.length - 1; + let newSelectedItems = allFilteredSelected && inputValue === '' ? [] : baseItems.slice(1); + + if (!allFilteredSelected && inputValue !== '') { + // Select all currently filtered items and deduplicate + newSelectedItems = [...new Set([...selectedItems, ...items.slice(1)])]; + } + + if (allFilteredSelected && inputValue !== '') { + // Deselect all currently filtered items + const filteredSet = new Set(items.slice(1).map((item) => item.value)); + newSelectedItems = selectedItems.filter((item) => !filteredSet.has(item.value)); + } + + onChange(getComboboxOptionsValues(newSelectedItems)); + break; + } if (newSelectedItem) { if (!isOptionSelected(newSelectedItem)) { onChange(getComboboxOptionsValues([...selectedItems, newSelectedItem])); @@ -136,6 +174,8 @@ export const MultiCombobox = (props: MultiComboboxPro case useCombobox.stateChangeTypes.InputChange: setInputValue(newInputValue ?? ''); break; + case useCombobox.stateChangeTypes.InputClick: + setIsOpen(true); default: break; } @@ -145,13 +185,15 @@ export const MultiCombobox = (props: MultiComboboxPro const virtualizerOptions = { count: items.length, getScrollElement: () => scrollRef.current, - estimateSize: (index: number) => (items[index].description ? MENU_OPTION_HEIGHT_DESCRIPTION : MENU_OPTION_HEIGHT), + estimateSize: (index: number) => + 'description' in items[index] ? MENU_OPTION_HEIGHT_DESCRIPTION : MENU_OPTION_HEIGHT, overscan: VIRTUAL_OVERSCAN_ITEMS, }; const rowVirtualizer = useVirtualizer(virtualizerOptions); - const visibleItems = isOpen ? selectedItems : selectedItems.slice(0, shownItems); + // Selected items that show up in the input field + const visibleItems = isOpen ? selectedItems.slice(0, MAX_SHOWN_ITEMS) : selectedItems.slice(0, shownItems); return (
@@ -159,7 +201,6 @@ export const MultiCombobox = (props: MultiComboboxPro style={{ width: width === 'auto' ? undefined : width }} className={cx(multiStyles.wrapper, { [multiStyles.disabled]: disabled })} ref={measureRef} - onClick={() => !disabled && selectedItems.length > 0 && setIsOpen(!isOpen)} > {visibleItems.map((item, index) => ( @@ -174,7 +215,7 @@ export const MultiCombobox = (props: MultiComboboxPro {itemToString(item)} ))} - {selectedItems.length > shownItems && !isOpen && ( + {selectedItems.length > visibleItems.length && ( {/* eslint-disable-next-line @grafana/no-untranslated-strings */} ... @@ -182,7 +223,7 @@ export const MultiCombobox = (props: MultiComboboxPro interactive content={ <> - {selectedItems.slice(shownItems).map((item) => ( + {selectedItems.slice(visibleItems.length).map((item) => (
{itemToString(item)}
))} @@ -193,15 +234,13 @@ export const MultiCombobox = (props: MultiComboboxPro )} 0, - })} + className={multiStyles.input} {...getInputProps( getDropdownProps({ disabled, preventKeyAction: isOpen, placeholder: selectedItems.length > 0 ? undefined : placeholder, - onFocus: () => setIsOpen(true), + onFocus: () => !disabled && setIsOpen(true), }) )} /> @@ -227,6 +266,10 @@ export const MultiCombobox = (props: MultiComboboxPro const itemProps = getItemProps({ item, index }); const isSelected = isOptionSelected(item); const id = 'multicombobox-option-' + item.value.toString(); + const isAll = item.value === ALL_OPTION_VALUE; + const allItemsSelected = + items[0]?.value === ALL_OPTION_VALUE && selectedItems.length === items.length - 1; + return (
  • (props: MultiComboboxPro 0 && !allItemsSelected} aria-labelledby={id} onClick={(e) => { e.stopPropagation(); }} /> - +
    • ); @@ -262,31 +315,29 @@ function getSelectedItemsFromValue( value: T[] | Array>, options: Array> ) { - if (!isComboboxOptions(value)) { - const resultingItems: Array | undefined> = []; - - for (const item of options) { - for (const [index, val] of value.entries()) { - if (val === item.value) { - resultingItems[index] = item; - } - } - if (resultingItems.length === value.length && !resultingItems.includes(undefined)) { - // We found all items for the values - break; - } - } + if (isComboboxOptions(value)) { + return value; + } + const valueMap = new Map(value.map((val, index) => [val, index])); + const resultingItems: Array> = []; - // Handle values that are not in options - for (const [index, val] of value.entries()) { - if (resultingItems[index] === undefined) { - resultingItems[index] = { value: val }; - } + for (const option of options) { + const index = valueMap.get(option.value); + if (index !== undefined) { + resultingItems[index] = option; + valueMap.delete(option.value); + } + if (valueMap.size === 0) { + // We found all values + break; } - return resultingItems.filter((item) => item !== undefined); // TODO: Not actually needed, but TS complains } - return value; + // Handle items that are not in options + for (const [val, index] of valueMap) { + resultingItems[index] = { value: val }; + } + return resultingItems; } function isComboboxOptions( diff --git a/packages/grafana-ui/src/components/Combobox/OptionListItem.tsx b/packages/grafana-ui/src/components/Combobox/OptionListItem.tsx index d80c70e01d2..436a662e204 100644 --- a/packages/grafana-ui/src/components/Combobox/OptionListItem.tsx +++ b/packages/grafana-ui/src/components/Combobox/OptionListItem.tsx @@ -1,21 +1,21 @@ import { useStyles2 } from '../../themes'; -import { ComboboxOption } from './Combobox'; import { getComboboxStyles } from './getComboboxStyles'; interface Props { - option: ComboboxOption; + label: string; + description?: string; id: string; } -export const OptionListItem = ({ option, id }: Props) => { +export const OptionListItem = ({ label, description, id }: Props) => { const styles = useStyles2(getComboboxStyles); return (
    - {option.label ?? option.value} + {label} - {option.description && {option.description}} + {description && {description}}
    ); }; diff --git a/packages/grafana-ui/src/components/Combobox/filter.ts b/packages/grafana-ui/src/components/Combobox/filter.ts index 09ab4653051..dbf86c062d6 100644 --- a/packages/grafana-ui/src/components/Combobox/filter.ts +++ b/packages/grafana-ui/src/components/Combobox/filter.ts @@ -1,4 +1,5 @@ import { ComboboxOption } from './Combobox'; +import { ALL_OPTION_VALUE } from './MultiCombobox'; export function itemToString(item?: ComboboxOption | null) { if (!item) { @@ -17,7 +18,8 @@ export function itemFilter(inputValue: string) { return ( !inputValue || item.label?.toLowerCase().includes(lowerCasedInputValue) || - item.value?.toString().toLowerCase().includes(lowerCasedInputValue) + item.value?.toString().toLowerCase().includes(lowerCasedInputValue) || + item.value.toString() === ALL_OPTION_VALUE ); }; } diff --git a/packages/grafana-ui/src/components/Combobox/getMultiComboboxStyles.ts b/packages/grafana-ui/src/components/Combobox/getMultiComboboxStyles.ts index 107a881cf5c..4347acdb0e7 100644 --- a/packages/grafana-ui/src/components/Combobox/getMultiComboboxStyles.ts +++ b/packages/grafana-ui/src/components/Combobox/getMultiComboboxStyles.ts @@ -39,12 +39,7 @@ export const getMultiComboboxStyles = ( outline: 'none', }, }), - inputClosed: css({ - width: 0, - flexGrow: 0, - paddingLeft: 0, - paddingRight: 0, - }), + pillWrapper: css({ display: 'inline-flex', flexWrap: isOpen ? 'wrap' : 'nowrap', diff --git a/packages/grafana-ui/src/components/Combobox/useMeasureMulti.ts b/packages/grafana-ui/src/components/Combobox/useMeasureMulti.ts index 79a7722df4d..4ff65a7d41f 100644 --- a/packages/grafana-ui/src/components/Combobox/useMeasureMulti.ts +++ b/packages/grafana-ui/src/components/Combobox/useMeasureMulti.ts @@ -8,6 +8,7 @@ import { ComboboxOption } from './Combobox'; const FONT_SIZE = 12; const EXTRA_PILL_SIZE = 50; const EXTRA_PILL_DISABLED_SIZE = 10; +export const MAX_SHOWN_ITEMS = 15; /** * Updates the number of shown items in the multi combobox based on the available width. @@ -34,8 +35,8 @@ export function useMeasureMulti( (disabled ? EXTRA_PILL_DISABLED_SIZE : EXTRA_PILL_SIZE); if (currWidth > maxWidth) { // If there is no space for that item, show the current number of items, - // but always show at least 1 item - setShownItems(i || 1); + // but always show at least 1 item. Cap at maximum number of items. + setShownItems(Math.min(i, MAX_SHOWN_ITEMS) || 1); break; } if (i === selectedItems.length - 1) { diff --git a/public/locales/en-US/grafana.json b/public/locales/en-US/grafana.json index cef1901e04c..f68a771ff6b 100644 --- a/public/locales/en-US/grafana.json +++ b/public/locales/en-US/grafana.json @@ -2049,6 +2049,12 @@ "title": "Why host with Grafana?" } }, + "multicombobox": { + "all": { + "title": "All", + "title-filtered": "All (filtered)" + } + }, "nav": { "add-new-connections": { "title": "Add new connection" diff --git a/public/locales/pseudo-LOCALE/grafana.json b/public/locales/pseudo-LOCALE/grafana.json index 60ef9dc7484..88ca996b76a 100644 --- a/public/locales/pseudo-LOCALE/grafana.json +++ b/public/locales/pseudo-LOCALE/grafana.json @@ -2049,6 +2049,12 @@ "title": "Ŵĥy ĥőşŧ ŵįŧĥ Ğřäƒäʼnä?" } }, + "multicombobox": { + "all": { + "title": "Åľľ", + "title-filtered": "Åľľ (ƒįľŧęřęđ)" + } + }, "nav": { "add-new-connections": { "title": "Åđđ ʼnęŵ čőʼnʼnęčŧįőʼn"