Control ComboBox scroll with scrollTop instead of scrollIntoVide (#1715)

trussworks · Oct 25, 2021 · 600d53b · 600d53b
1 parent e070bbd
commit 600d53b
Show file tree

Hide file tree

Showing 4 changed files with 71 additions and 28 deletions.
diff --git a/src/components/forms/ComboBox/ComboBox.stories.tsx b/src/components/forms/ComboBox/ComboBox.stories.tsx
@@ -58,7 +58,7 @@ export const withDefaultValue = (): React.ReactElement => {
         name="input-ComboBox"
         options={fruitList}
         onChange={noop}
-        defaultValue="avocado"
+        defaultValue="mango"
       />
     </Form>
   )

diff --git a/src/components/forms/ComboBox/ComboBox.test.tsx b/src/components/forms/ComboBox/ComboBox.test.tsx
@@ -18,17 +18,6 @@ const fruitOptions = Object.entries(fruits).map(([value, key]) => ({
 }))
 
 describe('ComboBox component', () => {
-  let scrollSpy: jest.Mock
-
-  beforeAll(() => {
-    scrollSpy = jest.fn()
-    window.HTMLElement.prototype.scrollIntoView = scrollSpy
-  })
-
-  beforeEach(() => {
-    scrollSpy.mockReset()
-  })
-
   it('renders the expected markup without errors', () => {
     render(
       <ComboBox
@@ -327,12 +316,26 @@ describe('ComboBox component', () => {
     )
   })
 
-  // TODO: ❓ Don't know how to test this
-  it.todo(
-    'scrolls options list to the very top when the menu opens if nothing is selected'
-  )
+  it('scrolls options list to the very top when the menu opens if nothing is selected', () => {
+    const { getByTestId } = render(
+      <ComboBox
+        id="favorite-fruit"
+        name="favorite-fruit"
+        options={fruitOptions}
+        onChange={jest.fn()}
+      />
+    )
+
+    const listEl = getByTestId('combo-box-option-list')
+    jest.spyOn(listEl, 'offsetHeight', 'get').mockReturnValue(205)
+    listEl.scrollTop = 2000 // Scroll list 2000px down
+
+    userEvent.click(getByTestId('combo-box-toggle'))
+
+    expect(listEl.scrollTop).toEqual(0)
+  })
 
-  it('scrolls to the selected option when the list is opened', async () => {
+  it('scrolls down to the selected option when the list is opened', () => {
     const { getByTestId } = render(
       <ComboBox
         id="favorite-fruit"
@@ -344,15 +347,46 @@ describe('ComboBox component', () => {
     )
 
     const mango = getByTestId('combo-box-option-mango')
+    const listEl = getByTestId('combo-box-option-list')
+
+    jest.spyOn(mango, 'offsetTop', 'get').mockReturnValue(1365)
+    jest.spyOn(mango, 'offsetHeight', 'get').mockReturnValue(39)
+    jest.spyOn(listEl, 'offsetHeight', 'get').mockReturnValue(205)
+    listEl.scrollTop = 0 // Scroll list to the top
 
     userEvent.click(getByTestId('combo-box-toggle'))
     expect(mango).toHaveClass(
       'usa-combo-box__list-option--focused usa-combo-box__list-option--selected'
     )
 
-    await waitFor(() => {
-      expect(scrollSpy).toHaveBeenCalledTimes(1)
-    })
+    expect(listEl.scrollTop).toEqual(1199)
+  })
+
+  it('scrolls up to the selected option when the list is opened', () => {
+    const { getByTestId } = render(
+      <ComboBox
+        id="favorite-fruit"
+        name="favorite-fruit"
+        options={fruitOptions}
+        onChange={jest.fn()}
+        defaultValue={'mango'}
+      />
+    )
+
+    const mango = getByTestId('combo-box-option-mango')
+    const listEl = getByTestId('combo-box-option-list')
+
+    jest.spyOn(mango, 'offsetTop', 'get').mockReturnValue(1365)
+    jest.spyOn(mango, 'offsetHeight', 'get').mockReturnValue(39)
+    jest.spyOn(listEl, 'offsetHeight', 'get').mockReturnValue(205)
+    listEl.scrollTop = 2292 // Scroll list 2292px down
+
+    userEvent.click(getByTestId('combo-box-toggle'))
+    expect(mango).toHaveClass(
+      'usa-combo-box__list-option--focused usa-combo-box__list-option--selected'
+    )
+
+    expect(listEl.scrollTop).toEqual(1365)
   })
 
   describe('filtering', () => {

diff --git a/src/components/forms/ComboBox/ComboBox.tsx b/src/components/forms/ComboBox/ComboBox.tsx
@@ -139,6 +139,7 @@ export const ComboBox = forwardRef(
     )
 
     const containerRef = useRef<HTMLDivElement>(null)
+    const listRef = useRef<HTMLUListElement>(null)
     const focusedItemRef = useRef<HTMLLIElement>(null)
 
     useEffect(() => {
@@ -161,9 +162,22 @@ export const ComboBox = forwardRef(
         state.isOpen &&
         state.focusedOption &&
         focusedItemRef.current &&
+        listRef.current &&
         state.focusMode === FocusMode.Input
       ) {
-        focusedItemRef.current.scrollIntoView(false)
+        const optionBottom =
+          focusedItemRef.current.offsetTop + focusedItemRef.current.offsetHeight
+        const currentBottom =
+          listRef.current.scrollTop + listRef.current.offsetHeight
+
+        if (optionBottom > currentBottom) {
+          listRef.current.scrollTop =
+            optionBottom - listRef.current.offsetHeight
+        }
+
+        if (focusedItemRef.current.offsetTop < listRef.current.scrollTop) {
+          listRef.current.scrollTop = focusedItemRef.current.offsetTop
+        }
       }
     }, [state.isOpen, state.focusedOption])
 
@@ -301,6 +315,7 @@ export const ComboBox = forwardRef(
         }
       }
     }
+
     const handleListItemBlur = (event: FocusEvent<HTMLLIElement>): void => {
       const { relatedTarget: newTarget } = event
 
@@ -425,6 +440,7 @@ export const ComboBox = forwardRef(
           id={listID}
           className="usa-combo-box__list"
           role="listbox"
+          ref={listRef}
           hidden={!state.isOpen}>
           {state.filteredOptions.map((option, index) => {
             const focused = option === state.focusedOption

diff --git a/src/components/forms/TimePicker/TimePicker.test.tsx b/src/components/forms/TimePicker/TimePicker.test.tsx
@@ -5,9 +5,6 @@ import { TimePicker } from './TimePicker'
 import userEvent from '@testing-library/user-event'
 
 describe('TimePicker Component', () => {
-  const scrollFunction = jest.fn()
-  window.HTMLElement.prototype.scrollIntoView = scrollFunction
-
   beforeEach(() => {
     jest.clearAllMocks()
   })
@@ -115,7 +112,6 @@ describe('TimePicker Component', () => {
     userEvent.type(comboBoxTextInput, '5:3p')
     expect(elementToSelect).toHaveClass('usa-combo-box__list-option--focused')
     expect(elementToSelect).not.toHaveFocus()
-    expect(scrollFunction).toHaveBeenCalledTimes(4) // 4 times: open, type: 5, 3, p
 
     fireEvent.keyDown(comboBoxTextInput, { key: 'ArrowDown' })
     expect(elementToSelect).toHaveClass('usa-combo-box__list-option--focused')
@@ -157,21 +153,18 @@ describe('TimePicker Component', () => {
     expect(fiveAm).toHaveClass('usa-combo-box__list-option--focused')
     expect(fiveAm).not.toHaveFocus()
     expect(comboBoxDropdownList.children.length).toEqual(48)
-    expect(scrollFunction).toHaveBeenCalledTimes(2)
 
     // Continue typing to filter by half hour
     userEvent.type(comboBoxTextInput, ':3')
     expect(fiveThirtyAm).toHaveClass('usa-combo-box__list-option--focused')
     expect(fiveThirtyAm).not.toHaveFocus()
     expect(comboBoxDropdownList.children.length).toEqual(48)
-    expect(scrollFunction).toHaveBeenCalledTimes(3)
 
     // Continue typing to filter by am/pm
     userEvent.type(comboBoxTextInput, 'p')
     expect(fiveThirtyPm).toHaveClass('usa-combo-box__list-option--focused')
     expect(fiveThirtyPm).not.toHaveFocus()
     expect(comboBoxDropdownList.children.length).toEqual(48)
-    expect(scrollFunction).toHaveBeenCalledTimes(4)
 
     // Focus the element by pressing the down key
     fireEvent.keyDown(comboBoxTextInput, { key: 'ArrowDown' })