From 5546af4bfe86332f3aa0ebfc64c159dfb3fa9796 Mon Sep 17 00:00:00 2001 From: Siddharth Kshetrapal Date: Tue, 23 Sep 2025 10:01:11 +0200 Subject: [PATCH 1/5] make item.id required --- .changeset/selectpanel-item-id-required.md | 5 +++ .../react/src/FilteredActionList/types.ts | 2 +- .../src/SelectPanel/SelectPanel.test.tsx | 40 +++++-------------- 3 files changed, 15 insertions(+), 32 deletions(-) create mode 100644 .changeset/selectpanel-item-id-required.md diff --git a/.changeset/selectpanel-item-id-required.md b/.changeset/selectpanel-item-id-required.md new file mode 100644 index 00000000000..6ae15c85e88 --- /dev/null +++ b/.changeset/selectpanel-item-id-required.md @@ -0,0 +1,5 @@ +--- +"@primer/react": major +--- + +SelectPanel: `item.id` is now required diff --git a/packages/react/src/FilteredActionList/types.ts b/packages/react/src/FilteredActionList/types.ts index 9bca16f3d5e..ba359388588 100644 --- a/packages/react/src/FilteredActionList/types.ts +++ b/packages/react/src/FilteredActionList/types.ts @@ -86,7 +86,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. diff --git a/packages/react/src/SelectPanel/SelectPanel.test.tsx b/packages/react/src/SelectPanel/SelectPanel.test.tsx index b42041c08c6..156442177ed 100644 --- a/packages/react/src/SelectPanel/SelectPanel.test.tsx +++ b/packages/react/src/SelectPanel/SelectPanel.test.tsx @@ -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) { @@ -933,15 +927,9 @@ for (const usingRemoveActiveDescendant of [false, true]) { renderWithFlag( , usingRemoveActiveDescendant, @@ -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 () => { From 669d5c66285d48b1179d632a4c80ec413b374ae9 Mon Sep 17 00:00:00 2001 From: Siddharth Kshetrapal Date: Tue, 23 Sep 2025 10:31:07 +0200 Subject: [PATCH 2/5] not conditional --- packages/react/src/SelectPanel/SelectPanel.examples.stories.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react/src/SelectPanel/SelectPanel.examples.stories.tsx b/packages/react/src/SelectPanel/SelectPanel.examples.stories.tsx index 05ccab9f5b9..676f00d44db 100644 --- a/packages/react/src/SelectPanel/SelectPanel.examples.stories.tsx +++ b/packages/react/src/SelectPanel/SelectPanel.examples.stories.tsx @@ -299,7 +299,7 @@ export const CustomItemRenderer = () => { onFilterChange={setFilter} overlayProps={{width: 'medium'}} renderItem={item => ( - + {item.text} )} From 625d921cabd2d5082a78e3d83035f3ca31608707 Mon Sep 17 00:00:00 2001 From: Siddharth Kshetrapal Date: Tue, 23 Sep 2025 10:49:36 +0200 Subject: [PATCH 3/5] make id required in ItemInput --- .../react/src/FilteredActionList/FilteredActionList.tsx | 4 ++-- packages/react/src/FilteredActionList/types.ts | 8 +++++++- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/packages/react/src/FilteredActionList/FilteredActionList.tsx b/packages/react/src/FilteredActionList/FilteredActionList.tsx index cc01dc685ab..ba7877d1eb5 100644 --- a/packages/react/src/FilteredActionList/FilteredActionList.tsx +++ b/packages/react/src/FilteredActionList/FilteredActionList.tsx @@ -291,7 +291,7 @@ export function FilteredActionList({ {group.header?.title ? group.header.title : `Group ${group.groupId}`} {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 ( { - const key = itemKey ?? item.id?.toString() ?? index.toString() + const key = itemKey || item.id.toString() || index.toString() return ( React.ReactEl export type ItemInput = | Merge, FilteredActionListItemProps> - | ((Partial & {renderItem: RenderItemFn}) & {key?: Key}) + | ((Partial & { + // un-partial these fields because they are required + id: FilteredActionListItemProps['id'] + renderItem: RenderItemFn + }) & { + key?: Key + }) export interface FilteredActionListItemProps extends SxProp { /** From e651410d60c231669f72bd86aa29dff4dd8fedb4 Mon Sep 17 00:00:00 2001 From: Siddharth Kshetrapal Date: Tue, 23 Sep 2025 10:53:34 +0200 Subject: [PATCH 4/5] oh also Group items --- packages/react/src/FilteredActionList/types.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/packages/react/src/FilteredActionList/types.ts b/packages/react/src/FilteredActionList/types.ts index 786a170c23f..f736c7d1535 100644 --- a/packages/react/src/FilteredActionList/types.ts +++ b/packages/react/src/FilteredActionList/types.ts @@ -127,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 & {renderItem: RenderItemFn})) & { + items: (( + | FilteredActionListItemProps + | (Partial & { + // un-partial these fields because they are required + id: FilteredActionListItemProps['id'] + renderItem: RenderItemFn + }) + ) & { groupId: string })[] } From 21fae66ceb7204938b7c0b4dca9e39a169c293fc Mon Sep 17 00:00:00 2001 From: Siddharth Kshetrapal Date: Mon, 6 Oct 2025 23:05:26 +0200 Subject: [PATCH 5/5] update changeset --- .changeset/selectpanel-item-id-required.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/selectpanel-item-id-required.md b/.changeset/selectpanel-item-id-required.md index 6ae15c85e88..fde2131ba48 100644 --- a/.changeset/selectpanel-item-id-required.md +++ b/.changeset/selectpanel-item-id-required.md @@ -2,4 +2,4 @@ "@primer/react": major --- -SelectPanel: `item.id` is now required +SelectPanel: Make `id` required for `items` passed to SelectPanel