diff --git a/packages/@react-aria/focus/src/FocusScope.tsx b/packages/@react-aria/focus/src/FocusScope.tsx index ab7c4c7f067..0a0dc1310f4 100644 --- a/packages/@react-aria/focus/src/FocusScope.tsx +++ b/packages/@react-aria/focus/src/FocusScope.tsx @@ -15,6 +15,7 @@ import { getActiveElement, getEventTarget, getOwnerDocument, + getOwnerWindow, isAndroid, isChrome, isFocusable, @@ -295,23 +296,37 @@ function shouldContainFocus(scopeRef: ScopeRef) { return true; } -function isTabbableRadio(element: HTMLInputElement) { - if (element.checked) { - return true; - } - let radios: HTMLInputElement[] = []; +function getRadiosInGroup(element: HTMLInputElement): HTMLInputElement[] { if (!element.form) { - radios = ([...getOwnerDocument(element).querySelectorAll(`input[type="radio"][name="${CSS.escape(element.name)}"]`)] as HTMLInputElement[]).filter(radio => !radio.form); - } else { - let radioList = element.form?.elements?.namedItem(element.name) as RadioNodeList; - radios = [...(radioList ?? [])] as HTMLInputElement[]; + // Radio buttons outside a form - query the document + return Array.from( + getOwnerDocument(element).querySelectorAll( + `input[type="radio"][name="${CSS.escape(element.name)}"]` + ) + ).filter(radio => !radio.form); } - if (!radios) { - return false; + + // namedItem returns RadioNodeList (iterable) for 2+ elements, but a single Element for exactly 1. + // https://developer.mozilla.org/en-US/docs/Web/API/HTMLFormControlsCollection/namedItem + const radioList = element.form.elements.namedItem(element.name); + let ownerWindow = getOwnerWindow(element); + if (radioList instanceof ownerWindow.RadioNodeList) { + return Array.from(radioList).filter( + (el): el is HTMLInputElement => el instanceof ownerWindow.HTMLInputElement + ); } - let anyChecked = radios.some(radio => radio.checked); + if (radioList instanceof ownerWindow.HTMLInputElement) { + return [radioList]; + } + return []; +} - return !anyChecked; +function isTabbableRadio(element: HTMLInputElement): boolean { + if (element.checked) { + return true; + } + const radios = getRadiosInGroup(element); + return radios.length > 0 && !radios.some(radio => radio.checked); } function useFocusContainment(scopeRef: RefObject, contain?: boolean) { diff --git a/packages/@react-aria/focus/test/FocusScope.test.js b/packages/@react-aria/focus/test/FocusScope.test.js index ef0b104c084..38648c12635 100644 --- a/packages/@react-aria/focus/test/FocusScope.test.js +++ b/packages/@react-aria/focus/test/FocusScope.test.js @@ -1538,6 +1538,32 @@ describe('FocusScope', function () { expect(document.activeElement).toBe(getByTestId('button1')); }); + it('handles forms with a single radio button without crashing', async function () { + // Regression test for https://github.com/adobe/react-spectrum/issues/9569 + // form.elements.namedItem() returns Element (not RadioNodeList) for single elements + function Test() { + return ( + + +
+ + +
+ + + ); + } + + let {getByTestId, getByRole} = render(); + let radio = getByRole('radio'); + await user.tab(); + expect(document.activeElement).toBe(getByTestId('button1')); + await user.tab(); + expect(document.activeElement).toBe(radio); + await user.tab(); + expect(document.activeElement).toBe(getByTestId('button2')); + }); + describe('nested focus scopes', function () { it('should make child FocusScopes the active scope regardless of DOM structure', function () { function ChildComponent(props) { diff --git a/packages/@react-aria/menu/src/useMenuItem.ts b/packages/@react-aria/menu/src/useMenuItem.ts index a8302c8c745..7e161eec2b6 100644 --- a/packages/@react-aria/menu/src/useMenuItem.ts +++ b/packages/@react-aria/menu/src/useMenuItem.ts @@ -188,7 +188,8 @@ export function useMenuItem(props: AriaMenuItemProps, state: TreeState, re } if (isVirtualized) { - ariaProps['aria-posinset'] = item?.index; + let index = Number(item?.index); + ariaProps['aria-posinset'] = Number.isNaN(index) ? undefined : index + 1; ariaProps['aria-setsize'] = getItemCount(state.collection); } diff --git a/packages/@react-aria/menu/test/useMenu.test.tsx b/packages/@react-aria/menu/test/useMenu.test.tsx index aafb79204c3..f99047417d3 100644 --- a/packages/@react-aria/menu/test/useMenu.test.tsx +++ b/packages/@react-aria/menu/test/useMenu.test.tsx @@ -10,12 +10,14 @@ * governing permissions and limitations under the License. */ + import {AriaMenuProps, useMenu, useMenuItem} from '../'; import {Item} from '@react-stately/collections'; +import {Key} from '@react-types/shared'; import {pointerMap, render} from '@react-spectrum/test-utils-internal'; import React from 'react'; +import {TreeState, useTreeState} from '@react-stately/tree'; import userEvent from '@testing-library/user-event'; -import {useTreeState} from '@react-stately/tree'; function Menu(props: AriaMenuProps & {onSelect: () => void}) { // Create menu state based on the incoming props @@ -51,6 +53,41 @@ function MenuItem({item, state, onAction}) { ); } +interface VirtualizedMenuItemProps { + item: {key: Key, rendered: React.ReactNode, index?: number}, + state: TreeState, + onAction?: (key: Key) => void +} + +function VirtualizedMenuItem({item, state, onAction}: VirtualizedMenuItemProps) { + let ref = React.useRef(null); + let {menuItemProps} = useMenuItem( + {key: item.key, onAction, isVirtualized: true}, + state, + ref + ); + + return ( +
  • + {item.rendered} +
    • + ); +} + +function VirtualizedMenu(props: AriaMenuProps) { + let state = useTreeState(props); + let ref = React.useRef(null); + let {menuProps} = useMenu(props, state, ref); + + return ( +
      + {[...state.collection].map((item) => ( + + ))} +
    + ); +} + describe('useMenuTrigger', function () { let user; beforeAll(() => { @@ -75,3 +112,37 @@ describe('useMenuTrigger', function () { expect(onSelect).toHaveBeenCalledTimes(1); }); }); + +describe('useMenuItem with isVirtualized', function () { + it('sets correct aria-posinset (1-based) for virtualized menu items', () => { + let {getAllByRole} = render( + + One + Two + Three + + ); + + let items = getAllByRole('menuitem'); + // aria-posinset should be 1-based (1, 2, 3), not 0-based (0, 1, 2) + expect(items[0]).toHaveAttribute('aria-posinset', '1'); + expect(items[1]).toHaveAttribute('aria-posinset', '2'); + expect(items[2]).toHaveAttribute('aria-posinset', '3'); + }); + + it('sets correct aria-setsize for virtualized menu items', () => { + let {getAllByRole} = render( + + One + Two + Three + + ); + + let items = getAllByRole('menuitem'); + // aria-setsize should match the total number of items + expect(items[0]).toHaveAttribute('aria-setsize', '3'); + expect(items[1]).toHaveAttribute('aria-setsize', '3'); + expect(items[2]).toHaveAttribute('aria-setsize', '3'); + }); +}); diff --git a/packages/@react-spectrum/s2/src/TableView.tsx b/packages/@react-spectrum/s2/src/TableView.tsx index b5e66fd84a2..df243e855c7 100644 --- a/packages/@react-spectrum/s2/src/TableView.tsx +++ b/packages/@react-spectrum/s2/src/TableView.tsx @@ -955,7 +955,8 @@ const cell = style({ ...commonCellStyles, color: { default: baseColor('neutral-subdued'), - isSelected: baseColor('neutral') + isSelected: baseColor('neutral'), + forcedColors: 'ButtonText' }, paddingY: centerPadding(), minHeight: { diff --git a/packages/react-aria-components/src/Button.tsx b/packages/react-aria-components/src/Button.tsx index d43d789db59..33887f318f0 100644 --- a/packages/react-aria-components/src/Button.tsx +++ b/packages/react-aria-components/src/Button.tsx @@ -168,11 +168,13 @@ export const Button = /*#__PURE__*/ createHideableComponent(function Button(prop ); }); +// Events to preserve when isPending is true (for tooltips and other overlays) +const PRESERVED_EVENT_PATTERN = /Focus|Blur|Hover|Pointer(Enter|Leave|Over|Out)|Mouse(Enter|Leave|Over|Out)/; + function useDisableInteractions(props, isPending) { - // Don't allow interaction while isPending is true if (isPending) { for (const key in props) { - if (key.startsWith('on') && !(key.includes('Focus') || key.includes('Blur'))) { + if (key.startsWith('on') && !PRESERVED_EVENT_PATTERN.test(key)) { props[key] = undefined; } } diff --git a/packages/react-aria-components/src/ComboBox.tsx b/packages/react-aria-components/src/ComboBox.tsx index 91c5cfe1208..0fe80d7c2c4 100644 --- a/packages/react-aria-components/src/ComboBox.tsx +++ b/packages/react-aria-components/src/ComboBox.tsx @@ -241,8 +241,10 @@ function ComboBoxInner({props, collection, comboBoxRef: ref}: data-open={state.isOpen || undefined} data-disabled={props.isDisabled || undefined} data-invalid={validation.isInvalid || undefined} - data-required={props.isRequired || undefined} /> - {name && formValue === 'key' && } + data-required={props.isRequired || undefined}> + {renderProps.children} + {name && formValue === 'key' && } + ); } diff --git a/packages/react-aria-components/src/Menu.tsx b/packages/react-aria-components/src/Menu.tsx index 91cb0727e4a..c6ab3cfb6aa 100644 --- a/packages/react-aria-components/src/Menu.tsx +++ b/packages/react-aria-components/src/Menu.tsx @@ -431,12 +431,13 @@ export const MenuItem = /*#__PURE__*/ createLeafComponent(ItemNode, function Men let state = useContext(MenuStateContext)!; let ref = useObjectRef(forwardedRef); let selectionManager = useContext(SelectionManagerContext)!; - + let {isVirtualized} = useContext(CollectionRendererContext); let {menuItemProps, labelProps, descriptionProps, keyboardShortcutProps, ...states} = useMenuItem({ ...props, id, key: item.key, - selectionManager + selectionManager, + isVirtualized: isVirtualized }, state, ref); let {hoverProps, isHovered} = useHover({ diff --git a/packages/react-aria-components/stories/Button.stories.tsx b/packages/react-aria-components/stories/Button.stories.tsx index 47ffbe8d1b1..32f6f09ea66 100644 --- a/packages/react-aria-components/stories/Button.stories.tsx +++ b/packages/react-aria-components/stories/Button.stories.tsx @@ -11,7 +11,7 @@ */ import {action} from '@storybook/addon-actions'; -import {Button, ProgressBar, Text} from 'react-aria-components'; +import {Button, ProgressBar, Text, Tooltip, TooltipTrigger} from 'react-aria-components'; import {mergeProps} from '@react-aria/utils'; import {Meta, StoryObj} from '@storybook/react'; import React, {useEffect, useRef, useState} from 'react'; @@ -39,6 +39,13 @@ export const PendingButton: ButtonStory = { } }; +export const PendingButtonTooltip: ButtonStory = { + render: (args) => , + args: { + children: 'Press me, then hover again to see tooltip' + } +}; + function PendingButtonExample(props) { let [isPending, setPending] = useState(false); @@ -84,6 +91,25 @@ function PendingButtonExample(props) { ); } +function PendingButtonTooltipExample(props) { + return ( + + + + Tooltip should appear on hover + + + ); +} + export const RippleButtonExample: ButtonStory = { render: () => ( Press me diff --git a/packages/react-aria-components/stories/Menu.stories.tsx b/packages/react-aria-components/stories/Menu.stories.tsx index 55810f87160..6c824451ca5 100644 --- a/packages/react-aria-components/stories/Menu.stories.tsx +++ b/packages/react-aria-components/stories/Menu.stories.tsx @@ -11,7 +11,7 @@ */ import {action} from '@storybook/addon-actions'; -import {Button, Header, Heading, Input, Keyboard, Label, Menu, MenuItemProps, MenuSection, MenuTrigger, Popover, Separator, SubmenuTrigger, SubmenuTriggerProps, Text, TextField} from 'react-aria-components'; +import {Button, Header, Heading, Input, Keyboard, Label, ListLayout, Menu, MenuItemProps, MenuSection, MenuTrigger, Popover, Separator, SubmenuTrigger, SubmenuTriggerProps, Text, TextField, Virtualizer} from 'react-aria-components'; import {Meta, StoryFn, StoryObj} from '@storybook/react'; import {MyMenuItem} from './utils'; import React, {JSX} from 'react'; @@ -444,7 +444,7 @@ function MenuItemWithCustomElement(props: MenuItemProps) { // Otherwise we'd need another way to set the expected element type. return ( 'href' in domProps ? :
    } /> ); } @@ -453,3 +453,32 @@ function RouterLink(props: React.AnchorHTMLAttributes) { // eslint-disable-next-line jsx-a11y/anchor-has-content return {e.preventDefault(); console.log('click');}})} />; } + +let items = Array.from({length: 600}, (_, index) => { + // Return the object structure for each element + return { + id: index + 1, + name: `Object ${index + 1}`, + value: Math.random() + }; +}); +export const VirtualizedExample: MenuStory = () => { + return ( + + + + + + {(item) => { + return {item.name}; + }} + + + + + ); +}; diff --git a/packages/react-aria-components/test/Tooltip.test.js b/packages/react-aria-components/test/Tooltip.test.js index e13ecf87b10..01c6cffe26c 100644 --- a/packages/react-aria-components/test/Tooltip.test.js +++ b/packages/react-aria-components/test/Tooltip.test.js @@ -61,6 +61,25 @@ describe('Tooltip', () => { expect(arrow).toHaveStyle('position: absolute'); }); + it('shows on hover when button has isPending', async () => { + let {getByRole} = render( + + + Tooltip content + + ); + + let button = getByRole('button'); + + fireEvent.mouseMove(document.body); + await user.hover(button); + act(() => jest.runAllTimers()); + + let tooltip = getByRole('tooltip'); + expect(tooltip).toBeInTheDocument(); + expect(tooltip).toHaveTextContent('Tooltip content'); + }); + it('shows on focus', async () => { let {getByRole} = renderTooltip(); let button = getByRole('button'); diff --git a/packages/react-aria-components/test/VirtualizedMenu.test.tsx b/packages/react-aria-components/test/VirtualizedMenu.test.tsx new file mode 100644 index 00000000000..37579b9f142 --- /dev/null +++ b/packages/react-aria-components/test/VirtualizedMenu.test.tsx @@ -0,0 +1,84 @@ +/* + * Copyright 2022 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 {Button, ListLayout, Menu, MenuItem, MenuTrigger, Popover, Virtualizer} from '..'; +import React from 'react'; +import {User} from '@react-aria/test-utils'; +import userEvent from '@testing-library/user-event'; + +let items = Array.from({length: 50}, (_, index) => { + // Return the object structure for each element + return { + id: index + 1, + name: `Object ${index + 1}`, + value: Math.random() + }; +}); +const VirtualizedExample = () => { + return ( + + + + + + {(item) => { + return {item.name}; + }} + + + + + ); +}; + +// @ts-ignore +window.getComputedStyle = (el) => el.style; + +describe('virtualized menu', () => { + let user; + let testUtilUser = new User({advanceTimer: jest.advanceTimersByTime}); + beforeAll(function () { + user = userEvent.setup({delay: null, pointerMap}); + jest.useFakeTimers(); + }); + + afterEach(() => { + act(() => {jest.runAllTimers();}); + }); + it('should support virtualized menu', async () => { + jest.restoreAllMocks(); // don't mock scrollTop for this test + jest.spyOn(window.HTMLElement.prototype, 'clientWidth', 'get').mockImplementation(() => 100); + jest.spyOn(window.HTMLElement.prototype, 'clientHeight', 'get').mockImplementation(() => 100); + + let {getAllByRole, container} = render(); + let tester = testUtilUser.createTester('Menu', {user, root: container}); + tester.setInteractionType('mouse'); + await tester.open(); + let items = getAllByRole('menuitem'); + let menu = tester.menu; + expect(menu).toBeInTheDocument(); + expect(items[0]).toHaveAttribute('aria-posinset', '1'); + expect(items[0]).toHaveAttribute('aria-setsize', '50'); + expect(items[items.length - 1]).toHaveAttribute('aria-posinset', items.length.toString()); + expect(items.length).toBeLessThan(50); + + await user.keyboard('{End}'); + items = getAllByRole('menuitem'); + expect(items.length).toBeLessThan(50); + expect(items[items.length - 1]).toHaveAttribute('aria-posinset', '50'); + }); +});