From 54e54a2d1c6100b122ebfc45079b3a90dcc9bec6 Mon Sep 17 00:00:00 2001 From: Daniel Lu Date: Tue, 14 Oct 2025 15:49:08 -0700 Subject: [PATCH 01/10] initial progress for dialog test util --- packages/@react-aria/test-utils/src/dialog.ts | 150 ++++++++++++++++++ packages/@react-aria/test-utils/src/types.ts | 15 ++ packages/@react-aria/test-utils/src/user.ts | 10 +- .../s2/test/EditableTableView.test.tsx | 25 +-- 4 files changed, 188 insertions(+), 12 deletions(-) create mode 100644 packages/@react-aria/test-utils/src/dialog.ts diff --git a/packages/@react-aria/test-utils/src/dialog.ts b/packages/@react-aria/test-utils/src/dialog.ts new file mode 100644 index 00000000000..008e6a55eb8 --- /dev/null +++ b/packages/@react-aria/test-utils/src/dialog.ts @@ -0,0 +1,150 @@ +/* + * Copyright 2025 Adobe. All rights reserved. + * This file is licensed to you under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. You may obtain a copy + * of the License at http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under + * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS + * OF ANY KIND, either express or implied. See the License for the specific language + * governing permissions and limitations under the License. + */ + +import {act, waitFor, within} from '@testing-library/react'; +import {DialogTesterOpts, UserOpts} from './types'; + +interface DialogOpenOpts { + /** + * What interaction type to use when opening the dialog. Defaults to the interaction type set on the tester. + */ + interactionType?: UserOpts['interactionType'] +} + +export class DialogTester { + private user; + private _interactionType: UserOpts['interactionType']; + private _trigger: HTMLElement | undefined; + private _dialog: HTMLElement | undefined; + // TODO: may not need this? Isn't really useful + private _dialogType: DialogTesterOpts['dialogType']; + private _overlayType: DialogTesterOpts['overlayType']; + + constructor(opts: DialogTesterOpts) { + let {root, user, interactionType, dialogType, overlayType} = opts; + this.user = user; + this._interactionType = interactionType || 'mouse'; + this._dialogType = dialogType || 'dialog'; + this._overlayType = overlayType || 'modal'; + + // Handle case where element provided is a wrapper of the trigger button + let trigger = within(root).queryByRole('button'); + if (trigger) { + this._trigger = trigger; + } else { + this._trigger = root; + } + } + + /** + * Set the interaction type used by the dialog tester. + */ + setInteractionType(type: UserOpts['interactionType']): void { + this._interactionType = type; + } + + /** + * Opens the dialog. Defaults to using the interaction type set on the dialog tester. + */ + async open(opts: DialogOpenOpts = {}): Promise { + let { + interactionType = this._interactionType + } = opts; + let trigger = this.trigger; + let isDisabled = 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' && !isDisabled) { + act(() => trigger.focus()); + await this.user.keyboard('[Enter]'); + } + + if (this._overlayType === 'popover') { + await waitFor(() => { + if (trigger.getAttribute('aria-controls') == null && !isDisabled) { + throw new Error('No aria-controls found on dialog trigger element.'); + } else { + return true; + } + }); + + if (!isDisabled) { + let dialogId = trigger.getAttribute('aria-controls'); + await waitFor(() => { + if (!dialogId || document.getElementById(dialogId) == null) { + throw new Error(`Dialog with id of ${dialogId} not found in document.`); + } else { + this._dialog = document.getElementById(dialogId)!; + return true; + } + }); + } + } else { + let dialog; + await waitFor(() => { + dialog = document.querySelector(`[role=${this._dialogType}]`); + if (dialog == null && !isDisabled) { + throw new Error(`No dialog of type ${this._dialogType} found after pressing the trigger.`); + } else { + return true; + } + }); + + if (dialog && document.activeElement !== this._trigger && dialog.contains(document.activeElement)) { + this._dialog = dialog; + } else { + // TODO: is it too brittle to throw here? + throw new Error('New modal dialog doesnt contain the active element OR the active element is still the trigger. Uncertain if the proper modal dialog was found'); + } + } + } + + /** + * Closes the dialog via the Escape key. + */ + async close(): Promise { + let dialog = this._dialog; + if (dialog) { + await this.user.keyboard('[Escape]'); + await waitFor(() => { + if (document.contains(dialog)) { + throw new Error('Expected the dialog to not be in the document after closing it.'); + } else { + this._dialog = undefined; + return true; + } + }); + } + } + + /** + * Returns the dialog's trigger. + */ + get trigger(): HTMLElement { + if (!this._trigger) { + throw new Error('No trigger element found for dialog.'); + } + + return this._trigger; + } + + /** + * Returns the dialog if present. + */ + get dialog(): HTMLElement | null { + return this._dialog && document.contains(this._dialog) ? this._dialog : null; + } +} diff --git a/packages/@react-aria/test-utils/src/types.ts b/packages/@react-aria/test-utils/src/types.ts index cef8e41322b..9cadbb997d9 100644 --- a/packages/@react-aria/test-utils/src/types.ts +++ b/packages/@react-aria/test-utils/src/types.ts @@ -52,6 +52,21 @@ export interface ComboBoxTesterOpts extends BaseTesterOpts { trigger?: HTMLElement } +export interface DialogTesterOpts extends BaseTesterOpts { + /** + * The trigger element for the dialog. + */ + root: HTMLElement, + /** + * The type of dialog. + */ + dialogType?: 'alertdialog' | 'dialog', + /** + * The overlay type of the dialog. Used to inform the tester how to find the dialog. + */ + overlayType?: 'modal' | 'popover' +} + export interface GridListTesterOpts extends BaseTesterOpts {} export interface ListBoxTesterOpts extends BaseTesterOpts { diff --git a/packages/@react-aria/test-utils/src/user.ts b/packages/@react-aria/test-utils/src/user.ts index ee2fb08bc01..89cfe473190 100644 --- a/packages/@react-aria/test-utils/src/user.ts +++ b/packages/@react-aria/test-utils/src/user.ts @@ -13,6 +13,7 @@ import {ComboBoxTester} from './combobox'; import { ComboBoxTesterOpts, + DialogTesterOpts, GridListTesterOpts, ListBoxTesterOpts, MenuTesterOpts, @@ -22,6 +23,7 @@ import { TreeTesterOpts, UserOpts } from './types'; +import {DialogTester} from './dialog'; import {GridListTester} from './gridlist'; import {ListBoxTester} from './listbox'; import {MenuTester} from './menu'; @@ -40,7 +42,8 @@ let keyToUtil: { 'GridList': typeof GridListTester, 'ListBox': typeof ListBoxTester, 'Tabs': typeof TabsTester, - 'Tree': typeof TreeTester + 'Tree': typeof TreeTester, + 'Dialog': typeof DialogTester } = { 'Select': SelectTester, 'Table': TableTester, @@ -49,13 +52,15 @@ let keyToUtil: { 'GridList': GridListTester, 'ListBox': ListBoxTester, 'Tabs': TabsTester, - 'Tree': TreeTester + 'Tree': TreeTester, + 'Dialog': DialogTester } as const; export type PatternNames = keyof typeof keyToUtil; // Conditional type: https://www.typescriptlang.org/docs/handbook/2/conditional-types.html type Tester = T extends 'ComboBox' ? ComboBoxTester : + T extends 'Dialog' ? DialogTester : T extends 'GridList' ? GridListTester : T extends 'ListBox' ? ListBoxTester : T extends 'Menu' ? MenuTester : @@ -67,6 +72,7 @@ type Tester = type TesterOpts = T extends 'ComboBox' ? ComboBoxTesterOpts : + T extends 'Dialog' ? DialogTesterOpts : T extends 'GridList' ? GridListTesterOpts : T extends 'ListBox' ? ListBoxTesterOpts : T extends 'Menu' ? MenuTesterOpts : diff --git a/packages/@react-spectrum/s2/test/EditableTableView.test.tsx b/packages/@react-spectrum/s2/test/EditableTableView.test.tsx index 745e36b9959..e0fe6efff54 100644 --- a/packages/@react-spectrum/s2/test/EditableTableView.test.tsx +++ b/packages/@react-spectrum/s2/test/EditableTableView.test.tsx @@ -242,7 +242,7 @@ describe('TableView', () => { } describe('keyboard', () => { - it('should edit text in a cell either through a TextField or a Picker', async () => { + it.only('should edit text in a cell either through a TextField or a Picker', async () => { let {getByRole} = render( ); @@ -250,12 +250,15 @@ describe('TableView', () => { let tableTester = testUtilUser.createTester('Table', {root: getByRole('grid')}); await user.tab(); await user.keyboard('{ArrowRight}'); - await user.keyboard('{Enter}'); - - 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 + 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? expect(dialog).toBeVisible(); - let input = within(dialog).getByRole('textbox'); + let input = within(dialog!).getByRole('textbox'); expect(input).toHaveFocus(); await user.keyboard('Apples Crisp'); @@ -271,18 +274,20 @@ describe('TableView', () => { await user.keyboard('{ArrowRight}'); await user.keyboard('{ArrowRight}'); await user.keyboard('{ArrowRight}'); - await user.keyboard('{Enter}'); - - dialog = getByRole('dialog'); + dialogTrigger = document.activeElement! as HTMLElement; + dialogTester = testUtilUser.createTester('Dialog', {root: dialogTrigger, interactionType: 'keyboard', overlayType: 'modal'}); + await dialogTester.open(); + dialog = dialogTester.dialog; + // TODO: also weird that it is dialog.dialog? expect(dialog).toBeVisible(); - let selectTester = testUtilUser.createTester('Select', {root: dialog}); + let selectTester = testUtilUser.createTester('Select', {root: dialog!}); expect(selectTester.trigger).toHaveFocus(); await selectTester.selectOption({option: 'Steven'}); act(() => {jest.runAllTimers();}); await user.tab(); await user.tab(); - expect(within(dialog).getByRole('button', {name: 'Save'})).toHaveFocus(); + expect(within(dialog!).getByRole('button', {name: 'Save'})).toHaveFocus(); await user.keyboard('{Enter}'); act(() => {jest.runAllTimers();}); From f1058b20591f30075723e258f06e3c4fb18dca03 Mon Sep 17 00:00:00 2001 From: Daniel Lu Date: Tue, 14 Oct 2025 16:38:42 -0700 Subject: [PATCH 02/10] test the dialog util --- packages/@react-aria/test-utils/src/dialog.ts | 67 +++++++++---------- .../dialog/test/DialogTrigger.test.js | 54 ++++----------- .../s2/test/EditableTableView.test.tsx | 2 +- .../react-aria-components/test/Dialog.test.js | 17 +++-- 4 files changed, 56 insertions(+), 84 deletions(-) diff --git a/packages/@react-aria/test-utils/src/dialog.ts b/packages/@react-aria/test-utils/src/dialog.ts index 008e6a55eb8..ebae5dcd275 100644 --- a/packages/@react-aria/test-utils/src/dialog.ts +++ b/packages/@react-aria/test-utils/src/dialog.ts @@ -60,28 +60,27 @@ export class DialogTester { interactionType = this._interactionType } = opts; let trigger = this.trigger; - let isDisabled = 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' && !isDisabled) { - act(() => trigger.focus()); - await this.user.keyboard('[Enter]'); - } - - if (this._overlayType === 'popover') { - await waitFor(() => { - if (trigger.getAttribute('aria-controls') == null && !isDisabled) { - throw new Error('No aria-controls found on dialog trigger element.'); + if (!trigger.hasAttribute('disabled')) { + if (interactionType === 'mouse' || interactionType === 'touch') { + if (interactionType === 'mouse') { + await this.user.click(trigger); } else { - return true; + await this.user.pointer({target: trigger, keys: '[TouchA]'}); } - }); + } else if (interactionType === 'keyboard') { + act(() => trigger.focus()); + await this.user.keyboard('[Enter]'); + } + + if (this._overlayType === 'popover') { + await waitFor(() => { + if (trigger.getAttribute('aria-controls') == null) { + throw new Error('No aria-controls found on dialog trigger element.'); + } else { + return true; + } + }); - if (!isDisabled) { let dialogId = trigger.getAttribute('aria-controls'); await waitFor(() => { if (!dialogId || document.getElementById(dialogId) == null) { @@ -91,23 +90,23 @@ export class DialogTester { return true; } }); - } - } else { - let dialog; - await waitFor(() => { - dialog = document.querySelector(`[role=${this._dialogType}]`); - if (dialog == null && !isDisabled) { - throw new Error(`No dialog of type ${this._dialogType} found after pressing the trigger.`); + } else { + let dialog; + await waitFor(() => { + dialog = document.querySelector(`[role=${this._dialogType}]`); + if (dialog == null) { + throw new Error(`No dialog of type ${this._dialogType} found after pressing the trigger.`); + } else { + return true; + } + }); + + if (dialog && document.activeElement !== this._trigger && dialog.contains(document.activeElement)) { + this._dialog = dialog; } else { - return true; + // TODO: is it too brittle to throw here? + throw new Error('New modal dialog doesnt contain the active element OR the active element is still the trigger. Uncertain if the proper modal dialog was found'); } - }); - - if (dialog && document.activeElement !== this._trigger && dialog.contains(document.activeElement)) { - this._dialog = dialog; - } else { - // TODO: is it too brittle to throw here? - throw new Error('New modal dialog doesnt contain the active element OR the active element is still the trigger. Uncertain if the proper modal dialog was found'); } } } diff --git a/packages/@react-spectrum/dialog/test/DialogTrigger.test.js b/packages/@react-spectrum/dialog/test/DialogTrigger.test.js index 52765f37a48..97d3a5a35be 100644 --- a/packages/@react-spectrum/dialog/test/DialogTrigger.test.js +++ b/packages/@react-spectrum/dialog/test/DialogTrigger.test.js @@ -10,7 +10,7 @@ * governing permissions and limitations under the License. */ -import {act, fireEvent, pointerMap, render, simulateDesktop, simulateMobile, waitFor, within} from '@react-spectrum/test-utils-internal'; +import {act, fireEvent, pointerMap, render, simulateDesktop, simulateMobile, User, waitFor, within} from '@react-spectrum/test-utils-internal'; import {ActionButton, Button} from '@react-spectrum/button'; import {ButtonGroup} from '@react-spectrum/buttongroup'; import {Content} from '@react-spectrum/view'; @@ -27,6 +27,7 @@ import userEvent from '@testing-library/user-event'; describe('DialogTrigger', function () { let warnMock; let user; + let testUtilUser = new User({advanceTimer: jest.advanceTimersByTime}); beforeAll(() => { user = userEvent.setup({delay: null, pointerMap}); @@ -71,13 +72,9 @@ describe('DialogTrigger', function () { expect(queryByRole('dialog')).toBeNull(); let button = getByRole('button'); - await user.click(button); - - act(() => { - jest.runAllTimers(); - }); - - let dialog = getByRole('dialog'); + let dialogTester = testUtilUser.createTester('Dialog', {root: button, overlayType: 'modal'}); + await dialogTester.open(); + let dialog = dialogTester.dialog; expect(dialog).toBeVisible(); let modal = getByTestId('modal'); @@ -123,13 +120,9 @@ describe('DialogTrigger', function () { expect(queryByRole('dialog')).toBeNull(); let button = getByRole('button'); - await user.click(button); - - act(() => { - jest.runAllTimers(); - }); - - let dialog = getByRole('dialog'); + let dialogTester = testUtilUser.createTester('Dialog', {root: button, overlayType: 'popover'}); + await dialogTester.open(); + let dialog = dialogTester.dialog; expect(dialog).toBeVisible(); let popover = getByTestId('popover'); @@ -277,38 +270,15 @@ describe('DialogTrigger', function () { ); let button = getByRole('button'); - act(() => {button.focus();}); - fireEvent.focusIn(button); - await user.click(button); - - act(() => { - jest.runAllTimers(); - }); - - let dialog = getByRole('dialog'); - - await waitFor(() => { - expect(dialog).toBeVisible(); - }); // wait for animation - + let dialogTester = testUtilUser.createTester('Dialog', {root: button, overlayType: 'modal'}); + await dialogTester.open(); + let dialog = dialogTester.dialog; expect(document.activeElement).toBe(dialog); - - fireEvent.keyDown(dialog, {key: 'Escape'}); - fireEvent.keyUp(dialog, {key: 'Escape'}); - - act(() => { - jest.runAllTimers(); - }); - - await waitFor(() => { - expect(dialog).not.toBeInTheDocument(); - }); // wait for animation - + await dialogTester.close(); // now that it's been unmounted, run the raf callback act(() => { jest.runAllTimers(); }); - expect(document.activeElement).toBe(button); }); diff --git a/packages/@react-spectrum/s2/test/EditableTableView.test.tsx b/packages/@react-spectrum/s2/test/EditableTableView.test.tsx index e0fe6efff54..88c7249e576 100644 --- a/packages/@react-spectrum/s2/test/EditableTableView.test.tsx +++ b/packages/@react-spectrum/s2/test/EditableTableView.test.tsx @@ -242,7 +242,7 @@ describe('TableView', () => { } describe('keyboard', () => { - it.only('should edit text in a cell either through a TextField or a Picker', async () => { + it('should edit text in a cell either through a TextField or a Picker', async () => { let {getByRole} = render( ); diff --git a/packages/react-aria-components/test/Dialog.test.js b/packages/react-aria-components/test/Dialog.test.js index 4bd6c76f1a6..8d33ba39856 100644 --- a/packages/react-aria-components/test/Dialog.test.js +++ b/packages/react-aria-components/test/Dialog.test.js @@ -29,10 +29,12 @@ import { } from '../'; import React, {useRef} from 'react'; import {UNSAFE_PortalProvider} from '@react-aria/overlays'; +import {User} from '@react-aria/test-utils'; import userEvent from '@testing-library/user-event'; describe('Dialog', () => { let user; + let testUtilUser = new User({advanceTimer: jest.advanceTimersByTime}); beforeAll(() => { user = userEvent.setup({delay: null, pointerMap}); jest.useFakeTimers(); @@ -68,9 +70,10 @@ describe('Dialog', () => { ); let button = getByRole('button'); - await user.click(button); - - let dialog = getByRole('alertdialog'); + let dialogTester = testUtilUser.createTester('Dialog', {root: button, dialogType: 'alertdialog', overlayType: 'modal'}); + await dialogTester.open(); + let dialog = dialogTester.dialog; + expect(dialog).toHaveAttribute('role', 'alertdialog'); let heading = getByRole('heading'); expect(dialog).toHaveAttribute('aria-labelledby', heading.id); expect(dialog).toHaveAttribute('data-test', 'dialog'); @@ -167,11 +170,11 @@ describe('Dialog', () => { let button = getByRole('button'); expect(button).not.toHaveAttribute('data-pressed'); - await user.click(button); - + let dialogTester = testUtilUser.createTester('Dialog', {root: button, overlayType: 'popover'}); + await dialogTester.open(); expect(button).toHaveAttribute('data-pressed'); - let dialog = getByRole('dialog'); + let dialog = dialogTester.dialog; let heading = getByRole('heading'); expect(dialog).toHaveAttribute('aria-labelledby', heading.id); expect(dialog).toHaveAttribute('data-test', 'dialog'); @@ -386,7 +389,7 @@ describe('Dialog', () => { await user.click(document.body); }); }); - + it('ensure Input autoFocus works when opening Modal from MenuItem via keyboard', async () => { function App() { const [isOpen, setOpen] = React.useState(false); From 72da10f3fd02fff0e9168552b5d6fadbe1abbb9c Mon Sep 17 00:00:00 2001 From: Daniel Lu Date: Wed, 15 Oct 2025 11:33:06 -0700 Subject: [PATCH 03/10] initial radiogroup tester --- .../@react-aria/test-utils/src/radiogroup.ts | 170 ++++++++++++++++++ packages/@react-aria/test-utils/src/types.ts | 8 + 2 files changed, 178 insertions(+) create mode 100644 packages/@react-aria/test-utils/src/radiogroup.ts diff --git a/packages/@react-aria/test-utils/src/radiogroup.ts b/packages/@react-aria/test-utils/src/radiogroup.ts new file mode 100644 index 00000000000..157e25d2882 --- /dev/null +++ b/packages/@react-aria/test-utils/src/radiogroup.ts @@ -0,0 +1,170 @@ +/* + * Copyright 2025 Adobe. All rights reserved. + * This file is licensed to you under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. You may obtain a copy + * of the License at http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under + * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS + * OF ANY KIND, either express or implied. See the License for the specific language + * governing permissions and limitations under the License. + */ + +import {act, within} from '@testing-library/react'; +import {Direction, Orientation, RadioGroupTesterOpts, UserOpts} from './types'; +import {pressElement} from './events'; + +interface TriggerRadioOptions { + /** + * What interaction type to use when triggering a radio. Defaults to the interaction type set on the tester. + */ + interactionType?: UserOpts['interactionType'], + /** + * The index, text, or node of the radio to toggle selection for. + */ + radio: number | string | HTMLElement +} + +export class RadioGroupTester { + private user; + private _interactionType: UserOpts['interactionType']; + private _radiogroup: HTMLElement; + private _direction: Direction; + + constructor(opts: RadioGroupTesterOpts) { + let {root, user, interactionType, direction} = opts; + this.user = user; + this._interactionType = interactionType || 'mouse'; + this._direction = direction || 'ltr'; + + this._radiogroup = root; + let radiogroup = within(root).queryAllByRole('radiogroup'); + if (radiogroup.length > 0) { + this._radiogroup = radiogroup[0]; + } + } + + /** + * Set the interaction type used by the radio tester. + */ + setInteractionType(type: UserOpts['interactionType']): void { + this._interactionType = type; + } + + /** + * Returns a radio matching the specified index or text content. + */ + findRadio(opts: {radioIndexOrText: number | string}): HTMLElement { + let { + radioIndexOrText + } = opts; + + let radio; + if (typeof radioIndexOrText === 'number') { + radio = this.radios[radioIndexOrText]; + } else if (typeof radioIndexOrText === 'string') { + let label = within(this.radiogroup).getByText(radioIndexOrText); + if (label) { + radio = within(label).queryByRole('radio'); + if (!radio) { + radio = label.closest('[role=radio]'); + } + } + } + + return radio; + } + + // TODO: test all cases here (aka RTL/LTR/horizontal/vertical/index based keyboard movement) + private async keyboardNavigateToRadio(opts: {radio: HTMLElement, orientation?: Orientation}) { + let {radio, orientation = 'vertical'} = opts; + let radios = this.radios; + let targetIndex = radios.indexOf(radio); + if (targetIndex === -1) { + throw new Error('Radio provided is not in the radiogroup'); + } + + if (!this.radiogroup.contains(document.activeElement)) { + let selectedRadio = this.selectedRadio; + if (selectedRadio != null) { + act(() => selectedRadio.focus()); + } else { + act(() => radios.find(radio => !(radio.hasAttribute('disabled') || radio.getAttribute('aria-disabled') === 'true'))?.focus()); + } + } + + let currIndex = this.radios.indexOf(document.activeElement as HTMLElement); + if (currIndex === -1) { + throw new Error('ActiveElement is not in the radiogroup'); + } + + let arrowUp = 'ArrowUp'; + let arrowDown = 'ArrowDown'; + if (orientation === 'horizontal') { + if (this._direction === 'ltr') { + arrowUp = 'ArrowLeft'; + arrowDown = 'ArrowRight'; + } else { + arrowUp = 'ArrowRight'; + arrowDown = 'ArrowLeft'; + } + } + + let movementDirection = targetIndex > currIndex ? 'down' : 'up'; + for (let i = 0; i < Math.abs(targetIndex - currIndex); i++) { + await this.user.keyboard(`[${movementDirection === 'down' ? arrowDown : arrowUp}]`); + } + }; + + /** + * Triggers the specified radio. Defaults to using the interaction type set on the radio tester. + */ + async triggerRadio(opts: TriggerRadioOptions): Promise { + let { + radio, + interactionType = this._interactionType + } = opts; + + if (typeof radio === 'string' || typeof radio === 'number') { + radio = this.findRadio({radioIndexOrText: radio}); + } + + if (!radio) { + throw new Error('Target radio not found in the radiogroup.'); + } else if (radio.hasAttribute('disabled')) { + throw new Error('Target radio is disabled.'); + } + + if (interactionType === 'keyboard') { + if (document.activeElement !== this._radiogroup && !this._radiogroup.contains(document.activeElement)) { + act(() => this._radiogroup.focus()); + } + + let radioOrientation = this._radiogroup.getAttribute('aria-orientation') || 'horizontal'; + await this.keyboardNavigateToRadio({radio, orientation: radioOrientation as Orientation}); + } else { + await pressElement(this.user, radio, interactionType); + } + } + + /** + * Returns the radiogroup. + */ + get radiogroup(): HTMLElement { + return this._radiogroup; + } + + /** + * Returns the radios. + */ + get radios(): HTMLElement[] { + return within(this.radiogroup).queryAllByRole('radio'); + } + + /** + * Returns the currently selected radio in the radiogroup if any. + */ + get selectedRadio(): HTMLElement | null { + return this.radios.find(radio => (radio as HTMLInputElement).checked) || null; + } +} diff --git a/packages/@react-aria/test-utils/src/types.ts b/packages/@react-aria/test-utils/src/types.ts index 9cadbb997d9..a24bb86c8df 100644 --- a/packages/@react-aria/test-utils/src/types.ts +++ b/packages/@react-aria/test-utils/src/types.ts @@ -91,6 +91,14 @@ export interface MenuTesterOpts extends BaseTesterOpts { rootMenu?: HTMLElement } +export interface RadioGroupTesterOpts extends BaseTesterOpts { + /** + * The horizontal layout direction, typically affected by locale. + * @default 'ltr' + */ + direction?: Direction +} + export interface SelectTesterOpts extends BaseTesterOpts { /** * The trigger element for the select. If provided the wrapping element around the target select (as is the case with a ref provided to RSP Select), From 5955a51a14cf26a6434bb971651e6e0750e6c679 Mon Sep 17 00:00:00 2001 From: Daniel Lu Date: Wed, 15 Oct 2025 15:56:30 -0700 Subject: [PATCH 04/10] fix case with disabled radios --- .../@react-aria/test-utils/src/radiogroup.ts | 18 +++++-- packages/@react-aria/test-utils/src/user.ts | 26 ++++++---- .../@react-spectrum/radio/test/Radio.test.js | 49 +++++++++++++++++-- .../test/RadioGroup.test.js | 33 ++++++++----- 4 files changed, 98 insertions(+), 28 deletions(-) diff --git a/packages/@react-aria/test-utils/src/radiogroup.ts b/packages/@react-aria/test-utils/src/radiogroup.ts index 157e25d2882..80bfb910517 100644 --- a/packages/@react-aria/test-utils/src/radiogroup.ts +++ b/packages/@react-aria/test-utils/src/radiogroup.ts @@ -64,10 +64,16 @@ export class RadioGroupTester { radio = this.radios[radioIndexOrText]; } else if (typeof radioIndexOrText === 'string') { let label = within(this.radiogroup).getByText(radioIndexOrText); + // Label may wrap the radio, or the actual label may be a sibling span, or the radio div could have the label within it if (label) { radio = within(label).queryByRole('radio'); if (!radio) { - radio = label.closest('[role=radio]'); + let labelWrapper = label.closest('label'); + if (labelWrapper) { + radio = within(labelWrapper).queryByRole('radio'); + } else { + radio = label.closest('[role=radio]'); + } } } } @@ -75,10 +81,14 @@ export class RadioGroupTester { return radio; } - // TODO: test all cases here (aka RTL/LTR/horizontal/vertical/index based keyboard movement) private async keyboardNavigateToRadio(opts: {radio: HTMLElement, orientation?: Orientation}) { let {radio, orientation = 'vertical'} = opts; let radios = this.radios; + radios = radios.filter(radio => !(radio.hasAttribute('disabled') || radio.getAttribute('aria-disabled') === 'true')); + if (radios.length === 0) { + throw new Error('Radio group doesnt have any activatable radios. Please double check your radio group.'); + } + let targetIndex = radios.indexOf(radio); if (targetIndex === -1) { throw new Error('Radio provided is not in the radiogroup'); @@ -89,11 +99,11 @@ export class RadioGroupTester { if (selectedRadio != null) { act(() => selectedRadio.focus()); } else { - act(() => radios.find(radio => !(radio.hasAttribute('disabled') || radio.getAttribute('aria-disabled') === 'true'))?.focus()); + act(() => radios[0]?.focus()); } } - let currIndex = this.radios.indexOf(document.activeElement as HTMLElement); + let currIndex = radios.indexOf(document.activeElement as HTMLElement); if (currIndex === -1) { throw new Error('ActiveElement is not in the radiogroup'); } diff --git a/packages/@react-aria/test-utils/src/user.ts b/packages/@react-aria/test-utils/src/user.ts index 89cfe473190..89bf3b5abc8 100644 --- a/packages/@react-aria/test-utils/src/user.ts +++ b/packages/@react-aria/test-utils/src/user.ts @@ -17,6 +17,7 @@ import { GridListTesterOpts, ListBoxTesterOpts, MenuTesterOpts, + RadioGroupTesterOpts, SelectTesterOpts, TableTesterOpts, TabsTesterOpts, @@ -28,6 +29,7 @@ import {GridListTester} from './gridlist'; import {ListBoxTester} from './listbox'; import {MenuTester} from './menu'; import {pointerMap} from './'; +import {RadioGroupTester} from './radiogroup'; import {SelectTester} from './select'; import {TableTester} from './table'; import {TabsTester} from './tabs'; @@ -35,25 +37,27 @@ import {TreeTester} from './tree'; import userEvent from '@testing-library/user-event'; let keyToUtil: { - 'Select': typeof SelectTester, - 'Table': typeof TableTester, - 'Menu': typeof MenuTester, 'ComboBox': typeof ComboBoxTester, + 'Dialog': typeof DialogTester, 'GridList': typeof GridListTester, 'ListBox': typeof ListBoxTester, + 'Menu': typeof MenuTester, + 'RadioGroup': typeof RadioGroupTester, + 'Select': typeof SelectTester, + 'Table': typeof TableTester, 'Tabs': typeof TabsTester, - 'Tree': typeof TreeTester, - 'Dialog': typeof DialogTester + 'Tree': typeof TreeTester } = { - 'Select': SelectTester, - 'Table': TableTester, - 'Menu': MenuTester, 'ComboBox': ComboBoxTester, + 'Dialog': DialogTester, 'GridList': GridListTester, 'ListBox': ListBoxTester, + 'Menu': MenuTester, + 'RadioGroup': RadioGroupTester, + 'Select': SelectTester, + 'Table': TableTester, 'Tabs': TabsTester, - 'Tree': TreeTester, - 'Dialog': DialogTester + 'Tree': TreeTester } as const; export type PatternNames = keyof typeof keyToUtil; @@ -64,6 +68,7 @@ type Tester = T extends 'GridList' ? GridListTester : T extends 'ListBox' ? ListBoxTester : T extends 'Menu' ? MenuTester : + T extends 'RadioGroup' ? RadioGroupTester : T extends 'Select' ? SelectTester : T extends 'Table' ? TableTester : T extends 'Tabs' ? TabsTester : @@ -76,6 +81,7 @@ type TesterOpts = T extends 'GridList' ? GridListTesterOpts : T extends 'ListBox' ? ListBoxTesterOpts : T extends 'Menu' ? MenuTesterOpts : + T extends 'RadioGroup' ? RadioGroupTesterOpts : T extends 'Select' ? SelectTesterOpts : T extends 'Table' ? TableTesterOpts : T extends 'Tabs' ? TabsTesterOpts : diff --git a/packages/@react-spectrum/radio/test/Radio.test.js b/packages/@react-spectrum/radio/test/Radio.test.js index 358a5db79c9..294eca8db16 100644 --- a/packages/@react-spectrum/radio/test/Radio.test.js +++ b/packages/@react-spectrum/radio/test/Radio.test.js @@ -10,7 +10,7 @@ * governing permissions and limitations under the License. */ -import {act, pointerMap, render} from '@react-spectrum/test-utils-internal'; +import {act, pointerMap, render, User} from '@react-spectrum/test-utils-internal'; import {Button} from '@react-spectrum/button'; import {Form} from '@react-spectrum/form'; import {Provider} from '@react-spectrum/provider'; @@ -102,6 +102,7 @@ expect.extend({ describe('Radios', function () { let onChangeSpy = jest.fn(); let user; + let testUtilUser = new User(); beforeAll(() => { user = userEvent.setup({delay: null, pointerMap}); }); @@ -462,9 +463,9 @@ describe('Radios', function () { if (parseInt(React.version, 10) >= 19) { it('resets to defaultValue when submitting form action', async () => { - function Test() { + function Test() { const [value, formAction] = React.useActionState(() => 'cats', 'dogs'); - + return (
@@ -947,4 +948,46 @@ describe('Radios', function () { }); }); }); + + describe('test util tests', () => { + it.each` + Name | props + ${'ltr + vertical'} | ${{locale: 'de-DE', orientation: 'vertical'}} + ${'rtl + verfical'} | ${{locale: 'ar-AE', orientation: 'vertical'}} + ${'ltr + horizontal'} | ${{locale: 'de-DE', orientation: 'horizontal'}} + ${'rtl + horizontal'} | ${{locale: 'ar-AE', orientation: 'horizontal'}} + `('$Name should select the correct radio via keyboard regardless of orientation and disabled radios', async function ({Name, props}) { + let {getByRole} = render( + + + Dogs + Cats + Dragons + Unicorns + Chocobo + + + ); + let direction = props.locale === 'ar-AE' ? 'rtl' : 'ltr'; + let radioGroupTester = testUtilUser.createTester('RadioGroup', {root: getByRole('radiogroup'), direction}); + let radios = radioGroupTester.radios; + await radioGroupTester.triggerRadio({radio: radios[0]}); + expect(radios[0]).toBeChecked(); + + await radioGroupTester.triggerRadio({radio: 4, interactionType: 'keyboard'}); + expect(radios[4]).toBeChecked(); + + let radio4 = radioGroupTester.findRadio({radioIndexOrText: 3}); + console.log('radio4', radio4.outerHTML); + await radioGroupTester.triggerRadio({radio: radio4, interactionType: 'keyboard'}); + expect(radios[3]).toBeChecked(); + + await radioGroupTester.triggerRadio({radio: 'Dogs', interactionType: 'mouse'}); + expect(radios[0]).toBeChecked(); + + let radio5 = radioGroupTester.findRadio({radioIndexOrText: 'Chocobo'}); + await radioGroupTester.triggerRadio({radio: radio5, interactionType: 'mouse'}); + expect(radios[4]).toBeChecked(); + }); + }); }); diff --git a/packages/react-aria-components/test/RadioGroup.test.js b/packages/react-aria-components/test/RadioGroup.test.js index 706a32e8e88..e9a5aa4c751 100644 --- a/packages/react-aria-components/test/RadioGroup.test.js +++ b/packages/react-aria-components/test/RadioGroup.test.js @@ -13,6 +13,7 @@ import {act, pointerMap, render, within} from '@react-spectrum/test-utils-internal'; import {Button, Dialog, DialogTrigger, FieldError, Label, Modal, Radio, RadioContext, RadioGroup, RadioGroupContext, Text} from '../'; import React from 'react'; +import {User} from '@react-aria/test-utils'; import userEvent from '@testing-library/user-event'; let TestRadioGroup = ({groupProps, radioProps}) => ( @@ -28,6 +29,7 @@ let renderGroup = (groupProps, radioProps) => render( { let user; + let testUtilUser = new User(); beforeAll(() => { user = userEvent.setup({delay: null, pointerMap}); }); @@ -238,23 +240,25 @@ describe('RadioGroup', () => { it('should support selected state', async () => { let onChange = jest.fn(); - let {getAllByRole} = renderGroup({onChange}, {className: ({isSelected}) => isSelected ? 'selected' : ''}); - let radios = getAllByRole('radio'); + let {getByRole} = renderGroup({onChange}, {className: ({isSelected}) => isSelected ? 'selected' : ''}); + let radioGroupTester = testUtilUser.createTester('RadioGroup', {root: getByRole('radiogroup')}); + let radios = radioGroupTester.radios; let label = radios[0].closest('label'); - expect(radios[0]).not.toBeChecked(); + expect(radioGroupTester.selectedRadio).toBeFalsy(); expect(label).not.toHaveAttribute('data-selected'); expect(label).not.toHaveClass('selected'); - await user.click(radios[0]); + await radioGroupTester.triggerRadio({radio: radios[0]}); expect(onChange).toHaveBeenLastCalledWith('a'); - expect(radios[0]).toBeChecked(); + expect(radioGroupTester.selectedRadio).toBe(radios[0]); expect(label).toHaveAttribute('data-selected', 'true'); expect(label).toHaveClass('selected'); - await user.click(radios[1]); + await radioGroupTester.triggerRadio({radio: radios[1]}); expect(onChange).toHaveBeenLastCalledWith('b'); expect(radios[0]).not.toBeChecked(); + expect(radioGroupTester.selectedRadio).toBe(radios[1]); expect(label).not.toHaveAttribute('data-selected'); expect(label).not.toHaveClass('selected'); }); @@ -300,12 +304,19 @@ describe('RadioGroup', () => { expect(label).toHaveClass('required'); }); - it('should support orientation', () => { - let {getByRole} = renderGroup({orientation: 'horizontal', className: ({orientation}) => orientation}); - let group = getByRole('radiogroup'); + it('should support orientation', async () => { + let onChange = jest.fn(); + let {getByRole} = renderGroup({onChange, orientation: 'horizontal', className: ({orientation}) => orientation}); + let radioGroupTester = testUtilUser.createTester('RadioGroup', {root: getByRole('radiogroup')}); + + expect(radioGroupTester.radiogroup).toHaveAttribute('aria-orientation', 'horizontal'); + expect(radioGroupTester.radiogroup).toHaveClass('horizontal'); + let radios = radioGroupTester.radios; + await radioGroupTester.triggerRadio({radio: radios[0]}); + expect(radios[0]).toBeChecked(); - expect(group).toHaveAttribute('aria-orientation', 'horizontal'); - expect(group).toHaveClass('horizontal'); + await radioGroupTester.triggerRadio({radio: 2, interactionType: 'keyboard'}); + expect(radios[2]).toBeChecked(); }); it('supports help text', () => { From b87cdac9340cfbfaa75c7c01019444a4bd6266d2 Mon Sep 17 00:00:00 2001 From: Daniel Lu Date: Thu, 16 Oct 2025 10:02:52 -0700 Subject: [PATCH 05/10] fix missing orientation in S2 Radiogroup --- .../radio/stories/Radio.stories.tsx | 27 +++---- .../@react-spectrum/radio/test/Radio.test.js | 1 - .../@react-spectrum/s2/src/RadioGroup.tsx | 2 +- .../s2/test/RadioGroup.test.tsx | 77 +++++++++++++++++++ 4 files changed, 87 insertions(+), 20 deletions(-) create mode 100644 packages/@react-spectrum/s2/test/RadioGroup.test.tsx diff --git a/packages/@react-spectrum/radio/stories/Radio.stories.tsx b/packages/@react-spectrum/radio/stories/Radio.stories.tsx index 0e11dc5e3be..ef55152869f 100644 --- a/packages/@react-spectrum/radio/stories/Radio.stories.tsx +++ b/packages/@react-spectrum/radio/stories/Radio.stories.tsx @@ -29,33 +29,24 @@ export default { necessityIndicator: 'icon', labelPosition: 'top', labelAlign: 'start', - isInvalid: false, - orientation: 'vertical' + isInvalid: false }, argTypes: { labelPosition: { - control: { - type: 'radio', - options: ['top', 'side'] - } + control: 'radio', + options: ['top', 'side'] }, necessityIndicator: { - control: { - type: 'radio', - options: ['icon', 'label'] - } + control: 'radio', + options: ['icon', 'label'] }, labelAlign: { - control: { - type: 'radio', - options: ['start', 'end'] - } + control: 'radio', + options: ['start', 'end'] }, orientation: { - control: { - type: 'radio', - options: ['horizontal', 'vertical'] - } + control: 'radio', + options: ['horizontal', 'vertical'] } } } as Meta; diff --git a/packages/@react-spectrum/radio/test/Radio.test.js b/packages/@react-spectrum/radio/test/Radio.test.js index 294eca8db16..429a55130ee 100644 --- a/packages/@react-spectrum/radio/test/Radio.test.js +++ b/packages/@react-spectrum/radio/test/Radio.test.js @@ -978,7 +978,6 @@ describe('Radios', function () { expect(radios[4]).toBeChecked(); let radio4 = radioGroupTester.findRadio({radioIndexOrText: 3}); - console.log('radio4', radio4.outerHTML); await radioGroupTester.triggerRadio({radio: radio4, interactionType: 'keyboard'}); expect(radios[3]).toBeChecked(); diff --git a/packages/@react-spectrum/s2/src/RadioGroup.tsx b/packages/@react-spectrum/s2/src/RadioGroup.tsx index 11ad561c3c1..2d979c3a350 100644 --- a/packages/@react-spectrum/s2/src/RadioGroup.tsx +++ b/packages/@react-spectrum/s2/src/RadioGroup.tsx @@ -74,10 +74,10 @@ export const RadioGroup = /*#__PURE__*/ forwardRef(function RadioGroup(props: Ra UNSAFE_style, ...groupProps } = props; - return ( { + let testUtilUser = new User(); + let user; + beforeAll(() => { + user = userEvent.setup({delay: null, pointerMap}); + }); + + it.each` + Name | props + ${'ltr + vertical'} | ${{locale: 'de-DE', orientation: 'vertical'}} + ${'rtl + verfical'} | ${{locale: 'ar-AE', orientation: 'vertical'}} + ${'ltr + horizontal'} | ${{locale: 'de-DE', orientation: 'horizontal'}} + ${'rtl + horizontal'} | ${{locale: 'ar-AE', orientation: 'horizontal'}} + `('$Name should select the correct radio via keyboard regardless of orientation and disabled radios', async function ({props}) { + let {getByRole} = render( + + + Dogs + Cats + Dragons + Unicorns + Chocobo + + + ); + let direction = props.locale === 'ar-AE' ? 'rtl' : 'ltr' as Direction; + let radioGroupTester = testUtilUser.createTester('RadioGroup', {root: getByRole('radiogroup'), direction}); + expect(radioGroupTester.radiogroup).toHaveAttribute('aria-orientation', props.orientation); + let radios = radioGroupTester.radios; + await radioGroupTester.triggerRadio({radio: radios[0]}); + expect(radios[0]).toBeChecked(); + + await radioGroupTester.triggerRadio({radio: 4, interactionType: 'keyboard'}); + expect(radios[4]).toBeChecked(); + + let radio4 = radioGroupTester.findRadio({radioIndexOrText: 3}); + await radioGroupTester.triggerRadio({radio: radio4, interactionType: 'keyboard'}); + expect(radios[3]).toBeChecked(); + + await radioGroupTester.triggerRadio({radio: 'Dogs', interactionType: 'mouse'}); + expect(radios[0]).toBeChecked(); + + let radio5 = radioGroupTester.findRadio({radioIndexOrText: 'Chocobo'}); + await radioGroupTester.triggerRadio({radio: radio5, interactionType: 'mouse'}); + expect(radios[4]).toBeChecked(); + + // This isn't using the radioGroup tester because the tester uses the attached aria-orientation to determine + // what arrow to press, which won't reproduce the original bug where we forgot to pass the orientation to the RadioGroup. + // The tester ends up still pressing the correct keys (ArrowUp/ArrowDown) to navigate properly even in the horizontal orientation + // instead of using ArrowLeft/ArrowRight + await user.keyboard('[ArrowLeft]'); + if (props.locale === 'ar-AE' && props.orientation === 'horizontal') { + expect(radios[0]).toBeChecked(); + } else { + expect(radios[3]).toBeChecked(); + } + }); +}); From e556c9cfc06f2df5eb31118d5173d7f039b91f0a Mon Sep 17 00:00:00 2001 From: Daniel Lu Date: Thu, 16 Oct 2025 11:43:30 -0700 Subject: [PATCH 06/10] add checkbox group test util --- .../test-utils/src/checkboxgroup.ts | 162 ++++++++++++++++++ .../@react-aria/test-utils/src/radiogroup.ts | 8 +- packages/@react-aria/test-utils/src/types.ts | 2 + packages/@react-aria/test-utils/src/user.ts | 8 +- .../checkbox/test/CheckboxGroup.test.js | 54 +++++- .../s2/test/CheckboxGroup.test.js | 53 ------ .../s2/test/CheckboxGroup.test.tsx | 97 +++++++++++ .../s2/test/RadioGroup.test.tsx | 4 +- 8 files changed, 325 insertions(+), 63 deletions(-) create mode 100644 packages/@react-aria/test-utils/src/checkboxgroup.ts delete mode 100644 packages/@react-spectrum/s2/test/CheckboxGroup.test.js create mode 100644 packages/@react-spectrum/s2/test/CheckboxGroup.test.tsx diff --git a/packages/@react-aria/test-utils/src/checkboxgroup.ts b/packages/@react-aria/test-utils/src/checkboxgroup.ts new file mode 100644 index 00000000000..f0c43d910cf --- /dev/null +++ b/packages/@react-aria/test-utils/src/checkboxgroup.ts @@ -0,0 +1,162 @@ +/* + * Copyright 2025 Adobe. All rights reserved. + * This file is licensed to you under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. You may obtain a copy + * of the License at http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under + * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS + * OF ANY KIND, either express or implied. See the License for the specific language + * governing permissions and limitations under the License. + */ + +import {act, within} from '@testing-library/react'; +import {CheckboxGroupTesterOpts, UserOpts} from './types'; +import {pressElement} from './events'; + +interface TriggerCheckboxOptions { + /** + * What interaction type to use when triggering a checkbox. Defaults to the interaction type set on the tester. + */ + interactionType?: UserOpts['interactionType'], + /** + * The index, text, or node of the checkbox to toggle selection for. + */ + checkbox: number | string | HTMLElement +} + +export class CheckboxGroupTester { + private user; + private _interactionType: UserOpts['interactionType']; + private _checkboxgroup: HTMLElement; + + + constructor(opts: CheckboxGroupTesterOpts) { + let {root, user, interactionType} = opts; + this.user = user; + this._interactionType = interactionType || 'mouse'; + + this._checkboxgroup = root; + let checkboxgroup = within(root).queryAllByRole('group'); + if (checkboxgroup.length > 0) { + this._checkboxgroup = checkboxgroup[0]; + } + } + + /** + * Set the interaction type used by the checkbox group tester. + */ + setInteractionType(type: UserOpts['interactionType']): void { + this._interactionType = type; + } + + /** + * Returns a checkbox matching the specified index or text content. + */ + findCheckbox(opts: {checkboxIndexOrText: number | string}): HTMLElement { + let { + checkboxIndexOrText + } = opts; + + let checkbox; + if (typeof checkboxIndexOrText === 'number') { + checkbox = this.checkboxes[checkboxIndexOrText]; + } else if (typeof checkboxIndexOrText === 'string') { + let label = within(this.checkboxgroup).getByText(checkboxIndexOrText); + + // Label may wrap the checkbox, or the actual label may be a sibling span, or the checkbox div could have the label within it + if (label) { + checkbox = within(label).queryByRole('checkbox'); + if (!checkbox) { + let labelWrapper = label.closest('label'); + if (labelWrapper) { + checkbox = within(labelWrapper).queryByRole('checkbox'); + } else { + checkbox = label.closest('[role=checkbox]'); + } + } + } + } + + return checkbox; + } + + private async keyboardNavigateToCheckbox(opts: {checkbox: HTMLElement}) { + let {checkbox} = opts; + let checkboxes = this.checkboxes; + checkboxes = checkboxes.filter(checkbox => !(checkbox.hasAttribute('disabled') || checkbox.getAttribute('aria-disabled') === 'true')); + if (checkboxes.length === 0) { + throw new Error('Checkbox group doesnt have any non-disabled checkboxes. Please double check your checkbox group.'); + } + + let targetIndex = checkboxes.indexOf(checkbox); + if (targetIndex === -1) { + throw new Error('Checkbox provided is not in the checkbox group.'); + } + + if (!this.checkboxgroup.contains(document.activeElement)) { + act(() => checkboxes[0].focus()); + } + + let currIndex = checkboxes.indexOf(document.activeElement as HTMLElement); + if (currIndex === -1) { + throw new Error('Active element is not in the checkbox group.'); + } + + for (let i = 0; i < Math.abs(targetIndex - currIndex); i++) { + await this.user.tab({shift: targetIndex < currIndex}); + } + }; + + /** + * Toggles the specified checkbox. Defaults to using the interaction type set on the checkbox tester. + */ + async toggleCheckbox(opts: TriggerCheckboxOptions): Promise { + let { + checkbox, + interactionType = this._interactionType + } = opts; + + if (typeof checkbox === 'string' || typeof checkbox === 'number') { + checkbox = this.findCheckbox({checkboxIndexOrText: checkbox}); + } + + if (!checkbox) { + throw new Error('Target checkbox not found in the checkboxgroup.'); + } else if (checkbox.hasAttribute('disabled')) { + throw new Error('Target checkbox is disabled.'); + } + + if (interactionType === 'keyboard') { + if (document.activeElement !== this._checkboxgroup && !this._checkboxgroup.contains(document.activeElement)) { + act(() => this._checkboxgroup.focus()); + } + + await this.keyboardNavigateToCheckbox({checkbox}); + await this.user.keyboard('[Space]'); + } else { + await pressElement(this.user, checkbox, interactionType); + } + } + + /** + * Returns the checkboxgroup. + */ + get checkboxgroup(): HTMLElement { + return this._checkboxgroup; + } + + /** + * Returns the checkboxes. + */ + get checkboxes(): HTMLElement[] { + return within(this.checkboxgroup).queryAllByRole('checkbox'); + } + + /** + * Returns the currently selected checkboxes in the checkboxgroup if any. + */ + get selectedCheckboxes(): HTMLElement[] { + return this.checkboxes.filter(checkbox => (checkbox as HTMLInputElement).checked || checkbox.getAttribute('aria-checked') === 'true'); + } +} diff --git a/packages/@react-aria/test-utils/src/radiogroup.ts b/packages/@react-aria/test-utils/src/radiogroup.ts index 80bfb910517..b90161b80e7 100644 --- a/packages/@react-aria/test-utils/src/radiogroup.ts +++ b/packages/@react-aria/test-utils/src/radiogroup.ts @@ -86,12 +86,12 @@ export class RadioGroupTester { let radios = this.radios; radios = radios.filter(radio => !(radio.hasAttribute('disabled') || radio.getAttribute('aria-disabled') === 'true')); if (radios.length === 0) { - throw new Error('Radio group doesnt have any activatable radios. Please double check your radio group.'); + throw new Error('Radio group doesnt have any non-disabled radios. Please double check your radio group.'); } let targetIndex = radios.indexOf(radio); if (targetIndex === -1) { - throw new Error('Radio provided is not in the radiogroup'); + throw new Error('Radio provided is not in the radio group.'); } if (!this.radiogroup.contains(document.activeElement)) { @@ -105,7 +105,7 @@ export class RadioGroupTester { let currIndex = radios.indexOf(document.activeElement as HTMLElement); if (currIndex === -1) { - throw new Error('ActiveElement is not in the radiogroup'); + throw new Error('Active element is not in the radio group.'); } let arrowUp = 'ArrowUp'; @@ -140,7 +140,7 @@ export class RadioGroupTester { } if (!radio) { - throw new Error('Target radio not found in the radiogroup.'); + throw new Error('Target radio not found in the radio group.'); } else if (radio.hasAttribute('disabled')) { throw new Error('Target radio is disabled.'); } diff --git a/packages/@react-aria/test-utils/src/types.ts b/packages/@react-aria/test-utils/src/types.ts index a24bb86c8df..034efd11d31 100644 --- a/packages/@react-aria/test-utils/src/types.ts +++ b/packages/@react-aria/test-utils/src/types.ts @@ -39,6 +39,8 @@ export interface BaseTesterOpts extends UserOpts { root: HTMLElement } +export interface CheckboxGroupTesterOpts extends BaseTesterOpts {} + export interface ComboBoxTesterOpts extends BaseTesterOpts { /** * The base element for the combobox. If provided the wrapping element around the target combobox (as is the the case with a ref provided to RSP ComboBox), diff --git a/packages/@react-aria/test-utils/src/user.ts b/packages/@react-aria/test-utils/src/user.ts index 89bf3b5abc8..a468cbf7e0e 100644 --- a/packages/@react-aria/test-utils/src/user.ts +++ b/packages/@react-aria/test-utils/src/user.ts @@ -10,8 +10,9 @@ * governing permissions and limitations under the License. */ -import {ComboBoxTester} from './combobox'; +import {CheckboxGroupTester} from './checkboxgroup'; import { + CheckboxGroupTesterOpts, ComboBoxTesterOpts, DialogTesterOpts, GridListTesterOpts, @@ -24,6 +25,7 @@ import { TreeTesterOpts, UserOpts } from './types'; +import {ComboBoxTester} from './combobox'; import {DialogTester} from './dialog'; import {GridListTester} from './gridlist'; import {ListBoxTester} from './listbox'; @@ -37,6 +39,7 @@ import {TreeTester} from './tree'; import userEvent from '@testing-library/user-event'; let keyToUtil: { + 'CheckboxGroup': typeof CheckboxGroupTester, 'ComboBox': typeof ComboBoxTester, 'Dialog': typeof DialogTester, 'GridList': typeof GridListTester, @@ -48,6 +51,7 @@ let keyToUtil: { 'Tabs': typeof TabsTester, 'Tree': typeof TreeTester } = { + 'CheckboxGroup': CheckboxGroupTester, 'ComboBox': ComboBoxTester, 'Dialog': DialogTester, 'GridList': GridListTester, @@ -63,6 +67,7 @@ export type PatternNames = keyof typeof keyToUtil; // Conditional type: https://www.typescriptlang.org/docs/handbook/2/conditional-types.html type Tester = + T extends 'CheckboxGroup' ? CheckboxGroupTester : T extends 'ComboBox' ? ComboBoxTester : T extends 'Dialog' ? DialogTester : T extends 'GridList' ? GridListTester : @@ -76,6 +81,7 @@ type Tester = never; type TesterOpts = + T extends 'CheckboxGroup' ? CheckboxGroupTesterOpts : T extends 'ComboBox' ? ComboBoxTesterOpts : T extends 'Dialog' ? DialogTesterOpts : T extends 'GridList' ? GridListTesterOpts : diff --git a/packages/@react-spectrum/checkbox/test/CheckboxGroup.test.js b/packages/@react-spectrum/checkbox/test/CheckboxGroup.test.js index 3f644c4af48..e86bfd9a968 100644 --- a/packages/@react-spectrum/checkbox/test/CheckboxGroup.test.js +++ b/packages/@react-spectrum/checkbox/test/CheckboxGroup.test.js @@ -10,7 +10,7 @@ * governing permissions and limitations under the License. */ -import {act, pointerMap, render, within} from '@react-spectrum/test-utils-internal'; +import {act, pointerMap, render, User, within} from '@react-spectrum/test-utils-internal'; import {Button} from '@react-spectrum/button'; import {Checkbox, CheckboxGroup} from '../'; import {Form} from '@react-spectrum/form'; @@ -21,6 +21,7 @@ import userEvent from '@testing-library/user-event'; describe('CheckboxGroup', () => { let user; + let testUtilUser = new User(); beforeAll(() => { user = userEvent.setup({delay: null, pointerMap}); }); @@ -417,9 +418,9 @@ describe('CheckboxGroup', () => { if (parseInt(React.version, 10) >= 19) { it('resets to defaultValue when submitting form action', async () => { - function Test() { + function Test() { const [value, formAction] = React.useActionState(() => ['dogs', 'cats'], []); - + return ( @@ -776,4 +777,51 @@ describe('CheckboxGroup', () => { }); }); }); + + describe('keyboard', () => { + it.each` + Name | props + ${'ltr + vertical'} | ${{locale: 'de-DE', orientation: 'vertical'}} + ${'rtl + verfical'} | ${{locale: 'ar-AE', orientation: 'vertical'}} + ${'ltr + horizontal'} | ${{locale: 'de-DE', orientation: 'horizontal'}} + ${'rtl + horizontal'} | ${{locale: 'ar-AE', orientation: 'horizontal'}} + `('$Name should select the correct checkbox regardless of orientation and disabled checkboxes', async function ({props}) { + let {getByRole} = render( + + + Soccer + Baseball + Basketball + Tennis + Rugby + + + ); + + let checkboxGroupTester = testUtilUser.createTester('CheckboxGroup', {root: getByRole('group')}); + expect(checkboxGroupTester.checkboxgroup).toHaveAttribute('role'); + let checkboxes = checkboxGroupTester.checkboxes; + await checkboxGroupTester.toggleCheckbox({checkbox: checkboxes[0]}); + expect(checkboxes[0]).toBeChecked(); + expect(checkboxGroupTester.selectedCheckboxes).toHaveLength(1); + + await checkboxGroupTester.toggleCheckbox({checkbox: 4, interactionType: 'keyboard'}); + expect(checkboxes[4]).toBeChecked(); + expect(checkboxGroupTester.selectedCheckboxes).toHaveLength(2); + + let checkbox4 = checkboxGroupTester.findCheckbox({checkboxIndexOrText: 3}); + await checkboxGroupTester.toggleCheckbox({checkbox: checkbox4, interactionType: 'keyboard'}); + expect(checkboxes[3]).toBeChecked(); + expect(checkboxGroupTester.selectedCheckboxes).toHaveLength(3); + + await checkboxGroupTester.toggleCheckbox({checkbox: 'Soccer', interactionType: 'keyboard'}); + expect(checkboxes[0]).not.toBeChecked(); + expect(checkboxGroupTester.selectedCheckboxes).toHaveLength(2); + + let checkbox5 = checkboxGroupTester.findCheckbox({checkboxIndexOrText: 'Rugby'}); + await checkboxGroupTester.toggleCheckbox({checkbox: checkbox5, interactionType: 'mouse'}); + expect(checkboxes[4]).not.toBeChecked(); + expect(checkboxGroupTester.selectedCheckboxes).toHaveLength(1); + }); + }); }); diff --git a/packages/@react-spectrum/s2/test/CheckboxGroup.test.js b/packages/@react-spectrum/s2/test/CheckboxGroup.test.js deleted file mode 100644 index 041ff5e3ae5..00000000000 --- a/packages/@react-spectrum/s2/test/CheckboxGroup.test.js +++ /dev/null @@ -1,53 +0,0 @@ -/* - * Copyright 2024 Adobe. All rights reserved. - * This file is licensed to you under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. You may obtain a copy - * of the License at http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software distributed under - * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS - * OF ANY KIND, either express or implied. See the License for the specific language - * governing permissions and limitations under the License. - */ - -import {act, pointerMap, render} from '@react-spectrum/test-utils-internal'; -import {Checkbox, CheckboxGroup, Form} from '../src'; -import React from 'react'; -import userEvent from '@testing-library/user-event'; - -describe('CheckboxGroup', () => { - let user; - beforeAll(() => { - user = userEvent.setup({delay: null, pointerMap}); - }); - - it('should not require all checkboxes to be checked when Form has isRequired', async () => { - let {getByRole, getAllByRole, getByTestId} = render( - - - Soccer - Baseball - Basketball - - - ); - - - let group = getByRole('group'); - let checkbox = getAllByRole('checkbox')[0]; - - await user.click(checkbox); - act(() => {getByTestId('form').checkValidity();}); - expect(group).not.toHaveAttribute('aria-describedby'); - expect(group).not.toHaveAttribute('data-invalid'); - - await user.click(checkbox); - act(() => {getByTestId('form').checkValidity();}); - expect(group).toHaveAttribute('data-invalid'); - expect(group).toHaveAttribute('aria-describedby'); - let errorMsg = document.getElementById(group.getAttribute('aria-describedby')); - expect(errorMsg).toHaveTextContent('Constraints not satisfied'); - }); -}); - - diff --git a/packages/@react-spectrum/s2/test/CheckboxGroup.test.tsx b/packages/@react-spectrum/s2/test/CheckboxGroup.test.tsx new file mode 100644 index 00000000000..8081ede4888 --- /dev/null +++ b/packages/@react-spectrum/s2/test/CheckboxGroup.test.tsx @@ -0,0 +1,97 @@ +/* + * Copyright 2024 Adobe. All rights reserved. + * This file is licensed to you under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. You may obtain a copy + * of the License at http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under + * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS + * OF ANY KIND, either express or implied. See the License for the specific language + * governing permissions and limitations under the License. + */ + +import {act, pointerMap, render, User} from '@react-spectrum/test-utils-internal'; +import {Checkbox, CheckboxGroup, Form, Provider} from '../src'; +import React from 'react'; +import userEvent from '@testing-library/user-event'; + +describe('CheckboxGroup', () => { + let testUtilUser = new User(); + let user; + beforeAll(() => { + user = userEvent.setup({delay: null, pointerMap}); + }); + + it('should not require all checkboxes to be checked when Form has isRequired', async () => { + let {getByRole, getAllByRole, getByTestId} = render( +
+ + Soccer + Baseball + Basketball + +
+ ); + + + let group = getByRole('group'); + let checkbox = getAllByRole('checkbox')[0]; + + await user.click(checkbox); + act(() => {(getByTestId('form') as HTMLFormElement).checkValidity();}); + expect(group).not.toHaveAttribute('aria-describedby'); + expect(group).not.toHaveAttribute('data-invalid'); + + await user.click(checkbox); + act(() => {(getByTestId('form') as HTMLFormElement).checkValidity();}); + expect(group).toHaveAttribute('data-invalid'); + expect(group).toHaveAttribute('aria-describedby'); + let errorMsg = document.getElementById(group.getAttribute('aria-describedby')!); + expect(errorMsg).toHaveTextContent('Constraints not satisfied'); + }); + + it.each` + Name | props + ${'ltr + vertical'} | ${{locale: 'de-DE', orientation: 'vertical'}} + ${'rtl + verfical'} | ${{locale: 'ar-AE', orientation: 'vertical'}} + ${'ltr + horizontal'} | ${{locale: 'de-DE', orientation: 'horizontal'}} + ${'rtl + horizontal'} | ${{locale: 'ar-AE', orientation: 'horizontal'}} + `('$Name should select the correct checkbox regardless of orientation and disabled checkboxes', async function ({props}) { + let {getByRole} = render( + + + Soccer + Baseball + Basketball + Tennis + Rugby + + + ); + + let checkboxGroupTester = testUtilUser.createTester('CheckboxGroup', {root: getByRole('group')}); + expect(checkboxGroupTester.checkboxgroup).toHaveAttribute('role'); + let checkboxes = checkboxGroupTester.checkboxes; + await checkboxGroupTester.toggleCheckbox({checkbox: checkboxes[0]}); + expect(checkboxes[0]).toBeChecked(); + expect(checkboxGroupTester.selectedCheckboxes).toHaveLength(1); + + await checkboxGroupTester.toggleCheckbox({checkbox: 4, interactionType: 'keyboard'}); + expect(checkboxes[4]).toBeChecked(); + expect(checkboxGroupTester.selectedCheckboxes).toHaveLength(2); + + let checkbox4 = checkboxGroupTester.findCheckbox({checkboxIndexOrText: 3}); + await checkboxGroupTester.toggleCheckbox({checkbox: checkbox4, interactionType: 'keyboard'}); + expect(checkboxes[3]).toBeChecked(); + expect(checkboxGroupTester.selectedCheckboxes).toHaveLength(3); + + await checkboxGroupTester.toggleCheckbox({checkbox: 'Soccer', interactionType: 'keyboard'}); + expect(checkboxes[0]).not.toBeChecked(); + expect(checkboxGroupTester.selectedCheckboxes).toHaveLength(2); + + let checkbox5 = checkboxGroupTester.findCheckbox({checkboxIndexOrText: 'Rugby'}); + await checkboxGroupTester.toggleCheckbox({checkbox: checkbox5, interactionType: 'mouse'}); + expect(checkboxes[4]).not.toBeChecked(); + expect(checkboxGroupTester.selectedCheckboxes).toHaveLength(1); + }); +}); diff --git a/packages/@react-spectrum/s2/test/RadioGroup.test.tsx b/packages/@react-spectrum/s2/test/RadioGroup.test.tsx index 93d06703674..1868fe10e7e 100644 --- a/packages/@react-spectrum/s2/test/RadioGroup.test.tsx +++ b/packages/@react-spectrum/s2/test/RadioGroup.test.tsx @@ -69,9 +69,9 @@ describe('RadioGroup', () => { // instead of using ArrowLeft/ArrowRight await user.keyboard('[ArrowLeft]'); if (props.locale === 'ar-AE' && props.orientation === 'horizontal') { - expect(radios[0]).toBeChecked(); + expect(radioGroupTester.selectedRadio).toBe(radios[0]); } else { - expect(radios[3]).toBeChecked(); + expect(radioGroupTester.selectedRadio).toBe(radios[3]); } }); }); From 5cf5388f90c77f52d21f437cb33dde41e82478f6 Mon Sep 17 00:00:00 2001 From: Daniel Lu Date: Thu, 16 Oct 2025 14:19:00 -0700 Subject: [PATCH 07/10] fix tabs test util so that it properly keyboard navigates over disabled tabs --- packages/@react-aria/test-utils/src/tabs.ts | 9 ++++- .../@react-spectrum/tabs/test/Tabs.test.js | 39 +++++++++++++++++++ 2 files changed, 46 insertions(+), 2 deletions(-) diff --git a/packages/@react-aria/test-utils/src/tabs.ts b/packages/@react-aria/test-utils/src/tabs.ts index d52672c6aa0..c26da3e7656 100644 --- a/packages/@react-aria/test-utils/src/tabs.ts +++ b/packages/@react-aria/test-utils/src/tabs.ts @@ -79,6 +79,11 @@ export class TabsTester { private async keyboardNavigateToTab(opts: {tab: HTMLElement, orientation?: Orientation}) { let {tab, orientation = 'vertical'} = opts; let tabs = this.tabs; + tabs = tabs.filter(tab => !(tab.hasAttribute('disabled') || tab.getAttribute('aria-disabled') === 'true')); + if (tabs.length === 0) { + throw new Error('Tablist doesnt have any non-disabled tabs. Please double check your tabs implementation.'); + } + let targetIndex = tabs.indexOf(tab); if (targetIndex === -1) { throw new Error('Tab provided is not in the tablist'); @@ -89,11 +94,11 @@ export class TabsTester { if (selectedTab != null) { act(() => selectedTab.focus()); } else { - act(() => tabs.find(tab => !(tab.hasAttribute('disabled') || tab.getAttribute('aria-disabled') === 'true'))?.focus()); + act(() => tabs[0]?.focus()); } } - let currIndex = this.tabs.indexOf(document.activeElement as HTMLElement); + let currIndex = tabs.indexOf(document.activeElement as HTMLElement); if (currIndex === -1) { throw new Error('ActiveElement is not in the tablist'); } diff --git a/packages/@react-spectrum/tabs/test/Tabs.test.js b/packages/@react-spectrum/tabs/test/Tabs.test.js index 802c7701e79..cfa024e2d40 100644 --- a/packages/@react-spectrum/tabs/test/Tabs.test.js +++ b/packages/@react-spectrum/tabs/test/Tabs.test.js @@ -1313,4 +1313,43 @@ describe('Tabs', function () { } }); }); + + describe('test utils', function () { + let items = [ + {name: 'Tab 1', children: 'Tab 1 body'}, + {name: 'Tab 2', children: 'Tab 2 body'}, + {name: 'Tab 3', children: 'Tab 3 body'}, + {name: 'Tab 4', children: 'Tab 4 body'}, + {name: 'Tab 5', children: 'Tab 5 body'} + ]; + it.each` + Name | props + ${'ltr + vertical'} | ${{locale: 'de-DE', orientation: 'vertical'}} + ${'rtl + verfical'} | ${{locale: 'ar-AE', orientation: 'vertical'}} + ${'ltr + horizontal'} | ${{locale: 'de-DE', orientation: 'horizontal'}} + ${'rtl + horizontal'} | ${{locale: 'ar-AE', orientation: 'horizontal'}} + `('$Name should select the correct radio via keyboard regardless of orientation and disabled radios', async function ({props}) { + let {getByRole} = renderComponent({items, orientation: props.orientation, disabledKeys: ['Tab 2', 'Tab 3'], providerProps: {locale: props.locale}}); + + let direction = props.locale === 'ar-AE' ? 'rtl' : 'ltr'; + let tabsTester = testUtilUser.createTester('Tabs', {root: getByRole('tablist'), direction}); + expect(tabsTester.tablist).toHaveAttribute('aria-orientation', props.orientation); + let tabs = tabsTester.tabs; + await tabsTester.triggerTab({tab: tabs[0]}); + expect(tabsTester.selectedTab).toBe(tabs[0]); + + await tabsTester.triggerTab({tab: 4, interactionType: 'keyboard'}); + expect(tabsTester.selectedTab).toBe(tabs[4]); + let tab4 = tabsTester.findTab({tabIndexOrText: 3}); + await tabsTester.triggerTab({tab: tab4, interactionType: 'keyboard'}); + expect(tabsTester.selectedTab).toBe(tabs[3]); + + await tabsTester.triggerTab({tab: 'Tab 1', interactionType: 'mouse'}); + expect(tabsTester.selectedTab).toBe(tabs[0]); + + let tab5 = tabsTester.findTab({tabIndexOrText: 'Tab 5'}); + await tabsTester.triggerTab({tab: tab5, interactionType: 'mouse'}); + expect(tabsTester.selectedTab).toBe(tabs[4]); + }); + }); }); From e8284126dc780dac2dcd964e21cb01cc877e45a1 Mon Sep 17 00:00:00 2001 From: Daniel Lu Date: Fri, 17 Oct 2025 11:35:48 -0700 Subject: [PATCH 08/10] review comments --- packages/@react-aria/test-utils/src/checkboxgroup.ts | 4 ---- packages/@react-aria/test-utils/src/dialog.ts | 10 ++++------ packages/@react-aria/test-utils/src/radiogroup.ts | 4 ---- .../@react-spectrum/s2/test/EditableTableView.test.tsx | 1 - 4 files changed, 4 insertions(+), 15 deletions(-) diff --git a/packages/@react-aria/test-utils/src/checkboxgroup.ts b/packages/@react-aria/test-utils/src/checkboxgroup.ts index f0c43d910cf..54e436a5cab 100644 --- a/packages/@react-aria/test-utils/src/checkboxgroup.ts +++ b/packages/@react-aria/test-utils/src/checkboxgroup.ts @@ -128,10 +128,6 @@ export class CheckboxGroupTester { } if (interactionType === 'keyboard') { - if (document.activeElement !== this._checkboxgroup && !this._checkboxgroup.contains(document.activeElement)) { - act(() => this._checkboxgroup.focus()); - } - await this.keyboardNavigateToCheckbox({checkbox}); await this.user.keyboard('[Space]'); } else { diff --git a/packages/@react-aria/test-utils/src/dialog.ts b/packages/@react-aria/test-utils/src/dialog.ts index ebae5dcd275..238b7556348 100644 --- a/packages/@react-aria/test-utils/src/dialog.ts +++ b/packages/@react-aria/test-utils/src/dialog.ts @@ -61,12 +61,10 @@ export class DialogTester { } = opts; let trigger = this.trigger; 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]'}); - } + 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]'); diff --git a/packages/@react-aria/test-utils/src/radiogroup.ts b/packages/@react-aria/test-utils/src/radiogroup.ts index b90161b80e7..6c1d0e38c9e 100644 --- a/packages/@react-aria/test-utils/src/radiogroup.ts +++ b/packages/@react-aria/test-utils/src/radiogroup.ts @@ -146,10 +146,6 @@ export class RadioGroupTester { } if (interactionType === 'keyboard') { - if (document.activeElement !== this._radiogroup && !this._radiogroup.contains(document.activeElement)) { - act(() => this._radiogroup.focus()); - } - let radioOrientation = this._radiogroup.getAttribute('aria-orientation') || 'horizontal'; await this.keyboardNavigateToRadio({radio, orientation: radioOrientation as Orientation}); } else { diff --git a/packages/@react-spectrum/s2/test/EditableTableView.test.tsx b/packages/@react-spectrum/s2/test/EditableTableView.test.tsx index 88c7249e576..6a87ebcacdf 100644 --- a/packages/@react-spectrum/s2/test/EditableTableView.test.tsx +++ b/packages/@react-spectrum/s2/test/EditableTableView.test.tsx @@ -255,7 +255,6 @@ describe('TableView', () => { 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? expect(dialog).toBeVisible(); let input = within(dialog!).getByRole('textbox'); From d1fa76c67205d80b84df2f303174d66d981cd97c Mon Sep 17 00:00:00 2001 From: Daniel Lu Date: Mon, 20 Oct 2025 10:32:52 -0700 Subject: [PATCH 09/10] review comments --- packages/@react-aria/test-utils/src/dialog.ts | 10 +++------- packages/@react-aria/test-utils/src/types.ts | 4 ---- .../@react-spectrum/s2/test/EditableTableView.test.tsx | 1 - packages/react-aria-components/test/Dialog.test.js | 2 +- 4 files changed, 4 insertions(+), 13 deletions(-) diff --git a/packages/@react-aria/test-utils/src/dialog.ts b/packages/@react-aria/test-utils/src/dialog.ts index 238b7556348..ee029531cf3 100644 --- a/packages/@react-aria/test-utils/src/dialog.ts +++ b/packages/@react-aria/test-utils/src/dialog.ts @@ -25,15 +25,12 @@ export class DialogTester { private _interactionType: UserOpts['interactionType']; private _trigger: HTMLElement | undefined; private _dialog: HTMLElement | undefined; - // TODO: may not need this? Isn't really useful - private _dialogType: DialogTesterOpts['dialogType']; private _overlayType: DialogTesterOpts['overlayType']; constructor(opts: DialogTesterOpts) { - let {root, user, interactionType, dialogType, overlayType} = opts; + let {root, user, interactionType, overlayType} = opts; this.user = user; this._interactionType = interactionType || 'mouse'; - this._dialogType = dialogType || 'dialog'; this._overlayType = overlayType || 'modal'; // Handle case where element provided is a wrapper of the trigger button @@ -91,9 +88,9 @@ export class DialogTester { } else { let dialog; await waitFor(() => { - dialog = document.querySelector(`[role=${this._dialogType}]`); + dialog = document.querySelector(`[role=dialog], [role=alertdialog]`); if (dialog == null) { - throw new Error(`No dialog of type ${this._dialogType} found after pressing the trigger.`); + throw new Error(`No dialog of type role="dialog" or role="alertdialog" found after pressing the trigger.`); } else { return true; } @@ -102,7 +99,6 @@ export class DialogTester { if (dialog && document.activeElement !== this._trigger && dialog.contains(document.activeElement)) { this._dialog = dialog; } else { - // TODO: is it too brittle to throw here? throw new Error('New modal dialog doesnt contain the active element OR the active element is still the trigger. Uncertain if the proper modal dialog was found'); } } diff --git a/packages/@react-aria/test-utils/src/types.ts b/packages/@react-aria/test-utils/src/types.ts index 034efd11d31..5ecbc80b52e 100644 --- a/packages/@react-aria/test-utils/src/types.ts +++ b/packages/@react-aria/test-utils/src/types.ts @@ -59,10 +59,6 @@ export interface DialogTesterOpts extends BaseTesterOpts { * The trigger element for the dialog. */ root: HTMLElement, - /** - * The type of dialog. - */ - dialogType?: 'alertdialog' | 'dialog', /** * The overlay type of the dialog. Used to inform the tester how to find the dialog. */ diff --git a/packages/@react-spectrum/s2/test/EditableTableView.test.tsx b/packages/@react-spectrum/s2/test/EditableTableView.test.tsx index 6a87ebcacdf..3d61c5cc052 100644 --- a/packages/@react-spectrum/s2/test/EditableTableView.test.tsx +++ b/packages/@react-spectrum/s2/test/EditableTableView.test.tsx @@ -251,7 +251,6 @@ describe('TableView', () => { await user.tab(); await user.keyboard('{ArrowRight}'); 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 let dialogTester = testUtilUser.createTester('Dialog', {root: dialogTrigger, interactionType: 'keyboard', overlayType: 'modal'}); await dialogTester.open(); let dialog = dialogTester.dialog; diff --git a/packages/react-aria-components/test/Dialog.test.js b/packages/react-aria-components/test/Dialog.test.js index 8d33ba39856..f21edee1e65 100644 --- a/packages/react-aria-components/test/Dialog.test.js +++ b/packages/react-aria-components/test/Dialog.test.js @@ -70,7 +70,7 @@ describe('Dialog', () => { ); let button = getByRole('button'); - let dialogTester = testUtilUser.createTester('Dialog', {root: button, dialogType: 'alertdialog', overlayType: 'modal'}); + let dialogTester = testUtilUser.createTester('Dialog', {root: button, overlayType: 'modal'}); await dialogTester.open(); let dialog = dialogTester.dialog; expect(dialog).toHaveAttribute('role', 'alertdialog'); From b46d053d5e764b26205eedaf9994efb7f1cff8b6 Mon Sep 17 00:00:00 2001 From: Daniel Lu Date: Tue, 21 Oct 2025 11:17:24 -0700 Subject: [PATCH 10/10] fix lint --- packages/@react-aria/test-utils/src/dialog.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/@react-aria/test-utils/src/dialog.ts b/packages/@react-aria/test-utils/src/dialog.ts index ee029531cf3..213c86c2b1a 100644 --- a/packages/@react-aria/test-utils/src/dialog.ts +++ b/packages/@react-aria/test-utils/src/dialog.ts @@ -88,9 +88,9 @@ export class DialogTester { } else { let dialog; await waitFor(() => { - dialog = document.querySelector(`[role=dialog], [role=alertdialog]`); + dialog = document.querySelector('[role=dialog], [role=alertdialog]'); if (dialog == null) { - throw new Error(`No dialog of type role="dialog" or role="alertdialog" found after pressing the trigger.`); + throw new Error('No dialog of type role="dialog" or role="alertdialog" found after pressing the trigger.'); } else { return true; }