Merge pull request #853 from laboratoriobridge/823-combobox-treating-…

…select-issues

#823 [Combobox] - Treating select issues

src/components/Combobox/Combobox.stories.tsx

@@ -174,6 +174,7 @@ export const MultiSelect = () => {
       clearable={boolean('clearable', true)}
       disabled={boolean('disabled', false)}
       openOnFocus={boolean('openOnFocus', true)}
+      clearFilterOnSelect={boolean('clearFilterOnSelect', true)}
       open={select('open', [undefined, true, false], undefined)}
       loading={boolean('loading', false)}
       onChange={action('changed')}

src/components/Combobox/ComboboxMultiselect.test.tsx

@@ -544,8 +544,31 @@ describe('async loading', () => {
 })
 
 describe('filtering', () => {
-  it('should keep the current filter after an item is selected', async () => {
-    const { container } = render(<ComboboxTest />)
+  test.each`
+    clearFilterOnSelect
+    ${true}
+    ${false}
+  `(
+    'should clear the current filter and the input value after menu is closed (clearFilterOnSelect: $clearFilterOnSelect)',
+    async ({ clearFilterOnSelect }) => {
+      const { container } = render(<ComboboxTest clearFilterOnSelect={clearFilterOnSelect} />)
+      const input = container.querySelector('input')!
+
+      fireEvent.focus(input)
+      fireEvent.change(input, { target: { value: 'pe' } })
+
+      await waitFor(() => expect(container.querySelectorAll('li')).toHaveLength(4))
+
+      fireEvent.click(container.querySelectorAll('li')[0])
+      fireEvent.blur(input)
+
+      expect(container.querySelectorAll('li')).toHaveLength(0)
+      expect(input.value).toEqual('')
+    }
+  )
+
+  it('should clear the current filter and the input value after an item is selected if clearFilterOnSelect is true', async () => {
+    const { container } = render(<ComboboxTest clearFilterOnSelect={true} />)
     const input = container.querySelector('input')!
 
     fireEvent.focus(input)
@@ -555,20 +578,28 @@ describe('filtering', () => {
 
     fireEvent.click(container.querySelectorAll('li')[0])
 
-    expect(input.value).toEqual('pe')
-    expect(container.querySelectorAll('li')).toHaveLength(4)
+    await waitFor(() => expect(input.value).toEqual(''))
+    await waitFor(() => expect(container.querySelectorAll('li')).toHaveLength(fruits.length))
   })
-  it('should clear the current filter and the input value after menu is closed', async () => {
-    const { container } = render(<ComboboxTest />)
+
+  it('should not clear the current filter and the input value after an item is selected if clearFilterOnSelect is false', async () => {
+    const { container } = render(<ComboboxTest clearFilterOnSelect={false} />)
     const input = container.querySelector('input')!
 
     fireEvent.focus(input)
     fireEvent.change(input, { target: { value: 'pe' } })
+
+    await waitFor(() => expect(container.querySelectorAll('li')).toHaveLength(4))
+
     fireEvent.click(container.querySelectorAll('li')[0])
-    fireEvent.blur(input)
 
-    expect(container.querySelectorAll('li')).toHaveLength(0)
-    expect(input.value).toEqual('')
+    expect(container.querySelectorAll('li')).toHaveLength(4)
+    expect(input.value).toEqual('pe')
+
+    fireEvent.click(container.querySelectorAll('li')[1])
+
+    expect(container.querySelectorAll('li')).toHaveLength(4)
+    expect(input.value).toEqual('pe')
   })
 })
 

src/components/Combobox/ComboboxMultiselect.tsx

@@ -3,6 +3,7 @@ import matchSorter from 'match-sorter'
 import React, { CSSProperties, useCallback, useEffect, useMemo, useRef, useState } from 'react'
 import { usePopper } from 'react-popper'
 import { isNil } from 'lodash'
+import { useLocale } from '../../i18n'
 import { Theme, useStyles } from '../../styles'
 import { composeHandlers, composeRefs } from '../../util/react'
 import { FormControl } from '../FormControl'
@@ -22,6 +23,7 @@ export interface ComboboxMultiselectProps<T>
   onChange?: (newValue: T[]) => void
   itemIsEqual(a: T, b: T): boolean
   components?: Partial<ComboboxMultiselectComponents<T>>
+  clearFilterOnSelect?: boolean
 }
 
 export function ComboboxMultiselect<T = DefaultComboboxItemType>(props: ComboboxMultiselectProps<T>) {
@@ -41,6 +43,7 @@ export function ComboboxMultiselect<T = DefaultComboboxItemType>(props: Combobox
     itemToString,
     menuMinWidth,
     openOnFocus = true,
+    clearFilterOnSelect = false,
     onClear,
     onChange,
     onFocus,
@@ -58,6 +61,7 @@ export function ComboboxMultiselect<T = DefaultComboboxItemType>(props: Combobox
   } = props
 
   const [itemsLoaded, setItemsLoaded] = useState(false)
+  const locale = useLocale()
 
   const isAsync = typeof items === 'function'
   const getItems = useCallback((query: string) => (typeof items === 'function' ? items(query) : filter(items, query)), [
@@ -120,6 +124,7 @@ export function ComboboxMultiselect<T = DefaultComboboxItemType>(props: Combobox
     getItemProps,
     openMenu,
     setInputValue,
+    toggleMenu,
   } = useCombobox<T>({
     defaultHighlightedIndex: 0,
     selectedItem: null,
@@ -130,7 +135,7 @@ export function ComboboxMultiselect<T = DefaultComboboxItemType>(props: Combobox
     onInputValueChange: ({ inputValue }) => composeHandlers(loadItems, onFilterChange)(inputValue),
     onSelectedItemChange: ({ selectedItem }) => {
       isSelected(selectedItem) ? removeSelectedItem(selectedItem) : addSelectedItem(selectedItem)
-      setInputValue('')
+      clearFilterOnSelect && setInputValue('')
     },
     onIsOpenChange: ({ isOpen, inputValue }) => {
       isOpen && !itemsLoaded && loadItems(inputValue)
@@ -170,7 +175,10 @@ export function ComboboxMultiselect<T = DefaultComboboxItemType>(props: Combobox
 
   const handleWrapperClick = () => inputRef.current.focus()
   const handleItemClick = useCallback(
-    (item: T) => (isSelected(item) ? removeSelectedItem(item) : addSelectedItem(item)),
+    (item: T) => {
+      isSelected(item) ? removeSelectedItem(item) : addSelectedItem(item)
+      clearFilterOnSelect && setInputValue('')
+    },
     [isSelected, removeSelectedItem, addSelectedItem]
   )
   const handleItemRemove = useCallback(
@@ -190,6 +198,11 @@ export function ComboboxMultiselect<T = DefaultComboboxItemType>(props: Combobox
     [components]
   )
 
+  const handleIconClick = useCallback(() => {
+    toggleMenu()
+    inputRef.current?.focus()
+  }, [toggleMenu])
+
   return (
     <div {...downshiftComboboxProps}>
       <FormControl {...formControlProps} labelId={internalLabelId} {...downshiftLabelProps}>
@@ -199,10 +212,13 @@ export function ComboboxMultiselect<T = DefaultComboboxItemType>(props: Combobox
           onClick={handleWrapperClick}
           clearVisible={clearable && !!selectedItems.length}
           onClear={composeHandlers(reset, onClear)}
+          icon={isOpen ? 'angleUp' : 'angleDown'}
+          iconAriaLabel={isOpen ? locale.combobox.hideOptions : locale.combobox.showOptions}
+          iconPosition='right'
+          onIconClick={handleIconClick}
         >
           {selectedItems.map((selectedItem, index) => (
             <SelectedItem
-              style={classes.selectedItem}
               key={`selected-item-${index}`}
               onRemove={handleItemRemove(selectedItem)}
               disabled={disabled}
@@ -264,30 +280,35 @@ function comboboxMultiselectStateReducer<T>(
     case useCombobox.stateChangeTypes.InputBlur:
       return {
         ...changes,
-        ...(!changes.selectedItem && {
-          inputValue: '',
-        }),
+        inputValue: '',
       }
     default:
       return changes
   }
 }
 
-export const createStyles = (theme: Theme, { disabled }: ComboboxMultiselectProps<any>, hasSelectedItems: boolean) => {
+export const createStyles = (
+  theme: Theme,
+  { disabled, clearable }: ComboboxMultiselectProps<any>,
+  hasSelectedItems: boolean
+) => {
   const parts = createStyleParts(theme)
   return {
     wrapper: {
       ...parts.base,
       cursor: 'text',
 
       display: 'flex',
+      gap: '0.25rem',
       flexWrap: 'wrap',
       alignItems: 'center',
 
       padding: hasSelectedItems ? 'calc(0.25rem - 1px) 0.25rem' : 'calc(0.5rem - 1px) 0.5rem',
       '&:hover': !disabled && parts.hover,
       '&:active': !disabled && parts.active,
       '&:focus-within': !disabled && parts.focus,
+
+      paddingRight: clearable && hasSelectedItems ? '5rem' : '3rem',
     } as CSSProperties,
 
     disabled: parts.disabled,
@@ -297,22 +318,25 @@ export const createStyles = (theme: Theme, { disabled }: ComboboxMultiselectProp
       '&:focus-within': parts.invalid[':not(:disabled):focus'],
     } as CSSProperties,
 
-    selectedItem: {
-      marginRight: '0.25rem',
-    } as CSSProperties,
-
     input: {
       fontFamily: theme.typography.fontFamily,
       fontSize: theme.typography.sizes.text,
       color: theme.pallete.text.main,
       lineHeight: '1rem',
       background: theme.pallete.surface.main,
       padding: 0,
+      paddingRight: '0 !important',
+      minWidth: '5rem',
       flex: 1,
       border: 0,
       outline: 0,
       '::placeholder': parts.placeholder,
       ':disabled': parts.disabled,
+      '~ span': {
+        top: 0,
+        right: 0,
+        bottom: 0,
+      },
     } as CSSProperties,
 
     menu: {

src/components/Combobox/ListBox.tsx

@@ -29,15 +29,16 @@ function ListBoxInner<T>(props: ListBoxProps<T>, ref: Ref<HTMLDivElement>) {
     createNewItem,
     isItemSelected,
     onItemClick,
+    tabIndex,
     ...rest
   } = props
   const { classes } = useStyles(createStyles)
 
   const { CreateItem, AppendItem, EmptyItem, Item, LoadingItem, PrependItem } = components
 
   return (
-    <div ref={ref} {...rest}>
-      <ul className={classes.list}>
+    <div ref={ref} tabIndex={tabIndex} {...rest}>
+      <ul className={classes.list} tabIndex={tabIndex}>
         {PrependItem && <PrependItem />}
         {loading && <LoadingItem />}
         {!loading && createNewItem && !items?.length && <CreateItem />}