Combobox: Fix Group Label Styling - Option Groups + Top Border (#102749)

* Combobox: Fix Group Label Styling

* unified piece of logic with existing code

* Update: Sort ungrouped options based on occurence / docs

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

@@ -35,6 +35,10 @@ Options can be an array of objects with seperate label and values, or an array o
 
 While Combobox can handle large sets of options, you should consider both the user experience of searching through many options, and the performance of loading many options from an API.
 
+### Grouping
+
+Each option object may also have a `group` string property. Options with matching group names will be sorted together, as will options with undefined groups. Combobox will order these based on the first occurence of each `group` (or absence of `group`) in the `options` prop.
+
 ### Async behaviour
 
 When using Combobox with options from a remote source as the user types, you can supply the `options` prop as an function that is called on each keypress with the current input value and returns a promise resolving to an array of options matching the input.
@@ -116,7 +120,6 @@ Some differences to note:
 - `isLoading: boolean` has been renamed to `loading: boolean`
 - `allowCustomValue` has been renamed to `createCustomValue`.
 - When specifying `width="auto"`, `minWidth` is also required.
-- Groups are not supported at this time.
 - Many props used to control subtle behaviour have been removed to simplify the API and improve performance.
   - Custom render props, or label as ReactNode is not supported at this time. Reach out if you have a hard requirement for this and we can discuss.
 

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

@@ -107,6 +107,26 @@ export const CustomValue: Story = {
   render: BaseCombobox,
 };
 
+export const GroupsWithMixedLabels: Story = {
+  args: {
+    options: [
+      { label: 'One', value: 'one', group: 'Group 1' },
+      { label: 'Two', value: 'two', group: 'Group 1' },
+      { label: 'Three', value: 'three', group: 'Group 3' },
+      { label: 'Four', value: 'four', group: 'Group 1' },
+      { label: 'Five', value: 'five' },
+      { label: 'Six', value: 'six' },
+      { label: 'Seven', value: 'seven', group: 'Group 2' },
+      { label: 'Eight', value: 'eight', group: 'Group 3' },
+      { label: 'Nine', value: 'nine', group: 'Group 3' },
+      { label: 'Ten', value: 'ten', group: 'Group 3' },
+      { label: 'Eleven', value: 'eleven' },
+    ],
+    value: '',
+  },
+  render: BaseCombobox,
+};
+
 export const Groups: Story = {
   args: {
     options: await generateGroupingOptions(500),

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

@@ -14,6 +14,14 @@ const options: ComboboxOption[] = [
   { label: 'Option 3', value: '3', description: 'This is option 3' },
   { label: 'Option 4', value: '4' },
 ];
+const optionsWithGroups: ComboboxOption[] = [
+  { label: 'Option 1', value: '1', group: 'Group 1' },
+  { label: 'Option 2', value: '2' },
+  { label: 'Option 3', value: '3', group: 'Group 1' },
+  { label: 'Option 4', value: '4' },
+  { label: 'Option 5', value: '5', group: 'Group 2' },
+  { label: 'Option 6', value: '6', group: 'Group 2' },
+];
 
 describe('Combobox', () => {
   const onChangeHandler = jest.fn();
@@ -192,6 +200,41 @@ describe('Combobox', () => {
         ['Group 1', 'Option 1', 'Option 3', 'Option 6', 'Group 2', 'Option 2', 'Option 4', 'Option 5'].join('')
       );
     });
+
+    it('puts ungrouped options relative to first occurrence', async () => {
+      render(<Combobox options={optionsWithGroups} value={null} onChange={onChangeHandler} />);
+
+      const input = screen.getByRole('combobox');
+      await userEvent.click(input);
+
+      const listbox = await screen.findByRole('listbox');
+      expect(listbox).toHaveTextContent(
+        ['Group 1', 'Option 1', 'Option 3', 'Option 2', 'Option 4', 'Group 2', 'Option 5', 'Option 6'].join('')
+      );
+    });
+
+    it('does not render group header labels for ungrouped options', async () => {
+      render(<Combobox options={optionsWithGroups} value={null} onChange={onChangeHandler} />);
+
+      const input = screen.getByRole('combobox');
+      await userEvent.click(input);
+
+      const allHeaders = await screen.findAllByRole('presentation');
+
+      expect(allHeaders[0]).toHaveTextContent('Group 1');
+      expect(allHeaders[1]).toHaveTextContent('');
+    });
+
+    it('does not render a top border for the first group header', async () => {
+      render(<Combobox options={optionsWithGroups} value={null} onChange={onChangeHandler} />);
+
+      const input = screen.getByRole('combobox');
+      await userEvent.click(input);
+
+      const allHeaders = await screen.findAllByRole('presentation');
+
+      expect(allHeaders[0]).toHaveStyle('border-top: none');
+    });
   });
 
   describe('size support', () => {

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

@@ -204,11 +204,13 @@ export const Combobox = <T extends string | number>(props: ComboboxProps<T>) =>
     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) {
+      if (firstGroupItem && hasGroup) {
         itemHeight += MENU_OPTION_HEIGHT;
       }
       return itemHeight;
@@ -402,7 +404,15 @@ export const Combobox = <T extends string | number>(props: ComboboxProps<T>) =>
                         }}
                       >
                         {startingNewGroup && (
-                          <div role="presentation" id={groupHeaderId} className={styles.newOptionGroup}>
+                          <div
+                            role="presentation"
+                            id={groupHeaderId}
+                            className={cx(
+                              styles.newOptionGroup,
+                              item.group && styles.newOptionGroupLabel,
+                              virtualRow.index === 0 && styles.newOptionGroupNoBorder
+                            )}
+                          >
                             {item.group}
                           </div>
                         )}

packages/grafana-ui/src/components/Combobox/getComboboxStyles.ts

@@ -43,6 +43,10 @@ export const getComboboxStyles = (theme: GrafanaTheme2) => {
     // New class used in single combobox group headers
     newOptionGroup: css({
       label: 'combobox-new-option-group',
+      borderTop: `1px solid ${theme.colors.border.weak}`,
+    }),
+
+    newOptionGroupLabel: css({
       textOverflow: 'ellipsis',
       overflow: 'hidden',
       letterSpacing: 0,
@@ -50,7 +54,10 @@ export const getComboboxStyles = (theme: GrafanaTheme2) => {
       fontSize: theme.typography.bodySmall.fontSize,
       fontWeight: theme.typography.fontWeightLight,
       padding: MENU_ITEM_PADDING,
-      borderTop: `1px solid ${theme.colors.border.weak}`,
+    }),
+
+    newOptionGroupNoBorder: css({
+      borderTop: 'none',
     }),
 
     optionBasic: css({

packages/grafana-ui/src/components/Combobox/useOptions.ts

@@ -150,20 +150,12 @@ export function sortByGroup<T extends string | number>(options: Array<ComboboxOp
 
   let currentIndex = 0;
 
-  // Fill result array with grouped options
+  // Fill result array with grouped and undefined grouped options
   for (const [group, groupOptions] of groupedOptions) {
     if (group) {
       groupStartIndices.set(group, currentIndex);
-      for (const option of groupOptions) {
-        result[currentIndex++] = option;
-      }
     }
-  }
-
-  // Add ungrouped options at the end
-  const ungrouped = groupedOptions.get(undefined);
-  if (ungrouped) {
-    for (const option of ungrouped) {
+    for (const option of groupOptions) {
       result[currentIndex++] = option;
     }
   }