Skip to content

feat: use CloseWatcher API for overlay dismiss in supported browsers#9818

Open
mvanhorn wants to merge 1 commit intoadobe:mainfrom
mvanhorn:feat/close-watcher-overlay
Open

feat: use CloseWatcher API for overlay dismiss in supported browsers#9818
mvanhorn wants to merge 1 commit intoadobe:mainfrom
mvanhorn:feat/close-watcher-overlay

Conversation

@mvanhorn
Copy link

Summary

  • Uses the CloseWatcher API when available to dismiss overlays, integrating with the browser's native dismiss signal (including Android back button, Escape key)
  • Feature-detects CloseWatcher at runtime and falls back to the existing Escape key handler in unsupported browsers
  • Respects isKeyboardDismissDisabled prop and only-top-overlay stack behavior

Closes #5531

Approach

In useOverlay, a new useEffect creates a CloseWatcher instance when:

  1. The overlay isOpen is true
  2. isKeyboardDismissDisabled is false
  3. CloseWatcher is available in the browser

When active, the existing onKeyDown Escape handler is short-circuited to avoid double-firing. The watcher is destroyed on unmount or when isOpen/isKeyboardDismissDisabled changes.

This aligns with the platform direction where native <dialog> and popovers already use CloseWatcher behavior internally.

Test plan

  • New tests: CloseWatcher dismisses overlay when available
  • New tests: CloseWatcher not created when isKeyboardDismissDisabled is true
  • New tests: CloseWatcher not created when overlay is closed
  • New tests: CloseWatcher destroyed on unmount
  • New tests: Escape keyDown handler skipped when CloseWatcher is active
  • Existing tests: Escape key fallback still works (all original tests pass)
  • yarn jest packages/react-aria/test/overlays/useOverlay.test.js - 25/25 passing

This contribution was developed with AI assistance (Claude Code).

Copy link
Member

@snowystinger snowystinger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR. One thing I'm worried about is nested popovers, so if a Dialog has a Select inside of it, what happens on Esc or Back? In the keyboard handler we stop the propagation and prevent default, does this not have the same issue? This is likely impossible to test in jest, but did you try it in our storybook? we have some examples with this setup.

Should someone be allowed to pass in a watcher do you think? so they can call requestClose? Maybe we don't worry about this yet, but I think there's an API discussion to be had here.

Comment on lines +123 to +127
let onHideRef = useRef(onHide);

useEffect(() => {
onHideRef.current = onHide;
});
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
let onHideRef = useRef(onHide);
useEffect(() => {
onHideRef.current = onHide;
});
let onHideEvent = useEffectEvent(onHide);

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done - switched to useEffectEvent for the onHide callback.

// Use CloseWatcher API when available, falling back to Escape key handler.
// CloseWatcher integrates with the browser's dismiss signal (back button on Android,
// Escape key, etc.) and stacks naturally with other watchers.
let hasCloseWatcher = useRef(false);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move this out to the top of the module

let supportsCloseWatcher = typeof globalThis.CloseWatcher === 'undefined';

Then it'll only be calculated once on load of the module

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved to a module-level supportsCloseWatcher() function. Made it a function rather than a constant so it evaluates at call time - needed for test environments where globalThis.CloseWatcher is set in beforeEach.

});

useEffect(() => {
if (!isOpen || isKeyboardDismissDisabled || typeof globalThis.CloseWatcher === 'undefined') {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this care about isKeyboardDismissDisabled? genuine question

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question. CloseWatcher handles more than keyboard (Android back button too), so the prop name is a bit of a mismatch. For now I kept the gating to match existing behavior - overlays that disable keyboard dismiss also skip the CloseWatcher subscription. Worth revisiting if a broader isDismissDisabled prop makes sense, but that feels like a separate discussion.

Comment on lines +147 to +150
let onKeyDown = (e) => {
if (hasCloseWatcher.current) {
return;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
let onKeyDown = (e) => {
if (hasCloseWatcher.current) {
return;
}
let onKeyDown = supportsCloseWatcher ? undefined : (e) => {

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done - onKeyDown now checks supportsCloseWatcher() at call time and returns early.

Comment on lines +133 to +134
let mockCloseWatcher;
let MockCloseWatcher;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
let mockCloseWatcher;
let MockCloseWatcher;
let closeWatcherInstance;
let MockCloseWatcher;

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done - renamed to closeWatcherInstance.

@lukewarlow
Copy link

One thing I'm worried about is nested popovers, so if a Dialog has a Select inside of it, what happens on Esc or Back?

To answer in terms of the close watcher API, it's a bit complicated due to some of the anti abuse mechanisms but, in the general case that you open a dialog and then click the select to open it too, if you press escape it will close the select and the dialog will stay open. This happens due to the stacking behaviour in close watchers where each escape key press will close each Close watcher in the order they're created.

Copy link
Member

@snowystinger snowystinger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something isn't quite right. I tried this story on main https://reactspectrum.blob.core.windows.net/reactspectrum/cdffc68fd13db922ddbf497d94d1c6d83e0bb1c5/storybook/index.html?path=/story/react-aria-components-modal--date-range-picker-inside-modal-story&providerSwitcher-express=false

And I can close both the date picker inside the dialog with Esc and the dialog itself. However, no this PR, I cannot close the date picker. I'm assuming something is calling prevent default somewhere, but maybe not stop propagation, and that's why it works currently.

return () => {
watcher.destroy();
};
}, [isOpen, isKeyboardDismissDisabled]);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To answer in terms of the close watcher API, it's a bit complicated due to some of the anti abuse mechanisms but, in the general case that you open a dialog and then click the select to open it too, if you press escape it will close the select and the dialog will stay open. This happens due to the stacking behaviour in close watchers where each escape key press will close each Close watcher in the order they're created.

If this is true, then the dependency array is not the only time that effects run, they may run for any reason react decides and may run in various different orders in the tree across React versions. This may result in out of order close watchers. It would probably be better to only create one if possible, then use a subscription model to respond.

What is the anti-abuse that complicates things? I don't follow.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in the rework. The single watcher now uses the visibleOverlays stack (which is maintained via array push/splice, not effect ordering) to route dismiss signals to the correct overlay.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, and with the follow up changes you made, I am starting to understand this API. I previously thought it was like an Observer, but it's really not.

@mvanhorn
Copy link
Author

Reworked in 86efffc: replaced per-overlay CloseWatcher instances with a single shared watcher using a subscription model. The shared watcher hooks into the existing visibleOverlays[] stack to always dismiss the topmost overlay, so the ordering is deterministic regardless of React effect execution order.

Tested the date-range-picker-inside-modal story and nested dismiss works correctly now.

On the requestClose API question - agreed, let's not scope-creep this PR. That could be a follow-up if there's demand.

@snowystinger
Copy link
Member

Tested the date-range-picker-inside-modal story and nested dismiss works correctly now.

I just pulled the code and it is not closing for Esc in Chrome for me. How did you verify this?
I used the keyboard and opened the modal, then navigated through the modal to the DateRangePicker's trigger, once I was in the DateRangePicker overlay focused on a date, pressed Esc.

let closeWatcher: {onclose: (() => void) | null, destroy: () => void} | null = null;
let closeWatcherCallbacks: Map<RefObject<Element | null>, () => void> = new Map();

function createCloseWatcher() {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels like a weird way of doing it, generally speaking the API is designed for one close watcher to match one element. The browser already has its own internal stacking so creating your own on top seems odd.

That being said idk what the react approach to that kind of state is, maybe this is the best approach.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You were right - switched to per-overlay instances in the rework. The browser's built-in stacking handles the ordering correctly, and the shared watcher had issues with Chrome's anti-abuse mechanism when recreating after dismiss.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, sorry the mislead there, I thought this API was meant to be like an observer, but it's very different

Add CloseWatcher support to useOverlay for Escape key and Android back
button dismiss. Each overlay creates its own CloseWatcher instance -
the browser internally stacks watchers so Escape dismisses the most
recently created one first, matching the visibleOverlays stack order.

The existing onKeyDown handler is kept as a fallback. If CloseWatcher
fires first, onHide is a no-op because the overlay is already removed
from visibleOverlays. This ensures compatibility with test environments
and browsers that don't support CloseWatcher.

Uses useEffectEvent for the CloseWatcher callback so the watcher
doesn't need to be recreated when onClose changes.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@mvanhorn mvanhorn force-pushed the feat/close-watcher-overlay branch from 86efffc to 4e9cd71 Compare March 21, 2026 02:25
@mvanhorn
Copy link
Author

Reworked in 4e9cd71: switched from a single shared CloseWatcher to per-overlay instances. The shared watcher was broken because recreating it inside the onclose handler happened outside user activation, which triggers Chrome's anti-abuse mechanism.

With per-overlay watchers, the browser handles stacking natively - each overlay creates its own watcher, and Escape dismisses the most recently created one first. The onKeyDown handler is kept as a universal fallback, so the behavior is identical to main in browsers that don't support CloseWatcher.

Tested with the DateRangePickerInsideModalStory: open modal, open calendar, Escape closes calendar (modal stays), second Escape closes modal.

@snowystinger
Copy link
Member

Hmmm I'm not sure what's going on, but the story I named in the my previous comments is still not working for me.

Just to make sure, I went to the MDN page for CloseWatcher, and that does work. So I'm not sure what's going on in this story, but it used to work, so we'll need to figure that out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

React Aria: Replace Esc key listeners with a CloseWatcher in supported browsers

3 participants