feat: Add CheckBoxGroup/Dialog/RadioGroup test utils #9039

LFDanLu · 2025-10-15T22:57:17Z

Closes

✅ 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:

If you wanna test the S2 RadioGroup fix, simply go to the S2 storybook and set it to horizontal + rtl. ArrowLeft/Right should move focus to the left and right as expected.

🧢 Your Project:

RSP

rspbot · 2025-10-16T17:11:30Z

Build successful! 🎉

rspbot · 2025-10-16T18:52:54Z

Build successful! 🎉

…ed tabs

rspbot · 2025-10-16T21:30:58Z

Build successful! 🎉

snowystinger · 2025-10-16T21:31:18Z

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

+    }
+
+    if (!this.checkboxgroup.contains(document.activeElement)) {
+      act(() => checkboxes[0].focus());


Should we do this by default? It can sometimes cause behaviour that doesn't match real life. Maybe there should be an option to force focus, but otherwise we throw if they aren't in the checkbox group already?

From feedback for other users and my own experience from converting tests in Quarry to use the test utils, there is often a lot of friction when people are integrating the utils into the flow of their existing test and expect that calling these "toggle/trigger" functions should just work, even if active focus is in a separate place in their app and the real life use case would be to press Tab 4x to get to the checkboxgroup/etc. I think ideally everyones tests would simulate the exact user flows but realistically I'd like there to be as little friction as possible in adoption.

snowystinger · 2025-10-16T21:33:30Z

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

+      if (document.activeElement !== this._checkboxgroup && !this._checkboxgroup.contains(document.activeElement)) {
+        act(() => this._checkboxgroup.focus());
+      }
+
+      await this.keyboardNavigateToCheckbox({checkbox});


won't keyboardNavigateToCheckbox already handle this forced focus?

oh true, this came from a copy pasta where this was necessary to simulate the collection focus -> inner collection element (e.g. row/cell) focus flow that actually happens in browser for other components

snowystinger · 2025-10-16T21:34:36Z

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

+    this._overlayType = overlayType || 'modal';
+
+    // Handle case where element provided is a wrapper of the trigger button
+    let trigger = within(root).queryByRole('button');


&& aria-expanded is an attribute on the button?

for buttons that open a modal type dialog, they don't get aria-expanded necessarly: https://www.w3.org/WAI/ARIA/apg/patterns/dialog-modal/examples/dialog/

snowystinger · 2025-10-16T21:37:04Z

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

+  async close(): Promise<void> {
+    let dialog = this._dialog;
+    if (dialog) {
+      await this.user.keyboard('[Escape]');


only for isKeyboardDismissable?

maybe close could take an argument to a button to click for that modality?

also, there's the hidden dismiss buttons for popovers if modality is virtual

I opted to start with simplicity first, especially since dialog content is so custom that the user is probably already querying for buttons that can close the dialog and thus can simply trigger a press themselves. If they aren't querying for a specific close button already, then they probably aren't testing that close uses a specific interaction mode and just want to close the dialog via a standard method. As for virtual modality, it depends on the implementation and I don't think if attempting to simulate a screenreader action is really that accurate with the tooling we have right now

My current philosophy for the test utils is that they should be used alongside your standard user events and element queries rather than be a wholesale replacement. Therefore, if the util doesn't cut down on the number of lines you write or help query for elements in a way that it also is asserting for roles/state then it doesn't need to handle it.

snowystinger · 2025-10-16T21:39:20Z

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

+    if (!trigger.hasAttribute('disabled')) {
+      if (interactionType === 'mouse' || interactionType === 'touch') {
+        if (interactionType === 'mouse') {
+          await this.user.click(trigger);
+        } else {
+          await this.user.pointer({target: trigger, keys: '[TouchA]'});
+        }
+      } else if (interactionType === 'keyboard') {
+        act(() => trigger.focus());
+        await this.user.keyboard('[Enter]');
+      }


Suggested change

if (!trigger.hasAttribute('disabled')) {

if (interactionType === 'mouse' || interactionType === 'touch') {

if (interactionType === 'mouse') {

await this.user.click(trigger);

} else {

await this.user.pointer({target: trigger, keys: '[TouchA]'});

}

} else if (interactionType === 'keyboard') {

act(() => trigger.focus());

await this.user.keyboard('[Enter]');

}

if (!trigger.hasAttribute('disabled')) {

if (interactionType === 'mouse') {

await this.user.click(trigger);

} else if (interactionType === 'touch') {

await this.user.pointer({target: trigger, keys: '[TouchA]'});

} else if (interactionType === 'keyboard') {

act(() => trigger.focus());

await this.user.keyboard('[Enter]');

}

similar issue with trigger.focus that i brought up above

snowystinger · 2025-10-16T21:42:32Z

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

+        if (dialog && document.activeElement !== this._trigger && dialog.contains(document.activeElement)) {
+          this._dialog = dialog;
+        } else {
+          // TODO: is it too brittle to throw here?


seems fine? were you running into unexpected cases?

mainly still unsure if timing might throw off this check, but seems ok so far

snowystinger · 2025-10-16T21:44:23Z

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

+        act(() => this._radiogroup.focus());
+      }
+
+      let radioOrientation = this._radiogroup.getAttribute('aria-orientation') || 'horizontal';


seems like keyboardNavigateToRadio should determine the orientation, no need to pass it in?

ya I might refactor this when I refactor some common flows out of the utils into generalized patterns (aka keyboardNavigateToElement might become a common util instead)

snowystinger · 2025-10-16T21:44:54Z

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

+    }
+
+    if (interactionType === 'keyboard') {
+      if (document.activeElement !== this._radiogroup && !this._radiogroup.contains(document.activeElement)) {


I think this is handled in keyboardNavigateToRadio already

snowystinger · 2025-10-16T21:47:28Z

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

+      let dialogTester = testUtilUser.createTester('Dialog', {root: dialogTrigger, interactionType: 'keyboard', overlayType: 'modal'});
+      await dialogTester.open();
+      let dialog = dialogTester.dialog;
+      // TODO: also weird that it is dialog.dialog?


whoops, this was when I had named the tester dialog rather than dialogTester, forgot to remove

snowystinger · 2025-10-16T21:47:56Z

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

-
-      let dialog = getByRole('dialog');
+      let dialogTrigger = document.activeElement! as HTMLElement;
+      // TODO: this is a weird case where the popover isn't actually linked to the button, behaving more like a modal


good point, should this be attached?

yeah, should it be a popover? Then again https://www.w3.org/WAI/ARIA/apg/patterns/dialog-modal/examples/datepicker-dialog/ is similar so maybe not a problem

rspbot · 2025-10-17T18:44:11Z

Build successful! 🎉

LFDanLu added 5 commits October 14, 2025 15:49

initial progress for dialog test util

54e54a2

test the dialog util

f1058b2

initial radiogroup tester

72da10f

fix case with disabled radios

5955a51

fix missing orientation in S2 Radiogroup

b87cdac

add checkbox group test util

e556c9c

fix tabs test util so that it properly keyboard navigates over disabl…

5cf5388

…ed tabs

LFDanLu changed the title ~~feat: WIP Add additional patterns to test utils~~ feat: Add CheckBoxGroup/Dialog/RadioGroup test utils Oct 16, 2025

LFDanLu marked this pull request as ready for review October 16, 2025 21:24

snowystinger reviewed Oct 16, 2025

View reviewed changes

review comments

e828412

LFDanLu added the ready for review label Oct 17, 2025

feat: Add CheckBoxGroup/Dialog/RadioGroup test utils #9039

Are you sure you want to change the base?

feat: Add CheckBoxGroup/Dialog/RadioGroup test utils #9039

Conversation

LFDanLu commented Oct 15, 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 Oct 16, 2025

Uh oh!

rspbot commented Oct 16, 2025

Uh oh!

rspbot commented Oct 16, 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!

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!

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!

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!

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 Oct 17, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

LFDanLu commented Oct 15, 2025 •

edited

Loading