Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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/perf-action-list-memoization.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@primer/react': patch
---

perf(ActionList): memoize context values, menuItemProps, and aria attributes
139 changes: 89 additions & 50 deletions packages/react/src/ActionList/Item.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,19 @@ const DivItemContainerNoBox = React.forwardRef<HTMLDivElement, React.HTMLAttribu
},
)

const baseSlots = {
leadingVisual: LeadingVisual,
trailingVisual: TrailingVisual,
trailingAction: TrailingAction,
subItem: SubItem,
}

const slotsConfig = {...baseSlots, description: Description}

// Pre-allocated array for selectableRoles check, avoids per-render allocation
const selectableRoles = ['menuitemradio', 'menuitemcheckbox', 'option']
const listRoleTypes = ['listbox', 'menu', 'list']

const UnwrappedItem = <As extends React.ElementType = 'li'>(
{
variant = 'default',
Expand All @@ -92,14 +105,7 @@ const UnwrappedItem = <As extends React.ElementType = 'li'>(
// eslint-disable-next-line @typescript-eslint/no-explicit-any
forwardedRef: React.Ref<any>,
): JSX.Element => {
const baseSlots = {
leadingVisual: LeadingVisual,
trailingVisual: TrailingVisual,
trailingAction: TrailingAction,
subItem: SubItem,
}

const [partialSlots, childrenWithoutSlots] = useSlots(props.children, {...baseSlots, description: Description})
const [partialSlots, childrenWithoutSlots] = useSlots(props.children, slotsConfig)

const slots = {description: undefined, ...partialSlots}

Expand Down Expand Up @@ -173,7 +179,6 @@ const UnwrappedItem = <As extends React.ElementType = 'li'>(
itemRole === 'menuitemcheckbox' ||
itemRole === 'tab'

const listRoleTypes = ['listbox', 'menu', 'list']
const listSemantics = (listRole && listRoleTypes.includes(listRole)) || inactive || listItemSemantics
const buttonSemantics = !listSemantics && !_PrivateItemWrapper

Expand All @@ -188,7 +193,7 @@ const UnwrappedItem = <As extends React.ElementType = 'li'>(
const keyPressHandler = React.useCallback(
(event: React.KeyboardEvent<HTMLElement>) => {
if (disabled || inactive || loading) return
if ([' ', 'Enter'].includes(event.key)) {
if (event.key === ' ' || event.key === 'Enter') {
if (event.key === ' ') {
event.preventDefault() // prevent scrolling on Space
// immediately reset defaultPrevented once its job is done
Expand All @@ -214,40 +219,61 @@ const UnwrappedItem = <As extends React.ElementType = 'li'>(

const ItemWrapper = _PrivateItemWrapper || DefaultItemWrapper

// only apply aria-selected and aria-checked to selectable items
const selectableRoles = ['menuitemradio', 'menuitemcheckbox', 'option']
const includeSelectionAttribute = itemSelectionAttribute && itemRole && selectableRoles.includes(itemRole)

let focusable

if (showInactiveIndicator) {
focusable = true
}
const focusable = showInactiveIndicator ? true : undefined

// Extract the variant prop value from the description slot component
const descriptionVariant = slots.description?.props.variant ?? 'inline'

const menuItemProps = {
onClick: clickHandler,
onKeyPress: !buttonSemantics ? keyPressHandler : undefined,
'aria-disabled': disabled ? true : undefined,
'data-inactive': inactive ? true : undefined,
'data-loading': loading && !inactive ? true : undefined,
tabIndex: focusable ? undefined : 0,
'aria-labelledby': `${labelId} ${slots.trailingVisual ? trailingVisualId : ''}`,
'aria-describedby':
[
slots.description && descriptionVariant === 'block' ? blockDescriptionId : undefined,
slots.description && descriptionVariant === 'inline' ? inlineDescriptionId : undefined,
inactiveWarningId ?? undefined,
]
.filter(String)
.join(' ')
.trim() || undefined,
...(includeSelectionAttribute && {[itemSelectionAttribute]: selected}),
role: itemRole,
id: itemId,
}
const hasTrailingVisualSlot = Boolean(slots.trailingVisual)
const hasDescriptionSlot = Boolean(slots.description)

const ariaLabelledBy = React.useMemo(() => {
const parts = [labelId]
if (hasTrailingVisualSlot) parts.push(trailingVisualId)
if (hasDescriptionSlot && descriptionVariant === 'inline') parts.push(inlineDescriptionId)
return parts.join(' ')
}, [labelId, hasTrailingVisualSlot, trailingVisualId, hasDescriptionSlot, descriptionVariant, inlineDescriptionId])

const ariaDescribedBy = React.useMemo(() => {
const parts: string[] = []
if (hasDescriptionSlot && descriptionVariant === 'block') parts.push(blockDescriptionId)
if (inactiveWarningId) parts.push(inactiveWarningId)
return parts.length > 0 ? parts.join(' ') : undefined
}, [hasDescriptionSlot, descriptionVariant, blockDescriptionId, inactiveWarningId])

const menuItemProps = React.useMemo(
() => ({
onClick: clickHandler,
onKeyPress: !buttonSemantics ? keyPressHandler : undefined,
'aria-disabled': disabled ? true : undefined,
'data-inactive': inactive ? true : undefined,
'data-loading': loading && !inactive ? true : undefined,
tabIndex: focusable ? undefined : 0,
'aria-labelledby': ariaLabelledBy,
'aria-describedby': ariaDescribedBy,
...(includeSelectionAttribute && {[itemSelectionAttribute]: selected}),
role: itemRole,
id: itemId,
}),
[
clickHandler,
buttonSemantics,
keyPressHandler,
disabled,
inactive,
loading,
focusable,
ariaLabelledBy,
ariaDescribedBy,
includeSelectionAttribute,
itemSelectionAttribute,
selected,
itemRole,
itemId,
],
)

const containerProps = _PrivateItemWrapper
? {role: itemRole ? 'none' : undefined, ...props}
Expand All @@ -262,19 +288,32 @@ const UnwrappedItem = <As extends React.ElementType = 'li'>(
ref: forwardedRef,
}

const itemContextValue = React.useMemo(
() => ({
variant,
size,
disabled,
inactive: Boolean(inactiveText),
inlineDescriptionId,
blockDescriptionId,
trailingVisualId,
setTruncatedText: buttonSemantics ? setTruncatedText : undefined,
}),
[
variant,
size,
disabled,
inactiveText,
inlineDescriptionId,
blockDescriptionId,
trailingVisualId,
buttonSemantics,
setTruncatedText,
],
)

return (
<ItemContext.Provider
value={{
variant,
size,
disabled,
inactive: Boolean(inactiveText),
inlineDescriptionId,
blockDescriptionId,
trailingVisualId,
setTruncatedText: buttonSemantics ? setTruncatedText : undefined,
}}
>
<ItemContext.Provider value={itemContextValue}>
<li
{...containerProps}
ref={listSemantics ? forwardedRef : null}
Expand Down
21 changes: 12 additions & 9 deletions packages/react/src/ActionList/List.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -55,16 +55,19 @@ const UnwrappedList = <As extends React.ElementType = 'ul'>(
listRole === 'menu' || container === 'SelectPanel' || container === 'FilteredActionList' ? 'wrap' : undefined,
})

const listContextValue = React.useMemo(
() => ({
variant,
selectionVariant: selectionVariant || containerSelectionVariant,
showDividers,
role: listRole,
headingId,
}),
[variant, selectionVariant, containerSelectionVariant, showDividers, listRole, headingId],
)

return (
<ListContext.Provider
value={{
variant,
selectionVariant: selectionVariant || containerSelectionVariant,
showDividers,
role: listRole,
headingId,
}}
>
<ListContext.Provider value={listContextValue}>
{slots.heading}
{/* @ts-expect-error ref needs a non nullable ref */}
<Component
Expand Down
Loading