Merge pull request #1892 from adevinta/combobox-disclosure

feat(combobox): combobox disclosure button
adevinta · Feb 15, 2024 · 220bb07 · 220bb07
2 parents e8d5b4f + 2ad52a2
commit 220bb07
Show file tree

Hide file tree

Showing 7 changed files with 255 additions and 159 deletions.
diff --git a/packages/components/combobox/src/Combobox.doc.mdx b/packages/components/combobox/src/Combobox.doc.mdx
@@ -40,6 +40,10 @@ import { Combobox } from '@spark-ui/combobox'
       description:
         'The typing area in which the user can type. The input behaviour will differ if `autoComplete` is used or not.',
     },
+    'Combobox.Disclosure': {
+      of: Combobox.Disclosure,
+      description: 'Optional visual button to open and close the combobox items list.',
+    },
     'Combobox.LeadingIcon': {
       of: Combobox.LeadingIcon,
       description: 'Prepend a decorative icon inside the input (to the left).',

diff --git a/packages/components/combobox/src/Combobox.stories.tsx b/packages/components/combobox/src/Combobox.stories.tsx
@@ -66,6 +66,7 @@ export const Default: StoryFn = _args => {
       <Combobox>
         <Combobox.Trigger>
           <Combobox.Input aria-label="Book" placeholder="Pick a book" />
+          <Combobox.Disclosure openedLabel="Close popup" closedLabel="Open popup" />
         </Combobox.Trigger>
 
         <Combobox.Popover>

diff --git a/packages/components/combobox/src/ComboboxDisclosure.tsx b/packages/components/combobox/src/ComboboxDisclosure.tsx
@@ -0,0 +1,57 @@
+/* eslint-disable complexity */
+import { Icon } from '@spark-ui/icon'
+import { IconButton } from '@spark-ui/icon-button'
+import { ArrowHorizontalDown } from '@spark-ui/icons/dist/icons/ArrowHorizontalDown'
+import { useMergeRefs } from '@spark-ui/use-merge-refs'
+import { ComponentProps, forwardRef, type Ref } from 'react'
+
+import { useComboboxContext } from './ComboboxContext'
+
+interface DisclosureProps extends Omit<ComponentProps<typeof IconButton>, 'aria-label'> {
+  className?: string
+  closedLabel: string
+  openedLabel: string
+}
+
+export const Disclosure = forwardRef(
+  (
+    {
+      className,
+      closedLabel,
+      openedLabel,
+      intent = 'neutral',
+      design = 'ghost',
+      size = 'sm',
+      ...props
+    }: DisclosureProps,
+    forwardedRef: Ref<HTMLButtonElement>
+  ) => {
+    const { getToggleButtonProps } = useComboboxContext()
+
+    const { ref: downshiftRef, ...downshiftDisclosureProps } = getToggleButtonProps()
+    const isOpen = downshiftDisclosureProps['aria-expanded']
+
+    const ref = useMergeRefs(forwardedRef, downshiftRef)
+
+    return (
+      <IconButton
+        ref={ref}
+        className={className}
+        intent={intent}
+        design={design}
+        size={size}
+        {...downshiftDisclosureProps}
+        {...props}
+        aria-label={isOpen ? openedLabel : closedLabel}
+      >
+        <Icon>
+          <Icon className="shrink-0" size="sm">
+            <ArrowHorizontalDown />
+          </Icon>
+        </Icon>
+      </IconButton>
+    )
+  }
+)
+
+Disclosure.displayName = 'Combobox.Disclosure'
diff --git a/packages/components/combobox/src/ComboboxTrigger.tsx b/packages/components/combobox/src/ComboboxTrigger.tsx
@@ -1,6 +1,4 @@
 /* eslint-disable complexity */
-import { Icon } from '@spark-ui/icon'
-import { IconButton } from '@spark-ui/icon-button'
 import { ArrowHorizontalDown } from '@spark-ui/icons/dist/icons/ArrowHorizontalDown'
 import { Popover } from '@spark-ui/popover'
 import { forwardRef, Fragment, ReactNode, type Ref } from 'react'
@@ -16,7 +14,7 @@ interface TriggerProps {
 
 export const Trigger = forwardRef(
   ({ className, children }: TriggerProps, forwardedRef: Ref<HTMLDivElement>) => {
-    const { getToggleButtonProps, hasPopover, disabled, readOnly, state } = useComboboxContext()
+    const { hasPopover, disabled, readOnly, state } = useComboboxContext()
 
     const [PopoverAnchor, popoverAnchorProps] = hasPopover
       ? [Popover.Anchor, { asChild: true, type: undefined }]
@@ -38,21 +36,6 @@ export const Trigger = forwardRef(
 
             {/* 4 - Combobox clear button (optional) */}
             <p>[clear]</p>
-
-            {/* 5 - Combobox disclosure button (optional, advised for autoComplete not autoSuggest) */}
-            <IconButton
-              intent="neutral"
-              design="ghost"
-              size="sm"
-              {...getToggleButtonProps()}
-              aria-label="Show popup"
-            >
-              <Icon>
-                <Icon className="shrink-0" size="sm">
-                  <ArrowHorizontalDown />
-                </Icon>
-              </Icon>
-            </IconButton>
           </div>
         </PopoverAnchor>
       </>

diff --git a/packages/components/combobox/src/index.ts b/packages/components/combobox/src/index.ts
@@ -2,6 +2,7 @@ import type { FC } from 'react'
 
 import { Combobox as Root, type ComboboxProps } from './Combobox'
 import { ComboboxProvider, useComboboxContext } from './ComboboxContext'
+import { Disclosure } from './ComboboxDisclosure'
 import { Divider } from './ComboboxDivider'
 import { Empty } from './ComboboxEmpty'
 import { Group } from './ComboboxGroup'
@@ -30,6 +31,7 @@ export const Combobox: FC<ComboboxProps> & {
   LeadingIcon: typeof LeadingIcon
   Empty: typeof Empty
   Input: typeof Input
+  Disclosure: typeof Disclosure
 } = Object.assign(Root, {
   Group,
   Item,
@@ -43,6 +45,7 @@ export const Combobox: FC<ComboboxProps> & {
   LeadingIcon,
   Empty,
   Input,
+  Disclosure,
 })
 
 Combobox.displayName = 'Combobox'
@@ -58,3 +61,4 @@ Trigger.displayName = 'Combobox.Trigger'
 LeadingIcon.displayName = 'Combobox.LeadingIcon'
 Empty.displayName = 'Combobox.Empty'
 Input.displayName = 'Combobox.Input'
+Disclosure.displayName = 'Combobox.Disclosure'
diff --git a/packages/components/combobox/src/tests/Combobox.test.tsx b/packages/components/combobox/src/tests/Combobox.test.tsx
@@ -1,10 +1,9 @@
 import { render, screen } from '@testing-library/react'
 import userEvent from '@testing-library/user-event'
-import { useState } from 'react'
 import { describe, expect, it } from 'vitest'
 
 import { Combobox } from '..'
-import { getInput, getItem, getListbox, queryItem } from './test-utils'
+import { getInput, getItem, getListbox } from './test-utils'
 
 describe('Combobox', () => {
   it('should render input and list of items', () => {
@@ -36,7 +35,7 @@ describe('Combobox', () => {
     it('should open/close the popover when interacting with its input', async () => {
       const user = userEvent.setup()
 
-      // Given a close combobox (default state)
+      // Given a closed combobox (default state)
       render(
         <Combobox>
           <Combobox.Trigger>
@@ -72,7 +71,7 @@ describe('Combobox', () => {
     it('should open/close the items list when interacting with its input (no popover)', async () => {
       const user = userEvent.setup()
 
-      // Given a close combobox without a popover(default state)
+      // Given a closed combobox without a popover(default state)
       render(
         <Combobox>
           <Combobox.Trigger>
@@ -191,49 +190,16 @@ describe('Combobox', () => {
     })
   })
 
-  describe('single selection', () => {
-    it('should select item', async () => {
+  describe('Combobox.Disclosure', () => {
+    it('should open/close the popover when interacting with its disclosure button', async () => {
       const user = userEvent.setup()
 
-      // Given a combobox with no selected value yet
+      // Given a closed combobox (default state)
       render(
         <Combobox>
-          <Combobox.Trigger>
-            <Combobox.Input aria-label="Book" placeholder="Pick a book" />
-          </Combobox.Trigger>
-          <Combobox.Popover>
-            <Combobox.Items>
-              <Combobox.Item value="book-1">War and Peace</Combobox.Item>
-              <Combobox.Item value="book-2">1984</Combobox.Item>
-              <Combobox.Item value="book-3">Pride and Prejudice</Combobox.Item>
-            </Combobox.Items>
-          </Combobox.Popover>
-        </Combobox>
-      )
-
-      // Then placeholder should be displayed
-      expect(getInput('Book').getAttribute('placeholder')).toBe('Pick a book')
-      expect(getInput('Book').getAttribute('value')).toBe('')
-
-      // When the user select an item
-      await user.click(getInput('Book'))
-      await user.click(getItem('Pride and Prejudice'))
-
-      // Then placeholder is replaced by the selected value
-      expect(screen.getByDisplayValue('Pride and Prejudice')).toBeInTheDocument()
-
-      // Then the proper item is selected
-      expect(getItem('War and Peace')).toHaveAttribute('aria-selected', 'false')
-      expect(getItem('1984')).toHaveAttribute('aria-selected', 'false')
-      expect(getItem('Pride and Prejudice')).toHaveAttribute('aria-selected', 'true')
-    })
-
-    it('should render default selected option', () => {
-      // Given a combobox with a default selected value
-      render(
-        <Combobox defaultValue="book-2">
           <Combobox.Trigger>
             <Combobox.Input aria-label="Book" />
+            <Combobox.Disclosure openedLabel="Close popup" closedLabel="Open popup" />
           </Combobox.Trigger>
           <Combobox.Popover>
             <Combobox.Items>
@@ -245,111 +211,21 @@ describe('Combobox', () => {
         </Combobox>
       )
 
-      // Then the corresponding item is selected
-      expect(getItem('1984')).toHaveAttribute('aria-selected', 'true')
-    })
-
-    it('should render default selected option with proper indicator when including it', () => {
-      // Given a combobox with a default selected value
-      render(
-        <Combobox defaultValue="book-2">
-          <Combobox.Trigger>
-            <Combobox.Input aria-label="Book" />
-          </Combobox.Trigger>
-          <Combobox.Popover>
-            <Combobox.Items>
-              <Combobox.Item value="book-1">
-                <Combobox.ItemIndicator label={'selected'} />
-                <Combobox.ItemText>War and Peace</Combobox.ItemText>
-              </Combobox.Item>
-              <Combobox.Item value="book-2">
-                <Combobox.ItemIndicator label={'selected'} />
-                <Combobox.ItemText>1984</Combobox.ItemText>
-              </Combobox.Item>
-              <Combobox.Item value="book-3">
-                <Combobox.ItemIndicator label={'selected'} />
-                <Combobox.ItemText>Pride and Prejudice</Combobox.ItemText>
-              </Combobox.Item>
-            </Combobox.Items>
-          </Combobox.Popover>
-        </Combobox>
-      )
-
-      // Then the corresponding item is selected
-      expect(screen.getByLabelText('selected')).toBeVisible()
-    })
-
-    it('should control value', async () => {
-      const user = userEvent.setup()
-
-      // Given we control value by outside state and selected value
-      const ControlledImplementation = () => {
-        const [value, setValue] = useState('book-1')
-        const [inputValue, setInputValue] = useState('')
-
-        return (
-          <Combobox value={value} onValueChange={setValue}>
-            <Combobox.Trigger>
-              <Combobox.Input aria-label="Book" value={inputValue} onValueChange={setInputValue} />
-            </Combobox.Trigger>
-            <Combobox.Popover>
-              <Combobox.Items>
-                <Combobox.Item value="book-1">War and Peace</Combobox.Item>
-                <Combobox.Item value="book-2">1984</Combobox.Item>
-                <Combobox.Item value="book-3">Pride and Prejudice</Combobox.Item>
-              </Combobox.Items>
-            </Combobox.Popover>
-          </Combobox>
-        )
-      }
-
-      render(<ControlledImplementation />)
-
-      expect(getItem('War and Peace')).toHaveAttribute('aria-selected', 'true')
-
-      expect(screen.getByDisplayValue('')).toBeInTheDocument()
-
-      // when the user select another item
-      await user.click(getInput('Book'))
-      await user.click(getItem('Pride and Prejudice'))
-
-      // Then the selected value has been updated
-      expect(screen.getByDisplayValue('Pride and Prejudice')).toBeInTheDocument()
-    })
-
-    it('should select item using autoFilter (keyboard)', async () => {
-      const user = userEvent.setup()
+      const input = getInput('Book')
 
-      // Given a combobox with no selected value yet
-      render(
-        <Combobox autoFilter>
-          <Combobox.Trigger>
-            <Combobox.Input aria-label="Book" placeholder="Pick a book" />
-          </Combobox.Trigger>
-          <Combobox.Popover>
-            <Combobox.Items>
-              <Combobox.Item value="book-1">War and Peace</Combobox.Item>
-              <Combobox.Item value="book-2">1984</Combobox.Item>
-              <Combobox.Item value="book-3">Pride and Prejudice</Combobox.Item>
-            </Combobox.Items>
-          </Combobox.Popover>
-        </Combobox>
-      )
+      expect(input).toHaveAttribute('aria-expanded', 'false')
 
-      // Then placeholder should be displayed
-      expect(getInput('Book').getAttribute('placeholder')).toBe('Pick a book')
+      // When the user interact with the disclosure
+      await user.click(screen.getByLabelText('Open popup'))
 
-      // When the user type "pri" to filter first item matching, then select it
-      await user.click(getInput('Book'))
-      await user.keyboard('{p}{r}{i}{ArrowDown}{Enter}')
+      // Then the combobox has expanded
+      expect(input).toHaveAttribute('aria-expanded', 'true')
 
-      // Then placeholder is replaced by the selected value
-      expect(screen.getByDisplayValue('Pride and Prejudice')).toBeInTheDocument()
+      // When the user interact with the disclosure while input is expanded
+      await user.click(screen.getByLabelText('Close popup'))
 
-      // Then the proper item is selected
-      expect(queryItem('War and Peace')).not.toBeInTheDocument()
-      expect(queryItem('1984')).not.toBeInTheDocument()
-      expect(getItem('Pride and Prejudice')).toHaveAttribute('aria-selected', 'true')
+      // Then the combobox is closed again
+      expect(input).toHaveAttribute('aria-expanded', 'false')
     })
   })
 })