diff --git a/packages/grafana-ui/src/components/Combobox/Combobox.test.tsx b/packages/grafana-ui/src/components/Combobox/Combobox.test.tsx index 7535cf76053..92c97f8d781 100644 --- a/packages/grafana-ui/src/components/Combobox/Combobox.test.tsx +++ b/packages/grafana-ui/src/components/Combobox/Combobox.test.tsx @@ -514,11 +514,9 @@ describe('Combobox', () => { }); 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'); + + expect(customItem).toHaveTextContent('fir'); + expect(customItem).toHaveTextContent('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 a6508d18297..0d0ab498727 100644 --- a/packages/grafana-ui/src/components/Combobox/Combobox.tsx +++ b/packages/grafana-ui/src/components/Combobox/Combobox.tsx @@ -9,9 +9,8 @@ import { Icon } from '../Icon/Icon'; import { AutoSizeInput } from '../Input/AutoSizeInput'; import { Input, Props as InputProps } from '../Input/Input'; import { Portal } from '../Portal/Portal'; -import { ScrollContainer } from '../ScrollContainer/ScrollContainer'; -import { AsyncError, NotFoundError } from './MessageRows'; +import { ComboboxList } from './ComboboxList'; import { itemToString } from './filter'; import { getComboboxStyles, MENU_OPTION_HEIGHT, MENU_OPTION_HEIGHT_DESCRIPTION } from './getComboboxStyles'; import { ComboboxOption } from './types'; @@ -375,82 +374,14 @@ export const Combobox = (props: ComboboxProps) => })} > {isOpen && ( - - {!asyncError && ( -
- {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 ( - // Wrapping div should have no styling other than virtual list positioning. - // It's children (header and option) should appear as flat list items. -
- {startingNewGroup && ( - - )} - -
-
- {item.label ?? item.value} - {item.description && {item.description}} -
-
-
- ); - })} -
- )} - -
- {asyncError && } - {filteredOptions.length === 0 && !asyncError && } -
- + )} diff --git a/packages/grafana-ui/src/components/Combobox/ComboboxList.tsx b/packages/grafana-ui/src/components/Combobox/ComboboxList.tsx new file mode 100644 index 00000000000..bf6a7e7e384 --- /dev/null +++ b/packages/grafana-ui/src/components/Combobox/ComboboxList.tsx @@ -0,0 +1,162 @@ +import { cx } from '@emotion/css'; +import { useVirtualizer } from '@tanstack/react-virtual'; +import type { UseComboboxPropGetters } from 'downshift'; +import { useCallback } from 'react'; + +import { useStyles2 } from '../../themes'; +import { Checkbox } from '../Forms/Checkbox'; +import { ScrollContainer } from '../ScrollContainer/ScrollContainer'; + +import { AsyncError, NotFoundError } from './MessageRows'; +import { getComboboxStyles, MENU_OPTION_HEIGHT, MENU_OPTION_HEIGHT_DESCRIPTION } from './getComboboxStyles'; +import { ALL_OPTION_VALUE, ComboboxOption } from './types'; +import { isNewGroup } from './utils'; + +export const VIRTUAL_OVERSCAN_ITEMS = 4; + +interface ComboboxListProps { + options: Array>; + highlightedIndex: number | null; + selectedItems?: Array>; + scrollRef: React.RefObject; + getItemProps: UseComboboxPropGetters>['getItemProps']; + enableAllOption?: boolean; + isMultiSelect?: boolean; + error?: boolean; +} + +export const ComboboxList = ({ + options, + highlightedIndex, + selectedItems = [], + scrollRef, + getItemProps, + enableAllOption, + isMultiSelect = false, + error = false, +}: ComboboxListProps) => { + const styles = useStyles2(getComboboxStyles); + + const estimateSize = useCallback( + (index: number) => { + const firstGroupItem = isNewGroup(options[index], index > 0 ? options[index - 1] : undefined); + const hasDescription = 'description' in options[index]; + const hasGroup = 'group' in options[index]; + + let itemHeight = MENU_OPTION_HEIGHT; + if (hasDescription) { + itemHeight = MENU_OPTION_HEIGHT_DESCRIPTION; + } + if (firstGroupItem && hasGroup) { + itemHeight += MENU_OPTION_HEIGHT; + } + return itemHeight; + }, + [options] + ); + + const rowVirtualizer = useVirtualizer({ + count: options.length, + getScrollElement: () => scrollRef.current, + estimateSize, + overscan: VIRTUAL_OVERSCAN_ITEMS, + }); + + const isOptionSelected = useCallback( + (item: ComboboxOption) => selectedItems.some((opt) => opt.value === item.value), + [selectedItems] + ); + + const allItemsSelected = enableAllOption && selectedItems.length === options.length - 1; + + return ( + +
+ {rowVirtualizer.getVirtualItems().map((virtualRow, index, allVirtualRows) => { + const item = options[virtualRow.index]; + const startingNewGroup = isNewGroup(item, options[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 = options[row.index]; + return rowItem.group === item.group; + }); + const groupHeaderItem = groupHeaderIndex && options[groupHeaderIndex.index]; + + const itemId = `combobox-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 ? `combobox-option-group-${groupHeaderItem.value}` : undefined; + + return ( + // Wrapping div should have no styling other than virtual list positioning. + // It's children (header and option) should appear as flat list items. +
+ {/* Group header */} + {startingNewGroup && ( + + )} + + {/* Option */} +
+ {isMultiSelect && ( +
+ 0 && !allItemsSelected} + aria-labelledby={itemId} + onClick={(e) => { + e.stopPropagation(); + }} + /> +
+ )} + +
+
{item.label ?? item.value}
+ + {item.description &&
{item.description}
} +
+
+
+ ); + })} +
+ +
+ {error && } + {options.length === 0 && !error && } +
+ + ); +}; diff --git a/packages/grafana-ui/src/components/Combobox/MultiCombobox.story.tsx b/packages/grafana-ui/src/components/Combobox/MultiCombobox.story.tsx index 86c7e94d2f0..6c99190c64a 100644 --- a/packages/grafana-ui/src/components/Combobox/MultiCombobox.story.tsx +++ b/packages/grafana-ui/src/components/Combobox/MultiCombobox.story.tsx @@ -25,11 +25,15 @@ const onChangeAction = action('onChange called'); const commonArgs = { options: [ - { 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' }, + { label: 'Australia', value: 'option1' }, + { label: 'Austria', value: 'option2' }, + { label: 'Fiji', value: 'option3' }, + { label: 'Iceland', value: 'option4' }, + { label: 'Ireland', value: 'option5' }, + { label: 'Finland', value: 'option6' }, + { label: 'The Netherlands', value: 'option7' }, + { label: 'Switzerland', value: 'option8' }, + { label: 'United Kingdom of Great Britain and Northern Ireland ', value: 'option9' }, ], value: ['option2'], placeholder: 'Select multiple options...', diff --git a/packages/grafana-ui/src/components/Combobox/MultiCombobox.tsx b/packages/grafana-ui/src/components/Combobox/MultiCombobox.tsx index 561cffbf2c2..56a24fd8eef 100644 --- a/packages/grafana-ui/src/components/Combobox/MultiCombobox.tsx +++ b/packages/grafana-ui/src/components/Combobox/MultiCombobox.tsx @@ -1,33 +1,27 @@ import { cx } from '@emotion/css'; -import { useVirtualizer } from '@tanstack/react-virtual'; import { useCombobox, useMultipleSelection } from 'downshift'; import { useCallback, useMemo, useState } from 'react'; import { useStyles2 } from '../../themes'; import { t } from '../../utils/i18n'; -import { Checkbox } from '../Forms/Checkbox'; import { Icon } from '../Icon/Icon'; import { Box } from '../Layout/Box/Box'; -import { Stack } from '../Layout/Stack/Stack'; import { Portal } from '../Portal/Portal'; -import { ScrollContainer } from '../ScrollContainer/ScrollContainer'; import { Text } from '../Text/Text'; import { Tooltip } from '../Tooltip'; -import { ComboboxBaseProps, AutoSizeConditionals, VIRTUAL_OVERSCAN_ITEMS } from './Combobox'; -import { NotFoundError } from './MessageRows'; -import { OptionListItem } from './OptionListItem'; +import { ComboboxBaseProps, AutoSizeConditionals } from './Combobox'; +import { ComboboxList } from './ComboboxList'; import { SuffixIcon } from './SuffixIcon'; import { ValuePill } from './ValuePill'; import { itemToString } from './filter'; -import { getComboboxStyles, MENU_OPTION_HEIGHT, MENU_OPTION_HEIGHT_DESCRIPTION } from './getComboboxStyles'; +import { getComboboxStyles } from './getComboboxStyles'; import { getMultiComboboxStyles } from './getMultiComboboxStyles'; import { ALL_OPTION_VALUE, ComboboxOption } from './types'; 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>; @@ -67,7 +61,12 @@ export const MultiCombobox = (props: MultiComboboxPro }, [inputValue]); // Handle async options and the 'All' option - const { options: baseOptions, updateOptions, asyncLoading } = useOptions(props.options, createCustomValue); + const { + options: baseOptions, + updateOptions, + asyncLoading, + asyncError, + } = 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; @@ -244,26 +243,6 @@ export const MultiCombobox = (props: MultiComboboxPro isClearable ); - const virtualizerOptions = { - count: options.length, - getScrollElement: () => scrollRef.current, - estimateSize: (index: number) => { - const firstGroupItem = isNewGroup(options[index], index > 0 ? options[index - 1] : undefined); - const hasDescription = 'description' in options[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); - // Selected items that show up in the input field const visibleItems = isOpen ? selectedItems.slice(0, MAX_SHOWN_ITEMS) : selectedItems.slice(0, shownItems); @@ -341,79 +320,23 @@ export const MultiCombobox = (props: MultiComboboxPro
{isOpen && ( - -
    - {rowVirtualizer.getVirtualItems().map((virtualRow) => { - const startingNewGroup = isNewGroup(options[virtualRow.index], options[virtualRow.index - 1]); - const index = virtualRow.index; - const item = options[index]; - 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 - // same length, but different, than the selected items (ask tobias) - const allItemsSelected = - options[0]?.value === ALL_OPTION_VALUE && selectedItems.length === options.length - 1; - - return ( -
  • - - {startingNewGroup && ( -
    - -
    - )} -
    - - 0 && !allItemsSelected} - aria-labelledby={id} - onClick={(e) => { - e.stopPropagation(); - }} - /> - - -
    -     -
    • - ); - })} -
-
{options.length === 0 && }
- + )}
diff --git a/packages/grafana-ui/src/components/Combobox/OptionListItem.tsx b/packages/grafana-ui/src/components/Combobox/OptionListItem.tsx deleted file mode 100644 index bf06e00d823..00000000000 --- a/packages/grafana-ui/src/components/Combobox/OptionListItem.tsx +++ /dev/null @@ -1,25 +0,0 @@ -import { cx } from '@emotion/css'; -import { ReactNode } from 'react'; - -import { useStyles2 } from '../../themes'; - -import { getComboboxStyles } from './getComboboxStyles'; - -interface Props { - label: ReactNode; - description?: ReactNode; - id: string; - isGroup?: boolean; -} - -export const OptionListItem = ({ label, description, id, isGroup = false }: Props) => { - const styles = useStyles2(getComboboxStyles); - return ( -
- - {label} - - {description && {description}} -
- ); -}; diff --git a/packages/grafana-ui/src/components/Combobox/getComboboxStyles.ts b/packages/grafana-ui/src/components/Combobox/getComboboxStyles.ts index 268e32cf940..382f53336fc 100644 --- a/packages/grafana-ui/src/components/Combobox/getComboboxStyles.ts +++ b/packages/grafana-ui/src/components/Combobox/getComboboxStyles.ts @@ -5,13 +5,16 @@ import { GrafanaTheme2 } from '@grafana/data'; // We need a px font size to accurately measure the width of items. // This should be in sync with the body font size in the theme. export const MENU_ITEM_FONT_SIZE = 14; +export const MENU_ITEM_DESCRIPTION_FONT_SIZE = 12; export const MENU_ITEM_FONT_WEIGHT = 500; export const MENU_ITEM_PADDING = 8; +export const MENU_ITEM_GAP = 2; 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_PADDING * 2 + MENU_ITEM_FONT_SIZE * MENU_ITEM_LINE_HEIGHT; -export const MENU_OPTION_HEIGHT_DESCRIPTION = MENU_OPTION_HEIGHT + MENU_ITEM_LINE_HEIGHT * MENU_ITEM_FONT_SIZE; +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 POPOVER_MAX_HEIGHT = MENU_OPTION_HEIGHT * 8.5; export const getComboboxStyles = (theme: GrafanaTheme2) => { @@ -33,20 +36,24 @@ export const getComboboxStyles = (theme: GrafanaTheme2) => { listStyle: 'none', }), - // New class for on the virtual list item. Should be on the wrapper around the group header and option. + // The wrapper around the group header and option, not the option itself. + // Should not contain visual styling itself. listItem: css({ label: 'list-item', - width: '100%', position: 'absolute', + width: '100%', }), - // New class used in single combobox group headers - newOptionGroup: css({ + optionGroupHeader: css({ label: 'combobox-new-option-group', borderTop: `1px solid ${theme.colors.border.weak}`, }), - newOptionGroupLabel: css({ + optionFirstGroupHeader: css({ + borderTop: 'none', + }), + + optionGroupLabel: css({ textOverflow: 'ellipsis', overflow: 'hidden', letterSpacing: 0, @@ -56,26 +63,20 @@ export const getComboboxStyles = (theme: GrafanaTheme2) => { padding: MENU_ITEM_PADDING, }), - newOptionGroupNoBorder: css({ - borderTop: 'none', - }), - - optionBasic: css({ - label: 'combobox-option', - position: 'absolute', - display: 'flex', - alignItems: 'center', - flexDirection: 'row', - flexShrink: 0, - whiteSpace: 'nowrap', - width: '100%', - overflow: 'hidden', - }), option: css({ - padding: MENU_ITEM_PADDING, - cursor: 'pointer', - borderRadius: theme.shape.radius.default, + label: 'combobox-option', + position: 'relative', // for the selection gradient to grab to + display: 'flex', width: '100%', + gap: theme.spacing(1), + alignItems: 'center', + padding: MENU_ITEM_PADDING, + marginBottom: MENU_ITEM_GAP, + borderRadius: theme.shape.radius.default, + fontWeight: theme.typography.fontWeightMedium, + whiteSpace: 'nowrap', + overflow: 'hidden', + cursor: 'pointer', '&:hover': { background: theme.colors.action.hover, '@media (forced-colors: active), (prefers-contrast: more)': { @@ -84,45 +85,40 @@ export const getComboboxStyles = (theme: GrafanaTheme2) => { }, }), - /** @deprecated - only used in multicombobox, will refactor it */ - optionGroup: css({ - cursor: 'default', - padding: MENU_ITEM_PADDING, - borderTop: `1px solid ${theme.colors.border.weak}`, + optionAccessory: css({ + label: 'combobox-option-accessory', + height: MENU_ITEM_FONT_SIZE * MENU_ITEM_LINE_HEIGHT, // Ensure the accessory doesn't make the option too tall }), + optionBody: css({ label: 'combobox-option-body', display: 'flex', - fontWeight: theme.typography.fontWeightMedium, flexDirection: 'column', flexGrow: 1, overflow: 'hidden', }), + optionLabel: css({ label: 'combobox-option-label', - textOverflow: 'ellipsis', - 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, - fontSize: theme.typography.bodySmall.fontSize, - fontWeight: theme.typography.fontWeightLight, - }), - optionDescription: css({ - label: 'combobox-option-description', - fontWeight: theme.typography.fontWeightRegular, - fontSize: theme.typography.bodySmall.fontSize, - color: theme.colors.text.secondary, - lineHeight: MENU_ITEM_LINE_HEIGHT, + letterSpacing: 0, textOverflow: 'ellipsis', overflow: 'hidden', }), + + optionDescription: css({ + label: 'combobox-option-description', + color: theme.colors.text.secondary, + fontSize: MENU_ITEM_DESCRIPTION_FONT_SIZE, + fontWeight: theme.typography.fontWeightRegular, + lineHeight: MENU_ITEM_LINE_HEIGHT, + letterSpacing: 0, + textOverflow: 'ellipsis', + overflow: 'hidden', + }), + optionFocused: css({ label: 'combobox-option-focused', // top: 0, diff --git a/packages/grafana-ui/src/components/Combobox/useComboboxFloat.ts b/packages/grafana-ui/src/components/Combobox/useComboboxFloat.ts index f2d5ca7de20..5787e88b9b9 100644 --- a/packages/grafana-ui/src/components/Combobox/useComboboxFloat.ts +++ b/packages/grafana-ui/src/components/Combobox/useComboboxFloat.ts @@ -18,6 +18,8 @@ const WIDTH_CALCULATION_LIMIT_ITEMS = 100_000; // Clearance around the popover to prevent it from being too close to the edge of the viewport const POPOVER_PADDING = 16; +const SCROLL_CONTAINER_PADDING = 8; + export const useComboboxFloat = (items: Array>, isOpen: boolean) => { const inputRef = useRef(null); const floatingRef = useRef(null); @@ -70,7 +72,7 @@ export const useComboboxFloat = (items: Array>, const size = measureText(longestItem, MENU_ITEM_FONT_SIZE, MENU_ITEM_FONT_WEIGHT).width; - return size + MENU_ITEM_PADDING * 2 + scrollbarWidth; + return size + SCROLL_CONTAINER_PADDING + MENU_ITEM_PADDING * 2 + scrollbarWidth; }, [items, scrollbarWidth]); const floatStyles = { diff --git a/public/locales/en-US/grafana.json b/public/locales/en-US/grafana.json index f7c125251d1..cca64ed4d00 100644 --- a/public/locales/en-US/grafana.json +++ b/public/locales/en-US/grafana.json @@ -1135,9 +1135,6 @@ "custom-value": { "description": "Use custom value" }, - "group": { - "undefined": "No group" - }, "options": { "no-found": "No options found." }