Skip to content
Open
Show file tree
Hide file tree
Changes from all 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-intermediate-selected-derive.md
Original file line number Diff line number Diff line change
@@ -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.
43 changes: 43 additions & 0 deletions packages/react/src/SelectPanel/SelectPanel.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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<ItemInput | undefined>(undefined)
const [filter, setFilter] = React.useState('')
const [open, setOpen] = React.useState(false)

return (
<SelectPanel
title="test title"
variant="modal"
onCancel={() => {}}
items={singleSelectItems}
placeholder="Select items"
placeholderText="Filter items"
selected={selected}
onSelectedChange={setSelected}
filterValue={filter}
onFilterChange={setFilter}
open={open}
onOpenChange={setOpen}
/>
)
}

renderWithProp(<SingleSelectModalPanel />, 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()

Expand Down
30 changes: 25 additions & 5 deletions packages/react/src/SelectPanel/SelectPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -245,11 +245,31 @@ 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
setIntermediateSelected(isSingleSelectModal ? selected : undefined)
}, [isSingleSelectModal, open, selected])
// 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 nextIntermediateSelected = isSingleSelectModal ? selected : undefined
const [intermediateSelectedResetKey, setIntermediateSelectedResetKey] = useState({
open,
selected,
isSingleSelectModal,
})
const intermediateSelectedInputsChanged =
intermediateSelectedResetKey.open !== open ||
intermediateSelectedResetKey.selected !== selected ||
intermediateSelectedResetKey.isSingleSelectModal !== isSingleSelectModal
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})
if (intermediateSelected !== nextIntermediateSelected) {
setIntermediateSelected(nextIntermediateSelected)
}
}
Comment thread
mattcosta7 marked this conversation as resolved.

const onListContainerRefChanged: FilteredActionListProps['onListContainerRefChanged'] = useCallback(
(node: HTMLElement | null) => {
Expand Down
Loading