From 3e8ba2b93bfa0d7902ee79115cc1bd201865fbd0 Mon Sep 17 00:00:00 2001
From: Suzanne Rozier <suz@truss.works>
Date: Wed, 13 Oct 2021 14:11:24 -0500
Subject: [PATCH] Minor updates to ComboBox markup

---
 .../forms/ComboBox/ComboBox.test.tsx          | 263 ++++++++++--------
 src/components/forms/ComboBox/ComboBox.tsx    |  18 +-
 2 files changed, 164 insertions(+), 117 deletions(-)

diff --git a/src/components/forms/ComboBox/ComboBox.test.tsx b/src/components/forms/ComboBox/ComboBox.test.tsx
index c5bbb0c327..05e5247953 100644
--- a/src/components/forms/ComboBox/ComboBox.test.tsx
+++ b/src/components/forms/ComboBox/ComboBox.test.tsx
@@ -1,5 +1,5 @@
 import React from 'react'
-import { render, fireEvent, waitFor } from '@testing-library/react'
+import { screen, render, fireEvent, waitFor } from '@testing-library/react'
 import userEvent from '@testing-library/user-event'
 
 import { ComboBox, ComboBoxRef } from './ComboBox'
@@ -29,8 +29,8 @@ describe('ComboBox component', () => {
     scrollSpy.mockReset()
   })
 
-  it('renders without errors', () => {
-    const { getByTestId } = render(
+  it('renders the expected markup without errors', () => {
+    render(
       <ComboBox
         id="favorite-fruit"
         name="favorite-fruit"
@@ -38,154 +38,197 @@ describe('ComboBox component', () => {
         onChange={jest.fn()}
       />
     )
-    expect(getByTestId('combo-box')).toBeInTheDocument()
-  })
 
-  it('renders hidden select element on load', () => {
-    const { getByTestId } = render(
-      <ComboBox
-        id="favorite-fruit"
-        name="favorite-fruit"
-        options={fruitOptions}
-        onChange={jest.fn()}
-        defaultValue="apple"
-      />
-    )
-    const comboBoxSelect = getByTestId('combo-box-select')
+    const comboBoxContainer = screen.getByTestId('combo-box')
+    expect(comboBoxContainer).toBeInTheDocument()
+    expect(comboBoxContainer).toHaveClass('usa-combo-box')
+    expect(comboBoxContainer).not.toHaveClass('usa-combo-box--pristine')
+    expect(comboBoxContainer).toHaveAttribute('data-enhanced', 'true')
+
+    const comboBoxSelect = screen.getByTestId('combo-box-select')
     expect(comboBoxSelect).toBeInstanceOf(HTMLSelectElement)
     expect(comboBoxSelect).toHaveAttribute('aria-hidden', 'true')
-    expect(comboBoxSelect).toHaveClass('usa-sr-only')
-  })
-
-  it('renders input element', () => {
-    const { getByRole } = render(
-      <ComboBox
-        id="favorite-fruit"
-        name="favorite-fruit"
-        options={fruitOptions}
-        onChange={jest.fn()}
-      />
+    expect(comboBoxSelect).toHaveClass(
+      'usa-select usa-sr-only usa-combo-box__select'
     )
 
-    const comboBox = getByRole('combobox')
-    expect(comboBox).toBeInTheDocument()
-    expect(comboBox).toBeInstanceOf(HTMLInputElement)
-  })
-
-  it('renders hidden options list on load', () => {
-    const { getByTestId } = render(
-      <ComboBox
-        id="favorite-fruit"
-        name="favorite-fruit"
-        options={fruitOptions}
-        onChange={jest.fn()}
-      />
+    const comboBoxInput = screen.getByRole('combobox')
+    expect(comboBoxInput).toBeInTheDocument()
+    expect(comboBoxInput).toBeInstanceOf(HTMLInputElement)
+    expect(comboBoxInput).toHaveAttribute('aria-owns', 'favorite-fruit--list')
+    expect(comboBoxInput).toHaveAttribute('aria-autocomplete', 'list')
+    expect(comboBoxInput).toHaveAttribute(
+      'aria-describedby',
+      'favorite-fruit--assistiveHint'
     )
-    expect(getByTestId('combo-box-option-list')).toBeInstanceOf(
-      HTMLUListElement
-    )
-    expect(getByTestId('combo-box-input')).toHaveAttribute(
-      'aria-expanded',
-      'false'
-    )
-    expect(getByTestId('combo-box-option-list')).not.toBeVisible()
+    expect(comboBoxInput).toHaveAttribute('aria-expanded', 'false')
+    expect(comboBoxInput).toHaveAttribute('autocapitalize', 'off')
+    expect(comboBoxInput).toHaveAttribute('autocomplete', 'off')
+    expect(comboBoxInput).toHaveAttribute('type', 'text')
+
+    const comboBoxList = screen.getByTestId('combo-box-option-list')
+    expect(comboBoxList).toBeInstanceOf(HTMLUListElement)
+    expect(comboBoxList).toHaveAttribute('id', 'favorite-fruit--list')
+    expect(comboBoxList).toHaveAttribute('role', 'listbox')
+    expect(comboBoxList).not.toBeVisible()
   })
 
-  it('shows options list when input toggle clicked', () => {
-    const { getByTestId } = render(
+  it('renders the expected markup with a default value', () => {
+    render(
       <ComboBox
         id="favorite-fruit"
         name="favorite-fruit"
         options={fruitOptions}
         onChange={jest.fn()}
+        defaultValue="avocado"
       />
     )
 
-    userEvent.click(getByTestId('combo-box-toggle'))
+    const comboBoxContainer = screen.getByTestId('combo-box')
+    expect(comboBoxContainer).toHaveClass('usa-combo-box--pristine')
+
+    const comboBoxSelect = screen.getByTestId('combo-box-select')
+    expect(comboBoxSelect).toHaveValue('avocado')
 
-    expect(getByTestId('combo-box-option-list')).toBeVisible()
+    const comboBoxInput = screen.getByRole('combobox')
+    expect(comboBoxInput).toHaveValue('Avocado')
   })
 
-  it('shows list when input is clicked', () => {
-    const { getByTestId } = render(
-      <ComboBox
-        id="favorite-fruit"
-        name="favorite-fruit"
-        options={fruitOptions}
-        onChange={jest.fn()}
-      />
-    )
+  describe('toggling the list', () => {
+    it('renders all options when the list is open', () => {
+      const fruitAbridged = fruitOptions.slice(0, 3)
 
-    userEvent.click(getByTestId('combo-box-input'))
+      const { getByTestId } = render(
+        <ComboBox
+          id="favorite-fruit"
+          name="favorite-fruit"
+          options={fruitAbridged}
+          onChange={jest.fn()}
+        />
+      )
 
-    expect(getByTestId('combo-box-option-list')).toBeVisible()
-  })
+      userEvent.click(getByTestId('combo-box-toggle'))
+      expect(screen.getAllByRole('option')).toHaveLength(fruitAbridged.length)
+
+      fruitAbridged.forEach((item, index) => {
+        const optionEl = screen.getByRole('option', { name: item.label })
+        expect(optionEl).toBeInTheDocument()
+        expect(optionEl).toHaveAttribute('value', item.value)
+        expect(optionEl).toHaveAttribute(
+          'aria-setsize',
+          `${fruitAbridged.length}`
+        )
+        expect(optionEl).toHaveAttribute('aria-posinset', `${index + 1}`)
+        expect(optionEl).toHaveAttribute(
+          'id',
+          `favorite-fruit--list--option-${index}`
+        )
+      })
+    })
 
-  it('shows list when input is typed into', () => {
-    const { getByTestId } = render(
-      <ComboBox
-        id="favorite-fruit"
-        name="favorite-fruit"
-        options={fruitOptions}
-        onChange={jest.fn()}
-      />
-    )
+    it('shows options list when input toggle clicked', () => {
+      const { getByTestId } = render(
+        <ComboBox
+          id="favorite-fruit"
+          name="favorite-fruit"
+          options={fruitOptions}
+          onChange={jest.fn()}
+        />
+      )
 
-    userEvent.type(getByTestId('combo-box-input'), 'b')
+      userEvent.click(getByTestId('combo-box-toggle'))
 
-    expect(getByTestId('combo-box-option-list')).toBeVisible()
-  })
+      expect(getByTestId('combo-box-option-list')).toBeVisible()
+    })
 
-  it('highlights the first option when opening the menu, when no default value exists', () => {
-    const { getByTestId } = render(
-      <ComboBox
-        id="favorite-fruit"
-        name="favorite-fruit"
-        options={fruitOptions}
-        onChange={jest.fn()}
-      />
-    )
+    it('shows list when input is clicked', () => {
+      const { getByTestId } = render(
+        <ComboBox
+          id="favorite-fruit"
+          name="favorite-fruit"
+          options={fruitOptions}
+          onChange={jest.fn()}
+        />
+      )
 
-    const firstItem = getByTestId('combo-box-option-list').children[0]
+      userEvent.click(getByTestId('combo-box-input'))
 
-    userEvent.click(getByTestId('combo-box-toggle'))
+      expect(getByTestId('combo-box-option-list')).toBeVisible()
+    })
 
-    expect(firstItem).toBeVisible()
-    expect(firstItem).not.toHaveFocus()
-    expect(firstItem).toHaveClass('usa-combo-box__list-option--focused')
-  })
+    it('shows list when input is typed into', () => {
+      const { getByTestId } = render(
+        <ComboBox
+          id="favorite-fruit"
+          name="favorite-fruit"
+          options={fruitOptions}
+          onChange={jest.fn()}
+        />
+      )
 
-  it('highlights the default value when opening the menu, when one exists', () => {
-    const { getByTestId } = render(
-      <ComboBox
-        id="favorite-fruit"
-        name="favorite-fruit"
-        options={fruitOptions}
-        onChange={jest.fn()}
-        defaultValue="avocado"
-      />
-    )
+      userEvent.type(getByTestId('combo-box-input'), 'b')
+      expect(getByTestId('combo-box-option-list')).toBeVisible()
+    })
 
-    userEvent.click(getByTestId('combo-box-input'))
+    it('highlights the first option when opening the menu, when no default value exists', () => {
+      const { getByTestId } = render(
+        <ComboBox
+          id="favorite-fruit"
+          name="favorite-fruit"
+          options={fruitOptions}
+          onChange={jest.fn()}
+        />
+      )
 
-    expect(getByTestId('combo-box-option-avocado')).toHaveAttribute(
-      'aria-selected',
-      'true'
-    )
+      const firstItem = getByTestId('combo-box-option-list').children[0]
+
+      userEvent.click(getByTestId('combo-box-toggle'))
+
+      expect(firstItem).toBeVisible()
+      expect(firstItem).not.toHaveFocus()
+      expect(firstItem).toHaveClass('usa-combo-box__list-option--focused')
+    })
+
+    it('highlights the default value when opening the menu, when one exists', () => {
+      const { getByTestId } = render(
+        <ComboBox
+          id="favorite-fruit"
+          name="favorite-fruit"
+          options={fruitOptions}
+          onChange={jest.fn()}
+          defaultValue="avocado"
+        />
+      )
+
+      userEvent.click(getByTestId('combo-box-input'))
+
+      expect(getByTestId('combo-box-option-avocado')).toHaveAttribute(
+        'aria-selected',
+        'true'
+      )
+    })
   })
 
   it('can be disabled', () => {
-    const { getByTestId } = render(
+    render(
       <ComboBox
         id="favorite-fruit"
         name="favorite-fruit"
-        options={fruitOptions}
+        options={[]}
         onChange={jest.fn()}
         disabled={true}
       />
     )
-    expect(getByTestId('combo-box-input')).toBeDisabled()
-    expect(getByTestId('combo-box-select')).toBeDisabled()
+
+    expect(screen.getByLabelText('Clear the select contents')).toBeDisabled()
+    expect(screen.getByLabelText('Clear the select contents')).not.toBeVisible()
+
+    expect(screen.getByRole('combobox')).toBeDisabled()
+    expect(
+      screen.getByRole('button', { name: 'Toggle the dropdown list' })
+    ).toBeDisabled()
+
+    expect(screen.getByTestId('combo-box-select')).not.toBeDisabled()
   })
 
   it('does not show the list when clicking the disabled component', () => {
diff --git a/src/components/forms/ComboBox/ComboBox.tsx b/src/components/forms/ComboBox/ComboBox.tsx
index 6d6ea94e66..3d505d9389 100644
--- a/src/components/forms/ComboBox/ComboBox.tsx
+++ b/src/components/forms/ComboBox/ComboBox.tsx
@@ -339,14 +339,15 @@ export const ComboBox = forwardRef(
     const containerClasses = classnames('usa-combo-box', className, {
       'usa-combo-box--pristine': isPristine,
     })
-    const listID = `combobox-${name}-list`
-    const assistiveHintID = `combobox-${name}-assistive-hint`
+
+    const listID = `${id}--list`
+    const assistiveHintID = `${id}--assistiveHint`
 
     return (
       <div
         data-testid="combo-box"
+        data-enhanced="true"
         className={containerClasses}
-        id={id}
         ref={containerRef}>
         <select
           {...selectProps}
@@ -355,8 +356,7 @@ export const ComboBox = forwardRef(
           aria-hidden
           tabIndex={-1}
           defaultValue={state.selectedOption?.value}
-          data-testid="combo-box-select"
-          disabled={isDisabled}>
+          data-testid="combo-box-select">
           {options.map((option) => (
             <option key={option.value} value={option.value}>
               {option.label}
@@ -380,8 +380,10 @@ export const ComboBox = forwardRef(
           value={state.inputValue}
           focused={state.focusMode === FocusMode.Input}
           aria-owns={listID}
+          aria-autocomplete="list"
           aria-describedby={assistiveHintID}
           aria-expanded={state.isOpen}
+          id={id}
           disabled={isDisabled}
         />
         <span className="usa-combo-box__clear-input__wrapper" tabIndex={-1}>
@@ -392,7 +394,8 @@ export const ComboBox = forwardRef(
             onClick={(): void => dispatch({ type: ActionTypes.CLEAR })}
             data-testid="combo-box-clear-button"
             onKeyDown={handleClearKeyDown}
-            hidden={!isPristine}>
+            hidden={!isPristine || isDisabled}
+            disabled={isDisabled}>
             &nbsp;
           </button>
         </span>
@@ -440,12 +443,13 @@ export const ComboBox = forwardRef(
                 tabIndex={focused ? 0 : -1}
                 role="option"
                 aria-selected={selected}
-                aria-setsize={64}
+                aria-setsize={state.filteredOptions.length}
                 aria-posinset={index + 1}
                 id={listID + `--option-${index}`}
                 onKeyDown={handleListItemKeyDown}
                 onBlur={handleListItemBlur}
                 data-testid={`combo-box-option-${option.value}`}
+                data-value={option.value}
                 onMouseEnter={(): void =>
                   dispatch({ type: ActionTypes.FOCUS_OPTION, option: option })
                 }