diff --git a/.changeset/grumpy-carrots-admire.md b/.changeset/grumpy-carrots-admire.md new file mode 100644 index 00000000000..0e8708d6a16 --- /dev/null +++ b/.changeset/grumpy-carrots-admire.md @@ -0,0 +1,5 @@ +--- +'@primer/react': minor +--- + +Accessibility fixes for SelectPanel. diff --git a/docs/content/SelectPanel.mdx b/docs/content/SelectPanel.mdx index a044fc2af21..c2067da79cf 100644 --- a/docs/content/SelectPanel.mdx +++ b/docs/content/SelectPanel.mdx @@ -37,19 +37,20 @@ const items = [ ] function DemoComponent() { - const [selected, setSelected] = React.useState([items[0], items[1]]) + const [selected, setSelected] = React.useState([items[2], items[4]]) const [filter, setFilter] = React.useState('') const filteredItems = items.filter(item => item.text.toLowerCase().startsWith(filter.toLowerCase())) const [open, setOpen] = React.useState(false) return ( ( - )} - placeholderText="Filter Labels" + title="Select Items" + inputLabel="Select Items" open={open} onOpenChange={setOpen} items={filteredItems} @@ -57,7 +58,7 @@ function DemoComponent() { onSelectedChange={setSelected} onFilterChange={setFilter} showItemDividers={true} - overlayProps={{width: 'small', height: 'xsmall'}} + overlayProps={{width: 'medium', height: 'medium'}} /> ) } diff --git a/docs/content/deprecated/ActionList.mdx b/docs/content/deprecated/ActionList.mdx index c25de07a382..4768212314a 100644 --- a/docs/content/deprecated/ActionList.mdx +++ b/docs/content/deprecated/ActionList.mdx @@ -14,12 +14,13 @@ Use [new version of ActionList](/ActionList) with composable API, design updates ```jsx ``` @@ -27,12 +28,12 @@ Use [new version of ActionList](/ActionList) with composable API, design updates **After** ```jsx - - New file - Copy link - Edit file + + New file + Copy link + Edit file - Delete file + Delete file ``` @@ -46,12 +47,13 @@ import {ActionList} from '@primer/react/deprecated' ```jsx live deprecated ``` @@ -60,6 +62,7 @@ import {ActionList} from '@primer/react/deprecated' ```jsx live deprecated ``` @@ -104,17 +110,21 @@ import {ActionList} from '@primer/react/deprecated' ```jsx deprecated }, { text: 'React Router link', + role: 'option', renderItem: props => }, { text: 'NextJS style', + role: 'option', renderItem: props => ( diff --git a/src/Autocomplete/AutocompleteMenu.tsx b/src/Autocomplete/AutocompleteMenu.tsx index 2171279b64e..f21a5441bac 100644 --- a/src/Autocomplete/AutocompleteMenu.tsx +++ b/src/Autocomplete/AutocompleteMenu.tsx @@ -151,6 +151,7 @@ function AutocompleteMenu(props: AutocompleteMe items.map(selectableItem => { return { ...selectableItem, + _legacyEnterSupport: true, //TODO: Change behaviour, the enter key should not be used here. role: 'option', id: selectableItem.id, selected: selectionVariant === 'multiple' ? selectedItemIds.includes(selectableItem.id) : undefined, @@ -217,6 +218,7 @@ function AutocompleteMenu(props: AutocompleteMe ? [ { ...addNewItem, + _legacyEnterSupport: true, //TODO: Change behaviour, the enter key should not be used here. leadingVisual: () => , onAction: (item: T) => { // TODO: make it possible to pass a leadingVisual when using `addNewItem` diff --git a/src/FilteredActionList/FilteredActionList.tsx b/src/FilteredActionList/FilteredActionList.tsx index 665cf6a9c12..7ff89e56ead 100644 --- a/src/FilteredActionList/FilteredActionList.tsx +++ b/src/FilteredActionList/FilteredActionList.tsx @@ -5,7 +5,6 @@ import TextInput, {TextInputProps} from '../TextInput' 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 {get} from '../constants' @@ -14,6 +13,7 @@ import useScrollFlash from '../hooks/useScrollFlash' import {scrollIntoView} from '@primer/behaviors' import type {ScrollIntoViewOptions} from '@primer/behaviors' import {SxProp} from '../sx' +import VisuallyHidden from '../_VisuallyHidden' const menuScrollMargins: ScrollIntoViewOptions = {startMargin: 0, endMargin: 8} @@ -22,7 +22,7 @@ export interface FilteredActionListProps ListPropsBase, SxProp { loading?: boolean - placeholderText: string + placeholderText?: string filterValue?: string onFilterChange: (value: string, e: React.ChangeEvent) => void textInputProps?: Partial> @@ -56,10 +56,10 @@ export function FilteredActionList({ ) const scrollContainerRef = useRef(null) - const listContainerRef = useRef(null) const inputRef = useProvidedRefOrCreate(providedInputRef) const activeDescendantRef = useRef() const listId = useSSRSafeId() + const inputDescriptionTextId = useSSRSafeId() const onInputKeyPress: KeyboardEventHandler = useCallback( event => { if (event.key === 'Enter' && activeDescendantRef.current) { @@ -74,28 +74,6 @@ export function FilteredActionList({ [activeDescendantRef] ) - useFocusZone( - { - containerRef: listContainerRef, - focusOutBehavior: 'wrap', - focusableElementFilter: element => { - return !(element instanceof HTMLInputElement) - }, - activeDescendantFocus: inputRef, - onActiveDescendantChanged: (current, previous, directlyActivated) => { - activeDescendantRef.current = current - - if (current && scrollContainerRef.current && directlyActivated) { - scrollIntoView(current, scrollContainerRef.current, menuScrollMargins) - } - } - }, - [ - // List ref isn't set while loading. Need to re-bind focus zone when it changes - loading - ] - ) - useEffect(() => { // if items changed, we want to instantly move active descendant into view if (activeDescendantRef.current && scrollContainerRef.current) { @@ -119,8 +97,10 @@ export function FilteredActionList({ placeholder={placeholderText} aria-label={placeholderText} aria-controls={listId} + aria-describedby={inputDescriptionTextId} {...textInputProps} /> + Items will be filtered as you type {loading ? ( @@ -128,7 +108,7 @@ export function FilteredActionList({ ) : ( - + )} diff --git a/src/SelectPanel/SelectPanel.tsx b/src/SelectPanel/SelectPanel.tsx index 7226bc63126..1d39cd2765e 100644 --- a/src/SelectPanel/SelectPanel.tsx +++ b/src/SelectPanel/SelectPanel.tsx @@ -1,7 +1,8 @@ -import React, {useCallback, useMemo} from 'react' +import React, {useCallback, useMemo, useState} from 'react' import {FilteredActionList, FilteredActionListProps} from '../FilteredActionList' import {OverlayProps} from '../Overlay' import {ItemInput} from '../deprecated/ActionList/List' +import Heading from '../Heading' import {FocusZoneHookSettings} from '../hooks/useFocusZone' import {DropdownButton} from '../deprecated/DropdownMenu' import {ItemProps} from '../deprecated/ActionList' @@ -10,6 +11,12 @@ import {TextInputProps} from '../TextInput' import {useProvidedStateOrCreate} from '../hooks/useProvidedStateOrCreate' import {AnchoredOverlayWrapperAnchorProps} from '../AnchoredOverlay/AnchoredOverlay' import {useProvidedRefOrCreate} from '../hooks' +import {Button} from '../Button' +import {SearchIcon} from '@primer/octicons-react' +import Box from '../Box' +import {useSSRSafeId} from '@react-aria/ssr' +import VisuallyHidden from '../_VisuallyHidden' +import ButtonClose from '../deprecated/Button/ButtonClose' interface SelectPanelSingleSelection { selected: ItemInput | undefined @@ -26,7 +33,11 @@ interface SelectPanelBaseProps { open: boolean, gesture: 'anchor-click' | 'anchor-key-press' | 'click-outside' | 'escape' | 'selection' ) => void - placeholder?: string + // TODO: Make `title` and `inputLabel` required and remove default values + // in the next major release + title?: string + inputLabel?: string + inputPlaceholder?: string overlayProps?: Partial } @@ -52,15 +63,17 @@ export function SelectPanel({ onOpenChange, renderAnchor = props => , anchorRef: externalAnchorRef, - placeholder, selected, onSelectedChange, + title = isMultiSelectVariant(selected) ? 'Select items' : 'Select an item', + inputLabel = 'Filter items', + inputPlaceholder, filterValue: externalFilterValue, onFilterChange: externalOnFilterChange, items, textInputProps, overlayProps, - sx, + sx: sxProp, ...listProps }: SelectPanelProps): JSX.Element { const [filterValue, setInternalFilterValue] = useProvidedStateOrCreate(externalFilterValue, undefined, '') @@ -72,6 +85,17 @@ export function SelectPanel({ [externalOnFilterChange, setInternalFilterValue] ) + const selectedItems = React.useMemo( + () => (Array.isArray(selected) ? selected : [...(selected ? [selected] : [])]), + [selected] + ) + + const [finalItemsSelected, setFinalItemsSelected] = useState(selectedItems) + + // Refresh the selected items state when the prop changes. + // This is necessary because sometimes the selected items need to be fetched async. + React.useEffect(() => setFinalItemsSelected(selectedItems), [selectedItems]) + const anchorRef = useProvidedRefOrCreate(externalAnchorRef) const onOpen: AnchoredOverlayProps['onOpen'] = useCallback(gesture => onOpenChange(true, gesture), [onOpenChange]) const onClose = useCallback( @@ -86,24 +110,20 @@ export function SelectPanel({ return null } - const selectedItems = Array.isArray(selected) ? selected : [...(selected ? [selected] : [])] - return >(props: T) => { return renderAnchor({ ...props, - children: selectedItems.length ? selectedItems.map(item => item.text).join(', ') : placeholder + children: selectedItems.length ? selectedItems.map(item => item.text).join(', ') : inputLabel }) } - }, [placeholder, renderAnchor, selected]) + }, [inputLabel, renderAnchor, selectedItems]) const itemsToRender = useMemo(() => { return items.map(item => { - const isItemSelected = isMultiSelectVariant(selected) ? selected.includes(item) : selected === item - return { ...item, role: 'option', - selected: 'selected' in item && item.selected === undefined ? undefined : isItemSelected, + selected: 'selected' in item && item.selected === undefined ? undefined : finalItemsSelected.includes(item), onAction: (itemFromAction, event) => { item.onAction?.(itemFromAction, event) @@ -112,24 +132,47 @@ export function SelectPanel({ } if (isMultiSelectVariant(selected)) { - const otherSelectedItems = selected.filter(selectedItem => selectedItem !== item) - const newSelectedItems = selected.includes(item) ? otherSelectedItems : [...otherSelectedItems, item] + const otherSelectedItems = finalItemsSelected.filter(selectedItem => selectedItem !== item) + const newSelectedItems = finalItemsSelected.includes(item) + ? otherSelectedItems + : [...otherSelectedItems, item] - const multiSelectOnChange = onSelectedChange as SelectPanelMultiSelection['onSelectedChange'] - multiSelectOnChange(newSelectedItems) - return + setFinalItemsSelected(newSelectedItems) + } else { + // single select + setFinalItemsSelected(finalItemsSelected.includes(item) ? [] : [item]) } - - // single select - const singleSelectOnChange = onSelectedChange as SelectPanelSingleSelection['onSelectedChange'] - singleSelectOnChange(item === selected ? undefined : item) - onClose('selection') } } as ItemProps }) - }, [onClose, onSelectedChange, items, selected]) + }, [items, selected, setFinalItemsSelected, finalItemsSelected]) + + const onSaveClickHandler = React.useCallback(() => { + if (isMultiSelectVariant(selected)) { + const multiSelectOnChange = onSelectedChange as SelectPanelMultiSelection['onSelectedChange'] + multiSelectOnChange(finalItemsSelected) + } else { + const singleSelectOnChange = onSelectedChange as SelectPanelSingleSelection['onSelectedChange'] + singleSelectOnChange(finalItemsSelected.length > 0 ? finalItemsSelected[0] : undefined) + } + onClose('selection') + }, [finalItemsSelected, onSelectedChange, onClose, selected]) + + const onCloseOverlay = React.useCallback( + (gesture?: 'anchor-click' | 'click-outside' | 'escape') => { + setFinalItemsSelected(selectedItems) + onClose(gesture ?? 'escape') + }, + [onClose, selectedItems] + ) + + const onCloseClickHandler = React.useCallback(() => { + setFinalItemsSelected(selectedItems) + onClose('escape') + }, [onClose, selectedItems]) const inputRef = React.useRef(null) + const titleId = useSSRSafeId() const focusTrapSettings = { initialFocusRef: inputRef } @@ -138,9 +181,22 @@ export function SelectPanel({ return { sx: {m: 2}, contrast: true, + leadingVisual: SearchIcon, + 'aria-label': inputLabel, + placeholder: inputPlaceholder, ...textInputProps } - }, [textInputProps]) + }, [textInputProps, inputLabel, inputPlaceholder]) + + const overlayKeyPressHandler = useCallback( + event => { + if (!event.defaultPrevented && ['Enter'].includes(event.key)) { + onSaveClickHandler() + event.preventDefault() + } + }, + [onSaveClickHandler] + ) return ( - + {/* inheriting height and maxHeight ensures that the content is never taller than the Overlay (which would break scrolling the items) */} + + + {filterValue === '' + ? 'Showing all items' + : items.length <= 0 + ? 'No matching items' + : `${items.length} matching ${items.length === 1 ? 'item' : 'items'}`} + + + + {title} + + + + + + + + + ) } diff --git a/src/__tests__/SelectPanel.test.tsx b/src/__tests__/SelectPanel.test.tsx index b88cb8f3149..75d396f22ff 100644 --- a/src/__tests__/SelectPanel.test.tsx +++ b/src/__tests__/SelectPanel.test.tsx @@ -23,7 +23,8 @@ function SimpleSelectPanel(): JSX.Element { null} items={[]} @@ -18,7 +19,8 @@ export function shouldAcceptCallWithNoProps() { export function shouldAcceptDOMPropsOnOverlay() { return ( null} items={[]} diff --git a/src/__tests__/__snapshots__/Autocomplete.test.tsx.snap b/src/__tests__/__snapshots__/Autocomplete.test.tsx.snap index 43c754dffc5..baf09f32c61 100644 --- a/src/__tests__/__snapshots__/Autocomplete.test.tsx.snap +++ b/src/__tests__/__snapshots__/Autocomplete.test.tsx.snap @@ -500,6 +500,8 @@ Array [ .c2 { margin-top: 8px; margin-bottom: 8px; + list-style: none; + padding-inline-start: 0; } .c8 { @@ -665,6 +667,9 @@ Array [ .c1 { font-size: 14px; line-height: 20px; + padding-inline-start: 0; + margin-block-start: 0; + margin-block-end: 0; } .c1[data-has-active-descendant], @@ -684,629 +689,636 @@ Array [ className="c0" >
-
-
+
    -
    -
    - -
    -
    -
    - - zero - -
    -
    -
    -
    -
    - -
    -
    -
    -
    +
    - one - -
    -
    -
    -
    -
    - + zero + +
    +
    + +
  • - - - -
-
-
- - two - -
-
-
-
-
- -
-
-
-
+
- three - -
-
-
-
-
- -
-
-
+ + one + +
+
+ +
  • - - four - -
    • -
    - -
    -
    - -
    -
    -
    -
    +
    - five - -
    -
    -
    -
    -
    - -
    -
    -
    + + two + +
    +
    + +
  • - - six - -
    • - - -
    -
    - +
    +
    +
    + + three + +
    +
    + +
  • - - - -
    • -
    -
    + +
    +
    +
    + + four + +
    +
    + +
  • - - seven - -
    • - - -
    -
    - +
    +
    +
    + + five + +
    +
    + +
  • - - - -
    • -
    -
    + +
    +
    +
    + + six + +
    +
    + +
  • - - twenty - -
    • - - -
    -
    - +
    +
    +
    + + seven + +
    +
    + +
  • - - - -
    • -
    -
    + +
    +
    +
    + + twenty + +
    +
    + +
  • - - twentyone - -
    • - - -
    -
    -
    -
    - +
    +
    + + twentyone + +
    +
    + +
  • - - Add new item - -
    • -
    - - +
    +
    + + Add new item + +
    +
    + + + + , @@ -1458,6 +1470,8 @@ Array [ .c2 { margin-top: 8px; margin-bottom: 8px; + list-style: none; + padding-inline-start: 0; } .c8 { @@ -1586,6 +1600,9 @@ Array [ .c1 { font-size: 14px; line-height: 20px; + padding-inline-start: 0; + margin-block-start: 0; + margin-block-end: 0; } .c1[data-has-active-descendant], @@ -1605,576 +1622,583 @@ Array [ className="c0" >
    -
    -
    +
      -
      -
      - -
      -
      -
      - - zero - -
      -
      -
      -
      -
      - -
      -
      -
      -
      +
      - one - -
      -
      -
      -
      -
      - -
      -
      -
      + + zero + +
      +
      + +
    • - - two - -
      • -
    -
    -
    -
    - -
    -
    -
    -
    +
    - three - -
    -
    -
    -
    -
    - -
    -
    -
    + + one + +
    +
    + +
  • - - four - -
    • -
    - -
    -
    - -
    -
    -
    -
    +
    - five - -
    -
    -
    -
    -
    - + two + +
    +
    + +
  • - - - - -
    -
    - - six - -
    -
    - -
    -
    - +
    +
    +
    + + three + +
    +
    +
    • +
  • - - - - -
    -
    + +
    +
    +
    + + four + +
    +
    +
    • +
  • - - seven - - - - -
    -
    - +
    +
    +
    + + five + +
    +
    +
    • +
  • - - - - -
    -
    + +
    +
    +
    + + six + +
    +
    +
    • +
  • - - twenty - - - - -
    -
    - +
    +
    +
    + + seven + +
    +
    +
    • +
  • - - - - -
    -
    + +
    +
    +
    + + twenty + +
    +
    +
    • +
  • - + + +
    - twentyone - -
    - - - +
    + + twentyone + +
    + +
    • + + + , @@ -2326,6 +2350,8 @@ Array [ .c2 { margin-top: 8px; margin-bottom: 8px; + list-style: none; + padding-inline-start: 0; } .c8 { @@ -2465,6 +2491,9 @@ Array [ .c1 { font-size: 14px; line-height: 20px; + padding-inline-start: 0; + margin-block-start: 0; + margin-block-end: 0; } .c1[data-has-active-descendant], @@ -2484,576 +2513,583 @@ Array [ className="c0" >
    -
    -
    +
      -
      -
      - -
      -
      -
      - - zero - -
      -
      -
      -
      -
      - -
      -
      -
      -
      +
      - one - -
      -
      -
      -
      -
      - -
      -
      -
      + + zero + +
      +
      + +
    • - - two - -
      • -
    -
    -
    -
    - -
    -
    -
    -
    +
    - three - -
    -
    -
    -
    -
    - -
    -
    -
    + + one + +
    +
    + +
  • - - four - -
    • -
    - -
    -
    - -
    -
    -
    -
    +
    - five - -
    -
    -
    -
    -
    - -
    -
    -
    + + two + +
    +
    + +
  • - - six - -
    • - - -
    -
    - +
    +
    +
    + + three + +
    +
    + +
  • - - - -
    • -
    -
    + +
    +
    +
    + + four + +
    +
    + +
  • - - seven - -
    • - - -
    -
    - +
    +
    +
    + + five + +
    +
    + +
  • - - - -
    • -
    -
    + +
    +
    +
    + + six + +
    +
    + +
  • - - twenty - -
    • - - -
    -
    - +
    +
    +
    + + seven + +
    +
    + +
  • - - - -
    • -
    -
    + +
    +
    +
    + + twenty + +
    +
    + +
  • - + +
    • +
    - twentyone - -
    - - - +
    + + twentyone + +
    + + + + + , @@ -3205,6 +3241,8 @@ Array [ .c2 { margin-top: 8px; margin-bottom: 8px; + list-style: none; + padding-inline-start: 0; } .c6 { @@ -3301,6 +3339,9 @@ Array [ .c1 { font-size: 14px; line-height: 20px; + padding-inline-start: 0; + margin-block-start: 0; + margin-block-end: 0; } .c1[data-has-active-descendant], @@ -3320,306 +3361,313 @@ Array [ className="c0" >
    -
    -
    +
      -
      -
      -
      - - zero - -
      -
      -
      -
      -
      -
      + + zero + +
      +
      + +
    • - - one - -
      • -
    -
    -
    -
    -
    + + one + +
    +
    + +
  • - - two - -
    • -
    - -
    -
    -
    + + two + +
    +
    + +
  • - - three - -
    • - - -
    -
    -
    + + three + +
    +
    + +
  • - - four - -
    • - - -
    -
    -
    + + four + +
    +
    + +
  • - - five - -
    • - - -
    -
    -
    + + five + +
    +
    + +
  • - - six - -
    • - - -
    -
    -
    + + six + +
    +
    + +
  • - - seven - -
    • - - -
    -
    -
    + + seven + +
    +
    + +
  • - - twenty - -
    • - - -
    -
    -
    + + twenty + +
    +
    + +
  • - - twentyone - -
    • - - - +
    + + twentyone + +
    + + + + + , @@ -3648,6 +3696,8 @@ Array [ .c2 { margin-top: 8px; margin-bottom: 8px; + list-style: none; + padding-inline-start: 0; } .c6 { @@ -3744,6 +3794,9 @@ Array [ .c1 { font-size: 14px; line-height: 20px; + padding-inline-start: 0; + margin-block-start: 0; + margin-block-end: 0; } .c1[data-has-active-descendant], @@ -3763,306 +3816,313 @@ Array [ className="c0" >
    -
    -
    +
      -
      -
      -
      - - zero - -
      -
      -
      -
      -
      -
      + + zero + +
      +
      + +
    • - - one - -
      • -
    -
    -
    -
    -
    + + one + +
    +
    + +
  • - - two - -
    • -
    - -
    -
    -
    + + two + +
    +
    + +
  • - - three - -
    • - - -
    -
    -
    + + three + +
    +
    + +
  • - - four - -
    • - - -
    -
    -
    + + four + +
    +
    + +
  • - - five - -
    • - - -
    -
    -
    + + five + +
    +
    + +
  • - - six - -
    • - - -
    -
    -
    + + six + +
    +
    + +
  • - - seven - -
    • - - -
    -
    -
    + + seven + +
    +
    + +
  • - - twenty - -
    • - - -
    -
    -
    + + twenty + +
    +
    + +
  • - - twentyone - -
    • - - - +
    + + twentyone + +
    + + + + + , diff --git a/src/__tests__/__snapshots__/SelectPanel.test.tsx.snap b/src/__tests__/__snapshots__/SelectPanel.test.tsx.snap index 6a30383ef8d..af2bfe92498 100644 --- a/src/__tests__/__snapshots__/SelectPanel.test.tsx.snap +++ b/src/__tests__/__snapshots__/SelectPanel.test.tsx.snap @@ -86,7 +86,7 @@ exports[`SelectPanel renders consistently 1`] = `