Skip to content

Commit

Permalink
Revert "Refactor FilteredActionList to address a11y violations and us…
Browse files Browse the repository at this point in the history
…e new ActionList. (#3247)" (#3349)

This reverts commit e865e3e.
  • Loading branch information
joshblack authored Jun 6, 2023
1 parent 9669267 commit a6b4169
Show file tree
Hide file tree
Showing 12 changed files with 127 additions and 165 deletions.
5 changes: 0 additions & 5 deletions .changeset/weak-jokes-chew.md

This file was deleted.

26 changes: 14 additions & 12 deletions docs/content/SelectPanel.mdx
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
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
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
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
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
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
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

0 comments on commit a6b4169

Please sign in to comment.