From 29ddac296138b0aace4df42447687154256ed245 Mon Sep 17 00:00:00 2001 From: Rob Snow Date: Tue, 21 Jul 2026 06:15:39 +1000 Subject: [PATCH 1/5] feat: close watchers --- .../2026-07-20-close-watchers-overlays.md | 685 ++++++++++++++++++ ...26-07-20-close-watchers-overlays-design.md | 238 ++++++ .../test/CloseWatcher.browser.test.tsx | 178 +++++ .../react-aria/src/overlays/closeWatchers.ts | 116 +++ .../react-aria/src/overlays/useOverlay.ts | 25 +- .../test/overlays/closeWatchers.test.js | 115 +++ .../test/overlays/useOverlay.test.js | 49 ++ 7 files changed, 1403 insertions(+), 3 deletions(-) create mode 100644 docs/superpowers/plans/2026-07-20-close-watchers-overlays.md create mode 100644 docs/superpowers/specs/2026-07-20-close-watchers-overlays-design.md create mode 100644 packages/react-aria-components/test/CloseWatcher.browser.test.tsx create mode 100644 packages/react-aria/src/overlays/closeWatchers.ts create mode 100644 packages/react-aria/test/overlays/closeWatchers.test.js diff --git a/docs/superpowers/plans/2026-07-20-close-watchers-overlays.md b/docs/superpowers/plans/2026-07-20-close-watchers-overlays.md new file mode 100644 index 00000000000..bd128d953e1 --- /dev/null +++ b/docs/superpowers/plans/2026-07-20-close-watchers-overlays.md @@ -0,0 +1,685 @@ +# CloseWatcher Support for Overlays — Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Route overlay dismissal (Escape / native close requests) through the platform `CloseWatcher` API via a singleton pool manager, while keeping the existing focus-bound Escape handler as a fallback. + +**Architecture:** A module-level singleton (`closeWatchers.ts`) owns a pool of native `CloseWatcher` instances, decoupled from any specific overlay. `useOverlay` subscribes on open and unsubscribes on close. When a watcher fires, the manager snapshots the `visibleOverlays` z-order stack and tells the top-most overlay to close. The pool keeps its size equal to the subscriber count, which also recreates a watcher when a controlled consumer ignores `onOpenChange(false)`. + +**Tech Stack:** TypeScript, React, `@react-aria/overlays` (source in `packages/react-aria/src/overlays/`), Jest (unit tests), Vitest + Playwright browser provider (browser tests). + +## Global Constraints + +- **No commits without explicit user confirmation** (project rule). The commit steps below are prepared but must be confirmed by the user before running. +- Source of truth for `@react-aria/overlays` lives in `packages/react-aria/src/overlays/` (the `packages/@react-aria/overlays/src/index.ts` is only a re-export shim). +- `CloseWatcher` is not present in the repo's TypeScript DOM lib (TS 5.8.2); the manager must declare its own minimal types. +- Apache license header (copyright 2026) required at the top of every new source and test file — copy the exact header block from `packages/react-aria-components/test/Dialog.browser.test.tsx`. +- Behavior parity: `isKeyboardDismissDisabled` gates dismissal; `isDismissable` does **not** gate Escape/close-request dismissal. +- When `CloseWatcher` is available, `useOverlay` must NOT `preventDefault` on the Escape keydown (that would suppress the native close request). + +--- + +### Task 1: `closeWatchers` manager module + +**Files:** +- Create: `packages/react-aria/src/overlays/closeWatchers.ts` +- Test: `packages/react-aria/test/overlays/closeWatchers.test.js` + +**Interfaces:** +- Consumes: `visibleOverlays` (exported from `useOverlay.ts` in Task 2). For Task 1's tests, a local test-owned array stands in — see note below. +- Produces: + - `export interface CloseWatcherSubscriber { ref: RefObject; onClose: () => void; }` + - `export function subscribeCloseWatcher(subscriber: CloseWatcherSubscriber): () => void` — registers the subscriber, reconciles the pool, returns an unsubscribe function. + +> **Import-cycle note:** `closeWatchers.ts` imports `visibleOverlays` from `useOverlay.ts`, and `useOverlay.ts` imports `subscribeCloseWatcher` from `closeWatchers.ts`. This cycle is safe because `closeWatchers.ts` only *reads* `visibleOverlays` at call time (inside `notify`), never at module-init time, and `subscribeCloseWatcher` is a hoisted function declaration. Since `useOverlay.ts` does not export `visibleOverlays` until Task 2, Task 1 temporarily declares `visibleOverlays` inside `closeWatchers.ts` and Task 2 moves the export and switches the import. (Both steps are shown.) + +- [ ] **Step 1: Write the failing test** + +Create `packages/react-aria/test/overlays/closeWatchers.test.js`: + +```js +/* + * 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 {subscribeCloseWatcher, visibleOverlays} from '../../src/overlays/closeWatchers'; + +// Minimal fake of the native CloseWatcher. Mirrors the auto-destroy-on-fire +// behavior: firing removes the instance from the live pool. +class FakeCloseWatcher { + static instances = []; + constructor() { + this.onclose = null; + FakeCloseWatcher.instances.push(this); + } + destroy() { + let i = FakeCloseWatcher.instances.indexOf(this); + if (i >= 0) { + FakeCloseWatcher.instances.splice(i, 1); + } + } + // Simulate a native close request delivered to this watcher. + fire() { + this.destroy(); + this.onclose?.(); + } +} + +function makeRef(el = document.createElement('div')) { + return {current: el}; +} + +describe('closeWatchers manager', () => { + beforeEach(() => { + FakeCloseWatcher.instances = []; + window.CloseWatcher = FakeCloseWatcher; + visibleOverlays.length = 0; + }); + + afterEach(() => { + delete window.CloseWatcher; + visibleOverlays.length = 0; + }); + + it('creates one watcher per subscriber and destroys on unsubscribe', () => { + let a = makeRef(); + visibleOverlays.push(a); + let unsub = subscribeCloseWatcher({ref: a, onClose: () => {}}); + expect(FakeCloseWatcher.instances).toHaveLength(1); + unsub(); + expect(FakeCloseWatcher.instances).toHaveLength(0); + }); + + it('closes only the top-most overlay when a watcher fires', () => { + let a = makeRef(); + let b = makeRef(); + visibleOverlays.push(a, b); // b is top-most + let onCloseA = jest.fn(); + let onCloseB = jest.fn(); + subscribeCloseWatcher({ref: a, onClose: onCloseA}); + subscribeCloseWatcher({ref: b, onClose: onCloseB}); + + FakeCloseWatcher.instances[FakeCloseWatcher.instances.length - 1].fire(); + + expect(onCloseB).toHaveBeenCalledTimes(1); + expect(onCloseA).not.toHaveBeenCalled(); + }); + + it('closes n distinct overlays for n grouped synchronous fires', () => { + let a = makeRef(); + let b = makeRef(); + visibleOverlays.push(a, b); + let onCloseA = jest.fn(); + let onCloseB = jest.fn(); + subscribeCloseWatcher({ref: a, onClose: onCloseA}); + subscribeCloseWatcher({ref: b, onClose: onCloseB}); + + // Grouped fire: two watchers fire synchronously before any microtask runs. + // visibleOverlays is NOT mutated between fires (React removal is async). + let live = [...FakeCloseWatcher.instances]; + live[1].fire(); + live[0].fire(); + + expect(onCloseB).toHaveBeenCalledTimes(1); + expect(onCloseA).toHaveBeenCalledTimes(1); + }); + + it('recreates a watcher when a subscriber stays open after firing (controlled-ignore)', () => { + let a = makeRef(); + visibleOverlays.push(a); + subscribeCloseWatcher({ref: a, onClose: () => {}}); // onClose ignores -> a stays in visibleOverlays + + expect(FakeCloseWatcher.instances).toHaveLength(1); + FakeCloseWatcher.instances[0].fire(); + // Subscriber never unsubscribed, so the pool is topped back up to 1. + expect(FakeCloseWatcher.instances).toHaveLength(1); + }); + + it('is a no-op when CloseWatcher is unavailable', () => { + delete window.CloseWatcher; + let a = makeRef(); + visibleOverlays.push(a); + let unsub = subscribeCloseWatcher({ref: a, onClose: () => {}}); + expect(FakeCloseWatcher.instances).toHaveLength(0); + expect(() => unsub()).not.toThrow(); + }); +}); +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `yarn jest packages/react-aria/test/overlays/closeWatchers.test.js` +Expected: FAIL — cannot find module `../../src/overlays/closeWatchers`. + +- [ ] **Step 3: Write the manager** + +Create `packages/react-aria/src/overlays/closeWatchers.ts`: + +```ts +/* + * 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 {RefObject} from '@react-types/shared'; + +// CloseWatcher is not yet in the TypeScript DOM lib (TS 5.8.2), so declare +// the minimal surface we rely on. +interface CloseWatcher { + destroy(): void; + onclose: (() => void) | null; +} +interface CloseWatcherConstructor { + new (): CloseWatcher; +} +declare global { + // eslint-disable-next-line no-var + interface Window { + CloseWatcher?: CloseWatcherConstructor; + } +} + +export interface CloseWatcherSubscriber { + /** The overlay container ref, used to determine z-order via visibleOverlays. */ + ref: RefObject; + /** Called when this overlay should close because it is the top-most. */ + onClose: () => void; +} + +// TEMPORARY for Task 1 only. Task 2 removes this and imports the shared +// array from useOverlay.ts instead. +export const visibleOverlays: RefObject[] = []; + +const subscribers = new Set(); +const watchers: CloseWatcher[] = []; + +// Refs asked to close during the current (synchronous) close request. Excluded +// from the top-most calculation so grouped synchronous fires close distinct +// overlays. Cleared on the next microtask. +const pendingClose = new Set>(); +let pendingClear = false; + +function hasCloseWatcher(): boolean { + return typeof window !== 'undefined' && typeof window.CloseWatcher === 'function'; +} + +function createWatcher(): void { + let watcher = new window.CloseWatcher!(); + watcher.onclose = () => { + let i = watchers.indexOf(watcher); + if (i >= 0) { + watchers.splice(i, 1); + } + notify(); + reconcile(); + }; + watchers.push(watcher); +} + +// Invariant: watchers.length === subscribers.size. +function reconcile(): void { + if (!hasCloseWatcher()) { + return; + } + while (watchers.length < subscribers.size) { + createWatcher(); + } + while (watchers.length > subscribers.size) { + watchers.pop()!.destroy(); + } +} + +function notify(): void { + // Snapshot at the start so mutations (closing removes from visibleOverlays) + // and synchronous unsubscribes do not shift the top-most check mid-pass. + let overlays = visibleOverlays.filter(o => !pendingClose.has(o)); + let topMost = overlays[overlays.length - 1]; + if (!topMost) { + return; + } + + pendingClose.add(topMost); + if (!pendingClear) { + pendingClear = true; + queueMicrotask(() => { + pendingClose.clear(); + pendingClear = false; + }); + } + + // Tell all subscribers; only the top-most one closes. + for (let subscriber of [...subscribers]) { + if (subscriber.ref === topMost) { + subscriber.onClose(); + } + } +} + +export function subscribeCloseWatcher(subscriber: CloseWatcherSubscriber): () => void { + if (!hasCloseWatcher()) { + return () => {}; + } + subscribers.add(subscriber); + reconcile(); + return () => { + subscribers.delete(subscriber); + reconcile(); + }; +} +``` + +- [ ] **Step 4: Run test to verify it passes** + +Run: `yarn jest packages/react-aria/test/overlays/closeWatchers.test.js` +Expected: PASS (all 5 tests). + +- [ ] **Step 5: Commit** (only after user confirmation) + +```bash +git add packages/react-aria/src/overlays/closeWatchers.ts packages/react-aria/test/overlays/closeWatchers.test.js +git commit -m "feat: add CloseWatcher pool manager for overlays" +``` + +--- + +### Task 2: `useOverlay` integration + shared `visibleOverlays` + +**Files:** +- Modify: `packages/react-aria/src/overlays/useOverlay.ts` (line 61 export; lines 128-139 keyboard fallback; add subscription effect) +- Modify: `packages/react-aria/src/overlays/closeWatchers.ts` (remove temporary `visibleOverlays`, import from `useOverlay`) +- Test: `packages/react-aria/test/overlays/useOverlay.test.js` (add CloseWatcher-path tests) + +**Interfaces:** +- Consumes: `subscribeCloseWatcher`, `CloseWatcherSubscriber` from Task 1. +- Produces: `export const visibleOverlays` from `useOverlay.ts` (moved out of Task 1's temporary declaration). + +- [ ] **Step 1: Write the failing test** + +Add to `packages/react-aria/test/overlays/useOverlay.test.js` (after the existing Escape tests, before the final closing `});` of the file — the two blocks below are top-level `it`s inside the `describe('useOverlay', ...)`): + +```js + describe('with CloseWatcher', () => { + class FakeCloseWatcher { + static instances = []; + constructor() { + this.onclose = null; + FakeCloseWatcher.instances.push(this); + } + destroy() { + let i = FakeCloseWatcher.instances.indexOf(this); + if (i >= 0) { + FakeCloseWatcher.instances.splice(i, 1); + } + } + fire() { + this.destroy(); + this.onclose?.(); + } + } + + beforeEach(() => { + FakeCloseWatcher.instances = []; + window.CloseWatcher = FakeCloseWatcher; + }); + afterEach(() => { + delete window.CloseWatcher; + }); + + it('closes via a fired close watcher instead of the keydown handler', function () { + let onClose = jest.fn(); + let res = render(); + let el = res.getByTestId('test'); + + // The keydown handler must NOT be registered when CloseWatcher exists, + // so Escape keydown alone does nothing. + fireEvent.keyDown(el, {key: 'Escape'}); + expect(onClose).not.toHaveBeenCalled(); + + // Firing the watcher (as the platform would) closes the overlay. + FakeCloseWatcher.instances[FakeCloseWatcher.instances.length - 1].fire(); + expect(onClose).toHaveBeenCalledTimes(1); + }); + + it('does not subscribe when isKeyboardDismissDisabled', function () { + let onClose = jest.fn(); + render(); + expect(FakeCloseWatcher.instances).toHaveLength(0); + }); + }); +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `yarn jest packages/react-aria/test/overlays/useOverlay.test.js -t "with CloseWatcher"` +Expected: FAIL — `onClose` is called by the still-present keydown handler (first test), and/or a watcher is created path missing. + +- [ ] **Step 3: Export `visibleOverlays` and wire the subscription in `useOverlay.ts`** + +In `packages/react-aria/src/overlays/useOverlay.ts`: + +Change the import line to also pull in the manager and the `useEffectEvent` +utility (keep imports sorted by their first binding name): + +```ts +import {subscribeCloseWatcher} from './closeWatchers'; +import {useEffect, useRef} from 'react'; +import {useEffectEvent} from '../utils/useEffectEvent'; +``` + +Export `visibleOverlays` (line 61): + +```ts +export const visibleOverlays: RefObject[] = []; +``` + +Add, immediately after `onHide` is defined (after line 98), the CloseWatcher +availability flag, a stable close-request callback, and the subscription +effect. **Do not read or write a ref during render** — use `useEffectEvent` +to keep `onClose` fresh (it returns a stable callback that always invokes the +latest `onClose`, updating its ref inside an insertion effect), and build the +subscriber object *inside* the effect: + +```ts + let hasCloseWatcher = typeof window !== 'undefined' && typeof window.CloseWatcher === 'function'; + + // A stable callback that always calls the latest onClose, so the close watcher + // subscription does not need to re-run (and churn the pool) on every re-render. + let onCloseRequest = useEffectEvent(() => onClose?.()); + + useEffect(() => { + if (!isOpen || isKeyboardDismissDisabled || !hasCloseWatcher) { + return; + } + return subscribeCloseWatcher({ref, onClose: onCloseRequest}); + // onCloseRequest is a stable useEffectEvent and must be omitted from deps. + }, [isOpen, isKeyboardDismissDisabled, hasCloseWatcher, ref]); +``` + +Replace the keyboard handler (lines 128-139) so the Escape shortcut is only registered as a fallback when `CloseWatcher` is unavailable (registering it when available would `preventDefault` and suppress the native close request): + +```ts + // Fallback Escape handling for browsers without CloseWatcher support. + // When CloseWatcher is available, Escape is handled via the subscription + // above and we must NOT preventDefault here or the native close request + // would be suppressed. + let {keyboardProps} = useKeyboard(hasCloseWatcher ? {} : { + shortcuts: { + Escape: () => { + if (!isKeyboardDismissDisabled) { + onHide(); + return; + } + return false; + } + } + }); +``` + +- [ ] **Step 4: Remove the temporary `visibleOverlays` from `closeWatchers.ts`** + +In `packages/react-aria/src/overlays/closeWatchers.ts`, delete the temporary block: + +```ts +// TEMPORARY for Task 1 only. Task 2 removes this and imports the shared +// array from useOverlay.ts instead. +export const visibleOverlays: RefObject[] = []; +``` + +and replace it with an import near the top (after the existing `import {RefObject} ...`): + +```ts +import {visibleOverlays} from './useOverlay'; +``` + +- [ ] **Step 5: Update Task 1's manager test import** + +In `packages/react-aria/test/overlays/closeWatchers.test.js`, change the import so `visibleOverlays` comes from `useOverlay` (its new home): + +```js +import {subscribeCloseWatcher} from '../../src/overlays/closeWatchers'; +import {visibleOverlays} from '../../src/overlays/useOverlay'; +``` + +- [ ] **Step 6: Run all affected unit tests** + +Run: `yarn jest packages/react-aria/test/overlays/useOverlay.test.js packages/react-aria/test/overlays/closeWatchers.test.js` +Expected: PASS. The pre-existing jsdom Escape tests still pass because jsdom has no `window.CloseWatcher`, so the fallback keydown handler stays active for them. + +- [ ] **Step 7: Run the broader overlay suite for regressions** + +Run: `yarn jest packages/react-aria/test/overlays/` +Expected: PASS (no regressions in `useModalOverlay`, `usePopover`, etc.). + +- [ ] **Step 8: Commit** (only after user confirmation) + +```bash +git add packages/react-aria/src/overlays/useOverlay.ts packages/react-aria/src/overlays/closeWatchers.ts packages/react-aria/test/overlays/ +git commit -m "feat: dismiss overlays via CloseWatcher, keep keydown fallback" +``` + +--- + +### Task 3: Browser tests driving native close requests + +**Files:** +- Create: `packages/react-aria-components/test/CloseWatcher.browser.test.tsx` + +**Interfaces:** +- Consumes: RAC `DialogTrigger`, `Modal`, `ModalOverlay`, `Popover`, `Dialog`, `Button`, `Heading`. Real Escape via `userEvent.keyboard('{Escape}')`. + +- [ ] **Step 1: Write the browser tests** + +Create `packages/react-aria-components/test/CloseWatcher.browser.test.tsx`: + +```tsx +/* + * 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 {Dialog, DialogTrigger} from '../src/Dialog'; +import {describe, expect, it} from 'vitest'; +import {Heading} from '../src/Heading'; +import {Modal, ModalOverlay} from '../src/Modal'; +import {Popover} from '../src/Popover'; +import React from 'react'; +import {render} from 'vitest-browser-react'; +import {userEvent} from 'vitest/browser'; + +// CloseWatcher is required for these tests. Skip on browsers that lack it +// rather than exercising the keydown fallback (covered by the jsdom suite). +let supportsCloseWatcher = typeof window !== 'undefined' && 'CloseWatcher' in window; +let describeCloseWatcher = supportsCloseWatcher ? describe : describe.skip; + +async function press(key: string) { + await userEvent.keyboard(`{${key}}`); +} + +describeCloseWatcher('CloseWatcher dismissal', () => { + it('closes a single modal on Escape', async () => { + let {getByRole, queryByRole} = await render( + + + + + Hello + + + + + ); + expect(getByRole('dialog')).toBeTruthy(); + await press('Escape'); + expect(queryByRole('dialog')).toBeNull(); + }); + + it('does not close when isKeyboardDismissDisabled', async () => { + let {getByRole} = await render( + + + + + + Locked + + + + + + ); + expect(getByRole('dialog')).toBeTruthy(); + await press('Escape'); + expect(getByRole('dialog')).toBeTruthy(); + }); + + it('closes the top-most overlay first for nested modals', async () => { + // Outer opens on mount (the single "free" watcher). The inner modal is + // opened by a click so it gets its own transient-user-activation watcher + // and is NOT grouped with the outer (grouping would close both at once). + let {getByRole, queryAllByRole} = await render( + + + + + Outer + + + + + Inner + + + + + + + + ); + expect(queryAllByRole('dialog')).toHaveLength(1); + await userEvent.click(getByRole('button', {name: 'Open inner'})); + expect(queryAllByRole('dialog')).toHaveLength(2); + await press('Escape'); + // Inner closed, outer remains. + expect(queryAllByRole('dialog')).toHaveLength(1); + expect(getByRole('dialog')).toBeTruthy(); + await press('Escape'); + // Outer closes on the replacement/next watcher. + expect(queryAllByRole('dialog')).toHaveLength(0); + }); + + it('closes an inner popover before the modal', async () => { + // Modal opens on mount (free watcher); popover opened via click so its + // watcher is independent (not grouped) and receives the first close request. + let {getByRole, getByText, queryByRole, queryByText} = await render( + + + + + Modal + + + + + + + + + + + + ); + await userEvent.click(getByRole('button', {name: 'Open popover'})); + expect(getByText('Popover content')).toBeTruthy(); + await press('Escape'); + // Popover (most-recent watcher) closes; modal stays. + expect(queryByText('Popover content')).toBeNull(); + expect(getByRole('dialog')).toBeTruthy(); + await press('Escape'); + // Modal closes on the next close request. + expect(queryByRole('dialog')).toBeNull(); + }); + + it('recreates a watcher when a controlled parent ignores onOpenChange', async () => { + let closeCount = 0; + function Controlled() { + // Always open; ignore onOpenChange(false) so the modal stays open. + return ( + { + if (!isOpen) { + closeCount++; + } + }}> + + + + Sticky + + + + + ); + } + let {getByRole} = await render(); + expect(getByRole('dialog')).toBeTruthy(); + + await press('Escape'); + expect(closeCount).toBe(1); + expect(getByRole('dialog')).toBeTruthy(); + + // Second Escape only fires if a replacement watcher was created. + await press('Escape'); + expect(closeCount).toBe(2); + expect(getByRole('dialog')).toBeTruthy(); + }); +}); +``` + +- [ ] **Step 2: Run the browser tests** + +Run: `yarn test:browser packages/react-aria-components/test/CloseWatcher.browser.test.tsx` +Expected: PASS across chromium/firefox/webkit (all support CloseWatcher). If a runner instance lacks CloseWatcher, the whole suite is skipped (not failed). + +> **If any test fails on unmount timing** (an overlay asserted closed is still momentarily in the DOM), verify the actual DOM with the browser tools first, and treat any real failure with the systematic-debugging skill rather than loosening the assertion (e.g. do not swap a strict `queryByRole(...).toBeNull()` for a weaker check without understanding why). + +- [ ] **Step 3: Commit** (only after user confirmation) + +```bash +git add packages/react-aria-components/test/CloseWatcher.browser.test.tsx +git commit -m "test: add browser tests for CloseWatcher overlay dismissal" +``` + +--- + +## Notes on spec edge cases (for the implementer) + +These are validated or documented, not separate tasks: + +- **Free watcher + grouping** — covered by Task 1's "n grouped fires" unit test and Task 3's nested-modal test. +- **Only the most-recent watcher receives events** — covered by Task 3's nested + popover tests (top-most closes first). +- **`preventDefault` on Escape suppresses the close request** — this is why component-specific Escape handlers (combobox, menu, etc.) still compose. Not changed by this work; note it in code comments only. +- **`cancel` only fires with transient activation / once per activation** — the reason an ignored `onOpenChange(false)` can't be prevented early; handled by watcher recreation (Task 1 unit test + Task 3 controlled test). No `cancel`/confirmation flow is implemented (out of scope). diff --git a/docs/superpowers/specs/2026-07-20-close-watchers-overlays-design.md b/docs/superpowers/specs/2026-07-20-close-watchers-overlays-design.md new file mode 100644 index 00000000000..63c6d2dd460 --- /dev/null +++ b/docs/superpowers/specs/2026-07-20-close-watchers-overlays-design.md @@ -0,0 +1,238 @@ +# CloseWatcher support for overlays + +**Date:** 2026-07-20 +**Status:** Approved design, pending implementation plan + +## Problem + +Overlays (dialogs, modals, popovers, menus) currently dismiss on the Escape +key via a React keyboard handler bound to the overlay element +([`useOverlay.ts:128-139`](../../../packages/react-aria/src/overlays/useOverlay.ts)). +This handler only fires when focus is inside the overlay, and it does not +participate in the platform's native "close request" mechanism (Escape on +desktop, the Android back gesture, etc.). + +We want overlays to integrate with the native +[`CloseWatcher`](https://html.spec.whatwg.org/multipage/interaction.html#close-requests-and-close-watchers) +API so that dismissal is driven by platform close requests rather than a +focus-bound key handler. + +## Goals + +- Route overlay dismissal through native `CloseWatcher` instances when the API + is available. +- Preserve the existing "only the top-most overlay closes" semantics, keyed off + the existing `visibleOverlays` stack. +- Handle the controlled-open case where the consumer ignores + `onOpenChange(false)` and the overlay stays open even though its + `CloseWatcher` was consumed. +- Fall back to the existing focus-bound Escape handler on browsers without + `CloseWatcher`. +- Add browser tests that trigger real native close requests via Escape. + +## Non-goals + +- Changing component-specific Escape handling that lives outside `useOverlay` + (combobox, menu, searchfield, calendar, etc.). Those keep their own handlers. +- Implementing a `cancel`/confirmation flow (see "Known limitations"). + +## Scope + +**All `useOverlay` consumers** use the `CloseWatcher` when the API is available +— not just modals. This avoids introducing a new prop and reduces the number of +distinct dismissal code paths to test. + +Accepted behavior change: non-modal popovers and menus will now close on a +close request regardless of where focus is, rather than only when focus is +inside the overlay (today's focus-bound behavior). + +## Architecture + +### 1. `CloseWatcher` manager (singleton pool) + +A module-level singleton (`packages/react-aria/src/overlays/closeWatchers.ts`) +owns a **pool** of native `CloseWatcher` instances. Watchers are **not** tied to +any specific overlay — they are a fungible pool whose only job is to keep the +platform's close-request machinery armed. The overlays keep their own stack +(`visibleOverlays`) and independently decide which one closes; the pool order +need not match the overlay stack order. + +State: + +- `subscribers: Set` — one entry per open, dismissable overlay. +- `watchers: CloseWatcher[]` — the pool. +- `pendingClose: Set` — refs asked to close during the current + close request; cleared on the next microtask (see grouping, below). + +A `Subscriber` is a stable object created once per `useOverlay` call. It reads +the latest `ref`/`onClose` from refs so the subscription never has to churn when +the consumer re-renders (consistent with the project preference to fix +re-renders via stored values rather than re-subscribing). + +Public API: + +- `subscribe(subscriber): () => void` — adds the subscriber, calls + `reconcile()`, and returns an unsubscribe function that removes the subscriber + and calls `reconcile()` again. + +Internal: + +- `reconcile()` — enforces the single invariant + **`watchers.length === subscribers.size`**, creating or `destroy()`-ing + watchers to match. Each created watcher's `onclose` handler removes itself + from the pool, calls `notify()`, then calls `reconcile()`. This one invariant + covers every lifecycle transition: + - subscribe → pool grows by one. + - unsubscribe (normal close) → pool shrinks by one. + - watcher fires + overlay closes normally → subscriber removed *and* watcher + consumed → still balanced. + - watcher fires + controlled parent ignores `onOpenChange(false)` → subscriber + remains but watcher was consumed → pool is short one → `reconcile()` creates + a replacement so the still-open overlay stays dismissable. + +- `notify()` — invoked by a fired watcher's `onclose`: + 1. Snapshot the subscriber set (`[...subscribers]`) so a synchronous + unsubscribe during close does not corrupt iteration. + 2. Determine the top-most overlay from `visibleOverlays` **minus** + `pendingClose` (so grouped synchronous fires target distinct overlays). + 3. Broadcast to every snapshot subscriber, passing the top-most ref; a + subscriber closes only if its own ref is that top-most ref + ("tell all, top-most closes"). + 4. Add the closed ref to `pendingClose` and schedule a microtask to clear + `pendingClose`. + +`visibleOverlays` is shared between `useOverlay` and the manager. It stays +defined in `useOverlay.ts` (which owns pushing/splicing it on mount/unmount) and +is exported so `closeWatchers.ts` can import and snapshot it inside `notify()`. +The manager references it only at call time, so the `useOverlay` ↔ +`closeWatchers` import cycle is safe (no module-init-time dependency). + +### 2. `useOverlay` integration + +Replace the focus-bound Escape `useKeyboard` shortcut with a subscription when +`CloseWatcher` is available: + +```ts +useEffect(() => { + if (!isOpen || isKeyboardDismissDisabled || !window.CloseWatcher) { + return; + } + return closeWatchers.subscribe(subscriber); // subscriber reads latest ref/onClose +}, [isOpen, isKeyboardDismissDisabled]); +``` + +- **Subscription gating:** subscribe only when `isOpen` and not + `isKeyboardDismissDisabled`. `isDismissable` does **not** gate subscription — + matching today's behavior where a default modal (`isDismissable=false`) still + closes on Escape. +- **Top-most check on close:** the subscriber's close callback reuses the + existing `onHide` semantics — it calls `onClose` only when its ref is the + top-most visible overlay. +- **Fallback:** when `window.CloseWatcher` is undefined, the subscription effect + is a no-op and `useOverlay` keeps the existing `useKeyboard` Escape handler. + When `CloseWatcher` *is* defined, the keyboard Escape handler is skipped to + avoid double-handling. + +### 3. Grouping — "close n" behavior + +The spec allows exactly one "free" `CloseWatcher` without transient user +activation; additional watchers created without activation are **grouped** with +the most-recently-created one, so a single close request fires every watcher in +the group synchronously. This happens for programmatically-opened nested +overlays (no user gesture between opens). + +Decision: **do not coalesce.** Each grouped watcher fires its own `notify()`, +and `n` grouped fires close `n` distinct overlays, top-down. + +Because grouped `onclose` events fire synchronously but React removes an overlay +from `visibleOverlays` asynchronously (effect cleanup), two back-to-back +`notify()` calls would otherwise both see the same top-most overlay and +double-close it. The `pendingClose` set (above) excludes already-asked refs so +each successive `notify()` targets the next overlay down: + +- Fire 1 → top-most = B → close B, mark B. +- Fire 2 → top-most = A (B excluded) → close A, mark A. +- `pendingClose` clears on the next microtask. + +## Data flow + +Open (with `CloseWatcher` support): + +``` +overlay opens → useOverlay effect → closeWatchers.subscribe(subscriber) + → reconcile() → new CloseWatcher pushed to pool +``` + +Close request (Escape / back gesture): + +``` +platform close request → most-recent watcher.onclose + → remove watcher from pool → notify() + → snapshot subscribers; topmost = last(visibleOverlays \ pendingClose) + → broadcast; subscriber whose ref === topmost calls onClose + → mark topmost in pendingClose (clear next microtask) + → reconcile() → if a still-open subscriber lost its watcher, create replacement +``` + +Close (normal): + +``` +overlay closes → useOverlay effect cleanup → unsubscribe() + → reconcile() → destroy() one pooled watcher +``` + +## Error handling / edge cases + +From the [close-requests spec](https://html.spec.whatwg.org/multipage/interaction.html#close-requests-and-close-watchers): + +1. **Free watcher + grouping.** Only one watcher is allowed without transient + user activation; extras group. Handled by the "close n" mechanism + + `pendingClose`. Covered by a nested-overlay browser test. +2. **Only the most-recently-created active watcher receives events.** Maps + naturally to "top-most overlay closes" because overlays subscribe in open + order. +3. **`preventDefault` on the Escape keydown suppresses the close request.** + Components with their own Escape handlers (combobox, menu, etc.) that + `preventDefault` will suppress the overlay close request — so they compose + correctly. Verified empirically by a browser test. +4. **`cancel` event only fires with transient user activation, and only once + per activation.** This is why an ignored `onOpenChange(false)` cannot be + prevented early — the watcher is already consumed. Handled by recreating a + replacement watcher via `reconcile()`. +5. **Watcher lifecycle.** A watcher deactivates after a `close` event, + `destroy()`, or an aborted signal. The pool removes fired watchers and + `destroy()`s surplus ones. + +## Known limitations + +- No `cancel`/confirmation flow is implemented. Because `cancel` only fires with + transient user activation and only once per activation, a reliable + "are you sure?" prompt is out of scope for this change. + +## Testing + +New `Modal.browser.test.tsx` (Vitest + Playwright browser provider, +`packages/react-aria-components/test/`, `*.browser.test.tsx`) that dispatches +**real** Escape keydowns so native `CloseWatcher` instances actually fire: + +1. **Single overlay** — Escape closes it. +2. **`isKeyboardDismissDisabled`** — Escape does nothing. +3. **Nested modals** (`InertTest`-style) — first Escape closes the top overlay + only; second Escape closes the next — top-most ordering. +4. **Modal + DateRangePicker popover** (`DateRangePickerInsideModalStory`) — + with the picker open, Escape closes the picker and the modal stays open; a + second Escape closes the modal. +5. **Controlled overlay ignoring `onOpenChange(false)`** — Escape fires, the + parent ignores it, the overlay stays open, and a *second* Escape still fires + (proving the replacement watcher was created). + +Fallback (no `CloseWatcher`) continues to be covered by the existing +non-browser Escape tests. + +## Files touched + +- `packages/react-aria/src/overlays/closeWatchers.ts` (new) — the manager. +- `packages/react-aria/src/overlays/useOverlay.ts` — subscribe/unsubscribe; + keep the keyboard handler as fallback; share `visibleOverlays`. +- `packages/react-aria-components/test/Modal.browser.test.tsx` (new) — browser + tests. diff --git a/packages/react-aria-components/test/CloseWatcher.browser.test.tsx b/packages/react-aria-components/test/CloseWatcher.browser.test.tsx new file mode 100644 index 00000000000..9a62483fc2c --- /dev/null +++ b/packages/react-aria-components/test/CloseWatcher.browser.test.tsx @@ -0,0 +1,178 @@ +/* + * 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 {describe, expect, it} from 'vitest'; +import {Dialog, DialogTrigger} from '../src/Dialog'; +import {Heading} from '../src/Heading'; +import {Modal, ModalOverlay} from '../src/Modal'; +import {Popover} from '../src/Popover'; +import React from 'react'; +import {render} from 'vitest-browser-react'; +import {userEvent} from 'vitest/browser'; + +// CloseWatcher is required for these tests. Skip on browsers that lack it +// rather than exercising the keydown fallback (covered by the jsdom suite). +let supportsCloseWatcher = typeof window.CloseWatcher === 'function'; +let describeCloseWatcher = supportsCloseWatcher ? describe : describe.skip; + +async function press(key: string) { + await userEvent.keyboard(`{${key}}`); +} + +describeCloseWatcher('CloseWatcher dismissal', () => { + it('closes a single modal on Escape', async () => { + // vitest-browser-react's render() returns Locator-based selectors + // (getByRole etc.), not testing-library's query*/queryAll* helpers. + // Presence/absence is asserted via the auto-retrying `expect.element(...)` + // matcher so we don't race the modal's exit animation/unmount. + let screen = await render( + + + + + Hello + + + + + ); + await expect.element(screen.getByRole('dialog')).toBeInTheDocument(); + await press('Escape'); + await expect.element(screen.getByRole('dialog')).not.toBeInTheDocument(); + }); + + it('does not close when isKeyboardDismissDisabled', async () => { + let screen = await render( + + + + + + Locked + + + + + + ); + await expect.element(screen.getByRole('dialog')).toBeInTheDocument(); + await press('Escape'); + await expect.element(screen.getByRole('dialog')).toBeInTheDocument(); + }); + + it('closes the top-most overlay first for nested modals', async () => { + // Outer opens on mount (the single "free" watcher). The inner modal is + // opened by a click so it gets its own transient-user-activation watcher + // and is NOT grouped with the outer (grouping would close both at once). + let screen = await render( + + + + + Outer + + + + + + Inner + + + + + + + + + ); + // Locator has no queryAllByRole equivalent; count via elements(), and poll + // rather than reading synchronously so exit-animation unmounts aren't missed. + await expect.poll(() => screen.getByRole('dialog').elements().length).toBe(1); + await userEvent.click(screen.getByRole('button', {name: 'Open inner'})); + await expect.poll(() => screen.getByRole('dialog').elements().length).toBe(2); + await press('Escape'); + // Inner closed, outer remains. + await expect.poll(() => screen.getByRole('dialog').elements().length).toBe(1); + await expect.element(screen.getByRole('dialog')).toBeInTheDocument(); + await press('Escape'); + // Outer closes on the replacement/next watcher. + await expect.poll(() => screen.getByRole('dialog').elements().length).toBe(0); + }); + + it('closes an inner popover before the modal', async () => { + // Modal opens on mount (free watcher); popover opened via click so its + // watcher is independent (not grouped) and receives the first close request. + let screen = await render( + + + + + Modal + + + + + + + + + + + + ); + await userEvent.click(screen.getByRole('button', {name: 'Open popover'})); + await expect.element(screen.getByText('Popover content')).toBeInTheDocument(); + await press('Escape'); + // Popover (most-recent watcher) closes; modal stays. + await expect.element(screen.getByText('Popover content')).not.toBeInTheDocument(); + await expect.element(screen.getByRole('dialog')).toBeInTheDocument(); + await press('Escape'); + // Modal closes on the next close request. + await expect.element(screen.getByRole('dialog')).not.toBeInTheDocument(); + }); + + it('recreates a watcher when a controlled parent ignores onOpenChange', async () => { + let closeCount = 0; + function Controlled() { + // Always open; ignore onOpenChange(false) so the modal stays open. + return ( + { + if (!isOpen) { + closeCount++; + } + }}> + + + + Sticky + + + + + ); + } + let screen = await render(); + await expect.element(screen.getByRole('dialog')).toBeInTheDocument(); + + await press('Escape'); + await expect.poll(() => closeCount).toBe(1); + await expect.element(screen.getByRole('dialog')).toBeInTheDocument(); + + // Second Escape only fires if a replacement watcher was created. + await press('Escape'); + await expect.poll(() => closeCount).toBe(2); + await expect.element(screen.getByRole('dialog')).toBeInTheDocument(); + }); +}); diff --git a/packages/react-aria/src/overlays/closeWatchers.ts b/packages/react-aria/src/overlays/closeWatchers.ts new file mode 100644 index 00000000000..647b3fb8c9c --- /dev/null +++ b/packages/react-aria/src/overlays/closeWatchers.ts @@ -0,0 +1,116 @@ +/* + * 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 {RefObject} from '@react-types/shared'; +import {visibleOverlays} from './useOverlay'; + +// CloseWatcher is not yet in the TypeScript DOM lib (TS 5.8.2), so declare +// the minimal surface we rely on. +interface CloseWatcher { + destroy(): void; + onclose: (() => void) | null; +} +interface CloseWatcherConstructor { + new (): CloseWatcher; +} +declare global { + interface Window { + CloseWatcher?: CloseWatcherConstructor; + } +} + +export interface CloseWatcherSubscriber { + /** The overlay container ref, used to determine z-order via visibleOverlays. */ + ref: RefObject; + /** Called when this overlay should close because it is the top-most. */ + onClose: () => void; +} + +const subscribers = new Set(); +const watchers: CloseWatcher[] = []; + +// Refs asked to close during the current (synchronous) close request. Excluded +// from the top-most calculation so grouped synchronous fires close distinct +// overlays. Cleared on the next microtask. +const pendingClose = new Set>(); +let pendingClear = false; + +function hasCloseWatcher(): boolean { + return typeof window !== 'undefined' && typeof window.CloseWatcher === 'function'; +} + +function createWatcher(): void { + let watcher = new window.CloseWatcher!(); + watcher.onclose = () => { + let i = watchers.indexOf(watcher); + if (i >= 0) { + watchers.splice(i, 1); + } + try { + notify(); + } finally { + reconcile(); + } + }; + watchers.push(watcher); +} + +// Invariant: watchers.length === subscribers.size. +function reconcile(): void { + if (!hasCloseWatcher()) { + return; + } + while (watchers.length < subscribers.size) { + createWatcher(); + } + while (watchers.length > subscribers.size) { + watchers.pop()!.destroy(); + } +} + +function notify(): void { + // Snapshot at the start so mutations (closing removes from visibleOverlays) + // and synchronous unsubscribes do not shift the top-most check mid-pass. + let overlays = visibleOverlays.filter(o => !pendingClose.has(o)); + let topMost = overlays[overlays.length - 1]; + if (!topMost) { + return; + } + + pendingClose.add(topMost); + if (!pendingClear) { + pendingClear = true; + queueMicrotask(() => { + pendingClose.clear(); + pendingClear = false; + }); + } + + // Tell all subscribers; only the top-most one closes. + for (let subscriber of [...subscribers]) { + if (subscriber.ref === topMost) { + subscriber.onClose(); + } + } +} + +export function subscribeCloseWatcher(subscriber: CloseWatcherSubscriber): () => void { + if (!hasCloseWatcher()) { + return () => {}; + } + subscribers.add(subscriber); + reconcile(); + return () => { + subscribers.delete(subscriber); + reconcile(); + }; +} diff --git a/packages/react-aria/src/overlays/useOverlay.ts b/packages/react-aria/src/overlays/useOverlay.ts index 1f2f476f4f8..92ad6e40e72 100644 --- a/packages/react-aria/src/overlays/useOverlay.ts +++ b/packages/react-aria/src/overlays/useOverlay.ts @@ -13,7 +13,9 @@ import {DOMAttributes, RefObject} from '@react-types/shared'; import {getEventTarget} from '../utils/shadowdom/DOMFunctions'; import {isElementInChildOfActiveScope} from '../focus/FocusScope'; +import {subscribeCloseWatcher} from './closeWatchers'; import {useEffect, useRef} from 'react'; +import {useEffectEvent} from '../utils/useEffectEvent'; import {useFocusWithin} from '../interactions/useFocusWithin'; import {useInteractOutside} from '../interactions/useInteractOutside'; import {useKeyboard} from '../interactions/useKeyboard'; @@ -58,7 +60,7 @@ export interface OverlayAria { underlayProps: DOMAttributes; } -const visibleOverlays: RefObject[] = []; +export const visibleOverlays: RefObject[] = []; /** * Provides the behavior for overlays such as dialogs, popovers, and menus. @@ -97,6 +99,20 @@ export function useOverlay(props: AriaOverlayProps, ref: RefObject onClose?.()); + + useEffect(() => { + if (!isOpen || isKeyboardDismissDisabled || !hasCloseWatcher) { + return; + } + return subscribeCloseWatcher({ref, onClose: onCloseRequest}); + // onCloseRequest is a stable useEffectEvent and must be omitted from deps. + }, [isOpen, isKeyboardDismissDisabled, hasCloseWatcher, ref]); + let onInteractOutsideStart = (e: PointerEvent) => { const topMostOverlay = visibleOverlays[visibleOverlays.length - 1]; lastVisibleOverlay.current = topMostOverlay; @@ -125,8 +141,11 @@ export function useOverlay(props: AriaOverlayProps, ref: RefObject { if (!isKeyboardDismissDisabled) { diff --git a/packages/react-aria/test/overlays/closeWatchers.test.js b/packages/react-aria/test/overlays/closeWatchers.test.js new file mode 100644 index 00000000000..913aff0a5fc --- /dev/null +++ b/packages/react-aria/test/overlays/closeWatchers.test.js @@ -0,0 +1,115 @@ +/* + * 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 {subscribeCloseWatcher} from '../../src/overlays/closeWatchers'; +import {visibleOverlays} from '../../src/overlays/useOverlay'; + +// Minimal fake of the native CloseWatcher. Mirrors the auto-destroy-on-fire +// behavior: firing removes the instance from the live pool. +class FakeCloseWatcher { + static instances = []; + constructor() { + this.onclose = null; + FakeCloseWatcher.instances.push(this); + } + destroy() { + let i = FakeCloseWatcher.instances.indexOf(this); + if (i >= 0) { + FakeCloseWatcher.instances.splice(i, 1); + } + } + // Simulate a native close request delivered to this watcher. + fire() { + this.destroy(); + this.onclose?.(); + } +} + +function makeRef(el = document.createElement('div')) { + return {current: el}; +} + +describe('closeWatchers manager', () => { + beforeEach(() => { + FakeCloseWatcher.instances = []; + window.CloseWatcher = FakeCloseWatcher; + visibleOverlays.length = 0; + }); + + afterEach(() => { + delete window.CloseWatcher; + visibleOverlays.length = 0; + }); + + it('creates one watcher per subscriber and destroys on unsubscribe', () => { + let a = makeRef(); + visibleOverlays.push(a); + let unsub = subscribeCloseWatcher({ref: a, onClose: () => {}}); + expect(FakeCloseWatcher.instances).toHaveLength(1); + unsub(); + expect(FakeCloseWatcher.instances).toHaveLength(0); + }); + + it('closes only the top-most overlay when a watcher fires', () => { + let a = makeRef(); + let b = makeRef(); + visibleOverlays.push(a, b); // b is top-most + let onCloseA = jest.fn(); + let onCloseB = jest.fn(); + subscribeCloseWatcher({ref: a, onClose: onCloseA}); + subscribeCloseWatcher({ref: b, onClose: onCloseB}); + + FakeCloseWatcher.instances[FakeCloseWatcher.instances.length - 1].fire(); + + expect(onCloseB).toHaveBeenCalledTimes(1); + expect(onCloseA).not.toHaveBeenCalled(); + }); + + it('closes n distinct overlays for n grouped synchronous fires', () => { + let a = makeRef(); + let b = makeRef(); + visibleOverlays.push(a, b); + let onCloseA = jest.fn(); + let onCloseB = jest.fn(); + subscribeCloseWatcher({ref: a, onClose: onCloseA}); + subscribeCloseWatcher({ref: b, onClose: onCloseB}); + + // Grouped fire: two watchers fire synchronously before any microtask runs. + // visibleOverlays is NOT mutated between fires (React removal is async). + let live = [...FakeCloseWatcher.instances]; + live[1].fire(); + live[0].fire(); + + expect(onCloseB).toHaveBeenCalledTimes(1); + expect(onCloseA).toHaveBeenCalledTimes(1); + }); + + it('recreates a watcher when a subscriber stays open after firing (controlled-ignore)', () => { + let a = makeRef(); + visibleOverlays.push(a); + subscribeCloseWatcher({ref: a, onClose: () => {}}); // onClose ignores -> a stays in visibleOverlays + + expect(FakeCloseWatcher.instances).toHaveLength(1); + FakeCloseWatcher.instances[0].fire(); + // Subscriber never unsubscribed, so the pool is topped back up to 1. + expect(FakeCloseWatcher.instances).toHaveLength(1); + }); + + it('is a no-op when CloseWatcher is unavailable', () => { + delete window.CloseWatcher; + let a = makeRef(); + visibleOverlays.push(a); + let unsub = subscribeCloseWatcher({ref: a, onClose: () => {}}); + expect(FakeCloseWatcher.instances).toHaveLength(0); + expect(() => unsub()).not.toThrow(); + }); +}); diff --git a/packages/react-aria/test/overlays/useOverlay.test.js b/packages/react-aria/test/overlays/useOverlay.test.js index 0ba68c5d2ee..4bd583fed64 100644 --- a/packages/react-aria/test/overlays/useOverlay.test.js +++ b/packages/react-aria/test/overlays/useOverlay.test.js @@ -138,4 +138,53 @@ describe('useOverlay', function () { fireEvent.keyDown(el, {key: 'Escape'}); expect(onClose).toHaveBeenCalledTimes(1); }); + + describe('with CloseWatcher', () => { + class FakeCloseWatcher { + static instances = []; + constructor() { + this.onclose = null; + FakeCloseWatcher.instances.push(this); + } + destroy() { + let i = FakeCloseWatcher.instances.indexOf(this); + if (i >= 0) { + FakeCloseWatcher.instances.splice(i, 1); + } + } + fire() { + this.destroy(); + this.onclose?.(); + } + } + + beforeEach(() => { + FakeCloseWatcher.instances = []; + window.CloseWatcher = FakeCloseWatcher; + }); + afterEach(() => { + delete window.CloseWatcher; + }); + + it('closes via a fired close watcher instead of the keydown handler', function () { + let onClose = jest.fn(); + let res = render(); + let el = res.getByTestId('test'); + + // The keydown handler must NOT be registered when CloseWatcher exists, + // so Escape keydown alone does nothing. + fireEvent.keyDown(el, {key: 'Escape'}); + expect(onClose).not.toHaveBeenCalled(); + + // Firing the watcher (as the platform would) closes the overlay. + FakeCloseWatcher.instances[FakeCloseWatcher.instances.length - 1].fire(); + expect(onClose).toHaveBeenCalledTimes(1); + }); + + it('does not subscribe when isKeyboardDismissDisabled', function () { + let onClose = jest.fn(); + render(); + expect(FakeCloseWatcher.instances).toHaveLength(0); + }); + }); }); From b31caf0b4d64f3b438099aa232baffad92991521 Mon Sep 17 00:00:00 2001 From: Rob Snow Date: Tue, 21 Jul 2026 16:30:05 +1000 Subject: [PATCH 2/5] Cleanup and more tests, more targeted annotations and explanations --- .../2026-07-20-close-watchers-overlays.md | 685 ------------------ ...26-07-20-close-watchers-overlays-design.md | 238 ------ .../test/CloseWatcher.browser.test.tsx | 218 +++++- .../react-aria/src/overlays/closeWatchers.ts | 33 +- .../react-aria/src/overlays/useOverlay.ts | 29 +- .../test/overlays/closeWatchers.test.js | 115 --- .../test/overlays/useOverlay.test.js | 49 -- 7 files changed, 240 insertions(+), 1127 deletions(-) delete mode 100644 docs/superpowers/plans/2026-07-20-close-watchers-overlays.md delete mode 100644 docs/superpowers/specs/2026-07-20-close-watchers-overlays-design.md delete mode 100644 packages/react-aria/test/overlays/closeWatchers.test.js diff --git a/docs/superpowers/plans/2026-07-20-close-watchers-overlays.md b/docs/superpowers/plans/2026-07-20-close-watchers-overlays.md deleted file mode 100644 index bd128d953e1..00000000000 --- a/docs/superpowers/plans/2026-07-20-close-watchers-overlays.md +++ /dev/null @@ -1,685 +0,0 @@ -# CloseWatcher Support for Overlays — Implementation Plan - -> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. - -**Goal:** Route overlay dismissal (Escape / native close requests) through the platform `CloseWatcher` API via a singleton pool manager, while keeping the existing focus-bound Escape handler as a fallback. - -**Architecture:** A module-level singleton (`closeWatchers.ts`) owns a pool of native `CloseWatcher` instances, decoupled from any specific overlay. `useOverlay` subscribes on open and unsubscribes on close. When a watcher fires, the manager snapshots the `visibleOverlays` z-order stack and tells the top-most overlay to close. The pool keeps its size equal to the subscriber count, which also recreates a watcher when a controlled consumer ignores `onOpenChange(false)`. - -**Tech Stack:** TypeScript, React, `@react-aria/overlays` (source in `packages/react-aria/src/overlays/`), Jest (unit tests), Vitest + Playwright browser provider (browser tests). - -## Global Constraints - -- **No commits without explicit user confirmation** (project rule). The commit steps below are prepared but must be confirmed by the user before running. -- Source of truth for `@react-aria/overlays` lives in `packages/react-aria/src/overlays/` (the `packages/@react-aria/overlays/src/index.ts` is only a re-export shim). -- `CloseWatcher` is not present in the repo's TypeScript DOM lib (TS 5.8.2); the manager must declare its own minimal types. -- Apache license header (copyright 2026) required at the top of every new source and test file — copy the exact header block from `packages/react-aria-components/test/Dialog.browser.test.tsx`. -- Behavior parity: `isKeyboardDismissDisabled` gates dismissal; `isDismissable` does **not** gate Escape/close-request dismissal. -- When `CloseWatcher` is available, `useOverlay` must NOT `preventDefault` on the Escape keydown (that would suppress the native close request). - ---- - -### Task 1: `closeWatchers` manager module - -**Files:** -- Create: `packages/react-aria/src/overlays/closeWatchers.ts` -- Test: `packages/react-aria/test/overlays/closeWatchers.test.js` - -**Interfaces:** -- Consumes: `visibleOverlays` (exported from `useOverlay.ts` in Task 2). For Task 1's tests, a local test-owned array stands in — see note below. -- Produces: - - `export interface CloseWatcherSubscriber { ref: RefObject; onClose: () => void; }` - - `export function subscribeCloseWatcher(subscriber: CloseWatcherSubscriber): () => void` — registers the subscriber, reconciles the pool, returns an unsubscribe function. - -> **Import-cycle note:** `closeWatchers.ts` imports `visibleOverlays` from `useOverlay.ts`, and `useOverlay.ts` imports `subscribeCloseWatcher` from `closeWatchers.ts`. This cycle is safe because `closeWatchers.ts` only *reads* `visibleOverlays` at call time (inside `notify`), never at module-init time, and `subscribeCloseWatcher` is a hoisted function declaration. Since `useOverlay.ts` does not export `visibleOverlays` until Task 2, Task 1 temporarily declares `visibleOverlays` inside `closeWatchers.ts` and Task 2 moves the export and switches the import. (Both steps are shown.) - -- [ ] **Step 1: Write the failing test** - -Create `packages/react-aria/test/overlays/closeWatchers.test.js`: - -```js -/* - * 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 {subscribeCloseWatcher, visibleOverlays} from '../../src/overlays/closeWatchers'; - -// Minimal fake of the native CloseWatcher. Mirrors the auto-destroy-on-fire -// behavior: firing removes the instance from the live pool. -class FakeCloseWatcher { - static instances = []; - constructor() { - this.onclose = null; - FakeCloseWatcher.instances.push(this); - } - destroy() { - let i = FakeCloseWatcher.instances.indexOf(this); - if (i >= 0) { - FakeCloseWatcher.instances.splice(i, 1); - } - } - // Simulate a native close request delivered to this watcher. - fire() { - this.destroy(); - this.onclose?.(); - } -} - -function makeRef(el = document.createElement('div')) { - return {current: el}; -} - -describe('closeWatchers manager', () => { - beforeEach(() => { - FakeCloseWatcher.instances = []; - window.CloseWatcher = FakeCloseWatcher; - visibleOverlays.length = 0; - }); - - afterEach(() => { - delete window.CloseWatcher; - visibleOverlays.length = 0; - }); - - it('creates one watcher per subscriber and destroys on unsubscribe', () => { - let a = makeRef(); - visibleOverlays.push(a); - let unsub = subscribeCloseWatcher({ref: a, onClose: () => {}}); - expect(FakeCloseWatcher.instances).toHaveLength(1); - unsub(); - expect(FakeCloseWatcher.instances).toHaveLength(0); - }); - - it('closes only the top-most overlay when a watcher fires', () => { - let a = makeRef(); - let b = makeRef(); - visibleOverlays.push(a, b); // b is top-most - let onCloseA = jest.fn(); - let onCloseB = jest.fn(); - subscribeCloseWatcher({ref: a, onClose: onCloseA}); - subscribeCloseWatcher({ref: b, onClose: onCloseB}); - - FakeCloseWatcher.instances[FakeCloseWatcher.instances.length - 1].fire(); - - expect(onCloseB).toHaveBeenCalledTimes(1); - expect(onCloseA).not.toHaveBeenCalled(); - }); - - it('closes n distinct overlays for n grouped synchronous fires', () => { - let a = makeRef(); - let b = makeRef(); - visibleOverlays.push(a, b); - let onCloseA = jest.fn(); - let onCloseB = jest.fn(); - subscribeCloseWatcher({ref: a, onClose: onCloseA}); - subscribeCloseWatcher({ref: b, onClose: onCloseB}); - - // Grouped fire: two watchers fire synchronously before any microtask runs. - // visibleOverlays is NOT mutated between fires (React removal is async). - let live = [...FakeCloseWatcher.instances]; - live[1].fire(); - live[0].fire(); - - expect(onCloseB).toHaveBeenCalledTimes(1); - expect(onCloseA).toHaveBeenCalledTimes(1); - }); - - it('recreates a watcher when a subscriber stays open after firing (controlled-ignore)', () => { - let a = makeRef(); - visibleOverlays.push(a); - subscribeCloseWatcher({ref: a, onClose: () => {}}); // onClose ignores -> a stays in visibleOverlays - - expect(FakeCloseWatcher.instances).toHaveLength(1); - FakeCloseWatcher.instances[0].fire(); - // Subscriber never unsubscribed, so the pool is topped back up to 1. - expect(FakeCloseWatcher.instances).toHaveLength(1); - }); - - it('is a no-op when CloseWatcher is unavailable', () => { - delete window.CloseWatcher; - let a = makeRef(); - visibleOverlays.push(a); - let unsub = subscribeCloseWatcher({ref: a, onClose: () => {}}); - expect(FakeCloseWatcher.instances).toHaveLength(0); - expect(() => unsub()).not.toThrow(); - }); -}); -``` - -- [ ] **Step 2: Run test to verify it fails** - -Run: `yarn jest packages/react-aria/test/overlays/closeWatchers.test.js` -Expected: FAIL — cannot find module `../../src/overlays/closeWatchers`. - -- [ ] **Step 3: Write the manager** - -Create `packages/react-aria/src/overlays/closeWatchers.ts`: - -```ts -/* - * 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 {RefObject} from '@react-types/shared'; - -// CloseWatcher is not yet in the TypeScript DOM lib (TS 5.8.2), so declare -// the minimal surface we rely on. -interface CloseWatcher { - destroy(): void; - onclose: (() => void) | null; -} -interface CloseWatcherConstructor { - new (): CloseWatcher; -} -declare global { - // eslint-disable-next-line no-var - interface Window { - CloseWatcher?: CloseWatcherConstructor; - } -} - -export interface CloseWatcherSubscriber { - /** The overlay container ref, used to determine z-order via visibleOverlays. */ - ref: RefObject; - /** Called when this overlay should close because it is the top-most. */ - onClose: () => void; -} - -// TEMPORARY for Task 1 only. Task 2 removes this and imports the shared -// array from useOverlay.ts instead. -export const visibleOverlays: RefObject[] = []; - -const subscribers = new Set(); -const watchers: CloseWatcher[] = []; - -// Refs asked to close during the current (synchronous) close request. Excluded -// from the top-most calculation so grouped synchronous fires close distinct -// overlays. Cleared on the next microtask. -const pendingClose = new Set>(); -let pendingClear = false; - -function hasCloseWatcher(): boolean { - return typeof window !== 'undefined' && typeof window.CloseWatcher === 'function'; -} - -function createWatcher(): void { - let watcher = new window.CloseWatcher!(); - watcher.onclose = () => { - let i = watchers.indexOf(watcher); - if (i >= 0) { - watchers.splice(i, 1); - } - notify(); - reconcile(); - }; - watchers.push(watcher); -} - -// Invariant: watchers.length === subscribers.size. -function reconcile(): void { - if (!hasCloseWatcher()) { - return; - } - while (watchers.length < subscribers.size) { - createWatcher(); - } - while (watchers.length > subscribers.size) { - watchers.pop()!.destroy(); - } -} - -function notify(): void { - // Snapshot at the start so mutations (closing removes from visibleOverlays) - // and synchronous unsubscribes do not shift the top-most check mid-pass. - let overlays = visibleOverlays.filter(o => !pendingClose.has(o)); - let topMost = overlays[overlays.length - 1]; - if (!topMost) { - return; - } - - pendingClose.add(topMost); - if (!pendingClear) { - pendingClear = true; - queueMicrotask(() => { - pendingClose.clear(); - pendingClear = false; - }); - } - - // Tell all subscribers; only the top-most one closes. - for (let subscriber of [...subscribers]) { - if (subscriber.ref === topMost) { - subscriber.onClose(); - } - } -} - -export function subscribeCloseWatcher(subscriber: CloseWatcherSubscriber): () => void { - if (!hasCloseWatcher()) { - return () => {}; - } - subscribers.add(subscriber); - reconcile(); - return () => { - subscribers.delete(subscriber); - reconcile(); - }; -} -``` - -- [ ] **Step 4: Run test to verify it passes** - -Run: `yarn jest packages/react-aria/test/overlays/closeWatchers.test.js` -Expected: PASS (all 5 tests). - -- [ ] **Step 5: Commit** (only after user confirmation) - -```bash -git add packages/react-aria/src/overlays/closeWatchers.ts packages/react-aria/test/overlays/closeWatchers.test.js -git commit -m "feat: add CloseWatcher pool manager for overlays" -``` - ---- - -### Task 2: `useOverlay` integration + shared `visibleOverlays` - -**Files:** -- Modify: `packages/react-aria/src/overlays/useOverlay.ts` (line 61 export; lines 128-139 keyboard fallback; add subscription effect) -- Modify: `packages/react-aria/src/overlays/closeWatchers.ts` (remove temporary `visibleOverlays`, import from `useOverlay`) -- Test: `packages/react-aria/test/overlays/useOverlay.test.js` (add CloseWatcher-path tests) - -**Interfaces:** -- Consumes: `subscribeCloseWatcher`, `CloseWatcherSubscriber` from Task 1. -- Produces: `export const visibleOverlays` from `useOverlay.ts` (moved out of Task 1's temporary declaration). - -- [ ] **Step 1: Write the failing test** - -Add to `packages/react-aria/test/overlays/useOverlay.test.js` (after the existing Escape tests, before the final closing `});` of the file — the two blocks below are top-level `it`s inside the `describe('useOverlay', ...)`): - -```js - describe('with CloseWatcher', () => { - class FakeCloseWatcher { - static instances = []; - constructor() { - this.onclose = null; - FakeCloseWatcher.instances.push(this); - } - destroy() { - let i = FakeCloseWatcher.instances.indexOf(this); - if (i >= 0) { - FakeCloseWatcher.instances.splice(i, 1); - } - } - fire() { - this.destroy(); - this.onclose?.(); - } - } - - beforeEach(() => { - FakeCloseWatcher.instances = []; - window.CloseWatcher = FakeCloseWatcher; - }); - afterEach(() => { - delete window.CloseWatcher; - }); - - it('closes via a fired close watcher instead of the keydown handler', function () { - let onClose = jest.fn(); - let res = render(); - let el = res.getByTestId('test'); - - // The keydown handler must NOT be registered when CloseWatcher exists, - // so Escape keydown alone does nothing. - fireEvent.keyDown(el, {key: 'Escape'}); - expect(onClose).not.toHaveBeenCalled(); - - // Firing the watcher (as the platform would) closes the overlay. - FakeCloseWatcher.instances[FakeCloseWatcher.instances.length - 1].fire(); - expect(onClose).toHaveBeenCalledTimes(1); - }); - - it('does not subscribe when isKeyboardDismissDisabled', function () { - let onClose = jest.fn(); - render(); - expect(FakeCloseWatcher.instances).toHaveLength(0); - }); - }); -``` - -- [ ] **Step 2: Run test to verify it fails** - -Run: `yarn jest packages/react-aria/test/overlays/useOverlay.test.js -t "with CloseWatcher"` -Expected: FAIL — `onClose` is called by the still-present keydown handler (first test), and/or a watcher is created path missing. - -- [ ] **Step 3: Export `visibleOverlays` and wire the subscription in `useOverlay.ts`** - -In `packages/react-aria/src/overlays/useOverlay.ts`: - -Change the import line to also pull in the manager and the `useEffectEvent` -utility (keep imports sorted by their first binding name): - -```ts -import {subscribeCloseWatcher} from './closeWatchers'; -import {useEffect, useRef} from 'react'; -import {useEffectEvent} from '../utils/useEffectEvent'; -``` - -Export `visibleOverlays` (line 61): - -```ts -export const visibleOverlays: RefObject[] = []; -``` - -Add, immediately after `onHide` is defined (after line 98), the CloseWatcher -availability flag, a stable close-request callback, and the subscription -effect. **Do not read or write a ref during render** — use `useEffectEvent` -to keep `onClose` fresh (it returns a stable callback that always invokes the -latest `onClose`, updating its ref inside an insertion effect), and build the -subscriber object *inside* the effect: - -```ts - let hasCloseWatcher = typeof window !== 'undefined' && typeof window.CloseWatcher === 'function'; - - // A stable callback that always calls the latest onClose, so the close watcher - // subscription does not need to re-run (and churn the pool) on every re-render. - let onCloseRequest = useEffectEvent(() => onClose?.()); - - useEffect(() => { - if (!isOpen || isKeyboardDismissDisabled || !hasCloseWatcher) { - return; - } - return subscribeCloseWatcher({ref, onClose: onCloseRequest}); - // onCloseRequest is a stable useEffectEvent and must be omitted from deps. - }, [isOpen, isKeyboardDismissDisabled, hasCloseWatcher, ref]); -``` - -Replace the keyboard handler (lines 128-139) so the Escape shortcut is only registered as a fallback when `CloseWatcher` is unavailable (registering it when available would `preventDefault` and suppress the native close request): - -```ts - // Fallback Escape handling for browsers without CloseWatcher support. - // When CloseWatcher is available, Escape is handled via the subscription - // above and we must NOT preventDefault here or the native close request - // would be suppressed. - let {keyboardProps} = useKeyboard(hasCloseWatcher ? {} : { - shortcuts: { - Escape: () => { - if (!isKeyboardDismissDisabled) { - onHide(); - return; - } - return false; - } - } - }); -``` - -- [ ] **Step 4: Remove the temporary `visibleOverlays` from `closeWatchers.ts`** - -In `packages/react-aria/src/overlays/closeWatchers.ts`, delete the temporary block: - -```ts -// TEMPORARY for Task 1 only. Task 2 removes this and imports the shared -// array from useOverlay.ts instead. -export const visibleOverlays: RefObject[] = []; -``` - -and replace it with an import near the top (after the existing `import {RefObject} ...`): - -```ts -import {visibleOverlays} from './useOverlay'; -``` - -- [ ] **Step 5: Update Task 1's manager test import** - -In `packages/react-aria/test/overlays/closeWatchers.test.js`, change the import so `visibleOverlays` comes from `useOverlay` (its new home): - -```js -import {subscribeCloseWatcher} from '../../src/overlays/closeWatchers'; -import {visibleOverlays} from '../../src/overlays/useOverlay'; -``` - -- [ ] **Step 6: Run all affected unit tests** - -Run: `yarn jest packages/react-aria/test/overlays/useOverlay.test.js packages/react-aria/test/overlays/closeWatchers.test.js` -Expected: PASS. The pre-existing jsdom Escape tests still pass because jsdom has no `window.CloseWatcher`, so the fallback keydown handler stays active for them. - -- [ ] **Step 7: Run the broader overlay suite for regressions** - -Run: `yarn jest packages/react-aria/test/overlays/` -Expected: PASS (no regressions in `useModalOverlay`, `usePopover`, etc.). - -- [ ] **Step 8: Commit** (only after user confirmation) - -```bash -git add packages/react-aria/src/overlays/useOverlay.ts packages/react-aria/src/overlays/closeWatchers.ts packages/react-aria/test/overlays/ -git commit -m "feat: dismiss overlays via CloseWatcher, keep keydown fallback" -``` - ---- - -### Task 3: Browser tests driving native close requests - -**Files:** -- Create: `packages/react-aria-components/test/CloseWatcher.browser.test.tsx` - -**Interfaces:** -- Consumes: RAC `DialogTrigger`, `Modal`, `ModalOverlay`, `Popover`, `Dialog`, `Button`, `Heading`. Real Escape via `userEvent.keyboard('{Escape}')`. - -- [ ] **Step 1: Write the browser tests** - -Create `packages/react-aria-components/test/CloseWatcher.browser.test.tsx`: - -```tsx -/* - * 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 {Dialog, DialogTrigger} from '../src/Dialog'; -import {describe, expect, it} from 'vitest'; -import {Heading} from '../src/Heading'; -import {Modal, ModalOverlay} from '../src/Modal'; -import {Popover} from '../src/Popover'; -import React from 'react'; -import {render} from 'vitest-browser-react'; -import {userEvent} from 'vitest/browser'; - -// CloseWatcher is required for these tests. Skip on browsers that lack it -// rather than exercising the keydown fallback (covered by the jsdom suite). -let supportsCloseWatcher = typeof window !== 'undefined' && 'CloseWatcher' in window; -let describeCloseWatcher = supportsCloseWatcher ? describe : describe.skip; - -async function press(key: string) { - await userEvent.keyboard(`{${key}}`); -} - -describeCloseWatcher('CloseWatcher dismissal', () => { - it('closes a single modal on Escape', async () => { - let {getByRole, queryByRole} = await render( - - - - - Hello - - - - - ); - expect(getByRole('dialog')).toBeTruthy(); - await press('Escape'); - expect(queryByRole('dialog')).toBeNull(); - }); - - it('does not close when isKeyboardDismissDisabled', async () => { - let {getByRole} = await render( - - - - - - Locked - - - - - - ); - expect(getByRole('dialog')).toBeTruthy(); - await press('Escape'); - expect(getByRole('dialog')).toBeTruthy(); - }); - - it('closes the top-most overlay first for nested modals', async () => { - // Outer opens on mount (the single "free" watcher). The inner modal is - // opened by a click so it gets its own transient-user-activation watcher - // and is NOT grouped with the outer (grouping would close both at once). - let {getByRole, queryAllByRole} = await render( - - - - - Outer - - - - - Inner - - - - - - - - ); - expect(queryAllByRole('dialog')).toHaveLength(1); - await userEvent.click(getByRole('button', {name: 'Open inner'})); - expect(queryAllByRole('dialog')).toHaveLength(2); - await press('Escape'); - // Inner closed, outer remains. - expect(queryAllByRole('dialog')).toHaveLength(1); - expect(getByRole('dialog')).toBeTruthy(); - await press('Escape'); - // Outer closes on the replacement/next watcher. - expect(queryAllByRole('dialog')).toHaveLength(0); - }); - - it('closes an inner popover before the modal', async () => { - // Modal opens on mount (free watcher); popover opened via click so its - // watcher is independent (not grouped) and receives the first close request. - let {getByRole, getByText, queryByRole, queryByText} = await render( - - - - - Modal - - - - - - - - - - - - ); - await userEvent.click(getByRole('button', {name: 'Open popover'})); - expect(getByText('Popover content')).toBeTruthy(); - await press('Escape'); - // Popover (most-recent watcher) closes; modal stays. - expect(queryByText('Popover content')).toBeNull(); - expect(getByRole('dialog')).toBeTruthy(); - await press('Escape'); - // Modal closes on the next close request. - expect(queryByRole('dialog')).toBeNull(); - }); - - it('recreates a watcher when a controlled parent ignores onOpenChange', async () => { - let closeCount = 0; - function Controlled() { - // Always open; ignore onOpenChange(false) so the modal stays open. - return ( - { - if (!isOpen) { - closeCount++; - } - }}> - - - - Sticky - - - - - ); - } - let {getByRole} = await render(); - expect(getByRole('dialog')).toBeTruthy(); - - await press('Escape'); - expect(closeCount).toBe(1); - expect(getByRole('dialog')).toBeTruthy(); - - // Second Escape only fires if a replacement watcher was created. - await press('Escape'); - expect(closeCount).toBe(2); - expect(getByRole('dialog')).toBeTruthy(); - }); -}); -``` - -- [ ] **Step 2: Run the browser tests** - -Run: `yarn test:browser packages/react-aria-components/test/CloseWatcher.browser.test.tsx` -Expected: PASS across chromium/firefox/webkit (all support CloseWatcher). If a runner instance lacks CloseWatcher, the whole suite is skipped (not failed). - -> **If any test fails on unmount timing** (an overlay asserted closed is still momentarily in the DOM), verify the actual DOM with the browser tools first, and treat any real failure with the systematic-debugging skill rather than loosening the assertion (e.g. do not swap a strict `queryByRole(...).toBeNull()` for a weaker check without understanding why). - -- [ ] **Step 3: Commit** (only after user confirmation) - -```bash -git add packages/react-aria-components/test/CloseWatcher.browser.test.tsx -git commit -m "test: add browser tests for CloseWatcher overlay dismissal" -``` - ---- - -## Notes on spec edge cases (for the implementer) - -These are validated or documented, not separate tasks: - -- **Free watcher + grouping** — covered by Task 1's "n grouped fires" unit test and Task 3's nested-modal test. -- **Only the most-recent watcher receives events** — covered by Task 3's nested + popover tests (top-most closes first). -- **`preventDefault` on Escape suppresses the close request** — this is why component-specific Escape handlers (combobox, menu, etc.) still compose. Not changed by this work; note it in code comments only. -- **`cancel` only fires with transient activation / once per activation** — the reason an ignored `onOpenChange(false)` can't be prevented early; handled by watcher recreation (Task 1 unit test + Task 3 controlled test). No `cancel`/confirmation flow is implemented (out of scope). diff --git a/docs/superpowers/specs/2026-07-20-close-watchers-overlays-design.md b/docs/superpowers/specs/2026-07-20-close-watchers-overlays-design.md deleted file mode 100644 index 63c6d2dd460..00000000000 --- a/docs/superpowers/specs/2026-07-20-close-watchers-overlays-design.md +++ /dev/null @@ -1,238 +0,0 @@ -# CloseWatcher support for overlays - -**Date:** 2026-07-20 -**Status:** Approved design, pending implementation plan - -## Problem - -Overlays (dialogs, modals, popovers, menus) currently dismiss on the Escape -key via a React keyboard handler bound to the overlay element -([`useOverlay.ts:128-139`](../../../packages/react-aria/src/overlays/useOverlay.ts)). -This handler only fires when focus is inside the overlay, and it does not -participate in the platform's native "close request" mechanism (Escape on -desktop, the Android back gesture, etc.). - -We want overlays to integrate with the native -[`CloseWatcher`](https://html.spec.whatwg.org/multipage/interaction.html#close-requests-and-close-watchers) -API so that dismissal is driven by platform close requests rather than a -focus-bound key handler. - -## Goals - -- Route overlay dismissal through native `CloseWatcher` instances when the API - is available. -- Preserve the existing "only the top-most overlay closes" semantics, keyed off - the existing `visibleOverlays` stack. -- Handle the controlled-open case where the consumer ignores - `onOpenChange(false)` and the overlay stays open even though its - `CloseWatcher` was consumed. -- Fall back to the existing focus-bound Escape handler on browsers without - `CloseWatcher`. -- Add browser tests that trigger real native close requests via Escape. - -## Non-goals - -- Changing component-specific Escape handling that lives outside `useOverlay` - (combobox, menu, searchfield, calendar, etc.). Those keep their own handlers. -- Implementing a `cancel`/confirmation flow (see "Known limitations"). - -## Scope - -**All `useOverlay` consumers** use the `CloseWatcher` when the API is available -— not just modals. This avoids introducing a new prop and reduces the number of -distinct dismissal code paths to test. - -Accepted behavior change: non-modal popovers and menus will now close on a -close request regardless of where focus is, rather than only when focus is -inside the overlay (today's focus-bound behavior). - -## Architecture - -### 1. `CloseWatcher` manager (singleton pool) - -A module-level singleton (`packages/react-aria/src/overlays/closeWatchers.ts`) -owns a **pool** of native `CloseWatcher` instances. Watchers are **not** tied to -any specific overlay — they are a fungible pool whose only job is to keep the -platform's close-request machinery armed. The overlays keep their own stack -(`visibleOverlays`) and independently decide which one closes; the pool order -need not match the overlay stack order. - -State: - -- `subscribers: Set` — one entry per open, dismissable overlay. -- `watchers: CloseWatcher[]` — the pool. -- `pendingClose: Set` — refs asked to close during the current - close request; cleared on the next microtask (see grouping, below). - -A `Subscriber` is a stable object created once per `useOverlay` call. It reads -the latest `ref`/`onClose` from refs so the subscription never has to churn when -the consumer re-renders (consistent with the project preference to fix -re-renders via stored values rather than re-subscribing). - -Public API: - -- `subscribe(subscriber): () => void` — adds the subscriber, calls - `reconcile()`, and returns an unsubscribe function that removes the subscriber - and calls `reconcile()` again. - -Internal: - -- `reconcile()` — enforces the single invariant - **`watchers.length === subscribers.size`**, creating or `destroy()`-ing - watchers to match. Each created watcher's `onclose` handler removes itself - from the pool, calls `notify()`, then calls `reconcile()`. This one invariant - covers every lifecycle transition: - - subscribe → pool grows by one. - - unsubscribe (normal close) → pool shrinks by one. - - watcher fires + overlay closes normally → subscriber removed *and* watcher - consumed → still balanced. - - watcher fires + controlled parent ignores `onOpenChange(false)` → subscriber - remains but watcher was consumed → pool is short one → `reconcile()` creates - a replacement so the still-open overlay stays dismissable. - -- `notify()` — invoked by a fired watcher's `onclose`: - 1. Snapshot the subscriber set (`[...subscribers]`) so a synchronous - unsubscribe during close does not corrupt iteration. - 2. Determine the top-most overlay from `visibleOverlays` **minus** - `pendingClose` (so grouped synchronous fires target distinct overlays). - 3. Broadcast to every snapshot subscriber, passing the top-most ref; a - subscriber closes only if its own ref is that top-most ref - ("tell all, top-most closes"). - 4. Add the closed ref to `pendingClose` and schedule a microtask to clear - `pendingClose`. - -`visibleOverlays` is shared between `useOverlay` and the manager. It stays -defined in `useOverlay.ts` (which owns pushing/splicing it on mount/unmount) and -is exported so `closeWatchers.ts` can import and snapshot it inside `notify()`. -The manager references it only at call time, so the `useOverlay` ↔ -`closeWatchers` import cycle is safe (no module-init-time dependency). - -### 2. `useOverlay` integration - -Replace the focus-bound Escape `useKeyboard` shortcut with a subscription when -`CloseWatcher` is available: - -```ts -useEffect(() => { - if (!isOpen || isKeyboardDismissDisabled || !window.CloseWatcher) { - return; - } - return closeWatchers.subscribe(subscriber); // subscriber reads latest ref/onClose -}, [isOpen, isKeyboardDismissDisabled]); -``` - -- **Subscription gating:** subscribe only when `isOpen` and not - `isKeyboardDismissDisabled`. `isDismissable` does **not** gate subscription — - matching today's behavior where a default modal (`isDismissable=false`) still - closes on Escape. -- **Top-most check on close:** the subscriber's close callback reuses the - existing `onHide` semantics — it calls `onClose` only when its ref is the - top-most visible overlay. -- **Fallback:** when `window.CloseWatcher` is undefined, the subscription effect - is a no-op and `useOverlay` keeps the existing `useKeyboard` Escape handler. - When `CloseWatcher` *is* defined, the keyboard Escape handler is skipped to - avoid double-handling. - -### 3. Grouping — "close n" behavior - -The spec allows exactly one "free" `CloseWatcher` without transient user -activation; additional watchers created without activation are **grouped** with -the most-recently-created one, so a single close request fires every watcher in -the group synchronously. This happens for programmatically-opened nested -overlays (no user gesture between opens). - -Decision: **do not coalesce.** Each grouped watcher fires its own `notify()`, -and `n` grouped fires close `n` distinct overlays, top-down. - -Because grouped `onclose` events fire synchronously but React removes an overlay -from `visibleOverlays` asynchronously (effect cleanup), two back-to-back -`notify()` calls would otherwise both see the same top-most overlay and -double-close it. The `pendingClose` set (above) excludes already-asked refs so -each successive `notify()` targets the next overlay down: - -- Fire 1 → top-most = B → close B, mark B. -- Fire 2 → top-most = A (B excluded) → close A, mark A. -- `pendingClose` clears on the next microtask. - -## Data flow - -Open (with `CloseWatcher` support): - -``` -overlay opens → useOverlay effect → closeWatchers.subscribe(subscriber) - → reconcile() → new CloseWatcher pushed to pool -``` - -Close request (Escape / back gesture): - -``` -platform close request → most-recent watcher.onclose - → remove watcher from pool → notify() - → snapshot subscribers; topmost = last(visibleOverlays \ pendingClose) - → broadcast; subscriber whose ref === topmost calls onClose - → mark topmost in pendingClose (clear next microtask) - → reconcile() → if a still-open subscriber lost its watcher, create replacement -``` - -Close (normal): - -``` -overlay closes → useOverlay effect cleanup → unsubscribe() - → reconcile() → destroy() one pooled watcher -``` - -## Error handling / edge cases - -From the [close-requests spec](https://html.spec.whatwg.org/multipage/interaction.html#close-requests-and-close-watchers): - -1. **Free watcher + grouping.** Only one watcher is allowed without transient - user activation; extras group. Handled by the "close n" mechanism + - `pendingClose`. Covered by a nested-overlay browser test. -2. **Only the most-recently-created active watcher receives events.** Maps - naturally to "top-most overlay closes" because overlays subscribe in open - order. -3. **`preventDefault` on the Escape keydown suppresses the close request.** - Components with their own Escape handlers (combobox, menu, etc.) that - `preventDefault` will suppress the overlay close request — so they compose - correctly. Verified empirically by a browser test. -4. **`cancel` event only fires with transient user activation, and only once - per activation.** This is why an ignored `onOpenChange(false)` cannot be - prevented early — the watcher is already consumed. Handled by recreating a - replacement watcher via `reconcile()`. -5. **Watcher lifecycle.** A watcher deactivates after a `close` event, - `destroy()`, or an aborted signal. The pool removes fired watchers and - `destroy()`s surplus ones. - -## Known limitations - -- No `cancel`/confirmation flow is implemented. Because `cancel` only fires with - transient user activation and only once per activation, a reliable - "are you sure?" prompt is out of scope for this change. - -## Testing - -New `Modal.browser.test.tsx` (Vitest + Playwright browser provider, -`packages/react-aria-components/test/`, `*.browser.test.tsx`) that dispatches -**real** Escape keydowns so native `CloseWatcher` instances actually fire: - -1. **Single overlay** — Escape closes it. -2. **`isKeyboardDismissDisabled`** — Escape does nothing. -3. **Nested modals** (`InertTest`-style) — first Escape closes the top overlay - only; second Escape closes the next — top-most ordering. -4. **Modal + DateRangePicker popover** (`DateRangePickerInsideModalStory`) — - with the picker open, Escape closes the picker and the modal stays open; a - second Escape closes the modal. -5. **Controlled overlay ignoring `onOpenChange(false)`** — Escape fires, the - parent ignores it, the overlay stays open, and a *second* Escape still fires - (proving the replacement watcher was created). - -Fallback (no `CloseWatcher`) continues to be covered by the existing -non-browser Escape tests. - -## Files touched - -- `packages/react-aria/src/overlays/closeWatchers.ts` (new) — the manager. -- `packages/react-aria/src/overlays/useOverlay.ts` — subscribe/unsubscribe; - keep the keyboard handler as fallback; share `visibleOverlays`. -- `packages/react-aria-components/test/Modal.browser.test.tsx` (new) — browser - tests. diff --git a/packages/react-aria-components/test/CloseWatcher.browser.test.tsx b/packages/react-aria-components/test/CloseWatcher.browser.test.tsx index 9a62483fc2c..852c75d9e5d 100644 --- a/packages/react-aria-components/test/CloseWatcher.browser.test.tsx +++ b/packages/react-aria-components/test/CloseWatcher.browser.test.tsx @@ -10,31 +10,30 @@ * governing permissions and limitations under the License. */ +import {Autocomplete} from '../src/Autocomplete'; import {Button} from '../src/Button'; +import {Checkbox} from '../src/Checkbox'; import {describe, expect, it} from 'vitest'; import {Dialog, DialogTrigger} from '../src/Dialog'; +import {GridList, GridListItem} from '../src/GridList'; import {Heading} from '../src/Heading'; +import {Input} from '../src/Input'; +import {Menu, MenuItem, MenuTrigger, SubmenuTrigger} from '../src/Menu'; import {Modal, ModalOverlay} from '../src/Modal'; import {Popover} from '../src/Popover'; import React from 'react'; import {render} from 'vitest-browser-react'; +import {SearchField} from '../src/SearchField'; +import {useFilter} from 'react-aria/useFilter'; +import {User} from '@react-aria/test-utils'; import {userEvent} from 'vitest/browser'; -// CloseWatcher is required for these tests. Skip on browsers that lack it -// rather than exercising the keydown fallback (covered by the jsdom suite). -let supportsCloseWatcher = typeof window.CloseWatcher === 'function'; -let describeCloseWatcher = supportsCloseWatcher ? describe : describe.skip; - async function press(key: string) { await userEvent.keyboard(`{${key}}`); } -describeCloseWatcher('CloseWatcher dismissal', () => { +describe('CloseWatcher dismissal', () => { it('closes a single modal on Escape', async () => { - // vitest-browser-react's render() returns Locator-based selectors - // (getByRole etc.), not testing-library's query*/queryAll* helpers. - // Presence/absence is asserted via the auto-retrying `expect.element(...)` - // matcher so we don't race the modal's exit animation/unmount. let screen = await render( @@ -95,18 +94,24 @@ describeCloseWatcher('CloseWatcher dismissal', () => { ); - // Locator has no queryAllByRole equivalent; count via elements(), and poll - // rather than reading synchronously so exit-animation unmounts aren't missed. - await expect.poll(() => screen.getByRole('dialog').elements().length).toBe(1); - await userEvent.click(screen.getByRole('button', {name: 'Open inner'})); - await expect.poll(() => screen.getByRole('dialog').elements().length).toBe(2); + await expect.element(screen.getByRole('button', {name: 'Open inner'})).toBeInTheDocument(); + let outerDialog = screen.getByRole('dialog'); + let outerButton = screen.getByRole('button', {name: 'Open inner'}); + await userEvent.click(outerButton); + await expect.element(screen.getByRole('button', {name: 'Inner focus'})).toBeInTheDocument(); + let innerButton = screen.getByRole('button', {name: 'Inner focus'}); await press('Escape'); // Inner closed, outer remains. - await expect.poll(() => screen.getByRole('dialog').elements().length).toBe(1); + await expect.element(innerButton).not.toBeInTheDocument(); await expect.element(screen.getByRole('dialog')).toBeInTheDocument(); + await expect.poll(() => screen.getByRole('dialog').elements().length).toBe(1); + // Focus restoration to the trigger is async (focus briefly falls to + // before FocusScope restores it). Wait for focus to return to the outer trigger + // otherwise Escape will fire on and never reach the overlay. + await expect.poll(() => document.activeElement).toBe(outerButton.element()); await press('Escape'); // Outer closes on the replacement/next watcher. - await expect.poll(() => screen.getByRole('dialog').elements().length).toBe(0); + await expect.element(outerDialog).not.toBeInTheDocument(); }); it('closes an inner popover before the modal', async () => { @@ -130,12 +135,15 @@ describeCloseWatcher('CloseWatcher dismissal', () => { ); - await userEvent.click(screen.getByRole('button', {name: 'Open popover'})); + let popoverTrigger = screen.getByRole('button', {name: 'Open popover'}); + await userEvent.click(popoverTrigger); await expect.element(screen.getByText('Popover content')).toBeInTheDocument(); await press('Escape'); // Popover (most-recent watcher) closes; modal stays. await expect.element(screen.getByText('Popover content')).not.toBeInTheDocument(); await expect.element(screen.getByRole('dialog')).toBeInTheDocument(); + + await expect.poll(() => document.activeElement).toBe(popoverTrigger.element()); await press('Escape'); // Modal closes on the next close request. await expect.element(screen.getByRole('dialog')).not.toBeInTheDocument(); @@ -144,14 +152,17 @@ describeCloseWatcher('CloseWatcher dismissal', () => { it('recreates a watcher when a controlled parent ignores onOpenChange', async () => { let closeCount = 0; function Controlled() { + let [isOpen, setIsOpen] = React.useState(true); // Always open; ignore onOpenChange(false) so the modal stays open. return ( { - if (!isOpen) { + if (!isOpen && closeCount < 1) { closeCount++; + return; } + setIsOpen(isOpen); }}> @@ -172,7 +183,172 @@ describeCloseWatcher('CloseWatcher dismissal', () => { // Second Escape only fires if a replacement watcher was created. await press('Escape'); - await expect.poll(() => closeCount).toBe(2); + await expect.element(screen.getByRole('dialog')).not.toBeInTheDocument(); + }); + + it('Escape clears the autocomplete input, then closes the popover, then the dialog', async () => { + function Example() { + let {contains} = useFilter({sensitivity: 'base'}); + return ( + + + + + Dialog + + + + contains(t, i)}> + + + + + Foo + Bar + Baz + + + + + + + + ); + } + let screen = await render(); + let popoverTrigger = screen.getByRole('button', {name: 'Open popover'}); + await userEvent.click(popoverTrigger); + let searchbox = screen.getByRole('searchbox'); + await expect.element(searchbox).toBeInTheDocument(); + await userEvent.type(searchbox, 'Fo'); + await expect.element(searchbox).toHaveValue('Fo'); + + // Esc #1: the autocomplete/search field consumes Escape to clear its input. + // The popover (and dialog) stay open, and the close request is suppressed. + await press('Escape'); + await expect.element(searchbox).toHaveValue(''); + await expect.element(searchbox).toBeInTheDocument(); + + // Esc #2: with an empty input, Escape closes the popover. The modal remains. + await press('Escape'); + await expect.element(screen.getByRole('searchbox')).not.toBeInTheDocument(); await expect.element(screen.getByRole('dialog')).toBeInTheDocument(); + + // Wait for focus to return to the popover trigger (async) so the fallback + // focus-bound Escape reaches the modal, then Esc #3 closes the dialog. + await expect.poll(() => document.activeElement).toBe(popoverTrigger.element()); + await press('Escape'); + await expect.element(screen.getByRole('dialog')).not.toBeInTheDocument(); + }); + + it('Escape clears the GridList selection, then closes the dialog', async () => { + let selection = new Set(); + function Example() { + return ( + + + + + List + { + selection = keys as Set; + }}> + + + One + + + + Two + + + + Three + + + + + + ); + } + let testUtilUser = new User(); + let screen = await render(); + await expect.element(screen.getByRole('grid')).toBeInTheDocument(); + let grid = screen.getByRole('grid').element() as HTMLElement; + let tester = testUtilUser.createTester('GridList', { + root: grid, + layout: 'grid', + interactionType: 'keyboard' + }); + let rows = tester.getRows(); + await tester.toggleRowSelection({row: rows[0]}); + await tester.toggleRowSelection({row: rows[1]}); + await expect.poll(() => selection.size).toBe(2); + + // Esc #1: the GridList consumes Escape to clear its selection. Dialog stays. + await press('Escape'); + await expect.poll(() => selection.size).toBe(0); + await expect.element(screen.getByRole('dialog')).toBeInTheDocument(); + + // Esc #2: with no selection, Escape closes the dialog. + await press('Escape'); + await expect.element(screen.getByRole('dialog')).not.toBeInTheDocument(); + }); + + it('Escape closes one submenu level at a time (3 deep)', async () => { + function Example() { + return ( + + + + + Foo + + Bar + + + Sub Foo + + Sub Baz + + + Deep Foo + Deep Bar + + + + + + + + + + ); + } + let menuCount = () => document.querySelectorAll('[role="menu"]').length; + let testUtilUser = new User(); + let screen = await render(); + let menuTester = testUtilUser.createTester('Menu', { + root: screen.getByRole('button', {name: 'Menu'}).element(), + interactionType: 'keyboard' + }); + await menuTester.open(); + let l2 = await menuTester.openSubmenu({submenuTrigger: menuTester.getSubmenuTriggers()[0]}); + await l2.openSubmenu({submenuTrigger: l2.getSubmenuTriggers()[0]}); + await expect.poll(menuCount).toBe(3); + + // Each Escape closes only the deepest submenu, restoring focus to its trigger. + await press('Escape'); + await expect.poll(menuCount).toBe(2); + await press('Escape'); + await expect.poll(menuCount).toBe(1); + await press('Escape'); + await expect.poll(menuCount).toBe(0); + await expect + .poll(() => document.activeElement) + .toBe(screen.getByRole('button', {name: 'Menu'}).element()); }); }); diff --git a/packages/react-aria/src/overlays/closeWatchers.ts b/packages/react-aria/src/overlays/closeWatchers.ts index 647b3fb8c9c..c03e2cae722 100644 --- a/packages/react-aria/src/overlays/closeWatchers.ts +++ b/packages/react-aria/src/overlays/closeWatchers.ts @@ -9,12 +9,10 @@ * OF ANY KIND, either express or implied. See the License for the specific language * governing permissions and limitations under the License. */ - import {RefObject} from '@react-types/shared'; import {visibleOverlays} from './useOverlay'; -// CloseWatcher is not yet in the TypeScript DOM lib (TS 5.8.2), so declare -// the minimal surface we rely on. +// CloseWatcher is not yet in the TypeScript DOM lib (TS 5.8.2) interface CloseWatcher { destroy(): void; onclose: (() => void) | null; @@ -28,6 +26,23 @@ declare global { } } +// CloseWatchers are an interesting API for web development. You may not be familiar with them +// https://developer.mozilla.org/en-US/docs/Web/API/CloseWatcher +// https://html.spec.whatwg.org/multipage/interaction.html#close-requests-and-close-watchers +// The quick synopsis is that they are EventTargets which emit cancel->close events and also +// call an `onclose` property. CloseWatchers will disconnect and be destroyed after they close. +// They are in a browser managed stack in terms of which one gets called for a given event. +// This stack isn't straightforward though, it contains "groupings", and one gesture/event may +// trigger multiple close watchers to fire synchronously. + +// We create a singleton which manages subscriptions from overlays that want to use CloseWatchers. +// It creates/destroys CloseWatchers as needed to match the number of subscribers. +// When a CloseWatcher fires, we determine which subscriber/overlay is the top-most and call its onClose. +// There is a weird edge case that we handle for controlled overlay triggers. If an overlay is +// controlled open and tries to close, but the parent ignores the onOpenChange, then the +// CloseWatcher will have been destroyed, but the total number of subscribers has not changed. +// In this case, we need to reconcile that difference and create a new CloseWatcher. + export interface CloseWatcherSubscriber { /** The overlay container ref, used to determine z-order via visibleOverlays. */ ref: RefObject; @@ -40,7 +55,7 @@ const watchers: CloseWatcher[] = []; // Refs asked to close during the current (synchronous) close request. Excluded // from the top-most calculation so grouped synchronous fires close distinct -// overlays. Cleared on the next microtask. +// overlays. Cleared in the next microtask. const pendingClose = new Set>(); let pendingClear = false; @@ -65,6 +80,10 @@ function createWatcher(): void { } // Invariant: watchers.length === subscribers.size. +// There may be a troublesome case here, creating them this way means they won't be +// "trusted" and may be "grouped" with other watchers and cause too many +// overlays to close at once. I'm unsure how to test this... but the spec seems to +// suggest this could happen. function reconcile(): void { if (!hasCloseWatcher()) { return; @@ -77,6 +96,10 @@ function reconcile(): void { } } +// Question about this, should all overlays close synchronously? Or should it be one at a time? +// When CloseWatchers are "grouped", they will all fire synchronously, but our current hooks +// may not like that. Either overlays have closed one at a time, or the parent has closed and unmounted +// all of them. I'm not sure if there is a race to worry about if they all try to close individually. function notify(): void { // Snapshot at the start so mutations (closing removes from visibleOverlays) // and synchronous unsubscribes do not shift the top-most check mid-pass. @@ -95,7 +118,7 @@ function notify(): void { }); } - // Tell all subscribers; only the top-most one closes. + // Only the top-most one closes. for (let subscriber of [...subscribers]) { if (subscriber.ref === topMost) { subscriber.onClose(); diff --git a/packages/react-aria/src/overlays/useOverlay.ts b/packages/react-aria/src/overlays/useOverlay.ts index 92ad6e40e72..e1c1af54fdb 100644 --- a/packages/react-aria/src/overlays/useOverlay.ts +++ b/packages/react-aria/src/overlays/useOverlay.ts @@ -101,16 +101,13 @@ export function useOverlay(props: AriaOverlayProps, ref: RefObject onClose?.()); - useEffect(() => { if (!isOpen || isKeyboardDismissDisabled || !hasCloseWatcher) { return; } + // Should *every* overlay be closed by close watcher? should some just be Esc? "Tooltip" for example? return subscribeCloseWatcher({ref, onClose: onCloseRequest}); - // onCloseRequest is a stable useEffectEvent and must be omitted from deps. }, [isOpen, isKeyboardDismissDisabled, hasCloseWatcher, ref]); let onInteractOutsideStart = (e: PointerEvent) => { @@ -145,17 +142,21 @@ export function useOverlay(props: AriaOverlayProps, ref: RefObject { - if (!isKeyboardDismissDisabled) { - onHide(); - return; + let {keyboardProps} = useKeyboard( + hasCloseWatcher + ? {} + : { + shortcuts: { + Escape: () => { + if (!isKeyboardDismissDisabled) { + onHide(); + return; + } + return false; + } + } } - return false; - } - } - }); + ); // Handle clicking outside the overlay to close it useInteractOutside({ diff --git a/packages/react-aria/test/overlays/closeWatchers.test.js b/packages/react-aria/test/overlays/closeWatchers.test.js deleted file mode 100644 index 913aff0a5fc..00000000000 --- a/packages/react-aria/test/overlays/closeWatchers.test.js +++ /dev/null @@ -1,115 +0,0 @@ -/* - * 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 {subscribeCloseWatcher} from '../../src/overlays/closeWatchers'; -import {visibleOverlays} from '../../src/overlays/useOverlay'; - -// Minimal fake of the native CloseWatcher. Mirrors the auto-destroy-on-fire -// behavior: firing removes the instance from the live pool. -class FakeCloseWatcher { - static instances = []; - constructor() { - this.onclose = null; - FakeCloseWatcher.instances.push(this); - } - destroy() { - let i = FakeCloseWatcher.instances.indexOf(this); - if (i >= 0) { - FakeCloseWatcher.instances.splice(i, 1); - } - } - // Simulate a native close request delivered to this watcher. - fire() { - this.destroy(); - this.onclose?.(); - } -} - -function makeRef(el = document.createElement('div')) { - return {current: el}; -} - -describe('closeWatchers manager', () => { - beforeEach(() => { - FakeCloseWatcher.instances = []; - window.CloseWatcher = FakeCloseWatcher; - visibleOverlays.length = 0; - }); - - afterEach(() => { - delete window.CloseWatcher; - visibleOverlays.length = 0; - }); - - it('creates one watcher per subscriber and destroys on unsubscribe', () => { - let a = makeRef(); - visibleOverlays.push(a); - let unsub = subscribeCloseWatcher({ref: a, onClose: () => {}}); - expect(FakeCloseWatcher.instances).toHaveLength(1); - unsub(); - expect(FakeCloseWatcher.instances).toHaveLength(0); - }); - - it('closes only the top-most overlay when a watcher fires', () => { - let a = makeRef(); - let b = makeRef(); - visibleOverlays.push(a, b); // b is top-most - let onCloseA = jest.fn(); - let onCloseB = jest.fn(); - subscribeCloseWatcher({ref: a, onClose: onCloseA}); - subscribeCloseWatcher({ref: b, onClose: onCloseB}); - - FakeCloseWatcher.instances[FakeCloseWatcher.instances.length - 1].fire(); - - expect(onCloseB).toHaveBeenCalledTimes(1); - expect(onCloseA).not.toHaveBeenCalled(); - }); - - it('closes n distinct overlays for n grouped synchronous fires', () => { - let a = makeRef(); - let b = makeRef(); - visibleOverlays.push(a, b); - let onCloseA = jest.fn(); - let onCloseB = jest.fn(); - subscribeCloseWatcher({ref: a, onClose: onCloseA}); - subscribeCloseWatcher({ref: b, onClose: onCloseB}); - - // Grouped fire: two watchers fire synchronously before any microtask runs. - // visibleOverlays is NOT mutated between fires (React removal is async). - let live = [...FakeCloseWatcher.instances]; - live[1].fire(); - live[0].fire(); - - expect(onCloseB).toHaveBeenCalledTimes(1); - expect(onCloseA).toHaveBeenCalledTimes(1); - }); - - it('recreates a watcher when a subscriber stays open after firing (controlled-ignore)', () => { - let a = makeRef(); - visibleOverlays.push(a); - subscribeCloseWatcher({ref: a, onClose: () => {}}); // onClose ignores -> a stays in visibleOverlays - - expect(FakeCloseWatcher.instances).toHaveLength(1); - FakeCloseWatcher.instances[0].fire(); - // Subscriber never unsubscribed, so the pool is topped back up to 1. - expect(FakeCloseWatcher.instances).toHaveLength(1); - }); - - it('is a no-op when CloseWatcher is unavailable', () => { - delete window.CloseWatcher; - let a = makeRef(); - visibleOverlays.push(a); - let unsub = subscribeCloseWatcher({ref: a, onClose: () => {}}); - expect(FakeCloseWatcher.instances).toHaveLength(0); - expect(() => unsub()).not.toThrow(); - }); -}); diff --git a/packages/react-aria/test/overlays/useOverlay.test.js b/packages/react-aria/test/overlays/useOverlay.test.js index 4bd583fed64..0ba68c5d2ee 100644 --- a/packages/react-aria/test/overlays/useOverlay.test.js +++ b/packages/react-aria/test/overlays/useOverlay.test.js @@ -138,53 +138,4 @@ describe('useOverlay', function () { fireEvent.keyDown(el, {key: 'Escape'}); expect(onClose).toHaveBeenCalledTimes(1); }); - - describe('with CloseWatcher', () => { - class FakeCloseWatcher { - static instances = []; - constructor() { - this.onclose = null; - FakeCloseWatcher.instances.push(this); - } - destroy() { - let i = FakeCloseWatcher.instances.indexOf(this); - if (i >= 0) { - FakeCloseWatcher.instances.splice(i, 1); - } - } - fire() { - this.destroy(); - this.onclose?.(); - } - } - - beforeEach(() => { - FakeCloseWatcher.instances = []; - window.CloseWatcher = FakeCloseWatcher; - }); - afterEach(() => { - delete window.CloseWatcher; - }); - - it('closes via a fired close watcher instead of the keydown handler', function () { - let onClose = jest.fn(); - let res = render(); - let el = res.getByTestId('test'); - - // The keydown handler must NOT be registered when CloseWatcher exists, - // so Escape keydown alone does nothing. - fireEvent.keyDown(el, {key: 'Escape'}); - expect(onClose).not.toHaveBeenCalled(); - - // Firing the watcher (as the platform would) closes the overlay. - FakeCloseWatcher.instances[FakeCloseWatcher.instances.length - 1].fire(); - expect(onClose).toHaveBeenCalledTimes(1); - }); - - it('does not subscribe when isKeyboardDismissDisabled', function () { - let onClose = jest.fn(); - render(); - expect(FakeCloseWatcher.instances).toHaveLength(0); - }); - }); }); From 7b781c2c24616bf0f6711f98cf81617741ac6cbb Mon Sep 17 00:00:00 2001 From: Rob Snow Date: Tue, 21 Jul 2026 16:36:04 +1000 Subject: [PATCH 3/5] fix comment --- packages/react-aria/src/overlays/closeWatchers.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react-aria/src/overlays/closeWatchers.ts b/packages/react-aria/src/overlays/closeWatchers.ts index c03e2cae722..812607ed64e 100644 --- a/packages/react-aria/src/overlays/closeWatchers.ts +++ b/packages/react-aria/src/overlays/closeWatchers.ts @@ -44,7 +44,7 @@ declare global { // In this case, we need to reconcile that difference and create a new CloseWatcher. export interface CloseWatcherSubscriber { - /** The overlay container ref, used to determine z-order via visibleOverlays. */ + /** The overlay container ref, used to determine react aria's stacking order via visibleOverlays. */ ref: RefObject; /** Called when this overlay should close because it is the top-most. */ onClose: () => void; From 29671294e075e0cc14eb39e8962f7c7bf7e3d941 Mon Sep 17 00:00:00 2001 From: Rob Snow Date: Tue, 21 Jul 2026 16:38:42 +1000 Subject: [PATCH 4/5] fix lint --- .../react-aria-components/test/CloseWatcher.browser.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react-aria-components/test/CloseWatcher.browser.test.tsx b/packages/react-aria-components/test/CloseWatcher.browser.test.tsx index 852c75d9e5d..688dbfdd41d 100644 --- a/packages/react-aria-components/test/CloseWatcher.browser.test.tsx +++ b/packages/react-aria-components/test/CloseWatcher.browser.test.tsx @@ -332,7 +332,7 @@ describe('CloseWatcher dismissal', () => { let testUtilUser = new User(); let screen = await render(); let menuTester = testUtilUser.createTester('Menu', { - root: screen.getByRole('button', {name: 'Menu'}).element(), + root: screen.getByRole('button', {name: 'Menu'}).element() as HTMLElement, interactionType: 'keyboard' }); await menuTester.open(); From 68c44daf8d5a432cd17b8b4c8812f4185c9f6f5f Mon Sep 17 00:00:00 2001 From: Rob Snow Date: Tue, 21 Jul 2026 16:52:53 +1000 Subject: [PATCH 5/5] add another test --- .../test/CloseWatcher.browser.test.tsx | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/packages/react-aria-components/test/CloseWatcher.browser.test.tsx b/packages/react-aria-components/test/CloseWatcher.browser.test.tsx index 688dbfdd41d..a0805ad49d6 100644 --- a/packages/react-aria-components/test/CloseWatcher.browser.test.tsx +++ b/packages/react-aria-components/test/CloseWatcher.browser.test.tsx @@ -186,6 +186,42 @@ describe('CloseWatcher dismissal', () => { await expect.element(screen.getByRole('dialog')).not.toBeInTheDocument(); }); + it('a non-dismissable modal on top blocks Escape from closing the modal beneath it', async () => { + // Outer (dismissable) opens on mount; inner (isKeyboardDismissDisabled) is + // opened on top. Escape must NOT close the outer modal past the inner one. + let screen = await render( + + + + + Outer + + + + + + Inner + + + + + + + + + ); + await userEvent.click(screen.getByRole('button', {name: 'Open inner'})); + await expect.poll(() => screen.getByRole('dialog').elements().length).toBe(2); + let innerContent = screen.getByRole('button', {name: 'Inner focus'}); + await expect.poll(() => document.activeElement).toBe(innerContent.element()); + + // Escape does nothing: the non-dismissable inner modal absorbs/blocks the + // close request, so neither modal closes. + await press('Escape'); + await press('Escape'); + await expect.poll(() => screen.getByRole('dialog').elements().length).toBe(2); + }); + it('Escape clears the autocomplete input, then closes the popover, then the dialog', async () => { function Example() { let {contains} = useFilter({sensitivity: 'base'});