From 71da567c1ec31dab959c50f431cba4915b18a342 Mon Sep 17 00:00:00 2001 From: onx2 Date: Fri, 21 Nov 2025 07:03:31 -0500 Subject: [PATCH 1/5] add onAction to type pick --- packages/@react-aria/tag/src/useTagGroup.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@react-aria/tag/src/useTagGroup.ts b/packages/@react-aria/tag/src/useTagGroup.ts index 11b32c9508a..2023d11f2bb 100644 --- a/packages/@react-aria/tag/src/useTagGroup.ts +++ b/packages/@react-aria/tag/src/useTagGroup.ts @@ -31,7 +31,7 @@ export interface TagGroupAria { errorMessageProps: DOMAttributes } -export interface AriaTagGroupProps extends CollectionBase, MultipleSelection, Pick, 'escapeKeyBehavior'>, DOMProps, LabelableProps, AriaLabelingProps, Omit { +export interface AriaTagGroupProps extends CollectionBase, MultipleSelection, Pick, 'escapeKeyBehavior' | 'onAction'>, DOMProps, LabelableProps, AriaLabelingProps, Omit { /** How multiple selection should behave in the collection. */ selectionBehavior?: SelectionBehavior, /** Whether selection should occur on press up instead of press down. */ From d01522de6687055a0831e91d5693f17ba748cffb Mon Sep 17 00:00:00 2001 From: Robert Snow Date: Thu, 18 Dec 2025 10:58:44 +1100 Subject: [PATCH 2/5] add tests --- .../test/TagGroup.test.js | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/packages/react-aria-components/test/TagGroup.test.js b/packages/react-aria-components/test/TagGroup.test.js index adc4f07135b..7c50f6f0317 100644 --- a/packages/react-aria-components/test/TagGroup.test.js +++ b/packages/react-aria-components/test/TagGroup.test.js @@ -521,6 +521,48 @@ describe('TagGroup', () => { expect(onRemove).toHaveBeenLastCalledWith(new Set(['dog'])); }); + it('should support onAction', async () => { + let onAction = jest.fn(); + let {getAllByRole} = renderTagGroup({onAction, selectionMode: 'none'}); + let items = getAllByRole('row'); + + await user.click({target: items[0]}); + expect(onAction).toHaveBeenCalledTimes(1); + }); + + it('should support onAction with selectionMode = single', async () => { + let onAction = jest.fn(); + let {getAllByRole} = renderTagGroup({onAction, selectionMode: 'single'}); + let items = getAllByRole('row'); + + await user.dblClick({target: items[0]}); + expect(onAction).toHaveBeenCalledTimes(1); + onAction.mockReset(); + + await user.click({target: items[1]}); + expect(onAction).not.toHaveBeenCalled(); + + await user.dblClick({target: items[0]}); + expect(onAction).toHaveBeenCalledTimes(1); + }); + + it('should support onAction with selectionMode = multiple', async () => { + let onAction = jest.fn(); + let {getAllByRole} = renderTagGroup({onAction, selectionMode: 'multiple'}); + let items = getAllByRole('row'); + + await user.dblClick({target: items[0]}); + expect(onAction).toHaveBeenCalledTimes(1); + onAction.mockReset(); + + await user.click({target: items[1]}); + expect(onAction).not.toHaveBeenCalled(); + onAction.mockReset(); + + await user.dblClick({target: items[0]}); + expect(onAction).toHaveBeenCalledTimes(1); + }); + describe('shouldSelectOnPressUp', () => { it('should select an item on pressing down when shouldSelectOnPressUp is not provided', async () => { let onSelectionChange = jest.fn(); From e9ffe4f165697316ebc12d628467b00cd366c964 Mon Sep 17 00:00:00 2001 From: Robert Snow Date: Thu, 18 Dec 2025 11:00:48 +1100 Subject: [PATCH 3/5] confirm selection happened --- packages/react-aria-components/test/TagGroup.test.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/react-aria-components/test/TagGroup.test.js b/packages/react-aria-components/test/TagGroup.test.js index 7c50f6f0317..81e5af0fde3 100644 --- a/packages/react-aria-components/test/TagGroup.test.js +++ b/packages/react-aria-components/test/TagGroup.test.js @@ -541,6 +541,7 @@ describe('TagGroup', () => { await user.click({target: items[1]}); expect(onAction).not.toHaveBeenCalled(); + expect(items[1]).toHaveAttribute('aria-selected', 'true'); await user.dblClick({target: items[0]}); expect(onAction).toHaveBeenCalledTimes(1); @@ -558,6 +559,7 @@ describe('TagGroup', () => { await user.click({target: items[1]}); expect(onAction).not.toHaveBeenCalled(); onAction.mockReset(); + expect(items[1]).toHaveAttribute('aria-selected', 'true'); await user.dblClick({target: items[0]}); expect(onAction).toHaveBeenCalledTimes(1); From c5e8593765bc8a295d4c7fed2e773e472598acc3 Mon Sep 17 00:00:00 2001 From: Robert Snow Date: Thu, 18 Dec 2025 11:03:22 +1100 Subject: [PATCH 4/5] add keyboard tests --- .../react-aria-components/test/TagGroup.test.js | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/packages/react-aria-components/test/TagGroup.test.js b/packages/react-aria-components/test/TagGroup.test.js index 81e5af0fde3..616a3b36f61 100644 --- a/packages/react-aria-components/test/TagGroup.test.js +++ b/packages/react-aria-components/test/TagGroup.test.js @@ -528,6 +528,10 @@ describe('TagGroup', () => { await user.click({target: items[0]}); expect(onAction).toHaveBeenCalledTimes(1); + onAction.mockReset(); + + await user.keyboard('{Enter}'); + expect(onAction).toHaveBeenCalledTimes(1); }); it('should support onAction with selectionMode = single', async () => { @@ -545,6 +549,12 @@ describe('TagGroup', () => { await user.dblClick({target: items[0]}); expect(onAction).toHaveBeenCalledTimes(1); + onAction.mockReset(); + + await user.keyboard('{Enter}'); + expect(onAction).not.toHaveBeenCalled(); + expect(items[0]).toHaveAttribute('aria-selected', 'true'); + expect(items[1]).not.toHaveAttribute('aria-selected'); }); it('should support onAction with selectionMode = multiple', async () => { @@ -563,6 +573,12 @@ describe('TagGroup', () => { await user.dblClick({target: items[0]}); expect(onAction).toHaveBeenCalledTimes(1); + onAction.mockReset(); + + await user.keyboard('{Enter}'); + expect(onAction).not.toHaveBeenCalled(); + expect(items[0]).toHaveAttribute('aria-selected', 'true'); + expect(items[1]).toHaveAttribute('aria-selected', 'true'); }); describe('shouldSelectOnPressUp', () => { From 0851eb8eb86e30e874535968db848ba8f3905110 Mon Sep 17 00:00:00 2001 From: Robert Snow Date: Thu, 18 Dec 2025 11:39:48 +1100 Subject: [PATCH 5/5] fix defaults and split tests on selection behaviour --- packages/@react-aria/tag/src/useTagGroup.ts | 5 +- .../src/useMultipleSelectionState.ts | 10 ++- .../test/TagGroup.test.js | 75 ++++++++++++++++--- 3 files changed, 77 insertions(+), 13 deletions(-) diff --git a/packages/@react-aria/tag/src/useTagGroup.ts b/packages/@react-aria/tag/src/useTagGroup.ts index 2023d11f2bb..d8578594b7e 100644 --- a/packages/@react-aria/tag/src/useTagGroup.ts +++ b/packages/@react-aria/tag/src/useTagGroup.ts @@ -32,7 +32,10 @@ export interface TagGroupAria { } export interface AriaTagGroupProps extends CollectionBase, MultipleSelection, Pick, 'escapeKeyBehavior' | 'onAction'>, DOMProps, LabelableProps, AriaLabelingProps, Omit { - /** How multiple selection should behave in the collection. */ + /** + * How multiple selection should behave in the collection. + * @default 'toggle' + */ selectionBehavior?: SelectionBehavior, /** Whether selection should occur on press up instead of press down. */ shouldSelectOnPressUp?: boolean, diff --git a/packages/@react-stately/selection/src/useMultipleSelectionState.ts b/packages/@react-stately/selection/src/useMultipleSelectionState.ts index e58d0823850..f7893f47764 100644 --- a/packages/@react-stately/selection/src/useMultipleSelectionState.ts +++ b/packages/@react-stately/selection/src/useMultipleSelectionState.ts @@ -31,11 +31,17 @@ function equalSets(setA, setB) { } export interface MultipleSelectionStateProps extends MultipleSelection { - /** How multiple selection should behave in the collection. */ + /** + * How multiple selection should behave in the collection. + * @default 'toggle' + */ selectionBehavior?: SelectionBehavior, /** Whether onSelectionChange should fire even if the new set of keys is the same as the last. */ allowDuplicateSelectionEvents?: boolean, - /** Whether `disabledKeys` applies to all interactions, or only selection. */ + /** + * Whether `disabledKeys` applies to all interactions, or only selection. + * @default 'all' + */ disabledBehavior?: DisabledBehavior } diff --git a/packages/react-aria-components/test/TagGroup.test.js b/packages/react-aria-components/test/TagGroup.test.js index 616a3b36f61..94173ed3d73 100644 --- a/packages/react-aria-components/test/TagGroup.test.js +++ b/packages/react-aria-components/test/TagGroup.test.js @@ -526,7 +526,7 @@ describe('TagGroup', () => { let {getAllByRole} = renderTagGroup({onAction, selectionMode: 'none'}); let items = getAllByRole('row'); - await user.click({target: items[0]}); + await user.click(items[0]); expect(onAction).toHaveBeenCalledTimes(1); onAction.mockReset(); @@ -534,21 +534,75 @@ describe('TagGroup', () => { expect(onAction).toHaveBeenCalledTimes(1); }); - it('should support onAction with selectionMode = single', async () => { + it('should support onAction with selectionMode = single, behaviour = replace', async () => { + let onAction = jest.fn(); + let {getAllByRole} = renderTagGroup({onAction, selectionMode: 'single', selectionBehavior: 'replace'}); + let items = getAllByRole('row'); + + await user.dblClick(items[0]); + expect(onAction).toHaveBeenCalledTimes(1); + onAction.mockReset(); + + await user.click(items[1]); + expect(onAction).not.toHaveBeenCalled(); + expect(items[1]).toHaveAttribute('aria-selected', 'true'); + + await user.dblClick(items[0]); + expect(onAction).toHaveBeenCalledTimes(1); + expect(items[0]).toHaveAttribute('aria-selected', 'false'); // should be true? + expect(items[1]).toHaveAttribute('aria-selected', 'false'); + onAction.mockReset(); + + await user.keyboard('{Enter}'); + expect(onAction).toHaveBeenCalledTimes(1); + expect(items[0]).toHaveAttribute('aria-selected', 'false'); // should be true? + expect(items[1]).toHaveAttribute('aria-selected', 'false'); + }); + + it('should support onAction with selectionMode = multiple, behaviour = replace', async () => { + let onAction = jest.fn(); + let {getAllByRole} = renderTagGroup({onAction, selectionMode: 'multiple', selectionBehavior: 'replace'}); + let items = getAllByRole('row'); + + await user.dblClick(items[0]); + expect(onAction).toHaveBeenCalledTimes(1); + onAction.mockReset(); + + await user.click(items[1]); + expect(onAction).not.toHaveBeenCalled(); + onAction.mockReset(); + expect(items[1]).toHaveAttribute('aria-selected', 'true'); + + await user.dblClick(items[0]); + expect(onAction).toHaveBeenCalledTimes(1); + expect(items[0]).toHaveAttribute('aria-selected', 'true'); + expect(items[1]).toHaveAttribute('aria-selected', 'false'); + onAction.mockReset(); + + await user.keyboard('{Enter}'); + expect(onAction).toHaveBeenCalledTimes(1); + expect(items[0]).toHaveAttribute('aria-selected', 'true'); + expect(items[1]).toHaveAttribute('aria-selected', 'false'); + }); + + // TODO: What do we want to do for this behaviour? Should we warn that it's not a valid combination? Or should it react to double click? + // Replace selectionBehavior works with double click, but toggle doesn't. + it.skip('should support onAction with selectionMode = single, behaviour = toggle', async () => { let onAction = jest.fn(); let {getAllByRole} = renderTagGroup({onAction, selectionMode: 'single'}); let items = getAllByRole('row'); - await user.dblClick({target: items[0]}); + await user.dblClick(items[0]); expect(onAction).toHaveBeenCalledTimes(1); onAction.mockReset(); - await user.click({target: items[1]}); + await user.click(items[1]); expect(onAction).not.toHaveBeenCalled(); expect(items[1]).toHaveAttribute('aria-selected', 'true'); - await user.dblClick({target: items[0]}); + await user.dblClick(items[0]); expect(onAction).toHaveBeenCalledTimes(1); + expect(items[0]).not.toHaveAttribute('aria-selected'); onAction.mockReset(); await user.keyboard('{Enter}'); @@ -557,22 +611,23 @@ describe('TagGroup', () => { expect(items[1]).not.toHaveAttribute('aria-selected'); }); - it('should support onAction with selectionMode = multiple', async () => { + it.skip('should support onAction with selectionMode = multiple, behaviour = toggle', async () => { let onAction = jest.fn(); let {getAllByRole} = renderTagGroup({onAction, selectionMode: 'multiple'}); let items = getAllByRole('row'); - await user.dblClick({target: items[0]}); + await user.dblClick(items[0]); expect(onAction).toHaveBeenCalledTimes(1); onAction.mockReset(); - await user.click({target: items[1]}); + await user.click(items[1]); expect(onAction).not.toHaveBeenCalled(); onAction.mockReset(); expect(items[1]).toHaveAttribute('aria-selected', 'true'); - await user.dblClick({target: items[0]}); + await user.dblClick(items[0]); expect(onAction).toHaveBeenCalledTimes(1); + expect(items[0]).not.toHaveAttribute('aria-selected'); onAction.mockReset(); await user.keyboard('{Enter}'); @@ -620,7 +675,7 @@ describe('TagGroup', () => { }); describe('press events', () => { - it.only.each` + it.each` interactionType ${'mouse'} ${'keyboard'}