From ffd5d77fe196aa4717fcdade887f43ec9dc02b0a Mon Sep 17 00:00:00 2001 From: joydaven Date: Mon, 19 Oct 2026 19:35:24 +0800 Subject: [PATCH] fix(ContextualMenu): don't close on clicks inside portalled content Content opened from a menu item, such as a Modal or ConfirmationButton, is rendered through a portal and so is not a DOM descendant of the menu. usePortal's outside-click check therefore treated clicks inside it as outside, closed the menu and unmounted the modal with it. Handle outside clicks in the component instead: a capture handler on the wrapper records any mousedown that starts inside the menu's React tree (React events bubble through portals), and the document listener ignores that event. Outside clicks, closeOnOutsideClick={false} and the main-button-only rule behave as before. Fixes #1049 --- .../ContextualMenu/ContextualMenu.stories.tsx | 30 ++++++ .../ContextualMenu/ContextualMenu.test.tsx | 97 ++++++++++++++++++- .../ContextualMenu/ContextualMenu.tsx | 48 ++++++++- 3 files changed, 172 insertions(+), 3 deletions(-) diff --git a/src/components/ContextualMenu/ContextualMenu.stories.tsx b/src/components/ContextualMenu/ContextualMenu.stories.tsx index 44d907f9d..14f45c147 100644 --- a/src/components/ContextualMenu/ContextualMenu.stories.tsx +++ b/src/components/ContextualMenu/ContextualMenu.stories.tsx @@ -5,6 +5,7 @@ import Button from "../Button"; import ContextualMenu from "./ContextualMenu"; import { ContextualMenuProps } from "."; import Modal from "components/Modal"; +import ConfirmationButton from "../ConfirmationButton"; const ScrollTemplate = (args: ContextualMenuProps) => (
), }; + +export const OpensModal: Story = { + name: "Opens a modal", + args: { hasToggleIcon: true, position: "right", toggleLabel: "Actions" }, + parameters: { + docs: { + description: { + story: + "Content opened from a menu item can be portalled elsewhere in the DOM, like this confirmation modal. Clicking inside it keeps the menu, and therefore the modal, open.", + }, + }, + }, + render: (args: ContextualMenuProps) => ( + + {}, + children: "This will permanently delete the item.", + }} + > + Delete + + + ), +}; diff --git a/src/components/ContextualMenu/ContextualMenu.test.tsx b/src/components/ContextualMenu/ContextualMenu.test.tsx index c75a76238..d1b0dc5b8 100644 --- a/src/components/ContextualMenu/ContextualMenu.test.tsx +++ b/src/components/ContextualMenu/ContextualMenu.test.tsx @@ -1,8 +1,10 @@ -import { render, screen, within } from "@testing-library/react"; +import { fireEvent, render, screen, within } from "@testing-library/react"; import React from "react"; +import { createPortal } from "react-dom"; import userEvent from "@testing-library/user-event"; import Button from "../Button"; +import ConfirmationButton from "../ConfirmationButton"; import ContextualMenu, { Label } from "./ContextualMenu"; import { Label as DropdownLabel } from "./ContextualMenuDropdown/ContextualMenuDropdown"; @@ -302,6 +304,99 @@ describe("ContextualMenu ", () => { expect(screen.getByLabelText(DropdownLabel.Dropdown)).toBeInTheDocument(); }); + describe("outside clicks", () => { + const openMenu = async () => { + await userEvent.click(screen.getByRole("button", { name: "Toggle" })); + expect(screen.getByLabelText(DropdownLabel.Dropdown)).toBeInTheDocument(); + }; + + it("closes the menu when clicking outside", async () => { + render( + <> + + + , + ); + await openMenu(); + await userEvent.click(screen.getByRole("button", { name: "Outside" })); + expect( + screen.queryByLabelText(DropdownLabel.Dropdown), + ).not.toBeInTheDocument(); + }); + + it("does not close the menu when clicking inside the dropdown", async () => { + render( + + + , + ); + await openMenu(); + await userEvent.click(screen.getByRole("button", { name: "Inside" })); + expect(screen.getByLabelText(DropdownLabel.Dropdown)).toBeInTheDocument(); + }); + + it("does not close the menu when clicking inside content portalled out of the dropdown", async () => { + const Portalled = () => + createPortal(, document.body); + render( + + + , + ); + await openMenu(); + await userEvent.click( + screen.getByRole("button", { name: "Inside portal" }), + ); + expect(screen.getByLabelText(DropdownLabel.Dropdown)).toBeInTheDocument(); + }); + + it("keeps a confirmation modal opened from the menu open when it is clicked", async () => { + render( + + + Delete + + , + ); + await openMenu(); + await userEvent.click(screen.getByRole("button", { name: "Delete" })); + expect(screen.getByRole("dialog")).toBeInTheDocument(); + await userEvent.click(screen.getByText("Are you sure?")); + expect(screen.getByRole("dialog")).toBeInTheDocument(); + expect(screen.getByLabelText(DropdownLabel.Dropdown)).toBeInTheDocument(); + }); + + it("does not close the menu on outside clicks when closeOnOutsideClick is false", async () => { + render( + <> + + + , + ); + await openMenu(); + await userEvent.click(screen.getByRole("button", { name: "Outside" })); + expect(screen.getByLabelText(DropdownLabel.Dropdown)).toBeInTheDocument(); + }); + + it("ignores outside clicks that are not with the main button", async () => { + render(); + await openMenu(); + fireEvent.mouseDown(document.body, { button: 2 }); + expect(screen.getByLabelText(DropdownLabel.Dropdown)).toBeInTheDocument(); + }); + }); + describe("focus behavior", () => { const setup = (props = {}) => { const links = [0, 1, 2].map((i) => ({ diff --git a/src/components/ContextualMenu/ContextualMenu.tsx b/src/components/ContextualMenu/ContextualMenu.tsx index 7148032fd..e13728c6c 100644 --- a/src/components/ContextualMenu/ContextualMenu.tsx +++ b/src/components/ContextualMenu/ContextualMenu.tsx @@ -44,7 +44,9 @@ export type BaseProps = PropsWithSpread< */ closeOnEsc?: boolean; /** - * Whether the menu should close when clicking outside the menu. + * Whether the menu should close when clicking outside the menu. Clicks on + * content rendered by the menu count as inside, even when that content is + * portalled elsewhere in the DOM (e.g. a modal opened from a menu item). */ closeOnOutsideClick?: boolean; /** @@ -179,6 +181,7 @@ const ContextualMenu = ({ dropdownProps, hasToggleIcon, links, + onMouseDownCapture, onToggleMenu, position = "right", positionNode, @@ -273,7 +276,9 @@ const ContextualMenu = ({ const { openPortal, closePortal, isOpen, Portal } = usePortal({ closeOnEsc, - closeOnOutsideClick, + // Outside clicks are handled below, so that clicks on content the menu + // portals elsewhere in the DOM (e.g. a modal) still count as inside. + closeOnOutsideClick: false, isOpen: visible, onOpen: (event) => { // Call the toggle callback, if supplied. @@ -356,6 +361,44 @@ const ContextualMenu = ({ focusFirstDropdownItem(); }; + /** + * Close the menu when the user clicks outside of it. + * + * `usePortal` can do this itself, but it decides what is "outside" with a + * DOM `contains` check against the dropdown and the toggle. Content that the + * menu portals somewhere else, such as a modal opened from a menu item, is + * a separate DOM subtree, so a click inside it would close the menu and + * unmount the modal with it. + * + * React synthetic events bubble through portals following the component + * tree instead, so the capture handler on the wrapper sees every mousedown + * that starts anywhere inside the menu, portalled or not. The document + * listener then ignores that same native event. + */ + const insideMouseDown = useRef(null); + + const onWrapperMouseDownCapture = (e: React.MouseEvent) => { + insideMouseDown.current = e.nativeEvent; + onMouseDownCapture?.(e); + }; + + useEffect(() => { + if (!isOpen || !closeOnOutsideClick) { + return undefined; + } + const handleMouseDown = (e: MouseEvent) => { + // Only the main button closes the menu, matching `usePortal`. + if (e.button !== 0 || e === insideMouseDown.current) { + return; + } + closePortal(); + }; + document.addEventListener("mousedown", handleMouseDown); + return () => { + document.removeEventListener("mousedown", handleMouseDown); + }; + }, [closeOnOutsideClick, closePortal, isOpen]); + const previousVisible = usePrevious(visible); const labelNode = toggleLabel && typeof toggleLabel === "string" ? ( @@ -474,6 +517,7 @@ const ContextualMenu = ({