diff --git a/.changeset/violet-yaks-lie.md b/.changeset/violet-yaks-lie.md new file mode 100644 index 00000000000..c17720f1334 --- /dev/null +++ b/.changeset/violet-yaks-lie.md @@ -0,0 +1,5 @@ +--- +'@primer/react': patch +--- + +SelectPanel: Fixes a bug in `SelectPanel` when the list of items was not memoized. diff --git a/packages/react/src/SelectPanel/SelectPanel.test.tsx b/packages/react/src/SelectPanel/SelectPanel.test.tsx index fa9b8771dfb..474ef3499fe 100644 --- a/packages/react/src/SelectPanel/SelectPanel.test.tsx +++ b/packages/react/src/SelectPanel/SelectPanel.test.tsx @@ -1,5 +1,6 @@ -import {render as HTMLRender} from '@testing-library/react' +import {render as HTMLRender, fireEvent, getByRole, getByText} from '@testing-library/react' import axe from 'axe-core' + import React from 'react' import theme from '../theme' import {SelectPanel} from '../SelectPanel' @@ -7,9 +8,14 @@ import {behavesAsComponent, checkExports} from '../utils/testing' import {BaseStyles, SSRProvider, ThemeProvider} from '..' import type {ItemInput} from '../deprecated/ActionList/List' -const items = [{text: 'Foo'}, {text: 'Bar'}, {text: 'Baz'}, {text: 'Bon'}] as ItemInput[] - function SimpleSelectPanel(): JSX.Element { + const items = [ + {text: 'Foo', id: 'foo'}, + {text: 'Bar', id: 'bar'}, + {text: 'Baz', id: 'baz'}, + {text: 'Bon', id: 'bon'}, + ] as ItemInput[] + const [selected, setSelected] = React.useState([]) const [, setFilter] = React.useState('') const [open, setOpen] = React.useState(false) @@ -27,6 +33,7 @@ function SimpleSelectPanel(): JSX.Element { onFilterChange={setFilter} open={open} onOpenChange={setOpen} + overlayProps={{width: 'medium', height: 'medium'}} />
@@ -36,10 +43,18 @@ function SimpleSelectPanel(): JSX.Element { } describe('SelectPanel', () => { + const originalScrollTo = Element.prototype.scrollTo + beforeAll(() => { + Element.prototype.scrollTo = () => {} + }) afterEach(() => { jest.clearAllMocks() }) + afterAll(() => { + Element.prototype.scrollTo = originalScrollTo + }) + behavesAsComponent({ Component: SelectPanel, options: {skipAs: true, skipSx: true}, @@ -56,4 +71,20 @@ describe('SelectPanel', () => { const results = await axe.run(container) expect(results).toHaveNoViolations() }) + + it('should render selected items', () => { + const item = HTMLRender() + const {container} = item + const button = getByRole(container, 'button') + expect(button).toBeTruthy() + + fireEvent.click(button) + const selector = getByText(container, 'Foo') + expect(selector).toBeVisible() + + fireEvent.click(selector) + + const selected = getByRole(container, 'option', {selected: true}) + expect(selected).toBeTruthy() + }) }) diff --git a/packages/react/src/SelectPanel/SelectPanel.tsx b/packages/react/src/SelectPanel/SelectPanel.tsx index 73bbcaa5948..e937bfe496c 100644 --- a/packages/react/src/SelectPanel/SelectPanel.tsx +++ b/packages/react/src/SelectPanel/SelectPanel.tsx @@ -126,9 +126,24 @@ export function SelectPanel({ } }, [placeholder, renderAnchor, selected]) + const testIsItemSelected = React.useCallback((selectedItem: ItemInput, item: ItemInput) => { + const itemType = typeof selectedItem + if (itemType === 'object') { + if (selectedItem.hasOwnProperty('id')) { + return selectedItem.id === item.id + } + } + + return selectedItem === item + }, []) + const itemsToRender = useMemo(() => { return items.map(item => { - const isItemSelected = isMultiSelectVariant(selected) ? selected.includes(item) : selected === item + const isItemSelected = isMultiSelectVariant(selected) + ? selected.some(selectedItem => { + return testIsItemSelected(selectedItem, item) + }) + : selected === item return { ...item, @@ -142,8 +157,13 @@ export function SelectPanel({ } if (isMultiSelectVariant(selected)) { - const otherSelectedItems = selected.filter(selectedItem => selectedItem !== item) - const newSelectedItems = selected.includes(item) ? otherSelectedItems : [...otherSelectedItems, item] + const wasPreviouslySelected = selected.some(selectedItem => { + return testIsItemSelected(selectedItem, item) + }) + const otherSelectedItems = selected.filter(selectedItem => { + return !testIsItemSelected(selectedItem, item) + }) + const newSelectedItems = wasPreviouslySelected ? otherSelectedItems : [...otherSelectedItems, item] const multiSelectOnChange = onSelectedChange as SelectPanelMultiSelection['onSelectedChange'] multiSelectOnChange(newSelectedItems) @@ -157,7 +177,7 @@ export function SelectPanel({ }, } as ItemProps }) - }, [onClose, onSelectedChange, items, selected]) + }, [onClose, onSelectedChange, items, selected, testIsItemSelected]) const inputRef = React.useRef(null) const focusTrapSettings = {