From 38151b1ae476ce9d4e1e9a52566ce06d04a27e83 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Laura=20Fern=C3=A1ndez?= Date: Fri, 14 Mar 2025 12:05:25 +0100 Subject: [PATCH] Combobox: add grouping functionality (#100603) * Use useOptions in Combobox * Clean code * Betterer results * Add grouping to Combobox * Fix code after mergin main * Manage ids * wip - first pass at improved dom structure for a11y * improve styling, remove old implementation * more style!!!! * more tidy up * deprecated comment * another comment! * tests * remember the index of each group --------- Co-authored-by: joshhunt --- .../components/Combobox/Combobox.story.tsx | 10 +- .../src/components/Combobox/Combobox.test.tsx | 43 ++++ .../src/components/Combobox/Combobox.tsx | 184 ++++++++++++------ .../src/components/Combobox/MultiCombobox.tsx | 18 +- .../components/Combobox/OptionListItem.tsx | 5 +- .../components/Combobox/getComboboxStyles.ts | 27 ++- .../src/components/Combobox/storyUtils.ts | 2 +- .../src/components/Combobox/useOptions.ts | 87 +++++---- .../src/components/Combobox/utils.ts | 15 ++ 9 files changed, 275 insertions(+), 116 deletions(-) create mode 100644 packages/grafana-ui/src/components/Combobox/utils.ts diff --git a/packages/grafana-ui/src/components/Combobox/Combobox.story.tsx b/packages/grafana-ui/src/components/Combobox/Combobox.story.tsx index fe3c9bca3f5..4a2ee2c627f 100644 --- a/packages/grafana-ui/src/components/Combobox/Combobox.story.tsx +++ b/packages/grafana-ui/src/components/Combobox/Combobox.story.tsx @@ -7,7 +7,7 @@ import { Field } from '../Forms/Field'; import { Combobox, ComboboxProps } from './Combobox'; import mdx from './Combobox.mdx'; -import { fakeSearchAPI, generateOptions } from './storyUtils'; +import { fakeSearchAPI, generateGroupingOptions, generateOptions } from './storyUtils'; import { ComboboxOption } from './types'; type PropsAndCustomArgs = ComboboxProps & { @@ -107,6 +107,14 @@ export const CustomValue: Story = { render: BaseCombobox, }; +export const Groups: Story = { + args: { + options: await generateGroupingOptions(500), + value: '34', + }, + render: BaseCombobox, +}; + export const ManyOptions: Story = { args: { numberOfOptions: 1e5, diff --git a/packages/grafana-ui/src/components/Combobox/Combobox.test.tsx b/packages/grafana-ui/src/components/Combobox/Combobox.test.tsx index e54c94d1977..9058f716ccf 100644 --- a/packages/grafana-ui/src/components/Combobox/Combobox.test.tsx +++ b/packages/grafana-ui/src/components/Combobox/Combobox.test.tsx @@ -151,6 +151,49 @@ describe('Combobox', () => { expect(screen.getByRole('option', { name: 'Default' })).toHaveAttribute('aria-selected', 'true'); }); + describe('groups', () => { + it('renders group headers', async () => { + const options = [ + { label: 'Option 1', value: '1', group: 'Group 1' }, + { label: 'Option 2', value: '2', group: 'Group 1' }, + { label: 'Option 3', value: '3', group: 'Group 2' }, + { label: 'Option 4', value: '4', group: 'Group 2' }, + ]; + + render(); + + const input = screen.getByRole('combobox'); + await userEvent.click(input); + + expect(screen.getByText('Group 1')).toBeInTheDocument(); + expect(screen.getByText('Group 2')).toBeInTheDocument(); + }); + + it('sorts options within groups', async () => { + const options = [ + { label: 'Option 1', value: '1', group: 'Group 1' }, + { label: 'Option 2', value: '2', group: 'Group 2' }, + { label: 'Option 3', value: '3', group: 'Group 1' }, + { label: 'Option 4', value: '4', group: 'Group 2' }, + { label: 'Option 5', value: '5', group: 'Group 2' }, + { label: 'Option 6', value: '6', group: 'Group 1' }, + ]; + + render(); + + const input = screen.getByRole('combobox'); + await userEvent.click(input); + + const allHeaders = await screen.findAllByRole('presentation'); + expect(allHeaders).toHaveLength(2); + + const listbox = await screen.findByRole('listbox'); + expect(listbox).toHaveTextContent( + ['Group 1', 'Option 1', 'Option 3', 'Option 6', 'Group 2', 'Option 2', 'Option 4', 'Option 5'].join('') + ); + }); + }); + describe('size support', () => { it('should require minWidth to be set with auto width', () => { // @ts-expect-error diff --git a/packages/grafana-ui/src/components/Combobox/Combobox.tsx b/packages/grafana-ui/src/components/Combobox/Combobox.tsx index 2a1ea6ae28c..e7096544fe7 100644 --- a/packages/grafana-ui/src/components/Combobox/Combobox.tsx +++ b/packages/grafana-ui/src/components/Combobox/Combobox.tsx @@ -1,7 +1,7 @@ import { cx } from '@emotion/css'; -import { useVirtualizer } from '@tanstack/react-virtual'; +import { useVirtualizer, type Range } from '@tanstack/react-virtual'; import { useCombobox } from 'downshift'; -import { useId, useMemo } from 'react'; +import { useCallback, useId, useMemo } from 'react'; import { useStyles2 } from '../../themes'; import { t } from '../../utils/i18n'; @@ -17,6 +17,7 @@ import { getComboboxStyles, MENU_OPTION_HEIGHT, MENU_OPTION_HEIGHT_DESCRIPTION } import { ComboboxOption } from './types'; import { useComboboxFloat } from './useComboboxFloat'; import { useOptions } from './useOptions'; +import { isNewGroup } from './utils'; // TODO: It would be great if ComboboxOption["label"] was more generic so that if consumers do pass it in (for async), // then the onChange handler emits ComboboxOption with the label as non-undefined. @@ -129,9 +130,11 @@ export const Combobox = (props: ComboboxProps) => // Value can be an actual scalar Value (string or number), or an Option (value + label), so // get a consistent Value from it const value = typeof valueProp === 'object' ? valueProp?.value : valueProp; + const baseId = useId().replace(/:/g, '--'); const { options: filteredOptions, + groupStartIndices, updateOptions, asyncLoading, asyncError, @@ -167,20 +170,52 @@ export const Combobox = (props: ComboboxProps) => return typeof valueProp === 'object' ? valueProp : { value: valueProp, label: valueProp.toString() }; }, [selectedItemIndex, isAsync, valueProp, allOptions]); - const menuId = `downshift-${useId().replace(/:/g, '--')}-menu`; - const labelId = `downshift-${useId().replace(/:/g, '--')}-label`; + const menuId = `${baseId}-downshift-menu`; + const labelId = `${baseId}-downshift-label`; const styles = useStyles2(getComboboxStyles); - const virtualizerOptions = { + // Injects the group header for the first rendered item into the range to render. + // Accepts the range that useVirtualizer wants to render, and then returns indexes + // to actually render. + const rangeExtractor = useCallback( + (range: Range) => { + const startIndex = Math.max(0, range.startIndex - range.overscan); + const endIndex = Math.min(filteredOptions.length - 1, range.endIndex + range.overscan); + const rangeToReturn = Array.from({ length: endIndex - startIndex + 1 }, (_, i) => startIndex + i); + + // If the first item doesn't have a group, no need to find a header for it + const firstDisplayedOption = filteredOptions[rangeToReturn[0]]; + if (firstDisplayedOption?.group) { + const groupStartIndex = groupStartIndices.get(firstDisplayedOption.group); + if (groupStartIndex !== undefined && groupStartIndex < rangeToReturn[0]) { + rangeToReturn.unshift(groupStartIndex); + } + } + + return rangeToReturn; + }, + [filteredOptions, groupStartIndices] + ); + + const rowVirtualizer = useVirtualizer({ count: filteredOptions.length, getScrollElement: () => scrollRef.current, - estimateSize: (index: number) => - filteredOptions[index].description ? MENU_OPTION_HEIGHT_DESCRIPTION : MENU_OPTION_HEIGHT, + estimateSize: (index: number) => { + const firstGroupItem = isNewGroup(filteredOptions[index], index > 0 ? filteredOptions[index - 1] : undefined); + const hasDescription = 'description' in filteredOptions[index]; + let itemHeight = MENU_OPTION_HEIGHT; + if (hasDescription) { + itemHeight = MENU_OPTION_HEIGHT_DESCRIPTION; + } + if (firstGroupItem) { + itemHeight += MENU_OPTION_HEIGHT; + } + return itemHeight; + }, overscan: VIRTUAL_OVERSCAN_ITEMS, - }; - - const rowVirtualizer = useVirtualizer(virtualizerOptions); + rangeExtractor, + }); const { isOpen, @@ -271,8 +306,8 @@ export const Combobox = (props: ComboboxProps) => const { inputRef, floatingRef, floatStyles, scrollRef } = useComboboxFloat(filteredOptions, isOpen); const isAutoSize = width === 'auto'; - const InputComponent = isAutoSize ? AutoSizeInput : Input; + const placeholder = (isOpen ? itemToString(selectedItem) : null) || placeholderProp; const suffixIcon = asyncLoading ? 'spinner' @@ -281,7 +316,29 @@ export const Combobox = (props: ComboboxProps) => ? 'search' : 'angle-down'; - const placeholder = (isOpen ? itemToString(selectedItem) : null) || placeholderProp; + const inputSuffix = ( + <> + {value && value === selectedItem?.value && isClearable && ( + { + selectItem(null); + }} + onKeyDown={(e) => { + if (e.key === 'Enter' || e.key === ' ') { + selectItem(null); + } + }} + /> + )} + + + + ); return (
@@ -294,36 +351,10 @@ export const Combobox = (props: ComboboxProps) => loading={loading} invalid={invalid} className={styles.input} - suffix={ - <> - {!!value && value === selectedItem?.value && isClearable && ( - { - selectItem(null); - }} - onKeyDown={(e) => { - if (e.key === 'Enter' || e.key === ' ') { - selectItem(null); - } - }} - /> - )} - - - - } + suffix={inputSuffix} {...getInputProps({ ref: inputRef, - /* Empty onCall to avoid TS error - * See issue here: https://github.com/downshift-js/downshift/issues/718 - * Downshift repo: https://github.com/downshift-js/downshift/tree/master - */ - onChange: noop, + onChange: noop, // Empty onCall to avoid TS error https://github.com/downshift-js/downshift/issues/718 'aria-labelledby': ariaLabelledBy, // Label should be handled with the Field component placeholder, })} @@ -331,9 +362,7 @@ export const Combobox = (props: ComboboxProps) =>
(props: ComboboxProps) => {isOpen && ( {!asyncError && ( -
    - {rowVirtualizer.getVirtualItems().map((virtualRow) => { +
    + {rowVirtualizer.getVirtualItems().map((virtualRow, index, allVirtualRows) => { const item = filteredOptions[virtualRow.index]; + const startingNewGroup = isNewGroup(item, filteredOptions[virtualRow.index - 1]); + + // Find the item that renders the group header. It can be this same item if this is rendering it. + const groupHeaderIndex = allVirtualRows.find((row) => { + const rowItem = filteredOptions[row.index]; + return rowItem.group === item.group; + }); + const groupHeaderItem = groupHeaderIndex && filteredOptions[groupHeaderIndex.index]; + + const itemId = `${baseId}-option-${item.value}`; + // If we're rendering the group header, this is the ID for it. Otherwise its used on + // the option for aria-describedby. + const groupHeaderId = groupHeaderItem + ? `${baseId}-option-group-${groupHeaderItem.value}` + : undefined; return ( -
  • -
    - {item.label ?? item.value} - {item.description && {item.description}} + {startingNewGroup && ( + + )} + +
    +
    + {item.label ?? item.value} + {item.description && {item.description}} +
    -
    • +
    ); })} -
+
)} +
{asyncError && } {filteredOptions.length === 0 && !asyncError && } diff --git a/packages/grafana-ui/src/components/Combobox/MultiCombobox.tsx b/packages/grafana-ui/src/components/Combobox/MultiCombobox.tsx index 7d18074910a..561cffbf2c2 100644 --- a/packages/grafana-ui/src/components/Combobox/MultiCombobox.tsx +++ b/packages/grafana-ui/src/components/Combobox/MultiCombobox.tsx @@ -27,6 +27,7 @@ import { useComboboxFloat } from './useComboboxFloat'; import { MAX_SHOWN_ITEMS, useMeasureMulti } from './useMeasureMulti'; import { useMultiInputAutoSize } from './useMultiInputAutoSize'; import { useOptions } from './useOptions'; +import { isNewGroup } from './utils'; interface MultiComboboxBaseProps extends Omit, 'value' | 'onChange'> { value?: T[] | Array>; @@ -353,6 +354,7 @@ export const MultiCombobox = (props: MultiComboboxPro const itemProps = getItemProps({ item, index }); const isSelected = isOptionSelected(item); const id = 'multicombobox-option-' + item.value.toString(); + const groupHeaderid = 'multicombobox-option-group-' + item.value.toString(); const isAll = item.value === ALL_OPTION_VALUE; // TODO: fix bug where if the search filtered items list is the @@ -373,7 +375,7 @@ export const MultiCombobox = (props: MultiComboboxPro
@@ -453,17 +455,3 @@ function isComboboxOptions( ): value is Array> { return typeof value[0] === 'object'; } - -const isNewGroup = (option: ComboboxOption, prevOption?: ComboboxOption) => { - const currentGroup = option.group; - - if (!currentGroup) { - return prevOption?.group ? true : false; - } - - if (!prevOption) { - return true; - } - - return prevOption.group !== currentGroup; -}; diff --git a/packages/grafana-ui/src/components/Combobox/OptionListItem.tsx b/packages/grafana-ui/src/components/Combobox/OptionListItem.tsx index 3a929be51a9..bf06e00d823 100644 --- a/packages/grafana-ui/src/components/Combobox/OptionListItem.tsx +++ b/packages/grafana-ui/src/components/Combobox/OptionListItem.tsx @@ -1,12 +1,13 @@ import { cx } from '@emotion/css'; +import { ReactNode } from 'react'; import { useStyles2 } from '../../themes'; import { getComboboxStyles } from './getComboboxStyles'; interface Props { - label: string; - description?: string; + label: ReactNode; + description?: ReactNode; id: string; isGroup?: boolean; } diff --git a/packages/grafana-ui/src/components/Combobox/getComboboxStyles.ts b/packages/grafana-ui/src/components/Combobox/getComboboxStyles.ts index 7bee7c19199..7f721a51d53 100644 --- a/packages/grafana-ui/src/components/Combobox/getComboboxStyles.ts +++ b/packages/grafana-ui/src/components/Combobox/getComboboxStyles.ts @@ -32,6 +32,27 @@ export const getComboboxStyles = (theme: GrafanaTheme2) => { label: 'combobox-menu-ul-container', listStyle: 'none', }), + + // New class for on the virtual list item. Should be on the wrapper around the group header and option. + listItem: css({ + label: 'list-item', + width: '100%', + position: 'absolute', + }), + + // New class used in single combobox group headers + newOptionGroup: css({ + label: 'combobox-new-option-group', + textOverflow: 'ellipsis', + overflow: 'hidden', + letterSpacing: 0, + color: theme.colors.text.secondary, + fontSize: theme.typography.bodySmall.fontSize, + fontWeight: theme.typography.fontWeightLight, + padding: MENU_ITEM_PADDING, + borderTop: `1px solid ${theme.colors.border.weak}`, + }), + optionBasic: css({ label: 'combobox-option', position: 'absolute', @@ -55,6 +76,8 @@ export const getComboboxStyles = (theme: GrafanaTheme2) => { }, }, }), + + /** @deprecated - only used in multicombobox, will refactor it */ optionGroup: css({ cursor: 'default', padding: MENU_ITEM_PADDING, @@ -74,8 +97,10 @@ export const getComboboxStyles = (theme: GrafanaTheme2) => { overflow: 'hidden', fontSize: MENU_ITEM_FONT_SIZE, fontWeight: MENU_ITEM_FONT_WEIGHT, + lineHeight: MENU_ITEM_LINE_HEIGHT, letterSpacing: 0, // pr todo: text in grafana has a slightly different letter spacing, which causes measureText() to be ~5% off }), + /** @deprecated - only used in multicombobox, will refactor it */ optionLabelGroup: css({ label: 'combobox-option-label-group', color: theme.colors.text.secondary, @@ -93,7 +118,7 @@ export const getComboboxStyles = (theme: GrafanaTheme2) => { }), optionFocused: css({ label: 'combobox-option-focused', - top: 0, + // top: 0, background: theme.colors.action.focus, '@media (forced-colors: active), (prefers-contrast: more)': { border: `1px solid ${theme.colors.primary.border}`, diff --git a/packages/grafana-ui/src/components/Combobox/storyUtils.ts b/packages/grafana-ui/src/components/Combobox/storyUtils.ts index d39040a6121..481b147b4ea 100644 --- a/packages/grafana-ui/src/components/Combobox/storyUtils.ts +++ b/packages/grafana-ui/src/components/Combobox/storyUtils.ts @@ -42,6 +42,6 @@ export async function generateGroupingOptions(amount: number): Promise ({ label: 'Option ' + index, value: index.toString(), - group: index % 9 !== 0 ? 'Group ' + Math.floor(index / 10) : undefined, + group: index % 19 !== 0 ? 'Group ' + Math.floor(index / 20) : undefined, })); } diff --git a/packages/grafana-ui/src/components/Combobox/useOptions.ts b/packages/grafana-ui/src/components/Combobox/useOptions.ts index fffad1613cb..7801fe5b007 100644 --- a/packages/grafana-ui/src/components/Combobox/useOptions.ts +++ b/packages/grafana-ui/src/components/Combobox/useOptions.ts @@ -92,43 +92,64 @@ export function useOptions(rawOptions: AsyncOptions>) => { - const groupedOptions = new Map>>(); - for (const option of options) { - const groupExists = groupedOptions.has(option.group); - if (groupExists) { - groupedOptions.get(option.group)?.push(option); - } else { - groupedOptions.set(option.group, [option]); - } - } - - // Reorganize options to have groups first, then undefined group - const reorganizeOptions = []; - for (const [group, groupOptions] of groupedOptions) { - if (!group) { - continue; - } - reorganizeOptions.push(...groupOptions); - } - - const undefinedGroupOptions = groupedOptions.get(undefined); - if (undefinedGroupOptions) { - reorganizeOptions.push(...undefinedGroupOptions); - } - return reorganizeOptions; - }, []); - const stringifiedOptions = useMemo(() => { return isAsync ? [] : rawOptions.map(itemToString); }, [isAsync, rawOptions]); - const finalOptions = useMemo(() => { - const currentOptions = isAsync ? asyncOptions : fuzzyFind(rawOptions, stringifiedOptions, userTypedSearch); - const currentOptionsOrganised = organizeOptionsByGroup(currentOptions); + // Create a list of options filtered by the current search. + // If async, just returns the async options. + const filteredOptions = useMemo(() => { + if (isAsync) { + return asyncOptions; + } - return addCustomValue(currentOptionsOrganised); - }, [isAsync, organizeOptionsByGroup, addCustomValue, asyncOptions, rawOptions, userTypedSearch, stringifiedOptions]); + return fuzzyFind(rawOptions, stringifiedOptions, userTypedSearch); + }, [asyncOptions, isAsync, rawOptions, stringifiedOptions, userTypedSearch]); - return { options: finalOptions, updateOptions, asyncLoading, asyncError }; + const [finalOptions, groupStartIndices] = useMemo(() => { + const { options, groupStartIndices } = sortByGroup(filteredOptions); + + return [addCustomValue(options), groupStartIndices]; + }, [filteredOptions, addCustomValue]); + + return { options: finalOptions, groupStartIndices, updateOptions, asyncLoading, asyncError }; +} + +function sortByGroup(options: Array>) { + const groupedOptions = new Map>>(); + for (const option of options) { + const groupExists = groupedOptions.has(option.group); + if (groupExists) { + groupedOptions.get(option.group)?.push(option); + } else { + groupedOptions.set(option.group, [option]); + } + } + + // Create a map to track the starting index of each group + const groupStartIndices = new Map(); + let currentIndex = 0; + + // Reorganize options to have groups first, then undefined group + const reorganizeOptions = []; + for (const [group, groupOptions] of groupedOptions) { + if (!group) { + continue; + } + + groupStartIndices.set(group, currentIndex); + reorganizeOptions.push(...groupOptions); + currentIndex += groupOptions.length; + } + + const undefinedGroupOptions = groupedOptions.get(undefined); + if (undefinedGroupOptions) { + groupStartIndices.set('undefined', currentIndex); + reorganizeOptions.push(...undefinedGroupOptions); + } + + return { + options: reorganizeOptions, + groupStartIndices, + }; } diff --git a/packages/grafana-ui/src/components/Combobox/utils.ts b/packages/grafana-ui/src/components/Combobox/utils.ts new file mode 100644 index 00000000000..e335e572ce0 --- /dev/null +++ b/packages/grafana-ui/src/components/Combobox/utils.ts @@ -0,0 +1,15 @@ +import { ComboboxOption } from './types'; + +export const isNewGroup = (option: ComboboxOption, prevOption?: ComboboxOption) => { + const currentGroup = option.group; + + if (!currentGroup) { + return prevOption?.group ? true : false; + } + + if (!prevOption) { + return true; + } + + return prevOption.group !== currentGroup; +};