From 10a094bee1a148e61604f27c95f865d8fa72799c Mon Sep 17 00:00:00 2001
From: Vladimir Cucu <108150922+vladimir-cucu@users.noreply.github.com>
Date: Tue, 26 Mar 2024 08:11:21 +0200
Subject: [PATCH] fix: stop click event propagation in Modal (#1058)

---
 .../ConfirmationModal.test.tsx                | 69 +++++++++++++++++++
 .../ConfirmationModal/ConfirmationModal.tsx   | 20 +++++-
 src/components/Modal/Modal.test.tsx           | 32 +++++++++
 src/components/Modal/Modal.tsx                | 58 ++++++++++------
 4 files changed, 156 insertions(+), 23 deletions(-)

diff --git a/src/components/ConfirmationModal/ConfirmationModal.test.tsx b/src/components/ConfirmationModal/ConfirmationModal.test.tsx
index a984f374d..73830c53b 100644
--- a/src/components/ConfirmationModal/ConfirmationModal.test.tsx
+++ b/src/components/ConfirmationModal/ConfirmationModal.test.tsx
@@ -55,4 +55,73 @@ describe("ConfirmationModal ", () => {
     await userEvent.click(screen.getByText("Proceed"));
     expect(onConfirm).toHaveBeenCalled();
   });
+
+  it("should stop click event propagation on cancel by default", async () => {
+    const handleExternalClick = jest.fn();
+    render(
+      <div onClick={handleExternalClick}>
+        <ConfirmationModal
+          cancelButtonLabel="Go back"
+          confirmButtonLabel="Proceed"
+          onConfirm={jest.fn()}
+        >
+          Test click propagation
+        </ConfirmationModal>
+      </div>
+    );
+
+    await userEvent.click(screen.getByText("Go back"));
+    expect(handleExternalClick).not.toHaveBeenCalled();
+  });
+
+  it("should propagate click event on cancel", async () => {
+    const handleExternalClick = jest.fn();
+    render(
+      <div onClick={handleExternalClick}>
+        <ConfirmationModal
+          cancelButtonLabel="Go back"
+          confirmButtonLabel="Proceed"
+          onConfirm={jest.fn()}
+          shouldPropagateClickEvent={true}
+        >
+          Test click propagation
+        </ConfirmationModal>
+      </div>
+    );
+
+    await userEvent.click(screen.getByText("Go back"));
+    expect(handleExternalClick).toHaveBeenCalledTimes(1);
+  });
+
+  it("should stop click event propagation on confirm by default", async () => {
+    const handleExternalClick = jest.fn();
+    render(
+      <div onClick={handleExternalClick}>
+        <ConfirmationModal confirmButtonLabel="Proceed" onConfirm={jest.fn()}>
+          Test click propagation
+        </ConfirmationModal>
+      </div>
+    );
+
+    await userEvent.click(screen.getByText("Proceed"));
+    expect(handleExternalClick).not.toHaveBeenCalled();
+  });
+
+  it("should propagate click event on confirm", async () => {
+    const handleExternalClick = jest.fn();
+    render(
+      <div onClick={handleExternalClick}>
+        <ConfirmationModal
+          confirmButtonLabel="Proceed"
+          onConfirm={jest.fn()}
+          shouldPropagateClickEvent={true}
+        >
+          Test click propagation
+        </ConfirmationModal>
+      </div>
+    );
+
+    await userEvent.click(screen.getByText("Proceed"));
+    expect(handleExternalClick).toHaveBeenCalledTimes(1);
+  });
 });
diff --git a/src/components/ConfirmationModal/ConfirmationModal.tsx b/src/components/ConfirmationModal/ConfirmationModal.tsx
index b647790c1..41edfe55e 100644
--- a/src/components/ConfirmationModal/ConfirmationModal.tsx
+++ b/src/components/ConfirmationModal/ConfirmationModal.tsx
@@ -29,7 +29,7 @@ export type Props = PropsWithSpread<
     /**
      * Function to perform the action prompted by the modal.
      */
-    onConfirm: (e: MouseEvent<HTMLElement>) => void;
+    onConfirm: (event: MouseEvent<HTMLElement>) => void;
   },
   Omit<ModalProps, "buttonRow">
 >;
@@ -43,18 +43,32 @@ export const ConfirmationModal = ({
   onConfirm,
   ...props
 }: Props): ReactElement => {
+  const handleClick =
+    <A extends Function>(action: A | null | undefined) =>
+    (event: MouseEvent<HTMLButtonElement>) => {
+      if (!props.shouldPropagateClickEvent) {
+        event.stopPropagation();
+      }
+      if (action) {
+        action(event);
+      }
+    };
+
   return (
     <Modal
       buttonRow={
         <>
           {confirmExtra}
-          <Button className="u-no-margin--bottom" onClick={props.close}>
+          <Button
+            className="u-no-margin--bottom"
+            onClick={handleClick(props.close)}
+          >
             {cancelButtonLabel}
           </Button>
           <Button
             appearance={confirmButtonAppearance}
             className="u-no-margin--bottom"
-            onClick={onConfirm}
+            onClick={handleClick(onConfirm)}
           >
             {confirmButtonLabel}
           </Button>
diff --git a/src/components/Modal/Modal.test.tsx b/src/components/Modal/Modal.test.tsx
index e6a61eb25..b596f1af8 100644
--- a/src/components/Modal/Modal.test.tsx
+++ b/src/components/Modal/Modal.test.tsx
@@ -139,4 +139,36 @@ describe("Modal ", () => {
     expect(mockHandleModalClose).toHaveBeenCalledTimes(1);
     expect(mockHandleEscPress).not.toHaveBeenCalled();
   });
+
+  it("should stop click event propagation on close by default", async () => {
+    const handleExternalClick = jest.fn();
+    const { container } = render(
+      <div onClick={handleExternalClick}>
+        <Modal title="Test" close={jest.fn()}>
+          Bare bones
+        </Modal>
+      </div>
+    );
+
+    const closeButton = container.querySelector("button.p-modal__close");
+    expect(closeButton).not.toBeNull();
+    await userEvent.click(closeButton!);
+    expect(handleExternalClick).not.toHaveBeenCalled();
+  });
+
+  it("should propagate click event on close", async () => {
+    const handleExternalClick = jest.fn();
+    const { container } = render(
+      <div onClick={handleExternalClick}>
+        <Modal title="Test" close={jest.fn()} shouldPropagateClickEvent={true}>
+          Bare bones
+        </Modal>
+      </div>
+    );
+
+    const closeButton = container.querySelector("button.p-modal__close");
+    expect(closeButton).not.toBeNull();
+    await userEvent.click(closeButton!);
+    expect(handleExternalClick).toHaveBeenCalledTimes(1);
+  });
 });
diff --git a/src/components/Modal/Modal.tsx b/src/components/Modal/Modal.tsx
index 5f66803e1..231347b4b 100644
--- a/src/components/Modal/Modal.tsx
+++ b/src/components/Modal/Modal.tsx
@@ -25,6 +25,10 @@ export type Props = PropsWithSpread<
      * The title of the modal.
      */
     title?: ReactNode | null;
+    /**
+     * Whether the button click event should propagate.
+     */
+    shouldPropagateClickEvent?: boolean;
   },
   HTMLProps<HTMLDivElement>
 >;
@@ -35,6 +39,7 @@ export const Modal = ({
   className,
   close,
   title,
+  shouldPropagateClickEvent = false,
   ...wrapperProps
 }: Props): ReactElement => {
   // list of focusable selectors is based on this Stack Overflow answer:
@@ -47,7 +52,7 @@ export const Modal = ({
 
   const modalRef: MutableRefObject<HTMLElement> = useRef(null);
   const focusableModalElements = useRef(null);
-  const handleTabKey = (e: React.KeyboardEvent<HTMLDivElement>) => {
+  const handleTabKey = (event: React.KeyboardEvent<HTMLDivElement>) => {
     if (focusableModalElements.current.length > 0) {
       const firstElement = focusableModalElements.current[0];
       const lastElement =
@@ -55,29 +60,31 @@ export const Modal = ({
           focusableModalElements.current.length - 1
         ];
 
-      if (!e.shiftKey && document.activeElement === lastElement) {
+      if (!event.shiftKey && document.activeElement === lastElement) {
         (firstElement as HTMLElement).focus();
-        e.preventDefault();
+        event.preventDefault();
       }
 
-      if (e.shiftKey && document.activeElement === firstElement) {
+      if (event.shiftKey && document.activeElement === firstElement) {
         (lastElement as HTMLElement).focus();
-        return e.preventDefault();
+        return event.preventDefault();
       }
     }
   };
 
   const handleEscKey = (
-    e: KeyboardEvent | React.KeyboardEvent<HTMLDivElement>
+    event: KeyboardEvent | React.KeyboardEvent<HTMLDivElement>
   ) => {
-    if ("nativeEvent" in e && e.nativeEvent.stopImmediatePropagation) {
-      e.nativeEvent.stopImmediatePropagation();
-    } else if ("stopImmediatePropagation" in e) {
-      e.stopImmediatePropagation();
-    } else if (e.stopPropagation) {
-      e.stopPropagation();
+    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();
     }
-    close();
   };
 
   const keyListenersMap = new Map([
@@ -102,9 +109,9 @@ export const Modal = ({
   }, [close]);
 
   useEffect(() => {
-    const keyDown = (e) => {
-      const listener = keyListenersMap.get(e.code);
-      return listener && listener(e);
+    const keyDown = (event: KeyboardEvent) => {
+      const listener = keyListenersMap.get(event.code);
+      return listener && listener(event);
     };
 
     document.addEventListener("keydown", keyDown);
@@ -121,18 +128,29 @@ export const Modal = ({
     shouldClose.current = false;
   };
 
-  const handleOverlayOnMouseDown = (event) => {
+  const handleOverlayOnMouseDown = (
+    event: React.MouseEvent<HTMLDivElement>
+  ) => {
     if (event.target === modalRef.current) {
       shouldClose.current = true;
     }
   };
 
-  const handleOverlayOnClick = () => {
-    if (shouldClose.current) {
+  const handleClose = (event: React.MouseEvent) => {
+    if (!shouldPropagateClickEvent) {
+      event.stopPropagation();
+    }
+    if (close) {
       close();
     }
   };
 
+  const handleOverlayOnClick = (event: React.MouseEvent<HTMLDivElement>) => {
+    if (shouldClose.current) {
+      handleClose(event);
+    }
+  };
+
   return (
     <div
       className={classNames("p-modal", className)}
@@ -159,7 +177,7 @@ export const Modal = ({
               <button
                 className="p-modal__close"
                 aria-label="Close active modal"
-                onClick={close}
+                onClick={handleClose}
               >
                 Close
               </button>