Allow custom indicator on Combobox/Select (#983)

Author: mdodgelooker

CHANGELOG.md

@@ -13,6 +13,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - `Tree`, `TreeItem` components
   - Includes docs and test suite
 - `ActionListItem` accepts `actionsButtonLabel` prop to help with testing
+- `ComboboxList` and `ComboboxOption` now both support a custom `indicator`
+  - `Select` and `SelectMulti` also support `indicator` at both the component and option level
 
 ### Changed
 

packages/components/src/Form/Inputs/Combobox/Combobox.test.tsx

@@ -33,6 +33,7 @@ import {
   ComboboxInput,
   ComboboxList,
   ComboboxOption,
+  ComboboxOptionIndicatorFunction,
   ComboboxMulti,
   ComboboxMultiInput,
   ComboboxMultiList,
@@ -193,6 +194,97 @@ describe('<Combobox/> with children', () => {
     fireEvent.click(document)
   })
 
+  const getIndicatorJSX = (listLevel: boolean) => (
+    indicator: ComboboxOptionIndicatorFunction
+  ) => (
+    <Combobox key="combobox" value={{ label: 'Foo', value: '101' }}>
+      <ComboboxInput placeholder="Type here" />
+      <ComboboxList indicator={listLevel ? indicator : undefined}>
+        <ComboboxOption
+          label="Foo"
+          value="101"
+          indicator={listLevel ? undefined : indicator}
+        />
+        <ComboboxOption
+          label="Bar"
+          value="102"
+          indicator={listLevel ? undefined : indicator}
+        />
+      </ComboboxList>
+    </Combobox>
+  )
+
+  const getIndicatorJSXMulti = (listLevel: boolean) => (
+    indicator: ComboboxOptionIndicatorFunction
+  ) => (
+    <ComboboxMulti
+      key="combobox-multi"
+      values={[{ label: 'Foo', value: '101' }]}
+    >
+      <ComboboxMultiInput placeholder="Type here" />
+      <ComboboxMultiList indicator={listLevel ? indicator : undefined}>
+        <ComboboxMultiOption
+          label="Foo"
+          value="101"
+          indicator={listLevel ? undefined : indicator}
+        />
+        <ComboboxMultiOption
+          label="Bar"
+          value="102"
+          indicator={listLevel ? undefined : indicator}
+        />
+      </ComboboxMultiList>
+    </ComboboxMulti>
+  )
+
+  test.each([
+    ['list level (Combobox)', getIndicatorJSX(true)],
+    ['list level (ComboboxMulti)', getIndicatorJSXMulti(true)],
+    ['option level (Combobox)', getIndicatorJSX(false)],
+    ['option level (ComboboxMulti)', getIndicatorJSXMulti(false)],
+  ])('Customize the indicator at the %s', (_, getJSX) => {
+    const indicator = jest.fn()
+    const { queryByTitle, getByText, getByPlaceholderText } = renderWithTheme(
+      getJSX(indicator)
+    )
+
+    const input = getByPlaceholderText('Type here')
+    fireEvent.click(input)
+
+    expect(indicator).toHaveBeenCalledTimes(2)
+    expect(indicator).toHaveBeenNthCalledWith(1, {
+      isActive: false,
+      isSelected: true,
+      label: 'Foo',
+      value: '101',
+    })
+    expect(indicator).toHaveBeenNthCalledWith(2, {
+      isActive: false,
+      isSelected: false,
+      label: 'Bar',
+      value: '102',
+    })
+
+    const check = queryByTitle('Check')
+    expect(check).not.toBeInTheDocument()
+
+    indicator.mockClear()
+
+    const bar = getByText('Bar')
+    fireEvent.mouseOver(bar)
+
+    expect(indicator).toHaveBeenCalledTimes(1)
+    expect(indicator).toHaveBeenCalledWith({
+      isActive: true,
+      isSelected: false,
+      label: 'Bar',
+      value: '102',
+    })
+
+    // Close popover to silence act() warning
+    fireEvent.click(document)
+  })
+
   test('Does not highlight current selected value', () => {
     const { getByText, getByPlaceholderText } = renderWithTheme(
       <Combobox key="combobox" value={{ label: 'Foo', value: '101' }}>

packages/components/src/Form/Inputs/Combobox/ComboboxContext.ts

@@ -33,6 +33,7 @@ import {
   Dispatch,
   SetStateAction,
 } from 'react'
+import { ComboboxOptionIndicatorProps } from './ComboboxOptionIndicator'
 import {
   ComboboxData,
   ComboboxMultiData,
@@ -67,6 +68,7 @@ export interface ComboboxContextProps<
   readOnlyPropRef?: MutableRefObject<boolean>
   windowedOptionsPropRef?: MutableRefObject<boolean>
   isAutoScrollingRef?: MutableRefObject<boolean>
+  indicatorPropRef?: MutableRefObject<ComboboxOptionIndicatorProps['indicator']>
   isVisible?: boolean
   openOnFocus?: boolean
   listScrollPosition?: number

packages/components/src/Form/Inputs/Combobox/ComboboxList.tsx

@@ -49,13 +49,15 @@ import once from 'lodash/once'
 import throttle from 'lodash/throttle'
 import { useForkedRef } from '../../../utils'
 import { usePopover } from '../../../Popover'
+import { ComboboxOptionIndicatorProps } from './ComboboxOptionIndicator'
 import { ComboboxContext, ComboboxMultiContext } from './ComboboxContext'
 import { useBlur } from './utils/useBlur'
 import { useKeyDown } from './utils/useKeyDown'
 import { ComboboxActionType } from './utils/state'
 
 export interface ComboboxListProps
-  extends SpaceProps,
+  extends Pick<ComboboxOptionIndicatorProps, 'indicator'>,
+    SpaceProps,
     LayoutProps,
     TypographyProps,
     CompatibleHTMLProps<HTMLUListElement> {
@@ -100,6 +102,7 @@ const ComboboxListInternal = forwardRef(
       closeOnSelect = true,
       // disables the optionsRef behavior, to be handled externally (support keyboard nav in long lists)
       windowedOptions = false,
+      indicator,
       isMulti,
       ...props
     }: ComboboxListInternalProps,
@@ -112,6 +115,7 @@ const ComboboxListInternal = forwardRef(
       persistSelectionPropRef,
       closeOnSelectPropRef,
       windowedOptionsPropRef,
+      indicatorPropRef,
       transition,
       wrapperElement,
       isVisible,
@@ -126,6 +130,7 @@ const ComboboxListInternal = forwardRef(
       persistSelectionPropRef.current = persistSelection
     if (closeOnSelectPropRef) closeOnSelectPropRef.current = closeOnSelect
     if (windowedOptionsPropRef) windowedOptionsPropRef.current = windowedOptions
+    if (indicatorPropRef) indicatorPropRef.current = indicator
 
     // WEIRD? Reset the options ref every render so that they are always
     // accurate and ready for keyboard navigation handlers. Using layout

packages/components/src/Form/Inputs/Combobox/ComboboxMultiOption.tsx

@@ -37,12 +37,12 @@ import {
 } from './ComboboxContext'
 import {
   comboboxOptionDefaultProps,
-  ComboboxOptionDetail,
   ComboboxOptionProps,
   comboboxOptionStyle,
   ComboboxOptionWrapper,
   ComboboxOptionText,
 } from './ComboboxOption'
+import { ComboboxOptionIndicator } from './ComboboxOptionIndicator'
 import { useAddOptionToContext } from './utils/useAddOptionToContext'
 import { useOptionEvents } from './utils/useOptionEvents'
 import { useOptionStatus } from './utils/useOptionStatus'
@@ -52,6 +52,7 @@ const ComboboxMultiOptionInternal = forwardRef(
   (
     {
       children,
+      indicator,
       highlightText = true,
       scrollIntoView,
       hideCheckMark,
@@ -92,9 +93,14 @@ const ComboboxMultiOptionInternal = forwardRef(
         ref={ref}
         aria-selected={isActive}
       >
-        <ComboboxOptionDetail>
+        <ComboboxOptionIndicator
+          indicator={indicator}
+          isActive={isActive}
+          isSelected={isSelected}
+          isMulti={true}
+        >
           {!hideCheckMark && <Checkbox checked={isSelected} />}
-        </ComboboxOptionDetail>
+        </ComboboxOptionIndicator>
         {children || <ComboboxOptionText highlightText={highlightText} />}
       </ComboboxOptionWrapper>
     )
@@ -105,8 +111,6 @@ ComboboxMultiOptionInternal.displayName = 'ComboboxMultiOptionInternal'
 
 export const ComboboxMultiOption = styled(ComboboxMultiOptionInternal)`
   ${comboboxOptionStyle}
-  grid-template-columns: ${(props) =>
-    props.hideCheckMark ? 0 : props.theme.space.xlarge} 1fr;
 `
 
 ComboboxMultiOption.defaultProps = {

packages/components/src/Form/Inputs/Combobox/ComboboxOption.tsx

@@ -42,7 +42,7 @@ import {
   TypographyProps,
 } from '@looker/design-tokens'
 import omit from 'lodash/omit'
-import React, { forwardRef, useContext, Ref } from 'react'
+import React, { forwardRef, useContext, Ref, ReactNode } from 'react'
 import styled, { css } from 'styled-components'
 import { Icon } from '../../../Icon'
 import { ReplaceText, Text } from '../../../Text'
@@ -59,6 +59,10 @@ import { getComboboxText } from './utils/getComboboxText'
 import { useOptionEvents } from './utils/useOptionEvents'
 import { useOptionStatus } from './utils/useOptionStatus'
 import { useAddOptionToContext } from './utils/useAddOptionToContext'
+import {
+  ComboboxOptionIndicator,
+  ComboboxOptionIndicatorProps,
+} from './ComboboxOptionIndicator'
 import { useOptionScroll } from './utils/useOptionScroll'
 
 export interface ComboboxOptionObject {
@@ -86,6 +90,7 @@ export interface HighlightTextProps {
 
 export interface ComboboxOptionProps
   extends ComboboxOptionObject,
+    Pick<ComboboxOptionIndicatorProps, 'indicator'>,
     HighlightTextProps,
     ColorProps,
     FlexboxProps,
@@ -105,7 +110,7 @@ export interface ComboboxOptionProps
    *     🍎 <ComboboxOptionText />
    *   </ComboboxOption>
    */
-  children?: React.ReactNode
+  children?: ReactNode
 }
 
 export const ComboboxOptionWrapper = forwardRef(
@@ -136,6 +141,7 @@ const ComboboxOptionInternal = forwardRef(
   (
     {
       children,
+      indicator,
       highlightText = true,
       scrollIntoView,
       ...props
@@ -175,9 +181,13 @@ const ComboboxOptionInternal = forwardRef(
         ref={ref}
         aria-selected={isActive}
       >
-        <ComboboxOptionDetail>
+        <ComboboxOptionIndicator
+          indicator={indicator}
+          isActive={isActive}
+          isSelected={isSelected}
+        >
           {isSelected && <Icon name="Check" mr={0} />}
-        </ComboboxOptionDetail>
+        </ComboboxOptionIndicator>
         {children || <ComboboxOptionText highlightText={highlightText} />}
       </ComboboxOptionWrapper>
     )
@@ -186,19 +196,6 @@ const ComboboxOptionInternal = forwardRef(
 
 ComboboxOptionInternal.displayName = 'ComboboxOptionInternal'
 
-export const ComboboxOptionDetail = styled.div`
-  display: flex;
-  align-items: center;
-  justify-content: center;
-  height: ${(props) => props.theme.space.large};
-`
-
-export const comboboxOptionGrid = css`
-  display: grid;
-  grid-gap: ${(props) => props.theme.space.xxsmall};
-  grid-template-columns: ${(props) => props.theme.space.medium} 1fr;
-`
-
 export const comboboxOptionStyle = css`
   ${reset}
   ${color}
@@ -207,8 +204,7 @@ export const comboboxOptionStyle = css`
   ${space}
   ${typography}
   cursor: default;
-  align-items: flex-start;
-  ${comboboxOptionGrid}
+  align-items: stretch;
   outline: none;
 
   &[aria-selected='true'] {