From 433441c2fab90ebbdad07d97e19748ee764c4a2f Mon Sep 17 00:00:00 2001 From: Koushik Nakka Date: Wed, 1 Apr 2026 17:02:30 -0400 Subject: [PATCH 1/6] fix(Modal): allow Escape to close ContextualMenu before closing Modal When a ContextualMenu is rendered inside a Modal, pressing Escape failed to close the menu because Modal's handleEscKey called stopImmediatePropagation() unconditionally. Since ContextualMenu renders via a Portal (a DOM sibling), its document-level keydown listener was silenced before it could run. Fix by checking for an open contextual menu dropdown (.p-contextual-menu__dropdown[aria-hidden="false"]) before stopping propagation. If one exists, the Escape event is allowed to continue so the menu's own handler can close it first. A subsequent Escape press will then close the Modal as expected. Fixes #1305 --- src/components/Modal/Modal.test.tsx | 36 +++++++++++++++++++++++++++++ src/components/Modal/Modal.tsx | 13 +++++++++++ 2 files changed, 49 insertions(+) diff --git a/src/components/Modal/Modal.test.tsx b/src/components/Modal/Modal.test.tsx index 7a423235e..c673eda58 100644 --- a/src/components/Modal/Modal.test.tsx +++ b/src/components/Modal/Modal.test.tsx @@ -4,6 +4,7 @@ import React, { FC, useEffect, useState } from "react"; import Button from "components/Button"; import Input from "components/Input"; +import ContextualMenu from "components/ContextualMenu"; import Modal from "./Modal"; describe("Modal ", () => { @@ -223,6 +224,41 @@ describe("Modal ", () => { expect(input).toHaveValue("delete item1"); }); + it("should allow Escape to close a ContextualMenu inside the modal before closing the modal", async () => { + const handleCloseModal = jest.fn(); + const handleMenuToggle = jest.fn(); + + render( + + + , + ); + + // The contextual menu dropdown should be open (aria-hidden="false") + const dropdown = document.querySelector( + '.p-contextual-menu__dropdown[aria-hidden="false"]', + ); + expect(dropdown).toBeInTheDocument(); + + // Press Escape — should close the menu, not the modal + await userEvent.keyboard("{Escape}"); + + // The modal close handler should NOT have been called + expect(handleCloseModal).not.toHaveBeenCalled(); + + // The dropdown should now be closed (no longer present with aria-hidden="false") + expect( + document.querySelector( + '.p-contextual-menu__dropdown[aria-hidden="false"]', + ), + ).not.toBeInTheDocument(); + }); + it("updates focusable elements when an initially disabled button becomes enabled", async () => { const user = userEvent.setup(); diff --git a/src/components/Modal/Modal.tsx b/src/components/Modal/Modal.tsx index 0294a101e..7ca08f566 100644 --- a/src/components/Modal/Modal.tsx +++ b/src/components/Modal/Modal.tsx @@ -91,6 +91,19 @@ export const Modal = ({ const handleEscKey = ( event: KeyboardEvent | React.KeyboardEvent, ) => { + // If there is an open contextual menu dropdown inside (or alongside) this + // modal, let the Escape event propagate so that the menu's own keydown + // handler can close it first. The modal should only intercept Escape when + // no child overlay is open. This fixes the bug where a ContextualMenu + // rendered inside a Modal cannot be closed with the Escape key because + // stopImmediatePropagation() was called unconditionally, preventing the + // menu's document-level keydown listener from ever firing. + const hasOpenDropdown = !!document.querySelector( + '.p-contextual-menu__dropdown[aria-hidden="false"]', + ); + if (hasOpenDropdown) { + return; + } if ("nativeEvent" in event && event.nativeEvent.stopImmediatePropagation) { event.nativeEvent.stopImmediatePropagation(); } else if ("stopImmediatePropagation" in event) { From 0b0d300673e2dd7ffec28ef03284adf9e5fc62f2 Mon Sep 17 00:00:00 2001 From: Koushik Nakka Date: Wed, 29 Apr 2026 16:26:29 -0400 Subject: [PATCH 2/6] fix(Modal): replace CSS-selector guard with a global LIFO escape-key stack MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses review feedback on #1339. The previous approach queried the DOM for a ContextualMenu-specific selector, making the fix tightly coupled to one component and invisible to custom consumer overlays. This replaces it with a component-agnostic solution used by every major overlay library (Radix, Headless UI, etc.): a module-level LIFO handler stack. - Add src/hooks/useEscapeStack.ts — exports pushEscapeHandler (low-level) and useEscapeStack (React hook). Only the top-of-stack handler fires per Escape keypress; stopImmediatePropagation() blocks raw document listeners. - Rewrite useOnEscapePressed to delegate to pushEscapeHandler. - Rewrite Modal to use useOnEscapePressed; remove Escape from its keydown map. - Rewrite usePortal to register on the stack only while the portal is open, so the LIFO order faithfully reflects the visual overlay stack. Result: the most recently opened overlay always handles Escape first, regardless of component type, DOM position, or portal placement. --- src/components/Modal/Modal.test.tsx | 36 +++++++-------- src/components/Modal/Modal.tsx | 39 +++-------------- src/external/usePortal.ts | 21 +++++---- src/hooks/index.ts | 1 + src/hooks/useEscapeStack.test.ts | 68 +++++++++++++++++++++++++++++ src/hooks/useEscapeStack.ts | 61 ++++++++++++++++++++++++++ src/hooks/useOnEscapePressed.ts | 24 ++++------ src/index.ts | 2 + 8 files changed, 171 insertions(+), 81 deletions(-) create mode 100644 src/hooks/useEscapeStack.test.ts create mode 100644 src/hooks/useEscapeStack.ts diff --git a/src/components/Modal/Modal.test.tsx b/src/components/Modal/Modal.test.tsx index c673eda58..1426000aa 100644 --- a/src/components/Modal/Modal.test.tsx +++ b/src/components/Modal/Modal.test.tsx @@ -105,7 +105,11 @@ describe("Modal ", () => { expect(closeButton).toHaveFocus(); }); - it("should stop immediate Esc press propagation", async () => { + it("should stop immediate Esc press propagation to other document listeners", async () => { + // A sibling component that registers a raw document keydown listener. + // It should NOT fire when the Modal handles Escape, because the global + // escape stack calls stopImmediatePropagation() before any other + // document listeners run. const MockEscEventComponent = ({ onEscPress, }: { @@ -113,7 +117,7 @@ describe("Modal ", () => { }): React.JSX.Element => { useEffect(() => { const handleEscPress = (e: KeyboardEvent) => { - if (e.code === "Escape") { + if (e.key === "Escape") { onEscPress(); } }; @@ -224,39 +228,31 @@ describe("Modal ", () => { expect(input).toHaveValue("delete item1"); }); - it("should allow Escape to close a ContextualMenu inside the modal before closing the modal", async () => { + it("should allow Escape to close an open overlay inside the modal before closing the modal", async () => { + // Regression test for https://github.com/canonical/react-components/issues/1305 + // + // Both Modal and ContextualMenu register on the global escape-key stack. + // The menu is opened after the modal, so its handler sits on top of the + // stack and must be called first (LIFO order). const handleCloseModal = jest.fn(); - const handleMenuToggle = jest.fn(); render( , ); - // The contextual menu dropdown should be open (aria-hidden="false") - const dropdown = document.querySelector( - '.p-contextual-menu__dropdown[aria-hidden="false"]', - ); - expect(dropdown).toBeInTheDocument(); - - // Press Escape — should close the menu, not the modal + // First Escape — should dismiss the ContextualMenu, not the Modal await userEvent.keyboard("{Escape}"); - - // The modal close handler should NOT have been called expect(handleCloseModal).not.toHaveBeenCalled(); - // The dropdown should now be closed (no longer present with aria-hidden="false") - expect( - document.querySelector( - '.p-contextual-menu__dropdown[aria-hidden="false"]', - ), - ).not.toBeInTheDocument(); + // Second Escape — menu is gone, so the Modal should now close + await userEvent.keyboard("{Escape}"); + expect(handleCloseModal).toHaveBeenCalledTimes(1); }); it("updates focusable elements when an initially disabled button becomes enabled", async () => { diff --git a/src/components/Modal/Modal.tsx b/src/components/Modal/Modal.tsx index 7ca08f566..901471e71 100644 --- a/src/components/Modal/Modal.tsx +++ b/src/components/Modal/Modal.tsx @@ -2,6 +2,7 @@ import classNames from "classnames"; import React, { useId, useRef, useEffect } from "react"; import type { HTMLProps, ReactNode, RefObject } from "react"; import { ClassName, PropsWithSpread } from "types"; +import { useOnEscapePressed } from "hooks"; export type Props = PropsWithSpread< { @@ -88,33 +89,7 @@ export const Modal = ({ } }; - const handleEscKey = ( - event: KeyboardEvent | React.KeyboardEvent, - ) => { - // If there is an open contextual menu dropdown inside (or alongside) this - // modal, let the Escape event propagate so that the menu's own keydown - // handler can close it first. The modal should only intercept Escape when - // no child overlay is open. This fixes the bug where a ContextualMenu - // rendered inside a Modal cannot be closed with the Escape key because - // stopImmediatePropagation() was called unconditionally, preventing the - // menu's document-level keydown listener from ever firing. - const hasOpenDropdown = !!document.querySelector( - '.p-contextual-menu__dropdown[aria-hidden="false"]', - ); - if (hasOpenDropdown) { - return; - } - if ("nativeEvent" in event && event.nativeEvent.stopImmediatePropagation) { - event.nativeEvent.stopImmediatePropagation(); - } else if ("stopImmediatePropagation" in event) { - event.stopImmediatePropagation(); - } else if (event.stopPropagation) { - event.stopPropagation(); - } - if (close) { - close(); - } - }; + useOnEscapePressed(() => close?.(), { isEnabled: !!close }); useEffect(() => { if (focusRef?.current) { @@ -127,14 +102,10 @@ export const Modal = ({ }, [focusRef]); useEffect(() => { - const keyListenersMap = new Map([ - ["Escape", handleEscKey], - ["Tab", handleTabKey], - ]); - const keyDown = (event: KeyboardEvent) => { - const listener = keyListenersMap.get(event.code); - return listener && listener(event); + if (event.code === "Tab") { + handleTabKey(event as unknown as React.KeyboardEvent); + } }; document.addEventListener("keydown", keyDown); diff --git a/src/external/usePortal.ts b/src/external/usePortal.ts index 0b30cc9d8..6fe270017 100644 --- a/src/external/usePortal.ts +++ b/src/external/usePortal.ts @@ -13,6 +13,7 @@ import { RefObject, MouseEvent, } from "react"; +import { pushEscapeHandler } from "../hooks/useEscapeStack"; import { createPortal } from "react-dom"; import { useSSR } from "./useSSR"; @@ -126,14 +127,6 @@ export const usePortal = ({ [closePortal, openPortal], ); - const handleKeydown = useCallback( - (e: KeyboardEvent): void => - e.key === "Escape" && closeOnEsc - ? closePortal(e as unknown as SyntheticEvent) - : undefined, - [closeOnEsc, closePortal], - ); - const handleOutsideMouseClick = useCallback( (e: MouseEvent): void => { const containsTarget = (target: RefObject) => @@ -179,21 +172,27 @@ export const usePortal = ({ const node = portal.current; elToMountTo.appendChild(portal.current); - document.addEventListener("keydown", handleKeydown); document.addEventListener( "mousedown", handleMouseDown as unknown as EventListener, ); return () => { - document.removeEventListener("keydown", handleKeydown); document.removeEventListener( "mousedown", handleMouseDown as unknown as EventListener, ); elToMountTo.removeChild(node); }; - }, [isServer, handleOutsideMouseClick, handleKeydown, elToMountTo, portal]); + }, [isServer, handleOutsideMouseClick, elToMountTo, portal]); + + // Register on the global escape-key stack only while the portal is open. + // LIFO ordering ensures the most recently opened overlay always handles + // Escape first, regardless of component type or DOM structure. + useEffect(() => { + if (isServer || !closeOnEsc || !isOpen) return undefined; + return pushEscapeHandler(() => closePortal()); + }, [isOpen, closeOnEsc, isServer, closePortal]); const Portal = useCallback( ({ children }: { children: ReactNode }) => { diff --git a/src/hooks/index.ts b/src/hooks/index.ts index 2d5a700f0..8085fd74a 100644 --- a/src/hooks/index.ts +++ b/src/hooks/index.ts @@ -1,4 +1,5 @@ export { useOnClickOutside, useClickOutside } from "./useOnClickOutside"; +export { pushEscapeHandler, useEscapeStack } from "./useEscapeStack"; export { useId } from "./useId"; export { useListener } from "./useListener"; export { useOnEscapePressed } from "./useOnEscapePressed"; diff --git a/src/hooks/useEscapeStack.test.ts b/src/hooks/useEscapeStack.test.ts new file mode 100644 index 000000000..4a644fd93 --- /dev/null +++ b/src/hooks/useEscapeStack.test.ts @@ -0,0 +1,68 @@ +import { pushEscapeHandler } from "./useEscapeStack"; + +const fireEscape = () => + document.dispatchEvent(new KeyboardEvent("keydown", { key: "Escape" })); + +afterEach(() => { + // Each test unregisters its own handlers via the returned cleanup functions, + // so nothing to do here — this guard is just for safety. +}); + +describe("pushEscapeHandler", () => { + it("calls the registered handler when Escape is pressed", () => { + const handler = jest.fn(); + const unregister = pushEscapeHandler(handler); + fireEscape(); + expect(handler).toHaveBeenCalledTimes(1); + unregister(); + }); + + it("does not call the handler after it is unregistered", () => { + const handler = jest.fn(); + const unregister = pushEscapeHandler(handler); + unregister(); + fireEscape(); + expect(handler).not.toHaveBeenCalled(); + }); + + it("calls only the most recently pushed handler (LIFO order)", () => { + const first = jest.fn(); + const second = jest.fn(); + const unregisterFirst = pushEscapeHandler(first); + const unregisterSecond = pushEscapeHandler(second); + + fireEscape(); + expect(second).toHaveBeenCalledTimes(1); + expect(first).not.toHaveBeenCalled(); + + unregisterSecond(); + unregisterFirst(); + }); + + it("falls back to the previous handler once the top handler is removed", () => { + const first = jest.fn(); + const second = jest.fn(); + const unregisterFirst = pushEscapeHandler(first); + const unregisterSecond = pushEscapeHandler(second); + + unregisterSecond(); + fireEscape(); + expect(first).toHaveBeenCalledTimes(1); + expect(second).not.toHaveBeenCalled(); + + unregisterFirst(); + }); + + it("stops propagation so other document keydown listeners do not fire", () => { + const outsideListener = jest.fn(); + // Register an external listener AFTER the stack listener would be added. + const unregister = pushEscapeHandler(jest.fn()); + document.addEventListener("keydown", outsideListener); + + fireEscape(); + expect(outsideListener).not.toHaveBeenCalled(); + + document.removeEventListener("keydown", outsideListener); + unregister(); + }); +}); diff --git a/src/hooks/useEscapeStack.ts b/src/hooks/useEscapeStack.ts new file mode 100644 index 000000000..7558b15a1 --- /dev/null +++ b/src/hooks/useEscapeStack.ts @@ -0,0 +1,61 @@ +/** + * A module-level LIFO stack of Escape-key handlers. + * + * Any overlay component (Modal, ContextualMenu, Drawer, …) can register a + * handler here. Only the most recently registered handler fires when Escape + * is pressed, so nested overlays always dismiss in the correct order + * regardless of their DOM position or portal placement. + */ + +import { useEffect } from "react"; + +const handlers: Array<() => void> = []; + +function onKeyDown(e: KeyboardEvent): void { + if (e.key !== "Escape" || handlers.length === 0) return; + e.stopImmediatePropagation(); + handlers[handlers.length - 1](); +} + +/** + * Push a callback onto the global escape-key stack. + * Returns a cleanup function that removes the handler (suitable for + * use as a `useEffect` cleanup return). + * Handlers are invoked in LIFO order — the last one pushed runs first, + * mirroring the visual stacking of overlays. + */ +export function pushEscapeHandler(handler: () => void): () => void { + if (handlers.length === 0) { + document.addEventListener("keydown", onKeyDown); + } + handlers.push(handler); + return () => { + const idx = handlers.lastIndexOf(handler); + if (idx !== -1) handlers.splice(idx, 1); + if (handlers.length === 0) { + document.removeEventListener("keydown", onKeyDown); + } + }; +} + +/** + * React hook that registers an Escape-key handler on the global LIFO stack + * for the lifetime of the component (or while `isActive` is true). + * + * The most recently registered handler always fires first, so nested overlays + * naturally dismiss in the correct order regardless of DOM structure. + * + * @param handler - Callback invoked when Escape is pressed and this handler + * is at the top of the stack. + * @param options.isActive - When `false` the handler is not registered + * (defaults to `true`). + */ +export const useEscapeStack = ( + handler: () => void, + { isActive } = { isActive: true }, +): void => { + useEffect(() => { + if (!isActive) return undefined; + return pushEscapeHandler(handler); + }, [handler, isActive]); +}; diff --git a/src/hooks/useOnEscapePressed.ts b/src/hooks/useOnEscapePressed.ts index a485de891..762dc3908 100644 --- a/src/hooks/useOnEscapePressed.ts +++ b/src/hooks/useOnEscapePressed.ts @@ -1,26 +1,18 @@ -import { useCallback, useEffect } from "react"; +import { useEffect } from "react"; +import { pushEscapeHandler } from "./useEscapeStack"; /** * Handle the escape key pressed. + * Registers the callback on the global escape-key stack so that nested + * overlays (modals, dropdowns, drawers, …) always dismiss in the correct + * LIFO order, regardless of DOM position or portal placement. */ export const useOnEscapePressed = ( onEscape: () => void, { isEnabled } = { isEnabled: true }, ) => { - const keyDown = useCallback( - (evt: KeyboardEvent) => { - if (evt.code === "Escape") { - onEscape(); - } - }, - [onEscape], - ); useEffect(() => { - if (isEnabled) { - document.addEventListener("keydown", keyDown); - } - return () => { - document.removeEventListener("keydown", keyDown); - }; - }, [keyDown, isEnabled]); + if (!isEnabled) return undefined; + return pushEscapeHandler(onEscape); + }, [onEscape, isEnabled]); }; diff --git a/src/index.ts b/src/index.ts index 50d9ec989..e2c658e05 100644 --- a/src/index.ts +++ b/src/index.ts @@ -223,6 +223,8 @@ export type { } from "./components/CustomSelect"; export { + pushEscapeHandler, + useEscapeStack, useOnClickOutside, useClickOutside, useId, From 16813f7ffa7241dced9e9cefa8a653fe9bdb0a9a Mon Sep 17 00:00:00 2001 From: Koushik Nakka Date: Tue, 9 Jun 2026 17:01:51 -0400 Subject: [PATCH 3/6] fix(useEscapeStack): address Copilot review feedback - Use capture phase in pushEscapeHandler so the stack fires before any bubble-phase listeners and cannot be bypassed by stopPropagation() on a focused child element - Guard pushEscapeHandler for SSR (no-op when document is undefined) - Use a ref in useEscapeStack and useOnEscapePressed so inline callbacks do not cause re-registration on every re-render, which would incorrectly reorder the LIFO stack - Use a ref for closePortal in usePortal so identity changes to onClose prop do not trigger unnecessary re-registration - Update regression test to open ContextualMenu via click (after Modal mounts) so effect registration order reflects real-world usage --- src/components/Modal/Modal.test.tsx | 14 +++++++++----- src/external/usePortal.ts | 13 +++++++++++-- src/hooks/useEscapeStack.ts | 28 +++++++++++++++++++++++----- src/hooks/useOnEscapePressed.ts | 15 ++++++++++++--- 4 files changed, 55 insertions(+), 15 deletions(-) diff --git a/src/components/Modal/Modal.test.tsx b/src/components/Modal/Modal.test.tsx index 1426000aa..b9d18d00b 100644 --- a/src/components/Modal/Modal.test.tsx +++ b/src/components/Modal/Modal.test.tsx @@ -232,8 +232,9 @@ describe("Modal ", () => { // Regression test for https://github.com/canonical/react-components/issues/1305 // // Both Modal and ContextualMenu register on the global escape-key stack. - // The menu is opened after the modal, so its handler sits on top of the - // stack and must be called first (LIFO order). + // The menu is opened *after* the modal mounts (via a click), so its handler + // sits on top of the stack and must be called first (LIFO order). + const user = userEvent.setup(); const handleCloseModal = jest.fn(); render( @@ -241,17 +242,20 @@ describe("Modal ", () => { , ); + // Open the ContextualMenu after the modal is already mounted — + // this pushes its escape handler on top of the modal's in the LIFO stack. + await user.click(screen.getByRole("button", { name: /open menu/i })); + // First Escape — should dismiss the ContextualMenu, not the Modal - await userEvent.keyboard("{Escape}"); + await user.keyboard("{Escape}"); expect(handleCloseModal).not.toHaveBeenCalled(); // Second Escape — menu is gone, so the Modal should now close - await userEvent.keyboard("{Escape}"); + await user.keyboard("{Escape}"); expect(handleCloseModal).toHaveBeenCalledTimes(1); }); diff --git a/src/external/usePortal.ts b/src/external/usePortal.ts index 6fe270017..875324c15 100644 --- a/src/external/usePortal.ts +++ b/src/external/usePortal.ts @@ -186,13 +186,22 @@ export const usePortal = ({ }; }, [isServer, handleOutsideMouseClick, elToMountTo, portal]); + // Keep a ref to closePortal so the escape-stack effect below does not need + // it as a dependency. This prevents closePortal identity changes (e.g. when + // onClose prop changes) from re-registering the handler and incorrectly + // moving an already-open portal to the top of the LIFO stack. + const closePortalRef = useRef(closePortal); + useEffect(() => { + closePortalRef.current = closePortal; + }, [closePortal]); + // Register on the global escape-key stack only while the portal is open. // LIFO ordering ensures the most recently opened overlay always handles // Escape first, regardless of component type or DOM structure. useEffect(() => { if (isServer || !closeOnEsc || !isOpen) return undefined; - return pushEscapeHandler(() => closePortal()); - }, [isOpen, closeOnEsc, isServer, closePortal]); + return pushEscapeHandler(() => closePortalRef.current()); + }, [isOpen, closeOnEsc, isServer]); const Portal = useCallback( ({ children }: { children: ReactNode }) => { diff --git a/src/hooks/useEscapeStack.ts b/src/hooks/useEscapeStack.ts index 7558b15a1..e62e0402a 100644 --- a/src/hooks/useEscapeStack.ts +++ b/src/hooks/useEscapeStack.ts @@ -7,10 +7,12 @@ * regardless of their DOM position or portal placement. */ -import { useEffect } from "react"; +import { useEffect, useRef } from "react"; const handlers: Array<() => void> = []; +// Use capture phase so the stack fires before any bubble-phase listeners +// and cannot be silenced by stopPropagation() on a focused child element. function onKeyDown(e: KeyboardEvent): void { if (e.key !== "Escape" || handlers.length === 0) return; e.stopImmediatePropagation(); @@ -23,17 +25,21 @@ function onKeyDown(e: KeyboardEvent): void { * use as a `useEffect` cleanup return). * Handlers are invoked in LIFO order — the last one pushed runs first, * mirroring the visual stacking of overlays. + * + * Safe to call in SSR environments — it no-ops when `document` is unavailable. */ export function pushEscapeHandler(handler: () => void): () => void { + if (typeof document === "undefined") return () => undefined; if (handlers.length === 0) { - document.addEventListener("keydown", onKeyDown); + // capture: true — fires before bubble-phase listeners on any descendant + document.addEventListener("keydown", onKeyDown, true); } handlers.push(handler); return () => { const idx = handlers.lastIndexOf(handler); if (idx !== -1) handlers.splice(idx, 1); if (handlers.length === 0) { - document.removeEventListener("keydown", onKeyDown); + document.removeEventListener("keydown", onKeyDown, true); } }; } @@ -45,6 +51,10 @@ export function pushEscapeHandler(handler: () => void): () => void { * The most recently registered handler always fires first, so nested overlays * naturally dismiss in the correct order regardless of DOM structure. * + * A stable wrapper is kept in a ref so that inline callbacks passed by callers + * do not cause the handler to be popped/pushed on every re-render, which would + * incorrectly move the overlay to the top of the stack. + * * @param handler - Callback invoked when Escape is pressed and this handler * is at the top of the stack. * @param options.isActive - When `false` the handler is not registered @@ -54,8 +64,16 @@ export const useEscapeStack = ( handler: () => void, { isActive } = { isActive: true }, ): void => { + // Always keep the ref pointing at the latest handler without changing + // the registered stable wrapper. + const handlerRef = useRef(handler); + useEffect(() => { + handlerRef.current = handler; + }, [handler]); + useEffect(() => { if (!isActive) return undefined; - return pushEscapeHandler(handler); - }, [handler, isActive]); + // Register a stable wrapper; the ref always calls the latest handler. + return pushEscapeHandler(() => handlerRef.current()); + }, [isActive]); }; diff --git a/src/hooks/useOnEscapePressed.ts b/src/hooks/useOnEscapePressed.ts index 762dc3908..6e45adebd 100644 --- a/src/hooks/useOnEscapePressed.ts +++ b/src/hooks/useOnEscapePressed.ts @@ -1,4 +1,4 @@ -import { useEffect } from "react"; +import { useEffect, useRef } from "react"; import { pushEscapeHandler } from "./useEscapeStack"; /** @@ -6,13 +6,22 @@ import { pushEscapeHandler } from "./useEscapeStack"; * Registers the callback on the global escape-key stack so that nested * overlays (modals, dropdowns, drawers, …) always dismiss in the correct * LIFO order, regardless of DOM position or portal placement. + * + * A stable wrapper is kept in a ref so that inline callbacks do not cause + * the handler to be unregistered/re-registered on every re-render (which + * would incorrectly move this overlay to the top of the stack). */ export const useOnEscapePressed = ( onEscape: () => void, { isEnabled } = { isEnabled: true }, ) => { + const onEscapeRef = useRef(onEscape); + useEffect(() => { + onEscapeRef.current = onEscape; + }, [onEscape]); + useEffect(() => { if (!isEnabled) return undefined; - return pushEscapeHandler(onEscape); - }, [onEscape, isEnabled]); + return pushEscapeHandler(() => onEscapeRef.current()); + }, [isEnabled]); }; From 82eb54b1303822f4ab66bf8d9a9ff5680046ecc1 Mon Sep 17 00:00:00 2001 From: Koushik Nakka Date: Wed, 10 Jun 2026 14:24:38 -0400 Subject: [PATCH 4/6] fix(useEscapeStack): align naming, fix Navigation/SearchAndFilter stack regression, trim public API - Rename useEscapeStack's isActive option to isEnabled for consistency with useOnEscapePressed. - Navigation and SearchAndFilter now only register their escape handler while their own overlay (search box / filter panel) is actually open, fixing a regression where they could permanently occupy the top of the LIFO stack and swallow Escape presses meant for other overlays. - Remove pushEscapeHandler from the public package exports; only the useEscapeStack hook is part of the public API now, consistent with the rest of the hooks folder. --- src/components/Navigation/Navigation.test.tsx | 37 ++++++++++++ src/components/Navigation/Navigation.tsx | 2 +- .../SearchAndFilter/SearchAndFilter.test.tsx | 57 +++++++++++++++++++ .../SearchAndFilter/SearchAndFilter.tsx | 2 +- src/hooks/index.ts | 2 +- src/hooks/useEscapeStack.ts | 13 +++-- src/index.ts | 1 - 7 files changed, 105 insertions(+), 9 deletions(-) diff --git a/src/components/Navigation/Navigation.test.tsx b/src/components/Navigation/Navigation.test.tsx index ff3f66d69..94419167e 100644 --- a/src/components/Navigation/Navigation.test.tsx +++ b/src/components/Navigation/Navigation.test.tsx @@ -275,6 +275,43 @@ it("closes the search form when the escape key is pressed", async () => { expect(screen.queryByRole("searchbox")).not.toBeInTheDocument(); }); +it("does not occupy the escape stack while the search box is closed", async () => { + // Regression test: Navigation must only join the global escape-key stack + // while its search box is open. Otherwise it can sit on top of the LIFO + // stack indefinitely and swallow Escape presses meant for another overlay + // (e.g. a SearchAndFilter panel) opened elsewhere on the page. + const onEscPress = jest.fn(); + + const MockEscEventComponent = (): React.JSX.Element => { + React.useEffect(() => { + const handleEscPress = (e: KeyboardEvent) => { + if (e.key === "Escape") { + onEscPress(); + } + }; + document.addEventListener("keydown", handleEscPress); + return () => { + document.removeEventListener("keydown", handleEscPress); + }; + }); + return
Mock component with event on Esc key press
; + }; + + render( + <> + } + searchProps={{ onSearch: jest.fn() }} + /> + + , + ); + + expect(screen.queryByRole("searchbox")).not.toBeInTheDocument(); + fireEvent.keyDown(document, { key: "Escape", code: "Escape" }); + expect(onEscPress).toHaveBeenCalledTimes(1); +}); + it("closes the search form when opening the mobile menu", async () => { render( toggleSearch(false)); + useOnEscapePressed(() => toggleSearch(false), { isEnabled: searchOpen }); useEffect(() => { if (searchOpen) { diff --git a/src/components/SearchAndFilter/SearchAndFilter.test.tsx b/src/components/SearchAndFilter/SearchAndFilter.test.tsx index b918ccb30..83493d723 100644 --- a/src/components/SearchAndFilter/SearchAndFilter.test.tsx +++ b/src/components/SearchAndFilter/SearchAndFilter.test.tsx @@ -394,4 +394,61 @@ describe("Search and filter", () => { expect(onPanelToggle).not.toHaveBeenCalled(); }); + + it("closes the filter panel when escape is pressed", async () => { + const returnSearchData = jest.fn(); + render( + , + ); + await userEvent.click(screen.getByTestId("searchandfilter")); + expect(getPanel()).toHaveAttribute("aria-hidden", "false"); + + await userEvent.keyboard("{Escape}"); + expect(getPanel()).toHaveAttribute("aria-hidden", "true"); + }); + + it("does not occupy the escape stack while the panel is closed", async () => { + // Regression test for https://github.com/canonical/react-components/pull/1339#discussion_r1234 + // + // SearchAndFilter must only join the global escape-key stack while its + // panel is open. Otherwise, if it is mounted alongside another overlay + // (e.g. Navigation's search box) and registers unconditionally, it can + // sit on top of the LIFO stack and swallow Escape presses meant for that + // other overlay even though its own panel is closed. + const returnSearchData = jest.fn(); + const onEscPress = jest.fn(); + + const MockEscEventComponent = (): React.JSX.Element => { + React.useEffect(() => { + const handleEscPress = (e: KeyboardEvent) => { + if (e.key === "Escape") { + onEscPress(); + } + }; + document.addEventListener("keydown", handleEscPress); + return () => { + document.removeEventListener("keydown", handleEscPress); + }; + }); + return
Mock component with event on Esc key press
; + }; + + render( + <> + + + , + ); + + expect(getPanel()).toHaveAttribute("aria-hidden", "true"); + await userEvent.keyboard("{Escape}"); + expect(onEscPress).toHaveBeenCalledTimes(1); + }); }); diff --git a/src/components/SearchAndFilter/SearchAndFilter.tsx b/src/components/SearchAndFilter/SearchAndFilter.tsx index 1323f0b71..8cc004737 100644 --- a/src/components/SearchAndFilter/SearchAndFilter.tsx +++ b/src/components/SearchAndFilter/SearchAndFilter.tsx @@ -92,7 +92,7 @@ const SearchAndFilter = ({ const closePanel = () => { setFilterPanelHidden(true); }; - useOnEscapePressed(() => closePanel()); + useOnEscapePressed(() => closePanel(), { isEnabled: !filterPanelHidden }); // This useEffect sets up listeners so the panel will close if user clicks // anywhere else on the page or hits the escape key diff --git a/src/hooks/index.ts b/src/hooks/index.ts index 8085fd74a..4a0cd7fee 100644 --- a/src/hooks/index.ts +++ b/src/hooks/index.ts @@ -1,5 +1,5 @@ export { useOnClickOutside, useClickOutside } from "./useOnClickOutside"; -export { pushEscapeHandler, useEscapeStack } from "./useEscapeStack"; +export { useEscapeStack } from "./useEscapeStack"; export { useId } from "./useId"; export { useListener } from "./useListener"; export { useOnEscapePressed } from "./useOnEscapePressed"; diff --git a/src/hooks/useEscapeStack.ts b/src/hooks/useEscapeStack.ts index e62e0402a..23f517ea0 100644 --- a/src/hooks/useEscapeStack.ts +++ b/src/hooks/useEscapeStack.ts @@ -27,6 +27,9 @@ function onKeyDown(e: KeyboardEvent): void { * mirroring the visual stacking of overlays. * * Safe to call in SSR environments — it no-ops when `document` is unavailable. + * + * This is the imperative primitive behind `useEscapeStack` and is not part + * of the public package API. React consumers should use `useEscapeStack`. */ export function pushEscapeHandler(handler: () => void): () => void { if (typeof document === "undefined") return () => undefined; @@ -46,7 +49,7 @@ export function pushEscapeHandler(handler: () => void): () => void { /** * React hook that registers an Escape-key handler on the global LIFO stack - * for the lifetime of the component (or while `isActive` is true). + * for the lifetime of the component (or while `isEnabled` is true). * * The most recently registered handler always fires first, so nested overlays * naturally dismiss in the correct order regardless of DOM structure. @@ -57,12 +60,12 @@ export function pushEscapeHandler(handler: () => void): () => void { * * @param handler - Callback invoked when Escape is pressed and this handler * is at the top of the stack. - * @param options.isActive - When `false` the handler is not registered + * @param options.isEnabled - When `false` the handler is not registered * (defaults to `true`). */ export const useEscapeStack = ( handler: () => void, - { isActive } = { isActive: true }, + { isEnabled } = { isEnabled: true }, ): void => { // Always keep the ref pointing at the latest handler without changing // the registered stable wrapper. @@ -72,8 +75,8 @@ export const useEscapeStack = ( }, [handler]); useEffect(() => { - if (!isActive) return undefined; + if (!isEnabled) return undefined; // Register a stable wrapper; the ref always calls the latest handler. return pushEscapeHandler(() => handlerRef.current()); - }, [isActive]); + }, [isEnabled]); }; diff --git a/src/index.ts b/src/index.ts index e2c658e05..092094305 100644 --- a/src/index.ts +++ b/src/index.ts @@ -223,7 +223,6 @@ export type { } from "./components/CustomSelect"; export { - pushEscapeHandler, useEscapeStack, useOnClickOutside, useClickOutside, From 99f88d10051fbd005cf7d84fd86877ffa32831bf Mon Sep 17 00:00:00 2001 From: Koushik Nakka Date: Wed, 10 Jun 2026 22:35:54 -0400 Subject: [PATCH 5/6] fix(useOnEscapePressed): revert to pre-PR implementation, no breaking change useOnEscapePressed is restored to its original implementation (a plain per-component document keydown listener). It no longer participates in the global LIFO escape stack, so its behavior for all existing consumers (internal and external) is byte-for-byte identical to before this PR. Modal now joins the LIFO stack directly via useEscapeStack instead of useOnEscapePressed, since Modal is the component that actually needs LIFO-ordered dismissal alongside Portal-based overlays (ContextualMenu, etc. via usePortal). This decouples the new opt-in stack from the existing widely-consumed hook entirely, addressing the breaking-change concern for external consumers of useOnEscapePressed. --- src/components/Modal/Modal.tsx | 4 ++-- src/hooks/useOnEscapePressed.ts | 33 ++++++++++++++++----------------- 2 files changed, 18 insertions(+), 19 deletions(-) diff --git a/src/components/Modal/Modal.tsx b/src/components/Modal/Modal.tsx index a6bc61b25..24cc4cb6e 100644 --- a/src/components/Modal/Modal.tsx +++ b/src/components/Modal/Modal.tsx @@ -2,7 +2,7 @@ import classNames from "classnames"; import React, { useId, useRef, useEffect } from "react"; import type { HTMLProps, ReactNode, RefObject } from "react"; import { ClassName, PropsWithSpread } from "types"; -import { useOnEscapePressed } from "hooks"; +import { useEscapeStack } from "hooks"; export type Props = PropsWithSpread< { @@ -89,7 +89,7 @@ export const Modal = ({ } }; - useOnEscapePressed(() => close?.(), { isEnabled: !!close }); + useEscapeStack(() => close?.(), { isEnabled: !!close }); useEffect(() => { if (focusRef?.current) { diff --git a/src/hooks/useOnEscapePressed.ts b/src/hooks/useOnEscapePressed.ts index 6e45adebd..a485de891 100644 --- a/src/hooks/useOnEscapePressed.ts +++ b/src/hooks/useOnEscapePressed.ts @@ -1,27 +1,26 @@ -import { useEffect, useRef } from "react"; -import { pushEscapeHandler } from "./useEscapeStack"; +import { useCallback, useEffect } from "react"; /** * Handle the escape key pressed. - * Registers the callback on the global escape-key stack so that nested - * overlays (modals, dropdowns, drawers, …) always dismiss in the correct - * LIFO order, regardless of DOM position or portal placement. - * - * A stable wrapper is kept in a ref so that inline callbacks do not cause - * the handler to be unregistered/re-registered on every re-render (which - * would incorrectly move this overlay to the top of the stack). */ export const useOnEscapePressed = ( onEscape: () => void, { isEnabled } = { isEnabled: true }, ) => { - const onEscapeRef = useRef(onEscape); + const keyDown = useCallback( + (evt: KeyboardEvent) => { + if (evt.code === "Escape") { + onEscape(); + } + }, + [onEscape], + ); useEffect(() => { - onEscapeRef.current = onEscape; - }, [onEscape]); - - useEffect(() => { - if (!isEnabled) return undefined; - return pushEscapeHandler(() => onEscapeRef.current()); - }, [isEnabled]); + if (isEnabled) { + document.addEventListener("keydown", keyDown); + } + return () => { + document.removeEventListener("keydown", keyDown); + }; + }, [keyDown, isEnabled]); }; From d5bc53b98fccfb23c4fe14048130836d87fd9b63 Mon Sep 17 00:00:00 2001 From: Koushik Nakka Date: Thu, 11 Jun 2026 11:40:14 -0400 Subject: [PATCH 6/6] fix(useEscapeStack): make non-Modal overlays non-exclusive on the escape stack Copilot correctly identified that the previous implementation's stopImmediatePropagation() ran whenever the stack was non-empty, so a single open ContextualMenu/Dropdown (via usePortal) would silently block unrelated document keydown listeners elsewhere on the page, including useOnEscapePressed-based components and any external consumer of that hook. This was a real, broader regression than the original Modal-only behavior (Modal has always claimed Escape exclusively while open - this is pre-existing, intentional, and covered by an existing test). Each escape-stack entry now has an `exclusive` flag (default true): - Exclusive entries (Modal, via useEscapeStack) claim Escape entirely while on top of the stack, preserving Modal's long-standing behaviour. - Non-exclusive entries (portal-based overlays, via usePortal) handle their own dismissal but let the event continue propagating, so they don't silence unrelated listeners when used standalone. When a non-exclusive entry sits above an exclusive one (e.g. ContextualMenu opened inside a Modal), the first Escape is handled by the ContextualMenu without claiming the event, and the Modal underneath is skipped for that press - preserving the two-step dismiss fix for #1305. Also fixes a related Copilot finding: pushEscapeHandler now stores a unique entry object per registration instead of removing by handler-reference (lastIndexOf), so registering the same function reference more than once can no longer corrupt the stack. Added tests for exclusive/non-exclusive ordering, the duplicate-reference unregister case, and a regression test confirming a standalone ContextualMenu no longer blocks an unrelated useOnEscapePressed listener. --- .../ContextualMenu/ContextualMenu.test.tsx | 29 ++++++++ src/external/usePortal.ts | 10 ++- src/hooks/useEscapeStack.test.ts | 66 +++++++++++++++++ src/hooks/useEscapeStack.ts | 71 +++++++++++++++---- 4 files changed, 162 insertions(+), 14 deletions(-) diff --git a/src/components/ContextualMenu/ContextualMenu.test.tsx b/src/components/ContextualMenu/ContextualMenu.test.tsx index 5fd5122f7..05d7157ce 100644 --- a/src/components/ContextualMenu/ContextualMenu.test.tsx +++ b/src/components/ContextualMenu/ContextualMenu.test.tsx @@ -3,6 +3,7 @@ import React from "react"; import userEvent from "@testing-library/user-event"; import Button from "../Button"; +import { useOnEscapePressed } from "hooks"; import ContextualMenu, { Label } from "./ContextualMenu"; import { Label as DropdownLabel } from "./ContextualMenuDropdown/ContextualMenuDropdown"; @@ -16,6 +17,34 @@ describe("ContextualMenu ", () => { expect(screen.getByTestId("menu")).toMatchSnapshot(); }); + it("does not block other Escape handlers elsewhere on the page when open", async () => { + // Regression test: a standalone ContextualMenu (i.e. not nested inside a + // Modal) registers itself as a non-exclusive entry on the global escape + // stack. Opening it should not prevent unrelated useOnEscapePressed-based + // components elsewhere on the page from also responding to Escape. + const onEscape = jest.fn(); + const Other = (): React.JSX.Element => { + useOnEscapePressed(onEscape); + return
other
; + }; + + const user = userEvent.setup(); + render( + <> + + + , + ); + + await user.click(screen.getByRole("button", { name: /open menu/i })); + await user.keyboard("{Escape}"); + + expect(onEscape).toHaveBeenCalledTimes(1); + }); + it("can be aligned to the right", () => { render(); const menu = screen.getByTestId("menu"); diff --git a/src/external/usePortal.ts b/src/external/usePortal.ts index 875324c15..64eef99e8 100644 --- a/src/external/usePortal.ts +++ b/src/external/usePortal.ts @@ -198,9 +198,17 @@ export const usePortal = ({ // Register on the global escape-key stack only while the portal is open. // LIFO ordering ensures the most recently opened overlay always handles // Escape first, regardless of component type or DOM structure. + // + // Registered as non-exclusive: this portal closes itself on Escape, but + // does not stop the event from propagating to unrelated `document` + // keydown listeners (e.g. useOnEscapePressed-based components elsewhere + // on the page). Exclusive ownership of Escape (e.g. while a Modal is open) + // is reserved for entries that opt into it explicitly. useEffect(() => { if (isServer || !closeOnEsc || !isOpen) return undefined; - return pushEscapeHandler(() => closePortalRef.current()); + return pushEscapeHandler(() => closePortalRef.current(), { + exclusive: false, + }); }, [isOpen, closeOnEsc, isServer]); const Portal = useCallback( diff --git a/src/hooks/useEscapeStack.test.ts b/src/hooks/useEscapeStack.test.ts index 4a644fd93..58c32bfa3 100644 --- a/src/hooks/useEscapeStack.test.ts +++ b/src/hooks/useEscapeStack.test.ts @@ -65,4 +65,70 @@ describe("pushEscapeHandler", () => { document.removeEventListener("keydown", outsideListener); unregister(); }); + + it("removes the correct entry when the same handler reference is pushed twice", () => { + // Regression test: unregistering must not rely on the handler's + // identity, since the same function reference could be passed to + // multiple registrations. + const shared = jest.fn(); + const unregisterFirst = pushEscapeHandler(shared); + const unregisterSecond = pushEscapeHandler(shared); + + // Unregister the first (bottom) entry — the second (top) entry should + // remain on the stack and still fire. + unregisterFirst(); + fireEscape(); + expect(shared).toHaveBeenCalledTimes(1); + + unregisterSecond(); + shared.mockClear(); + fireEscape(); + expect(shared).not.toHaveBeenCalled(); + }); + + describe("non-exclusive entries", () => { + it("calls the handler but does not stop propagation to other listeners", () => { + const handler = jest.fn(); + const outsideListener = jest.fn(); + const unregister = pushEscapeHandler(handler, { exclusive: false }); + document.addEventListener("keydown", outsideListener); + + fireEscape(); + expect(handler).toHaveBeenCalledTimes(1); + expect(outsideListener).toHaveBeenCalledTimes(1); + + document.removeEventListener("keydown", outsideListener); + unregister(); + }); + + it("is skipped in favour of a non-exclusive entry above it, without stopping propagation", () => { + // Mirrors a ContextualMenu (non-exclusive) opened inside a Modal + // (exclusive): the first Escape press should be handled by the + // ContextualMenu only, and should not be silenced for other listeners. + const exclusiveHandler = jest.fn(); + const nonExclusiveHandler = jest.fn(); + const outsideListener = jest.fn(); + + const unregisterExclusive = pushEscapeHandler(exclusiveHandler); + const unregisterNonExclusive = pushEscapeHandler(nonExclusiveHandler, { + exclusive: false, + }); + document.addEventListener("keydown", outsideListener); + + fireEscape(); + expect(nonExclusiveHandler).toHaveBeenCalledTimes(1); + expect(exclusiveHandler).not.toHaveBeenCalled(); + expect(outsideListener).toHaveBeenCalledTimes(1); + + // Once the non-exclusive entry is gone, the exclusive one underneath + // takes over and reclaims exclusive ownership of Escape. + unregisterNonExclusive(); + fireEscape(); + expect(exclusiveHandler).toHaveBeenCalledTimes(1); + expect(outsideListener).toHaveBeenCalledTimes(1); + + document.removeEventListener("keydown", outsideListener); + unregisterExclusive(); + }); + }); }); diff --git a/src/hooks/useEscapeStack.ts b/src/hooks/useEscapeStack.ts index 23f517ea0..44c80d8ab 100644 --- a/src/hooks/useEscapeStack.ts +++ b/src/hooks/useEscapeStack.ts @@ -5,18 +5,43 @@ * handler here. Only the most recently registered handler fires when Escape * is pressed, so nested overlays always dismiss in the correct order * regardless of their DOM position or portal placement. + * + * Each entry can be `exclusive` (the default) or not: + * - Exclusive entries (e.g. Modal) claim Escape entirely while they are on + * top of the stack — `stopImmediatePropagation()` is called, so no other + * `document` keydown listener (including non-stack ones, such as + * `useOnEscapePressed`) sees the event. This preserves Modal's long + * standing behaviour of owning Escape while it is open. + * - Non-exclusive entries (e.g. portal-based overlays such as + * ContextualMenu) handle Escape themselves but let the event continue to + * propagate, so they don't silence unrelated listeners elsewhere on the + * page when used on their own. + * + * When a non-exclusive entry sits on top of an exclusive one (e.g. a + * ContextualMenu opened inside a Modal), the first Escape press is handled + * by the non-exclusive entry without claiming the event, and the exclusive + * entry below it is skipped for that press — giving correct two-step + * dismissal (innermost overlay first). */ import { useEffect, useRef } from "react"; -const handlers: Array<() => void> = []; +type EscapeStackEntry = { + onEscape: () => void; + exclusive: boolean; +}; + +const stack: EscapeStackEntry[] = []; // Use capture phase so the stack fires before any bubble-phase listeners // and cannot be silenced by stopPropagation() on a focused child element. function onKeyDown(e: KeyboardEvent): void { - if (e.key !== "Escape" || handlers.length === 0) return; - e.stopImmediatePropagation(); - handlers[handlers.length - 1](); + if (e.key !== "Escape" || stack.length === 0) return; + const top = stack[stack.length - 1]; + if (top.exclusive) { + e.stopImmediatePropagation(); + } + top.onEscape(); } /** @@ -26,22 +51,36 @@ function onKeyDown(e: KeyboardEvent): void { * Handlers are invoked in LIFO order — the last one pushed runs first, * mirroring the visual stacking of overlays. * + * @param onEscape - Callback invoked when Escape is pressed and this entry + * is at the top of the stack. + * @param options.exclusive - When `true` (the default), this entry claims + * the Escape press entirely while on top of the stack, preventing any + * other `document` keydown listener from seeing it. Set to `false` for + * overlays that should not silence unrelated Escape handlers when used on + * their own (e.g. portal-based overlays). + * * Safe to call in SSR environments — it no-ops when `document` is unavailable. * * This is the imperative primitive behind `useEscapeStack` and is not part * of the public package API. React consumers should use `useEscapeStack`. */ -export function pushEscapeHandler(handler: () => void): () => void { +export function pushEscapeHandler( + onEscape: () => void, + { exclusive = true }: { exclusive?: boolean } = {}, +): () => void { if (typeof document === "undefined") return () => undefined; - if (handlers.length === 0) { + // Wrap in a fresh object so each registration has a unique identity, even + // if the same callback reference is pushed more than once. + const entry: EscapeStackEntry = { onEscape, exclusive }; + if (stack.length === 0) { // capture: true — fires before bubble-phase listeners on any descendant document.addEventListener("keydown", onKeyDown, true); } - handlers.push(handler); + stack.push(entry); return () => { - const idx = handlers.lastIndexOf(handler); - if (idx !== -1) handlers.splice(idx, 1); - if (handlers.length === 0) { + const idx = stack.indexOf(entry); + if (idx !== -1) stack.splice(idx, 1); + if (stack.length === 0) { document.removeEventListener("keydown", onKeyDown, true); } }; @@ -62,10 +101,16 @@ export function pushEscapeHandler(handler: () => void): () => void { * is at the top of the stack. * @param options.isEnabled - When `false` the handler is not registered * (defaults to `true`). + * @param options.exclusive - See `pushEscapeHandler`. Defaults to `true`. */ +export type UseEscapeStackOptions = { + isEnabled?: boolean; + exclusive?: boolean; +}; + export const useEscapeStack = ( handler: () => void, - { isEnabled } = { isEnabled: true }, + { isEnabled = true, exclusive = true }: UseEscapeStackOptions = {}, ): void => { // Always keep the ref pointing at the latest handler without changing // the registered stable wrapper. @@ -77,6 +122,6 @@ export const useEscapeStack = ( useEffect(() => { if (!isEnabled) return undefined; // Register a stable wrapper; the ref always calls the latest handler. - return pushEscapeHandler(() => handlerRef.current()); - }, [isEnabled]); + return pushEscapeHandler(() => handlerRef.current(), { exclusive }); + }, [isEnabled, exclusive]); };