From 6a8755a8af3f902d342d7f8aeb3b621e98755c25 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Laura=20Fern=C3=A1ndez?= Date: Thu, 14 Nov 2024 13:24:02 +0100 Subject: [PATCH] New Select: decrease item height (#95952) --- .../src/components/Combobox/Combobox.test.tsx | 4 ++-- .../src/components/Combobox/Combobox.tsx | 16 ++++++++++++---- .../src/components/Combobox/getComboboxStyles.ts | 13 +++++++++---- .../src/components/Combobox/useComboboxFloat.ts | 16 +++++++++------- public/locales/en-US/grafana.json | 2 +- public/locales/pseudo-LOCALE/grafana.json | 2 +- 6 files changed, 34 insertions(+), 19 deletions(-) diff --git a/packages/grafana-ui/src/components/Combobox/Combobox.test.tsx b/packages/grafana-ui/src/components/Combobox/Combobox.test.tsx index 1c3ac5766da..d45c2cc2402 100644 --- a/packages/grafana-ui/src/components/Combobox/Combobox.test.tsx +++ b/packages/grafana-ui/src/components/Combobox/Combobox.test.tsx @@ -166,7 +166,7 @@ describe('Combobox', () => { expect(onChangeHandler).toHaveBeenCalledWith(expect.objectContaining({ value: 'custom value' })); }); - it('should proivde custom string when all options are numbers', async () => { + it('should provide custom string when all options are numbers', async () => { const options = [ { label: '1', value: 1 }, { label: '2', value: 2 }, @@ -333,7 +333,7 @@ describe('Combobox', () => { jest.advanceTimersByTime(500); // Custom value while typing }); - const customItem = screen.queryByRole('option', { name: 'fir Create custom value' }); + const customItem = screen.queryByRole('option', { name: 'Custom value: fir' }); expect(customItem).toBeInTheDocument(); }); diff --git a/packages/grafana-ui/src/components/Combobox/Combobox.tsx b/packages/grafana-ui/src/components/Combobox/Combobox.tsx index a0086614259..6bc2d9bd9e7 100644 --- a/packages/grafana-ui/src/components/Combobox/Combobox.tsx +++ b/packages/grafana-ui/src/components/Combobox/Combobox.tsx @@ -13,8 +13,8 @@ import { Box } from '../Layout/Box/Box'; import { Stack } from '../Layout/Stack/Stack'; import { ScrollContainer } from '../ScrollContainer/ScrollContainer'; -import { getComboboxStyles } from './getComboboxStyles'; -import { useComboboxFloat, OPTION_HEIGHT } from './useComboboxFloat'; +import { getComboboxStyles, MENU_OPTION_HEIGHT } from './getComboboxStyles'; +import { useComboboxFloat } from './useComboboxFloat'; import { StaleResultError, useLatestAsyncCall } from './useLatestAsyncCall'; export type ComboboxOption = { @@ -69,6 +69,9 @@ type AutoSizeConditionals = type ComboboxProps = ComboboxBaseProps & AutoSizeConditionals; function itemToString(item: ComboboxOption | null) { + if (item?.label?.includes('Custom value: ')) { + return item?.value.toString(); + } return item?.label ?? item?.value.toString() ?? ''; } @@ -122,13 +125,18 @@ export const Combobox = (props: ComboboxProps) => let itemsToSet = items; if (inputValue && createCustomValue) { - const optionMatchingInput = items.find((opt) => opt.label === inputValue || opt.value === inputValue); + const optionMatchingInput = items.find( + (opt) => opt.label === 'Custom value: ' + inputValue || opt.value === inputValue + ); if (!optionMatchingInput) { const customValueOption = { + label: t('combobox.custom-value.label', 'Custom value: ') + inputValue, // Type casting needed to make this work when T is a number value: inputValue as unknown as T, + /* TODO: Add this back when we do support descriptions and have need for it description: t('combobox.custom-value.create', 'Create custom value'), + */ }; itemsToSet = items.slice(0); @@ -174,7 +182,7 @@ export const Combobox = (props: ComboboxProps) => const virtualizerOptions = { count: items.length, getScrollElement: () => scrollRef.current, - estimateSize: () => OPTION_HEIGHT, + estimateSize: () => MENU_OPTION_HEIGHT, overscan: 4, }; diff --git a/packages/grafana-ui/src/components/Combobox/getComboboxStyles.ts b/packages/grafana-ui/src/components/Combobox/getComboboxStyles.ts index c6926de6537..f5f685b2269 100644 --- a/packages/grafana-ui/src/components/Combobox/getComboboxStyles.ts +++ b/packages/grafana-ui/src/components/Combobox/getComboboxStyles.ts @@ -6,7 +6,12 @@ import { GrafanaTheme2 } from '@grafana/data'; // 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 MENU_ITEM_PADDING = 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_PADDING * 2 + MENU_ITEM_FONT_SIZE * MENU_ITEM_LINE_HEIGHT; +export const POPOVER_MAX_HEIGHT = MENU_OPTION_HEIGHT * 8.5; export const getComboboxStyles = (theme: GrafanaTheme2) => { return { @@ -27,7 +32,7 @@ export const getComboboxStyles = (theme: GrafanaTheme2) => { }), option: css({ label: 'grafana-select-option', - padding: MENU_ITEM_PADDING_X, + padding: MENU_ITEM_PADDING, position: 'absolute', display: 'flex', alignItems: 'center', @@ -65,7 +70,7 @@ export const getComboboxStyles = (theme: GrafanaTheme2) => { fontWeight: 'normal', fontSize: theme.typography.bodySmall.fontSize, color: theme.colors.text.secondary, - lineHeight: theme.typography.body.lineHeight, + lineHeight: MENU_ITEM_LINE_HEIGHT, textOverflow: 'ellipsis', overflow: 'hidden', }), @@ -84,7 +89,7 @@ export const getComboboxStyles = (theme: GrafanaTheme2) => { borderRadius: theme.shape.radius.default, content: '" "', display: 'block', - height: '100%', + height: MENU_OPTION_HEIGHT, position: 'absolute', width: theme.spacing(0.5), left: 0, diff --git a/packages/grafana-ui/src/components/Combobox/useComboboxFloat.ts b/packages/grafana-ui/src/components/Combobox/useComboboxFloat.ts index e2f7e3ac851..3a72680203f 100644 --- a/packages/grafana-ui/src/components/Combobox/useComboboxFloat.ts +++ b/packages/grafana-ui/src/components/Combobox/useComboboxFloat.ts @@ -4,15 +4,17 @@ 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'; +import { + MENU_ITEM_FONT_SIZE, + MENU_ITEM_FONT_WEIGHT, + MENU_ITEM_PADDING, + MENU_OPTION_HEIGHT, + POPOVER_MAX_HEIGHT, +} from './getComboboxStyles'; // 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 -export const OPTION_HEIGHT = 45; -const POPOVER_MAX_HEIGHT = OPTION_HEIGHT * 8.5; - // Clearance around the popover to prevent it from being too close to the edge of the viewport const POPOVER_PADDING = 16; @@ -41,7 +43,7 @@ export const useComboboxFloat = ( const preferredMaxHeight = availableHeight - POPOVER_PADDING; const width = Math.max(preferredMaxWidth, 0); - const height = Math.min(Math.max(preferredMaxHeight, OPTION_HEIGHT * 6), POPOVER_MAX_HEIGHT); + const height = Math.min(Math.max(preferredMaxHeight, MENU_OPTION_HEIGHT * 6), POPOVER_MAX_HEIGHT); setPopoverMaxSize({ width, height }); }, @@ -68,7 +70,7 @@ export const useComboboxFloat = ( const size = measureText(longestItem, MENU_ITEM_FONT_SIZE, MENU_ITEM_FONT_WEIGHT).width; - return size + MENU_ITEM_PADDING_X * 2 + scrollbarWidth; + return size + 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 528d8a31e79..76146131d30 100644 --- a/public/locales/en-US/grafana.json +++ b/public/locales/en-US/grafana.json @@ -455,7 +455,7 @@ "title": "Clear value" }, "custom-value": { - "create": "Create custom value" + "label": "Custom value: " }, "options": { "no-found": "No options found." diff --git a/public/locales/pseudo-LOCALE/grafana.json b/public/locales/pseudo-LOCALE/grafana.json index df94a7497f3..46da0d46687 100644 --- a/public/locales/pseudo-LOCALE/grafana.json +++ b/public/locales/pseudo-LOCALE/grafana.json @@ -455,7 +455,7 @@ "title": "Cľęäř väľūę" }, "custom-value": { - "create": "Cřęäŧę čūşŧőm väľūę" + "label": "Cūşŧőm väľūę: " }, "options": { "no-found": "Ńő őpŧįőʼnş ƒőūʼnđ."