From 12801692620856994422a1e2547198b8f6d4dd01 Mon Sep 17 00:00:00 2001 From: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> Date: Sat, 18 Jul 2026 02:43:43 -0700 Subject: [PATCH 1/3] fix: restore focus without scrolling in FocusScope containment --- packages/react-aria/src/focus/FocusScope.tsx | 5 +++-- packages/react-aria/test/focus/FocusScope.test.js | 14 ++++++++++++++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/packages/react-aria/src/focus/FocusScope.tsx b/packages/react-aria/src/focus/FocusScope.tsx index 6f423f12b7f..57e3c321497 100644 --- a/packages/react-aria/src/focus/FocusScope.tsx +++ b/packages/react-aria/src/focus/FocusScope.tsx @@ -19,6 +19,7 @@ import {getInteractionModality} from '../interactions/useFocusVisible'; import {getOwnerDocument, getOwnerWindow} from '../utils/domHelpers'; import {isAndroid, isChrome} from '../utils/platform'; import {isFocusable, isTabbable} from '../utils/isFocusable'; +import {focusWithoutScrolling} from '../utils/focusWithoutScrolling'; import React, {JSX, ReactNode, useContext, useEffect, useMemo, useRef} from 'react'; import {useLayoutEffect} from '../utils/useLayoutEffect'; @@ -410,7 +411,7 @@ function useFocusContainment(scopeRef: RefObject, contain?: bo // If a focus event occurs outside the active scope (e.g. user tabs from browser location bar), // restore focus to the previously focused node or the first tabbable element in the active scope. if (focusedNode.current) { - focusedNode.current.focus(); + focusWithoutScrolling(focusedNode.current); } else if (activeScope && activeScope.current) { focusFirstInScope(activeScope.current); } @@ -444,7 +445,7 @@ function useFocusContainment(scopeRef: RefObject, contain?: bo let target = getEventTarget(e) as FocusableElement; if (target && target.isConnected) { focusedNode.current = target; - focusedNode.current?.focus(); + focusWithoutScrolling(focusedNode.current); } else if (activeScope.current) { focusFirstInScope(activeScope.current); } diff --git a/packages/react-aria/test/focus/FocusScope.test.js b/packages/react-aria/test/focus/FocusScope.test.js index 39dd0d8be39..1ef89242006 100644 --- a/packages/react-aria/test/focus/FocusScope.test.js +++ b/packages/react-aria/test/focus/FocusScope.test.js @@ -23,6 +23,7 @@ import {DialogContainer} from '@adobe/react-spectrum/DialogContainer'; import {enableShadowDOM} from 'react-stately/private/flags/flags'; import {FocusScope, useFocusManager} from '../../src/focus/FocusScope'; import {focusScopeTree} from '../../src/focus/FocusScope'; +import {focusWithoutScrolling} from '../../src/utils/focusWithoutScrolling'; import {Provider} from '@adobe/react-spectrum/Provider'; import React, {useEffect, useState} from 'react'; import ReactDOM from 'react-dom'; @@ -30,6 +31,14 @@ import {Example as StorybookExample} from '../../stories/focus/FocusScope.storie import {useEvent} from '../../src/utils/useEvent'; import userEvent from '@testing-library/user-event'; +jest.mock('../../src/utils/focusWithoutScrolling', () => { + let actual = jest.requireActual('../../src/utils/focusWithoutScrolling'); + return { + ...actual, + focusWithoutScrolling: jest.fn(actual.focusWithoutScrolling) + }; +}); + describe('FocusScope', function () { let user; @@ -324,11 +333,13 @@ describe('FocusScope', function () { }); expect(document.activeElement).toBe(input2); + focusWithoutScrolling.mockClear(); act(() => { outside.focus(); }); fireEvent.focusIn(outside); expect(document.activeElement).toBe(input2); + expect(focusWithoutScrolling).toHaveBeenCalledWith(input2); }); it('should restore focus to the last focused element in the scope on focus out', async function () { @@ -357,6 +368,8 @@ describe('FocusScope', function () { }); expect(document.activeElement).toBe(input2); + focusWithoutScrolling.mockClear(); + act(() => { input2.blur(); }); @@ -364,6 +377,7 @@ describe('FocusScope', function () { jest.runAllTimers(); }); expect(document.activeElement).toBe(input2); + expect(focusWithoutScrolling).toHaveBeenCalledWith(input2); fireEvent.focusOut(input2); expect(document.activeElement).toBe(input2); }); From 352c26f758aa9ff6fc03308f3efbf9c5be3354a0 Mon Sep 17 00:00:00 2001 From: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> Date: Sat, 18 Jul 2026 02:47:13 -0700 Subject: [PATCH 2/3] fix: route containment restore through focusElement for owner-document scroll safety --- packages/react-aria/src/focus/FocusScope.tsx | 5 ++--- .../react-aria/test/focus/FocusScope.test.js | 16 ++++++++-------- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/packages/react-aria/src/focus/FocusScope.tsx b/packages/react-aria/src/focus/FocusScope.tsx index 57e3c321497..ad4f0a427a3 100644 --- a/packages/react-aria/src/focus/FocusScope.tsx +++ b/packages/react-aria/src/focus/FocusScope.tsx @@ -19,7 +19,6 @@ import {getInteractionModality} from '../interactions/useFocusVisible'; import {getOwnerDocument, getOwnerWindow} from '../utils/domHelpers'; import {isAndroid, isChrome} from '../utils/platform'; import {isFocusable, isTabbable} from '../utils/isFocusable'; -import {focusWithoutScrolling} from '../utils/focusWithoutScrolling'; import React, {JSX, ReactNode, useContext, useEffect, useMemo, useRef} from 'react'; import {useLayoutEffect} from '../utils/useLayoutEffect'; @@ -411,7 +410,7 @@ function useFocusContainment(scopeRef: RefObject, contain?: bo // If a focus event occurs outside the active scope (e.g. user tabs from browser location bar), // restore focus to the previously focused node or the first tabbable element in the active scope. if (focusedNode.current) { - focusWithoutScrolling(focusedNode.current); + focusElement(focusedNode.current); } else if (activeScope && activeScope.current) { focusFirstInScope(activeScope.current); } @@ -445,7 +444,7 @@ function useFocusContainment(scopeRef: RefObject, contain?: bo let target = getEventTarget(e) as FocusableElement; if (target && target.isConnected) { focusedNode.current = target; - focusWithoutScrolling(focusedNode.current); + focusElement(focusedNode.current); } else if (activeScope.current) { focusFirstInScope(activeScope.current); } diff --git a/packages/react-aria/test/focus/FocusScope.test.js b/packages/react-aria/test/focus/FocusScope.test.js index 1ef89242006..9b9a7162ece 100644 --- a/packages/react-aria/test/focus/FocusScope.test.js +++ b/packages/react-aria/test/focus/FocusScope.test.js @@ -23,7 +23,7 @@ import {DialogContainer} from '@adobe/react-spectrum/DialogContainer'; import {enableShadowDOM} from 'react-stately/private/flags/flags'; import {FocusScope, useFocusManager} from '../../src/focus/FocusScope'; import {focusScopeTree} from '../../src/focus/FocusScope'; -import {focusWithoutScrolling} from '../../src/utils/focusWithoutScrolling'; +import {focusSafely} from '../../src/interactions/focusSafely'; import {Provider} from '@adobe/react-spectrum/Provider'; import React, {useEffect, useState} from 'react'; import ReactDOM from 'react-dom'; @@ -31,11 +31,11 @@ import {Example as StorybookExample} from '../../stories/focus/FocusScope.storie import {useEvent} from '../../src/utils/useEvent'; import userEvent from '@testing-library/user-event'; -jest.mock('../../src/utils/focusWithoutScrolling', () => { - let actual = jest.requireActual('../../src/utils/focusWithoutScrolling'); +jest.mock('../../src/interactions/focusSafely', () => { + let actual = jest.requireActual('../../src/interactions/focusSafely'); return { ...actual, - focusWithoutScrolling: jest.fn(actual.focusWithoutScrolling) + focusSafely: jest.fn(actual.focusSafely) }; }); @@ -333,13 +333,13 @@ describe('FocusScope', function () { }); expect(document.activeElement).toBe(input2); - focusWithoutScrolling.mockClear(); + focusSafely.mockClear(); act(() => { outside.focus(); }); fireEvent.focusIn(outside); expect(document.activeElement).toBe(input2); - expect(focusWithoutScrolling).toHaveBeenCalledWith(input2); + expect(focusSafely).toHaveBeenCalledWith(input2); }); it('should restore focus to the last focused element in the scope on focus out', async function () { @@ -368,7 +368,7 @@ describe('FocusScope', function () { }); expect(document.activeElement).toBe(input2); - focusWithoutScrolling.mockClear(); + focusSafely.mockClear(); act(() => { input2.blur(); @@ -377,7 +377,7 @@ describe('FocusScope', function () { jest.runAllTimers(); }); expect(document.activeElement).toBe(input2); - expect(focusWithoutScrolling).toHaveBeenCalledWith(input2); + expect(focusSafely).toHaveBeenCalledWith(input2); fireEvent.focusOut(input2); expect(document.activeElement).toBe(input2); }); From 44c452f03a8b05cf14de9e91a34c9d2bd33802a2 Mon Sep 17 00:00:00 2001 From: Rob Snow Date: Mon, 20 Jul 2026 16:24:27 +1000 Subject: [PATCH 3/3] create a test for the goal, not the implementation --- .../test/Modal.browser.test.tsx | 110 ++++++++++++++++++ .../react-aria/test/focus/FocusScope.test.js | 14 --- vitest.browser.config.ts | 23 ++++ 3 files changed, 133 insertions(+), 14 deletions(-) create mode 100644 packages/react-aria-components/test/Modal.browser.test.tsx diff --git a/packages/react-aria-components/test/Modal.browser.test.tsx b/packages/react-aria-components/test/Modal.browser.test.tsx new file mode 100644 index 00000000000..bb721dd5b93 --- /dev/null +++ b/packages/react-aria-components/test/Modal.browser.test.tsx @@ -0,0 +1,110 @@ +/* + * Copyright 2026 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 {Button} from '../src/Button'; +import {commands, page, userEvent} from 'vitest/browser'; +import {Dialog, DialogTrigger} from '../src/Dialog'; +import {expect, it} from 'vitest'; +import {Heading} from '../src/Heading'; +import {Modal, ModalOverlay} from '../src/Modal'; +import React from 'react'; +import {render} from 'vitest-browser-react'; + +declare module 'vitest/browser' { + interface BrowserCommands { + mouseDownOnElement: (selector: string, offsetX?: number, offsetY?: number) => Promise; + mouseUp: () => Promise; + } +} + +const OFFSET_VH = 5; + +function ScrollJumpExample() { + return ( + + + + + + Modal in a scrollable overlay + {Array.from({length: 10}, (_, i) => ( +
+ {i + 1} +
+ ))} + + + + + ); +} + +// mousedown on the backdrop moves focus to in Chrome/Safari; Firefox does not. +// FocusScope containment must restore focus to the Dialog without scrolling, +// otherwise the modal visibly jumps to the top of the screen. +// Uses a trusted press so the native focus move actually happens. This cannot be +// tested in a unit test nor in Chromatic play. +it('does not scroll the modal into view when the backdrop is pressed', async () => { + await render(); + + await userEvent.click(page.getByRole('button', {name: 'Open modal'})); + await expect.element(page.getByRole('dialog')).toBeInTheDocument(); + + let overlay = page.getByTestId('scroll-jump-backdrop').element() as HTMLElement; + let modal = page.getByTestId('scroll-jump-modal').element() as HTMLElement; + + overlay.scrollTop = 0; + let modalTopBefore = Math.round(modal.getBoundingClientRect().top); + expect(overlay.scrollTop).toBe(0); + expect(modalTopBefore).toBeGreaterThan(0); + + // Do not release so we can observe the state + await commands.mouseDownOnElement(page.getByTestId('scroll-jump-backdrop').selector, 5); + + // Wait a couple frames for FocusScope's requestAnimationFrame focus restore to run. + await new Promise(resolve => + requestAnimationFrame(() => requestAnimationFrame(() => resolve(null))) + ); + + // the modal stays at its offset + expect(overlay.scrollTop).toBe(0); + expect(Math.round(modal.getBoundingClientRect().top)).toBe(modalTopBefore); + + await commands.mouseUp(); +}); diff --git a/packages/react-aria/test/focus/FocusScope.test.js b/packages/react-aria/test/focus/FocusScope.test.js index 9b9a7162ece..39dd0d8be39 100644 --- a/packages/react-aria/test/focus/FocusScope.test.js +++ b/packages/react-aria/test/focus/FocusScope.test.js @@ -23,7 +23,6 @@ import {DialogContainer} from '@adobe/react-spectrum/DialogContainer'; import {enableShadowDOM} from 'react-stately/private/flags/flags'; import {FocusScope, useFocusManager} from '../../src/focus/FocusScope'; import {focusScopeTree} from '../../src/focus/FocusScope'; -import {focusSafely} from '../../src/interactions/focusSafely'; import {Provider} from '@adobe/react-spectrum/Provider'; import React, {useEffect, useState} from 'react'; import ReactDOM from 'react-dom'; @@ -31,14 +30,6 @@ import {Example as StorybookExample} from '../../stories/focus/FocusScope.storie import {useEvent} from '../../src/utils/useEvent'; import userEvent from '@testing-library/user-event'; -jest.mock('../../src/interactions/focusSafely', () => { - let actual = jest.requireActual('../../src/interactions/focusSafely'); - return { - ...actual, - focusSafely: jest.fn(actual.focusSafely) - }; -}); - describe('FocusScope', function () { let user; @@ -333,13 +324,11 @@ describe('FocusScope', function () { }); expect(document.activeElement).toBe(input2); - focusSafely.mockClear(); act(() => { outside.focus(); }); fireEvent.focusIn(outside); expect(document.activeElement).toBe(input2); - expect(focusSafely).toHaveBeenCalledWith(input2); }); it('should restore focus to the last focused element in the scope on focus out', async function () { @@ -368,8 +357,6 @@ describe('FocusScope', function () { }); expect(document.activeElement).toBe(input2); - focusSafely.mockClear(); - act(() => { input2.blur(); }); @@ -377,7 +364,6 @@ describe('FocusScope', function () { jest.runAllTimers(); }); expect(document.activeElement).toBe(input2); - expect(focusSafely).toHaveBeenCalledWith(input2); fireEvent.focusOut(input2); expect(document.activeElement).toBe(input2); }); diff --git a/vitest.browser.config.ts b/vitest.browser.config.ts index d250a6b8b1f..1b0e7732da9 100644 --- a/vitest.browser.config.ts +++ b/vitest.browser.config.ts @@ -191,6 +191,10 @@ declare module 'vitest/browser' { ) => Promise; // Commit text that doesn't come from a key press (finalizes an active composition). commitComposition: (text: string) => Promise; + // Placeholder until newer version of library + mouseDownOnElement: (selector: string, offsetX?: number, offsetY?: number) => Promise; + // Same as above + mouseUp: () => Promise; } } @@ -305,6 +309,25 @@ export default defineConfig({ commitComposition: async ({page, context}: any, text) => { const cdp = await getCDP(page, context); await cdp.send('Input.insertText', {text}); + }, + // Once we upgrade to a newer version, we can use the below and delete mouseDownOnElement + // await userEvent.hover(button) + // await userEvent.pointer({ keys: '[MouseLeft>]', target: button }) + // await userEvent.pointer('[/MouseLeft]') + mouseDownOnElement: async ( + {page, iframe}: any, + selector: string, + offsetX: number = 5, + offsetY?: number + ) => { + const box = await iframe.locator(selector).boundingBox(); + const x = box.x + offsetX; + const y = offsetY == null ? box.y + box.height / 2 : box.y + offsetY; + await page.mouse.move(x, y); + await page.mouse.down(); + }, + mouseUp: async ({page}: any) => { + await page.mouse.up(); } } },