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

Remove deprecated ActionList from SelectPanel and FilteredActionList. #3538

7 changes: 7 additions & 0 deletions .changeset/fluffy-waves-kiss.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@primer/react": minor
---

Remove deprecated ActionList from SelectPanel and FilteredActionList.

<!-- Changed components: SelectPanel -->
7 changes: 4 additions & 3 deletions src/ActionList/Item.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -178,9 +178,10 @@ export const Item = React.forwardRef<HTMLLIElement, ActionListItemProps>(
)

// use props.id if provided, otherwise generate one.
const labelId = useId(id)
const inlineDescriptionId = useId(id && `${id}--inline-description`)
const blockDescriptionId = useId(id && `${id}--block-description`)
const _id = id !== undefined ? id.toString() : undefined
const labelId = useId(_id)
const inlineDescriptionId = useId(_id && `${_id}--inline-description`)
const blockDescriptionId = useId(_id && `${_id}--block-description`)

const ItemWrapper = _PrivateItemWrapper || React.Fragment

Expand Down
2 changes: 1 addition & 1 deletion src/ActionList/shared.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ export type ActionListItemProps = {
/**
* id to attach to the root element of the Item
*/
id?: string
id?: string | number
/**
* Private API for use internally only. Used by LinkItem to wrap contents in an anchor
*/
Expand Down
14 changes: 8 additions & 6 deletions src/Autocomplete/AutocompleteMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ function AutocompleteMenu<T extends AutocompleteItemProps>(props: AutocompleteMe
const listContainerRef = useRef<HTMLDivElement>(null)
const allItemsToRenderRef = useRef<T[]>([])
const [highlightedItem, setHighlightedItem] = useState<T>()
const [sortedItemIds, setSortedItemIds] = useState<Array<string>>(items.map(({id: itemId}) => itemId))
const [sortedItemIds, setSortedItemIds] = useState<Array<string>>(items.map(({id: itemId}) => itemId.toString()))
const generatedUniqueId = useSSRSafeId(id)

const selectableItems = useMemo(
Expand All @@ -160,18 +160,19 @@ function AutocompleteMenu<T extends AutocompleteItemProps>(props: AutocompleteMe
role: 'option',
id: selectableItem.id,
active: highlightedItem?.id === selectableItem.id,
selected: selectionVariant === 'multiple' ? selectedItemIds.includes(selectableItem.id) : undefined,
selected:
selectionVariant === 'multiple' ? selectedItemIds.includes(selectableItem.id.toString()) : undefined,
onAction: (item: T) => {
const otherSelectedItemIds = selectedItemIds.filter(selectedItemId => selectedItemId !== item.id)
const newSelectedItemIds = selectedItemIds.includes(item.id)
const newSelectedItemIds = selectedItemIds.includes(item.id.toString())
? otherSelectedItemIds
: [...otherSelectedItemIds, item.id]
const onSelectedChangeFn = onSelectedChange
? onSelectedChange
: getdefaultCheckedSelectionChange(setInputValue)

onSelectedChangeFn(
newSelectedItemIds.map(newSelectedItemId => getItemById(newSelectedItemId, items)) as T[],
newSelectedItemIds.map(newSelectedItemId => getItemById(newSelectedItemId.toString(), items)) as T[],
)

if (selectionVariant === 'multiple') {
Expand Down Expand Up @@ -228,7 +229,8 @@ function AutocompleteMenu<T extends AutocompleteItemProps>(props: AutocompleteMe
role: 'option',
key: addNewItem.id,
active: highlightedItem?.id === addNewItem.id,
selected: selectionVariant === 'multiple' ? selectedItemIds.includes(addNewItem.id) : undefined,
selected:
selectionVariant === 'multiple' ? selectedItemIds.includes(addNewItem.id.toString()) : undefined,
leadingVisual: () => <PlusIcon />,
onAction: (item: T) => {
// TODO: make it possible to pass a leadingVisual when using `addNewItem`
Expand Down Expand Up @@ -289,7 +291,7 @@ function AutocompleteMenu<T extends AutocompleteItemProps>(props: AutocompleteMe
)

useEffect(() => {
if (highlightedItem?.text?.startsWith(inputValue) && !selectedItemIds.includes(highlightedItem.id)) {
if (highlightedItem?.text?.startsWith(inputValue) && !selectedItemIds.includes(highlightedItem.id.toString())) {
setAutocompleteSuggestion(highlightedItem.text)
} else {
setAutocompleteSuggestion('')
Expand Down
14 changes: 7 additions & 7 deletions src/FilteredActionList/FilteredActionList.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -43,13 +43,13 @@ function getColorCircle(color: string) {
}

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 function Default(): JSX.Element {
Expand Down
69 changes: 60 additions & 9 deletions src/FilteredActionList/FilteredActionList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,35 +6,78 @@ import Box from '../Box'
import Spinner from '../Spinner'
import TextInput, {TextInputProps} from '../TextInput'
import {get} from '../constants'
import {ActionList} from '../deprecated/ActionList'
import {GroupedListProps, ListPropsBase} from '../deprecated/ActionList/List'
import {ActionList, ActionListProps, ActionListItemProps} from '../ActionList'
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 {VisuallyHidden} from '../internal/components/VisuallyHidden'
import {SxProp} from '../sx'
import {isValidElementType} from 'react-is'

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?: React.ElementType
onAction?: (itemFromAction: ItemInput, event: React.MouseEvent) => void
selected?: boolean
text?: string
trailingVisual?: React.ElementType | React.ReactNode
}
>

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: TrailingVisual,
leadingVisual: 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>
{typeof TrailingVisual !== 'string' && isValidElementType(TrailingVisual) ? (
<TrailingVisual />
) : (
TrailingVisual
)}
</ActionList.TrailingVisual>
)}
</ActionList.Item>
)
}

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

const scrollContainerRef = useRef<HTMLDivElement>(null)
const listContainerRef = useRef<HTMLDivElement>(null)
const listContainerRef = useRef<HTMLUListElement>(null)
const inputRef = useProvidedRefOrCreate<HTMLInputElement>(providedInputRef)
const activeDescendantRef = useRef<HTMLElement>()
const listId = useId()
Expand All @@ -84,7 +127,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 @@ -132,7 +175,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(renderFn)}
</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'
32 changes: 16 additions & 16 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 Down Expand Up @@ -31,13 +31,13 @@ function getColorCircle(color: string) {
}

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 +63,7 @@ export const SingleSelectStory = () => {
selected={selected}
onSelectedChange={setSelected}
onFilterChange={setFilter}
showItemDividers={true}
showDividers={true}
overlayProps={{width: 'small', height: 'xsmall'}}
/>
</>
Expand Down Expand Up @@ -94,7 +94,7 @@ export const ExternalAnchorStory = () => {
selected={selected}
onSelectedChange={setSelected}
onFilterChange={setFilter}
showItemDividers={true}
showDividers={true}
overlayProps={{width: 'small', height: 'xsmall'}}
/>
</>
Expand Down Expand Up @@ -125,7 +125,7 @@ export const SelectPanelHeightInitialWithOverflowingItemsStory = () => {
selected={selected}
onSelectedChange={setSelected}
onFilterChange={setFilter}
showItemDividers={true}
showDividers={true}
overlayProps={{width: 'small', height: 'initial', maxHeight: 'xsmall'}}
/>
</>
Expand Down Expand Up @@ -157,7 +157,7 @@ export const SelectPanelHeightInitialWithUnderflowingItemsStory = () => {
selected={selected}
onSelectedChange={setSelected}
onFilterChange={setFilter}
showItemDividers={true}
showDividers={true}
overlayProps={{width: 'small', height: 'initial', maxHeight: 'xsmall'}}
/>
</>
Expand Down Expand Up @@ -202,7 +202,7 @@ export const SelectPanelHeightInitialWithUnderflowingItemsAfterFetch = () => {
selected={selected}
onSelectedChange={setSelected}
onFilterChange={setFilter}
showItemDividers={true}
showDividers={true}
overlayProps={{width: 'small', height, maxHeight: 'xsmall'}}
/>
</>
Expand Down Expand Up @@ -234,7 +234,7 @@ export const SelectPanelAboveTallBody = () => {
selected={selected}
onSelectedChange={setSelected}
onFilterChange={setFilter}
showItemDividers={true}
showDividers={true}
overlayProps={{width: 'small', height: 'xsmall'}}
/>
<div
Expand Down Expand Up @@ -276,7 +276,7 @@ export const SelectPanelHeightAndScroll = () => {
selected={selectedA}
onSelectedChange={setSelectedA}
onFilterChange={setFilter}
showItemDividers={true}
showDividers={true}
overlayProps={{height: 'medium'}}
/>
<h2>With height:auto, maxheight:medium</h2>
Expand All @@ -293,7 +293,7 @@ export const SelectPanelHeightAndScroll = () => {
selected={selectedB}
onSelectedChange={setSelectedB}
onFilterChange={setFilter}
showItemDividers={true}
showDividers={true}
overlayProps={{
height: 'auto',
maxHeight: 'medium',
Expand Down
17 changes: 8 additions & 9 deletions src/SelectPanel/SelectPanel.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import React, {useState} from 'react'
import Box from '../Box'
import {Button} from '../Button'
import {SelectPanel} from '../SelectPanel'
import {ItemInput} from '../deprecated/ActionList/List'
import {ItemInput} from '../FilteredActionList'

export default {
title: 'Components/SelectPanel',
Expand All @@ -32,13 +32,13 @@ function getColorCircle(color: string) {
}

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 Default = () => {
Expand All @@ -65,7 +65,6 @@ export const Default = () => {
selected={selected}
onSelectedChange={setSelected}
onFilterChange={setFilter}
showItemDividers={true}
overlayProps={{width: 'small', height: 'xsmall'}}
/>
</>
Expand Down
2 changes: 1 addition & 1 deletion src/SelectPanel/SelectPanel.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import theme from '../theme'
import {SelectPanel} from '../SelectPanel'
import {behavesAsComponent, checkExports} from '../utils/testing'
import {BaseStyles, SSRProvider, ThemeProvider} from '..'
import {ItemInput} from '../deprecated/ActionList/List'
import {ItemInput} from '../FilteredActionList'

expect.extend(toHaveNoViolations)

Expand Down
Loading