Skip to content

Commit 2053cb3

Browse files
mperrottiCopilot
mperrotti
and
Copilot
authored
Remove Box usage and sx prop from NavList and related components (#6868)
Co-authored-by: Copilot <[email protected]>
1 parent 0f075d1 commit 2053cb3

File tree

14 files changed

+517
-489
lines changed

14 files changed

+517
-489
lines changed

‎.changeset/cool-clubs-think.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@primer/react': major
3+
---
4+
5+
Removes Box usage and sx prop from NavList and ActionList

‎packages/react/src/ActionList/Item.tsx‎

Lines changed: 277 additions & 264 deletions
Large diffs are not rendered by default.

‎packages/react/src/ActionList/List.tsx‎

Lines changed: 70 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import React from 'react'
2-
import type {ForwardRefComponent as PolymorphicForwardRefComponent} from '../utils/polymorphic'
2+
import {fixedForwardRef} from '../utils/modern-polymorphic'
33
import {ActionListContainerContext} from './ActionListContainerContext'
44
import {useSlots} from '../hooks/useSlots'
55
import {Heading} from './Heading'
@@ -11,68 +11,80 @@ import {clsx} from 'clsx'
1111
import classes from './ActionList.module.css'
1212
import {BoxWithFallback} from '../internal/components/BoxWithFallback'
1313

14-
export const List = React.forwardRef<HTMLUListElement, ActionListProps>(
15-
(
16-
{variant = 'inset', selectionVariant, showDividers = false, role, disableFocusZone = false, className, ...props},
17-
forwardedRef,
18-
): JSX.Element => {
19-
const [slots, childrenWithoutSlots] = useSlots(props.children, {
20-
heading: Heading,
21-
})
14+
const UnwrappedList = <As extends React.ElementType = 'ul'>(
15+
props: ActionListProps<As>,
16+
forwardedRef: React.Ref<unknown>,
17+
): JSX.Element => {
18+
const {
19+
as: Component = 'ul',
20+
variant = 'inset',
21+
selectionVariant,
22+
showDividers = false,
23+
role,
24+
disableFocusZone = false,
25+
className,
26+
...restProps
27+
} = props
28+
const [slots, childrenWithoutSlots] = useSlots(restProps.children, {
29+
heading: Heading,
30+
})
2231

23-
const headingId = useId()
32+
const headingId = useId()
2433

25-
/** if list is inside a Menu, it will get a role from the Menu */
26-
const {
27-
listRole: listRoleFromContainer,
28-
listLabelledBy,
29-
selectionVariant: containerSelectionVariant, // TODO: Remove after DropdownMenu2 deprecation
30-
enableFocusZone: enableFocusZoneFromContainer,
31-
container,
32-
} = React.useContext(ActionListContainerContext)
34+
/** if list is inside a Menu, it will get a role from the Menu */
35+
const {
36+
listRole: listRoleFromContainer,
37+
listLabelledBy,
38+
selectionVariant: containerSelectionVariant, // TODO: Remove after DropdownMenu2 deprecation
39+
enableFocusZone: enableFocusZoneFromContainer,
40+
container,
41+
} = React.useContext(ActionListContainerContext)
3342

34-
const ariaLabelledBy = slots.heading ? (slots.heading.props.id ?? headingId) : listLabelledBy
35-
const listRole = role || listRoleFromContainer
36-
const listRef = useProvidedRefOrCreate(forwardedRef as React.RefObject<HTMLUListElement>)
43+
const ariaLabelledBy = slots.heading ? (slots.heading.props.id ?? headingId) : listLabelledBy
44+
const listRole = role || listRoleFromContainer
45+
const listRef = useProvidedRefOrCreate(forwardedRef as React.RefObject<HTMLUListElement>)
3746

38-
let enableFocusZone = false
39-
if (enableFocusZoneFromContainer !== undefined) enableFocusZone = enableFocusZoneFromContainer
40-
else if (listRole && !disableFocusZone) enableFocusZone = ['menu', 'menubar', 'listbox'].includes(listRole)
47+
let enableFocusZone = false
48+
if (enableFocusZoneFromContainer !== undefined) enableFocusZone = enableFocusZoneFromContainer
49+
else if (listRole && !disableFocusZone) enableFocusZone = ['menu', 'menubar', 'listbox'].includes(listRole)
4150

42-
useFocusZone({
43-
disabled: !enableFocusZone,
44-
containerRef: listRef,
45-
bindKeys: FocusKeys.ArrowVertical | FocusKeys.HomeAndEnd | FocusKeys.PageUpDown,
46-
focusOutBehavior:
47-
listRole === 'menu' || container === 'SelectPanel' || container === 'FilteredActionList' ? 'wrap' : undefined,
48-
})
51+
useFocusZone({
52+
disabled: !enableFocusZone,
53+
containerRef: listRef,
54+
bindKeys: FocusKeys.ArrowVertical | FocusKeys.HomeAndEnd | FocusKeys.PageUpDown,
55+
focusOutBehavior:
56+
listRole === 'menu' || container === 'SelectPanel' || container === 'FilteredActionList' ? 'wrap' : undefined,
57+
})
4958

50-
return (
51-
<ListContext.Provider
52-
value={{
53-
variant,
54-
selectionVariant: selectionVariant || containerSelectionVariant,
55-
showDividers,
56-
role: listRole,
57-
headingId,
58-
}}
59+
return (
60+
<ListContext.Provider
61+
value={{
62+
variant,
63+
selectionVariant: selectionVariant || containerSelectionVariant,
64+
showDividers,
65+
role: listRole,
66+
headingId,
67+
}}
68+
>
69+
{slots.heading}
70+
<BoxWithFallback
71+
as={Component}
72+
className={clsx(classes.ActionList, className)}
73+
role={listRole}
74+
aria-labelledby={ariaLabelledBy}
75+
ref={listRef}
76+
data-dividers={showDividers}
77+
data-variant={variant}
78+
{...restProps}
5979
>
60-
{slots.heading}
61-
<BoxWithFallback
62-
as="ul"
63-
className={clsx(classes.ActionList, className)}
64-
role={listRole}
65-
aria-labelledby={ariaLabelledBy}
66-
ref={listRef}
67-
data-dividers={showDividers}
68-
data-variant={variant}
69-
{...props}
70-
>
71-
{childrenWithoutSlots}
72-
</BoxWithFallback>
73-
</ListContext.Provider>
74-
)
75-
},
76-
) as PolymorphicForwardRefComponent<'ul', ActionListProps>
80+
{childrenWithoutSlots}
81+
</BoxWithFallback>
82+
</ListContext.Provider>
83+
)
84+
}
7785

78-
List.displayName = 'ActionList'
86+
const List = fixedForwardRef(UnwrappedList)
87+
88+
Object.assign(List, {displayName: 'ActionList'})
89+
90+
export {List}

‎packages/react/src/ActionList/shared.ts‎

Lines changed: 46 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,15 @@
11
import React from 'react'
22
import type {SxProp} from '../sx'
33
import type {AriaRole} from '../utils/types'
4+
import type {PolymorphicProps} from '../utils/modern-polymorphic'
45

5-
export type ActionListItemProps = {
6+
// need to explicitly omit `onSelect` from native HTML <li> attributes to avoid collision
7+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
8+
type ExcludeSelectEventHandler<T> = T extends any ? Omit<T, 'onSelect'> : never
9+
10+
export type ActionListItemProps<As extends React.ElementType = 'li'> = ExcludeSelectEventHandler<
11+
PolymorphicProps<As, 'li'>
12+
> & {
613
/**
714
* Primary content for an Item
815
*/
@@ -57,6 +64,10 @@ export type ActionListItemProps = {
5764
groupId?: string
5865
renderItem?: (item: React.FC<React.PropsWithChildren<MenuItemProps>>) => React.ReactNode
5966
handleAddItem?: (item: React.FC<React.PropsWithChildren<MenuItemProps>>) => void
67+
/**
68+
* @deprecated `as` prop has no effect on `ActionList.Item`, only `ActionList.LinkItem`
69+
*/
70+
as?: As
6071
} & SxProp
6172

6273
type MenuItemProps = {
@@ -70,7 +81,7 @@ type MenuItemProps = {
7081
className?: string
7182
}
7283

73-
export type ItemContext = Pick<ActionListItemProps, 'variant' | 'disabled' | 'size'> & {
84+
export type ItemContext = Pick<ActionListItemProps<React.ElementType>, 'variant' | 'disabled' | 'size'> & {
7485
inlineDescriptionId?: string
7586
blockDescriptionId?: string
7687
trailingVisualId?: string
@@ -80,8 +91,8 @@ export type ItemContext = Pick<ActionListItemProps, 'variant' | 'disabled' | 'si
8091
export const ItemContext = React.createContext<ItemContext>({})
8192

8293
export const getVariantStyles = (
83-
variant: ActionListItemProps['variant'],
84-
disabled: ActionListItemProps['disabled'],
94+
variant: ActionListItemProps<React.ElementType>['variant'],
95+
disabled: ActionListItemProps<React.ElementType>['disabled'],
8596
inactive?: boolean,
8697
) => {
8798
if (disabled) {
@@ -120,32 +131,39 @@ export const getVariantStyles = (
120131

121132
export const TEXT_ROW_HEIGHT = '20px' // custom value off the scale
122133

123-
export type ActionListProps = React.PropsWithChildren<{
124-
/**
125-
* `inset` children are offset (vertically and horizontally) from `List`’s edges, `full` children are flush (vertically and horizontally) with `List` edges
126-
*/
127-
variant?: 'inset' | 'horizontal-inset' | 'full'
128-
/**
129-
* Whether multiple Items or a single Item can be selected.
130-
*/
131-
selectionVariant?: 'single' | 'radio' | 'multiple'
132-
/**
133-
* Display a divider above each `Item` in this `List` when it does not follow a `Header` or `Divider`.
134-
*/
135-
showDividers?: boolean
136-
/**
137-
* The ARIA role describing the function of `List` component. `listbox` or `menu` are a common values.
138-
*/
139-
role?: AriaRole
140-
/**
141-
* Disables the focus zone for the list if applicable. Focus zone is enabled by default for `menu` and `listbox` roles, or components such as `ActionMenu` and `SelectPanel`.
142-
*/
143-
disableFocusZone?: boolean
144-
className?: string
145-
}> &
134+
export type ActionListProps<As extends React.ElementType = 'ul'> = PolymorphicProps<
135+
As,
136+
'ul',
137+
React.PropsWithChildren<{
138+
/**
139+
* `inset` children are offset (vertically and horizontally) from `List`’s edges, `full` children are flush (vertically and horizontally) with `List` edges
140+
*/
141+
variant?: 'inset' | 'horizontal-inset' | 'full'
142+
/**
143+
* Whether multiple Items or a single Item can be selected.
144+
*/
145+
selectionVariant?: 'single' | 'radio' | 'multiple'
146+
/**
147+
* Display a divider above each `Item` in this `List` when it does not follow a `Header` or `Divider`.
148+
*/
149+
showDividers?: boolean
150+
/**
151+
* The ARIA role describing the function of `List` component. `listbox` or `menu` are a common values.
152+
*/
153+
role?: AriaRole
154+
/**
155+
* Disables the focus zone for the list if applicable. Focus zone is enabled by default for `menu` and `listbox` roles, or components such as `ActionMenu` and `SelectPanel`.
156+
*/
157+
disableFocusZone?: boolean
158+
className?: string
159+
}>
160+
> &
146161
SxProp
147162

148-
type ContextProps = Pick<ActionListProps, 'variant' | 'selectionVariant' | 'showDividers' | 'role'> & {
163+
type ContextProps = Pick<
164+
ActionListProps<React.ElementType>,
165+
'variant' | 'selectionVariant' | 'showDividers' | 'role'
166+
> & {
149167
headingId?: string
150168
}
151169

‎packages/react/src/Autocomplete/AutocompleteMenu.tsx‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import type {ScrollIntoViewOptions} from '@primer/behaviors'
66
import type {ActionListItemProps} from '../ActionList'
77
import {ActionList} from '../ActionList'
88
import {useFocusZone} from '../hooks/useFocusZone'
9-
import type {ComponentProps, MandateProps} from '../utils/types'
9+
import type {ComponentProps, MandateProps, AriaRole} from '../utils/types'
1010
import Spinner from '../Spinner'
1111
import {useId} from '../hooks/useId'
1212
import {AutocompleteContext} from './AutocompleteContext'
@@ -365,19 +365,25 @@ function AutocompleteMenu<T extends AutocompleteItemProps>(props: AutocompleteMe
365365
text,
366366
leadingVisual: LeadingVisual,
367367
trailingVisual: TrailingVisual,
368-
// @ts-expect-error this is defined in the items above but is
369-
// missing in TS
370368
key,
369+
role,
371370
...itemProps
372371
} = item
373372
return (
374-
<ActionList.Item key={key ?? id} onSelect={() => onAction(item)} {...itemProps} id={id} data-id={id}>
373+
<ActionList.Item
374+
key={(key ?? id) as string | number}
375+
onSelect={() => onAction(item)}
376+
{...itemProps}
377+
id={id}
378+
data-id={id}
379+
role={role as AriaRole}
380+
>
375381
{LeadingVisual && (
376382
<ActionList.LeadingVisual>
377383
{isElement(LeadingVisual) ? LeadingVisual : <LeadingVisual />}
378384
</ActionList.LeadingVisual>
379385
)}
380-
{children ?? text}
386+
{(children ?? text) as React.ReactNode}
381387
{TrailingVisual && (
382388
<ActionList.TrailingVisual>
383389
{isElement(TrailingVisual) ? TrailingVisual : <TrailingVisual />}

0 commit comments

Comments
 (0)