Skip to content

SelectPanel: Add filter input label and description #3312

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

Merged
merged 8 commits into from
May 24, 2023
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/early-beds-whisper.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@primer/react": patch
---

SelectPanel: Add filter input label and description
6 changes: 6 additions & 0 deletions generated/components.json
Original file line number Diff line number Diff line change
Expand Up @@ -3655,6 +3655,12 @@
"defaultValue": "",
"description": ""
},
{
"name": "inputLabel",
"type": "string",
"defaultValue": "Same as placeholderText",
"description": "The aria-label for the filter input"
},
{
"name": "overlayProps",
"type": "Partial<OverlayProps>",
Expand Down
24 changes: 14 additions & 10 deletions src/FilteredActionList/FilteredActionList.tsx
Original file line number Diff line number Diff line change
@@ -1,19 +1,20 @@
import type {ScrollIntoViewOptions} from '@primer/behaviors'
import {scrollIntoView} from '@primer/behaviors'
import React, {KeyboardEventHandler, useCallback, useEffect, useRef} from 'react'
import {GroupedListProps, ListPropsBase} from '../deprecated/ActionList/List'
import TextInput, {TextInputProps} from '../TextInput'
import styled from 'styled-components'
import Box from '../Box'
import {ActionList} from '../deprecated/ActionList'
import Spinner from '../Spinner'
import {useFocusZone} from '../hooks/useFocusZone'
import {useProvidedStateOrCreate} from '../hooks/useProvidedStateOrCreate'
import styled from 'styled-components'
import TextInput, {TextInputProps} from '../TextInput'
import {get} from '../constants'
import {ActionList} from '../deprecated/ActionList'
import {GroupedListProps, ListPropsBase} from '../deprecated/ActionList/List'
import {useFocusZone} from '../hooks/useFocusZone'
import {useId} from '../hooks/useId'
import {useProvidedRefOrCreate} from '../hooks/useProvidedRefOrCreate'
import {useProvidedStateOrCreate} from '../hooks/useProvidedStateOrCreate'
import useScrollFlash from '../hooks/useScrollFlash'
import {scrollIntoView} from '@primer/behaviors'
import type {ScrollIntoViewOptions} from '@primer/behaviors'
import {VisuallyHidden} from '../internal/components/VisuallyHidden'
import {SxProp} from '../sx'
import {useId} from '../hooks/useId'

const menuScrollMargins: ScrollIntoViewOptions = {startMargin: 0, endMargin: 8}

Expand All @@ -22,7 +23,7 @@ export interface FilteredActionListProps
ListPropsBase,
SxProp {
loading?: boolean
placeholderText: string
placeholderText?: string
filterValue?: string
onFilterChange: (value: string, e: React.ChangeEvent<HTMLInputElement>) => void
textInputProps?: Partial<Omit<TextInputProps, 'onChange'>>
Expand Down Expand Up @@ -60,6 +61,7 @@ export function FilteredActionList({
const inputRef = useProvidedRefOrCreate<HTMLInputElement>(providedInputRef)
const activeDescendantRef = useRef<HTMLElement>()
const listId = useId()
const inputDescriptionTextId = useId()
const onInputKeyPress: KeyboardEventHandler = useCallback(
event => {
if (event.key === 'Enter' && activeDescendantRef.current) {
Expand Down Expand Up @@ -119,9 +121,11 @@ export function FilteredActionList({
placeholder={placeholderText}
aria-label={placeholderText}
aria-controls={listId}
aria-describedby={inputDescriptionTextId}
{...textInputProps}
/>
</StyledHeader>
<VisuallyHidden id={inputDescriptionTextId}>Items will be filtered as you type</VisuallyHidden>
<Box ref={scrollContainerRef} overflow="auto">
{loading ? (
<Box width="100%" display="flex" flexDirection="row" justifyContent="center" pt={6} pb={7}>
Expand Down
8 changes: 7 additions & 1 deletion src/SelectPanel/SelectPanel.docs.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,12 @@
"defaultValue": "",
"description": ""
},
{
"name": "inputLabel",
"type": "string",
"defaultValue": "Same as placeholderText",
"description": "The aria-label for the filter input"
},
{
"name": "overlayProps",
"type": "Partial<OverlayProps>",
Expand Down Expand Up @@ -56,4 +62,4 @@
}
],
"subcomponents": []
}
}
10 changes: 9 additions & 1 deletion src/SelectPanel/SelectPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {TextInputProps} from '../TextInput'
import {useProvidedStateOrCreate} from '../hooks/useProvidedStateOrCreate'
import {AnchoredOverlayWrapperAnchorProps} from '../AnchoredOverlay/AnchoredOverlay'
import {useProvidedRefOrCreate} from '../hooks'
import {SearchIcon} from '@primer/octicons-react'

interface SelectPanelSingleSelection {
selected: ItemInput | undefined
Expand All @@ -27,6 +28,8 @@ interface SelectPanelBaseProps {
gesture: 'anchor-click' | 'anchor-key-press' | 'click-outside' | 'escape' | 'selection',
) => void
placeholder?: string
// TODO: Make `inputLabel` required in next major version
inputLabel?: string
overlayProps?: Partial<OverlayProps>
}

Expand All @@ -53,6 +56,8 @@ export function SelectPanel({
renderAnchor = props => <DropdownButton {...props} />,
anchorRef: externalAnchorRef,
placeholder,
placeholderText = 'Filter items',
inputLabel = placeholderText,
selected,
onSelectedChange,
filterValue: externalFilterValue,
Expand Down Expand Up @@ -141,9 +146,11 @@ export function SelectPanel({
return {
sx: {m: 2},
contrast: true,
leadingVisual: SearchIcon,
'aria-label': inputLabel,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:accessibility: A11y question: Should we use a visually hidden label instead of aria-label for the filter input?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd say it doesn't matter as much as they both will announce the same. I'd say one benefit that you could potentially gain with a visually hidden label and aria-labelledby is that it might translate better with automated translation tools versus an aria-label, but I believe support has gotten better since I last checked. Other than that, I think using aria-label is fine.

...textInputProps,
}
}, [textInputProps])
}, [inputLabel, textInputProps])

return (
<AnchoredOverlay
Expand All @@ -159,6 +166,7 @@ export function SelectPanel({
<FilteredActionList
filterValue={filterValue}
onFilterChange={onFilterChange}
placeholderText={placeholderText}
{...listProps}
role="listbox"
aria-multiselectable={isMultiSelectVariant(selected) ? 'true' : 'false'}
Expand Down