From 43fa9311d75ec70f3f188aec1c22bd8b3ab1a194 Mon Sep 17 00:00:00 2001 From: samsch Date: Fri, 5 Dec 2025 11:05:15 -0500 Subject: [PATCH] POC of Combobox functioning with custom rendered options without significant performance loss --- .../components/Combobox/Combobox.story.tsx | 62 +++++++++++++++- .../src/components/Combobox/Combobox.tsx | 70 +++++++------------ .../src/components/Combobox/ComboboxList.tsx | 29 ++++++-- .../src/components/Combobox/filter.ts | 5 ++ .../components/Combobox/getComboboxStyles.ts | 4 +- .../src/components/Combobox/storyUtils.ts | 12 ++-- .../src/components/Combobox/types.ts | 21 +++++- .../components/Combobox/useComboboxFloat.ts | 3 +- .../components/Combobox/useMeasureMulti.ts | 6 +- 9 files changed, 151 insertions(+), 61 deletions(-) diff --git a/packages/grafana-ui/src/components/Combobox/Combobox.story.tsx b/packages/grafana-ui/src/components/Combobox/Combobox.story.tsx index eea67016ada..698d0107a61 100644 --- a/packages/grafana-ui/src/components/Combobox/Combobox.story.tsx +++ b/packages/grafana-ui/src/components/Combobox/Combobox.story.tsx @@ -1,12 +1,13 @@ import { action } from '@storybook/addon-actions'; import { useArgs } from '@storybook/preview-api'; import { Meta, StoryFn, StoryObj } from '@storybook/react'; -import { useEffect, useState } from 'react'; +import { useEffect, useMemo, useState } from 'react'; import { Field } from '../Forms/Field'; import { Combobox, ComboboxProps } from './Combobox'; import mdx from './Combobox.mdx'; +import { MENU_ITEM_FONT_SIZE, MENU_ITEM_LINE_HEIGHT, MENU_OPTION_HEIGHT } from './getComboboxStyles'; import { fakeSearchAPI, generateGroupingOptions, generateOptions } from './storyUtils'; import { ComboboxOption } from './types'; @@ -171,6 +172,65 @@ export const ManyOptions: Story = { }, }; +export const NodeOptions: Story = { + args: { + numberOfOptions: 350, + options: undefined, + value: undefined, + }, + parameters: { + docs: { + source: { + // necessary to keep storybook from choking on the option generation code + type: 'code', + }, + }, + }, + render: ({ numberOfOptions, ...args }: PropsAndCustomArgs) => { + // eslint-disable-next-line react-hooks/rules-of-hooks + const [dynamicArgs, setArgs] = useArgs(); + + // eslint-disable-next-line react-hooks/rules-of-hooks + const options: ComboboxOption[] = useMemo(() => { + return Array.from({ length: numberOfOptions }, (_, index) => { + const label = + index % 2 === 0 + ? { + node: ( + <> + {`Option ${index}`} +
+ (Even) + + ), + text: `Option ${index}`, + size: MENU_ITEM_FONT_SIZE * MENU_ITEM_LINE_HEIGHT + MENU_OPTION_HEIGHT, + } + : `Option ${index}`; + return { + label, + value: String(index), + }; + }); + }, [numberOfOptions]); + + const { onChange, ...rest } = args; + return ( + + { + setArgs({ value: value?.value || null }); + onChangeAction(value); + }} + /> + + ); + }, +}; + function loadOptionsWithLabels(inputValue: string) { loadOptionsAction(inputValue); return fakeSearchAPI(`http://example.com/search?errorOnQuery=break&query=${inputValue}`); diff --git a/packages/grafana-ui/src/components/Combobox/Combobox.tsx b/packages/grafana-ui/src/components/Combobox/Combobox.tsx index a4664c62441..9665ff3f736 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, type Range } from '@tanstack/react-virtual'; import { useCombobox } from 'downshift'; -import React, { ComponentProps, useCallback, useId, useMemo } from 'react'; +import React, { ComponentProps, useId, useMemo } from 'react'; +import { Subject } from 'rxjs'; import { t } from '@grafana/i18n'; @@ -14,11 +14,10 @@ import { Portal } from '../Portal/Portal'; import { ComboboxList } from './ComboboxList'; import { SuffixIcon } from './SuffixIcon'; import { itemToString } from './filter'; -import { getComboboxStyles, MENU_OPTION_HEIGHT, MENU_OPTION_HEIGHT_DESCRIPTION } from './getComboboxStyles'; +import { getComboboxStyles } from './getComboboxStyles'; 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. @@ -154,7 +153,6 @@ export const Combobox = (props: ComboboxProps) => const { options: filteredOptions, - groupStartIndices, updateOptions, asyncLoading, asyncError, @@ -195,49 +193,34 @@ export const Combobox = (props: ComboboxProps) => const styles = useStyles2(getComboboxStyles); + // Maybe need to move this to ComboboxList? Is this actually necessary? it's only expanding the returned + // array of indexes, and that's already expanded by the overscan setting // 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); + // 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); - } - } + // // 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] - ); + // return rangeToReturn; + // }, + // [filteredOptions, groupStartIndices] + // ); - const rowVirtualizer = useVirtualizer({ - count: filteredOptions.length, - getScrollElement: () => scrollRef.current, - estimateSize: (index: number) => { - const firstGroupItem = isNewGroup(filteredOptions[index], index > 0 ? filteredOptions[index - 1] : undefined); - const hasDescription = 'description' in filteredOptions[index]; - const hasGroup = 'group' in filteredOptions[index]; - - let itemHeight = MENU_OPTION_HEIGHT; - if (hasDescription) { - itemHeight = MENU_OPTION_HEIGHT_DESCRIPTION; - } - if (firstGroupItem && hasGroup) { - itemHeight += MENU_OPTION_HEIGHT; - } - return itemHeight; - }, - overscan: VIRTUAL_OVERSCAN_ITEMS, - rangeExtractor, - }); + const scrollToIndexObservable = useMemo(() => { + return new Subject(); + }, []); const { isOpen, @@ -290,7 +273,7 @@ export const Combobox = (props: ComboboxProps) => onHighlightedIndexChange: ({ highlightedIndex, type }) => { if (type !== useCombobox.stateChangeTypes.MenuMouseLeave) { - rowVirtualizer.scrollToIndex(highlightedIndex); + scrollToIndexObservable.next(highlightedIndex); } }, onStateChange: ({ inputValue: newInputValue, type, selectedItem: newSelectedItem }) => { @@ -418,6 +401,7 @@ export const Combobox = (props: ComboboxProps) => scrollRef={scrollRef} getItemProps={getItemProps} error={asyncError} + scrollToIndexObservable={scrollToIndexObservable} /> )} diff --git a/packages/grafana-ui/src/components/Combobox/ComboboxList.tsx b/packages/grafana-ui/src/components/Combobox/ComboboxList.tsx index bc738338204..03a38f0294c 100644 --- a/packages/grafana-ui/src/components/Combobox/ComboboxList.tsx +++ b/packages/grafana-ui/src/components/Combobox/ComboboxList.tsx @@ -1,14 +1,15 @@ import { cx } from '@emotion/css'; import { useVirtualizer } from '@tanstack/react-virtual'; import type { UseComboboxPropGetters } from 'downshift'; -import { useCallback } from 'react'; +import { useCallback, useEffect } from 'react'; +import { Subject } from 'rxjs'; import { useStyles2 } from '../../themes/ThemeContext'; import { Checkbox } from '../Forms/Checkbox'; import { ScrollContainer } from '../ScrollContainer/ScrollContainer'; import { AsyncError, LoadingOptions, NotFoundError } from './MessageRows'; -import { getComboboxStyles, MENU_OPTION_HEIGHT, MENU_OPTION_HEIGHT_DESCRIPTION } from './getComboboxStyles'; +import { DESCRIPTION_HEIGHT, getComboboxStyles, MENU_OPTION_HEIGHT } from './getComboboxStyles'; import { ALL_OPTION_VALUE, ComboboxOption } from './types'; import { isNewGroup } from './utils'; @@ -24,6 +25,7 @@ interface ComboboxListProps { isMultiSelect?: boolean; error?: boolean; loading?: boolean; + scrollToIndexObservable?: Subject; } export const ComboboxList = ({ @@ -36,6 +38,7 @@ export const ComboboxList = ({ isMultiSelect = false, error = false, loading = false, + scrollToIndexObservable, }: ComboboxListProps) => { const styles = useStyles2(getComboboxStyles); @@ -46,8 +49,11 @@ export const ComboboxList = ({ const hasGroup = 'group' in options[index]; let itemHeight = MENU_OPTION_HEIGHT; + if (typeof options[index].label === 'object') { + itemHeight = options[index].label.size; + } if (hasDescription) { - itemHeight = MENU_OPTION_HEIGHT_DESCRIPTION; + itemHeight += DESCRIPTION_HEIGHT; } if (firstGroupItem && hasGroup) { itemHeight += MENU_OPTION_HEIGHT; @@ -64,6 +70,19 @@ export const ComboboxList = ({ overscan: VIRTUAL_OVERSCAN_ITEMS, }); + useEffect(() => { + if (scrollToIndexObservable) { + const subscription = scrollToIndexObservable.subscribe((index) => { + rowVirtualizer.scrollToIndex(index); + }); + + return () => { + subscription.unsubscribe(); + }; + } + return undefined; + }, [scrollToIndexObservable, rowVirtualizer]); + const isOptionSelected = useCallback( (item: ComboboxOption) => selectedItems.some((opt) => opt.value === item.value), [selectedItems] @@ -90,6 +109,8 @@ export const ComboboxList = ({ // the option for aria-describedby. const groupHeaderId = groupHeaderItem ? `combobox-option-group-${groupHeaderItem.value}` : undefined; + const label = typeof item.label === 'object' ? item.label.node : (item.label ?? item.value); + return ( // Wrapping div should have no styling other than virtual list positioning. // It's children (header and option) should appear as flat list items. @@ -151,7 +172,7 @@ export const ComboboxList = ({ )}
-
{item.label ?? item.value}
+
{label}
{item.description &&
{item.description}
}
diff --git a/packages/grafana-ui/src/components/Combobox/filter.ts b/packages/grafana-ui/src/components/Combobox/filter.ts index 4562a486b87..868bfc45fd4 100644 --- a/packages/grafana-ui/src/components/Combobox/filter.ts +++ b/packages/grafana-ui/src/components/Combobox/filter.ts @@ -6,6 +6,11 @@ export function itemToString(item?: ComboboxOption if (item == null) { return ''; } + + if (typeof item.label === 'object') { + return item.label.text; + } + return item.label ?? item.value.toString(); } diff --git a/packages/grafana-ui/src/components/Combobox/getComboboxStyles.ts b/packages/grafana-ui/src/components/Combobox/getComboboxStyles.ts index 15c30dadd82..340f7bc78e5 100644 --- a/packages/grafana-ui/src/components/Combobox/getComboboxStyles.ts +++ b/packages/grafana-ui/src/components/Combobox/getComboboxStyles.ts @@ -13,8 +13,8 @@ export const MENU_ITEM_LINE_HEIGHT = 1.5; // Used with Downshift to get the height of each item export const MENU_OPTION_HEIGHT = MENU_ITEM_GAP + MENU_ITEM_PADDING * 2 + MENU_ITEM_FONT_SIZE * MENU_ITEM_LINE_HEIGHT; -export const MENU_OPTION_HEIGHT_DESCRIPTION = - MENU_OPTION_HEIGHT + MENU_ITEM_DESCRIPTION_FONT_SIZE * MENU_ITEM_LINE_HEIGHT; +export const DESCRIPTION_HEIGHT = MENU_ITEM_DESCRIPTION_FONT_SIZE * MENU_ITEM_LINE_HEIGHT; +export const MENU_OPTION_HEIGHT_DESCRIPTION = MENU_OPTION_HEIGHT + DESCRIPTION_HEIGHT; export const POPOVER_MAX_HEIGHT = MENU_OPTION_HEIGHT * 8.5; export const getComboboxStyles = (theme: GrafanaTheme2) => { diff --git a/packages/grafana-ui/src/components/Combobox/storyUtils.ts b/packages/grafana-ui/src/components/Combobox/storyUtils.ts index 481b147b4ea..d519fe0a045 100644 --- a/packages/grafana-ui/src/components/Combobox/storyUtils.ts +++ b/packages/grafana-ui/src/components/Combobox/storyUtils.ts @@ -1,7 +1,7 @@ -import { ComboboxOption } from './types'; +import { ComboboxStringOption } from './types'; -let fakeApiOptions: Array>; -export async function fakeSearchAPI(urlString: string): Promise>> { +let fakeApiOptions: Array>; +export async function fakeSearchAPI(urlString: string): Promise>> { const searchParams = new URL(urlString).searchParams; const errorOnQuery = searchParams.get('errorOnQuery')?.toLowerCase(); @@ -26,19 +26,19 @@ export async function fakeSearchAPI(urlString: string): Promise>>((resolve) => { + return new Promise>>((resolve) => { setTimeout(() => resolve(filteredOptions), delay); }); } -export async function generateOptions(amount: number): Promise { +export async function generateOptions(amount: number): Promise { return Array.from({ length: amount }, (_, index) => ({ label: 'Option ' + index, value: index.toString(), })); } -export async function generateGroupingOptions(amount: number): Promise { +export async function generateGroupingOptions(amount: number): Promise { return Array.from({ length: amount }, (_, index) => ({ label: 'Option ' + index, value: index.toString(), diff --git a/packages/grafana-ui/src/components/Combobox/types.ts b/packages/grafana-ui/src/components/Combobox/types.ts index 537982d5865..9d53f43fba8 100644 --- a/packages/grafana-ui/src/components/Combobox/types.ts +++ b/packages/grafana-ui/src/components/Combobox/types.ts @@ -1,9 +1,28 @@ +import { ReactNode } from 'react'; + export const ALL_OPTION_VALUE = '__GRAFANA_INTERNAL_MULTICOMBOBOX_ALL_OPTION__'; +interface NodeOption { + node: ReactNode; + /** + * text property much exactly match the text in the `node` ReactNode property + */ + text: string; + size: number; +} + export type ComboboxOption = { - label?: string; + label?: string | NodeOption; value: T; description?: string; group?: string; infoOption?: boolean; }; + +export interface ComboboxStringOption { + label?: string; + value: T; + description?: string; + group?: string; + infoOption?: boolean; +} diff --git a/packages/grafana-ui/src/components/Combobox/useComboboxFloat.ts b/packages/grafana-ui/src/components/Combobox/useComboboxFloat.ts index f2bf1f968db..8db9c6f48c8 100644 --- a/packages/grafana-ui/src/components/Combobox/useComboboxFloat.ts +++ b/packages/grafana-ui/src/components/Combobox/useComboboxFloat.ts @@ -67,7 +67,8 @@ export const useComboboxFloat = (items: Array>, const itemsToLookAt = Math.min(items.length, WIDTH_CALCULATION_LIMIT_ITEMS); for (let i = 0; i < itemsToLookAt; i++) { - const itemLabel = items[i].label ?? items[i].value.toString(); + const label = items[i].label; + const itemLabel = (typeof label === 'object' ? label.text : label) ?? items[i].value.toString(); longestItem = itemLabel.length > longestItem.length ? itemLabel : longestItem; } diff --git a/packages/grafana-ui/src/components/Combobox/useMeasureMulti.ts b/packages/grafana-ui/src/components/Combobox/useMeasureMulti.ts index 2b516c75fbc..87676880831 100644 --- a/packages/grafana-ui/src/components/Combobox/useMeasureMulti.ts +++ b/packages/grafana-ui/src/components/Combobox/useMeasureMulti.ts @@ -30,9 +30,9 @@ export function useMeasureMulti( let currWidth = 0; for (let i = 0; i < selectedItems.length; i++) { // Measure text width and add size of padding, separator and close button - currWidth += - measureText(selectedItems[i].label || '', FONT_SIZE).width + - (disabled ? EXTRA_PILL_DISABLED_SIZE : EXTRA_PILL_SIZE); + const item = selectedItems[i]; + const text = typeof item.label === 'object' ? item.label.text : item.label || ''; + currWidth += measureText(text, FONT_SIZE).width + (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. Cap at maximum number of items.