From 5abc62f90cac33cd67396d78f01bf91b703db45c Mon Sep 17 00:00:00 2001 From: Matthew Costabile Date: Mon, 22 Jun 2026 13:25:32 +0000 Subject: [PATCH 1/3] perf(SelectPanel): reset intermediate selection during render, not in an effect Replace the useEffect that reset the single-select modal's intermediate selection with the adjust-state-during-render pattern (tracking previous {open, selected, isSingleSelectModal}). Behavior is identical but avoids the extra post-commit render, and removing the eslint-disable lets the lint rule guard against regressions. --- ...electpanel-intermediate-selected-derive.md | 5 +++++ .../react/src/SelectPanel/SelectPanel.tsx | 19 +++++++++++++++---- 2 files changed, 20 insertions(+), 4 deletions(-) create mode 100644 .changeset/selectpanel-intermediate-selected-derive.md diff --git a/.changeset/selectpanel-intermediate-selected-derive.md b/.changeset/selectpanel-intermediate-selected-derive.md new file mode 100644 index 00000000000..6ed9a5f6507 --- /dev/null +++ b/.changeset/selectpanel-intermediate-selected-derive.md @@ -0,0 +1,5 @@ +--- +'@primer/react': patch +--- + +SelectPanel: Reset the single-select modal's intermediate selection during render instead of in an effect, avoiding an extra re-render when the panel opens or the selection changes. diff --git a/packages/react/src/SelectPanel/SelectPanel.tsx b/packages/react/src/SelectPanel/SelectPanel.tsx index 9bf81f39d8e..ac43ae820fe 100644 --- a/packages/react/src/SelectPanel/SelectPanel.tsx +++ b/packages/react/src/SelectPanel/SelectPanel.tsx @@ -241,11 +241,22 @@ function Panel({ isSingleSelectModal ? selected : undefined, ) - // Reset the intermediate selected item when the panel is open/closed - useEffect(() => { - // eslint-disable-next-line react-hooks/set-state-in-effect, react-you-might-not-need-an-effect/no-derived-state + // Reset the intermediate selected item when the panel is opened/closed or the external + // selection changes. Adjusting state during render (tracking the previous inputs) rather + // than syncing it from an effect avoids an extra post-commit render. + const [intermediateSelectedResetKey, setIntermediateSelectedResetKey] = useState({ + open, + selected, + isSingleSelectModal, + }) + if ( + intermediateSelectedResetKey.open !== open || + intermediateSelectedResetKey.selected !== selected || + intermediateSelectedResetKey.isSingleSelectModal !== isSingleSelectModal + ) { + setIntermediateSelectedResetKey({open, selected, isSingleSelectModal}) setIntermediateSelected(isSingleSelectModal ? selected : undefined) - }, [isSingleSelectModal, open, selected]) + } const onListContainerRefChanged: FilteredActionListProps['onListContainerRefChanged'] = useCallback( (node: HTMLElement | null) => { From c962a6d73767121bd7d7fd12af8aa5c32041accd Mon Sep 17 00:00:00 2001 From: Matthew Costabile Date: Wed, 24 Jun 2026 01:16:25 +0000 Subject: [PATCH 2/3] Only reset intermediate selection when it actually changes --- packages/react/src/SelectPanel/SelectPanel.tsx | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/packages/react/src/SelectPanel/SelectPanel.tsx b/packages/react/src/SelectPanel/SelectPanel.tsx index ac43ae820fe..8efd4f1b936 100644 --- a/packages/react/src/SelectPanel/SelectPanel.tsx +++ b/packages/react/src/SelectPanel/SelectPanel.tsx @@ -243,19 +243,23 @@ function Panel({ // Reset the intermediate selected item when the panel is opened/closed or the external // selection changes. Adjusting state during render (tracking the previous inputs) rather - // than syncing it from an effect avoids an extra post-commit render. + // than syncing it from an effect avoids an extra post-commit render. The reset is also + // gated on the intermediate selection actually changing, so variants that never use it + // (e.g. multi-select / anchored, where it stays `undefined`) don't pay for a render-phase + // restart on every open/selection change. + const nextIntermediateSelected = isSingleSelectModal ? selected : undefined const [intermediateSelectedResetKey, setIntermediateSelectedResetKey] = useState({ open, selected, isSingleSelectModal, }) - if ( + const intermediateSelectedInputsChanged = intermediateSelectedResetKey.open !== open || intermediateSelectedResetKey.selected !== selected || intermediateSelectedResetKey.isSingleSelectModal !== isSingleSelectModal - ) { + if (intermediateSelectedInputsChanged && intermediateSelected !== nextIntermediateSelected) { setIntermediateSelectedResetKey({open, selected, isSingleSelectModal}) - setIntermediateSelected(isSingleSelectModal ? selected : undefined) + setIntermediateSelected(nextIntermediateSelected) } const onListContainerRefChanged: FilteredActionListProps['onListContainerRefChanged'] = useCallback( From 50046a309618be2b649baf62b6ff2c7c9f85492c Mon Sep 17 00:00:00 2001 From: Matthew Costabile Date: Tue, 7 Jul 2026 20:13:07 +0000 Subject: [PATCH 3/3] fix(SelectPanel): advance intermediate-selection reset key whenever inputs change Decouples the reset-key advance from the value-difference check so it can't go stale and clobber a user-driven selection on the next render (single-select modal). Adds a regression test. --- .../src/SelectPanel/SelectPanel.test.tsx | 43 +++++++++++++++++++ .../react/src/SelectPanel/SelectPanel.tsx | 17 +++++--- 2 files changed, 54 insertions(+), 6 deletions(-) diff --git a/packages/react/src/SelectPanel/SelectPanel.test.tsx b/packages/react/src/SelectPanel/SelectPanel.test.tsx index 32a3ade1f74..f8b151a014b 100644 --- a/packages/react/src/SelectPanel/SelectPanel.test.tsx +++ b/packages/react/src/SelectPanel/SelectPanel.test.tsx @@ -1197,6 +1197,49 @@ for (const usingRemoveActiveDescendant of [false, true]) { expect(screen.getByRole('button', {name: 'Cancel'})).toBeInTheDocument() }) + it('keeps the intermediate selection after clicking an item in a single-select modal', async () => { + const user = userEvent.setup() + + const singleSelectItems: SelectPanelProps['items'] = [ + {id: '1', text: 'item one'}, + {id: '2', text: 'item two'}, + {id: '3', text: 'item three'}, + ] + + function SingleSelectModalPanel() { + const [selected, setSelected] = React.useState(undefined) + const [filter, setFilter] = React.useState('') + const [open, setOpen] = React.useState(false) + + return ( + {}} + items={singleSelectItems} + placeholder="Select items" + placeholderText="Filter items" + selected={selected} + onSelectedChange={setSelected} + filterValue={filter} + onFilterChange={setFilter} + open={open} + onOpenChange={setOpen} + /> + ) + } + + renderWithProp(, usingRemoveActiveDescendant) + + await user.click(screen.getByText('Select items')) + + // Selecting an item must persist. A stale intermediate-selection reset key + // previously reverted the selection on the very next render, so the option + // never stayed selected. + await user.click(screen.getByRole('option', {name: 'item one'})) + expect(screen.getByRole('option', {name: 'item one'})).toHaveAttribute('aria-selected', 'true') + }) + it('locks body scroll when modal is open', async () => { const user = userEvent.setup() diff --git a/packages/react/src/SelectPanel/SelectPanel.tsx b/packages/react/src/SelectPanel/SelectPanel.tsx index 3d991a3d2e6..523765383ad 100644 --- a/packages/react/src/SelectPanel/SelectPanel.tsx +++ b/packages/react/src/SelectPanel/SelectPanel.tsx @@ -247,10 +247,7 @@ function Panel({ // Reset the intermediate selected item when the panel is opened/closed or the external // selection changes. Adjusting state during render (tracking the previous inputs) rather - // than syncing it from an effect avoids an extra post-commit render. The reset is also - // gated on the intermediate selection actually changing, so variants that never use it - // (e.g. multi-select / anchored, where it stays `undefined`) don't pay for a render-phase - // restart on every open/selection change. + // than syncing it from an effect avoids an extra post-commit render. const nextIntermediateSelected = isSingleSelectModal ? selected : undefined const [intermediateSelectedResetKey, setIntermediateSelectedResetKey] = useState({ open, @@ -261,9 +258,17 @@ function Panel({ intermediateSelectedResetKey.open !== open || intermediateSelectedResetKey.selected !== selected || intermediateSelectedResetKey.isSingleSelectModal !== isSingleSelectModal - if (intermediateSelectedInputsChanged && intermediateSelected !== nextIntermediateSelected) { + if (intermediateSelectedInputsChanged) { + // Always advance the reset key when the inputs change so it never goes stale. If the key + // only advanced when the value also differed, it could stay behind while the panel is + // open and clobber the next user-driven `setIntermediateSelected` on the following render + // (breaking single-select modal interaction). Only reset the intermediate selection when + // it actually differs, so multi-select / anchored variants (where it stays `undefined`) + // don't call `setIntermediateSelected` at all. setIntermediateSelectedResetKey({open, selected, isSingleSelectModal}) - setIntermediateSelected(nextIntermediateSelected) + if (intermediateSelected !== nextIntermediateSelected) { + setIntermediateSelected(nextIntermediateSelected) + } } const onListContainerRefChanged: FilteredActionListProps['onListContainerRefChanged'] = useCallback(