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 FilterSelect jumping issue #5477

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
5 changes: 5 additions & 0 deletions .changeset/fast-beds-crash.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@kaizen/components': patch
---

Update ListBox focus management to check for stable Y position before refocusing to first element
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import React, { useState } from 'react'
import { type Meta, type StoryObj } from '@storybook/react'
import { fn } from '@storybook/test'
import { renderTriggerControls } from '~components/Filter/_docs/controls/renderTriggerControls'
import { Well } from '~components/Well'
import { FilterButton } from '../../FilterButton'
import { FilterSelect } from '../FilterSelect'
import { type SelectOption } from '../types'
Expand Down Expand Up @@ -100,3 +101,44 @@ export const AdditionalProperties: Story = {
},
name: 'Additional option properties',
}

/**
* Extend the option type to have additional properties to use for rendering.
*/
export const FilterSelectBelowPageContent: Story = {
render: () => {
const [isOpen, setIsOpen] = useState<boolean>(false)

return (
<div>
<Well color="gray" style={{ height: '1500px' }}>
Page content above the FilterSelect
</Well>
<FilterSelect
label="Label"
isOpen={isOpen}
setIsOpen={setIsOpen}
renderTrigger={(triggerProps) => <FilterButton {...triggerProps} />}
items={groupedMockItems}
>
{({ items }): JSX.Element[] =>
items.map((item) => {
if (item.type === 'item') {
return (
<FilterSelect.Option
key={item.key}
item={{
...item,
}}
/>
)
}
return <FilterSelect.ItemDefaultRender key={item.key} item={item} />
})
}
</FilterSelect>
</div>
)
},
name: 'FilterSelect below page content',
}
18 changes: 14 additions & 4 deletions packages/components/src/__rc__/Select/Select.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -188,14 +188,24 @@ describe('<Select />', () => {
})
})
it('is closed when hits the escape key', async () => {
const { getByRole } = render(<SelectWrapper defaultOpen />)
const menu = getByRole('listbox')
const { getByRole, queryByRole } = render(<SelectWrapper />)
const trigger = getByRole('combobox', {
name: 'Mock Label',
})
await user.tab()
await waitFor(() => {
expect(trigger).toHaveFocus()
})
await user.keyboard('{Enter}')

await waitFor(() => {
expect(menu).toBeVisible()
expect(queryByRole('listbox')).toBeVisible()
})

await user.keyboard('{Escape}')

await waitFor(() => {
expect(menu).not.toBeInTheDocument()
expect(queryByRole('listbox')).toBe(null)
})
})
})
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import { useEffect, useState } from 'react'

/** This is a helper util to resolve the focus jumping issue with future Select and FilterSelect.
* Due to the floating element's position starting as a negative value on render and then jumping to the correct position, this caused the focus to jump to the top of the page.
* This now polls to check if the element's position is stable by comparing the first and last position.
*/
export const useHasCalculatedListboxPosition = (ref: React.RefObject<HTMLElement>): boolean => {
const [hasStablePosition, setHasStablePosition] = useState(false)
const [lastYPosition, setLastYPosition] = useState<number | null>(null)

useEffect(() => {
const checkPosition = (): void => {
if (ref.current) {
const { y } = ref.current.getBoundingClientRect()
if (lastYPosition === null) {
setLastYPosition(y)
} else if (y === lastYPosition && y >= 0) {
setHasStablePosition(true)
} else {
setLastYPosition(y)
}
}
}

const intervalId = setInterval(() => {
if (hasStablePosition) {
clearInterval(intervalId)
return
}
checkPosition()
}, 1)

return () => clearInterval(intervalId)
}, [ref, lastYPosition, hasStablePosition])

return hasStablePosition
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@ import React, { useEffect, useRef, type HTMLAttributes, type Key, type ReactNode
import { useListBox, type AriaListBoxOptions } from '@react-aria/listbox'
import { type SelectState } from '@react-stately/select'
import classnames from 'classnames'
import { useIsClientReady } from '~components/__utilities__/useIsClientReady'
import { type OverrideClassName } from '~components/types/OverrideClassName'
import { useSelectContext } from '../../context'
import { type SelectItem, type SelectOption } from '../../types'
import { useHasCalculatedListboxPosition } from '../../hooks/useHasCalculatedListboxPosition'
import { type SelectItem, type SelectItemNode, type SelectOption } from '../../types'
import styles from './ListBox.module.scss'

export type SingleListBoxProps<Option extends SelectOption> = OverrideClassName<
Expand All @@ -16,17 +16,38 @@ export type SingleListBoxProps<Option extends SelectOption> = OverrideClassName<
menuProps: AriaListBoxOptions<SelectItem<Option>>
}

/** A util to retrieve the key of the correct focusable items based of the focus strategy
/** determines is the first or last key passed in is a section. If not it will return the key, otherwise will return the first option key of that section */
const getOptionOrSectionKey = (
optionKey: SelectOption['value'] | null,
state: SelectState<SelectItem<any>>,
): Key | null => {
if (!optionKey) return null

const option = state.collection.getItem(optionKey) as SelectItemNode | null
const optionType = option?.type

if (optionType === 'section') {
const sectionOptions = option?.value?.options

return sectionOptions ? Array.from(sectionOptions)[0]?.value : null
}
return optionKey
}

/** A util to retrieve the key of the correct focusable option based of the focus strategy
* This is used to determine which element from the collection to focus to on open base on the keyboard event
* ie: UpArrow will set the focusStrategy to "last"
*/
const getOptionKeyFromCollection = (state: SelectState<SelectItem<any>>): Key | null => {
if (state.selectedItem) {
return state.selectedItem.key
} else if (state.focusStrategy === 'last') {
return state.collection.getLastKey()
}
return state.collection.getFirstKey()

if (state.focusStrategy === 'last') {
return getOptionOrSectionKey(state.collection.getLastKey(), state)
}

return getOptionOrSectionKey(state.collection.getFirstKey(), state)
}

/** This makes the use of query selector less brittle in instances where a failed selector is passed in
Expand All @@ -47,9 +68,9 @@ export const ListBox = <Option extends SelectOption>({
classNameOverride,
...restProps
}: SingleListBoxProps<Option>): JSX.Element => {
const isClientReady = useIsClientReady()
const { state } = useSelectContext<Option>()
const ref = useRef<HTMLUListElement>(null)
const hasCalculatedListboxPosition = useHasCalculatedListboxPosition(ref)
const { listBoxProps } = useListBox(
{
...menuProps,
Expand All @@ -62,23 +83,22 @@ export const ListBox = <Option extends SelectOption>({
)

/**
* This uses the new useIsClientReady to ensure document exists before trying to querySelector and give the time to focus to the correct element
* When the Listbox is opened the initial position starts above the window, which can cause the out of the box behaviour in react-aria's listbox to jump a user to the top of the page.
*/
useEffect(() => {
if (isClientReady) {
if (hasCalculatedListboxPosition) {
const optionKey = getOptionKeyFromCollection(state)
const focusToElement = safeQuerySelector(`[data-key='${optionKey}']`)

if (focusToElement) {
focusToElement.focus()
} else {
// If an element is not found, focus on the listbox. This ensures the list can still be navigated to via keyboard if the keys do not align to the data attributes of the list items.
ref.current?.focus()
}
}
// Only run this effect for checking the first successful render
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [isClientReady])
}, [hasCalculatedListboxPosition])

return (
<ul
Expand Down
Loading