From a32b16e4283210dad39274fda804e5ee7857b2fe Mon Sep 17 00:00:00 2001 From: Daniel Lu Date: Wed, 29 Jan 2025 16:54:32 -0800 Subject: [PATCH 01/17] attempt to get rid of jest calls in menu util --- packages/@react-aria/test-utils/src/menu.ts | 22 +++++++++++++++++--- packages/@react-aria/test-utils/src/types.ts | 2 +- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/packages/@react-aria/test-utils/src/menu.ts b/packages/@react-aria/test-utils/src/menu.ts index f8cf4a346cf..a8805724afe 100644 --- a/packages/@react-aria/test-utils/src/menu.ts +++ b/packages/@react-aria/test-utils/src/menu.ts @@ -241,6 +241,17 @@ export class MenuTester { throw new Error('Expected menu element to not be in the document after selecting an option'); } } + // else { + // // TODO: this is a bit unfortunate, but from a submenu point of view, we can't perform a check that waits for + // // focus to return to the original trigger since we don't have that information in the submenu trigger tester. Even if we did + // // it is a bit unpredicatable where focus might end up landing and waiting for the option/menu to disappear isn't sufficient if there are + // // more transitions in play. For now advance times by a certain amount of time and rely on the user to advance them even more if need be + // if (this._advanceTimer == null) { + // throw new Error('No advanceTimers provided for long press.'); + // } else { + // await act(async () => await this._advanceTimer(1000)); + // } + // } } else { throw new Error("Attempted to select a option in the menu, but menu wasn't found."); } @@ -269,18 +280,23 @@ export class MenuTester { submenuTrigger = (within(menu!).getByText(submenuTrigger).closest('[role=menuitem]'))! as HTMLElement; } - let submenuTriggerTester = new MenuTester({user: this.user, interactionType: this._interactionType, root: submenuTrigger, isSubmenu: true}); + let submenuTriggerTester = new MenuTester({user: this.user, interactionType: this._interactionType, root: submenuTrigger, isSubmenu: true, advanceTimer: this._advanceTimer}); if (interactionType === 'mouse') { await this.user.pointer({target: submenuTrigger}); - act(() => {jest.runAllTimers();}); } else if (interactionType === 'keyboard') { await this.keyboardNavigateToOption({option: submenuTrigger}); await this.user.keyboard('[ArrowRight]'); - act(() => {jest.runAllTimers();}); } else { await submenuTriggerTester.open(); } + await waitFor(() => { + if (submenuTriggerTester._trigger?.getAttribute('aria-expanded') !== 'true') { + throw new Error('aria-expanded for the submenu trigger wasn\'t changed to "true", unable to confirm the existance of the submenu'); + } else { + return true; + } + }); return submenuTriggerTester; } diff --git a/packages/@react-aria/test-utils/src/types.ts b/packages/@react-aria/test-utils/src/types.ts index 3c58dbc61f3..c749a9dba95 100644 --- a/packages/@react-aria/test-utils/src/types.ts +++ b/packages/@react-aria/test-utils/src/types.ts @@ -29,7 +29,7 @@ export interface UserOpts { * A function used by the test utils to advance timers during interactions. Required for certain aria patterns (e.g. table). This can be overridden * at the aria pattern tester level if needed. */ - advanceTimer?: (time?: number) => void | Promise + advanceTimer?: (time?: number) => void | Promise | any } export interface BaseTesterOpts extends UserOpts { From e326a356aa974959b7f9e4614f5efd5fd60eae8b Mon Sep 17 00:00:00 2001 From: Daniel Lu Date: Wed, 29 Jan 2025 17:01:04 -0800 Subject: [PATCH 02/17] update RSP testing docs to directly mention mocks that maybe needed --- packages/@react-spectrum/combobox/docs/ComboBox.mdx | 2 +- packages/@react-spectrum/list/docs/ListView.mdx | 2 +- packages/@react-spectrum/listbox/docs/ListBox.mdx | 2 +- packages/@react-spectrum/menu/docs/MenuTrigger.mdx | 2 +- packages/@react-spectrum/picker/docs/Picker.mdx | 2 +- packages/@react-spectrum/table/docs/TableView.mdx | 2 +- packages/@react-spectrum/tabs/docs/Tabs.mdx | 2 +- packages/@react-spectrum/tree/docs/TreeView.mdx | 2 +- 8 files changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/@react-spectrum/combobox/docs/ComboBox.mdx b/packages/@react-spectrum/combobox/docs/ComboBox.mdx index b4000f4de81..20d2e86936e 100644 --- a/packages/@react-spectrum/combobox/docs/ComboBox.mdx +++ b/packages/@react-spectrum/combobox/docs/ComboBox.mdx @@ -1006,7 +1006,7 @@ import {theme} from '@react-spectrum/theme-default'; import {User} from '@react-spectrum/test-utils'; let testUtilUser = new User({interactionType: 'mouse'}); -// ... +// Other setup, be sure to check out the suggested mocks mentioned above in https://react-spectrum.adobe.com/react-spectrum/ComboBox.html#testing it('ComboBox can select an option via keyboard', async function () { // Render your test component/app and initialize the combobox tester diff --git a/packages/@react-spectrum/list/docs/ListView.mdx b/packages/@react-spectrum/list/docs/ListView.mdx index 73c326ebae5..553410132a7 100644 --- a/packages/@react-spectrum/list/docs/ListView.mdx +++ b/packages/@react-spectrum/list/docs/ListView.mdx @@ -1205,7 +1205,7 @@ import {theme} from '@react-spectrum/theme-default'; import {User} from '@react-spectrum/test-utils'; let testUtilUser = new User({interactionType: 'mouse'}); -// ... +// Other setup, be sure to check out the suggested mocks mentioned above in https://react-spectrum.adobe.com/react-spectrum/ListView.html#testing it('ListView can select a row via keyboard', async function () { // Render your test component/app and initialize the gridlist tester diff --git a/packages/@react-spectrum/listbox/docs/ListBox.mdx b/packages/@react-spectrum/listbox/docs/ListBox.mdx index 56de8d1e1e1..93a1a1ad259 100644 --- a/packages/@react-spectrum/listbox/docs/ListBox.mdx +++ b/packages/@react-spectrum/listbox/docs/ListBox.mdx @@ -423,7 +423,7 @@ import {theme} from '@react-spectrum/theme-default'; import {User} from '@react-spectrum/test-utils'; let testUtilUser = new User({interactionType: 'mouse'}); -// ... +// Other setup, be sure to check out the suggested mocks mentioned above in https://react-spectrum.adobe.com/react-spectrum/ListBox.html#testing it('ListBox can select an option via keyboard', async function () { // Render your test component/app and initialize the listbox tester diff --git a/packages/@react-spectrum/menu/docs/MenuTrigger.mdx b/packages/@react-spectrum/menu/docs/MenuTrigger.mdx index 4641da4f53e..6d90a7dc207 100644 --- a/packages/@react-spectrum/menu/docs/MenuTrigger.mdx +++ b/packages/@react-spectrum/menu/docs/MenuTrigger.mdx @@ -270,7 +270,7 @@ import {theme} from '@react-spectrum/theme-default'; import {User} from '@react-spectrum/test-utils'; let testUtilUser = new User({interactionType: 'mouse'}); -// ... +// Other setup, be sure to check out the suggested mocks mentioned above in https://react-spectrum.adobe.com/react-spectrum/MenuTrigger.html#testing it('Menu can open its submenu via keyboard', async function () { // Render your test component/app and initialize the menu tester diff --git a/packages/@react-spectrum/picker/docs/Picker.mdx b/packages/@react-spectrum/picker/docs/Picker.mdx index 323384fcc5f..3a00852e26a 100644 --- a/packages/@react-spectrum/picker/docs/Picker.mdx +++ b/packages/@react-spectrum/picker/docs/Picker.mdx @@ -602,7 +602,7 @@ import {theme} from '@react-spectrum/theme-default'; import {User} from '@react-spectrum/test-utils'; let testUtilUser = new User({interactionType: 'mouse'}); -// ... +// Other setup, be sure to check out the suggested mocks mentioned above in https://react-spectrum.adobe.com/react-spectrum/Picker.html#testing it('Picker can select an option via keyboard', async function () { // Render your test component/app and initialize the select tester diff --git a/packages/@react-spectrum/table/docs/TableView.mdx b/packages/@react-spectrum/table/docs/TableView.mdx index facd9856abd..1ccb1b3f604 100644 --- a/packages/@react-spectrum/table/docs/TableView.mdx +++ b/packages/@react-spectrum/table/docs/TableView.mdx @@ -1971,7 +1971,7 @@ import {theme} from '@react-spectrum/theme-default'; import {User} from '@react-spectrum/test-utils'; let testUtilUser = new User({interactionType: 'mouse', advanceTimer: jest.advanceTimersByTime}); -// ... +// Other setup, be sure to check out the suggested mocks mentioned above in https://react-spectrum.adobe.com/react-spectrum/TableView.html#testing it('TableView can toggle row selection', async function () { // Render your test component/app and initialize the table tester diff --git a/packages/@react-spectrum/tabs/docs/Tabs.mdx b/packages/@react-spectrum/tabs/docs/Tabs.mdx index 85fa97d208c..27f655b55ec 100644 --- a/packages/@react-spectrum/tabs/docs/Tabs.mdx +++ b/packages/@react-spectrum/tabs/docs/Tabs.mdx @@ -649,7 +649,7 @@ import {theme} from '@react-spectrum/theme-default'; import {User} from '@react-spectrum/test-utils'; let testUtilUser = new User({interactionType: 'mouse'}); -// ... +// Other setup, be sure to check out the suggested mocks mentioned above in https://react-spectrum.adobe.com/react-spectrum/Tabs.html#testing it('Tabs can change selection via keyboard', async function () { // Render your test component/app and initialize the listbox tester diff --git a/packages/@react-spectrum/tree/docs/TreeView.mdx b/packages/@react-spectrum/tree/docs/TreeView.mdx index 9e1fd3a0d75..bd0ae9ba489 100644 --- a/packages/@react-spectrum/tree/docs/TreeView.mdx +++ b/packages/@react-spectrum/tree/docs/TreeView.mdx @@ -470,7 +470,7 @@ import {theme} from '@react-spectrum/theme-default'; import {User} from '@react-spectrum/test-utils'; let testUtilUser = new User({interactionType: 'mouse'}); -// ... +// Other setup, be sure to check out the suggested mocks mentioned above in https://react-spectrum.adobe.com/react-spectrum/TreeView.html#testing it('TreeView can select a row via keyboard', async function () { // Render your test component/app and initialize the Tree tester From 948167393d9077a79b837a9af191e9add5ac5a5f Mon Sep 17 00:00:00 2001 From: Daniel Lu Date: Wed, 29 Jan 2025 17:06:58 -0800 Subject: [PATCH 03/17] bump versions of RTL to 16 --- package.json | 4 +- packages/dev/test-utils/package.json | 4 +- yarn.lock | 153 ++++++++------------------- 3 files changed, 49 insertions(+), 112 deletions(-) diff --git a/package.json b/package.json index 8b895e76673..5222fe24334 100644 --- a/package.json +++ b/package.json @@ -124,8 +124,8 @@ "@swc/jest": "^0.2.36", "@tailwindcss/postcss": "^4.0.0", "@testing-library/dom": "^10.1.0", - "@testing-library/jest-dom": "^5.16.5", - "@testing-library/react": "^15.0.7", + "@testing-library/jest-dom": "^6.0.0", + "@testing-library/react": "^16.0.0", "@testing-library/user-event": "^14.6.1", "@types/react": "npm:types-react@19.0.0-rc.0", "@types/react-dom": "npm:types-react-dom@19.0.0-rc.0", diff --git a/packages/dev/test-utils/package.json b/packages/dev/test-utils/package.json index 949ccaf1ee4..b1f92d7fc27 100644 --- a/packages/dev/test-utils/package.json +++ b/packages/dev/test-utils/package.json @@ -25,8 +25,8 @@ "@react-spectrum/theme-default": "^3.5.13", "@swc/helpers": "^0.5.0", "@testing-library/dom": "^10.1.0", - "@testing-library/jest-dom": "^5.16.4", - "@testing-library/react": "^15.0.7", + "@testing-library/jest-dom": "^6.0.0", + "@testing-library/react": "^16.0.0", "@testing-library/user-event": "^14.4.3", "jest": "^29.5.0", "resolve": "^1.17.0" diff --git a/yarn.lock b/yarn.lock index 7e43f1e8063..32239a9c50f 100644 --- a/yarn.lock +++ b/yarn.lock @@ -22,10 +22,10 @@ __metadata: languageName: node linkType: hard -"@adobe/css-tools@npm:^4.0.1": - version: 4.2.0 - resolution: "@adobe/css-tools@npm:4.2.0" - checksum: 10c0/b8dbfd9c54df73a398e9b20c922abe26c67732e16afc50668402af0e3d101409e0c944baf69bf814343eb8639014637b96f209426088b06943cea288c1ef1486 +"@adobe/css-tools@npm:^4.4.0": + version: 4.4.1 + resolution: "@adobe/css-tools@npm:4.4.1" + checksum: 10c0/1a68ad9af490f45fce7b6e50dd2d8ac0c546d74431649c0d42ee4ceb1a9fa057fae0a7ef1e148effa12d84ec00ed71869ebfe0fb1dcdcc80bfcb6048c12abcc0 languageName: node linkType: hard @@ -3017,18 +3017,6 @@ __metadata: languageName: node linkType: hard -"@jest/types@npm:^25.1.0": - version: 25.1.0 - resolution: "@jest/types@npm:25.1.0" - dependencies: - "@types/istanbul-lib-coverage": "npm:^2.0.0" - "@types/istanbul-reports": "npm:^1.1.1" - "@types/yargs": "npm:^15.0.0" - chalk: "npm:^3.0.0" - checksum: 10c0/d9340658d1090f0ef3914fb91290ed2ce670eac6ad14bbdd9029041b605eccfa6f52bb30825b5c7d23b76a3736213f03caf7ed98d9063fdfd069f60748725c1b - languageName: node - linkType: hard - "@jest/types@npm:^27.4.2, @jest/types@npm:^27.5.1": version: 27.5.1 resolution: "@jest/types@npm:27.5.1" @@ -8175,8 +8163,8 @@ __metadata: "@react-spectrum/theme-default": "npm:^3.5.13" "@swc/helpers": "npm:^0.5.0" "@testing-library/dom": "npm:^10.1.0" - "@testing-library/jest-dom": "npm:^5.16.4" - "@testing-library/react": "npm:^15.0.7" + "@testing-library/jest-dom": "npm:^6.0.0" + "@testing-library/react": "npm:^16.0.0" "@testing-library/user-event": "npm:^14.4.3" jest: "npm:^29.5.0" resolve: "npm:^1.17.0" @@ -10855,20 +10843,18 @@ __metadata: languageName: node linkType: hard -"@testing-library/jest-dom@npm:^5.16.4, @testing-library/jest-dom@npm:^5.16.5": - version: 5.17.0 - resolution: "@testing-library/jest-dom@npm:5.17.0" +"@testing-library/jest-dom@npm:^6.0.0": + version: 6.6.3 + resolution: "@testing-library/jest-dom@npm:6.6.3" dependencies: - "@adobe/css-tools": "npm:^4.0.1" - "@babel/runtime": "npm:^7.9.2" - "@types/testing-library__jest-dom": "npm:^5.9.1" + "@adobe/css-tools": "npm:^4.4.0" aria-query: "npm:^5.0.0" chalk: "npm:^3.0.0" css.escape: "npm:^1.5.1" - dom-accessibility-api: "npm:^0.5.6" - lodash: "npm:^4.17.15" + dom-accessibility-api: "npm:^0.6.3" + lodash: "npm:^4.17.21" redent: "npm:^3.0.0" - checksum: 10c0/24e09c5779ea44644945ec26f2e4e5f48aecfe57d469decf2317a3253a5db28d865c55ad0ea4818d8d1df7572a6486c45daa06fa09644a833a7dd84563881939 + checksum: 10c0/5566b6c0b7b0709bc244aec3aa3dc9e5f4663e8fb2b99d8cd456fc07279e59db6076cbf798f9d3099a98fca7ef4cd50e4e1f4c4dec5a60a8fad8d24a638a5bf6 languageName: node linkType: hard @@ -10890,6 +10876,26 @@ __metadata: languageName: node linkType: hard +"@testing-library/react@npm:^16.0.0": + version: 16.2.0 + resolution: "@testing-library/react@npm:16.2.0" + dependencies: + "@babel/runtime": "npm:^7.12.5" + peerDependencies: + "@testing-library/dom": ^10.0.0 + "@types/react": ^18.0.0 || ^19.0.0 + "@types/react-dom": ^18.0.0 || ^19.0.0 + react: ^18.0.0 || ^19.0.0 + react-dom: ^18.0.0 || ^19.0.0 + peerDependenciesMeta: + "@types/react": + optional: true + "@types/react-dom": + optional: true + checksum: 10c0/7adaedaf237002b42e04a6261d2756074a19cbca0f0c79ba375660f618e123c0ee56256ced00aeb0bb7225ba1a8a81b92b692cca053521a21bb92a8cace1e4c6 + languageName: node + linkType: hard + "@testing-library/user-event@npm:^14.0.0, @testing-library/user-event@npm:^14.4.0, @testing-library/user-event@npm:^14.4.3, @testing-library/user-event@npm:^14.6.1": version: 14.6.1 resolution: "@testing-library/user-event@npm:14.6.1" @@ -11245,16 +11251,6 @@ __metadata: languageName: node linkType: hard -"@types/istanbul-reports@npm:^1.1.1": - version: 1.1.1 - resolution: "@types/istanbul-reports@npm:1.1.1" - dependencies: - "@types/istanbul-lib-coverage": "npm:*" - "@types/istanbul-lib-report": "npm:*" - checksum: 10c0/a91dccd7399231a1871f730d6d933cb8a6d3f5aa4532670348b7aae261104d5306a6add81f734fa03a629ed8d806d287a40413a19e95489676b08ede21d220fc - languageName: node - linkType: hard - "@types/istanbul-reports@npm:^3.0.0": version: 3.0.0 resolution: "@types/istanbul-reports@npm:3.0.0" @@ -11264,16 +11260,6 @@ __metadata: languageName: node linkType: hard -"@types/jest@npm:*": - version: 25.1.4 - resolution: "@types/jest@npm:25.1.4" - dependencies: - jest-diff: "npm:^25.1.0" - pretty-format: "npm:^25.1.0" - checksum: 10c0/b1ed79448840d804879326380e8fa73d6ab4fc9243b4420b3264f94292ede6bb04ca7839898e0244491b0666f9dc4b701840e84269d98ab063b04820619d66db - languageName: node - linkType: hard - "@types/jscodeshift@npm:^0.11.11": version: 0.11.11 resolution: "@types/jscodeshift@npm:0.11.11" @@ -11511,15 +11497,6 @@ __metadata: languageName: node linkType: hard -"@types/testing-library__jest-dom@npm:^5.9.1": - version: 5.9.2 - resolution: "@types/testing-library__jest-dom@npm:5.9.2" - dependencies: - "@types/jest": "npm:*" - checksum: 10c0/81431e8f95854e02a94f9cb54ff8fe0bdad66dc5bd3603b3d51dae01682484fdc4968496edcb2315a734eca6966e172ad450332fcd9e0c89b4279b898744626d - languageName: node - linkType: hard - "@types/tough-cookie@npm:*": version: 4.0.2 resolution: "@types/tough-cookie@npm:4.0.2" @@ -11578,15 +11555,6 @@ __metadata: languageName: node linkType: hard -"@types/yargs@npm:^15.0.0": - version: 15.0.4 - resolution: "@types/yargs@npm:15.0.4" - dependencies: - "@types/yargs-parser": "npm:*" - checksum: 10c0/9749242ffa26a73ee6e32f750ca1108b7aaf270afa92cb80c075d54f2c4d9d956ae0157cf54d4e442963c9f0a01eae9bcabd7e7b6dd5d2a72b42693420dc35e4 - languageName: node - linkType: hard - "@types/yargs@npm:^16.0.0": version: 16.0.3 resolution: "@types/yargs@npm:16.0.3" @@ -12223,7 +12191,7 @@ __metadata: languageName: node linkType: hard -"ansi-regex@npm:^5.0.0, ansi-regex@npm:^5.0.1": +"ansi-regex@npm:^5.0.1": version: 5.0.1 resolution: "ansi-regex@npm:5.0.1" checksum: 10c0/9a64bb8627b434ba9327b60c027742e5d17ac69277960d041898596271d992d4d52ba7267a63ca10232e29f6107fc8a835f6ce8d719b88c5f8493f8254813737 @@ -16313,13 +16281,6 @@ __metadata: languageName: node linkType: hard -"diff-sequences@npm:^25.1.0": - version: 25.1.0 - resolution: "diff-sequences@npm:25.1.0" - checksum: 10c0/e237d6847f119f3a082357c7148b39f1fcad81b7bb3e133ad2ffbbee24bd49bbcf73ba0bb9694a2aa53104e97658c3e073ab6630e506257b536fc269645c4c49 - languageName: node - linkType: hard - "diff-sequences@npm:^29.6.3": version: 29.6.3 resolution: "diff-sequences@npm:29.6.3" @@ -16410,13 +16371,20 @@ __metadata: languageName: node linkType: hard -"dom-accessibility-api@npm:^0.5.6, dom-accessibility-api@npm:^0.5.9": +"dom-accessibility-api@npm:^0.5.9": version: 0.5.10 resolution: "dom-accessibility-api@npm:0.5.10" checksum: 10c0/59ef8de881d28181a28c969a976beb89538ce13dce78da1f81f432369c4723f48dd5c2671526eb0276ff320c2e2ee46b84636024c6b668209fd224886f1847f3 languageName: node linkType: hard +"dom-accessibility-api@npm:^0.6.3": + version: 0.6.3 + resolution: "dom-accessibility-api@npm:0.6.3" + checksum: 10c0/10bee5aa514b2a9a37c87cd81268db607a2e933a050074abc2f6fa3da9080ebed206a320cbc123567f2c3087d22292853bdfdceaffdd4334ffe2af9510b29360 + languageName: node + linkType: hard + "dom-helpers@npm:^5.0.1": version: 5.2.1 resolution: "dom-helpers@npm:5.2.1" @@ -22284,18 +22252,6 @@ __metadata: languageName: node linkType: hard -"jest-diff@npm:^25.1.0": - version: 25.1.0 - resolution: "jest-diff@npm:25.1.0" - dependencies: - chalk: "npm:^3.0.0" - diff-sequences: "npm:^25.1.0" - jest-get-type: "npm:^25.1.0" - pretty-format: "npm:^25.1.0" - checksum: 10c0/d0ad8bdc67cfaaaaf9c415d54ba175c4ee0b8eb2694a43a2f10ec9d0eb45ed70c886e104a58b81ef3115dc083b93e8ec2e120be9eccf16062f19e0878b7a2f64 - languageName: node - linkType: hard - "jest-diff@npm:^29.7.0": version: 29.7.0 resolution: "jest-diff@npm:29.7.0" @@ -22365,13 +22321,6 @@ __metadata: languageName: node linkType: hard -"jest-get-type@npm:^25.1.0": - version: 25.1.0 - resolution: "jest-get-type@npm:25.1.0" - checksum: 10c0/991118ce96d07c4ba2a1ae65da3b0249d6dbc86dfadc9047744045a9ceedba74c4ae6303d5c03d7b70764dc73cfb5f672fd203df9280a20e681b4a72cf6f64dd - languageName: node - linkType: hard - "jest-get-type@npm:^29.6.3": version: 29.6.3 resolution: "jest-get-type@npm:29.6.3" @@ -28414,18 +28363,6 @@ __metadata: languageName: node linkType: hard -"pretty-format@npm:^25.1.0": - version: 25.1.0 - resolution: "pretty-format@npm:25.1.0" - dependencies: - "@jest/types": "npm:^25.1.0" - ansi-regex: "npm:^5.0.0" - ansi-styles: "npm:^4.0.0" - react-is: "npm:^16.12.0" - checksum: 10c0/7d8ef7bd1d5a87a48c9dffa94a276cddb71e882d8916a7cddcaa3b1fb191f5ad0a720a1458c787c4b8bf7e18fa1b5e3f3e98b4da9b93dd743fcc80d50aedc522 - languageName: node - linkType: hard - "pretty-format@npm:^27.0.2": version: 27.4.2 resolution: "pretty-format@npm:27.4.2" @@ -29195,7 +29132,7 @@ __metadata: languageName: node linkType: hard -"react-is@npm:^16.12.0, react-is@npm:^16.13.1": +"react-is@npm:^16.13.1": version: 16.13.1 resolution: "react-is@npm:16.13.1" checksum: 10c0/33977da7a5f1a287936a0c85639fec6ca74f4f15ef1e59a6bc20338fc73dc69555381e211f7a3529b8150a1f71e4225525b41b60b52965bda53ce7d47377ada1 @@ -29337,8 +29274,8 @@ __metadata: "@swc/jest": "npm:^0.2.36" "@tailwindcss/postcss": "npm:^4.0.0" "@testing-library/dom": "npm:^10.1.0" - "@testing-library/jest-dom": "npm:^5.16.5" - "@testing-library/react": "npm:^15.0.7" + "@testing-library/jest-dom": "npm:^6.0.0" + "@testing-library/react": "npm:^16.0.0" "@testing-library/user-event": "npm:^14.6.1" "@types/react": "npm:types-react@19.0.0-rc.0" "@types/react-dom": "npm:types-react-dom@19.0.0-rc.0" From f625818552b40b2c77d0c3a736e62e6537cab3e3 Mon Sep 17 00:00:00 2001 From: Daniel Lu Date: Mon, 31 Mar 2025 13:27:56 -0700 Subject: [PATCH 04/17] use alternative to calling jest run timers in menu option selection --- packages/@react-aria/test-utils/src/menu.ts | 34 ++++++++++----------- 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/packages/@react-aria/test-utils/src/menu.ts b/packages/@react-aria/test-utils/src/menu.ts index a8805724afe..64d5d529fb6 100644 --- a/packages/@react-aria/test-utils/src/menu.ts +++ b/packages/@react-aria/test-utils/src/menu.ts @@ -226,32 +226,30 @@ export class MenuTester { await this.user.pointer({target: option, keys: '[TouchA]'}); } } - act(() => {jest.runAllTimers();}); - if (option.getAttribute('href') == null && option.getAttribute('aria-haspopup') == null && menuSelectionMode === 'single' && closesOnSelect && keyboardActivation !== 'Space' && !this._isSubmenu) { + if ( + !(menuSelectionMode === 'single' && !closesOnSelect) && + !(menuSelectionMode === 'multiple' && (keyboardActivation === 'Space' || interactionType === 'mouse')) + ) { + // If user isn't trying to select multiple menu options or closeOnSelect is true then we can assume that + // the menu will close or some action is triggered. In cases like that focus should move somewhere after the menu closes + // but we can't really know where so just make sure it doesn't get lost to the body. await waitFor(() => { - if (document.activeElement !== trigger) { - throw new Error(`Expected the document.activeElement after selecting an option to be the menu trigger but got ${document.activeElement}`); + if (document.activeElement === option) { + throw new Error('Expected focus after selecting an option to move away from the option'); } else { return true; } }); - if (document.contains(menu)) { - throw new Error('Expected menu element to not be in the document after selecting an option'); - } + await waitFor(() => { + if (document.activeElement === document.body) { + throw new Error('Expected focus to move to somewhere other than the body after selecting a menu option.'); + } else { + return true; + } + }); } - // else { - // // TODO: this is a bit unfortunate, but from a submenu point of view, we can't perform a check that waits for - // // focus to return to the original trigger since we don't have that information in the submenu trigger tester. Even if we did - // // it is a bit unpredicatable where focus might end up landing and waiting for the option/menu to disappear isn't sufficient if there are - // // more transitions in play. For now advance times by a certain amount of time and rely on the user to advance them even more if need be - // if (this._advanceTimer == null) { - // throw new Error('No advanceTimers provided for long press.'); - // } else { - // await act(async () => await this._advanceTimer(1000)); - // } - // } } else { throw new Error("Attempted to select a option in the menu, but menu wasn't found."); } From 69d289f4651ed6bd680c173b9797c51a648eea30 Mon Sep 17 00:00:00 2001 From: Daniel Lu Date: Mon, 31 Mar 2025 14:26:03 -0700 Subject: [PATCH 05/17] fixing types and properly testing long press --- packages/@react-aria/test-utils/src/events.ts | 2 +- packages/@react-aria/test-utils/src/types.ts | 2 +- ...eUtils.test.js => TestTableUtils.test.tsx} | 60 +++++++++++++++++-- .../react-aria-components/test/Menu.test.tsx | 2 +- 4 files changed, 58 insertions(+), 8 deletions(-) rename packages/@react-spectrum/table/test/{TestTableUtils.test.js => TestTableUtils.test.tsx} (77%) diff --git a/packages/@react-aria/test-utils/src/events.ts b/packages/@react-aria/test-utils/src/events.ts index 73a048de8d8..68f9f92937a 100644 --- a/packages/@react-aria/test-utils/src/events.ts +++ b/packages/@react-aria/test-utils/src/events.ts @@ -22,7 +22,7 @@ export const DEFAULT_LONG_PRESS_TIME = 500; * @param opts.advanceTimer - Function that when called advances the timers in your test suite by a specific amount of time(ms). * @param opts.pointeropts - Options to pass to the simulated event. Defaults to mouse. See https://testing-library.com/docs/dom-testing-library/api-events/#fireevent for more info. */ -export async function triggerLongPress(opts: {element: HTMLElement, advanceTimer: (time?: number) => void | Promise, pointerOpts?: Record}) { +export async function triggerLongPress(opts: {element: HTMLElement, advanceTimer: (time: number) => void | Promise, pointerOpts?: Record}) { // TODO: note that this only works if the code from installPointerEvent is called somewhere in the test BEFORE the // render. Perhaps we should rely on the user setting that up since I'm not sure there is a great way to set that up here in the // util before first render. Will need to document it well diff --git a/packages/@react-aria/test-utils/src/types.ts b/packages/@react-aria/test-utils/src/types.ts index c749a9dba95..198d2f2a216 100644 --- a/packages/@react-aria/test-utils/src/types.ts +++ b/packages/@react-aria/test-utils/src/types.ts @@ -29,7 +29,7 @@ export interface UserOpts { * A function used by the test utils to advance timers during interactions. Required for certain aria patterns (e.g. table). This can be overridden * at the aria pattern tester level if needed. */ - advanceTimer?: (time?: number) => void | Promise | any + advanceTimer?: (time: number) => void | Promise } export interface BaseTesterOpts extends UserOpts { diff --git a/packages/@react-spectrum/table/test/TestTableUtils.test.js b/packages/@react-spectrum/table/test/TestTableUtils.test.tsx similarity index 77% rename from packages/@react-spectrum/table/test/TestTableUtils.test.js rename to packages/@react-spectrum/table/test/TestTableUtils.test.tsx index 1cd7ad410e0..cd6ecbd9f17 100644 --- a/packages/@react-spectrum/table/test/TestTableUtils.test.js +++ b/packages/@react-spectrum/table/test/TestTableUtils.test.tsx @@ -12,12 +12,12 @@ import {act, render, screen} from '@react-spectrum/test-utils-internal'; import {Cell, Column, Row, TableBody, TableHeader, TableView} from '../'; +import {installPointerEvent, User} from '@react-aria/test-utils'; import {Provider} from '@react-spectrum/provider'; import React, {useState} from 'react'; import {theme} from '@react-spectrum/theme-default'; -import {User} from '@react-aria/test-utils'; -let manyItems = []; +let manyItems = [] as any[]; for (let i = 1; i <= 10; i++) { manyItems.push({id: i, foo: 'Foo ' + i, bar: 'Bar ' + i, baz: 'Baz ' + i}); } @@ -31,7 +31,7 @@ let columns = [ describe('Table ', function () { let onSelectionChange = jest.fn(); let onSortChange = jest.fn(); - let testUtilRealTimer = new User(); + let testUtilRealTimer = new User({advanceTimer: async (waitTime) => await new Promise((resolve) => setTimeout(resolve, waitTime))}); let TableExample = (props) => { let [sort, setSort] = useState({}); @@ -128,6 +128,30 @@ describe('Table ', function () { }); }); + describe('long press, real timers', () => { + installPointerEvent(); + beforeAll(function () { + jest.useRealTimers(); + }); + + afterEach(function () { + jest.clearAllMocks(); + }); + + it('highlight selection should switch to selection mode on long press', async function () { + render(); + let tableTester = testUtilRealTimer.createTester('Table', {root: screen.getByTestId('test'), interactionType: 'touch'}); + tableTester.setInteractionType('touch'); + await tableTester.toggleRowSelection({row: 2, needsLongPress: true}); + expect(onSelectionChange).toHaveBeenCalledTimes(1); + expect(new Set(onSelectionChange.mock.calls[0][0])).toEqual(new Set(['Foo 3'])); + + await tableTester.toggleRowSelection({row: 'Foo 4'}); + expect(onSelectionChange).toHaveBeenCalledTimes(2); + expect(new Set(onSelectionChange.mock.calls[1][0])).toEqual(new Set(['Foo 3', 'Foo 4'])); + }); + }); + describe.each` interactionType ${'mouse'} @@ -179,11 +203,11 @@ describe('Table ', function () { let tableTester = testUtilFakeTimer.createTester('Table', {root: screen.getByTestId('test')}); tableTester.setInteractionType(interactionType); - await tableTester.toggleRowSelection({row: 2, focusToSelect: true}); + await tableTester.toggleRowSelection({row: 2}); expect(onSelectionChange).toHaveBeenCalledTimes(1); expect(new Set(onSelectionChange.mock.calls[0][0])).toEqual(new Set(['Foo 3'])); - await tableTester.toggleRowSelection({row: 'Foo 4', focusToSelect: true}); + await tableTester.toggleRowSelection({row: 'Foo 4'}); expect(onSelectionChange).toHaveBeenCalledTimes(2); expect(new Set(onSelectionChange.mock.calls[1][0])).toEqual(new Set(['Foo 4'])); @@ -200,4 +224,30 @@ describe('Table ', function () { expect(onSortChange).toHaveBeenLastCalledWith({column: 'foo', direction: 'descending'}); }); }); + + describe('long press, fake timers', () => { + installPointerEvent(); + let testUtilFakeTimer = new User({interactionType: 'touch', advanceTimer: jest.advanceTimersByTime}); + beforeAll(function () { + jest.useFakeTimers(); + }); + + afterEach(function () { + act(() => jest.runAllTimers()); + jest.clearAllMocks(); + }); + + it('highlight selection should switch to selection mode on long press', async function () { + render(); + let tableTester = testUtilFakeTimer.createTester('Table', {root: screen.getByTestId('test')}); + + await tableTester.toggleRowSelection({row: 2, needsLongPress: true}); + expect(onSelectionChange).toHaveBeenCalledTimes(1); + expect(new Set(onSelectionChange.mock.calls[0][0])).toEqual(new Set(['Foo 3'])); + + await tableTester.toggleRowSelection({row: 'Foo 4'}); + expect(onSelectionChange).toHaveBeenCalledTimes(2); + expect(new Set(onSelectionChange.mock.calls[1][0])).toEqual(new Set(['Foo 3', 'Foo 4'])); + }); + }); }); diff --git a/packages/react-aria-components/test/Menu.test.tsx b/packages/react-aria-components/test/Menu.test.tsx index 4ac8f863ef9..0432d51607e 100644 --- a/packages/react-aria-components/test/Menu.test.tsx +++ b/packages/react-aria-components/test/Menu.test.tsx @@ -55,7 +55,7 @@ let renderMenu = (menuProps = {}, itemProps = {}) => render( { let user; - let testUtilUser = new User(); + let testUtilUser = new User({advanceTimer: jest.advanceTimersByTime}); beforeAll(() => { user = userEvent.setup({delay: null, pointerMap}); From 366c1f8e448d1faf5b54c2f173ad8d79fc689a60 Mon Sep 17 00:00:00 2001 From: Daniel Lu Date: Mon, 31 Mar 2025 14:43:26 -0700 Subject: [PATCH 06/17] fix lint --- package.json | 1 + packages/@react-aria/test-utils/src/types.ts | 2 +- tsconfig.json | 1 + yarn.lock | 15 +++++++++++++-- 4 files changed, 16 insertions(+), 3 deletions(-) diff --git a/package.json b/package.json index 5222fe24334..2dddfd5e5de 100644 --- a/package.json +++ b/package.json @@ -127,6 +127,7 @@ "@testing-library/jest-dom": "^6.0.0", "@testing-library/react": "^16.0.0", "@testing-library/user-event": "^14.6.1", + "@types/jest": "^29.5.14", "@types/react": "npm:types-react@19.0.0-rc.0", "@types/react-dom": "npm:types-react-dom@19.0.0-rc.0", "@types/storybook__react": "^4.0.2", diff --git a/packages/@react-aria/test-utils/src/types.ts b/packages/@react-aria/test-utils/src/types.ts index 198d2f2a216..782c00e583c 100644 --- a/packages/@react-aria/test-utils/src/types.ts +++ b/packages/@react-aria/test-utils/src/types.ts @@ -29,7 +29,7 @@ export interface UserOpts { * A function used by the test utils to advance timers during interactions. Required for certain aria patterns (e.g. table). This can be overridden * at the aria pattern tester level if needed. */ - advanceTimer?: (time: number) => void | Promise + advanceTimer?: (time: number) => any | Promise } export interface BaseTesterOpts extends UserOpts { diff --git a/tsconfig.json b/tsconfig.json index 4df9d95a37e..9dd18553b1f 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -1,5 +1,6 @@ { "compilerOptions": { + "types": ["@testing-library/jest-dom"], // we can explicitly declare `any`, but we don't want to infer `any` "noImplicitAny": false, // maybe bump to 'esNext'? diff --git a/yarn.lock b/yarn.lock index 32239a9c50f..8c2387829ab 100644 --- a/yarn.lock +++ b/yarn.lock @@ -11260,6 +11260,16 @@ __metadata: languageName: node linkType: hard +"@types/jest@npm:^29.5.14": + version: 29.5.14 + resolution: "@types/jest@npm:29.5.14" + dependencies: + expect: "npm:^29.0.0" + pretty-format: "npm:^29.0.0" + checksum: 10c0/18e0712d818890db8a8dab3d91e9ea9f7f19e3f83c2e50b312f557017dc81466207a71f3ed79cf4428e813ba939954fa26ffa0a9a7f153181ba174581b1c2aed + languageName: node + linkType: hard + "@types/jscodeshift@npm:^0.11.11": version: 0.11.11 resolution: "@types/jscodeshift@npm:0.11.11" @@ -17890,7 +17900,7 @@ __metadata: languageName: node linkType: hard -"expect@npm:^29.7.0": +"expect@npm:^29.0.0, expect@npm:^29.7.0": version: 29.7.0 resolution: "expect@npm:29.7.0" dependencies: @@ -28375,7 +28385,7 @@ __metadata: languageName: node linkType: hard -"pretty-format@npm:^29.7.0": +"pretty-format@npm:^29.0.0, pretty-format@npm:^29.7.0": version: 29.7.0 resolution: "pretty-format@npm:29.7.0" dependencies: @@ -29277,6 +29287,7 @@ __metadata: "@testing-library/jest-dom": "npm:^6.0.0" "@testing-library/react": "npm:^16.0.0" "@testing-library/user-event": "npm:^14.6.1" + "@types/jest": "npm:^29.5.14" "@types/react": "npm:types-react@19.0.0-rc.0" "@types/react-dom": "npm:types-react-dom@19.0.0-rc.0" "@types/storybook__react": "npm:^4.0.2" From 8f552608d97ba183f5a244f35e0a54eaca329856 Mon Sep 17 00:00:00 2001 From: Daniel Lu Date: Mon, 31 Mar 2025 15:22:59 -0700 Subject: [PATCH 07/17] revert to pre testing library bump for clean slate --- package.json | 5 +- packages/dev/test-utils/package.json | 6 +- tsconfig.json | 1 - yarn.lock | 162 ++++++++++++++++++--------- 4 files changed, 112 insertions(+), 62 deletions(-) diff --git a/package.json b/package.json index afc95a9889a..c93c72dbd67 100644 --- a/package.json +++ b/package.json @@ -124,10 +124,9 @@ "@swc/jest": "^0.2.36", "@tailwindcss/postcss": "^4.0.0", "@testing-library/dom": "^10.1.0", - "@testing-library/jest-dom": "^6.0.0", - "@testing-library/react": "^16.0.0", + "@testing-library/jest-dom": "^5.16.5", + "@testing-library/react": "^15.0.7", "@testing-library/user-event": "patch:@testing-library/user-event@npm%3A14.6.1#~/.yarn/patches/@testing-library-user-event-npm-14.6.1-5da7e1d4e2.patch", - "@types/jest": "^29.5.14", "@types/react": "npm:types-react@19.0.0-rc.0", "@types/react-dom": "npm:types-react-dom@19.0.0-rc.0", "@types/storybook__react": "^4.0.2", diff --git a/packages/dev/test-utils/package.json b/packages/dev/test-utils/package.json index e1b9bc54649..1bf0df3a768 100644 --- a/packages/dev/test-utils/package.json +++ b/packages/dev/test-utils/package.json @@ -25,9 +25,9 @@ "@react-spectrum/theme-default": "^3.5.13", "@swc/helpers": "^0.5.0", "@testing-library/dom": "^10.1.0", - "@testing-library/jest-dom": "^6.0.0", - "@testing-library/react": "^16.0.0", - "@testing-library/user-event": "^14.4.3", + "@testing-library/jest-dom": "^5.16.4", + "@testing-library/react": "^15.0.7", + "@testing-library/user-event": "^14.0.0", "jest": "^29.5.0", "resolve": "^1.17.0" }, diff --git a/tsconfig.json b/tsconfig.json index 03b26cab918..b05da2d90c7 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -1,6 +1,5 @@ { "compilerOptions": { - "types": ["@testing-library/jest-dom"], // we can explicitly declare `any`, but we don't want to infer `any` "noImplicitAny": false, // maybe bump to 'esNext'? diff --git a/yarn.lock b/yarn.lock index 06e30c0eb00..b98821a6f65 100644 --- a/yarn.lock +++ b/yarn.lock @@ -22,10 +22,10 @@ __metadata: languageName: node linkType: hard -"@adobe/css-tools@npm:^4.4.0": - version: 4.4.2 - resolution: "@adobe/css-tools@npm:4.4.2" - checksum: 10c0/19433666ad18536b0ed05d4b53fbb3dd6ede266996796462023ec77a90b484890ad28a3e528cdf3ab8a65cb2fcdff5d8feb04db6bc6eed6ca307c40974239c94 +"@adobe/css-tools@npm:^4.0.1": + version: 4.2.0 + resolution: "@adobe/css-tools@npm:4.2.0" + checksum: 10c0/b8dbfd9c54df73a398e9b20c922abe26c67732e16afc50668402af0e3d101409e0c944baf69bf814343eb8639014637b96f209426088b06943cea288c1ef1486 languageName: node linkType: hard @@ -3046,6 +3046,18 @@ __metadata: languageName: node linkType: hard +"@jest/types@npm:^25.1.0": + version: 25.1.0 + resolution: "@jest/types@npm:25.1.0" + dependencies: + "@types/istanbul-lib-coverage": "npm:^2.0.0" + "@types/istanbul-reports": "npm:^1.1.1" + "@types/yargs": "npm:^15.0.0" + chalk: "npm:^3.0.0" + checksum: 10c0/d9340658d1090f0ef3914fb91290ed2ce670eac6ad14bbdd9029041b605eccfa6f52bb30825b5c7d23b76a3736213f03caf7ed98d9063fdfd069f60748725c1b + languageName: node + linkType: hard + "@jest/types@npm:^27.4.2, @jest/types@npm:^27.5.1": version: 27.5.1 resolution: "@jest/types@npm:27.5.1" @@ -8206,9 +8218,9 @@ __metadata: "@react-spectrum/theme-default": "npm:^3.5.13" "@swc/helpers": "npm:^0.5.0" "@testing-library/dom": "npm:^10.1.0" - "@testing-library/jest-dom": "npm:^6.0.0" - "@testing-library/react": "npm:^16.0.0" - "@testing-library/user-event": "npm:^14.4.3" + "@testing-library/jest-dom": "npm:^5.16.4" + "@testing-library/react": "npm:^15.0.7" + "@testing-library/user-event": "npm:^14.0.0" jest: "npm:^29.5.0" resolve: "npm:^1.17.0" peerDependencies: @@ -10882,18 +10894,20 @@ __metadata: languageName: node linkType: hard -"@testing-library/jest-dom@npm:^6.0.0": - version: 6.6.3 - resolution: "@testing-library/jest-dom@npm:6.6.3" +"@testing-library/jest-dom@npm:^5.16.4, @testing-library/jest-dom@npm:^5.16.5": + version: 5.17.0 + resolution: "@testing-library/jest-dom@npm:5.17.0" dependencies: - "@adobe/css-tools": "npm:^4.4.0" + "@adobe/css-tools": "npm:^4.0.1" + "@babel/runtime": "npm:^7.9.2" + "@types/testing-library__jest-dom": "npm:^5.9.1" aria-query: "npm:^5.0.0" chalk: "npm:^3.0.0" css.escape: "npm:^1.5.1" - dom-accessibility-api: "npm:^0.6.3" - lodash: "npm:^4.17.21" + dom-accessibility-api: "npm:^0.5.6" + lodash: "npm:^4.17.15" redent: "npm:^3.0.0" - checksum: 10c0/5566b6c0b7b0709bc244aec3aa3dc9e5f4663e8fb2b99d8cd456fc07279e59db6076cbf798f9d3099a98fca7ef4cd50e4e1f4c4dec5a60a8fad8d24a638a5bf6 + checksum: 10c0/24e09c5779ea44644945ec26f2e4e5f48aecfe57d469decf2317a3253a5db28d865c55ad0ea4818d8d1df7572a6486c45daa06fa09644a833a7dd84563881939 languageName: node linkType: hard @@ -10915,26 +10929,6 @@ __metadata: languageName: node linkType: hard -"@testing-library/react@npm:^16.0.0": - version: 16.2.0 - resolution: "@testing-library/react@npm:16.2.0" - dependencies: - "@babel/runtime": "npm:^7.12.5" - peerDependencies: - "@testing-library/dom": ^10.0.0 - "@types/react": ^18.0.0 || ^19.0.0 - "@types/react-dom": ^18.0.0 || ^19.0.0 - react: ^18.0.0 || ^19.0.0 - react-dom: ^18.0.0 || ^19.0.0 - peerDependenciesMeta: - "@types/react": - optional: true - "@types/react-dom": - optional: true - checksum: 10c0/7adaedaf237002b42e04a6261d2756074a19cbca0f0c79ba375660f618e123c0ee56256ced00aeb0bb7225ba1a8a81b92b692cca053521a21bb92a8cace1e4c6 - languageName: node - linkType: hard - "@testing-library/user-event@npm:14.6.1": version: 14.6.1 resolution: "@testing-library/user-event@npm:14.6.1" @@ -11299,6 +11293,16 @@ __metadata: languageName: node linkType: hard +"@types/istanbul-reports@npm:^1.1.1": + version: 1.1.1 + resolution: "@types/istanbul-reports@npm:1.1.1" + dependencies: + "@types/istanbul-lib-coverage": "npm:*" + "@types/istanbul-lib-report": "npm:*" + checksum: 10c0/a91dccd7399231a1871f730d6d933cb8a6d3f5aa4532670348b7aae261104d5306a6add81f734fa03a629ed8d806d287a40413a19e95489676b08ede21d220fc + languageName: node + linkType: hard + "@types/istanbul-reports@npm:^3.0.0": version: 3.0.0 resolution: "@types/istanbul-reports@npm:3.0.0" @@ -11308,13 +11312,13 @@ __metadata: languageName: node linkType: hard -"@types/jest@npm:^29.5.14": - version: 29.5.14 - resolution: "@types/jest@npm:29.5.14" +"@types/jest@npm:*": + version: 25.1.4 + resolution: "@types/jest@npm:25.1.4" dependencies: - expect: "npm:^29.0.0" - pretty-format: "npm:^29.0.0" - checksum: 10c0/18e0712d818890db8a8dab3d91e9ea9f7f19e3f83c2e50b312f557017dc81466207a71f3ed79cf4428e813ba939954fa26ffa0a9a7f153181ba174581b1c2aed + jest-diff: "npm:^25.1.0" + pretty-format: "npm:^25.1.0" + checksum: 10c0/b1ed79448840d804879326380e8fa73d6ab4fc9243b4420b3264f94292ede6bb04ca7839898e0244491b0666f9dc4b701840e84269d98ab063b04820619d66db languageName: node linkType: hard @@ -11555,6 +11559,15 @@ __metadata: languageName: node linkType: hard +"@types/testing-library__jest-dom@npm:^5.9.1": + version: 5.9.2 + resolution: "@types/testing-library__jest-dom@npm:5.9.2" + dependencies: + "@types/jest": "npm:*" + checksum: 10c0/81431e8f95854e02a94f9cb54ff8fe0bdad66dc5bd3603b3d51dae01682484fdc4968496edcb2315a734eca6966e172ad450332fcd9e0c89b4279b898744626d + languageName: node + linkType: hard + "@types/tough-cookie@npm:*": version: 4.0.2 resolution: "@types/tough-cookie@npm:4.0.2" @@ -11613,6 +11626,15 @@ __metadata: languageName: node linkType: hard +"@types/yargs@npm:^15.0.0": + version: 15.0.4 + resolution: "@types/yargs@npm:15.0.4" + dependencies: + "@types/yargs-parser": "npm:*" + checksum: 10c0/9749242ffa26a73ee6e32f750ca1108b7aaf270afa92cb80c075d54f2c4d9d956ae0157cf54d4e442963c9f0a01eae9bcabd7e7b6dd5d2a72b42693420dc35e4 + languageName: node + linkType: hard + "@types/yargs@npm:^16.0.0": version: 16.0.3 resolution: "@types/yargs@npm:16.0.3" @@ -12431,7 +12453,7 @@ __metadata: languageName: node linkType: hard -"ansi-regex@npm:^5.0.1": +"ansi-regex@npm:^5.0.0, ansi-regex@npm:^5.0.1": version: 5.0.1 resolution: "ansi-regex@npm:5.0.1" checksum: 10c0/9a64bb8627b434ba9327b60c027742e5d17ac69277960d041898596271d992d4d52ba7267a63ca10232e29f6107fc8a835f6ce8d719b88c5f8493f8254813737 @@ -16561,6 +16583,13 @@ __metadata: languageName: node linkType: hard +"diff-sequences@npm:^25.1.0": + version: 25.1.0 + resolution: "diff-sequences@npm:25.1.0" + checksum: 10c0/e237d6847f119f3a082357c7148b39f1fcad81b7bb3e133ad2ffbbee24bd49bbcf73ba0bb9694a2aa53104e97658c3e073ab6630e506257b536fc269645c4c49 + languageName: node + linkType: hard + "diff-sequences@npm:^29.6.3": version: 29.6.3 resolution: "diff-sequences@npm:29.6.3" @@ -16651,20 +16680,13 @@ __metadata: languageName: node linkType: hard -"dom-accessibility-api@npm:^0.5.9": +"dom-accessibility-api@npm:^0.5.6, dom-accessibility-api@npm:^0.5.9": version: 0.5.10 resolution: "dom-accessibility-api@npm:0.5.10" checksum: 10c0/59ef8de881d28181a28c969a976beb89538ce13dce78da1f81f432369c4723f48dd5c2671526eb0276ff320c2e2ee46b84636024c6b668209fd224886f1847f3 languageName: node linkType: hard -"dom-accessibility-api@npm:^0.6.3": - version: 0.6.3 - resolution: "dom-accessibility-api@npm:0.6.3" - checksum: 10c0/10bee5aa514b2a9a37c87cd81268db607a2e933a050074abc2f6fa3da9080ebed206a320cbc123567f2c3087d22292853bdfdceaffdd4334ffe2af9510b29360 - languageName: node - linkType: hard - "dom-helpers@npm:^5.0.1": version: 5.2.1 resolution: "dom-helpers@npm:5.2.1" @@ -18175,7 +18197,7 @@ __metadata: languageName: node linkType: hard -"expect@npm:^29.0.0, expect@npm:^29.7.0": +"expect@npm:^29.7.0": version: 29.7.0 resolution: "expect@npm:29.7.0" dependencies: @@ -22618,6 +22640,18 @@ __metadata: languageName: node linkType: hard +"jest-diff@npm:^25.1.0": + version: 25.1.0 + resolution: "jest-diff@npm:25.1.0" + dependencies: + chalk: "npm:^3.0.0" + diff-sequences: "npm:^25.1.0" + jest-get-type: "npm:^25.1.0" + pretty-format: "npm:^25.1.0" + checksum: 10c0/d0ad8bdc67cfaaaaf9c415d54ba175c4ee0b8eb2694a43a2f10ec9d0eb45ed70c886e104a58b81ef3115dc083b93e8ec2e120be9eccf16062f19e0878b7a2f64 + languageName: node + linkType: hard + "jest-diff@npm:^29.7.0": version: 29.7.0 resolution: "jest-diff@npm:29.7.0" @@ -22687,6 +22721,13 @@ __metadata: languageName: node linkType: hard +"jest-get-type@npm:^25.1.0": + version: 25.1.0 + resolution: "jest-get-type@npm:25.1.0" + checksum: 10c0/991118ce96d07c4ba2a1ae65da3b0249d6dbc86dfadc9047744045a9ceedba74c4ae6303d5c03d7b70764dc73cfb5f672fd203df9280a20e681b4a72cf6f64dd + languageName: node + linkType: hard + "jest-get-type@npm:^29.6.3": version: 29.6.3 resolution: "jest-get-type@npm:29.6.3" @@ -28625,6 +28666,18 @@ __metadata: languageName: node linkType: hard +"pretty-format@npm:^25.1.0": + version: 25.1.0 + resolution: "pretty-format@npm:25.1.0" + dependencies: + "@jest/types": "npm:^25.1.0" + ansi-regex: "npm:^5.0.0" + ansi-styles: "npm:^4.0.0" + react-is: "npm:^16.12.0" + checksum: 10c0/7d8ef7bd1d5a87a48c9dffa94a276cddb71e882d8916a7cddcaa3b1fb191f5ad0a720a1458c787c4b8bf7e18fa1b5e3f3e98b4da9b93dd743fcc80d50aedc522 + languageName: node + linkType: hard + "pretty-format@npm:^27.0.2": version: 27.4.2 resolution: "pretty-format@npm:27.4.2" @@ -28637,7 +28690,7 @@ __metadata: languageName: node linkType: hard -"pretty-format@npm:^29.0.0, pretty-format@npm:^29.7.0": +"pretty-format@npm:^29.7.0": version: 29.7.0 resolution: "pretty-format@npm:29.7.0" dependencies: @@ -29371,7 +29424,7 @@ __metadata: languageName: node linkType: hard -"react-is@npm:^16.13.1": +"react-is@npm:^16.12.0, react-is@npm:^16.13.1": version: 16.13.1 resolution: "react-is@npm:16.13.1" checksum: 10c0/33977da7a5f1a287936a0c85639fec6ca74f4f15ef1e59a6bc20338fc73dc69555381e211f7a3529b8150a1f71e4225525b41b60b52965bda53ce7d47377ada1 @@ -29513,10 +29566,9 @@ __metadata: "@swc/jest": "npm:^0.2.36" "@tailwindcss/postcss": "npm:^4.0.0" "@testing-library/dom": "npm:^10.1.0" - "@testing-library/jest-dom": "npm:^6.0.0" - "@testing-library/react": "npm:^16.0.0" + "@testing-library/jest-dom": "npm:^5.16.5" + "@testing-library/react": "npm:^15.0.7" "@testing-library/user-event": "patch:@testing-library/user-event@npm%3A14.6.1#~/.yarn/patches/@testing-library-user-event-npm-14.6.1-5da7e1d4e2.patch" - "@types/jest": "npm:^29.5.14" "@types/react": "npm:types-react@19.0.0-rc.0" "@types/react-dom": "npm:types-react-dom@19.0.0-rc.0" "@types/storybook__react": "npm:^4.0.2" From 93cbb3370aaf2b2076c1544e5ed9f883fc15cc61 Mon Sep 17 00:00:00 2001 From: Daniel Lu Date: Mon, 31 Mar 2025 16:08:44 -0700 Subject: [PATCH 08/17] fix build and another submenu edge case now we shouldnt need to call runAllTimers after selectOption --- package.json | 2 +- packages/@react-aria/test-utils/package.json | 5 +-- packages/@react-aria/test-utils/src/menu.ts | 29 +++++++++++++++- packages/@react-spectrum/s2/package.json | 2 +- .../@react-spectrum/test-utils/package.json | 2 +- packages/dev/test-utils/package.json | 2 +- .../test/AriaMenu.test-util.tsx | 5 --- yarn.lock | 33 ++++++++++--------- 8 files changed, 53 insertions(+), 27 deletions(-) diff --git a/package.json b/package.json index c93c72dbd67..b23b0cd7c7a 100644 --- a/package.json +++ b/package.json @@ -125,7 +125,7 @@ "@tailwindcss/postcss": "^4.0.0", "@testing-library/dom": "^10.1.0", "@testing-library/jest-dom": "^5.16.5", - "@testing-library/react": "^15.0.7", + "@testing-library/react": "^16.0.0", "@testing-library/user-event": "patch:@testing-library/user-event@npm%3A14.6.1#~/.yarn/patches/@testing-library-user-event-npm-14.6.1-5da7e1d4e2.patch", "@types/react": "npm:types-react@19.0.0-rc.0", "@types/react-dom": "npm:types-react-dom@19.0.0-rc.0", diff --git a/packages/@react-aria/test-utils/package.json b/packages/@react-aria/test-utils/package.json index 0d333c76612..0a18e2c0dfb 100644 --- a/packages/@react-aria/test-utils/package.json +++ b/packages/@react-aria/test-utils/package.json @@ -22,10 +22,11 @@ "url": "https://github.com/adobe/react-spectrum" }, "dependencies": { - "@swc/helpers": "^0.5.0" + "@swc/helpers": "^0.5.0", + "react-aria-components": "^1.7.1" }, "peerDependencies": { - "@testing-library/react": "^15.0.7", + "@testing-library/react": "^16.0.0", "@testing-library/user-event": "^14.0.0", "react": "^16.8.0 || ^17.0.0-rc.1 || ^18.0.0 || ^19.0.0-rc.1" }, diff --git a/packages/@react-aria/test-utils/src/menu.ts b/packages/@react-aria/test-utils/src/menu.ts index 4e7f8a51dc7..b261d21b5c7 100644 --- a/packages/@react-aria/test-utils/src/menu.ts +++ b/packages/@react-aria/test-utils/src/menu.ts @@ -227,21 +227,48 @@ export class MenuTester { } } + // This chain of waitFors is needed in place of running all timers since we don't know how long transitions may take, or what action + // the menu option select may trigger. if ( !(menuSelectionMode === 'single' && !closesOnSelect) && !(menuSelectionMode === 'multiple' && (keyboardActivation === 'Space' || interactionType === 'mouse')) ) { + // For RSP, clicking on a submenu option seems to briefly lose focus to the body before moving to the clicked option in the test so we need to wait + // for focus to be coerced to somewhere else in place of running all timers. + if (this._isSubmenu) { + await waitFor(() => { + if (document.activeElement === document.body) { + throw new Error('Expected focus to move to somewhere other than the body after selecting a submenu option.'); + } else { + return true; + } + }); + } + // If user isn't trying to select multiple menu options or closeOnSelect is true then we can assume that // the menu will close or some action is triggered. In cases like that focus should move somewhere after the menu closes // but we can't really know where so just make sure it doesn't get lost to the body. await waitFor(() => { if (document.activeElement === option) { - throw new Error('Expected focus after selecting an option to move away from the option'); + throw new Error('Expected focus after selecting an option to move away from the option.'); } else { return true; } }); + // We'll also want to wait for focus to move away from the original submenu trigger since the entire submenu tree should + // close + if (this._isSubmenu) { + await waitFor(() => { + if (document.activeElement === this.trigger) { + throw new Error('Expected focus after selecting an submenu option to move away from the original submenu trigger.'); + } else { + return true; + } + }); + } + + // Finally wait for focus to be coerced somewhere final when the menu tree is removed from the DOM await waitFor(() => { if (document.activeElement === document.body) { throw new Error('Expected focus to move to somewhere other than the body after selecting a menu option.'); diff --git a/packages/@react-spectrum/s2/package.json b/packages/@react-spectrum/s2/package.json index c2bc2d56890..36cb974f753 100644 --- a/packages/@react-spectrum/s2/package.json +++ b/packages/@react-spectrum/s2/package.json @@ -124,7 +124,7 @@ "@parcel/macros": "^2.14.0", "@react-aria/test-utils": "1.0.0-alpha.3", "@testing-library/dom": "^10.1.0", - "@testing-library/react": "^15.0.7", + "@testing-library/react": "^16.0.0", "@testing-library/user-event": "^14.0.0", "jest": "^29.5.0" }, diff --git a/packages/@react-spectrum/test-utils/package.json b/packages/@react-spectrum/test-utils/package.json index 2e0a4619626..de07e376194 100644 --- a/packages/@react-spectrum/test-utils/package.json +++ b/packages/@react-spectrum/test-utils/package.json @@ -28,7 +28,7 @@ "@swc/helpers": "^0.5.0" }, "peerDependencies": { - "@testing-library/react": "^15.0.7", + "@testing-library/react": "^16.0.0", "@testing-library/user-event": "^14.0.0", "jest": "^29.5.0", "react": "^16.8.0 || ^17.0.0-rc.1 || ^18.0.0 || ^19.0.0-rc.1" diff --git a/packages/dev/test-utils/package.json b/packages/dev/test-utils/package.json index 1bf0df3a768..3d5b8161a57 100644 --- a/packages/dev/test-utils/package.json +++ b/packages/dev/test-utils/package.json @@ -26,7 +26,7 @@ "@swc/helpers": "^0.5.0", "@testing-library/dom": "^10.1.0", "@testing-library/jest-dom": "^5.16.4", - "@testing-library/react": "^15.0.7", + "@testing-library/react": "^16.0.0", "@testing-library/user-event": "^14.0.0", "jest": "^29.5.0", "resolve": "^1.17.0" diff --git a/packages/react-aria-components/test/AriaMenu.test-util.tsx b/packages/react-aria-components/test/AriaMenu.test-util.tsx index 0c22b6a9ebf..2913168417d 100644 --- a/packages/react-aria-components/test/AriaMenu.test-util.tsx +++ b/packages/react-aria-components/test/AriaMenu.test-util.tsx @@ -621,8 +621,6 @@ export const AriaMenuTests = ({renderers, setup, prefix}: AriaMenuTestProps) => expect(submenu).toBeInTheDocument(); await submenuUtil.selectOption({option: submenuUtil.options().filter(item => item.getAttribute('aria-haspopup') == null)[0]}); - // TODO: not ideal, this runAllTimers is only needed for RSPv3, not RAC or S2 - act(() => {jest.runAllTimers();}); expect(menu).not.toBeInTheDocument(); expect(submenu).not.toBeInTheDocument(); expect(document.activeElement).toBe(menuTester.trigger); @@ -639,7 +637,6 @@ export const AriaMenuTests = ({renderers, setup, prefix}: AriaMenuTestProps) => expect(submenuTrigger).toHaveAttribute('aria-expanded', 'false'); let submenuUtil = (await menuTester.openSubmenu({submenuTrigger}))!; - act(() => {jest.runAllTimers();}); expect(submenuTrigger).toHaveAttribute('aria-expanded', 'true'); let submenu = submenuUtil.menu; expect(submenu).toBeInTheDocument(); @@ -648,13 +645,11 @@ export const AriaMenuTests = ({renderers, setup, prefix}: AriaMenuTestProps) => expect(nestedSubmenuTrigger).toHaveAttribute('aria-expanded', 'false'); let nestedSubmenuUtil = (await submenuUtil.openSubmenu({submenuTrigger: nestedSubmenuTrigger}))!; - act(() => {jest.runAllTimers();}); expect(nestedSubmenuTrigger).toHaveAttribute('aria-expanded', 'true'); let nestedSubmenu = nestedSubmenuUtil.menu; expect(nestedSubmenu).toBeInTheDocument(); await nestedSubmenuUtil.selectOption({option: nestedSubmenuUtil.options().filter(item => item.getAttribute('aria-haspopup') == null)[0]}); - act(() => {jest.runAllTimers();}); expect(menu).not.toBeInTheDocument(); expect(submenu).not.toBeInTheDocument(); expect(nestedSubmenu).not.toBeInTheDocument(); diff --git a/yarn.lock b/yarn.lock index b98821a6f65..b147b92f534 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6729,8 +6729,9 @@ __metadata: resolution: "@react-aria/test-utils@workspace:packages/@react-aria/test-utils" dependencies: "@swc/helpers": "npm:^0.5.0" + react-aria-components: "npm:^1.7.1" peerDependencies: - "@testing-library/react": ^15.0.7 + "@testing-library/react": ^16.0.0 "@testing-library/user-event": ^14.0.0 react: ^16.8.0 || ^17.0.0-rc.1 || ^18.0.0 || ^19.0.0-rc.1 languageName: unknown @@ -7954,7 +7955,7 @@ __metadata: "@react-types/table": "npm:^3.11.0" "@react-types/textfield": "npm:^3.12.0" "@testing-library/dom": "npm:^10.1.0" - "@testing-library/react": "npm:^15.0.7" + "@testing-library/react": "npm:^16.0.0" "@testing-library/user-event": "npm:^14.0.0" csstype: "npm:^3.0.2" jest: "npm:^29.5.0" @@ -8219,7 +8220,7 @@ __metadata: "@swc/helpers": "npm:^0.5.0" "@testing-library/dom": "npm:^10.1.0" "@testing-library/jest-dom": "npm:^5.16.4" - "@testing-library/react": "npm:^15.0.7" + "@testing-library/react": "npm:^16.0.0" "@testing-library/user-event": "npm:^14.0.0" jest: "npm:^29.5.0" resolve: "npm:^1.17.0" @@ -8238,7 +8239,7 @@ __metadata: "@react-aria/test-utils": "npm:1.0.0-alpha.5" "@swc/helpers": "npm:^0.5.0" peerDependencies: - "@testing-library/react": ^15.0.7 + "@testing-library/react": ^16.0.0 "@testing-library/user-event": ^14.0.0 jest: ^29.5.0 react: ^16.8.0 || ^17.0.0-rc.1 || ^18.0.0 || ^19.0.0-rc.1 @@ -10862,7 +10863,7 @@ __metadata: languageName: node linkType: hard -"@testing-library/dom@npm:^10.0.0, @testing-library/dom@npm:^10.1.0": +"@testing-library/dom@npm:^10.1.0": version: 10.1.0 resolution: "@testing-library/dom@npm:10.1.0" dependencies: @@ -10911,21 +10912,23 @@ __metadata: languageName: node linkType: hard -"@testing-library/react@npm:^15.0.7": - version: 15.0.7 - resolution: "@testing-library/react@npm:15.0.7" +"@testing-library/react@npm:^16.0.0": + version: 16.2.0 + resolution: "@testing-library/react@npm:16.2.0" dependencies: "@babel/runtime": "npm:^7.12.5" - "@testing-library/dom": "npm:^10.0.0" - "@types/react-dom": "npm:^18.0.0" peerDependencies: - "@types/react": ^18.0.0 - react: ^18.0.0 - react-dom: ^18.0.0 + "@testing-library/dom": ^10.0.0 + "@types/react": ^18.0.0 || ^19.0.0 + "@types/react-dom": ^18.0.0 || ^19.0.0 + react: ^18.0.0 || ^19.0.0 + react-dom: ^18.0.0 || ^19.0.0 peerDependenciesMeta: "@types/react": optional: true - checksum: 10c0/ac8ee8968e81949ecb35f7ee34741c2c043f73dd7fee2247d56f6de6a30de4742af94f25264356863974e54387485b46c9448ecf3f6ca41cf4339011c369f2d4 + "@types/react-dom": + optional: true + checksum: 10c0/7adaedaf237002b42e04a6261d2756074a19cbca0f0c79ba375660f618e123c0ee56256ced00aeb0bb7225ba1a8a81b92b692cca053521a21bb92a8cace1e4c6 languageName: node linkType: hard @@ -29567,7 +29570,7 @@ __metadata: "@tailwindcss/postcss": "npm:^4.0.0" "@testing-library/dom": "npm:^10.1.0" "@testing-library/jest-dom": "npm:^5.16.5" - "@testing-library/react": "npm:^15.0.7" + "@testing-library/react": "npm:^16.0.0" "@testing-library/user-event": "patch:@testing-library/user-event@npm%3A14.6.1#~/.yarn/patches/@testing-library-user-event-npm-14.6.1-5da7e1d4e2.patch" "@types/react": "npm:types-react@19.0.0-rc.0" "@types/react-dom": "npm:types-react-dom@19.0.0-rc.0" From 5207639b84aec9415bee9b386e4a4dd0554e0797 Mon Sep 17 00:00:00 2001 From: Daniel Lu Date: Mon, 31 Mar 2025 17:29:58 -0700 Subject: [PATCH 09/17] fix react 16 bug --- packages/@react-aria/test-utils/package.json | 3 ++- packages/@react-aria/test-utils/src/menu.ts | 17 +++++++++++++---- packages/@react-aria/test-utils/src/types.ts | 6 +++++- .../@react-spectrum/test-utils/package.json | 3 ++- yarn.lock | 2 ++ 5 files changed, 24 insertions(+), 7 deletions(-) diff --git a/packages/@react-aria/test-utils/package.json b/packages/@react-aria/test-utils/package.json index 0a18e2c0dfb..525e4a2fe2f 100644 --- a/packages/@react-aria/test-utils/package.json +++ b/packages/@react-aria/test-utils/package.json @@ -28,7 +28,8 @@ "peerDependencies": { "@testing-library/react": "^16.0.0", "@testing-library/user-event": "^14.0.0", - "react": "^16.8.0 || ^17.0.0-rc.1 || ^18.0.0 || ^19.0.0-rc.1" + "react": "^16.8.0 || ^17.0.0-rc.1 || ^18.0.0 || ^19.0.0-rc.1", + "react-dom": "^16.8.0 || ^17.0.0-rc.1 || ^18.0.0 || ^19.0.0-rc.1" }, "publishConfig": { "access": "public" diff --git a/packages/@react-aria/test-utils/src/menu.ts b/packages/@react-aria/test-utils/src/menu.ts index b261d21b5c7..232f7b7685c 100644 --- a/packages/@react-aria/test-utils/src/menu.ts +++ b/packages/@react-aria/test-utils/src/menu.ts @@ -64,9 +64,10 @@ export class MenuTester { private _advanceTimer: UserOpts['advanceTimer']; private _trigger: HTMLElement | undefined; private _isSubmenu: boolean = false; + private _rootMenu: HTMLElement | undefined; constructor(opts: MenuTesterOpts) { - let {root, user, interactionType, advanceTimer, isSubmenu} = opts; + let {root, user, interactionType, advanceTimer, isSubmenu, rootMenu} = opts; this.user = user; this._interactionType = interactionType || 'mouse'; this._advanceTimer = advanceTimer; @@ -85,6 +86,7 @@ export class MenuTester { } this._isSubmenu = isSubmenu || false; + this._rootMenu = rootMenu; } /** @@ -257,10 +259,10 @@ export class MenuTester { }); // We'll also want to wait for focus to move away from the original submenu trigger since the entire submenu tree should - // close + // close. In React 16, focus actually makes it all the way to the root menu's submenu trigger so we need check the root menu if (this._isSubmenu) { await waitFor(() => { - if (document.activeElement === this.trigger) { + if (document.activeElement === this.trigger || this._rootMenu?.contains(document.activeElement)) { throw new Error('Expected focus after selecting an submenu option to move away from the original submenu trigger.'); } else { return true; @@ -305,7 +307,14 @@ export class MenuTester { submenuTrigger = (within(menu!).getByText(submenuTrigger).closest('[role=menuitem]'))! as HTMLElement; } - let submenuTriggerTester = new MenuTester({user: this.user, interactionType: this._interactionType, root: submenuTrigger, isSubmenu: true, advanceTimer: this._advanceTimer}); + let submenuTriggerTester = new MenuTester({ + user: this.user, + interactionType: this._interactionType, + root: submenuTrigger, + isSubmenu: true, + advanceTimer: this._advanceTimer, + rootMenu: (this._isSubmenu ? this._rootMenu : this.menu) || undefined + }); if (interactionType === 'mouse') { await this.user.pointer({target: submenuTrigger}); } else if (interactionType === 'keyboard') { diff --git a/packages/@react-aria/test-utils/src/types.ts b/packages/@react-aria/test-utils/src/types.ts index 782c00e583c..6e198687774 100644 --- a/packages/@react-aria/test-utils/src/types.ts +++ b/packages/@react-aria/test-utils/src/types.ts @@ -69,7 +69,11 @@ export interface MenuTesterOpts extends BaseTesterOpts { /** * Whether the current menu is a submenu. */ - isSubmenu?: boolean + isSubmenu?: boolean, + /** + * The root menu of the menu tree. Only available if the menu is a submenu. + */ + rootMenu?: HTMLElement } export interface SelectTesterOpts extends BaseTesterOpts { diff --git a/packages/@react-spectrum/test-utils/package.json b/packages/@react-spectrum/test-utils/package.json index de07e376194..4a29aff7157 100644 --- a/packages/@react-spectrum/test-utils/package.json +++ b/packages/@react-spectrum/test-utils/package.json @@ -31,7 +31,8 @@ "@testing-library/react": "^16.0.0", "@testing-library/user-event": "^14.0.0", "jest": "^29.5.0", - "react": "^16.8.0 || ^17.0.0-rc.1 || ^18.0.0 || ^19.0.0-rc.1" + "react": "^16.8.0 || ^17.0.0-rc.1 || ^18.0.0 || ^19.0.0-rc.1", + "react-dom": "^16.8.0 || ^17.0.0-rc.1 || ^18.0.0 || ^19.0.0-rc.1" }, "publishConfig": { "access": "public" diff --git a/yarn.lock b/yarn.lock index b147b92f534..ea0314d6b5a 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6734,6 +6734,7 @@ __metadata: "@testing-library/react": ^16.0.0 "@testing-library/user-event": ^14.0.0 react: ^16.8.0 || ^17.0.0-rc.1 || ^18.0.0 || ^19.0.0-rc.1 + react-dom: ^16.8.0 || ^17.0.0-rc.1 || ^18.0.0 || ^19.0.0-rc.1 languageName: unknown linkType: soft @@ -8243,6 +8244,7 @@ __metadata: "@testing-library/user-event": ^14.0.0 jest: ^29.5.0 react: ^16.8.0 || ^17.0.0-rc.1 || ^18.0.0 || ^19.0.0-rc.1 + react-dom: ^16.8.0 || ^17.0.0-rc.1 || ^18.0.0 || ^19.0.0-rc.1 languageName: unknown linkType: soft From 14452d8c4229eef12f1903016e4365e305c9d4a0 Mon Sep 17 00:00:00 2001 From: Daniel Lu Date: Wed, 2 Apr 2025 10:26:37 -0700 Subject: [PATCH 10/17] update return type of advanceTimer and docs copy --- packages/@react-aria/test-utils/src/events.ts | 2 +- packages/@react-aria/test-utils/src/types.ts | 2 +- packages/dev/docs/pages/react-aria/testing.mdx | 2 +- packages/dev/docs/pages/react-spectrum/testing.mdx | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/@react-aria/test-utils/src/events.ts b/packages/@react-aria/test-utils/src/events.ts index 9f29579b2f8..0d09ca80217 100644 --- a/packages/@react-aria/test-utils/src/events.ts +++ b/packages/@react-aria/test-utils/src/events.ts @@ -22,7 +22,7 @@ export const DEFAULT_LONG_PRESS_TIME = 500; * @param opts.advanceTimer - Function that when called advances the timers in your test suite by a specific amount of time(ms). * @param opts.pointeropts - Options to pass to the simulated event. Defaults to mouse. See https://testing-library.com/docs/dom-testing-library/api-events/#fireevent for more info. */ -export async function triggerLongPress(opts: {element: HTMLElement, advanceTimer: (time: number) => any | Promise, pointerOpts?: Record}): Promise { +export async function triggerLongPress(opts: {element: HTMLElement, advanceTimer: (time: number) => unknown | Promise, pointerOpts?: Record}): Promise { // TODO: note that this only works if the code from installPointerEvent is called somewhere in the test BEFORE the // render. Perhaps we should rely on the user setting that up since I'm not sure there is a great way to set that up here in the // util before first render. Will need to document it well diff --git a/packages/@react-aria/test-utils/src/types.ts b/packages/@react-aria/test-utils/src/types.ts index 6e198687774..0cc7e6a6c16 100644 --- a/packages/@react-aria/test-utils/src/types.ts +++ b/packages/@react-aria/test-utils/src/types.ts @@ -29,7 +29,7 @@ export interface UserOpts { * A function used by the test utils to advance timers during interactions. Required for certain aria patterns (e.g. table). This can be overridden * at the aria pattern tester level if needed. */ - advanceTimer?: (time: number) => any | Promise + advanceTimer?: (time: number) => unknown | Promise } export interface BaseTesterOpts extends UserOpts { diff --git a/packages/dev/docs/pages/react-aria/testing.mdx b/packages/dev/docs/pages/react-aria/testing.mdx index dd26321bb55..85f8939afaf 100644 --- a/packages/dev/docs/pages/react-aria/testing.mdx +++ b/packages/dev/docs/pages/react-aria/testing.mdx @@ -162,7 +162,7 @@ the resulting state of the component. yarn add --dev @react-aria/test-utils ``` -Please note that this library uses [@testing-library/react@15](https://www.npmjs.com/package/@testing-library/react) and [@testing-library/user-event](https://www.npmjs.com/package/@testing-library/user-event/v/13.1.5). This means that you need +Please note that this library uses [@testing-library/react@16](https://www.npmjs.com/package/@testing-library/react) and [@testing-library/user-event@14](https://www.npmjs.com/package/@testing-library/user-event). This means that you need to be on React 18+ in order for these utilities to work. ### Setup diff --git a/packages/dev/docs/pages/react-spectrum/testing.mdx b/packages/dev/docs/pages/react-spectrum/testing.mdx index 2bfc2600d20..01b7dd001af 100644 --- a/packages/dev/docs/pages/react-spectrum/testing.mdx +++ b/packages/dev/docs/pages/react-spectrum/testing.mdx @@ -353,7 +353,7 @@ we can make assumptions about the existence of various aria attributes in a comp yarn add --dev @react-spectrum/test-utils ``` -Please note that this library uses [@testing-library/react@15](https://www.npmjs.com/package/@testing-library/react) and [@testing-library/user-event](https://www.npmjs.com/package/@testing-library/user-event/v/13.1.5). This means that you need +Please note that this library uses [@testing-library/react@16](https://www.npmjs.com/package/@testing-library/react) and [@testing-library/user-event@14](https://www.npmjs.com/package/@testing-library/user-event). This means that you need to be on React 18+ in order for these utilities to work. ### Setup From 8bd33c47109242943c42d1f6048a693c6516ac2a Mon Sep 17 00:00:00 2001 From: Daniel Lu Date: Wed, 2 Apr 2025 15:29:24 -0700 Subject: [PATCH 11/17] Initial support for tree highlight selection support --- packages/@react-aria/test-utils/package.json | 4 +- packages/@react-aria/test-utils/src/events.ts | 5 +- .../@react-aria/test-utils/src/gridlist.ts | 9 +- .../@react-aria/test-utils/src/listbox.ts | 14 +- packages/@react-aria/test-utils/src/tree.ts | 54 ++++++-- packages/@react-aria/test-utils/src/types.ts | 12 +- .../tree/test/TreeView.test.tsx | 122 +++++++++++++++++- 7 files changed, 192 insertions(+), 28 deletions(-) diff --git a/packages/@react-aria/test-utils/package.json b/packages/@react-aria/test-utils/package.json index 525e4a2fe2f..57e61e5000d 100644 --- a/packages/@react-aria/test-utils/package.json +++ b/packages/@react-aria/test-utils/package.json @@ -22,8 +22,8 @@ "url": "https://github.com/adobe/react-spectrum" }, "dependencies": { - "@swc/helpers": "^0.5.0", - "react-aria-components": "^1.7.1" + "@react-aria/utils": "^3.28.1", + "@swc/helpers": "^0.5.0" }, "peerDependencies": { "@testing-library/react": "^16.0.0", diff --git a/packages/@react-aria/test-utils/src/events.ts b/packages/@react-aria/test-utils/src/events.ts index 0d09ca80217..15fdaad1cbb 100644 --- a/packages/@react-aria/test-utils/src/events.ts +++ b/packages/@react-aria/test-utils/src/events.ts @@ -58,9 +58,10 @@ export async function triggerLongPress(opts: {element: HTMLElement, advanceTimer } // Docs cannot handle the types that userEvent actually declares, so hopefully this sub set is okay -export async function pressElement(user: {click: (element: Element) => Promise, keyboard: (keys: string) => Promise, pointer: (opts: {target: Element, keys: string}) => Promise}, element: HTMLElement, interactionType: UserOpts['interactionType']): Promise { +export async function pressElement(user: {click: (element: Element) => Promise, keyboard: (keys: string) => Promise, pointer: (opts: {target: Element, keys: string, coords?: any}) => Promise}, element: HTMLElement, interactionType: UserOpts['interactionType']): Promise { if (interactionType === 'mouse') { - await user.click(element); + // Add coords with pressure so this isn't detected as a virtual click + await user.pointer({target: element, keys: '[MouseLeft]', coords: {pressure: .5}}); } else if (interactionType === 'keyboard') { // TODO: For the keyboard flow, I wonder if it would be reasonable to just do fireEvent directly on the obtained row node or if we should // stick to simulting an actual user's keyboard operations as closely as possible diff --git a/packages/@react-aria/test-utils/src/gridlist.ts b/packages/@react-aria/test-utils/src/gridlist.ts index 1072576eae0..8be873475fd 100644 --- a/packages/@react-aria/test-utils/src/gridlist.ts +++ b/packages/@react-aria/test-utils/src/gridlist.ts @@ -64,6 +64,11 @@ export class GridListTester { if (targetIndex === -1) { throw new Error('Option provided is not in the gridlist'); } + + if (document.activeElement !== this._gridlist || !this._gridlist.contains(document.activeElement)) { + act(() => this._gridlist.focus()); + } + if (document.activeElement === this._gridlist) { await this.user.keyboard('[ArrowDown]'); } else if (this._gridlist.contains(document.activeElement) && document.activeElement!.getAttribute('role') !== 'row') { @@ -161,10 +166,6 @@ export class GridListTester { return; } - if (document.activeElement !== this._gridlist || !this._gridlist.contains(document.activeElement)) { - act(() => this._gridlist.focus()); - } - await this.keyboardNavigateToRow({row}); await this.user.keyboard('[Enter]'); } else { diff --git a/packages/@react-aria/test-utils/src/listbox.ts b/packages/@react-aria/test-utils/src/listbox.ts index 06ef338268c..152d7426665 100644 --- a/packages/@react-aria/test-utils/src/listbox.ts +++ b/packages/@react-aria/test-utils/src/listbox.ts @@ -93,10 +93,12 @@ export class ListBoxTester { throw new Error('Option provided is not in the listbox'); } - if (document.activeElement === this._listbox) { - await this.user.keyboard('[ArrowDown]'); + if (document.activeElement !== this._listbox || !this._listbox.contains(document.activeElement)) { + act(() => this._listbox.focus()); } + await this.user.keyboard('[ArrowDown]'); + // TODO: not sure about doing same while loop that exists in other implementations of keyboardNavigateToOption, // feels like it could break easily if (document.activeElement?.getAttribute('role') !== 'option') { @@ -135,10 +137,6 @@ export class ListBoxTester { return; } - if (document.activeElement !== this._listbox || !this._listbox.contains(document.activeElement)) { - act(() => this._listbox.focus()); - } - await this.keyboardNavigateToOption({option}); await this.user.keyboard(`[${keyboardActivation}]`); } else { @@ -179,10 +177,6 @@ export class ListBoxTester { return; } - if (document.activeElement !== this._listbox || !this._listbox.contains(document.activeElement)) { - act(() => this._listbox.focus()); - } - await this.keyboardNavigateToOption({option}); await this.user.keyboard('[Enter]'); } else { diff --git a/packages/@react-aria/test-utils/src/tree.ts b/packages/@react-aria/test-utils/src/tree.ts index 593e566e3c5..3f7df2cf074 100644 --- a/packages/@react-aria/test-utils/src/tree.ts +++ b/packages/@react-aria/test-utils/src/tree.ts @@ -12,6 +12,7 @@ import {act, within} from '@testing-library/react'; import {BaseGridRowInteractionOpts, GridRowActionOpts, ToggleGridRowOpts, TreeTesterOpts, UserOpts} from './types'; +import {isMac} from '@react-aria/utils'; import {pressElement, triggerLongPress} from './events'; interface TreeToggleExpansionOpts extends BaseGridRowInteractionOpts {} @@ -64,15 +65,21 @@ export class TreeTester { } // TODO: RTL - private async keyboardNavigateToRow(opts: {row: HTMLElement}) { - let {row} = opts; + private async keyboardNavigateToRow(opts: {row: HTMLElement, needsModifierKey?: boolean}) { + let {row, needsModifierKey} = opts; + let modifierKeyCode = isMac() ? 'Alt' : 'ControlLeft'; let rows = this.rows; let targetIndex = rows.indexOf(row); if (targetIndex === -1) { throw new Error('Option provided is not in the tree'); } + + if (document.activeElement !== this._tree || !this._tree.contains(document.activeElement)) { + act(() => this._tree.focus()); + } + if (document.activeElement === this.tree) { - await this.user.keyboard('[ArrowDown]'); + await this.user.keyboard(`${needsModifierKey ? `[${modifierKeyCode}>]` : ''}[ArrowDown]${needsModifierKey ? `[/${modifierKeyCode}]` : ''}`); } else if (this._tree.contains(document.activeElement) && document.activeElement!.getAttribute('role') !== 'row') { do { await this.user.keyboard('[ArrowLeft]'); @@ -84,22 +91,34 @@ export class TreeTester { } let direction = targetIndex > currIndex ? 'down' : 'up'; + if (needsModifierKey) { + await this.user.keyboard(`[${modifierKeyCode}>]`); + } for (let i = 0; i < Math.abs(targetIndex - currIndex); i++) { await this.user.keyboard(`[${direction === 'down' ? 'ArrowDown' : 'ArrowUp'}]`); } + if (needsModifierKey) { + await this.user.keyboard(`[/${modifierKeyCode}]`); + } }; /** * Toggles the selection for the specified tree row. Defaults to using the interaction type set on the tree tester. + * Note that this will endevor to always add/remove JUST the provided row to the set of selected rows. */ async toggleRowSelection(opts: TreeToggleRowOpts): Promise { let { row, needsLongPress, checkboxSelection = true, - interactionType = this._interactionType + interactionType = this._interactionType, + // TODO: perhaps this should just be shouldUseModifierKeys? + selectionBehavior = 'toggle' } = opts; + let keyboardModifierKey = isMac() ? 'Alt' : 'ControlLeft'; + let pointerModifierKey = isMac() ? 'MetaLeft' : 'ControlLeft'; + if (typeof row === 'string' || typeof row === 'number') { row = this.findRow({rowIndexOrText: row}); } @@ -119,8 +138,14 @@ export class TreeTester { // this would be better than the check to do nothing in events.ts // also, it'd be good to be able to trigger selection on the row instead of having to go to the checkbox directly if (interactionType === 'keyboard' && !checkboxSelection) { - await this.keyboardNavigateToRow({row}); + await this.keyboardNavigateToRow({row, needsModifierKey: selectionBehavior === 'replace'}); + if (selectionBehavior === 'replace') { + await this.user.keyboard(`[${keyboardModifierKey}>]`); + } await this.user.keyboard('{Space}'); + if (selectionBehavior === 'replace') { + await this.user.keyboard(`[/${keyboardModifierKey}]`); + } return; } if (rowCheckbox && checkboxSelection) { @@ -135,7 +160,15 @@ export class TreeTester { // Note that long press interactions with rows is strictly touch only for grid rows await triggerLongPress({element: cell, advanceTimer: this._advanceTimer, pointerOpts: {pointerType: 'touch'}}); } else { - await pressElement(this.user, cell, interactionType); + + // TODO add modifiers here? Maybe move into pressElement if we get more cases for different types of modifier keys + if (selectionBehavior === 'replace' && interactionType !== 'touch') { + await this.user.keyboard(`[${pointerModifierKey}>]`); + } + await pressElement(this.user, row, interactionType); + if (selectionBehavior === 'replace' && interactionType !== 'touch') { + await this.user.keyboard(`[/${pointerModifierKey}]`); + } } } }; @@ -172,7 +205,10 @@ export class TreeTester { return; } - await this.keyboardNavigateToRow({row}); + // TODO: We always Use Option/Ctrl when keyboard navigating so selection isn't changed + // in selectionmode="replace"/highlight selection when navigating to the row that the user wants + // to expand. Discuss if this is useful or not + await this.keyboardNavigateToRow({row, needsModifierKey: true}); if (row.getAttribute('aria-expanded') === 'true') { await this.user.keyboard('[ArrowLeft]'); } else { @@ -210,7 +246,9 @@ export class TreeTester { act(() => this._tree.focus()); } - await this.keyboardNavigateToRow({row}); + // TODO: same as above, uses the modifier key to make sure we don't modify selection state on row focus + // as we keyboard navigate to the row we want activate + await this.keyboardNavigateToRow({row, needsModifierKey: true}); await this.user.keyboard('[Enter]'); } else { await pressElement(this.user, row, interactionType); diff --git a/packages/@react-aria/test-utils/src/types.ts b/packages/@react-aria/test-utils/src/types.ts index 0cc7e6a6c16..cd0beb835bc 100644 --- a/packages/@react-aria/test-utils/src/types.ts +++ b/packages/@react-aria/test-utils/src/types.ts @@ -126,7 +126,17 @@ export interface ToggleGridRowOpts extends BaseGridRowInteractionOpts { * Whether the checkbox should be used to select the row. If false, will attempt to select the row via press. * @default 'true' */ - checkboxSelection?: boolean + checkboxSelection?: boolean, + // TODO: this api feels a bit confusing tbh... + /** + * Whether the grid has a selectionBehavior of "toggle" or "replace" (aka highlight selection). This affects the user operations + * required to toggle row selection by adding modifier keys during user actions, useful when performing multi-row selection in a "selectionBehavior: 'replace'" grid. + * If you would like to still simulate user actions (aka press) without these modifiers keys for a "selectionBehavior: replace" grid, simply omit this option. + * See the "Selection Behavior" section of the appropriate React Aria Component docs for more information (e.g. https://react-spectrum.adobe.com/react-aria/Tree.html#selection-behavior). + * + * @default 'toggle' + */ + selectionBehavior?: 'toggle' | 'replace' } export interface GridRowActionOpts extends BaseGridRowInteractionOpts { diff --git a/packages/@react-spectrum/tree/test/TreeView.test.tsx b/packages/@react-spectrum/tree/test/TreeView.test.tsx index 5d2fee7c61d..5a318efdf2f 100644 --- a/packages/@react-spectrum/tree/test/TreeView.test.tsx +++ b/packages/@react-spectrum/tree/test/TreeView.test.tsx @@ -19,11 +19,11 @@ import Edit from '@spectrum-icons/workflow/Edit'; import Folder from '@spectrum-icons/workflow/Folder'; import {Heading, Text} from '@react-spectrum/text'; import {IllustratedMessage} from '@react-spectrum/illustratedmessage'; +import {installPointerEvent, User} from '@react-aria/test-utils'; import {Provider} from '@react-spectrum/provider'; import React from 'react'; import {theme} from '@react-spectrum/theme-default'; import {TreeView, TreeViewItem, TreeViewItemContent} from '../'; -import {User} from '@react-aria/test-utils'; import userEvent from '@testing-library/user-event'; let onSelectionChange = jest.fn(); @@ -674,6 +674,126 @@ describe('Tree', () => { expect(onScroll).toHaveBeenCalled(); }); + describe('highlight selection', () => { + // Required for proper touch detection + installPointerEvent(); + describe.each(['mouse', 'keyboard', 'touch'])('%s', (type) => { + it('should perform selection for highlight mode with single selection', async () => { + let {getByRole} = render(); + let treeTester = testUtilUser.createTester('Tree', {user, root: getByRole('treegrid'), interactionType: type as 'keyboard' | 'mouse' | 'touch'}); + let rows = treeTester.rows; + + for (let row of treeTester.rows) { + let checkbox = within(row).queryByRole('checkbox'); + expect(checkbox).toBeNull(); + expect(row).toHaveAttribute('aria-selected', 'false'); + expect(row).not.toHaveAttribute('data-selected'); + expect(row).toHaveAttribute('data-selection-mode', 'single'); + } + + let row2 = rows[2]; + await treeTester.toggleRowSelection({row: 'Projects-1'}); + expect(row2).toHaveAttribute('aria-selected', 'true'); + expect(row2).toHaveAttribute('data-selected', 'true'); + expect(onSelectionChange).toHaveBeenCalledTimes(1); + expect(new Set(onSelectionChange.mock.calls[0][0])).toEqual(new Set(['Projects-1'])); + expect(treeTester.selectedRows).toHaveLength(1); + expect(treeTester.selectedRows[0]).toBe(row2); + + let row1 = rows[1]; + await treeTester.toggleRowSelection({row: row1}); + expect(row1).toHaveAttribute('aria-selected', 'true'); + expect(row1).toHaveAttribute('data-selected', 'true'); + expect(row2).toHaveAttribute('aria-selected', 'false'); + expect(row2).not.toHaveAttribute('data-selected'); + expect(onSelectionChange).toHaveBeenCalledTimes(2); + expect(new Set(onSelectionChange.mock.calls[1][0])).toEqual(new Set(['Projects'])); + expect(treeTester.selectedRows).toHaveLength(1); + expect(treeTester.selectedRows[0]).toBe(row1); + }); + + it('should perform toggle selection in highlight mode when using modifier keys', async () => { + let {getByRole} = render(); + let treeTester = testUtilUser.createTester('Tree', {user, root: getByRole('treegrid'), interactionType: type as 'keyboard' | 'mouse' | 'touch'}); + let rows = treeTester.rows; + + for (let row of treeTester.rows) { + let checkbox = within(row).queryByRole('checkbox'); + expect(checkbox).toBeNull(); + expect(row).toHaveAttribute('aria-selected', 'false'); + expect(row).not.toHaveAttribute('data-selected'); + expect(row).toHaveAttribute('data-selection-mode', 'multiple'); + } + + let row2 = rows[2]; + await treeTester.toggleRowSelection({row: 'Projects-1', selectionBehavior: 'replace'}); + expect(row2).toHaveAttribute('aria-selected', 'true'); + expect(row2).toHaveAttribute('data-selected', 'true'); + expect(onSelectionChange).toHaveBeenCalledTimes(1); + expect(new Set(onSelectionChange.mock.calls[0][0])).toEqual(new Set(['Projects-1'])); + expect(treeTester.selectedRows).toHaveLength(1); + expect(treeTester.selectedRows[0]).toBe(row2); + + let row1 = rows[1]; + await treeTester.toggleRowSelection({row: row1, selectionBehavior: 'replace'}); + expect(row1).toHaveAttribute('aria-selected', 'true'); + expect(row1).toHaveAttribute('data-selected', 'true'); + expect(row2).toHaveAttribute('aria-selected', 'true'); + expect(row2).toHaveAttribute('data-selected', 'true'); + expect(onSelectionChange).toHaveBeenCalledTimes(2); + expect(new Set(onSelectionChange.mock.calls[1][0])).toEqual(new Set(['Projects', 'Projects-1'])); + expect(treeTester.selectedRows).toHaveLength(2); + expect(treeTester.selectedRows[0]).toBe(row1); + }); + + it('should perform replace selection in highlight mode when not using modifier keys', async () => { + let {getByRole} = render(); + let treeTester = testUtilUser.createTester('Tree', {user, root: getByRole('treegrid'), interactionType: type as 'keyboard' | 'mouse' | 'touch'}); + let rows = treeTester.rows; + + for (let row of treeTester.rows) { + let checkbox = within(row).queryByRole('checkbox'); + expect(checkbox).toBeNull(); + expect(row).toHaveAttribute('aria-selected', 'false'); + expect(row).not.toHaveAttribute('data-selected'); + expect(row).toHaveAttribute('data-selection-mode', 'multiple'); + } + + let row2 = rows[2]; + await treeTester.toggleRowSelection({row: 'Projects-1'}); + expect(row2).toHaveAttribute('aria-selected', 'true'); + expect(row2).toHaveAttribute('data-selected', 'true'); + expect(onSelectionChange).toHaveBeenCalledTimes(1); + expect(new Set(onSelectionChange.mock.calls[0][0])).toEqual(new Set(['Projects-1'])); + expect(treeTester.selectedRows).toHaveLength(1); + expect(treeTester.selectedRows[0]).toBe(row2); + + let row1 = rows[1]; + await treeTester.toggleRowSelection({row: row1}); + if (type !== 'touch') { + expect(row1).toHaveAttribute('aria-selected', 'true'); + expect(row1).toHaveAttribute('data-selected', 'true'); + expect(row2).toHaveAttribute('aria-selected', 'false'); + expect(row2).not.toHaveAttribute('data-selected'); + expect(onSelectionChange).toHaveBeenCalledTimes(2); + expect(new Set(onSelectionChange.mock.calls[1][0])).toEqual(new Set(['Projects'])); + expect(treeTester.selectedRows).toHaveLength(1); + expect(treeTester.selectedRows[0]).toBe(row1); + } else { + // touch always behaves as toggle + expect(row1).toHaveAttribute('aria-selected', 'true'); + expect(row1).toHaveAttribute('data-selected', 'true'); + expect(row2).toHaveAttribute('aria-selected', 'true'); + expect(row2).toHaveAttribute('data-selected', 'true'); + expect(onSelectionChange).toHaveBeenCalledTimes(2); + expect(new Set(onSelectionChange.mock.calls[1][0])).toEqual(new Set(['Projects', 'Projects-1'])); + expect(treeTester.selectedRows).toHaveLength(2); + expect(treeTester.selectedRows[0]).toBe(row1); + } + }); + }); + }); + describe('links', function () { describe.each(['mouse', 'keyboard'])('%s', (type) => { let trigger = async (item, key = 'Enter') => { From dd52a74fc48360861c0c72dcb0a503f36c1872a4 Mon Sep 17 00:00:00 2001 From: Daniel Lu Date: Wed, 2 Apr 2025 15:42:26 -0700 Subject: [PATCH 12/17] move some general fixes from selectionMode="replace" branch here --- packages/@react-aria/test-utils/src/gridlist.ts | 9 +++++---- packages/@react-aria/test-utils/src/listbox.ts | 14 ++++---------- packages/@react-aria/test-utils/src/tree.ts | 9 +++++---- 3 files changed, 14 insertions(+), 18 deletions(-) diff --git a/packages/@react-aria/test-utils/src/gridlist.ts b/packages/@react-aria/test-utils/src/gridlist.ts index 1072576eae0..8be873475fd 100644 --- a/packages/@react-aria/test-utils/src/gridlist.ts +++ b/packages/@react-aria/test-utils/src/gridlist.ts @@ -64,6 +64,11 @@ export class GridListTester { if (targetIndex === -1) { throw new Error('Option provided is not in the gridlist'); } + + if (document.activeElement !== this._gridlist || !this._gridlist.contains(document.activeElement)) { + act(() => this._gridlist.focus()); + } + if (document.activeElement === this._gridlist) { await this.user.keyboard('[ArrowDown]'); } else if (this._gridlist.contains(document.activeElement) && document.activeElement!.getAttribute('role') !== 'row') { @@ -161,10 +166,6 @@ export class GridListTester { return; } - if (document.activeElement !== this._gridlist || !this._gridlist.contains(document.activeElement)) { - act(() => this._gridlist.focus()); - } - await this.keyboardNavigateToRow({row}); await this.user.keyboard('[Enter]'); } else { diff --git a/packages/@react-aria/test-utils/src/listbox.ts b/packages/@react-aria/test-utils/src/listbox.ts index 06ef338268c..152d7426665 100644 --- a/packages/@react-aria/test-utils/src/listbox.ts +++ b/packages/@react-aria/test-utils/src/listbox.ts @@ -93,10 +93,12 @@ export class ListBoxTester { throw new Error('Option provided is not in the listbox'); } - if (document.activeElement === this._listbox) { - await this.user.keyboard('[ArrowDown]'); + if (document.activeElement !== this._listbox || !this._listbox.contains(document.activeElement)) { + act(() => this._listbox.focus()); } + await this.user.keyboard('[ArrowDown]'); + // TODO: not sure about doing same while loop that exists in other implementations of keyboardNavigateToOption, // feels like it could break easily if (document.activeElement?.getAttribute('role') !== 'option') { @@ -135,10 +137,6 @@ export class ListBoxTester { return; } - if (document.activeElement !== this._listbox || !this._listbox.contains(document.activeElement)) { - act(() => this._listbox.focus()); - } - await this.keyboardNavigateToOption({option}); await this.user.keyboard(`[${keyboardActivation}]`); } else { @@ -179,10 +177,6 @@ export class ListBoxTester { return; } - if (document.activeElement !== this._listbox || !this._listbox.contains(document.activeElement)) { - act(() => this._listbox.focus()); - } - await this.keyboardNavigateToOption({option}); await this.user.keyboard('[Enter]'); } else { diff --git a/packages/@react-aria/test-utils/src/tree.ts b/packages/@react-aria/test-utils/src/tree.ts index 593e566e3c5..c767948a43c 100644 --- a/packages/@react-aria/test-utils/src/tree.ts +++ b/packages/@react-aria/test-utils/src/tree.ts @@ -71,6 +71,11 @@ export class TreeTester { if (targetIndex === -1) { throw new Error('Option provided is not in the tree'); } + + if (document.activeElement !== this._tree || !this._tree.contains(document.activeElement)) { + act(() => this._tree.focus()); + } + if (document.activeElement === this.tree) { await this.user.keyboard('[ArrowDown]'); } else if (this._tree.contains(document.activeElement) && document.activeElement!.getAttribute('role') !== 'row') { @@ -206,10 +211,6 @@ export class TreeTester { return; } - if (document.activeElement !== this._tree || !this._tree.contains(document.activeElement)) { - act(() => this._tree.focus()); - } - await this.keyboardNavigateToRow({row}); await this.user.keyboard('[Enter]'); } else { From 5ef9f194441f951d6d67468dce0017524db27f3a Mon Sep 17 00:00:00 2001 From: Daniel Lu Date: Wed, 2 Apr 2025 16:22:03 -0700 Subject: [PATCH 13/17] add highlight selection support to gridlist, listbox, and table --- packages/@react-aria/test-utils/src/events.ts | 8 +++ .../@react-aria/test-utils/src/gridlist.ts | 42 +++++++++++--- .../@react-aria/test-utils/src/listbox.ts | 55 ++++++++++++++++--- packages/@react-aria/test-utils/src/table.ts | 20 ++++++- packages/@react-aria/test-utils/src/tree.ts | 42 ++++++-------- 5 files changed, 124 insertions(+), 43 deletions(-) diff --git a/packages/@react-aria/test-utils/src/events.ts b/packages/@react-aria/test-utils/src/events.ts index 15fdaad1cbb..4760fc8e6d7 100644 --- a/packages/@react-aria/test-utils/src/events.ts +++ b/packages/@react-aria/test-utils/src/events.ts @@ -11,9 +11,17 @@ */ import {act, fireEvent} from '@testing-library/react'; +import {isMac} from '@react-aria/utils'; import {UserOpts} from './types'; export const DEFAULT_LONG_PRESS_TIME = 500; +export function getAltKey(): 'Alt' | 'ControlLeft' { + return isMac() ? 'Alt' : 'ControlLeft'; +} + +export function getMetaKey(): 'MetaLeft' | 'ControlLeft' { + return isMac() ? 'MetaLeft' : 'ControlLeft'; +} /** * Simulates a "long press" event on a element. diff --git a/packages/@react-aria/test-utils/src/gridlist.ts b/packages/@react-aria/test-utils/src/gridlist.ts index 8be873475fd..fdc1dcd56c3 100644 --- a/packages/@react-aria/test-utils/src/gridlist.ts +++ b/packages/@react-aria/test-utils/src/gridlist.ts @@ -12,7 +12,7 @@ import {act, within} from '@testing-library/react'; import {GridListTesterOpts, GridRowActionOpts, ToggleGridRowOpts, UserOpts} from './types'; -import {pressElement, triggerLongPress} from './events'; +import {getAltKey, getMetaKey, pressElement, triggerLongPress} from './events'; interface GridListToggleRowOpts extends ToggleGridRowOpts {} interface GridListRowActionOpts extends GridRowActionOpts {} @@ -57,8 +57,9 @@ export class GridListTester { } // TODO: RTL - private async keyboardNavigateToRow(opts: {row: HTMLElement}) { - let {row} = opts; + private async keyboardNavigateToRow(opts: {row: HTMLElement, useAltKey?: boolean}) { + let {row, useAltKey} = opts; + let altKey = getAltKey(); let rows = this.rows; let targetIndex = rows.indexOf(row); if (targetIndex === -1) { @@ -70,7 +71,7 @@ export class GridListTester { } if (document.activeElement === this._gridlist) { - await this.user.keyboard('[ArrowDown]'); + await this.user.keyboard(`${useAltKey ? `[${altKey}>]` : ''}[ArrowDown]${useAltKey ? `[/${altKey}]` : ''}`); } else if (this._gridlist.contains(document.activeElement) && document.activeElement!.getAttribute('role') !== 'row') { do { await this.user.keyboard('[ArrowLeft]'); @@ -83,21 +84,33 @@ export class GridListTester { let direction = targetIndex > currIndex ? 'down' : 'up'; for (let i = 0; i < Math.abs(targetIndex - currIndex); i++) { + if (useAltKey) { + await this.user.keyboard(`[${altKey}>]`); + } await this.user.keyboard(`[${direction === 'down' ? 'ArrowDown' : 'ArrowUp'}]`); + if (useAltKey) { + await this.user.keyboard(`[/${altKey}]`); + } } }; /** * Toggles the selection for the specified gridlist row. Defaults to using the interaction type set on the gridlist tester. + * Note that this will endevor to always add/remove JUST the provided row to the set of selected rows. */ async toggleRowSelection(opts: GridListToggleRowOpts): Promise { let { row, needsLongPress, checkboxSelection = true, - interactionType = this._interactionType + interactionType = this._interactionType, + // TODO: perhaps this should just be shouldUseModifierKeys? + selectionBehavior = 'toggle' } = opts; + let altKey = getAltKey(); + let metaKey = getMetaKey(); + if (typeof row === 'string' || typeof row === 'number') { row = this.findRow({rowIndexOrText: row}); } @@ -117,8 +130,14 @@ export class GridListTester { // this would be better than the check to do nothing in events.ts // also, it'd be good to be able to trigger selection on the row instead of having to go to the checkbox directly if (interactionType === 'keyboard' && !checkboxSelection) { - await this.keyboardNavigateToRow({row}); + await this.keyboardNavigateToRow({row, useAltKey: selectionBehavior === 'replace'}); + if (selectionBehavior === 'replace') { + await this.user.keyboard(`[${altKey}>]`); + } await this.user.keyboard('{Space}'); + if (selectionBehavior === 'replace') { + await this.user.keyboard(`[/${altKey}]`); + } return; } if (rowCheckbox && checkboxSelection) { @@ -132,9 +151,14 @@ export class GridListTester { // Note that long press interactions with rows is strictly touch only for grid rows await triggerLongPress({element: cell, advanceTimer: this._advanceTimer, pointerOpts: {pointerType: 'touch'}}); - } else { - await pressElement(this.user, cell, interactionType); + if (selectionBehavior === 'replace' && interactionType !== 'touch') { + await this.user.keyboard(`[${metaKey}>]`); + } + await pressElement(this.user, row, interactionType); + if (selectionBehavior === 'replace' && interactionType !== 'touch') { + await this.user.keyboard(`[/${metaKey}]`); + } } } } @@ -166,7 +190,7 @@ export class GridListTester { return; } - await this.keyboardNavigateToRow({row}); + await this.keyboardNavigateToRow({row, useAltKey: true}); await this.user.keyboard('[Enter]'); } else { await pressElement(this.user, row, interactionType); diff --git a/packages/@react-aria/test-utils/src/listbox.ts b/packages/@react-aria/test-utils/src/listbox.ts index 152d7426665..e16fae40283 100644 --- a/packages/@react-aria/test-utils/src/listbox.ts +++ b/packages/@react-aria/test-utils/src/listbox.ts @@ -11,8 +11,8 @@ */ import {act, within} from '@testing-library/react'; +import {getAltKey, getMetaKey, pressElement, triggerLongPress} from './events'; import {ListBoxTesterOpts, UserOpts} from './types'; -import {pressElement, triggerLongPress} from './events'; interface ListBoxToggleOptionOpts { /** @@ -31,7 +31,16 @@ interface ListBoxToggleOptionOpts { /** * Whether the option needs to be long pressed to be selected. Depends on the listbox's implementation. */ - needsLongPress?: boolean + needsLongPress?: boolean, + /** + * Whether the listbox has a selectionBehavior of "toggle" or "replace" (aka highlight selection). This affects the user operations + * required to toggle option selection by adding modifier keys during user actions, useful when performing multi-option selection in a "selectionBehavior: 'replace'" listbox. + * If you would like to still simulate user actions (aka press) without these modifiers keys for a "selectionBehavior: replace" listbox, simply omit this option. + * See the [RAC Listbox docs](https://react-spectrum.adobe.com/react-aria/ListBox.html#selection-behavior) for more info on this behavior. + * + * @default 'toggle' + */ + selectionBehavior?: 'toggle' | 'replace' } interface ListBoxOptionActionOpts extends Omit { @@ -85,8 +94,9 @@ export class ListBoxTester { // TODO: this is basically the same as menu except for the error message, refactor later so that they share // TODO: this also doesn't support grid layout yet - private async keyboardNavigateToOption(opts: {option: HTMLElement}) { - let {option} = opts; + private async keyboardNavigateToOption(opts: {option: HTMLElement, useAltKey?: boolean}) { + let {option, useAltKey} = opts; + let altKey = getAltKey(); let options = this.options(); let targetIndex = options.indexOf(option); if (targetIndex === -1) { @@ -97,7 +107,7 @@ export class ListBoxTester { act(() => this._listbox.focus()); } - await this.user.keyboard('[ArrowDown]'); + await this.user.keyboard(`${useAltKey ? `[${altKey}>]` : ''}[ArrowDown]${useAltKey ? `[/${altKey}]` : ''}`); // TODO: not sure about doing same while loop that exists in other implementations of keyboardNavigateToOption, // feels like it could break easily @@ -113,16 +123,32 @@ export class ListBoxTester { } let direction = targetIndex > currIndex ? 'down' : 'up'; + if (useAltKey) { + await this.user.keyboard(`[${altKey}>]`); + } for (let i = 0; i < Math.abs(targetIndex - currIndex); i++) { await this.user.keyboard(`[${direction === 'down' ? 'ArrowDown' : 'ArrowUp'}]`); } + if (useAltKey) { + await this.user.keyboard(`[/${altKey}]`); + } }; /** * Toggles the selection for the specified listbox option. Defaults to using the interaction type set on the listbox tester. */ async toggleOptionSelection(opts: ListBoxToggleOptionOpts): Promise { - let {option, needsLongPress, keyboardActivation = 'Enter', interactionType = this._interactionType} = opts; + let { + option, + needsLongPress, + keyboardActivation = 'Enter', + interactionType = this._interactionType, + // TODO: perhaps this should just be shouldUseModifierKeys? + selectionBehavior = 'toggle' + } = opts; + + let altKey = getAltKey(); + let metaKey = getMetaKey(); if (typeof option === 'string' || typeof option === 'number') { option = this.findOption({optionIndexOrText: option}); @@ -137,8 +163,15 @@ export class ListBoxTester { return; } - await this.keyboardNavigateToOption({option}); + await this.keyboardNavigateToOption({option, useAltKey: selectionBehavior === 'replace'}); + if (selectionBehavior === 'replace') { + await this.user.keyboard(`[${altKey}>]`); + } await this.user.keyboard(`[${keyboardActivation}]`); + + if (selectionBehavior === 'replace') { + await this.user.keyboard(`[/${altKey}]`); + } } else { if (needsLongPress && interactionType === 'touch') { if (this._advanceTimer == null) { @@ -147,7 +180,13 @@ export class ListBoxTester { await triggerLongPress({element: option, advanceTimer: this._advanceTimer, pointerOpts: {pointerType: 'touch'}}); } else { + if (selectionBehavior === 'replace' && interactionType !== 'touch') { + await this.user.keyboard(`[${metaKey}>]`); + } await pressElement(this.user, option, interactionType); + if (selectionBehavior === 'replace' && interactionType !== 'touch') { + await this.user.keyboard(`[/${metaKey}]`); + } } } } @@ -177,7 +216,7 @@ export class ListBoxTester { return; } - await this.keyboardNavigateToOption({option}); + await this.keyboardNavigateToOption({option, useAltKey: true}); await this.user.keyboard('[Enter]'); } else { await pressElement(this.user, option, interactionType); diff --git a/packages/@react-aria/test-utils/src/table.ts b/packages/@react-aria/test-utils/src/table.ts index 4609b4d63ca..a80f0a6b57d 100644 --- a/packages/@react-aria/test-utils/src/table.ts +++ b/packages/@react-aria/test-utils/src/table.ts @@ -12,7 +12,7 @@ import {act, waitFor, within} from '@testing-library/react'; import {GridRowActionOpts, TableTesterOpts, ToggleGridRowOpts, UserOpts} from './types'; -import {pressElement, triggerLongPress} from './events'; +import {getMetaKey, pressElement, triggerLongPress} from './events'; interface TableToggleRowOpts extends ToggleGridRowOpts {} interface TableToggleSortOpts { @@ -56,9 +56,13 @@ export class TableTester { row, needsLongPress, checkboxSelection = true, - interactionType = this._interactionType + interactionType = this._interactionType, + selectionBehavior = 'toggle' } = opts; + let altKey = getMetaKey(); + let metaKey = getMetaKey(); + if (typeof row === 'string' || typeof row === 'number') { row = this.findRow({rowIndexOrText: row}); } @@ -74,7 +78,13 @@ export class TableTester { await act(async () => { row.focus(); }); + if (selectionBehavior === 'replace') { + await this.user.keyboard(`[${altKey}>]`); + } await this.user.keyboard('{Space}'); + if (selectionBehavior === 'replace') { + await this.user.keyboard(`[/${altKey}]`); + } return; } if (rowCheckbox && checkboxSelection) { @@ -89,7 +99,13 @@ export class TableTester { // Note that long press interactions with rows is strictly touch only for grid rows await triggerLongPress({element: cell, advanceTimer: this._advanceTimer, pointerOpts: {pointerType: 'touch'}}); } else { + if (selectionBehavior === 'replace' && interactionType !== 'touch') { + await this.user.keyboard(`[${metaKey}>]`); + } await pressElement(this.user, cell, interactionType); + if (selectionBehavior === 'replace' && interactionType !== 'touch') { + await this.user.keyboard(`[/${metaKey}]`); + } } } }; diff --git a/packages/@react-aria/test-utils/src/tree.ts b/packages/@react-aria/test-utils/src/tree.ts index 3f7df2cf074..a535c0f7d43 100644 --- a/packages/@react-aria/test-utils/src/tree.ts +++ b/packages/@react-aria/test-utils/src/tree.ts @@ -12,8 +12,7 @@ import {act, within} from '@testing-library/react'; import {BaseGridRowInteractionOpts, GridRowActionOpts, ToggleGridRowOpts, TreeTesterOpts, UserOpts} from './types'; -import {isMac} from '@react-aria/utils'; -import {pressElement, triggerLongPress} from './events'; +import {getAltKey, getMetaKey, pressElement, triggerLongPress} from './events'; interface TreeToggleExpansionOpts extends BaseGridRowInteractionOpts {} interface TreeToggleRowOpts extends ToggleGridRowOpts {} @@ -65,9 +64,9 @@ export class TreeTester { } // TODO: RTL - private async keyboardNavigateToRow(opts: {row: HTMLElement, needsModifierKey?: boolean}) { - let {row, needsModifierKey} = opts; - let modifierKeyCode = isMac() ? 'Alt' : 'ControlLeft'; + private async keyboardNavigateToRow(opts: {row: HTMLElement, useAltKey?: boolean}) { + let {row, useAltKey} = opts; + let altKey = getAltKey(); let rows = this.rows; let targetIndex = rows.indexOf(row); if (targetIndex === -1) { @@ -79,7 +78,7 @@ export class TreeTester { } if (document.activeElement === this.tree) { - await this.user.keyboard(`${needsModifierKey ? `[${modifierKeyCode}>]` : ''}[ArrowDown]${needsModifierKey ? `[/${modifierKeyCode}]` : ''}`); + await this.user.keyboard(`${useAltKey ? `[${altKey}>]` : ''}[ArrowDown]${useAltKey ? `[/${altKey}]` : ''}`); } else if (this._tree.contains(document.activeElement) && document.activeElement!.getAttribute('role') !== 'row') { do { await this.user.keyboard('[ArrowLeft]'); @@ -91,14 +90,14 @@ export class TreeTester { } let direction = targetIndex > currIndex ? 'down' : 'up'; - if (needsModifierKey) { - await this.user.keyboard(`[${modifierKeyCode}>]`); + if (useAltKey) { + await this.user.keyboard(`[${altKey}>]`); } for (let i = 0; i < Math.abs(targetIndex - currIndex); i++) { await this.user.keyboard(`[${direction === 'down' ? 'ArrowDown' : 'ArrowUp'}]`); } - if (needsModifierKey) { - await this.user.keyboard(`[/${modifierKeyCode}]`); + if (useAltKey) { + await this.user.keyboard(`[/${altKey}]`); } }; @@ -116,8 +115,8 @@ export class TreeTester { selectionBehavior = 'toggle' } = opts; - let keyboardModifierKey = isMac() ? 'Alt' : 'ControlLeft'; - let pointerModifierKey = isMac() ? 'MetaLeft' : 'ControlLeft'; + let altKey = getAltKey(); + let metaKey = getMetaKey(); if (typeof row === 'string' || typeof row === 'number') { row = this.findRow({rowIndexOrText: row}); @@ -138,13 +137,13 @@ export class TreeTester { // this would be better than the check to do nothing in events.ts // also, it'd be good to be able to trigger selection on the row instead of having to go to the checkbox directly if (interactionType === 'keyboard' && !checkboxSelection) { - await this.keyboardNavigateToRow({row, needsModifierKey: selectionBehavior === 'replace'}); + await this.keyboardNavigateToRow({row, useAltKey: selectionBehavior === 'replace'}); if (selectionBehavior === 'replace') { - await this.user.keyboard(`[${keyboardModifierKey}>]`); + await this.user.keyboard(`[${altKey}>]`); } await this.user.keyboard('{Space}'); if (selectionBehavior === 'replace') { - await this.user.keyboard(`[/${keyboardModifierKey}]`); + await this.user.keyboard(`[/${altKey}]`); } return; } @@ -160,14 +159,13 @@ export class TreeTester { // Note that long press interactions with rows is strictly touch only for grid rows await triggerLongPress({element: cell, advanceTimer: this._advanceTimer, pointerOpts: {pointerType: 'touch'}}); } else { - // TODO add modifiers here? Maybe move into pressElement if we get more cases for different types of modifier keys if (selectionBehavior === 'replace' && interactionType !== 'touch') { - await this.user.keyboard(`[${pointerModifierKey}>]`); + await this.user.keyboard(`[${metaKey}>]`); } await pressElement(this.user, row, interactionType); if (selectionBehavior === 'replace' && interactionType !== 'touch') { - await this.user.keyboard(`[/${pointerModifierKey}]`); + await this.user.keyboard(`[/${metaKey}]`); } } } @@ -208,7 +206,7 @@ export class TreeTester { // TODO: We always Use Option/Ctrl when keyboard navigating so selection isn't changed // in selectionmode="replace"/highlight selection when navigating to the row that the user wants // to expand. Discuss if this is useful or not - await this.keyboardNavigateToRow({row, needsModifierKey: true}); + await this.keyboardNavigateToRow({row, useAltKey: true}); if (row.getAttribute('aria-expanded') === 'true') { await this.user.keyboard('[ArrowLeft]'); } else { @@ -242,13 +240,9 @@ export class TreeTester { return; } - if (document.activeElement !== this._tree || !this._tree.contains(document.activeElement)) { - act(() => this._tree.focus()); - } - // TODO: same as above, uses the modifier key to make sure we don't modify selection state on row focus // as we keyboard navigate to the row we want activate - await this.keyboardNavigateToRow({row, needsModifierKey: true}); + await this.keyboardNavigateToRow({row, useAltKey: true}); await this.user.keyboard('[Enter]'); } else { await pressElement(this.user, row, interactionType); From fc4b72753da2455a0616a096220a5e261b4e916d Mon Sep 17 00:00:00 2001 From: Daniel Lu Date: Wed, 2 Apr 2025 17:04:17 -0700 Subject: [PATCH 14/17] add test for deselection with modifier and add gridlist tests --- .../@react-aria/test-utils/src/gridlist.ts | 2 +- packages/@react-aria/test-utils/src/table.ts | 2 +- .../tree/test/TreeView.test.tsx | 35 ++++ .../test/GridList.test.js | 171 +++++++++++++++++- 4 files changed, 207 insertions(+), 3 deletions(-) diff --git a/packages/@react-aria/test-utils/src/gridlist.ts b/packages/@react-aria/test-utils/src/gridlist.ts index fdc1dcd56c3..b837245abc8 100644 --- a/packages/@react-aria/test-utils/src/gridlist.ts +++ b/packages/@react-aria/test-utils/src/gridlist.ts @@ -11,8 +11,8 @@ */ import {act, within} from '@testing-library/react'; -import {GridListTesterOpts, GridRowActionOpts, ToggleGridRowOpts, UserOpts} from './types'; import {getAltKey, getMetaKey, pressElement, triggerLongPress} from './events'; +import {GridListTesterOpts, GridRowActionOpts, ToggleGridRowOpts, UserOpts} from './types'; interface GridListToggleRowOpts extends ToggleGridRowOpts {} interface GridListRowActionOpts extends GridRowActionOpts {} diff --git a/packages/@react-aria/test-utils/src/table.ts b/packages/@react-aria/test-utils/src/table.ts index a80f0a6b57d..558d3348b10 100644 --- a/packages/@react-aria/test-utils/src/table.ts +++ b/packages/@react-aria/test-utils/src/table.ts @@ -11,8 +11,8 @@ */ import {act, waitFor, within} from '@testing-library/react'; -import {GridRowActionOpts, TableTesterOpts, ToggleGridRowOpts, UserOpts} from './types'; import {getMetaKey, pressElement, triggerLongPress} from './events'; +import {GridRowActionOpts, TableTesterOpts, ToggleGridRowOpts, UserOpts} from './types'; interface TableToggleRowOpts extends ToggleGridRowOpts {} interface TableToggleSortOpts { diff --git a/packages/@react-spectrum/tree/test/TreeView.test.tsx b/packages/@react-spectrum/tree/test/TreeView.test.tsx index 5a318efdf2f..937e716cce9 100644 --- a/packages/@react-spectrum/tree/test/TreeView.test.tsx +++ b/packages/@react-spectrum/tree/test/TreeView.test.tsx @@ -710,6 +710,15 @@ describe('Tree', () => { expect(new Set(onSelectionChange.mock.calls[1][0])).toEqual(new Set(['Projects'])); expect(treeTester.selectedRows).toHaveLength(1); expect(treeTester.selectedRows[0]).toBe(row1); + + await treeTester.toggleRowSelection({row: row1}); + expect(row1).toHaveAttribute('aria-selected', 'false'); + expect(row1).not.toHaveAttribute('data-selected'); + expect(row2).toHaveAttribute('aria-selected', 'false'); + expect(row2).not.toHaveAttribute('data-selected'); + expect(onSelectionChange).toHaveBeenCalledTimes(3); + expect(new Set(onSelectionChange.mock.calls[2][0])).toEqual(new Set([])); + expect(treeTester.selectedRows).toHaveLength(0); }); it('should perform toggle selection in highlight mode when using modifier keys', async () => { @@ -744,6 +753,17 @@ describe('Tree', () => { expect(new Set(onSelectionChange.mock.calls[1][0])).toEqual(new Set(['Projects', 'Projects-1'])); expect(treeTester.selectedRows).toHaveLength(2); expect(treeTester.selectedRows[0]).toBe(row1); + + // With modifier key, you should be able to deselect on press of the same row + await treeTester.toggleRowSelection({row: row1, selectionBehavior: 'replace'}); + expect(row1).toHaveAttribute('aria-selected', 'false'); + expect(row1).not.toHaveAttribute('data-selected'); + expect(row2).toHaveAttribute('aria-selected', 'true'); + expect(row2).toHaveAttribute('data-selected', 'true'); + expect(onSelectionChange).toHaveBeenCalledTimes(3); + expect(new Set(onSelectionChange.mock.calls[2][0])).toEqual(new Set(['Projects-1'])); + expect(treeTester.selectedRows).toHaveLength(1); + expect(treeTester.selectedRows[0]).toBe(row2); }); it('should perform replace selection in highlight mode when not using modifier keys', async () => { @@ -779,6 +799,13 @@ describe('Tree', () => { expect(new Set(onSelectionChange.mock.calls[1][0])).toEqual(new Set(['Projects'])); expect(treeTester.selectedRows).toHaveLength(1); expect(treeTester.selectedRows[0]).toBe(row1); + + // pressing without modifier keys won't deselect the row + await treeTester.toggleRowSelection({row: row1}); + expect(row1).toHaveAttribute('aria-selected', 'true'); + expect(row1).toHaveAttribute('data-selected', 'true'); + expect(onSelectionChange).toHaveBeenCalledTimes(2); + expect(treeTester.selectedRows).toHaveLength(1); } else { // touch always behaves as toggle expect(row1).toHaveAttribute('aria-selected', 'true'); @@ -789,6 +816,14 @@ describe('Tree', () => { expect(new Set(onSelectionChange.mock.calls[1][0])).toEqual(new Set(['Projects', 'Projects-1'])); expect(treeTester.selectedRows).toHaveLength(2); expect(treeTester.selectedRows[0]).toBe(row1); + + await treeTester.toggleRowSelection({row: row1}); + expect(row1).toHaveAttribute('aria-selected', 'false'); + expect(row1).not.toHaveAttribute('data-selected'); + expect(onSelectionChange).toHaveBeenCalledTimes(3); + expect(new Set(onSelectionChange.mock.calls[2][0])).toEqual(new Set(['Projects-1'])); + expect(treeTester.selectedRows).toHaveLength(1); + expect(treeTester.selectedRows[0]).toBe(row2); } }); }); diff --git a/packages/react-aria-components/test/GridList.test.js b/packages/react-aria-components/test/GridList.test.js index 9a4d106fe3c..51e2b91ca2b 100644 --- a/packages/react-aria-components/test/GridList.test.js +++ b/packages/react-aria-components/test/GridList.test.js @@ -30,8 +30,8 @@ import { useDragAndDrop, Virtualizer } from '../'; +import {installPointerEvent, User} from '@react-aria/test-utils'; import React from 'react'; -import {User} from '@react-aria/test-utils'; import userEvent from '@testing-library/user-event'; let TestGridList = ({listBoxProps, itemProps}) => ( @@ -62,6 +62,7 @@ let renderGridList = (listBoxProps, itemProps) => render( { let user; let testUtilUser = new User(); + let onSelectionChange = jest.fn(); beforeAll(() => { user = userEvent.setup({delay: null, pointerMap}); @@ -73,6 +74,10 @@ describe('GridList', () => { jest.clearAllMocks(); }); + afterAll(() => { + jest.restoreAllMocks(); + }); + it('should render with default classes', () => { let {getByRole} = renderGridList(); let gridListTester = testUtilUser.createTester('GridList', {root: getByRole('grid')}); @@ -392,6 +397,170 @@ describe('GridList', () => { expect(items[2]).toHaveAttribute('aria-selected', 'true'); }); + describe('selectionBehavior="replace"', () => { + // Required for proper touch detection + installPointerEvent(); + let GridListNoCheckboxes = ({listBoxProps, itemProps}) => ( + + Cat + Dog + Kangaroo + + ); + + describe.each(['mouse', 'keyboard', 'touch'])('%s', (type) => { + it('should perform selection with single selection', async () => { + let {getByRole} = render(); + let gridListTester = testUtilUser.createTester('GridList', {user, root: getByRole('grid'), interactionType: type}); + let rows = gridListTester.rows; + + for (let row of gridListTester.rows) { + let checkbox = within(row).queryByRole('checkbox'); + expect(checkbox).toBeNull(); + expect(row).toHaveAttribute('aria-selected', 'false'); + expect(row).not.toHaveAttribute('data-selected'); + expect(row).toHaveAttribute('data-selection-mode', 'single'); + } + + let row2 = rows[2]; + await gridListTester.toggleRowSelection({row: 'Kangaroo'}); + expect(row2).toHaveAttribute('aria-selected', 'true'); + expect(row2).toHaveAttribute('data-selected', 'true'); + expect(onSelectionChange).toHaveBeenCalledTimes(1); + expect(new Set(onSelectionChange.mock.calls[0][0])).toEqual(new Set(['kangaroo'])); + expect(gridListTester.selectedRows).toHaveLength(1); + expect(gridListTester.selectedRows[0]).toBe(row2); + + let row1 = rows[1]; + await gridListTester.toggleRowSelection({row: row1}); + expect(row1).toHaveAttribute('aria-selected', 'true'); + expect(row1).toHaveAttribute('data-selected', 'true'); + expect(row2).toHaveAttribute('aria-selected', 'false'); + expect(row2).not.toHaveAttribute('data-selected'); + expect(onSelectionChange).toHaveBeenCalledTimes(2); + expect(new Set(onSelectionChange.mock.calls[1][0])).toEqual(new Set(['dog'])); + expect(gridListTester.selectedRows).toHaveLength(1); + expect(gridListTester.selectedRows[0]).toBe(row1); + + await gridListTester.toggleRowSelection({row: row1}); + expect(row1).toHaveAttribute('aria-selected', 'false'); + expect(row1).not.toHaveAttribute('data-selected'); + expect(row2).toHaveAttribute('aria-selected', 'false'); + expect(row2).not.toHaveAttribute('data-selected'); + expect(onSelectionChange).toHaveBeenCalledTimes(3); + expect(new Set(onSelectionChange.mock.calls[2][0])).toEqual(new Set([])); + expect(gridListTester.selectedRows).toHaveLength(0); + }); + + it('should perform toggle selection in highlight mode when using modifier keys', async () => { + let {getByRole} = render(); + let gridListTester = testUtilUser.createTester('GridList', {user, root: getByRole('grid'), interactionType: type}); + let rows = gridListTester.rows; + + for (let row of gridListTester.rows) { + let checkbox = within(row).queryByRole('checkbox'); + expect(checkbox).toBeNull(); + expect(row).toHaveAttribute('aria-selected', 'false'); + expect(row).not.toHaveAttribute('data-selected'); + expect(row).toHaveAttribute('data-selection-mode', 'multiple'); + } + + let row2 = rows[2]; + await gridListTester.toggleRowSelection({row: 'Kangaroo', selectionBehavior: 'replace'}); + expect(row2).toHaveAttribute('aria-selected', 'true'); + expect(row2).toHaveAttribute('data-selected', 'true'); + expect(onSelectionChange).toHaveBeenCalledTimes(1); + expect(new Set(onSelectionChange.mock.calls[0][0])).toEqual(new Set(['kangaroo'])); + expect(gridListTester.selectedRows).toHaveLength(1); + expect(gridListTester.selectedRows[0]).toBe(row2); + + let row1 = rows[1]; + await gridListTester.toggleRowSelection({row: row1, selectionBehavior: 'replace'}); + expect(row1).toHaveAttribute('aria-selected', 'true'); + expect(row1).toHaveAttribute('data-selected', 'true'); + expect(row2).toHaveAttribute('aria-selected', 'true'); + expect(row2).toHaveAttribute('data-selected', 'true'); + expect(onSelectionChange).toHaveBeenCalledTimes(2); + expect(new Set(onSelectionChange.mock.calls[1][0])).toEqual(new Set(['dog', 'kangaroo'])); + expect(gridListTester.selectedRows).toHaveLength(2); + expect(gridListTester.selectedRows[0]).toBe(row1); + expect(gridListTester.selectedRows[1]).toBe(row2); + + // With modifier key, you should be able to deselect on press of the same row + await gridListTester.toggleRowSelection({row: row1, selectionBehavior: 'replace'}); + expect(row1).toHaveAttribute('aria-selected', 'false'); + expect(row1).not.toHaveAttribute('data-selected'); + expect(row2).toHaveAttribute('aria-selected', 'true'); + expect(row2).toHaveAttribute('data-selected', 'true'); + expect(onSelectionChange).toHaveBeenCalledTimes(3); + expect(new Set(onSelectionChange.mock.calls[2][0])).toEqual(new Set(['kangaroo'])); + expect(gridListTester.selectedRows).toHaveLength(1); + expect(gridListTester.selectedRows[0]).toBe(row2); + }); + + it('should perform replace selection in highlight mode when not using modifier keys', async () => { + let {getByRole} = render(); + let gridListTester = testUtilUser.createTester('GridList', {user, root: getByRole('grid'), interactionType: type}); + let rows = gridListTester.rows; + + for (let row of gridListTester.rows) { + let checkbox = within(row).queryByRole('checkbox'); + expect(checkbox).toBeNull(); + expect(row).toHaveAttribute('aria-selected', 'false'); + expect(row).not.toHaveAttribute('data-selected'); + expect(row).toHaveAttribute('data-selection-mode', 'multiple'); + } + + let row2 = rows[2]; + await gridListTester.toggleRowSelection({row: 'Kangaroo'}); + expect(row2).toHaveAttribute('aria-selected', 'true'); + expect(row2).toHaveAttribute('data-selected', 'true'); + expect(onSelectionChange).toHaveBeenCalledTimes(1); + expect(new Set(onSelectionChange.mock.calls[0][0])).toEqual(new Set(['kangaroo'])); + expect(gridListTester.selectedRows).toHaveLength(1); + expect(gridListTester.selectedRows[0]).toBe(row2); + + let row1 = rows[1]; + await gridListTester.toggleRowSelection({row: row1}); + if (type !== 'touch') { + expect(row1).toHaveAttribute('aria-selected', 'true'); + expect(row1).toHaveAttribute('data-selected', 'true'); + expect(row2).toHaveAttribute('aria-selected', 'false'); + expect(row2).not.toHaveAttribute('data-selected'); + expect(onSelectionChange).toHaveBeenCalledTimes(2); + expect(new Set(onSelectionChange.mock.calls[1][0])).toEqual(new Set(['dog'])); + expect(gridListTester.selectedRows).toHaveLength(1); + expect(gridListTester.selectedRows[0]).toBe(row1); + + // pressing without modifier keys won't deselect the row + await gridListTester.toggleRowSelection({row: row1}); + expect(row1).toHaveAttribute('aria-selected', 'true'); + expect(row1).toHaveAttribute('data-selected', 'true'); + expect(onSelectionChange).toHaveBeenCalledTimes(2); + expect(gridListTester.selectedRows).toHaveLength(1); + } else { + // touch always behaves as toggle + expect(row1).toHaveAttribute('aria-selected', 'true'); + expect(row1).toHaveAttribute('data-selected', 'true'); + expect(row2).toHaveAttribute('aria-selected', 'true'); + expect(row2).toHaveAttribute('data-selected', 'true'); + expect(onSelectionChange).toHaveBeenCalledTimes(2); + expect(new Set(onSelectionChange.mock.calls[1][0])).toEqual(new Set(['dog', 'kangaroo'])); + expect(gridListTester.selectedRows).toHaveLength(2); + expect(gridListTester.selectedRows[0]).toBe(row1); + + await gridListTester.toggleRowSelection({row: row1}); + expect(row1).toHaveAttribute('aria-selected', 'false'); + expect(row1).not.toHaveAttribute('data-selected'); + expect(onSelectionChange).toHaveBeenCalledTimes(3); + expect(new Set(onSelectionChange.mock.calls[2][0])).toEqual(new Set(['kangaroo'])); + expect(gridListTester.selectedRows).toHaveLength(1); + expect(gridListTester.selectedRows[0]).toBe(row2); + } + }); + }); + }); + it('should support virtualizer', async () => { let items = []; for (let i = 0; i < 50; i++) { From 972e5a231ff61d1881ce444d83d3a02cffc50d11 Mon Sep 17 00:00:00 2001 From: Daniel Lu Date: Thu, 3 Apr 2025 09:40:53 -0700 Subject: [PATCH 15/17] fix build --- yarn.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/yarn.lock b/yarn.lock index ea0314d6b5a..3f29895dd2f 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6728,8 +6728,8 @@ __metadata: version: 0.0.0-use.local resolution: "@react-aria/test-utils@workspace:packages/@react-aria/test-utils" dependencies: + "@react-aria/utils": "npm:^3.28.1" "@swc/helpers": "npm:^0.5.0" - react-aria-components: "npm:^1.7.1" peerDependencies: "@testing-library/react": ^16.0.0 "@testing-library/user-event": ^14.0.0 From 53763bd31382a2cecb63bdfc69308b0e1ac42fb5 Mon Sep 17 00:00:00 2001 From: Daniel Lu Date: Thu, 3 Apr 2025 13:37:44 -0700 Subject: [PATCH 16/17] add listbox test and fix logic for keyboard selection in utils if a checkbox wasnt present we werent using the keyboard navigate logic flow --- .../@react-aria/test-utils/src/gridlist.ts | 6 +- .../@react-aria/test-utils/src/listbox.ts | 16 +- packages/@react-aria/test-utils/src/menu.ts | 2 +- packages/@react-aria/test-utils/src/select.ts | 2 +- packages/@react-aria/test-utils/src/table.ts | 2 +- packages/@react-aria/test-utils/src/tabs.ts | 2 +- packages/@react-aria/test-utils/src/tree.ts | 6 +- .../tree/test/TreeView.test.tsx | 102 +++++++--- .../test/GridList.test.js | 104 +++++++--- .../test/ListBox.test.js | 182 ++++++++++++++++++ 10 files changed, 348 insertions(+), 76 deletions(-) diff --git a/packages/@react-aria/test-utils/src/gridlist.ts b/packages/@react-aria/test-utils/src/gridlist.ts index b837245abc8..b2c6e7df09e 100644 --- a/packages/@react-aria/test-utils/src/gridlist.ts +++ b/packages/@react-aria/test-utils/src/gridlist.ts @@ -66,7 +66,7 @@ export class GridListTester { throw new Error('Option provided is not in the gridlist'); } - if (document.activeElement !== this._gridlist || !this._gridlist.contains(document.activeElement)) { + if (document.activeElement !== this._gridlist && !this._gridlist.contains(document.activeElement)) { act(() => this._gridlist.focus()); } @@ -129,12 +129,12 @@ export class GridListTester { // this would be better than the check to do nothing in events.ts // also, it'd be good to be able to trigger selection on the row instead of having to go to the checkbox directly - if (interactionType === 'keyboard' && !checkboxSelection) { + if (interactionType === 'keyboard' && (!checkboxSelection || !rowCheckbox)) { await this.keyboardNavigateToRow({row, useAltKey: selectionBehavior === 'replace'}); if (selectionBehavior === 'replace') { await this.user.keyboard(`[${altKey}>]`); } - await this.user.keyboard('{Space}'); + await this.user.keyboard('[Space]'); if (selectionBehavior === 'replace') { await this.user.keyboard(`[/${altKey}]`); } diff --git a/packages/@react-aria/test-utils/src/listbox.ts b/packages/@react-aria/test-utils/src/listbox.ts index e16fae40283..7c0093d4032 100644 --- a/packages/@react-aria/test-utils/src/listbox.ts +++ b/packages/@react-aria/test-utils/src/listbox.ts @@ -103,26 +103,17 @@ export class ListBoxTester { throw new Error('Option provided is not in the listbox'); } - if (document.activeElement !== this._listbox || !this._listbox.contains(document.activeElement)) { + if (document.activeElement !== this._listbox && !this._listbox.contains(document.activeElement)) { act(() => this._listbox.focus()); - } - - await this.user.keyboard(`${useAltKey ? `[${altKey}>]` : ''}[ArrowDown]${useAltKey ? `[/${altKey}]` : ''}`); - - // TODO: not sure about doing same while loop that exists in other implementations of keyboardNavigateToOption, - // feels like it could break easily - if (document.activeElement?.getAttribute('role') !== 'option') { - await act(async () => { - option.focus(); - }); + await this.user.keyboard(`${useAltKey ? `[${altKey}>]` : ''}[ArrowDown]${useAltKey ? `[/${altKey}]` : ''}`); } let currIndex = options.indexOf(document.activeElement as HTMLElement); if (currIndex === -1) { throw new Error('ActiveElement is not in the listbox'); } - let direction = targetIndex > currIndex ? 'down' : 'up'; + let direction = targetIndex > currIndex ? 'down' : 'up'; if (useAltKey) { await this.user.keyboard(`[${altKey}>]`); } @@ -168,7 +159,6 @@ export class ListBoxTester { await this.user.keyboard(`[${altKey}>]`); } await this.user.keyboard(`[${keyboardActivation}]`); - if (selectionBehavior === 'replace') { await this.user.keyboard(`[/${altKey}]`); } diff --git a/packages/@react-aria/test-utils/src/menu.ts b/packages/@react-aria/test-utils/src/menu.ts index 232f7b7685c..87af5c11fd0 100644 --- a/packages/@react-aria/test-utils/src/menu.ts +++ b/packages/@react-aria/test-utils/src/menu.ts @@ -215,7 +215,7 @@ export class MenuTester { return; } - if (document.activeElement !== menu || !menu.contains(document.activeElement)) { + if (document.activeElement !== menu && !menu.contains(document.activeElement)) { act(() => menu.focus()); } diff --git a/packages/@react-aria/test-utils/src/select.ts b/packages/@react-aria/test-utils/src/select.ts index 5321e3c39a3..5f84dc4267d 100644 --- a/packages/@react-aria/test-utils/src/select.ts +++ b/packages/@react-aria/test-utils/src/select.ts @@ -183,7 +183,7 @@ export class SelectTester { return; } - if (document.activeElement !== listbox || !listbox.contains(document.activeElement)) { + if (document.activeElement !== listbox && !listbox.contains(document.activeElement)) { act(() => listbox.focus()); } await this.keyboardNavigateToOption({option}); diff --git a/packages/@react-aria/test-utils/src/table.ts b/packages/@react-aria/test-utils/src/table.ts index 558d3348b10..69b783fc949 100644 --- a/packages/@react-aria/test-utils/src/table.ts +++ b/packages/@react-aria/test-utils/src/table.ts @@ -81,7 +81,7 @@ export class TableTester { if (selectionBehavior === 'replace') { await this.user.keyboard(`[${altKey}>]`); } - await this.user.keyboard('{Space}'); + await this.user.keyboard('[Space]'); if (selectionBehavior === 'replace') { await this.user.keyboard(`[/${altKey}]`); } diff --git a/packages/@react-aria/test-utils/src/tabs.ts b/packages/@react-aria/test-utils/src/tabs.ts index a51690bf25b..d52672c6aa0 100644 --- a/packages/@react-aria/test-utils/src/tabs.ts +++ b/packages/@react-aria/test-utils/src/tabs.ts @@ -137,7 +137,7 @@ export class TabsTester { } if (interactionType === 'keyboard') { - if (document.activeElement !== this._tablist || !this._tablist.contains(document.activeElement)) { + if (document.activeElement !== this._tablist && !this._tablist.contains(document.activeElement)) { act(() => this._tablist.focus()); } diff --git a/packages/@react-aria/test-utils/src/tree.ts b/packages/@react-aria/test-utils/src/tree.ts index a535c0f7d43..1fc98a0265c 100644 --- a/packages/@react-aria/test-utils/src/tree.ts +++ b/packages/@react-aria/test-utils/src/tree.ts @@ -73,7 +73,7 @@ export class TreeTester { throw new Error('Option provided is not in the tree'); } - if (document.activeElement !== this._tree || !this._tree.contains(document.activeElement)) { + if (document.activeElement !== this._tree && !this._tree.contains(document.activeElement)) { act(() => this._tree.focus()); } @@ -136,12 +136,12 @@ export class TreeTester { // this would be better than the check to do nothing in events.ts // also, it'd be good to be able to trigger selection on the row instead of having to go to the checkbox directly - if (interactionType === 'keyboard' && !checkboxSelection) { + if (interactionType === 'keyboard' && (!checkboxSelection || !rowCheckbox)) { await this.keyboardNavigateToRow({row, useAltKey: selectionBehavior === 'replace'}); if (selectionBehavior === 'replace') { await this.user.keyboard(`[${altKey}>]`); } - await this.user.keyboard('{Space}'); + await this.user.keyboard('[Space]'); if (selectionBehavior === 'replace') { await this.user.keyboard(`[/${altKey}]`); } diff --git a/packages/@react-spectrum/tree/test/TreeView.test.tsx b/packages/@react-spectrum/tree/test/TreeView.test.tsx index 937e716cce9..d414d7ca015 100644 --- a/packages/@react-spectrum/tree/test/TreeView.test.tsx +++ b/packages/@react-spectrum/tree/test/TreeView.test.tsx @@ -692,32 +692,45 @@ describe('Tree', () => { } let row2 = rows[2]; - await treeTester.toggleRowSelection({row: 'Projects-1'}); + await treeTester.toggleRowSelection({row: 'Projects-1', selectionBehavior: 'replace'}); expect(row2).toHaveAttribute('aria-selected', 'true'); expect(row2).toHaveAttribute('data-selected', 'true'); - expect(onSelectionChange).toHaveBeenCalledTimes(1); - expect(new Set(onSelectionChange.mock.calls[0][0])).toEqual(new Set(['Projects-1'])); + if (type === 'keyboard') { + // Called twice because initial focus will select the first keyboard focused row + expect(onSelectionChange).toHaveBeenCalledTimes(2); + } else { + expect(onSelectionChange).toHaveBeenCalledTimes(1); + } + expect(new Set(onSelectionChange.mock.calls.at(-1)[0])).toEqual(new Set(['Projects-1'])); expect(treeTester.selectedRows).toHaveLength(1); expect(treeTester.selectedRows[0]).toBe(row2); let row1 = rows[1]; - await treeTester.toggleRowSelection({row: row1}); + await treeTester.toggleRowSelection({row: row1, selectionBehavior: 'replace'}); expect(row1).toHaveAttribute('aria-selected', 'true'); expect(row1).toHaveAttribute('data-selected', 'true'); expect(row2).toHaveAttribute('aria-selected', 'false'); expect(row2).not.toHaveAttribute('data-selected'); - expect(onSelectionChange).toHaveBeenCalledTimes(2); - expect(new Set(onSelectionChange.mock.calls[1][0])).toEqual(new Set(['Projects'])); + if (type === 'keyboard') { + expect(onSelectionChange).toHaveBeenCalledTimes(3); + } else { + expect(onSelectionChange).toHaveBeenCalledTimes(2); + } + expect(new Set(onSelectionChange.mock.calls.at(-1)[0])).toEqual(new Set(['Projects'])); expect(treeTester.selectedRows).toHaveLength(1); expect(treeTester.selectedRows[0]).toBe(row1); - await treeTester.toggleRowSelection({row: row1}); + await treeTester.toggleRowSelection({row: row1, selectionBehavior: 'replace'}); expect(row1).toHaveAttribute('aria-selected', 'false'); expect(row1).not.toHaveAttribute('data-selected'); expect(row2).toHaveAttribute('aria-selected', 'false'); expect(row2).not.toHaveAttribute('data-selected'); - expect(onSelectionChange).toHaveBeenCalledTimes(3); - expect(new Set(onSelectionChange.mock.calls[2][0])).toEqual(new Set([])); + if (type === 'keyboard') { + expect(onSelectionChange).toHaveBeenCalledTimes(4); + } else { + expect(onSelectionChange).toHaveBeenCalledTimes(3); + } + expect(new Set(onSelectionChange.mock.calls.at(-1)[0])).toEqual(new Set([])); expect(treeTester.selectedRows).toHaveLength(0); }); @@ -738,10 +751,18 @@ describe('Tree', () => { await treeTester.toggleRowSelection({row: 'Projects-1', selectionBehavior: 'replace'}); expect(row2).toHaveAttribute('aria-selected', 'true'); expect(row2).toHaveAttribute('data-selected', 'true'); - expect(onSelectionChange).toHaveBeenCalledTimes(1); - expect(new Set(onSelectionChange.mock.calls[0][0])).toEqual(new Set(['Projects-1'])); - expect(treeTester.selectedRows).toHaveLength(1); - expect(treeTester.selectedRows[0]).toBe(row2); + if (type === 'keyboard') { + // Called twice because initial focus will select the first keyboard focused row, meaning we have two items selected + expect(onSelectionChange).toHaveBeenCalledTimes(2); + expect(new Set(onSelectionChange.mock.calls.at(-1)[0])).toEqual(new Set(['Photos', 'Projects-1'])); + expect(treeTester.selectedRows).toHaveLength(2); + expect(treeTester.selectedRows[1]).toBe(row2); + } else { + expect(onSelectionChange).toHaveBeenCalledTimes(1); + expect(new Set(onSelectionChange.mock.calls.at(-1)[0])).toEqual(new Set(['Projects-1'])); + expect(treeTester.selectedRows).toHaveLength(1); + expect(treeTester.selectedRows[0]).toBe(row2); + } let row1 = rows[1]; await treeTester.toggleRowSelection({row: row1, selectionBehavior: 'replace'}); @@ -749,10 +770,19 @@ describe('Tree', () => { expect(row1).toHaveAttribute('data-selected', 'true'); expect(row2).toHaveAttribute('aria-selected', 'true'); expect(row2).toHaveAttribute('data-selected', 'true'); - expect(onSelectionChange).toHaveBeenCalledTimes(2); - expect(new Set(onSelectionChange.mock.calls[1][0])).toEqual(new Set(['Projects', 'Projects-1'])); - expect(treeTester.selectedRows).toHaveLength(2); - expect(treeTester.selectedRows[0]).toBe(row1); + if (type === 'keyboard') { + expect(onSelectionChange).toHaveBeenCalledTimes(3); + expect(new Set(onSelectionChange.mock.calls.at(-1)[0])).toEqual(new Set(['Photos', 'Projects-1', 'Projects'])); + expect(treeTester.selectedRows).toHaveLength(3); + expect(treeTester.selectedRows[1]).toBe(row1); + expect(treeTester.selectedRows[2]).toBe(row2); + } else { + expect(onSelectionChange).toHaveBeenCalledTimes(2); + expect(new Set(onSelectionChange.mock.calls.at(-1)[0])).toEqual(new Set(['Projects-1', 'Projects'])); + expect(treeTester.selectedRows).toHaveLength(2); + expect(treeTester.selectedRows[0]).toBe(row1); + expect(treeTester.selectedRows[1]).toBe(row2); + } // With modifier key, you should be able to deselect on press of the same row await treeTester.toggleRowSelection({row: row1, selectionBehavior: 'replace'}); @@ -760,10 +790,17 @@ describe('Tree', () => { expect(row1).not.toHaveAttribute('data-selected'); expect(row2).toHaveAttribute('aria-selected', 'true'); expect(row2).toHaveAttribute('data-selected', 'true'); - expect(onSelectionChange).toHaveBeenCalledTimes(3); - expect(new Set(onSelectionChange.mock.calls[2][0])).toEqual(new Set(['Projects-1'])); - expect(treeTester.selectedRows).toHaveLength(1); - expect(treeTester.selectedRows[0]).toBe(row2); + if (type === 'keyboard') { + expect(onSelectionChange).toHaveBeenCalledTimes(4); + expect(new Set(onSelectionChange.mock.calls.at(-1)[0])).toEqual(new Set(['Photos', 'Projects-1'])); + expect(treeTester.selectedRows).toHaveLength(2); + expect(treeTester.selectedRows[1]).toBe(row2); + } else { + expect(onSelectionChange).toHaveBeenCalledTimes(3); + expect(new Set(onSelectionChange.mock.calls.at(-1)[0])).toEqual(new Set(['Projects-1'])); + expect(treeTester.selectedRows).toHaveLength(1); + expect(treeTester.selectedRows[0]).toBe(row2); + } }); it('should perform replace selection in highlight mode when not using modifier keys', async () => { @@ -783,8 +820,13 @@ describe('Tree', () => { await treeTester.toggleRowSelection({row: 'Projects-1'}); expect(row2).toHaveAttribute('aria-selected', 'true'); expect(row2).toHaveAttribute('data-selected', 'true'); - expect(onSelectionChange).toHaveBeenCalledTimes(1); - expect(new Set(onSelectionChange.mock.calls[0][0])).toEqual(new Set(['Projects-1'])); + if (type === 'keyboard') { + // Called multiple times since selection changes on option focus as we arrow down to the target option + expect(onSelectionChange).toHaveBeenCalledTimes(3); + } else { + expect(onSelectionChange).toHaveBeenCalledTimes(1); + } + expect(new Set(onSelectionChange.mock.calls.at(-1)[0])).toEqual(new Set(['Projects-1'])); expect(treeTester.selectedRows).toHaveLength(1); expect(treeTester.selectedRows[0]).toBe(row2); @@ -795,8 +837,12 @@ describe('Tree', () => { expect(row1).toHaveAttribute('data-selected', 'true'); expect(row2).toHaveAttribute('aria-selected', 'false'); expect(row2).not.toHaveAttribute('data-selected'); - expect(onSelectionChange).toHaveBeenCalledTimes(2); - expect(new Set(onSelectionChange.mock.calls[1][0])).toEqual(new Set(['Projects'])); + if (type === 'keyboard') { + expect(onSelectionChange).toHaveBeenCalledTimes(4); + } else { + expect(onSelectionChange).toHaveBeenCalledTimes(2); + } + expect(new Set(onSelectionChange.mock.calls.at(-1)[0])).toEqual(new Set(['Projects'])); expect(treeTester.selectedRows).toHaveLength(1); expect(treeTester.selectedRows[0]).toBe(row1); @@ -804,7 +850,11 @@ describe('Tree', () => { await treeTester.toggleRowSelection({row: row1}); expect(row1).toHaveAttribute('aria-selected', 'true'); expect(row1).toHaveAttribute('data-selected', 'true'); - expect(onSelectionChange).toHaveBeenCalledTimes(2); + if (type === 'keyboard') { + expect(onSelectionChange).toHaveBeenCalledTimes(4); + } else { + expect(onSelectionChange).toHaveBeenCalledTimes(2); + } expect(treeTester.selectedRows).toHaveLength(1); } else { // touch always behaves as toggle diff --git a/packages/react-aria-components/test/GridList.test.js b/packages/react-aria-components/test/GridList.test.js index 51e2b91ca2b..bd3cfd6a25a 100644 --- a/packages/react-aria-components/test/GridList.test.js +++ b/packages/react-aria-components/test/GridList.test.js @@ -423,32 +423,46 @@ describe('GridList', () => { } let row2 = rows[2]; - await gridListTester.toggleRowSelection({row: 'Kangaroo'}); + expect(onSelectionChange).toHaveBeenCalledTimes(0); + await gridListTester.toggleRowSelection({row: 'Kangaroo', selectionBehavior: 'replace'}); expect(row2).toHaveAttribute('aria-selected', 'true'); expect(row2).toHaveAttribute('data-selected', 'true'); - expect(onSelectionChange).toHaveBeenCalledTimes(1); - expect(new Set(onSelectionChange.mock.calls[0][0])).toEqual(new Set(['kangaroo'])); + if (type === 'keyboard') { + // Called twice because initial focus will select the first keyboard focused row + expect(onSelectionChange).toHaveBeenCalledTimes(2); + } else { + expect(onSelectionChange).toHaveBeenCalledTimes(1); + } + expect(new Set(onSelectionChange.mock.calls.at(-1)[0])).toEqual(new Set(['kangaroo'])); expect(gridListTester.selectedRows).toHaveLength(1); expect(gridListTester.selectedRows[0]).toBe(row2); let row1 = rows[1]; - await gridListTester.toggleRowSelection({row: row1}); + await gridListTester.toggleRowSelection({row: row1, selectionBehavior: 'replace'}); expect(row1).toHaveAttribute('aria-selected', 'true'); expect(row1).toHaveAttribute('data-selected', 'true'); expect(row2).toHaveAttribute('aria-selected', 'false'); expect(row2).not.toHaveAttribute('data-selected'); - expect(onSelectionChange).toHaveBeenCalledTimes(2); - expect(new Set(onSelectionChange.mock.calls[1][0])).toEqual(new Set(['dog'])); + if (type === 'keyboard') { + expect(onSelectionChange).toHaveBeenCalledTimes(3); + } else { + expect(onSelectionChange).toHaveBeenCalledTimes(2); + } + expect(new Set(onSelectionChange.mock.calls.at(-1)[0])).toEqual(new Set(['dog'])); expect(gridListTester.selectedRows).toHaveLength(1); expect(gridListTester.selectedRows[0]).toBe(row1); - await gridListTester.toggleRowSelection({row: row1}); + await gridListTester.toggleRowSelection({row: row1, selectionBehavior: 'replace'}); expect(row1).toHaveAttribute('aria-selected', 'false'); expect(row1).not.toHaveAttribute('data-selected'); expect(row2).toHaveAttribute('aria-selected', 'false'); expect(row2).not.toHaveAttribute('data-selected'); - expect(onSelectionChange).toHaveBeenCalledTimes(3); - expect(new Set(onSelectionChange.mock.calls[2][0])).toEqual(new Set([])); + if (type === 'keyboard') { + expect(onSelectionChange).toHaveBeenCalledTimes(4); + } else { + expect(onSelectionChange).toHaveBeenCalledTimes(3); + } + expect(new Set(onSelectionChange.mock.calls.at(-1)[0])).toEqual(new Set([])); expect(gridListTester.selectedRows).toHaveLength(0); }); @@ -469,10 +483,18 @@ describe('GridList', () => { await gridListTester.toggleRowSelection({row: 'Kangaroo', selectionBehavior: 'replace'}); expect(row2).toHaveAttribute('aria-selected', 'true'); expect(row2).toHaveAttribute('data-selected', 'true'); - expect(onSelectionChange).toHaveBeenCalledTimes(1); - expect(new Set(onSelectionChange.mock.calls[0][0])).toEqual(new Set(['kangaroo'])); - expect(gridListTester.selectedRows).toHaveLength(1); - expect(gridListTester.selectedRows[0]).toBe(row2); + if (type === 'keyboard') { + // Called twice because initial focus will select the first keyboard focused row, meaning we have two items selected + expect(onSelectionChange).toHaveBeenCalledTimes(2); + expect(new Set(onSelectionChange.mock.calls.at(-1)[0])).toEqual(new Set(['cat', 'kangaroo'])); + expect(gridListTester.selectedRows).toHaveLength(2); + expect(gridListTester.selectedRows[1]).toBe(row2); + } else { + expect(onSelectionChange).toHaveBeenCalledTimes(1); + expect(new Set(onSelectionChange.mock.calls.at(-1)[0])).toEqual(new Set(['kangaroo'])); + expect(gridListTester.selectedRows).toHaveLength(1); + expect(gridListTester.selectedRows[0]).toBe(row2); + } let row1 = rows[1]; await gridListTester.toggleRowSelection({row: row1, selectionBehavior: 'replace'}); @@ -480,11 +502,19 @@ describe('GridList', () => { expect(row1).toHaveAttribute('data-selected', 'true'); expect(row2).toHaveAttribute('aria-selected', 'true'); expect(row2).toHaveAttribute('data-selected', 'true'); - expect(onSelectionChange).toHaveBeenCalledTimes(2); - expect(new Set(onSelectionChange.mock.calls[1][0])).toEqual(new Set(['dog', 'kangaroo'])); - expect(gridListTester.selectedRows).toHaveLength(2); - expect(gridListTester.selectedRows[0]).toBe(row1); - expect(gridListTester.selectedRows[1]).toBe(row2); + if (type === 'keyboard') { + expect(onSelectionChange).toHaveBeenCalledTimes(3); + expect(new Set(onSelectionChange.mock.calls.at(-1)[0])).toEqual(new Set(['cat', 'dog', 'kangaroo'])); + expect(gridListTester.selectedRows).toHaveLength(3); + expect(gridListTester.selectedRows[1]).toBe(row1); + expect(gridListTester.selectedRows[2]).toBe(row2); + } else { + expect(onSelectionChange).toHaveBeenCalledTimes(2); + expect(new Set(onSelectionChange.mock.calls.at(-1)[0])).toEqual(new Set(['dog', 'kangaroo'])); + expect(gridListTester.selectedRows).toHaveLength(2); + expect(gridListTester.selectedRows[0]).toBe(row1); + expect(gridListTester.selectedRows[1]).toBe(row2); + } // With modifier key, you should be able to deselect on press of the same row await gridListTester.toggleRowSelection({row: row1, selectionBehavior: 'replace'}); @@ -492,10 +522,17 @@ describe('GridList', () => { expect(row1).not.toHaveAttribute('data-selected'); expect(row2).toHaveAttribute('aria-selected', 'true'); expect(row2).toHaveAttribute('data-selected', 'true'); - expect(onSelectionChange).toHaveBeenCalledTimes(3); - expect(new Set(onSelectionChange.mock.calls[2][0])).toEqual(new Set(['kangaroo'])); - expect(gridListTester.selectedRows).toHaveLength(1); - expect(gridListTester.selectedRows[0]).toBe(row2); + if (type === 'keyboard') { + expect(onSelectionChange).toHaveBeenCalledTimes(4); + expect(new Set(onSelectionChange.mock.calls.at(-1)[0])).toEqual(new Set(['cat', 'kangaroo'])); + expect(gridListTester.selectedRows).toHaveLength(2); + expect(gridListTester.selectedRows[1]).toBe(row2); + } else { + expect(onSelectionChange).toHaveBeenCalledTimes(3); + expect(new Set(onSelectionChange.mock.calls.at(-1)[0])).toEqual(new Set(['kangaroo'])); + expect(gridListTester.selectedRows).toHaveLength(1); + expect(gridListTester.selectedRows[0]).toBe(row2); + } }); it('should perform replace selection in highlight mode when not using modifier keys', async () => { @@ -515,8 +552,13 @@ describe('GridList', () => { await gridListTester.toggleRowSelection({row: 'Kangaroo'}); expect(row2).toHaveAttribute('aria-selected', 'true'); expect(row2).toHaveAttribute('data-selected', 'true'); - expect(onSelectionChange).toHaveBeenCalledTimes(1); - expect(new Set(onSelectionChange.mock.calls[0][0])).toEqual(new Set(['kangaroo'])); + if (type === 'keyboard') { + // Called multiple times since selection changes on option focus as we arrow down to the target option + expect(onSelectionChange).toHaveBeenCalledTimes(3); + } else { + expect(onSelectionChange).toHaveBeenCalledTimes(1); + } + expect(new Set(onSelectionChange.mock.calls.at(-1)[0])).toEqual(new Set(['kangaroo'])); expect(gridListTester.selectedRows).toHaveLength(1); expect(gridListTester.selectedRows[0]).toBe(row2); @@ -527,8 +569,12 @@ describe('GridList', () => { expect(row1).toHaveAttribute('data-selected', 'true'); expect(row2).toHaveAttribute('aria-selected', 'false'); expect(row2).not.toHaveAttribute('data-selected'); - expect(onSelectionChange).toHaveBeenCalledTimes(2); - expect(new Set(onSelectionChange.mock.calls[1][0])).toEqual(new Set(['dog'])); + if (type === 'keyboard') { + expect(onSelectionChange).toHaveBeenCalledTimes(4); + } else { + expect(onSelectionChange).toHaveBeenCalledTimes(2); + } + expect(new Set(onSelectionChange.mock.calls.at(-1)[0])).toEqual(new Set(['dog'])); expect(gridListTester.selectedRows).toHaveLength(1); expect(gridListTester.selectedRows[0]).toBe(row1); @@ -536,7 +582,11 @@ describe('GridList', () => { await gridListTester.toggleRowSelection({row: row1}); expect(row1).toHaveAttribute('aria-selected', 'true'); expect(row1).toHaveAttribute('data-selected', 'true'); - expect(onSelectionChange).toHaveBeenCalledTimes(2); + if (type === 'keyboard') { + expect(onSelectionChange).toHaveBeenCalledTimes(4); + } else { + expect(onSelectionChange).toHaveBeenCalledTimes(2); + } expect(gridListTester.selectedRows).toHaveLength(1); } else { // touch always behaves as toggle diff --git a/packages/react-aria-components/test/ListBox.test.js b/packages/react-aria-components/test/ListBox.test.js index 869f2d841d9..209568a3861 100644 --- a/packages/react-aria-components/test/ListBox.test.js +++ b/packages/react-aria-components/test/ListBox.test.js @@ -62,6 +62,7 @@ let keyPress = (key) => { describe('ListBox', () => { let user; let testUtilUser = new User(); + let onSelectionChange = jest.fn(); beforeAll(() => { user = userEvent.setup({delay: null, pointerMap}); @@ -525,6 +526,187 @@ describe('ListBox', () => { expect(onAction).toHaveBeenCalledTimes(1); }); + describe('selectionBehavior="replace"', () => { + // Required for proper touch detection + installPointerEvent(); + describe.each(['mouse', 'keyboard', 'touch'])('%s', (type) => { + it('should perform selection with single selection', async () => { + let {getByRole} = renderListbox({selectionMode: 'single', selectionBehavior: 'replace', onSelectionChange}); + let listboxTester = testUtilUser.createTester('ListBox', {root: getByRole('listbox'), interactionType: type}); + let options = listboxTester.options(); + + expect(onSelectionChange).toHaveBeenCalledTimes(0); + let option2 = options[2]; + await listboxTester.toggleOptionSelection({option: 'Kangaroo', selectionBehavior: 'replace'}); + expect(option2).toHaveAttribute('aria-selected', 'true'); + expect(option2).toHaveAttribute('data-selected', 'true'); + if (type === 'keyboard') { + // Called twice because initial focus will select the first keyboard focused row + expect(onSelectionChange).toHaveBeenCalledTimes(2); + } else { + expect(onSelectionChange).toHaveBeenCalledTimes(1); + } + expect(new Set(onSelectionChange.mock.calls.at(-1)[0])).toEqual(new Set(['kangaroo'])); + expect(listboxTester.selectedOptions).toHaveLength(1); + expect(listboxTester.selectedOptions[0]).toBe(option2); + + let option1 = options[1]; + await listboxTester.toggleOptionSelection({option: option1, selectionBehavior: 'replace'}); + expect(option1).toHaveAttribute('aria-selected', 'true'); + expect(option1).toHaveAttribute('data-selected', 'true'); + expect(option2).toHaveAttribute('aria-selected', 'false'); + expect(option2).not.toHaveAttribute('data-selected'); + if (type === 'keyboard') { + expect(onSelectionChange).toHaveBeenCalledTimes(3); + } else { + expect(onSelectionChange).toHaveBeenCalledTimes(2); + } + expect(new Set(onSelectionChange.mock.calls.at(-1)[0])).toEqual(new Set(['dog'])); + expect(listboxTester.selectedOptions).toHaveLength(1); + expect(listboxTester.selectedOptions[0]).toBe(option1); + + await listboxTester.toggleOptionSelection({option: option1, selectionBehavior: 'replace'}); + expect(option1).toHaveAttribute('aria-selected', 'false'); + expect(option1).not.toHaveAttribute('data-selected'); + expect(option2).toHaveAttribute('aria-selected', 'false'); + expect(option2).not.toHaveAttribute('data-selected'); + if (type === 'keyboard') { + expect(onSelectionChange).toHaveBeenCalledTimes(4); + } else { + expect(onSelectionChange).toHaveBeenCalledTimes(3); + } + expect(new Set(onSelectionChange.mock.calls.at(-1)[0])).toEqual(new Set([])); + expect(listboxTester.selectedOptions).toHaveLength(0); + }); + + it('should perform toggle selection in highlight mode when using modifier keys', async () => { + let {getByRole} = renderListbox({selectionMode: 'multiple', selectionBehavior: 'replace', onSelectionChange}); + let listboxTester = testUtilUser.createTester('ListBox', {root: getByRole('listbox'), interactionType: type}); + let options = listboxTester.options(); + + let option2 = options[2]; + await listboxTester.toggleOptionSelection({option: 'Kangaroo', selectionBehavior: 'replace'}); + expect(option2).toHaveAttribute('aria-selected', 'true'); + expect(option2).toHaveAttribute('data-selected', 'true'); + if (type === 'keyboard') { + // Called twice because initial focus will select the first keyboard focused row, meaning we have two items selected + expect(onSelectionChange).toHaveBeenCalledTimes(2); + expect(new Set(onSelectionChange.mock.calls.at(-1)[0])).toEqual(new Set(['cat', 'kangaroo'])); + expect(listboxTester.selectedOptions).toHaveLength(2); + expect(listboxTester.selectedOptions[1]).toBe(option2); + } else { + expect(onSelectionChange).toHaveBeenCalledTimes(1); + expect(new Set(onSelectionChange.mock.calls.at(-1)[0])).toEqual(new Set(['kangaroo'])); + expect(listboxTester.selectedOptions).toHaveLength(1); + expect(listboxTester.selectedOptions[0]).toBe(option2); + } + + let option1 = options[1]; + await listboxTester.toggleOptionSelection({option: option1, selectionBehavior: 'replace'}); + expect(option1).toHaveAttribute('aria-selected', 'true'); + expect(option1).toHaveAttribute('data-selected', 'true'); + expect(option2).toHaveAttribute('aria-selected', 'true'); + expect(option2).toHaveAttribute('data-selected', 'true'); + if (type === 'keyboard') { + expect(onSelectionChange).toHaveBeenCalledTimes(3); + expect(new Set(onSelectionChange.mock.calls.at(-1)[0])).toEqual(new Set(['cat', 'dog', 'kangaroo'])); + expect(listboxTester.selectedOptions).toHaveLength(3); + expect(listboxTester.selectedOptions[1]).toBe(option1); + expect(listboxTester.selectedOptions[2]).toBe(option2); + } else { + expect(onSelectionChange).toHaveBeenCalledTimes(2); + expect(new Set(onSelectionChange.mock.calls.at(-1)[0])).toEqual(new Set(['dog', 'kangaroo'])); + expect(listboxTester.selectedOptions).toHaveLength(2); + expect(listboxTester.selectedOptions[0]).toBe(option1); + expect(listboxTester.selectedOptions[1]).toBe(option2); + } + + // With modifier key, you should be able to deselect on press of the same row + await listboxTester.toggleOptionSelection({option: option1, selectionBehavior: 'replace'}); + expect(option1).toHaveAttribute('aria-selected', 'false'); + expect(option1).not.toHaveAttribute('data-selected'); + expect(option2).toHaveAttribute('aria-selected', 'true'); + expect(option2).toHaveAttribute('data-selected', 'true'); + if (type === 'keyboard') { + expect(onSelectionChange).toHaveBeenCalledTimes(4); + expect(new Set(onSelectionChange.mock.calls.at(-1)[0])).toEqual(new Set(['cat', 'kangaroo'])); + expect(listboxTester.selectedOptions).toHaveLength(2); + expect(listboxTester.selectedOptions[1]).toBe(option2); + } else { + expect(onSelectionChange).toHaveBeenCalledTimes(3); + expect(new Set(onSelectionChange.mock.calls.at(-1)[0])).toEqual(new Set(['kangaroo'])); + expect(listboxTester.selectedOptions).toHaveLength(1); + expect(listboxTester.selectedOptions[0]).toBe(option2); + } + }); + + it('should perform replace selection in highlight mode when not using modifier keys', async () => { + let {getByRole} = renderListbox({selectionMode: 'multiple', selectionBehavior: 'replace', onSelectionChange}); + let listboxTester = testUtilUser.createTester('ListBox', {root: getByRole('listbox'), interactionType: type}); + let options = listboxTester.options(); + + let option2 = options[2]; + await listboxTester.toggleOptionSelection({option: 'Kangaroo'}); + expect(option2).toHaveAttribute('aria-selected', 'true'); + expect(option2).toHaveAttribute('data-selected', 'true'); + if (type === 'keyboard') { + // Called multiple times since selection changes on option focus as we arrow down to the target option + expect(onSelectionChange).toHaveBeenCalledTimes(3); + } else { + expect(onSelectionChange).toHaveBeenCalledTimes(1); + } + expect(new Set(onSelectionChange.mock.calls.at(-1)[0])).toEqual(new Set(['kangaroo'])); + expect(listboxTester.selectedOptions).toHaveLength(1); + expect(listboxTester.selectedOptions[0]).toBe(option2); + + let option1 = options[1]; + await listboxTester.toggleOptionSelection({option: option1}); + if (type !== 'touch') { + expect(option1).toHaveAttribute('aria-selected', 'true'); + expect(option1).toHaveAttribute('data-selected', 'true'); + expect(option2).toHaveAttribute('aria-selected', 'false'); + expect(option2).not.toHaveAttribute('data-selected'); + if (type === 'keyboard') { + expect(onSelectionChange).toHaveBeenCalledTimes(4); + } else { + expect(onSelectionChange).toHaveBeenCalledTimes(2); + } + expect(new Set(onSelectionChange.mock.calls.at(-1)[0])).toEqual(new Set(['dog'])); + expect(listboxTester.selectedOptions).toHaveLength(1); + expect(listboxTester.selectedOptions[0]).toBe(option1); + // pressing without modifier keys won't deselect the row + await listboxTester.toggleOptionSelection({option: option1}); + expect(option1).toHaveAttribute('aria-selected', 'true'); + expect(option1).toHaveAttribute('data-selected', 'true'); + if (type === 'keyboard') { + expect(onSelectionChange).toHaveBeenCalledTimes(4); + } else { + expect(onSelectionChange).toHaveBeenCalledTimes(2); + } + expect(listboxTester.selectedOptions).toHaveLength(1); + } else { + // touch always behaves as toggle + expect(option1).toHaveAttribute('aria-selected', 'true'); + expect(option1).toHaveAttribute('data-selected', 'true'); + expect(option2).toHaveAttribute('aria-selected', 'true'); + expect(option2).toHaveAttribute('data-selected', 'true'); + expect(onSelectionChange).toHaveBeenCalledTimes(2); + expect(new Set(onSelectionChange.mock.calls[1][0])).toEqual(new Set(['dog', 'kangaroo'])); + expect(listboxTester.selectedOptions).toHaveLength(2); + expect(listboxTester.selectedOptions[0]).toBe(option1); + + await listboxTester.toggleOptionSelection({option: option1}); + expect(option1).toHaveAttribute('aria-selected', 'false'); + expect(option1).not.toHaveAttribute('data-selected'); + expect(onSelectionChange).toHaveBeenCalledTimes(3); + expect(new Set(onSelectionChange.mock.calls[2][0])).toEqual(new Set(['kangaroo'])); + expect(listboxTester.selectedOptions).toHaveLength(1); + expect(listboxTester.selectedOptions[0]).toBe(option2); + } + }); + }); + }); + describe('with pointer events', () => { installPointerEvent(); it('should trigger selection on long press if both onAction and selection exist (touch only)', async () => { From 96b4a85085713f4b5dad7760c872fda6a3d08b8b Mon Sep 17 00:00:00 2001 From: Daniel Lu Date: Thu, 3 Apr 2025 14:36:10 -0700 Subject: [PATCH 17/17] add table util highlight selection tests and add proper keyboard navigation simulation to util --- .../@react-aria/test-utils/src/gridlist.ts | 12 +- packages/@react-aria/test-utils/src/table.ts | 61 ++++- .../table/test/TestTableUtils.test.tsx | 71 +++++- .../react-aria-components/test/Table.test.js | 226 ++++++++++++++++++ 4 files changed, 348 insertions(+), 22 deletions(-) diff --git a/packages/@react-aria/test-utils/src/gridlist.ts b/packages/@react-aria/test-utils/src/gridlist.ts index b2c6e7df09e..f0f6b0d4533 100644 --- a/packages/@react-aria/test-utils/src/gridlist.ts +++ b/packages/@react-aria/test-utils/src/gridlist.ts @@ -83,14 +83,14 @@ export class GridListTester { } let direction = targetIndex > currIndex ? 'down' : 'up'; + if (useAltKey) { + await this.user.keyboard(`[${altKey}>]`); + } for (let i = 0; i < Math.abs(targetIndex - currIndex); i++) { - if (useAltKey) { - await this.user.keyboard(`[${altKey}>]`); - } await this.user.keyboard(`[${direction === 'down' ? 'ArrowDown' : 'ArrowUp'}]`); - if (useAltKey) { - await this.user.keyboard(`[/${altKey}]`); - } + } + if (useAltKey) { + await this.user.keyboard(`[/${altKey}]`); } }; diff --git a/packages/@react-aria/test-utils/src/table.ts b/packages/@react-aria/test-utils/src/table.ts index 69b783fc949..ffb786a4dba 100644 --- a/packages/@react-aria/test-utils/src/table.ts +++ b/packages/@react-aria/test-utils/src/table.ts @@ -11,7 +11,7 @@ */ import {act, waitFor, within} from '@testing-library/react'; -import {getMetaKey, pressElement, triggerLongPress} from './events'; +import {getAltKey, getMetaKey, pressElement, triggerLongPress} from './events'; import {GridRowActionOpts, TableTesterOpts, ToggleGridRowOpts, UserOpts} from './types'; interface TableToggleRowOpts extends ToggleGridRowOpts {} @@ -48,6 +48,55 @@ export class TableTester { this._interactionType = type; } + // TODO: RTL + private async keyboardNavigateToRow(opts: {row: HTMLElement, useAltKey?: boolean}) { + let {row, useAltKey} = opts; + let altKey = getAltKey(); + let rows = this.rows; + let targetIndex = rows.indexOf(row); + if (targetIndex === -1) { + throw new Error('Row provided is not in the table'); + } + + // Move focus into the table + if (document.activeElement !== this._table && !this._table.contains(document.activeElement)) { + act(() => this._table.focus()); + } + + if (document.activeElement === this._table) { + await this.user.keyboard('[ArrowDown]'); + } + + // If focus is currently somewhere in the first row group (aka on a column), we want to keyboard navigate downwards till we reach the rows + if (this.rowGroups[0].contains(document.activeElement)) { + do { + await this.user.keyboard('[ArrowDown]'); + } while (!this.rowGroups[1].contains(document.activeElement)); + } + + // Move focus onto the row itself + if (this.rowGroups[1].contains(document.activeElement) && document.activeElement!.getAttribute('role') !== 'row') { + do { + await this.user.keyboard('[ArrowLeft]'); + } while (document.activeElement!.getAttribute('role') !== 'row'); + } + let currIndex = rows.indexOf(document.activeElement as HTMLElement); + if (currIndex === -1) { + throw new Error('Current active element is not on any of the table rows'); + } + let direction = targetIndex > currIndex ? 'down' : 'up'; + + if (useAltKey) { + await this.user.keyboard(`[${altKey}>]`); + } + for (let i = 0; i < Math.abs(targetIndex - currIndex); i++) { + await this.user.keyboard(`[${direction === 'down' ? 'ArrowDown' : 'ArrowUp'}]`); + } + if (useAltKey) { + await this.user.keyboard(`[/${altKey}]`); + } + }; + /** * Toggles the selection for the specified table row. Defaults to using the interaction type set on the table tester. */ @@ -73,11 +122,8 @@ export class TableTester { let rowCheckbox = within(row).queryByRole('checkbox'); - if (interactionType === 'keyboard' && !checkboxSelection) { - // TODO: for now focus the row directly until I add keyboard navigation - await act(async () => { - row.focus(); - }); + if (interactionType === 'keyboard' && (!checkboxSelection || !rowCheckbox)) { + await this.keyboardNavigateToRow({row, useAltKey: selectionBehavior === 'replace'}); if (selectionBehavior === 'replace') { await this.user.keyboard(`[${altKey}>]`); } @@ -221,8 +267,7 @@ export class TableTester { if (needsDoubleClick) { await this.user.dblClick(row); } else if (interactionType === 'keyboard') { - // TODO: add keyboard navigation instead of focusing the row directly. Will need to consider if the focus in in the columns - act(() => row.focus()); + await this.keyboardNavigateToRow({row, useAltKey: true}); await this.user.keyboard('[Enter]'); } else { await pressElement(this.user, row, interactionType); diff --git a/packages/@react-spectrum/table/test/TestTableUtils.test.tsx b/packages/@react-spectrum/table/test/TestTableUtils.test.tsx index cd6ecbd9f17..7340e51cb08 100644 --- a/packages/@react-spectrum/table/test/TestTableUtils.test.tsx +++ b/packages/@react-spectrum/table/test/TestTableUtils.test.tsx @@ -107,12 +107,22 @@ describe('Table ', function () { let tableTester = testUtilRealTimer.createTester('Table', {root: screen.getByTestId('test')}); tableTester.setInteractionType(interactionType); await tableTester.toggleRowSelection({row: 2}); - expect(onSelectionChange).toHaveBeenCalledTimes(1); - expect(new Set(onSelectionChange.mock.calls[0][0])).toEqual(new Set(['Foo 3'])); + // Called multiple times because highlight selections will change selection on focus unless you use + // a modifier key + if (interactionType === 'keyboard') { + expect(onSelectionChange).toHaveBeenCalledTimes(3); + } else { + expect(onSelectionChange).toHaveBeenCalledTimes(1); + } + expect(new Set(onSelectionChange.mock.calls.at(-1)[0])).toEqual(new Set(['Foo 3'])); await tableTester.toggleRowSelection({row: 'Foo 4'}); - expect(onSelectionChange).toHaveBeenCalledTimes(2); - expect(new Set(onSelectionChange.mock.calls[1][0])).toEqual(new Set(['Foo 4'])); + if (interactionType === 'keyboard') { + expect(onSelectionChange).toHaveBeenCalledTimes(4); + } else { + expect(onSelectionChange).toHaveBeenCalledTimes(2); + } + expect(new Set(onSelectionChange.mock.calls.at(-1)[0])).toEqual(new Set(['Foo 4'])); await tableTester.toggleSort({column: 2}); expect(onSortChange).toHaveBeenCalledTimes(1); @@ -159,6 +169,8 @@ describe('Table ', function () { ${'touch'} `('with fake timers, interactionType: $interactionType ', ({interactionType}) => { let testUtilFakeTimer = new User({advanceTimer: jest.advanceTimersByTime}); + // For proper touch simulation + installPointerEvent(); beforeAll(function () { jest.useFakeTimers(); }); @@ -204,12 +216,29 @@ describe('Table ', function () { tableTester.setInteractionType(interactionType); await tableTester.toggleRowSelection({row: 2}); - expect(onSelectionChange).toHaveBeenCalledTimes(1); - expect(new Set(onSelectionChange.mock.calls[0][0])).toEqual(new Set(['Foo 3'])); + // Called multiple times because highlight selections will change selection on focus unless you use + // a modifier key + if (interactionType === 'keyboard') { + expect(onSelectionChange).toHaveBeenCalledTimes(3); + } else { + expect(onSelectionChange).toHaveBeenCalledTimes(1); + } + expect(new Set(onSelectionChange.mock.calls.at(-1)[0])).toEqual(new Set(['Foo 3'])); await tableTester.toggleRowSelection({row: 'Foo 4'}); - expect(onSelectionChange).toHaveBeenCalledTimes(2); - expect(new Set(onSelectionChange.mock.calls[1][0])).toEqual(new Set(['Foo 4'])); + if (interactionType === 'keyboard') { + expect(onSelectionChange).toHaveBeenCalledTimes(4); + } else { + expect(onSelectionChange).toHaveBeenCalledTimes(2); + } + + // touch always acts as toggle + if (interactionType === 'touch') { + expect(new Set(onSelectionChange.mock.calls.at(-1)[0])).toEqual(new Set(['Foo 3', 'Foo 4'])); + } else { + expect(new Set(onSelectionChange.mock.calls.at(-1)[0])).toEqual(new Set(['Foo 4'])); + } + await tableTester.toggleSort({column: 2}); expect(onSortChange).toHaveBeenCalledTimes(1); @@ -223,6 +252,32 @@ describe('Table ', function () { expect(onSortChange).toHaveBeenCalledTimes(3); expect(onSortChange).toHaveBeenLastCalledWith({column: 'foo', direction: 'descending'}); }); + + it('highlight selection with modifier key', async function () { + render(); + let tableTester = testUtilFakeTimer.createTester('Table', {root: screen.getByTestId('test')}); + tableTester.setInteractionType(interactionType); + + await tableTester.toggleRowSelection({row: 2, selectionBehavior: 'replace'}); + // Called multiple times because highlight selections will change selection on focus unless you use + // a modifier key + if (interactionType === 'keyboard') { + expect(onSelectionChange).toHaveBeenCalledTimes(2); + expect(new Set(onSelectionChange.mock.calls.at(-1)[0])).toEqual(new Set(['Foo 1', 'Foo 3'])); + } else { + expect(onSelectionChange).toHaveBeenCalledTimes(1); + expect(new Set(onSelectionChange.mock.calls.at(-1)[0])).toEqual(new Set(['Foo 3'])); + } + + await tableTester.toggleRowSelection({row: 'Foo 4', selectionBehavior: 'replace'}); + if (interactionType === 'keyboard') { + expect(onSelectionChange).toHaveBeenCalledTimes(3); + expect(new Set(onSelectionChange.mock.calls.at(-1)[0])).toEqual(new Set(['Foo 1', 'Foo 3', 'Foo 4'])); + } else { + expect(onSelectionChange).toHaveBeenCalledTimes(2); + expect(new Set(onSelectionChange.mock.calls.at(-1)[0])).toEqual(new Set(['Foo 3', 'Foo 4'])); + } + }); }); describe('long press, fake timers', () => { diff --git a/packages/react-aria-components/test/Table.test.js b/packages/react-aria-components/test/Table.test.js index 3d72ef98283..d70f085ad4c 100644 --- a/packages/react-aria-components/test/Table.test.js +++ b/packages/react-aria-components/test/Table.test.js @@ -206,6 +206,7 @@ let renderTable = (props) => render(); describe('Table', () => { let user; let testUtilUser = new User(); + let onSelectionChange = jest.fn(); beforeAll(() => { user = userEvent.setup({delay: null, pointerMap}); }); @@ -2146,4 +2147,229 @@ describe('Table', () => { expect(rows[4]).toHaveTextContent('RatSat'); }); }); + + describe('selectionBehavior="replace"', () => { + // Required for proper touch detection + installPointerEvent(); + + describe.each(['mouse', 'keyboard', 'touch'])('%s', (type) => { + it('should perform selection with single selection', async () => { + let {getByRole} = renderTable({ + tableProps: { + selectionMode: 'single', + selectionBehavior: 'replace', + onSelectionChange + } + }); + let tableTester = testUtilUser.createTester('Table', {user, root: getByRole('grid'), interactionType: type}); + let rows = tableTester.rows; + + for (let row of tableTester.rows) { + let checkbox = within(row).queryByRole('checkbox'); + expect(checkbox).toBeNull(); + expect(row).toHaveAttribute('aria-selected', 'false'); + expect(row).not.toHaveAttribute('data-selected'); + expect(row).toHaveAttribute('data-selection-mode', 'single'); + } + + let row2 = rows[2]; + expect(onSelectionChange).toHaveBeenCalledTimes(0); + await tableTester.toggleRowSelection({row: row2, selectionBehavior: 'replace'}); + expect(row2).toHaveAttribute('aria-selected', 'true'); + expect(row2).toHaveAttribute('data-selected', 'true'); + if (type === 'keyboard') { + // Called twice because initial focus will select the first keyboard focused row + expect(onSelectionChange).toHaveBeenCalledTimes(2); + } else { + expect(onSelectionChange).toHaveBeenCalledTimes(1); + } + expect(new Set(onSelectionChange.mock.calls.at(-1)[0])).toEqual(new Set(['3'])); + expect(tableTester.selectedRows).toHaveLength(1); + expect(tableTester.selectedRows[0]).toBe(row2); + + let row1 = rows[1]; + await tableTester.toggleRowSelection({row: row1, selectionBehavior: 'replace'}); + expect(row1).toHaveAttribute('aria-selected', 'true'); + expect(row1).toHaveAttribute('data-selected', 'true'); + expect(row2).toHaveAttribute('aria-selected', 'false'); + expect(row2).not.toHaveAttribute('data-selected'); + if (type === 'keyboard') { + expect(onSelectionChange).toHaveBeenCalledTimes(3); + } else { + expect(onSelectionChange).toHaveBeenCalledTimes(2); + } + expect(new Set(onSelectionChange.mock.calls.at(-1)[0])).toEqual(new Set(['2'])); + expect(tableTester.selectedRows).toHaveLength(1); + expect(tableTester.selectedRows[0]).toBe(row1); + + await tableTester.toggleRowSelection({row: row1, selectionBehavior: 'replace'}); + expect(row1).toHaveAttribute('aria-selected', 'false'); + expect(row1).not.toHaveAttribute('data-selected'); + expect(row2).toHaveAttribute('aria-selected', 'false'); + expect(row2).not.toHaveAttribute('data-selected'); + if (type === 'keyboard') { + expect(onSelectionChange).toHaveBeenCalledTimes(4); + } else { + expect(onSelectionChange).toHaveBeenCalledTimes(3); + } + expect(new Set(onSelectionChange.mock.calls.at(-1)[0])).toEqual(new Set([])); + expect(tableTester.selectedRows).toHaveLength(0); + }); + + it('should perform toggle selection in highlight mode when using modifier keys', async () => { + let {getByRole} = renderTable({ + tableProps: { + selectionMode: 'multiple', + selectionBehavior: 'replace', + onSelectionChange + } + }); + let tableTester = testUtilUser.createTester('Table', {user, root: getByRole('grid'), interactionType: type}); + let rows = tableTester.rows; + + for (let row of tableTester.rows) { + let checkbox = within(row).queryByRole('checkbox'); + expect(checkbox).toBeNull(); + expect(row).toHaveAttribute('aria-selected', 'false'); + expect(row).not.toHaveAttribute('data-selected'); + expect(row).toHaveAttribute('data-selection-mode', 'multiple'); + } + + let row2 = rows[2]; + await tableTester.toggleRowSelection({row: row2, selectionBehavior: 'replace'}); + expect(row2).toHaveAttribute('aria-selected', 'true'); + expect(row2).toHaveAttribute('data-selected', 'true'); + if (type === 'keyboard') { + // Called twice because initial focus will select the first keyboard focused row, meaning we have two items selected + expect(onSelectionChange).toHaveBeenCalledTimes(2); + expect(new Set(onSelectionChange.mock.calls.at(-1)[0])).toEqual(new Set(['1', '3'])); + expect(tableTester.selectedRows).toHaveLength(2); + expect(tableTester.selectedRows[1]).toBe(row2); + } else { + expect(onSelectionChange).toHaveBeenCalledTimes(1); + expect(new Set(onSelectionChange.mock.calls.at(-1)[0])).toEqual(new Set(['3'])); + expect(tableTester.selectedRows).toHaveLength(1); + expect(tableTester.selectedRows[0]).toBe(row2); + } + + let row1 = rows[1]; + await tableTester.toggleRowSelection({row: row1, selectionBehavior: 'replace'}); + expect(row1).toHaveAttribute('aria-selected', 'true'); + expect(row1).toHaveAttribute('data-selected', 'true'); + expect(row2).toHaveAttribute('aria-selected', 'true'); + expect(row2).toHaveAttribute('data-selected', 'true'); + if (type === 'keyboard') { + expect(onSelectionChange).toHaveBeenCalledTimes(3); + expect(new Set(onSelectionChange.mock.calls.at(-1)[0])).toEqual(new Set(['1', '2', '3'])); + expect(tableTester.selectedRows).toHaveLength(3); + expect(tableTester.selectedRows[1]).toBe(row1); + expect(tableTester.selectedRows[2]).toBe(row2); + } else { + expect(onSelectionChange).toHaveBeenCalledTimes(2); + expect(new Set(onSelectionChange.mock.calls.at(-1)[0])).toEqual(new Set(['2', '3'])); + expect(tableTester.selectedRows).toHaveLength(2); + expect(tableTester.selectedRows[0]).toBe(row1); + expect(tableTester.selectedRows[1]).toBe(row2); + } + + // With modifier key, you should be able to deselect on press of the same row + await tableTester.toggleRowSelection({row: row1, selectionBehavior: 'replace'}); + expect(row1).toHaveAttribute('aria-selected', 'false'); + expect(row1).not.toHaveAttribute('data-selected'); + expect(row2).toHaveAttribute('aria-selected', 'true'); + expect(row2).toHaveAttribute('data-selected', 'true'); + if (type === 'keyboard') { + expect(onSelectionChange).toHaveBeenCalledTimes(4); + expect(new Set(onSelectionChange.mock.calls.at(-1)[0])).toEqual(new Set(['1', '3'])); + expect(tableTester.selectedRows).toHaveLength(2); + expect(tableTester.selectedRows[1]).toBe(row2); + } else { + expect(onSelectionChange).toHaveBeenCalledTimes(3); + expect(new Set(onSelectionChange.mock.calls.at(-1)[0])).toEqual(new Set(['3'])); + expect(tableTester.selectedRows).toHaveLength(1); + expect(tableTester.selectedRows[0]).toBe(row2); + } + }); + + it('should perform replace selection in highlight mode when not using modifier keys', async () => { + let {getByRole} = renderTable({ + tableProps: { + selectionMode: 'multiple', + selectionBehavior: 'replace', + onSelectionChange + } + }); + let tableTester = testUtilUser.createTester('Table', {user, root: getByRole('grid'), interactionType: type}); + let rows = tableTester.rows; + + for (let row of tableTester.rows) { + let checkbox = within(row).queryByRole('checkbox'); + expect(checkbox).toBeNull(); + expect(row).toHaveAttribute('aria-selected', 'false'); + expect(row).not.toHaveAttribute('data-selected'); + expect(row).toHaveAttribute('data-selection-mode', 'multiple'); + } + + let row2 = rows[2]; + await tableTester.toggleRowSelection({row: row2}); + expect(row2).toHaveAttribute('aria-selected', 'true'); + expect(row2).toHaveAttribute('data-selected', 'true'); + if (type === 'keyboard') { + // Called multiple times since selection changes on option focus as we arrow down to the target option + expect(onSelectionChange).toHaveBeenCalledTimes(3); + } else { + expect(onSelectionChange).toHaveBeenCalledTimes(1); + } + expect(new Set(onSelectionChange.mock.calls.at(-1)[0])).toEqual(new Set(['3'])); + expect(tableTester.selectedRows).toHaveLength(1); + expect(tableTester.selectedRows[0]).toBe(row2); + + let row1 = rows[1]; + await tableTester.toggleRowSelection({row: row1}); + if (type !== 'touch') { + expect(row1).toHaveAttribute('aria-selected', 'true'); + expect(row1).toHaveAttribute('data-selected', 'true'); + expect(row2).toHaveAttribute('aria-selected', 'false'); + expect(row2).not.toHaveAttribute('data-selected'); + if (type === 'keyboard') { + expect(onSelectionChange).toHaveBeenCalledTimes(4); + } else { + expect(onSelectionChange).toHaveBeenCalledTimes(2); + } + expect(new Set(onSelectionChange.mock.calls.at(-1)[0])).toEqual(new Set(['2'])); + expect(tableTester.selectedRows).toHaveLength(1); + expect(tableTester.selectedRows[0]).toBe(row1); + + // pressing without modifier keys won't deselect the row + await tableTester.toggleRowSelection({row: row1}); + expect(row1).toHaveAttribute('aria-selected', 'true'); + expect(row1).toHaveAttribute('data-selected', 'true'); + if (type === 'keyboard') { + expect(onSelectionChange).toHaveBeenCalledTimes(4); + } else { + expect(onSelectionChange).toHaveBeenCalledTimes(2); + } + expect(tableTester.selectedRows).toHaveLength(1); + } else { + // touch always behaves as toggle + expect(row1).toHaveAttribute('aria-selected', 'true'); + expect(row1).toHaveAttribute('data-selected', 'true'); + expect(row2).toHaveAttribute('aria-selected', 'true'); + expect(row2).toHaveAttribute('data-selected', 'true'); + expect(onSelectionChange).toHaveBeenCalledTimes(2); + expect(new Set(onSelectionChange.mock.calls[1][0])).toEqual(new Set(['2', '3'])); + expect(tableTester.selectedRows).toHaveLength(2); + expect(tableTester.selectedRows[0]).toBe(row1); + + await tableTester.toggleRowSelection({row: row1}); + expect(row1).toHaveAttribute('aria-selected', 'false'); + expect(row1).not.toHaveAttribute('data-selected'); + expect(onSelectionChange).toHaveBeenCalledTimes(3); + expect(new Set(onSelectionChange.mock.calls[2][0])).toEqual(new Set(['3'])); + expect(tableTester.selectedRows).toHaveLength(1); + expect(tableTester.selectedRows[0]).toBe(row2); + } + }); + }); + }); });