Skip to content
Open
Show file tree
Hide file tree
Changes from 6 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/selectpanel-item-id-required.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@primer/react": major
---

SelectPanel: `item.id` is now required
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@ export function FilteredActionList({
{group.header?.title ? group.header.title : `Group ${group.groupId}`}
</ActionList.GroupHeading>
{getItemListForEachGroup(group.groupId).map(({key: itemKey, ...item}, itemIndex) => {
const key = itemKey ?? item.id?.toString() ?? itemIndex.toString()
const key = itemKey || item.id.toString() || itemIndex.toString()
return (
<MappedActionListItem
key={key}
Expand All @@ -307,7 +307,7 @@ export function FilteredActionList({
)
})
: items.map(({key: itemKey, ...item}, index) => {
const key = itemKey ?? item.id?.toString() ?? index.toString()
const key = itemKey || item.id.toString() || index.toString()
return (
<MappedActionListItem
key={key}
Expand Down
19 changes: 16 additions & 3 deletions packages/react/src/FilteredActionList/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,13 @@ export type RenderItemFn = (props: FilteredActionListItemProps) => React.ReactEl

export type ItemInput =
| Merge<React.ComponentPropsWithoutRef<'div'>, FilteredActionListItemProps>
| ((Partial<FilteredActionListItemProps> & {renderItem: RenderItemFn}) & {key?: Key})
| ((Partial<FilteredActionListItemProps> & {
// un-partial these fields because they are required
id: FilteredActionListItemProps['id']
renderItem: RenderItemFn
}) & {
key?: Key
})

export interface FilteredActionListItemProps extends SxProp {
/**
Expand Down Expand Up @@ -86,7 +92,7 @@ export interface FilteredActionListItemProps extends SxProp {
/**
* An id associated with this item. Should be unique between items
*/
id?: number | string
id: number | string

/**
* Node to be included inside the item before the text.
Expand Down Expand Up @@ -121,7 +127,14 @@ export interface GroupedListProps extends ListPropsBase {
* A collection of `Item` props, plus associated group identifiers
* and `Item`-level custom `Item` renderers.
*/
items: ((FilteredActionListItemProps | (Partial<FilteredActionListItemProps> & {renderItem: RenderItemFn})) & {
items: ((
| FilteredActionListItemProps
| (Partial<FilteredActionListItemProps> & {
// un-partial these fields because they are required
id: FilteredActionListItemProps['id']
renderItem: RenderItemFn
})
) & {
groupId: string
})[]
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,7 @@ export const CustomItemRenderer = () => {
onFilterChange={setFilter}
overlayProps={{width: 'medium'}}
renderItem={item => (
<ActionList.Item id={item.id?.toString()} className={styles.CustomActionListItem}>
<ActionList.Item id={item.id.toString()} className={styles.CustomActionListItem}>
<ActionList.Description truncate>{item.text}</ActionList.Description>
</ActionList.Item>
)}
Expand Down
40 changes: 9 additions & 31 deletions packages/react/src/SelectPanel/SelectPanel.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,15 +33,9 @@ const renderWithFlag = (children: React.ReactNode, flag: boolean) => {
}

const items: SelectPanelProps['items'] = [
{
text: 'item one',
},
{
text: 'item two',
},
{
text: 'item three',
},
{id: 'one', text: 'item one'},
{id: 'two', text: 'item two'},
{id: 'three', text: 'item three'},
]

function BasicSelectPanel(passthroughProps: Record<string, unknown>) {
Expand Down Expand Up @@ -933,15 +927,9 @@ for (const usingRemoveActiveDescendant of [false, true]) {
renderWithFlag(
<SelectPanelWithCustomMessages
items={[
{
text: 'item one',
},
{
text: 'item two',
},
{
text: 'item three',
},
{id: 'one', text: 'item one'},
{id: 'two', text: 'item two'},
{id: 'three', text: 'item three'},
]}
/>,
usingRemoveActiveDescendant,
Expand Down Expand Up @@ -1186,19 +1174,9 @@ for (const usingRemoveActiveDescendant of [false, true]) {

describe('sorting', () => {
const items = [
{
text: 'item one',
id: '3',
},
{
text: 'item two',
id: '1',
selected: true,
},
{
text: 'item three',
id: '2',
},
{text: 'item one', id: '3'},
{text: 'item two', id: '1', selected: true},
{text: 'item three', id: '2'},
]

it('should render selected items at the top by default when FF on', async () => {
Expand Down
Loading