Combobox: Support undefined, null value and improve typing (#96523)

* Support undefined value

* Check truthiness of value instead

* check falsy

* Conditional typing for clearing value

* Less restrictive default typing

* simplify props

* Add tests for autosizing

* Write failing test case

* Add list of falsy values

* Check if nullish

* Check nullish in itemToString

* Nvm, it doesn't matter here

* Add support for autoFocus

* Pick from InputProps

* Move docstring

* Solve type issues in Storybook

* Fix failing story

packages/grafana-ui/src/components/Combobox/Combobox.story.tsx

@@ -4,16 +4,16 @@ import React, { ComponentProps, useCallback, useEffect, useState } from 'react';
 
 import { SelectableValue } from '@grafana/data';
 
-import { useTheme2 } from '../../themes/ThemeContext';
 import { Alert } from '../Alert/Alert';
-import { Divider } from '../Divider/Divider';
 import { Field } from '../Forms/Field';
-import { AsyncSelect, Select } from '../Select/Select';
+import { AsyncSelect } from '../Select/Select';
 
 import { Combobox, ComboboxOption } from './Combobox';
 import mdx from './Combobox.mdx';
 
-type PropsAndCustomArgs = ComponentProps<typeof Combobox> & { numberOfOptions: number };
+type PropsAndCustomArgs<T extends string | number = string> = ComponentProps<typeof Combobox<T>> & {
+  numberOfOptions: number;
+};
 
 const meta: Meta<PropsAndCustomArgs> = {
   title: 'Forms/Combobox',
@@ -27,6 +27,7 @@ const meta: Meta<PropsAndCustomArgs> = {
     loading: undefined,
     invalid: undefined,
     width: undefined,
+    isClearable: false,
     placeholder: 'Select an option...',
     options: [
       { label: 'Apple', value: 'apple' },
@@ -45,12 +46,6 @@ const meta: Meta<PropsAndCustomArgs> = {
       { label: 'Honeydew', value: 'honeydew' },
       { label: 'Iceberg Lettuce', value: 'iceberg-lettuce' },
       { label: 'Jackfruit', value: 'jackfruit' },
-      { label: '1', value: 1 },
-      { label: '2', value: 2 },
-      { label: '3', value: 3 },
-      { label: '4', value: 4 },
-      { label: '5', value: 5 },
-      { label: '6', value: 6 },
     ],
     value: 'banana',
   },
@@ -59,16 +54,16 @@ const meta: Meta<PropsAndCustomArgs> = {
   decorators: [InDevDecorator],
 };
 
-const BasicWithState: StoryFn<typeof Combobox> = (args) => {
+const BasicWithState: StoryFn<PropsAndCustomArgs> = (args) => {
-  const [value, setValue] = useState(args.value);
+  const [value, setValue] = useState<string | null>();
-
   return (
     <Field label="Test input" description="Input with a few options">
       <Combobox
         id="test-combobox"
         {...args}
         value={value}
-        onChange={(val) => {
+        onChange={(val: ComboboxOption | null) => {
+          // TODO: Figure out how to update value on args
           setValue(val?.value || null);
           action('onChange')(val);
         }}
@@ -88,7 +83,7 @@ async function generateOptions(amount: number): Promise<ComboboxOption[]> {
   }));
 }
 
-const ManyOptionsStory: StoryFn<PropsAndCustomArgs> = ({ numberOfOptions, ...args }) => {
+const ManyOptionsStory: StoryFn<PropsAndCustomArgs<string>> = ({ numberOfOptions, ...args }) => {
   const [value, setValue] = useState<string | null>(null);
   const [options, setOptions] = useState<ComboboxOption[]>([]);
   const [isLoading, setIsLoading] = useState(true);
@@ -103,13 +98,14 @@ const ManyOptionsStory: StoryFn<PropsAndCustomArgs> = ({ numberOfOptions, ...arg
     }, 1000);
   }, [numberOfOptions]);
 
+  const { onChange, ...rest } = args;
   return (
     <Combobox
-      {...args}
+      {...rest}
       loading={isLoading}
       options={options}
       value={value}
-      onChange={(opt) => {
+      onChange={(opt: ComboboxOption | null) => {
         setValue(opt?.value || null);
         action('onChange')(opt);
       }}
@@ -117,132 +113,6 @@ const ManyOptionsStory: StoryFn<PropsAndCustomArgs> = ({ numberOfOptions, ...arg
   );
 };
 
-const SelectComparisonStory: StoryFn<typeof Combobox> = (args) => {
-  const [comboboxValue, setComboboxValue] = useState(args.value);
-  const theme = useTheme2();
-
-  if (typeof args.options === 'function') {
-    throw new Error('This story does not support async options');
-  }
-
-  return (
-    <div style={{ border: '1px solid ' + theme.colors.border.weak, padding: 16 }}>
-      <Field label="Combobox with default size">
-        <Combobox
-          {...args}
-          id="combobox-default-size"
-          value={comboboxValue}
-          options={args.options}
-          onChange={(val) => {
-            setComboboxValue(val?.value || null);
-            action('onChange')(val);
-          }}
-        />
-      </Field>
-
-      <Field label="Select with default size">
-        <Select
-          id="select-default-size"
-          value={comboboxValue}
-          options={args.options}
-          onChange={(val) => {
-            setComboboxValue(val?.value || null);
-            action('onChange')(val);
-          }}
-        />
-      </Field>
-
-      <Divider />
-
-      <Field label="Combobox with explicit size (25)">
-        {/*@ts-ignore minWidth and maxWidth has never, which is incompatible with args. It lacks the context that width=25 on the component*/}
-        <Combobox
-          {...args}
-          id="combobox-explicit-size"
-          width={25}
-          value={comboboxValue}
-          options={args.options}
-          onChange={(val) => {
-            setComboboxValue(val?.value || null);
-            action('onChange')(val);
-          }}
-        />
-      </Field>
-
-      <Field label="Select with explicit size (25)">
-        <Select
-          id="select-explicit-size"
-          width={25}
-          value={comboboxValue}
-          options={args.options}
-          onChange={(val) => {
-            setComboboxValue(val?.value || null);
-            action('onChange')(val);
-          }}
-        />
-      </Field>
-
-      <Divider />
-
-      <Field label="Combobox with auto width, minWidth 15">
-        <Combobox
-          {...args}
-          id="combobox-auto-size"
-          width="auto"
-          minWidth={15}
-          value={comboboxValue}
-          options={args.options}
-          onChange={(val) => {
-            setComboboxValue(val?.value || null);
-            action('onChange')(val);
-          }}
-        />
-      </Field>
-
-      <Field label="Select with auto width">
-        <Select
-          id="select-auto-size"
-          width="auto"
-          value={comboboxValue}
-          options={args.options}
-          onChange={(val) => {
-            setComboboxValue(val?.value || null);
-            action('onChange')(val);
-          }}
-        />
-      </Field>
-
-      <Field label="Combobox with auto width, minWidth 15, empty value">
-        <Combobox
-          {...args}
-          id="combobox-auto-size-empty"
-          width="auto"
-          minWidth={15}
-          value={null}
-          options={args.options}
-          onChange={(val) => {
-            setComboboxValue(val?.value || null);
-            action('onChange')(val);
-          }}
-        />
-      </Field>
-
-      <Field label="Select with auto width, empty value">
-        <Select
-          id="select-auto-size-empty"
-          width="auto"
-          value={null}
-          options={args.options}
-          onChange={(val) => {
-            setComboboxValue(val?.value || null);
-            action('onChange')(val);
-          }}
-        />
-      </Field>
-    </div>
-  );
-};
-
 export const AutoSize: StoryObj<PropsAndCustomArgs> = {
   args: {
     width: 'auto',
@@ -294,6 +164,8 @@ const AsyncStory: StoryFn<PropsAndCustomArgs> = (args) => {
     }
   }, []);
 
+  const { onChange, ...rest } = args;
+
   return (
     <>
       <Field
@@ -301,12 +173,12 @@ const AsyncStory: StoryFn<PropsAndCustomArgs> = (args) => {
         description="This tests when options have both a label and a value. Consumers are required to pass in a full ComboboxOption as a value with a label"
       >
         <Combobox
-          {...args}
+          {...rest}
           id="test-combobox-one"
           placeholder="Select an option"
           options={loadOptionsWithLabels}
           value={selectedOption}
-          onChange={(val) => {
+          onChange={(val: ComboboxOption | null) => {
             action('onChange')(val);
             setSelectedOption(val);
           }}
@@ -324,7 +196,7 @@ const AsyncStory: StoryFn<PropsAndCustomArgs> = (args) => {
           placeholder="Select an option"
           options={loadOptionsOnlyValues}
           value={selectedOption?.value ?? null}
-          onChange={(val) => {
+          onChange={(val: ComboboxOption | null) => {
             action('onChange')(val);
             setSelectedOption(val);
           }}
@@ -366,7 +238,7 @@ const AsyncStory: StoryFn<PropsAndCustomArgs> = (args) => {
           placeholder="Select an option"
           options={loadOptionsWithErrors}
           value={selectedOption}
-          onChange={(val) => {
+          onChange={(val: ComboboxOption | null) => {
             action('onChange')(val);
             setSelectedOption(val);
           }}
@@ -426,13 +298,6 @@ export const PositioningTest: StoryObj<PropsAndCustomArgs> = {
   render: PositioningTestStory,
 };
 
-export const ComparisonToSelect: StoryObj<PropsAndCustomArgs> = {
-  args: {
-    numberOfOptions: 100,
-  },
-  render: SelectComparisonStory,
-};
-
 export default meta;
 
 function InDevDecorator(Story: React.ElementType) {

packages/grafana-ui/src/components/Combobox/Combobox.test.tsx

@@ -1,5 +1,6 @@
 import { act, render, screen, fireEvent } from '@testing-library/react';
 import userEvent from '@testing-library/user-event';
+import React from 'react';
 
 import { Combobox, ComboboxOption } from './Combobox';
 
@@ -49,7 +50,7 @@ describe('Combobox', () => {
     expect(onChangeHandler).toHaveBeenCalledWith(options[0]);
   });
 
-  it("shows the placeholder with the menu open when there's no value", async () => {
+  it('shows the placeholder with the menu open when value is null', async () => {
     render(<Combobox options={options} value={null} onChange={onChangeHandler} placeholder="Select an option" />);
 
     const input = screen.getByRole('combobox');
@@ -58,6 +59,15 @@ describe('Combobox', () => {
     expect(input).toHaveAttribute('placeholder', 'Select an option');
   });
 
+  it('shows the placeholder with the menu open when value is undefined', async () => {
+    render(<Combobox options={options} value={undefined} onChange={onChangeHandler} placeholder="Select an option" />);
+
+    const input = screen.getByRole('combobox');
+    await userEvent.click(input);
+
+    expect(input).toHaveAttribute('placeholder', 'Select an option');
+  });
+
   it('selects value by clicking that needs scrolling', async () => {
     render(<Combobox options={options} value={null} onChange={onChangeHandler} />);
 
@@ -106,6 +116,73 @@ describe('Combobox', () => {
     expect(screen.queryByDisplayValue('Option 2')).not.toBeInTheDocument();
   });
 
+  it.each(['very valid value', '', 0])('should handle an option with %p as a value', async (val) => {
+    const options = [
+      { label: 'Second option', value: '2' },
+      { label: 'Default', value: val },
+    ];
+
+    const ControlledCombobox = () => {
+      const [value, setValue] = React.useState<string | number | null>(null);
+
+      return (
+        <Combobox
+          options={options}
+          value={value}
+          onChange={(opt) => {
+            setValue(opt.value);
+          }}
+        />
+      );
+    };
+
+    render(<ControlledCombobox />);
+
+    const input = screen.getByRole('combobox');
+    await userEvent.click(input);
+    await userEvent.click(screen.getByRole('option', { name: 'Default' }));
+    expect(screen.queryByDisplayValue('Default')).toBeInTheDocument();
+
+    await userEvent.click(input);
+
+    expect(screen.getByRole('option', { name: 'Default' })).toHaveAttribute('aria-selected', 'true');
+  });
+
+  describe('size support', () => {
+    it('should require minWidth to be set with auto width', () => {
+      // @ts-expect-error
+      render(<Combobox options={options} value={null} onChange={onChangeHandler} width="auto" />);
+    });
+
+    it('should change width when typing things with auto width', async () => {
+      render(<Combobox options={options} value={null} onChange={onChangeHandler} width="auto" minWidth={2} />);
+
+      const input = screen.getByRole('combobox');
+      const inputWrapper = screen.getByTestId('input-wrapper');
+      const initialWidth = getComputedStyle(inputWrapper).width;
+
+      fireEvent.change(input, { target: { value: 'very very long value' } });
+
+      const newWidth = getComputedStyle(inputWrapper).width;
+
+      expect(initialWidth).not.toBe(newWidth);
+    });
+
+    it('should not change width when typing things with fixed width', async () => {
+      render(<Combobox options={options} value={null} onChange={onChangeHandler} width={2} />);
+      const input = screen.getByRole('combobox');
+
+      const inputWrapper = screen.getByTestId('input-wrapper');
+      const initialWidth = getComputedStyle(inputWrapper).width;
+
+      fireEvent.change(input, { target: { value: 'very very long value' } });
+
+      const newWidth = getComputedStyle(inputWrapper).width;
+
+      expect(initialWidth).toBe(newWidth);
+    });
+  });
+
   describe('with a value already selected', () => {
     it('shows an empty text input when opening the menu', async () => {
       const selectedValue = options[0].value;

packages/grafana-ui/src/components/Combobox/Combobox.tsx

@@ -28,7 +28,10 @@ export type ComboboxOption<T extends string | number = string> = {
 // 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.
 interface ComboboxBaseProps<T extends string | number>
-  extends Omit<InputProps, 'prefix' | 'suffix' | 'value' | 'addonBefore' | 'addonAfter' | 'onChange' | 'width'> {
+  extends Pick<
+    InputProps,
+    'placeholder' | 'autoFocus' | 'id' | 'aria-labelledby' | 'disabled' | 'loading' | 'invalid'
+  > {
   /**
    * An `X` appears in the UI, which clears the input and sets the value to `null`. Do not use if you have no `null` case.
    */
@@ -38,20 +41,31 @@ interface ComboboxBaseProps<T extends string | number>
    */
   createCustomValue?: boolean;
   options: Array<ComboboxOption<T>> | ((inputValue: string) => Promise<Array<ComboboxOption<T>>>);
-  onChange: (option: ComboboxOption<T> | null) => void;
+  onChange: (option: ComboboxOption<T>) => void;
   /**
    * Most consumers should pass value in as a scalar string | number. However, sometimes with Async because we don't
    * have the full options loaded to match the value to, consumers may also pass in an Option with a label to display.
    */
-  value: T | ComboboxOption<T> | null;
+  value?: T | ComboboxOption<T> | null;
   /**
    * Defaults to 100%. Number is a multiple of 8px. 'auto' will size the input to the content.
    * */
   width?: number | 'auto';
+  onBlur?: () => void;
 }
 
 const RECOMMENDED_ITEMS_AMOUNT = 100_000;
 
+type ClearableConditionals<T extends number | string> =
+  | {
+      isClearable: true;
+      /**
+       * The onChange handler is called with `null` when clearing the Combobox.
+       */
+      onChange: (option: ComboboxOption<T> | null) => void;
+    }
+  | { isClearable?: false; onChange: (option: ComboboxOption<T>) => void };
+
 type AutoSizeConditionals =
   | {
       width: 'auto';
@@ -70,13 +84,16 @@ type AutoSizeConditionals =
       maxWidth?: never;
     };
 
-type ComboboxProps<T extends string | number> = ComboboxBaseProps<T> & AutoSizeConditionals;
+type ComboboxProps<T extends string | number> = ComboboxBaseProps<T> & AutoSizeConditionals & ClearableConditionals<T>;
 
-function itemToString<T extends string | number>(item: ComboboxOption<T> | null) {
+function itemToString<T extends string | number>(item?: ComboboxOption<T> | null) {
-  if (item?.label?.includes('Custom value: ')) {
+  if (!item) {
-    return item?.value.toString();
+    return '';
   }
-  return item?.label ?? item?.value.toString() ?? '';
+  if (item.label?.includes('Custom value: ')) {
+    return item.value.toString();
+  }
+  return item.label ?? item.value.toString();
 }
 
 function itemFilter<T extends string | number>(inputValue: string) {
@@ -85,8 +102,8 @@ function itemFilter<T extends string | number>(inputValue: string) {
   return (item: ComboboxOption<T>) => {
     return (
       !inputValue ||
-      item?.label?.toLowerCase().includes(lowerCasedInputValue) ||
+      item.label?.toLowerCase().includes(lowerCasedInputValue) ||
-      item?.value?.toString().toLowerCase().includes(lowerCasedInputValue)
+      item.value?.toString().toLowerCase().includes(lowerCasedInputValue)
     );
   };
 }
@@ -109,8 +126,14 @@ export const Combobox = <T extends string | number>(props: ComboboxProps<T>) =>
     createCustomValue = false,
     id,
     width,
+    minWidth,
+    maxWidth,
     'aria-labelledby': ariaLabelledBy,
-    ...restProps
+    autoFocus,
+    onBlur,
+    disabled,
+    loading,
+    invalid,
   } = props;
 
   // Value can be an actual scalar Value (string or number), or an Option (value + label), so
@@ -158,7 +181,7 @@ export const Combobox = <T extends string | number>(props: ComboboxProps<T>) =>
       return null;
     }
 
-    if (value === null) {
+    if (valueProp === undefined || valueProp === null) {
       return null;
     }
 
@@ -168,9 +191,13 @@ export const Combobox = <T extends string | number>(props: ComboboxProps<T>) =>
     }
 
     return index;
-  }, [options, value, isAsync]);
+  }, [valueProp, options, value, isAsync]);
 
   const selectedItem = useMemo(() => {
+    if (valueProp === undefined || valueProp === null) {
+      return null;
+    }
+
     if (selectedItemIndex !== null && !isAsync) {
       return options[selectedItemIndex];
     }
@@ -329,7 +356,9 @@ export const Combobox = <T extends string | number>(props: ComboboxProps<T>) =>
 
   const { inputRef, floatingRef, floatStyles, scrollRef } = useComboboxFloat(items, rowVirtualizer.range, isOpen);
 
-  const InputComponent = width === 'auto' ? AutoSizeInput : Input;
+  const isAutoSize = width === 'auto';
+
+  const InputComponent = isAutoSize ? AutoSizeInput : Input;
 
   const suffixIcon = asyncLoading
     ? 'spinner'
@@ -343,7 +372,13 @@ export const Combobox = <T extends string | number>(props: ComboboxProps<T>) =>
   return (
     <div>
       <InputComponent
-        width={width === 'auto' ? undefined : width}
+        width={isAutoSize ? undefined : width}
+        {...(isAutoSize ? { minWidth, maxWidth } : {})}
+        autoFocus={autoFocus}
+        onBlur={onBlur}
+        disabled={disabled}
+        loading={loading}
+        invalid={invalid}
         className={styles.input}
         suffix={
           <>
@@ -368,7 +403,6 @@ export const Combobox = <T extends string | number>(props: ComboboxProps<T>) =>
             <Icon name={suffixIcon} />
           </>
         }
-        {...restProps}
         {...getInputProps({
           ref: inputRef,
           /*  Empty onCall to avoid TS error

public/app/core/components/SharedPreferences/SharedPreferences.tsx

@@ -111,10 +111,7 @@ export class SharedPreferences extends PureComponent<Props, State> {
     }
   };
 
-  onThemeChanged = (value: ComboboxOption<string> | null) => {
+  onThemeChanged = (value: ComboboxOption<string>) => {
-    if (!value) {
-      return;
-    }
     this.setState({ theme: value.value });
 
     if (value.value) {