fix(Modal): allow Escape to close ContextualMenu before closing Modal

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 ", () => {
@@ -104,15 +105,19 @@ 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,
     }: {
       onEscPress: () => void;
     }): React.JSX.Element => {
       useEffect(() => {
         const handleEscPress = (e: KeyboardEvent) => {
-          if (e.code === "Escape") {
+          if (e.key === "Escape") {
             onEscPress();
           }
         };
@@ -223,6 +228,37 @@ describe("Modal ", () => {
     expect(input).toHaveValue("delete item1");
   });
 
+  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 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(
+      <Modal title="Test" close={handleCloseModal}>
+        <ContextualMenu
+          toggleLabel="Open menu"
+          links={[{ children: "Item 1" }]}
+        />
+      </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 user.keyboard("{Escape}");
+    expect(handleCloseModal).not.toHaveBeenCalled();
+
+    // Second Escape — menu is gone, so the Modal should now close
+    await user.keyboard("{Escape}");
+    expect(handleCloseModal).toHaveBeenCalledTimes(1);
+  });
+
   it("updates focusable elements when an initially disabled button becomes enabled", async () => {
     const user = userEvent.setup();
 

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 { useEscapeStack } from "hooks";
 
 export type Props = PropsWithSpread<
   {
@@ -88,20 +89,7 @@ export const Modal = ({
     }
   };
 
-  const handleEscKey = (
-    event: KeyboardEvent | React.KeyboardEvent<HTMLDivElement>,
-  ) => {
-    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();
-    }
-  };
+  useEscapeStack(() => close?.(), { isEnabled: !!close });
 
   useEffect(() => {
     if (focusRef?.current) {
@@ -114,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<HTMLDivElement>);
+      }
     };
 
     document.addEventListener("keydown", keyDown, true);

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 <div>Mock component with event on Esc key press</div>;
+  };
+
+  render(
+    <>
+      <Navigation
+        logo={<img src="" alt="" />}
+        searchProps={{ onSearch: jest.fn() }}
+      />
+      <MockEscEventComponent />
+    </>,
+  );
+
+  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(
     <Navigation

src/components/Navigation/Navigation.tsx

@@ -203,7 +203,7 @@ const Navigation = ({
     }
   };
   // Hide the searchbox when the escape key is pressed.
-  useOnEscapePressed(() => toggleSearch(false));
+  useOnEscapePressed(() => toggleSearch(false), { isEnabled: searchOpen });
 
   useEffect(() => {
     if (searchOpen) {

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(
+      <SearchAndFilter
+        data-testid="searchandfilter"
+        filterPanelData={sampleData}
+        returnSearchData={returnSearchData}
+      />,
+    );
+    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 <div>Mock component with event on Esc key press</div>;
+    };
+
+    render(
+      <>
+        <SearchAndFilter
+          filterPanelData={sampleData}
+          returnSearchData={returnSearchData}
+        />
+        <MockEscEventComponent />
+      </>,
+    );
+
+    expect(getPanel()).toHaveAttribute("aria-hidden", "true");
+    await userEvent.keyboard("{Escape}");
+    expect(onEscPress).toHaveBeenCalledTimes(1);
+  });
 });

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

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<HTMLElement, Event>)
-        : undefined,
-    [closeOnEsc, closePortal],
-  );
-
   const handleOutsideMouseClick = useCallback(
     (e: MouseEvent): void => {
       const containsTarget = (target: RefObject<HTMLElement>) =>
@@ -179,21 +172,36 @@ 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]);
+
+  // 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(() => closePortalRef.current());
+  }, [isOpen, closeOnEsc, isServer]);
 
   const Portal = useCallback(
     ({ children }: { children: ReactNode }) => {

src/hooks/index.ts

@@ -1,4 +1,5 @@
 export { useOnClickOutside, useClickOutside } from "./useOnClickOutside";
+export { useEscapeStack } from "./useEscapeStack";
 export { useId } from "./useId";
 export { useListener } from "./useListener";
 export { useOnEscapePressed } from "./useOnEscapePressed";

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();
+  });
+});