Skip to content
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
5 changes: 5 additions & 0 deletions .changeset/chubby-wombats-start.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@primer/react': minor
---

FilteredActionList: Adds new prop `setInitialFocus` which will prevent `aria-activedescendant` from being set until user action
5 changes: 5 additions & 0 deletions .changeset/many-planets-take.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@primer/react': minor
---

FilteredActionList: Add prop `disableSelectOnHover` which will disable the ability where hovering over an item sets it as the `aria-activedescendant` value
8 changes: 4 additions & 4 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion packages/react/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@
"@github/tab-container-element": "^4.8.2",
"@lit-labs/react": "1.2.1",
"@oddbird/popover-polyfill": "^0.5.2",
"@primer/behaviors": "^1.8.2",
"@primer/behaviors": "^1.9.0",
"@primer/live-region-element": "^0.7.1",
"@primer/octicons-react": "^19.13.0",
"@primer/primitives": "10.x || 11.x",
Expand Down
13 changes: 13 additions & 0 deletions packages/react/src/FilteredActionList/FilteredActionList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,15 @@ export interface FilteredActionListProps extends Partial<Omit<GroupedListProps,
* @default 'active-descendant'
*/
_PrivateFocusManagement?: 'roving-tabindex' | 'active-descendant'
/**
* If true, disables selecting items when hovering over them with the mouse.
*/
disableSelectOnHover?: boolean
/**
* If true, focus remains where it was and the user must interact to move focus.
* If false, sets initial focus to the first item in the list when rendered, enabling keyboard navigation immediately.
*/
setInitialFocus?: boolean
}

export function FilteredActionList({
Expand All @@ -106,6 +115,8 @@ export function FilteredActionList({
actionListProps,
focusOutBehavior = 'wrap',
_PrivateFocusManagement = 'active-descendant',
disableSelectOnHover = false,
setInitialFocus = false,
...listProps
}: FilteredActionListProps): JSX.Element {
const [filterValue, setInternalFilterValue] = useProvidedStateOrCreate(externalFilterValue, undefined, '')
Expand Down Expand Up @@ -233,6 +244,8 @@ export function FilteredActionList({
scrollIntoView(current, scrollContainerRef.current, menuScrollMargins)
}
},
focusInStrategy: setInitialFocus ? 'initial' : 'previous',
ignoreHoverEvents: disableSelectOnHover,
}
: undefined,
[listContainerElement, usingRovingTabindex],
Expand Down
62 changes: 62 additions & 0 deletions packages/react/src/SelectPanel/SelectPanel.dev.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -381,3 +381,65 @@ export const LotsOfItems = () => {
</>
)
}

export const WithDisableOnHover = ({onCancel, secondaryAction}: ParamProps) => {
const [selected, setSelected] = useState<ItemInput[]>(simpleItems.slice(1, 3))
const [filter, setFilter] = useState('')
const filteredItems = simpleItems.filter(item => item.text.toLowerCase().startsWith(filter.toLowerCase()))
const [open, setOpen] = useState(false)

return (
<SelectPanel
title="Select labels"
placeholder="Select labels"
subtitle="Use labels to organize issues and pull requests"
renderAnchor={({children, ...anchorProps}) => (
<Button trailingAction={TriangleDownIcon} {...anchorProps} aria-haspopup="dialog">
{children}
</Button>
)}
open={open}
onOpenChange={setOpen}
items={filteredItems}
selected={selected}
onSelectedChange={setSelected}
onFilterChange={setFilter}
width="medium"
message={filteredItems.length === 0 ? NoResultsMessage(filter) : undefined}
onCancel={onCancel}
secondaryAction={secondaryAction}
disableSelectOnHover
/>
)
}

export const WithInitialFocusEnabled = ({onCancel, secondaryAction}: ParamProps) => {
const [selected, setSelected] = useState<ItemInput[]>(simpleItems.slice(1, 3))
const [filter, setFilter] = useState('')
const filteredItems = simpleItems.filter(item => item.text.toLowerCase().startsWith(filter.toLowerCase()))
const [open, setOpen] = useState(false)

return (
<SelectPanel
title="Select labels"
placeholder="Select labels"
subtitle="Use labels to organize issues and pull requests"
renderAnchor={({children, ...anchorProps}) => (
<Button trailingAction={TriangleDownIcon} {...anchorProps} aria-haspopup="dialog">
{children}
</Button>
)}
open={open}
onOpenChange={setOpen}
items={filteredItems}
selected={selected}
onSelectedChange={setSelected}
onFilterChange={setFilter}
width="medium"
message={filteredItems.length === 0 ? NoResultsMessage(filter) : undefined}
onCancel={onCancel}
secondaryAction={secondaryAction}
setInitialFocus={true}
/>
)
}
116 changes: 116 additions & 0 deletions packages/react/src/SelectPanel/SelectPanel.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1519,6 +1519,122 @@ for (const usingRemoveActiveDescendant of [false, true]) {
expect(selectAllCheckbox).toHaveProperty('indeterminate', true)
})
})

describe('disableSelectOnHover', () => {
it('should not update aria-activedescendant when hovering over items when disableSelectOnHover is true', async () => {
const user = userEvent.setup()

render(<BasicSelectPanel disableSelectOnHover={true} />)

await user.click(screen.getByText('Select items'))

const input = screen.getByPlaceholderText('Filter items')
const options = screen.getAllByRole('option')

// Initially, aria-activedescendant should not be set if setInitialFocus is false (default)
const initialActiveDescendant = input.getAttribute('aria-activedescendant')

// Hover over the first item
await user.hover(options[0])

// aria-activedescendant should not change when disableSelectOnHover is true
expect(input.getAttribute('aria-activedescendant')).toBe(initialActiveDescendant)

// Hover over the second item
await user.hover(options[1])

// aria-activedescendant should still not change
expect(input.getAttribute('aria-activedescendant')).toBe(initialActiveDescendant)
})

it('should update aria-activedescendant when hovering over items when disableSelectOnHover is false (default)', async () => {
const user = userEvent.setup()

render(<BasicSelectPanel />)

await user.click(screen.getByText('Select items'))

const input = screen.getByPlaceholderText('Filter items')
const options = screen.getAllByRole('option')

// Hover over the first item
await user.hover(options[0])

// aria-activedescendant should be set to the first item
expect(input.getAttribute('aria-activedescendant')).toBe(options[0].id)

// Hover over the second item
await user.hover(options[1])

// aria-activedescendant should be updated to the second item
expect(input.getAttribute('aria-activedescendant')).toBe(options[1].id)
})
})

describe('setInitialFocus', () => {
it('should not set aria-activedescendant until user interaction when setInitialFocus is true', async () => {
const user = userEvent.setup()

render(<BasicSelectPanel setInitialFocus={true} />)

await user.click(screen.getByText('Select items'))

const input = screen.getByPlaceholderText('Filter items')
const options = screen.getAllByRole('option')

// Initially, aria-activedescendant should not be set
expect(input.getAttribute('aria-activedescendant')).toBeFalsy()

// User interacts with keyboard
await user.keyboard('{ArrowDown}')

// Now aria-activedescendant should be set to the first item
expect(input.getAttribute('aria-activedescendant')).toBe(options[0].id)
})

it('should set aria-activedescendant to the first item on mount when setInitialFocus is false (default)', async () => {
const user = userEvent.setup()

render(<BasicSelectPanel />)

await user.click(screen.getByText('Select items'))

const input = screen.getByPlaceholderText('Filter items')
const options = screen.getAllByRole('option')

// Wait a tick for the effect to run
await new Promise(resolve => setTimeout(resolve, 0))

// aria-activedescendant should be set to the first item
expect(input.getAttribute('aria-activedescendant')).toBe(options[0].id)
})

it('should not set aria-activedescendant on mouse hover until after first interaction when setInitialFocus is true', async () => {
const user = userEvent.setup()

render(<BasicSelectPanel setInitialFocus={true} />)

await user.click(screen.getByText('Select items'))

const input = screen.getByPlaceholderText('Filter items')
const options = screen.getAllByRole('option')

// Initially, aria-activedescendant should not be set
expect(input.getAttribute('aria-activedescendant')).toBeFalsy()

// Hover over the first item (this is the first interaction)
await user.hover(options[0])

// Now aria-activedescendant should be set to the first item
expect(input.getAttribute('aria-activedescendant')).toBe(options[0].id)

// Hover over the second item
await user.hover(options[1])

// aria-activedescendant should update to the second item
expect(input.getAttribute('aria-activedescendant')).toBe(options[1].id)
})
})
})

describe('Event propagation', () => {
Expand Down
Loading