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.

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

Refactor FilteredActionList to address a11y violations and use new ActionList. #3247

Merged
merged 53 commits into from
May 26, 2023
Merged
Show file tree
Hide file tree
Changes from 46 commits
Commits
Show all changes
53 commits
Select commit Hold shift + click to select a range
127b7a6
Update FilteredActionList to use non-deprecated ActionList.
radglob Apr 27, 2023
353465d
Merge branch 'main' of github.com:primer/react into refactor-filtered…
radglob May 1, 2023
0ee48c6
Use non-deprecated ActionList in FilteredActionList.
radglob May 3, 2023
5558264
Fix a11y issues in FilteredActionList story.
radglob May 3, 2023
4525416
Add prop to hide selection component if needed.
radglob May 3, 2023
f67e7db
Remove unused hook import.
radglob May 3, 2023
c7e1ffd
Get SavedReplies to look as it did with deprecated ActionList.
radglob May 3, 2023
b4238fb
Fix failing test.
radglob May 3, 2023
40a08b5
Create weak-jokes-chew.md
radglob May 3, 2023
0ec32c1
Merge branch 'main' into refactor-filtered-action-list
radglob May 3, 2023
6f4fc14
Update generated/components.json
radglob May 3, 2023
adefbbe
Fix themePreval snapshot.
radglob May 3, 2023
30839b0
Linting fixes.
radglob May 3, 2023
6fb05d0
Fix type-check errors.
radglob May 3, 2023
470e250
Update themePreval snapshot again.
radglob May 3, 2023
3da704f
Fix themePreval snapshot to match origin. Unsure why it's not generat…
radglob May 3, 2023
069e747
Hide selections in MarkdownEditor saved replies.
radglob May 3, 2023
b152869
Merge branch 'main' into refactor-filtered-action-list
radglob May 8, 2023
82ba413
Merge branch 'main' into refactor-filtered-action-list
radglob May 8, 2023
1232337
Merge branch 'main' into refactor-filtered-action-list
radglob May 8, 2023
bd3ce03
Merge branch 'main' into refactor-filtered-action-list
radglob May 9, 2023
4f7f4d0
Merge branch 'main' into refactor-filtered-action-list
radglob May 9, 2023
dd6f3f6
Merge branch 'main' into refactor-filtered-action-list
radglob May 9, 2023
542240b
Merge branch 'main' into refactor-filtered-action-list
radglob May 10, 2023
19b3674
Remove hideSelection prop and add defaultRenderFn to FilteredActionLi…
radglob May 10, 2023
dc8b1bb
Fix selection rendering (needed explicit selected boolean) and fix Se…
radglob May 10, 2023
61694c6
Pass selectionVariant illegally to SelectPanel in src/MarkdownEditor/…
radglob May 10, 2023
ca4bfc3
Merge branch 'main' into refactor-filtered-action-list
radglob May 11, 2023
99281c2
Remove remaining references of hideSelection prop.
radglob May 11, 2023
83bdd99
Update changeset to reflect that changes impact SelectPanel.
radglob May 11, 2023
2021aab
Remove renderFn prop from SelectPanel and use default for FilteredAct…
radglob May 11, 2023
dca5ee7
Fix truncation in SavedReplies descriptions.
radglob May 11, 2023
936b08c
Update generated/components.json
radglob May 11, 2023
626efc4
Fix linting error.
radglob May 11, 2023
47968e0
Don't make renderFn a prop (if we need to make this configurable, we …
radglob May 12, 2023
c6f65b9
Merge branch 'main' into refactor-filtered-action-list
radglob May 12, 2023
13e256b
Use showDividers prop in SelectPanel story.
radglob May 12, 2023
de41c26
Merge branch 'main' into refactor-filtered-action-list
radglob May 15, 2023
15bf1a4
Merge branch 'main' into refactor-filtered-action-list
radglob May 15, 2023
a1897a6
Merge branch 'main' into refactor-filtered-action-list
radglob May 15, 2023
616d3a3
Merge branch 'main' into refactor-filtered-action-list
radglob May 16, 2023
90e395e
Merge branch 'main' into refactor-filtered-action-list
radglob May 16, 2023
e3edf33
Formatting.
radglob May 16, 2023
431bce9
Merge branch 'main' into refactor-filtered-action-list
radglob May 17, 2023
bd076ec
Merge branch 'main' into refactor-filtered-action-list
radglob May 19, 2023
1e2b1a9
Merge branch 'main' into refactor-filtered-action-list
radglob May 22, 2023
c361650
Add temporary support for showItemDividers prop to SelectPanel to kee…
radglob May 23, 2023
059b825
Merge branch 'main' into refactor-filtered-action-list
radglob May 23, 2023
a66b7cd
Merge branch 'main' into refactor-filtered-action-list
radglob May 23, 2023
09ce036
Merge branch 'main' of github.com:primer/react into refactor-filtered…
radglob May 25, 2023
80ccbef
Merge branch 'main' into refactor-filtered-action-list
radglob May 25, 2023
794faf4
Support passing deprecated showItemDividers prop in ActionList.
radglob May 25, 2023
4740613
Merge branch 'main' into refactor-filtered-action-list
radglob May 26, 2023
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/weak-jokes-chew.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@primer/react": patch
---

FilteredActionList now uses new ActionList as a base, and SelectPanel reflects those changes.
26 changes: 12 additions & 14 deletions docs/content/SelectPanel.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -12,20 +12,18 @@ A `SelectPanel` provides an anchor that will open an overlay with a list of sele

```javascript live noinline
function getColorCircle(color) {
return function () {
return (
<Box
borderWidth="1px"
borderStyle="solid"
bg={color}
borderColor={color}
width={14}
height={14}
borderRadius={10}
margin="auto"
/>
)
}
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
Original file line number Diff line number Diff line change
Expand Up @@ -3638,7 +3638,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 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}"
"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 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}"
}
],
"props": [
Expand Down
46 changes: 22 additions & 24 deletions src/FilteredActionList/FilteredActionList.stories.tsx
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} from '../FilteredActionList'
import {FilteredActionList, ItemInput} from '../FilteredActionList'
import BaseStyles from '../BaseStyles'
import Box from '../Box'

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

function getColorCircle(color: string) {
return function () {
return (
<Box
bg={color}
borderColor={color}
width={14}
height={14}
borderRadius={10}
margin="auto"
borderWidth="1px"
borderStyle="solid"
/>
)
}
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'},
] as ItemInput[]

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
56 changes: 47 additions & 9 deletions src/FilteredActionList/FilteredActionList.tsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import React, {KeyboardEventHandler, useCallback, useEffect, useRef} from 'react'
import {GroupedListProps, ListPropsBase} from '../deprecated/ActionList/List'
import TextInput, {TextInputProps} from '../TextInput'
import Box from '../Box'
import {ActionList} from '../deprecated/ActionList'
import {ActionList, ActionListProps, ActionListItemProps} from '../ActionList'
import Spinner from '../Spinner'
import {useFocusZone} from '../hooks/useFocusZone'
import {useProvidedStateOrCreate} from '../hooks/useProvidedStateOrCreate'
Expand All @@ -17,23 +16,54 @@ import {useId} from '../hooks/useId'

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

export interface FilteredActionListProps
extends Partial<Omit<GroupedListProps, keyof ListPropsBase>>,
ListPropsBase,
SxProp {
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 {
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 @@ -56,7 +86,7 @@ export function FilteredActionList({
)

const scrollContainerRef = useRef<HTMLDivElement>(null)
const listContainerRef = useRef<HTMLDivElement>(null)
const listContainerRef = useRef<HTMLUListElement>(null)
radglob marked this conversation as resolved.
Show resolved Hide resolved
const inputRef = useProvidedRefOrCreate<HTMLInputElement>(providedInputRef)
const activeDescendantRef = useRef<HTMLElement>()
const listId = useId()
Expand All @@ -82,7 +112,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 @@ -128,7 +158,15 @@ export function FilteredActionList({
<Spinner />
</Box>
) : (
<ActionList ref={listContainerRef} items={items} {...listProps} role="listbox" id={listId} />
<ActionList
ref={listContainerRef}
{...listProps}
role="listbox"
id={listId}
aria-label={`${placeholderText} options`}
>
{items.map(i => renderFn(i))}
</ActionList>
)}
</Box>
</Box>
Expand Down
2 changes: 1 addition & 1 deletion src/FilteredActionList/index.ts
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
export {FilteredActionList} from './FilteredActionList'
export type {FilteredActionListProps} from './FilteredActionList'
export type {FilteredActionListProps, ItemInput} from './FilteredActionList'
51 changes: 21 additions & 30 deletions src/SelectPanel/SelectPanel.features.stories.tsx
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 '../deprecated/ActionList/List'
import {ItemInput} from '../FilteredActionList'
import {SelectPanel} from './SelectPanel'
import {TriangleDownIcon} from '@primer/octicons-react'
import type {OverlayProps} from '../Overlay'
Expand All @@ -14,30 +14,28 @@ export default {
} as ComponentMeta<typeof SelectPanel>

function getColorCircle(color: string) {
return function () {
return (
<Box
bg={color}
borderColor={color}
width={14}
height={14}
borderRadius={10}
margin="auto"
borderWidth="1px"
borderStyle="solid"
/>
)
}
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 @@ -63,7 +61,7 @@ export const SingleSelectStory = () => {
selected={selected}
onSelectedChange={setSelected}
onFilterChange={setFilter}
showItemDividers={true}
radglob marked this conversation as resolved.
Show resolved Hide resolved
showDividers={true}
overlayProps={{width: 'small', height: 'xsmall'}}
/>
</>
Expand Down Expand Up @@ -94,7 +92,6 @@ export const ExternalAnchorStory = () => {
selected={selected}
onSelectedChange={setSelected}
onFilterChange={setFilter}
showItemDividers={true}
overlayProps={{width: 'small', height: 'xsmall'}}
/>
</>
Expand Down Expand Up @@ -125,7 +122,6 @@ export const SelectPanelHeightInitialWithOverflowingItemsStory = () => {
selected={selected}
onSelectedChange={setSelected}
onFilterChange={setFilter}
showItemDividers={true}
overlayProps={{width: 'small', height: 'initial', maxHeight: 'xsmall'}}
/>
</>
Expand Down Expand Up @@ -157,7 +153,6 @@ export const SelectPanelHeightInitialWithUnderflowingItemsStory = () => {
selected={selected}
onSelectedChange={setSelected}
onFilterChange={setFilter}
showItemDividers={true}
overlayProps={{width: 'small', height: 'initial', maxHeight: 'xsmall'}}
/>
</>
Expand Down Expand Up @@ -202,7 +197,6 @@ export const SelectPanelHeightInitialWithUnderflowingItemsAfterFetch = () => {
selected={selected}
onSelectedChange={setSelected}
onFilterChange={setFilter}
showItemDividers={true}
overlayProps={{width: 'small', height, maxHeight: 'xsmall'}}
/>
</>
Expand Down Expand Up @@ -234,7 +228,6 @@ export const SelectPanelAboveTallBody = () => {
selected={selected}
onSelectedChange={setSelected}
onFilterChange={setFilter}
showItemDividers={true}
overlayProps={{width: 'small', height: 'xsmall'}}
/>
<div
Expand Down Expand Up @@ -276,7 +269,6 @@ export const SelectPanelHeightAndScroll = () => {
selected={selectedA}
onSelectedChange={setSelectedA}
onFilterChange={setFilter}
showItemDividers={true}
overlayProps={{height: 'medium'}}
/>
<h2>With height:auto, maxheight:medium</h2>
Expand All @@ -293,7 +285,6 @@ export const SelectPanelHeightAndScroll = () => {
selected={selectedB}
onSelectedChange={setSelectedB}
onFilterChange={setFilter}
showItemDividers={true}
overlayProps={{
height: 'auto',
maxHeight: 'medium',
Expand Down
Loading