diff --git a/packages/@react-aria/dnd/src/DragManager.ts b/packages/@react-aria/dnd/src/DragManager.ts index 6f0e3114433..d9ec3727ce2 100644 --- a/packages/@react-aria/dnd/src/DragManager.ts +++ b/packages/@react-aria/dnd/src/DragManager.ts @@ -408,7 +408,7 @@ class DragSession { this.dragTarget.element, ...validDropItems.flatMap(item => item.activateButtonRef?.current ? [item.element, item.activateButtonRef?.current] : [item.element]), ...visibleDropTargets.flatMap(target => target.activateButtonRef?.current ? [target.element, target.activateButtonRef?.current] : [target.element]) - ], {shouldUseInert: true}); + ]); this.mutationObserver.observe(document.body, {subtree: true, attributes: true, attributeFilter: ['aria-hidden']}); } diff --git a/packages/@react-aria/focus/src/FocusScope.tsx b/packages/@react-aria/focus/src/FocusScope.tsx index 6c5a3d2c7c5..6f04afe7720 100644 --- a/packages/@react-aria/focus/src/FocusScope.tsx +++ b/packages/@react-aria/focus/src/FocusScope.tsx @@ -24,6 +24,7 @@ import { } from '@react-aria/utils'; import {FocusableElement, RefObject} from '@react-types/shared'; import {focusSafely, getInteractionModality} from '@react-aria/interactions'; +import {isElementVisible} from './isElementVisible'; import React, {JSX, ReactNode, useContext, useEffect, useMemo, useRef} from 'react'; export interface FocusScopeProps { @@ -757,6 +758,7 @@ export function getFocusableTreeWalker(root: Element, opts?: FocusManagerOptions } if (filter(node as Element) + && isElementVisible(node as Element) && (!scope || isElementInScope(node as Element, scope)) && (!opts?.accept || opts.accept(node as Element)) ) { diff --git a/packages/@react-aria/utils/src/isElementVisible.ts b/packages/@react-aria/focus/src/isElementVisible.ts similarity index 90% rename from packages/@react-aria/utils/src/isElementVisible.ts rename to packages/@react-aria/focus/src/isElementVisible.ts index e404cb7a264..b931d9bf6f5 100644 --- a/packages/@react-aria/utils/src/isElementVisible.ts +++ b/packages/@react-aria/focus/src/isElementVisible.ts @@ -10,9 +10,7 @@ * governing permissions and limitations under the License. */ -import {getOwnerWindow} from './domHelpers'; - -const supportsCheckVisibility = typeof Element !== 'undefined' && 'checkVisibility' in Element.prototype; +import {getOwnerWindow} from '@react-aria/utils'; function isStyleVisible(element: Element) { const windowObject = getOwnerWindow(element); @@ -62,10 +60,6 @@ function isAttributeVisible(element: Element, childElement?: Element) { * @param element - Element to evaluate for display or visibility. */ export function isElementVisible(element: Element, childElement?: Element): boolean { - if (supportsCheckVisibility) { - return element.checkVisibility(); - } - return ( element.nodeName !== '#comment' && isStyleVisible(element) && diff --git a/packages/@react-aria/overlays/src/ariaHideOutside.ts b/packages/@react-aria/overlays/src/ariaHideOutside.ts index 21470304752..b7b2a9586de 100644 --- a/packages/@react-aria/overlays/src/ariaHideOutside.ts +++ b/packages/@react-aria/overlays/src/ariaHideOutside.ts @@ -10,15 +10,6 @@ * governing permissions and limitations under the License. */ -import {getOwnerWindow} from '@react-aria/utils'; - -const supportsInert = typeof HTMLElement !== 'undefined' && 'inert' in HTMLElement.prototype; - -interface AriaHideOutsideOptions { - root?: Element, - shouldUseInert?: boolean -} - // Keeps a ref count of all hidden elements. Added to when hiding an element, and // subtracted from when showing it again. When it reaches zero, aria-hidden is removed. let refCountMap = new WeakMap(); @@ -38,28 +29,10 @@ let observerStack: Array = []; * @param root - Nothing will be hidden above this element. * @returns - A function to restore all hidden elements. */ -export function ariaHideOutside(targets: Element[], options?: AriaHideOutsideOptions | Element) { - let windowObj = getOwnerWindow(targets?.[0]); - let opts = options instanceof windowObj.Element ? {root: options} : options; - let root = opts?.root ?? document.body; - let shouldUseInert = opts?.shouldUseInert && supportsInert; +export function ariaHideOutside(targets: Element[], root = document.body) { let visibleNodes = new Set(targets); let hiddenNodes = new Set(); - let getHidden = (element: Element) => { - return shouldUseInert && element instanceof windowObj.HTMLElement ? element.inert : element.getAttribute('aria-hidden') === 'true'; - }; - - let setHidden = (element: Element, hidden: boolean) => { - if (shouldUseInert && element instanceof windowObj.HTMLElement) { - element.inert = hidden; - } else if (hidden) { - element.setAttribute('aria-hidden', 'true'); - } else { - element.removeAttribute('aria-hidden'); - } - }; - let walk = (root: Element) => { // Keep live announcer and top layer elements (e.g. toasts) visible. for (let element of root.querySelectorAll('[data-live-announcer], [data-react-aria-top-layer]')) { @@ -114,12 +87,12 @@ export function ariaHideOutside(targets: Element[], options?: AriaHideOutsideOpt // If already aria-hidden, and the ref count is zero, then this element // was already hidden and there's nothing for us to do. - if (getHidden(node) && refCount === 0) { + if (node.getAttribute('aria-hidden') === 'true' && refCount === 0) { return; } if (refCount === 0) { - setHidden(node, true); + node.setAttribute('aria-hidden', 'true'); } hiddenNodes.add(node); @@ -188,7 +161,7 @@ export function ariaHideOutside(targets: Element[], options?: AriaHideOutsideOpt continue; } if (count === 1) { - setHidden(node, false); + node.removeAttribute('aria-hidden'); refCountMap.delete(node); } else { refCountMap.set(node, count - 1); diff --git a/packages/@react-aria/overlays/src/useModalOverlay.ts b/packages/@react-aria/overlays/src/useModalOverlay.ts index c0fbb70a0b1..4f95655a60b 100644 --- a/packages/@react-aria/overlays/src/useModalOverlay.ts +++ b/packages/@react-aria/overlays/src/useModalOverlay.ts @@ -58,7 +58,7 @@ export function useModalOverlay(props: AriaModalOverlayProps, state: OverlayTrig useEffect(() => { if (state.isOpen && ref.current) { - return ariaHideOutside([ref.current], {shouldUseInert: true}); + return ariaHideOutside([ref.current]); } }, [state.isOpen, ref]); diff --git a/packages/@react-aria/overlays/src/usePopover.ts b/packages/@react-aria/overlays/src/usePopover.ts index 281c3509b8e..068c67e7bf4 100644 --- a/packages/@react-aria/overlays/src/usePopover.ts +++ b/packages/@react-aria/overlays/src/usePopover.ts @@ -13,10 +13,9 @@ import {ariaHideOutside, keepVisible} from './ariaHideOutside'; import {AriaPositionProps, useOverlayPosition} from './useOverlayPosition'; import {DOMAttributes, RefObject} from '@react-types/shared'; -import {mergeProps} from '@react-aria/utils'; +import {mergeProps, useLayoutEffect} from '@react-aria/utils'; import {OverlayTriggerState} from '@react-stately/overlays'; import {PlacementAxis} from '@react-types/overlays'; -import {useEffect} from 'react'; import {useOverlay} from './useOverlay'; import {usePreventScroll} from './usePreventScroll'; @@ -114,12 +113,12 @@ export function usePopover(props: AriaPopoverProps, state: OverlayTriggerState): isDisabled: isNonModal || !state.isOpen }); - useEffect(() => { + useLayoutEffect(() => { if (state.isOpen && popoverRef.current) { if (isNonModal) { return keepVisible(groupRef?.current ?? popoverRef.current); } else { - return ariaHideOutside([groupRef?.current ?? popoverRef.current], {shouldUseInert: true}); + return ariaHideOutside([groupRef?.current ?? popoverRef.current]); } } }, [isNonModal, state.isOpen, popoverRef, groupRef]); diff --git a/packages/@react-aria/utils/src/isFocusable.ts b/packages/@react-aria/utils/src/isFocusable.ts index c1a8c0dc6a5..f0fd6ad5140 100644 --- a/packages/@react-aria/utils/src/isFocusable.ts +++ b/packages/@react-aria/utils/src/isFocusable.ts @@ -1,17 +1,3 @@ -/* - * 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 {isElementVisible} from './isElementVisible'; - const focusableElements = [ 'input:not([disabled]):not([type=hidden])', 'select:not([disabled])', @@ -34,22 +20,9 @@ focusableElements.push('[tabindex]:not([tabindex="-1"]):not([disabled])'); const TABBABLE_ELEMENT_SELECTOR = focusableElements.join(':not([hidden]):not([tabindex="-1"]),'); export function isFocusable(element: Element): boolean { - return element.matches(FOCUSABLE_ELEMENT_SELECTOR) && isElementVisible(element) && !isInert(element); + return element.matches(FOCUSABLE_ELEMENT_SELECTOR); } export function isTabbable(element: Element): boolean { - return element.matches(TABBABLE_ELEMENT_SELECTOR) && isElementVisible(element) && !isInert(element); -} - -function isInert(element: Element): boolean { - let node: Element | null = element; - while (node != null) { - if (node instanceof node.ownerDocument.defaultView!.HTMLElement && node.inert) { - return true; - } - - node = node.parentElement; - } - - return false; + return element.matches(TABBABLE_ELEMENT_SELECTOR); } diff --git a/packages/@react-spectrum/dialog/test/DialogTrigger.test.js b/packages/@react-spectrum/dialog/test/DialogTrigger.test.js index 52765f37a48..f0eb6ff8efc 100644 --- a/packages/@react-spectrum/dialog/test/DialogTrigger.test.js +++ b/packages/@react-spectrum/dialog/test/DialogTrigger.test.js @@ -15,6 +15,7 @@ import {ActionButton, Button} from '@react-spectrum/button'; import {ButtonGroup} from '@react-spectrum/buttongroup'; import {Content} from '@react-spectrum/view'; import {Dialog, DialogTrigger} from '../'; +import {Heading} from '@react-spectrum/text'; import {Item, Menu, MenuTrigger} from '@react-spectrum/menu'; import {Provider} from '@react-spectrum/provider'; import React from 'react'; @@ -976,6 +977,55 @@ describe('DialogTrigger', function () { expect(document.activeElement).toBe(innerInput); }); + it('will not lose focus to body', async () => { + let {getByRole, getByTestId} = render( + + + Trigger + + The Heading + + + Test + + Item 1 + Item 2 + Item 3 + + + + + + + ); + let button = getByRole('button'); + await user.click(button); + + act(() => { + jest.runAllTimers(); + }); + + let outerDialog = getByRole('dialog'); + + await waitFor(() => { + expect(outerDialog).toBeVisible(); + }); // wait for animation + let innerButton = getByTestId('innerButton'); + await user.tab(); + fireEvent.keyDown(document.activeElement, {key: 'Enter'}); + fireEvent.keyUp(document.activeElement, {key: 'Enter'}); + + act(() => { + jest.runAllTimers(); + }); + await user.tab(); + act(() => { + jest.runAllTimers(); + }); + + expect(document.activeElement).toBe(innerButton); + }); + describe('portalContainer', () => { function InfoDialog(props) { let {container} = props; diff --git a/packages/@react-spectrum/menu/src/MenuTrigger.tsx b/packages/@react-spectrum/menu/src/MenuTrigger.tsx index 6f23974af9e..8c9e59d8cc7 100644 --- a/packages/@react-spectrum/menu/src/MenuTrigger.tsx +++ b/packages/@react-spectrum/menu/src/MenuTrigger.tsx @@ -103,8 +103,7 @@ export const MenuTrigger = forwardRef(function MenuTrigger(props: SpectrumMenuTr scrollRef={menuRef} placement={initialPlacement} hideArrow - shouldFlip={shouldFlip} - shouldContainFocus> + shouldFlip={shouldFlip}> {menu} ); diff --git a/packages/@react-spectrum/menu/test/MenuTrigger.test.js b/packages/@react-spectrum/menu/test/MenuTrigger.test.js index 567ff686c51..265940e0c88 100644 --- a/packages/@react-spectrum/menu/test/MenuTrigger.test.js +++ b/packages/@react-spectrum/menu/test/MenuTrigger.test.js @@ -809,7 +809,7 @@ describe('MenuTrigger', function () { }); }); - it('does not close if menu is tabbed away from', async function () { + it('closes if menu is tabbed away from', async function () { let tree = render( @@ -835,8 +835,8 @@ describe('MenuTrigger', function () { await user.tab(); act(() => {jest.runAllTimers();}); act(() => {jest.runAllTimers();}); - expect(menu).toBeInTheDocument(); - expect(document.activeElement).toBe(menuTester.options()[0]); + expect(menu).not.toBeInTheDocument(); + expect(document.activeElement).toBe(menuTester.trigger); }); }); @@ -930,6 +930,22 @@ AriaMenuTests({ multipleSelection: () => render( ), + siblingFocusableElement: () => render( + + + + + + One + Two + Three + + + + + ), multipleMenus: () => render( @@ -1066,6 +1082,24 @@ AriaMenuTests({ multipleSelection: () => render( ), + siblingFocusableElement: () => render( + + + + + + {item => ( +
+ {item => {item.name}} +
+ )} + + + + + ), multipleMenus: () => render( diff --git a/packages/@react-spectrum/overlays/src/Overlay.tsx b/packages/@react-spectrum/overlays/src/Overlay.tsx index 7c43194a04e..e999fa5a546 100644 --- a/packages/@react-spectrum/overlays/src/Overlay.tsx +++ b/packages/@react-spectrum/overlays/src/Overlay.tsx @@ -22,7 +22,6 @@ export const Overlay = React.forwardRef(function Overlay(props: OverlayProps, re children, isOpen, disableFocusManagement, - shouldContainFocus, container, onEnter, onEntering, @@ -57,7 +56,7 @@ export const Overlay = React.forwardRef(function Overlay(props: OverlayProps, re } return ( - + (props: hideArrow state={state} triggerRef={unwrappedTriggerRef} - scrollRef={listboxRef} - shouldContainFocus> + scrollRef={listboxRef}> {listbox} ); diff --git a/packages/@react-spectrum/picker/test/Picker.test.js b/packages/@react-spectrum/picker/test/Picker.test.js index 3e45c065a67..da7c4ad6960 100644 --- a/packages/@react-spectrum/picker/test/Picker.test.js +++ b/packages/@react-spectrum/picker/test/Picker.test.js @@ -583,9 +583,9 @@ describe('Picker', function () { expect(document.activeElement).toBe(picker); }); - it('does not shift focus when tabbing', async function () { + it('tabs to the next element after the trigger and closes the menu', async function () { let onOpenChange = jest.fn(); - let {getByRole} = render( + let {getByRole, getByTestId} = render( @@ -611,10 +611,51 @@ describe('Picker', function () { fireEvent.keyDown(document.activeElement, {key: 'Tab'}); act(() => jest.runAllTimers()); - expect(listbox).toBeInTheDocument(); + expect(listbox).not.toBeInTheDocument(); + expect(picker).toHaveAttribute('aria-expanded', 'false'); + expect(picker).not.toHaveAttribute('aria-controls'); + expect(onOpenChange).toBeCalledTimes(2); + expect(onOpenChange).toHaveBeenCalledWith(false); + + expect(document.activeElement).toBe(getByTestId('after-input')); + }); + + it('shift tabs to the previous element before the trigger and closes the menu', async function () { + let onOpenChange = jest.fn(); + let {getByRole, getByTestId} = render( + + + + One + Two + Three + + + + ); + + let picker = getByRole('button'); + await user.click(picker); + act(() => jest.runAllTimers()); + + let listbox = getByRole('listbox'); + expect(listbox).toBeVisible(); + expect(onOpenChange).toBeCalledTimes(1); + expect(onOpenChange).toHaveBeenCalledWith(true); expect(picker).toHaveAttribute('aria-expanded', 'true'); expect(picker).toHaveAttribute('aria-controls', listbox.id); - expect(document.activeElement).toBe(listbox); + + fireEvent.keyDown(document.activeElement, {key: 'Tab', shiftKey: true}); + fireEvent.keyUp(document.activeElement, {key: 'Tab', shiftKey: true}); + act(() => jest.runAllTimers()); + + expect(listbox).not.toBeInTheDocument(); + expect(picker).toHaveAttribute('aria-expanded', 'false'); + expect(picker).not.toHaveAttribute('aria-controls'); + expect(onOpenChange).toBeCalledTimes(2); + expect(onOpenChange).toHaveBeenCalledWith(false); + + expect(document.activeElement).toBe(getByTestId('before-input')); }); it('should have a hidden dismiss button for screen readers', async function () { @@ -1889,6 +1930,58 @@ describe('Picker', function () { expect(document.activeElement).toBe(beforeBtn); }); + it('calls onBlur and onFocus for the open Picker', async function () { + let {getByRole, getByTestId} = render( + +