Skip to content

Commit

Permalink
Select Panel: Functional adjustments (#4906)
Browse files Browse the repository at this point in the history
* super wip

* just use the actionlist component and revert the type updates

* Update packages/react/src/FilteredActionList/FilteredActionList.tsx

* Some more progress

* revert the type changes and cast it :/

* clean

* wip

* wip wip

* add stories

* fix linting

* add tests for groups

* Map groups

* Update story names for e2e tests

* oops remove unintended file

* update story name

* same - update story name

* disable animations

* test(vrt): update snapshots

* Update tests since new action list has different semantics for group headings

* logging

* pass the rest

* extract children and use before text

* remove logging

* test(vrt): update snapshots

* add active styles to ActionList

* rename component name to be clearer

* remove variant full from examples

* tiny clean up

* fix showItemDividers

* another tiny cleanup

* pull MappedActionListItem to make it stable

* test(vrt): update snapshots

* show active styles only when used with keyboard

* backward compat: expose id as data-id

* update snapshots

* add story for long strings

* fishing for errors

* backward compatibility for renderItem

* remove todo now

* add a feature flag

* clean up dual filter list setup

* run jests test with both states of feature flags

* refactor snapshot tests with scenarios

* remove feature flag for main

* test(vrt): update snapshots

* add feature flag to e2e matrix

* test(vrt): update snapshots

* backward compat: allow groupMetadata to be empty array

* sigh newline

* Create sour-cooks-dress.md

* copy SelectPanel snapshots from main

* remove unrelated changes in this PR

* test(vrt): update snapshots

* add more bindkeys

* add for deprecated version as well

* label listbox/ActionList by panel title

* Create wicked-books-occur.md

* remove Home and End

* add test for PageDown and PageUp

* update changeset

* active styles for both directly and indirectly

* add role=combobox

* update snapshot

* test(vrt): update snapshots

* remove features from deprecated version

* update test

* only use aria-labelledby when aria-label is not passed

* remove combobox for this PR

---------

Co-authored-by: Armagan Ersoz <broccolinisoup@github.com>
Co-authored-by: broccolinisoup <broccolinisoup@users.noreply.github.com>
Co-authored-by: siddharthkp <siddharthkp@users.noreply.github.com>
  • Loading branch information
4 people committed Sep 12, 2024
1 parent 373ce95 commit dbf82f4
Show file tree
Hide file tree
Showing 5 changed files with 64 additions and 2 deletions.
7 changes: 7 additions & 0 deletions .changeset/wicked-books-occur.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@primer/react": minor
---

SelectPanel: Support <kbd>PageDown</kbd> and <kbd>PageUp</kbd> for keyboard navigation

SelectPanel: Label `listbox` by the title of the panel
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type {ScrollIntoViewOptions} from '@primer/behaviors'
import {scrollIntoView} from '@primer/behaviors'
import {scrollIntoView, FocusKeys} from '@primer/behaviors'
import type {KeyboardEventHandler} from 'react'
import React, {useCallback, useEffect, useRef} from 'react'
import styled from 'styled-components'
Expand Down Expand Up @@ -86,6 +86,7 @@ export function FilteredActionList({
useFocusZone(
{
containerRef: listContainerRef,
bindKeys: FocusKeys.ArrowVertical | FocusKeys.PageUpDown,
focusOutBehavior: 'wrap',
focusableElementFilter: element => {
return !(element instanceof HTMLInputElement)
Expand Down
48 changes: 47 additions & 1 deletion packages/react/src/SelectPanel/SelectPanel.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ const items: SelectPanelProps['items'] = [
},
]

function BasicSelectPanel() {
function BasicSelectPanel(passthroughProps: Record<string, unknown>) {
const [selected, setSelected] = React.useState<SelectPanelProps['items']>([])
const [filter, setFilter] = React.useState('')
const [open, setOpen] = React.useState(false)
Expand All @@ -51,6 +51,7 @@ function BasicSelectPanel() {
onOpenChange={isOpen => {
setOpen(isOpen)
}}
{...passthroughProps}
/>
</ThemeProvider>
)
Expand Down Expand Up @@ -200,6 +201,22 @@ for (const useModernActionList of [false, true]) {
expect(onOpenChange).toHaveBeenLastCalledWith(false, 'click-outside')
})

it('should label the list by title unless a aria-label is explicitly passed', async () => {
const user = userEvent.setup()

renderWithFlag(<BasicSelectPanel />, useModernActionList)
await user.click(screen.getByText('Select items'))
expect(screen.getByRole('listbox', {name: 'test title'})).toBeInTheDocument()
})

it('should label the list by aria-label when explicitly passed', async () => {
const user = userEvent.setup()

renderWithFlag(<BasicSelectPanel aria-label="Custom label" />, useModernActionList)
await user.click(screen.getByText('Select items'))
expect(screen.getByRole('listbox', {name: 'Custom label'})).toBeInTheDocument()
})

describe('selection', () => {
it('should select an active option when activated', async () => {
const user = userEvent.setup()
Expand Down Expand Up @@ -288,6 +305,35 @@ for (const useModernActionList of [false, true]) {
screen.getByRole('option', {name: 'item one'}).id,
)
})

it('should support navigating through items with PageDown and PageUp', async () => {
if (!useModernActionList) return // this feature is only enabled with feature flag on

const user = userEvent.setup()

renderWithFlag(<BasicSelectPanel />, useModernActionList)

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

// First item by default should be the active element
expect(document.activeElement!).toHaveAttribute(
'aria-activedescendant',
screen.getByRole('option', {name: 'item one'}).id,
)

await user.type(document.activeElement!, '{PageDown}')

expect(document.activeElement!).toHaveAttribute(
'aria-activedescendant',
screen.getByRole('option', {name: 'item three'}).id,
)

await user.type(document.activeElement!, '{PageUp}')
expect(document.activeElement!).toHaveAttribute(
'aria-activedescendant',
screen.getByRole('option', {name: 'item one'}).id,
)
})
})

describe('filtering', () => {
Expand Down
3 changes: 3 additions & 0 deletions packages/react/src/SelectPanel/SelectPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,9 @@ export function SelectPanel({
placeholderText={placeholderText}
{...listProps}
role="listbox"
// browsers give aria-labelledby precedence over aria-label so we need to make sure
// we don't accidentally override props.aria-label
aria-labelledby={listProps['aria-label'] ? undefined : titleId}
aria-multiselectable={isMultiSelectVariant(selected) ? 'true' : 'false'}
selectionVariant={isMultiSelectVariant(selected) ? 'multiple' : 'single'}
items={itemsToRender}
Expand Down
5 changes: 5 additions & 0 deletions packages/react/src/deprecated/ActionList/List.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,11 @@ export interface ListPropsBase {
*/
id?: string

/**
* aria-label to attach to the base DOM node of the list
*/
'aria-label'?: string

/**
* A `List`-level custom `Item` renderer. Every `Item` within this `List`
* without a `Group`-level or `Item`-level custom `Item` renderer will be
Expand Down

0 comments on commit dbf82f4

Please sign in to comment.