Skip to content

Commit 5446544

Browse files
authored
Prevent ActionList crash when selected prop is used without selectionVariant (#6235)
1 parent 0055f75 commit 5446544

File tree

3 files changed

+26
-20
lines changed

3 files changed

+26
-20
lines changed

.changeset/true-lines-find.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@primer/react': patch
3+
---
4+
5+
Prevent ActionList crash when selected prop is true without selectionVariant.

packages/react/src/ActionList/ActionList.test.tsx

Lines changed: 17 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -30,21 +30,24 @@ describe('ActionList', () => {
3030
expect(results).toHaveNoViolations()
3131
})
3232

33-
it('should throw when selected is provided without a selectionVariant on parent', async () => {
34-
// we expect console.error to be called, so we suppress that in the test
35-
const mockError = vi.spyOn(console, 'error').mockImplementation(() => vi.fn())
36-
37-
expect(() => {
38-
HTMLRender(
39-
<ActionList showDividers role="listbox" aria-label="Select a project">
40-
<ActionList.Item role="option" selected={true}>
41-
Primer React
42-
</ActionList.Item>
43-
</ActionList>,
44-
)
45-
}).toThrow('For Item to be selected, ActionList or ActionList.Group needs to have a selectionVariant defined')
33+
it('should warn when selected is provided without a selectionVariant on parent', async () => {
34+
// we expect console.warn to be called, so we spy on that in the test
35+
const spy = vi.spyOn(console, 'warn').mockImplementation(() => vi.fn())
36+
37+
HTMLRender(
38+
<ActionList showDividers role="listbox" aria-label="Select a project">
39+
<ActionList.Item role="option" selected={true}>
40+
Primer React
41+
</ActionList.Item>
42+
</ActionList>,
43+
)
44+
45+
expect(spy).toHaveBeenCalledWith(
46+
'Warning:',
47+
'For Item to be selected, ActionList or ActionList.Group should have a selectionVariant defined.',
48+
)
4649

47-
mockError.mockRestore()
50+
spy.mockRestore()
4851
})
4952

5053
it('should be navigatable with arrow keys for certain roles', async () => {

packages/react/src/ActionList/Selection.tsx

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import {type ActionListProps, type ActionListItemProps, ListContext} from './sha
66
import {VisualContainer} from './Visuals'
77
import classes from './ActionList.module.css'
88
import Radio from '../Radio'
9+
import {warning} from '../utils/warning'
910

1011
type SelectionProps = Pick<ActionListItemProps, 'selected' | 'className'>
1112
export const Selection: React.FC<React.PropsWithChildren<SelectionProps>> = ({selected, className}) => {
@@ -20,14 +21,11 @@ export const Selection: React.FC<React.PropsWithChildren<SelectionProps>> = ({se
2021

2122
if (!selectionVariant) {
2223
// if selectionVariant is not set on List, but Item is selected
23-
// fail loudly instead of silently ignoring
24+
// warn in development
2425
if (selected) {
25-
throw new Error(
26-
'For Item to be selected, ActionList or ActionList.Group needs to have a selectionVariant defined',
27-
)
28-
} else {
29-
return null
26+
warning(true, 'For Item to be selected, ActionList or ActionList.Group should have a selectionVariant defined.')
3027
}
28+
return null
3129
}
3230

3331
if (selectionVariant === 'radio') {

0 commit comments

Comments
 (0)