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.

Sign up for GitHub

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

Revert "Refactor FilteredActionList to address a11y violations and use new ActionList." #3349

Merged
merged 1 commit into from
Jun 6, 2023
Merged

Revert "Refactor FilteredActionList to address a11y violations and use new ActionList." #3349

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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: 0 additions & 5 deletions .changeset/weak-jokes-chew.md
View file
Open in desktop

This file was deleted.

26 changes: 14 additions & 12 deletions docs/content/SelectPanel.mdx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,18 +12,20 @@ A `SelectPanel` provides an anchor that will open an overlay with a list of sele

```javascript live noinline
function getColorCircle(color) {
return (
<Box
borderWidth="1px"
borderStyle="solid"
bg={color}
borderColor={color}
width={14}
height={14}
borderRadius={10}
margin="auto"
/>
)
return function () {
return (
<Box
borderWidth="1px"
borderStyle="solid"
bg={color}
borderColor={color}
width={14}
height={14}
borderRadius={10}
margin="auto"
/>
)
}
}

const items = [
Expand Down
2 changes: 1 addition & 1 deletion generated/components.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3626,7 +3626,7 @@
"stories": [
{
"id": "components-selectpanel--default",
"code": "() => {\n const [selected, setSelected] = React.useState<ItemInput[]>([\n items[0],\n items[1],\n ])\n const [filter, setFilter] = React.useState('')\n const filteredItems = items.filter((item) =>\n item.text.toLowerCase().startsWith(filter.toLowerCase()),\n )\n const [open, setOpen] = useState(false)\n return (\n <>\n <h1>Multi Select Panel</h1>\n <div>Please select labels that describe your issue:</div>\n <SelectPanel\n title=\"Select labels\"\n renderAnchor={({\n children,\n 'aria-labelledby': ariaLabelledBy,\n ...anchorProps\n }) => (\n <Button\n trailingAction={TriangleDownIcon}\n aria-labelledby={` ${ariaLabelledBy}`}\n {...anchorProps}\n >\n {children ?? 'Select Labels'}\n </Button>\n )}\n placeholderText=\"Filter labels\"\n open={open}\n onOpenChange={setOpen}\n items={filteredItems}\n selected={selected}\n onSelectedChange={setSelected}\n onFilterChange={setFilter}\n overlayProps={{\n width: 'small',\n height: 'xsmall',\n }}\n />\n </>\n )\n}"
"code": "() => {\n const [selected, setSelected] = React.useState<ItemInput[]>([\n items[0],\n items[1],\n ])\n const [filter, setFilter] = React.useState('')\n const filteredItems = items.filter((item) =>\n item.text.toLowerCase().startsWith(filter.toLowerCase()),\n )\n const [open, setOpen] = useState(false)\n return (\n <>\n <h1>Multi Select Panel</h1>\n <div>Please select labels that describe your issue:</div>\n <SelectPanel\n title=\"Select labels\"\n renderAnchor={({\n children,\n 'aria-labelledby': ariaLabelledBy,\n ...anchorProps\n }) => (\n <Button\n trailingAction={TriangleDownIcon}\n aria-labelledby={` ${ariaLabelledBy}`}\n {...anchorProps}\n >\n {children ?? 'Select Labels'}\n </Button>\n )}\n placeholderText=\"Filter labels\"\n open={open}\n onOpenChange={setOpen}\n items={filteredItems}\n selected={selected}\n onSelectedChange={setSelected}\n onFilterChange={setFilter}\n showItemDividers={true}\n overlayProps={{\n width: 'small',\n height: 'xsmall',\n }}\n />\n </>\n )\n}"
}
],
"props": [
Expand Down
3 changes: 1 addition & 2 deletions src/ActionList/List.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,7 @@ export const List = React.forwardRef<HTMLUListElement, ActionListProps>(
value={{
variant,
selectionVariant: selectionVariant || containerSelectionVariant,
// @ts-ignore showItemDividers may be passed by some components until next major.
showDividers: showDividers || !!props.showItemDividers,
showDividers,
role: role || listRole,
headingId,
}}
Expand Down
46 changes: 24 additions & 22 deletions src/FilteredActionList/FilteredActionList.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import {Meta} from '@storybook/react'
import React from 'react'
import {ThemeProvider} from '..'
import {FilteredActionList, ItemInput} from '../FilteredActionList'
import {FilteredActionList} from '../FilteredActionList'
import BaseStyles from '../BaseStyles'
import Box from '../Box'

Expand All @@ -26,33 +26,35 @@ const meta: Meta = {
export default meta

function getColorCircle(color: string) {
return (
<Box
bg={color}
borderColor={color}
width={14}
height={14}
borderRadius={10}
margin="auto"
borderWidth="1px"
borderStyle="solid"
/>
)
return function () {
return (
<Box
bg={color}
borderColor={color}
width={14}
height={14}
borderRadius={10}
margin="auto"
borderWidth="1px"
borderStyle="solid"
/>
)
}
}

const items = [
{leadingVisual: getColorCircle('#a2eeef'), text: 'enhancement', id: '1'},
{leadingVisual: getColorCircle('#d73a4a'), text: 'bug', id: '2'},
{leadingVisual: getColorCircle('#0cf478'), text: 'good first issue', id: '3'},
{leadingVisual: getColorCircle('#ffd78e'), text: 'design', id: '4'},
{leadingVisual: getColorCircle('#ff0000'), text: 'blocker', id: '5'},
{leadingVisual: getColorCircle('#a4f287'), text: 'backend', id: '6'},
{leadingVisual: getColorCircle('#8dc6fc'), text: 'frontend', id: '7'},
] as ItemInput[]
{leadingVisual: getColorCircle('#a2eeef'), text: 'enhancement', id: 1},
{leadingVisual: getColorCircle('#d73a4a'), text: 'bug', id: 2},
{leadingVisual: getColorCircle('#0cf478'), text: 'good first issue', id: 3},
{leadingVisual: getColorCircle('#ffd78e'), text: 'design', id: 4},
{leadingVisual: getColorCircle('#ff0000'), text: 'blocker', id: 5},
{leadingVisual: getColorCircle('#a4f287'), text: 'backend', id: 6},
{leadingVisual: getColorCircle('#8dc6fc'), text: 'frontend', id: 7},
]

export function Default(): JSX.Element {
const [filter, setFilter] = React.useState('')
const filteredItems = items.filter(item => item.text?.toLowerCase().startsWith(filter.toLowerCase()))
const filteredItems = items.filter(item => item.text.toLowerCase().startsWith(filter.toLowerCase()))

return (
<>
Expand Down
70 changes: 16 additions & 54 deletions src/FilteredActionList/FilteredActionList.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,70 +1,40 @@
import type {ScrollIntoViewOptions} from '@primer/behaviors'
import {scrollIntoView} from '@primer/behaviors'
import React, {KeyboardEventHandler, useCallback, useEffect, useRef} from 'react'
import TextInput, {TextInputProps} from '../TextInput'
import styled from 'styled-components'
import Box from '../Box'
import {ActionList, ActionListProps, ActionListItemProps} from '../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 {useId} from '../hooks/useId'
import {VisuallyHidden} from '../internal/components/VisuallyHidden'
import {SxProp} from '../sx'

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

export type ItemInput = Partial<
ActionListItemProps & {
description?: string | React.ReactElement
descriptionVariant?: 'inline' | 'block'
leadingVisual?: JSX.Element
onAction?: (itemFromAction: ItemInput, event: React.MouseEvent) => void
selected?: boolean
text?: string
trailingVisual?: string
}
>

export interface FilteredActionListProps extends ActionListProps, SxProp {
export interface FilteredActionListProps
extends Partial<Omit<GroupedListProps, keyof ListPropsBase>>,
ListPropsBase,
SxProp {
loading?: boolean
placeholderText?: string
filterValue?: string
onFilterChange: (value: string, e: React.ChangeEvent<HTMLInputElement>) => void
textInputProps?: Partial<Omit<TextInputProps, 'onChange'>>
inputRef?: React.RefObject<HTMLInputElement>
items: ItemInput[]
}

const StyledHeader = styled.div`
box-shadow: 0 1px 0 ${get('colors.border.default')};
z-index: 1;
`

const renderFn = ({
description,
descriptionVariant,
id,
sx,
text,
trailingVisual,
leadingVisual,
onSelect,
selected,
}: ItemInput): React.ReactElement => {
return (
<ActionList.Item key={id} sx={sx} role="option" onSelect={onSelect} selected={selected}>
{!!leadingVisual && <ActionList.LeadingVisual>{leadingVisual}</ActionList.LeadingVisual>}
<Box>{text ? text : null}</Box>
{description ? <ActionList.Description variant={descriptionVariant}>{description}</ActionList.Description> : null}
{!!trailingVisual && <ActionList.TrailingVisual>{trailingVisual}</ActionList.TrailingVisual>}
</ActionList.Item>
)
}

export function FilteredActionList({
loading = false,
placeholderText,
Expand All @@ -87,7 +57,7 @@ export function FilteredActionList({
)

const scrollContainerRef = useRef<HTMLDivElement>(null)
const listContainerRef = useRef<HTMLUListElement>(null)
const listContainerRef = useRef<HTMLDivElement>(null)
const inputRef = useProvidedRefOrCreate<HTMLInputElement>(providedInputRef)
const activeDescendantRef = useRef<HTMLElement>()
const listId = useId()
Expand All @@ -114,7 +84,7 @@ export function FilteredActionList({
return !(element instanceof HTMLInputElement)
},
activeDescendantFocus: inputRef,
onActiveDescendantChanged: (current, _previous, directlyActivated) => {
onActiveDescendantChanged: (current, previous, directlyActivated) => {
activeDescendantRef.current = current

if (current && scrollContainerRef.current && directlyActivated) {
Expand Down Expand Up @@ -162,15 +132,7 @@ export function FilteredActionList({
<Spinner />
</Box>
) : (
<ActionList
ref={listContainerRef}
{...listProps}
role="listbox"
id={listId}
aria-label={`${placeholderText} options`}
>
{items.map(i => renderFn(i))}
</ActionList>
<ActionList ref={listContainerRef} items={items} {...listProps} role="listbox" id={listId} />
)}
</Box>
</Box>
Expand Down
2 changes: 1 addition & 1 deletion src/FilteredActionList/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
export {FilteredActionList} from './FilteredActionList'
export type {FilteredActionListProps, ItemInput} from './FilteredActionList'
export type {FilteredActionListProps} from './FilteredActionList'
50 changes: 29 additions & 21 deletions src/SelectPanel/SelectPanel.features.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import {ComponentMeta} from '@storybook/react'

import Box from '../Box'
import {Button} from '../Button'
import {ItemInput} from '../FilteredActionList'
import {ItemInput} from '../deprecated/ActionList/List'
import {SelectPanel} from './SelectPanel'
import {TriangleDownIcon} from '@primer/octicons-react'
import type {OverlayProps} from '../Overlay'
Expand All @@ -14,28 +14,30 @@ export default {
} as ComponentMeta<typeof SelectPanel>

function getColorCircle(color: string) {
return (
<Box
bg={color}
borderColor={color}
width={14}
height={14}
borderRadius={10}
margin="auto"
borderWidth="1px"
borderStyle="solid"
/>
)
return function () {
return (
<Box
bg={color}
borderColor={color}
width={14}
height={14}
borderRadius={10}
margin="auto"
borderWidth="1px"
borderStyle="solid"
/>
)
}
}

const items = [
{leadingVisual: getColorCircle('#a2eeef'), text: 'enhancement', id: '1'},
{leadingVisual: getColorCircle('#d73a4a'), text: 'bug', id: '2'},
{leadingVisual: getColorCircle('#0cf478'), text: 'good first issue', id: '3'},
{leadingVisual: getColorCircle('#ffd78e'), text: 'design', id: '4'},
{leadingVisual: getColorCircle('#ff0000'), text: 'blocker', id: '5'},
{leadingVisual: getColorCircle('#a4f287'), text: 'backend', id: '6'},
{leadingVisual: getColorCircle('#8dc6fc'), text: 'frontend', id: '7'},
{leadingVisual: getColorCircle('#a2eeef'), text: 'enhancement', id: 1},
{leadingVisual: getColorCircle('#d73a4a'), text: 'bug', id: 2},
{leadingVisual: getColorCircle('#0cf478'), text: 'good first issue', id: 3},
{leadingVisual: getColorCircle('#ffd78e'), text: 'design', id: 4},
{leadingVisual: getColorCircle('#ff0000'), text: 'blocker', id: 5},
{leadingVisual: getColorCircle('#a4f287'), text: 'backend', id: 6},
{leadingVisual: getColorCircle('#8dc6fc'), text: 'frontend', id: 7},
]

export const SingleSelectStory = () => {
Expand All @@ -61,7 +63,6 @@ export const SingleSelectStory = () => {
selected={selected}
onSelectedChange={setSelected}
onFilterChange={setFilter}
showDividers={true}
showItemDividers={true}
overlayProps={{width: 'small', height: 'xsmall'}}
/>
Expand Down Expand Up @@ -93,6 +94,7 @@ export const ExternalAnchorStory = () => {
selected={selected}
onSelectedChange={setSelected}
onFilterChange={setFilter}
showItemDividers={true}
overlayProps={{width: 'small', height: 'xsmall'}}
/>
</>
Expand Down Expand Up @@ -123,6 +125,7 @@ export const SelectPanelHeightInitialWithOverflowingItemsStory = () => {
selected={selected}
onSelectedChange={setSelected}
onFilterChange={setFilter}
showItemDividers={true}
overlayProps={{width: 'small', height: 'initial', maxHeight: 'xsmall'}}
/>
</>
Expand Down Expand Up @@ -154,6 +157,7 @@ export const SelectPanelHeightInitialWithUnderflowingItemsStory = () => {
selected={selected}
onSelectedChange={setSelected}
onFilterChange={setFilter}
showItemDividers={true}
overlayProps={{width: 'small', height: 'initial', maxHeight: 'xsmall'}}
/>
</>
Expand Down Expand Up @@ -198,6 +202,7 @@ export const SelectPanelHeightInitialWithUnderflowingItemsAfterFetch = () => {
selected={selected}
onSelectedChange={setSelected}
onFilterChange={setFilter}
showItemDividers={true}
overlayProps={{width: 'small', height, maxHeight: 'xsmall'}}
/>
</>
Expand Down Expand Up @@ -229,6 +234,7 @@ export const SelectPanelAboveTallBody = () => {
selected={selected}
onSelectedChange={setSelected}
onFilterChange={setFilter}
showItemDividers={true}
overlayProps={{width: 'small', height: 'xsmall'}}
/>
<div
Expand Down Expand Up @@ -270,6 +276,7 @@ export const SelectPanelHeightAndScroll = () => {
selected={selectedA}
onSelectedChange={setSelectedA}
onFilterChange={setFilter}
showItemDividers={true}
overlayProps={{height: 'medium'}}
/>
<h2>With height:auto, maxheight:medium</h2>
Expand All @@ -286,6 +293,7 @@ export const SelectPanelHeightAndScroll = () => {
selected={selectedB}
onSelectedChange={setSelectedB}
onFilterChange={setFilter}
showItemDividers={true}
overlayProps={{
height: 'auto',
maxHeight: 'medium',
Expand Down
Loading