feat: support selectionMode="replace" in grid collection test utils #8028

LFDanLu · 2025-04-02T22:31:03Z

Closes #7884

✅ Pull Request Checklist:

Included link to corresponding React Spectrum GitHub Issue.
Added/updated unit tests and storybook for this change (for new code or code which already has tests).
Filled out test instructions.
Updated documentation (if it already exists for this component).
Looked at the Accessibility Practices for this feature - Aria Practices

📝 Test Instructions:

🧢 Your Project:

…_bug_fixes

now we shouldnt need to call runAllTimers after selectOption

… into highlight_selection_test_util_support

if a checkbox wasnt present we werent using the keyboard navigate logic flow

…gation simulation to util

rspbot · 2025-04-03T21:48:57Z

Build successful! 🎉

LFDanLu · 2025-04-03T22:47:20Z

packages/@react-aria/test-utils/src/gridlist.ts

+      // TODO: perhaps this should just be shouldUseModifierKeys?
+      selectionBehavior = 'toggle'


This API naming feels unclear TBH, people may be confused as to when they would apply it. When set to selectionBehavior="replace" this causes the alt/meta key to be used when keyboard navigating/pressing to select a row so that it doesn't cause your previous selection to be replaced on focus/click. I wasn't certain if something like shouldUseModifierKeys was clear either since a user may not know what the "modifier keys" even do

I think selectionBehavior is fine or gridListSelectionBehavior if you want to be clear it should match, then the user profiles/interactions should handle most things behind the scenes

…_selection_test_util_support

rspbot · 2025-04-14T18:14:43Z

Build successful! 🎉

packages/@react-aria/test-utils/package.json

packages/@react-aria/test-utils/src/events.ts

snowystinger · 2025-04-15T02:36:40Z

packages/@react-aria/test-utils/src/gridlist.ts

+      // TODO: perhaps this should just be shouldUseModifierKeys?
+      selectionBehavior = 'toggle'


I think selectionBehavior is fine or gridListSelectionBehavior if you want to be clear it should match, then the user profiles/interactions should handle most things behind the scenes

…_selection_test_util_support

rspbot · 2025-05-19T20:23:07Z

Build successful! 🎉

packages/react-aria-components/test/GridList.test.js

packages/@react-spectrum/table/test/TestTableUtils.test.tsx

packages/@react-aria/test-utils/src/tree.ts

snowystinger · 2025-05-19T20:35:10Z

packages/@react-aria/test-utils/src/tree.ts

@@ -140,7 +159,14 @@ 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


i don't quite follow this todo

this was more of a musing around perhaps consolidating all of these "trigger a press while holding a modifier key" into pressElement itself if it turns out that we have a lot of these kinds of interactions in our components

packages/@react-aria/test-utils/src/tree.ts

snowystinger · 2025-05-19T20:36:28Z

packages/@react-aria/test-utils/src/tree.ts

  };

  /**
   * 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.


maybe an option shouldExpandCurrentSelection?: boolean

for now I want to avoid that because having shouldExpandCurrentSelection=false in a multiselect tree that has selectionBehavior='toggle' would mean the user would expect all rows to be unselected (via click/keyboard/etc) and then their target row toggled on which adds to the complexity here, especially for a virtualized case.

snowystinger · 2025-05-19T20:39:00Z

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)) {


instead of calling focus if it's not on the tablist, perhaps we should be throwing an error sayings, "navigate to the tablist"?
I know you didn't change that behaviour here, so more of a future topic
I'm just worried about the case that came up recently where calling focus directly was problematic because it didn't follow user interactions closely enough, skipped some steps, which caused the wrong state in the UI

yeah its a bit tricky, trying to balance usability (forcing the user to simulate all the interactions required to go from where ever they are in their test to the target TabList) vs realism. I think I'd lean towards the latter to avoid the issues you mentioned but will see about do that in a follow up.

packages/@react-aria/test-utils/src/tabs.ts

snowystinger · 2025-05-19T21:49:37Z

packages/@react-aria/test-utils/src/tree.ts

@@ -137,7 +136,7 @@ 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 || !rowCheckbox)) {
-      await this.keyboardNavigateToRow({row, useAltKey: selectionBehavior === 'replace'});
+      await this.keyboardNavigateToRow({row, avoidSelectionOnNav: selectionBehavior === 'replace'});


"avoid" feels a bit, "i'll try my best, but it might happen" to me, maybe one of these instead?
noSelectionOnNav
shouldNotSelectOnNav

haha, I was thinking about exactly this when naming the option. I actually did want to give that kind of impression since using noSelectionOnNav or shouldNotSelectOnNav would mean it would sound like selection SHOULD change on nav if those were false, something which wouldn't happen for a collection with selectionBehavior: 'toggle' without me firing Enter after every ArrowUp/Down.

avoidSelectionOnNav is more of the following: if true we endeavor to avoid selection on keyboard navigation via the Alt modifier, otherwise if false we won't do anything special when keyboard navigating and if something gets selected as you keyboard navigate that is expected due to the selectionBehavior of your collection

hrmmmmm..... sounds tricky to document. maybe instead of a boolean, it should be
selectionOnNav: 'default' | 'none' and it defaults to 'default'

that sounds good, will update

rspbot · 2025-05-19T21:57:02Z

Build successful! 🎉

rspbot · 2025-05-23T22:51:28Z

Build successful! 🎉

…_selection_test_util_support

…tual help is provided

LFDanLu · 2025-06-04T00:26:02Z

packages/@react-spectrum/s2/test/Combobox.test.tsx

+    // Issue with how we don't render the contextual help button in the fake DOM since PressResponder isn't using createHideableComponent
+    let warn = jest.spyOn(global.console, 'warn').mockImplementation();


Gross that this is a thing, I remember discussing this last month or so but don't think we landed on anything...

to be clear, it warns that the PressResponder doesn't have a button in the fake DOM

bleh, we'll need to do something about this, it also cropped up in #8151

rspbot · 2025-06-04T00:27:26Z

Build successful! 🎉

LFDanLu added 12 commits January 29, 2025 16:54

attempt to get rid of jest calls in menu util

a32b16e

update RSP testing docs to directly mention mocks that maybe needed

e326a35

bump versions of RTL to 16

9481673

use alternative to calling jest run timers in menu option selection

f625818

fixing types and properly testing long press

69d289f

fix lint

366c1f8

Merge branch 'main' of github.com:adobe/react-spectrum into test_util…

5a502b8

…_bug_fixes

revert to pre testing library bump for clean slate

8f55260

fix build and another submenu edge case

93cbb33

now we shouldnt need to call runAllTimers after selectOption

fix react 16 bug

5207639

update return type of advanceTimer and docs copy

14452d8

Initial support for tree highlight selection support

8bd33c4

LFDanLu changed the base branch from main to test_util_bug_fixes April 2, 2025 22:31

LFDanLu added 7 commits April 2, 2025 15:42

move some general fixes from selectionMode="replace" branch here

dd52a74

add highlight selection support to gridlist, listbox, and table

5ef9f19

add test for deselection with modifier and add gridlist tests

fc4b727

Merge branch 'test_util_bug_fixes' of github.com:adobe/react-spectrum…

40d7b2f

… into highlight_selection_test_util_support

fix build

972e5a2

add listbox test and fix logic for keyboard selection in utils

53763bd

if a checkbox wasnt present we werent using the keyboard navigate logic flow

add table util highlight selection tests and add proper keyboard navi…

96b4a85

…gation simulation to util

LFDanLu changed the title ~~feat: (WIP) enhance selectionMode="replace" support in grid collection test utils~~ feat: support selectionMode="replace" in grid collection test utils Apr 3, 2025

LFDanLu commented Apr 3, 2025

View reviewed changes

LFDanLu marked this pull request as ready for review April 8, 2025 17:05

LFDanLu added in sprint tech/dev/garage week labels Apr 8, 2025

Base automatically changed from test_util_bug_fixes to main April 8, 2025 21:31

Merge branch 'main' of github.com:adobe/react-spectrum into highlight…

94b383c

…_selection_test_util_support

snowystinger reviewed Apr 15, 2025

View reviewed changes

LFDanLu added 3 commits May 19, 2025 11:29

Merge branch 'main' of github.com:adobe/react-spectrum into highlight…

d48a7c8

…_selection_test_util_support

remove dep on react-aria/utils

e046e74

update yarn lock

2ba33fc

snowystinger reviewed May 19, 2025

View reviewed changes

review comments

5e9ddb5

snowystinger reviewed May 19, 2025

View reviewed changes

update prop name for clarity

772a5a5

LFDanLu added the release label May 30, 2025

snowystinger previously approved these changes Jun 2, 2025

View reviewed changes

LFDanLu added 2 commits June 3, 2025 16:57

Merge branch 'main' of github.com:adobe/react-spectrum into highlight…

f522076

…_selection_test_util_support

fix case where S2 Combobox and Picker returns wrong trigger if contex…

5c11a97

…tual help is provided

LFDanLu dismissed snowystinger’s stale review via 5c11a97 June 4, 2025 00:17

LFDanLu commented Jun 4, 2025

View reviewed changes

snowystinger approved these changes Jun 4, 2025

View reviewed changes

yihuiliao approved these changes Jun 4, 2025

View reviewed changes

LFDanLu added this pull request to the merge queue Jun 4, 2025

Merged via the queue into main with commit 21d1950 Jun 4, 2025
30 checks passed

LFDanLu deleted the highlight_selection_test_util_support branch June 4, 2025 18:45

		// TODO: perhaps this should just be shouldUseModifierKeys?
		selectionBehavior = 'toggle'

		// Issue with how we don't render the contextual help button in the fake DOM since PressResponder isn't using createHideableComponent
		let warn = jest.spyOn(global.console, 'warn').mockImplementation();

feat: support selectionMode="replace" in grid collection test utils #8028

feat: support selectionMode="replace" in grid collection test utils #8028

Uh oh!

Conversation

LFDanLu commented Apr 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Pull Request Checklist:

📝 Test Instructions:

🧢 Your Project:

Uh oh!

rspbot commented Apr 3, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rspbot commented Apr 14, 2025

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rspbot commented May 19, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rspbot commented May 19, 2025

Uh oh!

rspbot commented May 23, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rspbot commented Jun 4, 2025

Uh oh!

Uh oh!

Uh oh!

LFDanLu commented Apr 2, 2025 •

edited

Loading