Skip to content

Commit

Permalink
Use item index as fallback key within groups (#1242)
Browse files Browse the repository at this point in the history
  • Loading branch information
dgreif authored May 21, 2021
1 parent c199131 commit ff177c8
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 8 deletions.
5 changes: 5 additions & 0 deletions .changeset/lucky-ducks-camp.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@primer/components": patch
---

Use `Item` `index` as fallback key within `List` groups
15 changes: 7 additions & 8 deletions src/ActionList/List.tsx
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
import React, {useMemo} from 'react'
import React from 'react'
import type {AriaRole} from '../utils/types'
import {Group, GroupProps} from './Group'
import {Item, ItemProps} from './Item'
import {Divider} from './Divider'
import styled from 'styled-components'
import {get} from '../constants'
import {SystemCssProperties} from '@styled-system/css'
import {uniqueId} from '../utils/uniqueId'

export type ItemInput = ItemProps | (Partial<ItemProps> & {renderItem: typeof Item})

Expand Down Expand Up @@ -151,10 +150,10 @@ export function List(props: ListProps): JSX.Element {
* An `Item`-level, `Group`-level, or `List`-level custom `Item` renderer,
* or the default `Item` renderer.
*/
const renderItem = (itemProps: ItemInput, item: ItemInput) => {
const renderItem = (itemProps: ItemInput, item: ItemInput, itemIndex: number) => {
// eslint-disable-next-line @typescript-eslint/no-unnecessary-condition
const ItemComponent = ('renderItem' in itemProps && itemProps.renderItem) || props.renderItem || Item
const key = itemProps.key ?? itemProps.id?.toString() ?? uniqueId()
const key = itemProps.key ?? itemProps.id?.toString() ?? itemIndex.toString()
return (
<ItemComponent
showDivider={props.showItemDividers}
Expand All @@ -173,11 +172,9 @@ export function List(props: ListProps): JSX.Element {
let groups: (GroupProps | (Partial<GroupProps> & {renderItem?: typeof Item; renderGroup?: typeof Group}))[] = []

// Collect rendered `Item`s into `Group`s, avoiding excess iteration over the lists of `items` and `groupMetadata`:
const singleGroupId = useMemo(uniqueId, [])

if (!isGroupedListProps(props)) {
// When no `groupMetadata`s is provided, collect rendered `Item`s into a single anonymous `Group`.
groups = [{items: props.items.map(item => renderItem(item, item)), groupId: singleGroupId}]
groups = [{items: props.items.map((item, index) => renderItem(item, item, index)), groupId: '0'}]
} else {
// When `groupMetadata` is provided, collect rendered `Item`s into their associated `Group`s.

Expand All @@ -192,6 +189,7 @@ export function List(props: ListProps): JSX.Element {
for (const itemProps of props.items) {
// Look up the group associated with the current item.
const group = groupMap.get(itemProps.groupId)
const itemIndex = group?.items?.length ?? 0

// Upsert the group to include the current item (rendered).
groupMap.set(itemProps.groupId, {
Expand All @@ -204,7 +202,8 @@ export function List(props: ListProps): JSX.Element {
...(group && 'renderItem' in group && {renderItem: group.renderItem}),
...itemProps
},
itemProps
itemProps,
itemIndex
)
]
})
Expand Down

1 comment on commit ff177c8

@vercel
Copy link

@vercel vercel bot commented on ff177c8 May 21, 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.