Skip to content

Commit

Permalink
Improved performance for lists in layered components (#1221)
Browse files Browse the repository at this point in the history
  • Loading branch information
dgreif committed May 12, 2021
1 parent c63fa4b commit a926081
Show file tree
Hide file tree
Showing 5 changed files with 40 additions and 28 deletions.
5 changes: 5 additions & 0 deletions .changeset/funny-shoes-dress.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@primer/components": patch
---

Improved performance for lists in `ActionMenu` and `DropdownMenu`
23 changes: 11 additions & 12 deletions src/ActionMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import {GroupedListProps, List, ListPropsBase} from './ActionList/List'
import {Item, ItemProps} from './ActionList/Item'
import {Divider} from './ActionList/Divider'
import Button, {ButtonProps} from './Button'
import React, {useCallback, useEffect, useRef} from 'react'
import React, {useCallback, useEffect, useMemo, useRef} from 'react'
import {AnchoredOverlay} from './AnchoredOverlay'
import {useProvidedStateOrCreate} from './hooks/useProvidedStateOrCreate'
import {OverlayProps} from './Overlay'
Expand Down Expand Up @@ -48,11 +48,11 @@ ActionMenuItem.displayName = 'ActionMenu.Item'
const ActionMenuBase = ({
anchorContent,
renderAnchor = <T extends ButtonProps>(props: T) => <Button {...props} />,
renderItem = Item,
onAction,
open,
setOpen,
overlayProps,
items,
...listProps
}: ActionMenuProps): JSX.Element => {
const pendingActionRef = useRef<() => unknown>()
Expand All @@ -71,23 +71,22 @@ const ActionMenuBase = ({
[anchorContent, renderAnchor]
)

const renderMenuItem: typeof Item = useCallback(
({onAction: itemOnAction, ...itemProps}) => {
return renderItem({
...itemProps,
const itemsToRender = useMemo(() => {
return items.map(item => {
return {
...item,
role: 'menuitem',
onAction: (props, event) => {
const actionCallback = itemOnAction ?? onAction
const actionCallback = item.onAction ?? onAction
pendingActionRef.current = () => actionCallback?.(props as ItemProps, event)
actionCallback?.(props as ItemProps, event)
if (!event.defaultPrevented) {
onClose()
}
}
})
},
[onAction, onClose, renderItem]
)
} as ItemProps
})
}, [items, onAction, onClose])

useEffect(() => {
// Wait until menu has re-rendered in a closed state before triggering action.
Expand All @@ -106,7 +105,7 @@ const ActionMenuBase = ({
onClose={onClose}
overlayProps={overlayProps}
>
<List {...listProps} role="menu" renderItem={renderMenuItem} />
<List {...listProps} role="menu" items={itemsToRender} />
</AnchoredOverlay>
)
}
Expand Down
26 changes: 12 additions & 14 deletions src/DropdownMenu/DropdownMenu.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import React, {useCallback, useState} from 'react'
import React, {useCallback, useMemo, useState} from 'react'
import {List, GroupedListProps, ListPropsBase, ItemInput} from '../ActionList/List'
import {DropdownButton, DropdownButtonProps} from './DropdownButton'
import {Item} from '../ActionList/Item'
import {ItemProps} from '../ActionList/Item'
import {AnchoredOverlay} from '../AnchoredOverlay'
import {OverlayProps} from '../Overlay'

Expand Down Expand Up @@ -42,11 +42,11 @@ export interface DropdownMenuProps extends Partial<Omit<GroupedListProps, keyof
*/
export function DropdownMenu({
renderAnchor = <T extends DropdownButtonProps>(props: T) => <DropdownButton {...props} />,
renderItem = Item,
placeholder,
selectedItem,
onChange,
overlayProps,
items,
...listProps
}: DropdownMenuProps): JSX.Element {
const [open, setOpen] = useState(false)
Expand All @@ -63,15 +63,14 @@ export function DropdownMenu({
[placeholder, renderAnchor, selectedItem?.text]
)

const renderMenuItem: typeof Item = useCallback(
({item, onAction: itemOnAction, ...itemProps}) => {
return renderItem({
...itemProps,
item,
const itemsToRender = useMemo(() => {
return items.map(item => {
return {
...item,
role: 'option',
selected: item === selectedItem,
onAction: (itemFromAction, event) => {
itemOnAction?.(itemFromAction, event)
item.onAction?.(itemFromAction, event)

if (event.defaultPrevented) {
return
Expand All @@ -80,10 +79,9 @@ export function DropdownMenu({
onClose()
onChange?.(item === selectedItem ? undefined : item)
}
})
},
[onChange, onClose, renderItem, selectedItem]
)
} as ItemProps
})
}, [items, onChange, onClose, selectedItem])

return (
<AnchoredOverlay
Expand All @@ -93,7 +91,7 @@ export function DropdownMenu({
onClose={onClose}
overlayProps={overlayProps}
>
<List {...listProps} role="listbox" renderItem={renderMenuItem} />
<List {...listProps} role="listbox" items={itemsToRender} />
</AnchoredOverlay>
)
}
Expand Down
7 changes: 6 additions & 1 deletion src/__tests__/ActionMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,12 @@ describe('ActionMenu', () => {
jest.clearAllMocks()
})

behavesAsComponent({Component: ActionMenu, systemPropArray: [COMMON], options: {skipAs: true, skipSx: true}})
behavesAsComponent({
Component: ActionMenu,
systemPropArray: [COMMON],
options: {skipAs: true, skipSx: true},
toRender: () => <ActionMenu items={[]} />
})

checkExports('ActionMenu', {
default: undefined,
Expand Down
7 changes: 6 additions & 1 deletion src/__tests__/DropdownMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,12 @@ describe('DropdownMenu', () => {
jest.clearAllMocks()
})

behavesAsComponent({Component: DropdownMenu, systemPropArray: [COMMON], options: {skipAs: true, skipSx: true}})
behavesAsComponent({
Component: DropdownMenu,
systemPropArray: [COMMON],
options: {skipAs: true, skipSx: true},
toRender: () => <DropdownMenu items={[]} />
})

checkExports('DropdownMenu', {
default: undefined,
Expand Down

1 comment on commit a926081

@vercel
Copy link

@vercel vercel bot commented on a926081 May 12, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.