From 64bb94cc627911ae2c453c5d8ae7378b58539b5c Mon Sep 17 00:00:00 2001 From: Josh Hunt Date: Fri, 11 Oct 2024 15:10:44 +0100 Subject: [PATCH] Combobox: Measure text of longest label for dropdown width (#93937) * Combobox: Measure text of longest label for dropdown width * remove commented out code * add story to compare to Select * move magic numbers to constants to reference, and calculate the scrollbar width * look at first 100,000 items --- .../components/Combobox/Combobox.story.tsx | 129 +++++++++++++++++- .../components/Combobox/getComboboxStyles.ts | 11 +- .../components/Combobox/useComboboxFloat.ts | 74 +++++----- 3 files changed, 172 insertions(+), 42 deletions(-) diff --git a/packages/grafana-ui/src/components/Combobox/Combobox.story.tsx b/packages/grafana-ui/src/components/Combobox/Combobox.story.tsx index 386e6fe56b9..9f81eb1fd72 100644 --- a/packages/grafana-ui/src/components/Combobox/Combobox.story.tsx +++ b/packages/grafana-ui/src/components/Combobox/Combobox.story.tsx @@ -3,8 +3,11 @@ import { Meta, StoryFn, StoryObj } from '@storybook/react'; import { Chance } from 'chance'; import React, { ComponentProps, useEffect, useState } from 'react'; +import { useTheme2 } from '../../themes/ThemeContext'; import { Alert } from '../Alert/Alert'; +import { Divider } from '../Divider/Divider'; import { Field } from '../Forms/Field'; +import { Select } from '../Select/Select'; import { Combobox, ComboboxOption } from './Combobox'; @@ -73,7 +76,6 @@ async function generateOptions(amount: number): Promise { return Array.from({ length: amount }, (_, index) => ({ label: chance.sentence({ words: index % 5 }), value: chance.guid(), - //description: chance.sentence(), })); } @@ -88,7 +90,6 @@ const ManyOptionsStory: StoryFn = ({ numberOfOptions, ...arg setIsLoading(false); setOptions(options); setValue(options[5].value); - console.log("I've set stuff"); }); }, 1000); }, [numberOfOptions]); @@ -107,6 +108,123 @@ const ManyOptionsStory: StoryFn = ({ numberOfOptions, ...arg ); }; +const SelectComparisonStory: StoryFn = (args) => { + const [comboboxValue, setComboboxValue] = useState(args.value); + const theme = useTheme2(); + + return ( +
+ + { + setComboboxValue(val?.value || null); + action('onChange')(val); + }} + /> + + + + { + setComboboxValue(val?.value || null); + action('onChange')(val); + }} + /> + + + + + + { + setComboboxValue(val?.value || null); + action('onChange')(val); + }} + /> + + + + { + setComboboxValue(val?.value || null); + action('onChange')(val); + }} + /> + +
+ ); +}; + export const AutoSize: StoryObj = { args: { width: 'auto', @@ -130,6 +248,13 @@ export const CustomValue: StoryObj = { }, }; +export const ComparisonToSelect: StoryObj = { + args: { + numberOfOptions: 100, + }, + render: SelectComparisonStory, +}; + export default meta; function InDevDecorator(Story: React.ElementType) { diff --git a/packages/grafana-ui/src/components/Combobox/getComboboxStyles.ts b/packages/grafana-ui/src/components/Combobox/getComboboxStyles.ts index d6de3c76dfd..e967c260f85 100644 --- a/packages/grafana-ui/src/components/Combobox/getComboboxStyles.ts +++ b/packages/grafana-ui/src/components/Combobox/getComboboxStyles.ts @@ -4,6 +4,12 @@ import { GrafanaTheme2 } from '@grafana/data'; const MAX_HEIGHT = 400; +// 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_FONT_WEIGHT = 500; +export const MENU_ITEM_PADDING_X = 8; + export const getComboboxStyles = (theme: GrafanaTheme2) => { return { menuClosed: css({ @@ -24,7 +30,7 @@ export const getComboboxStyles = (theme: GrafanaTheme2) => { }), option: css({ label: 'grafana-select-option', - padding: '8px', + padding: MENU_ITEM_PADDING_X, position: 'absolute', display: 'flex', alignItems: 'center', @@ -53,6 +59,9 @@ export const getComboboxStyles = (theme: GrafanaTheme2) => { label: 'grafana-select-option-label', textOverflow: 'ellipsis', overflow: 'hidden', + fontSize: MENU_ITEM_FONT_SIZE, + fontWeight: MENU_ITEM_FONT_WEIGHT, + letterSpacing: 0, // pr todo: text in grafana has a slightly different letter spacing, which causes measureText() to be ~5% off }), optionDescription: css({ label: 'grafana-select-option-description', diff --git a/packages/grafana-ui/src/components/Combobox/useComboboxFloat.ts b/packages/grafana-ui/src/components/Combobox/useComboboxFloat.ts index bdac3088f96..60916c13ce5 100644 --- a/packages/grafana-ui/src/components/Combobox/useComboboxFloat.ts +++ b/packages/grafana-ui/src/components/Combobox/useComboboxFloat.ts @@ -1,12 +1,13 @@ import { autoUpdate, flip, size, useFloating } from '@floating-ui/react'; -import { useEffect, useRef, useState } from 'react'; +import { useMemo, useRef, useState } from 'react'; + +import { measureText } from '../../utils'; import { ComboboxOption } from './Combobox'; +import { MENU_ITEM_FONT_SIZE, MENU_ITEM_FONT_WEIGHT, MENU_ITEM_PADDING_X } from './getComboboxStyles'; -// On every 100th index we will recalculate the width of the popover. -const INDEX_WIDTH_CALCULATION = 100; -// A multiplier guesstimate times the amount of characters. If any padding or image support etc. is added this will need to be updated. -const WIDTH_MULTIPLIER = 7.3; +// Only consider the first n items when calculating the width of the popover. +const WIDTH_CALCULATION_LIMIT_ITEMS = 100_000; /** * Used with Downshift to get the height of each item @@ -22,9 +23,10 @@ export const useComboboxFloat = ( ) => { const inputRef = useRef(null); const floatingRef = useRef(null); - const [popoverWidth, setPopoverWidth] = useState(undefined); const [popoverMaxWidth, setPopoverMaxWidth] = useState(undefined); + const scrollbarWidth = useMemo(() => getScrollbarWidth(), []); + // the order of middleware is important! const middleware = [ flip({ @@ -48,49 +50,43 @@ export const useComboboxFloat = ( whileElementsMounted: autoUpdate, }); - useEffect(() => { - if (range === null) { - return; - } - const startVisibleIndex = range?.startIndex; - const endVisibleIndex = range?.endIndex; + const longestItemWidth = useMemo(() => { + let longestItem = ''; + const itemsToLookAt = Math.min(items.length, WIDTH_CALCULATION_LIMIT_ITEMS); - if (typeof startVisibleIndex === 'undefined' || typeof endVisibleIndex === 'undefined') { - return; + for (let i = 0; i < itemsToLookAt; i++) { + const itemLabel = items[i].label; + longestItem = itemLabel.length > longestItem.length ? itemLabel : longestItem; } - // Scroll down and default case - if ( - startVisibleIndex === 0 || - (startVisibleIndex % INDEX_WIDTH_CALCULATION === 0 && startVisibleIndex >= INDEX_WIDTH_CALCULATION) - ) { - let maxLength = 0; - const calculationEnd = Math.min(items.length, endVisibleIndex + INDEX_WIDTH_CALCULATION); + const size = measureText(longestItem, MENU_ITEM_FONT_SIZE, MENU_ITEM_FONT_WEIGHT).width; - for (let i = startVisibleIndex; i < calculationEnd; i++) { - maxLength = Math.max(maxLength, items[i].label.length); - } - - setPopoverWidth(maxLength * WIDTH_MULTIPLIER); - } else if (endVisibleIndex % INDEX_WIDTH_CALCULATION === 0 && endVisibleIndex >= INDEX_WIDTH_CALCULATION) { - // Scroll up case - let maxLength = 0; - const calculationStart = Math.max(0, startVisibleIndex - INDEX_WIDTH_CALCULATION); - - for (let i = calculationStart; i < endVisibleIndex; i++) { - maxLength = Math.max(maxLength, items[i].label.length); - } - - setPopoverWidth(maxLength * WIDTH_MULTIPLIER); - } - }, [items, range, setPopoverWidth]); + return size + MENU_ITEM_PADDING_X * 2 + scrollbarWidth; + }, [items, scrollbarWidth]); const floatStyles = { ...floatingStyles, - width: popoverWidth, + width: longestItemWidth, maxWidth: popoverMaxWidth, minWidth: inputRef.current?.offsetWidth, }; return { inputRef, floatingRef, floatStyles }; }; + +// Creates a temporary div with a scrolling inner div to calculate the width of the scrollbar +function getScrollbarWidth(): number { + const outer = document.createElement('div'); + outer.style.visibility = 'hidden'; + outer.style.overflow = 'scroll'; + document.body.appendChild(outer); + + const inner = document.createElement('div'); + outer.appendChild(inner); + + const scrollbarWidth = outer.offsetWidth - inner.offsetWidth; + + outer.parentNode?.removeChild(outer); + + return scrollbarWidth; +}