Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Control ComboBox scroll with scrollTop instead of scrollIntoView #1715

Merged
merged 1 commit into from
Oct 25, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/components/forms/ComboBox/ComboBox.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ export const withDefaultValue = (): React.ReactElement => {
name="input-ComboBox"
options={fruitList}
onChange={noop}
defaultValue="avocado"
defaultValue="mango"
/>
</Form>
)
Expand Down
72 changes: 53 additions & 19 deletions src/components/forms/ComboBox/ComboBox.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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"
Expand All @@ -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', () => {
Expand Down
18 changes: 17 additions & 1 deletion src/components/forms/ComboBox/ComboBox.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ export const ComboBox = forwardRef(
)

const containerRef = useRef<HTMLDivElement>(null)
const listRef = useRef<HTMLUListElement>(null)
const focusedItemRef = useRef<HTMLLIElement>(null)

useEffect(() => {
Expand All @@ -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])

Expand Down Expand Up @@ -301,6 +315,7 @@ export const ComboBox = forwardRef(
}
}
}

const handleListItemBlur = (event: FocusEvent<HTMLLIElement>): void => {
const { relatedTarget: newTarget } = event

Expand Down Expand Up @@ -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
Expand Down
7 changes: 0 additions & 7 deletions src/components/forms/TimePicker/TimePicker.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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()
})
Expand Down Expand Up @@ -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')
Expand Down Expand Up @@ -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' })
Expand Down