From 6f88e0c1d9d3787a70a1b430d73d92ea70c4f31f Mon Sep 17 00:00:00 2001
From: Deepak Jose <deepak.jose@bigbinary.com>
Date: Fri, 4 Oct 2024 10:35:19 +0530
Subject: [PATCH] Fixes issues due to transition delay in closing alerts.
 (#2321)

* Created memoized children component that updates the children only when the should update prop is set to true.

* Wrapped the modal children in the memoized component and set the shouldUpdate to isOpen or forceRender prop.

* Added the new prop to the props list and the types file.

* Updated button to be disabled only when the submission was going on.

* Removed the test to verify the submit button calls are prevented during closing transition since it was a false test case.

* Update types/Modal.d.ts

Co-authored-by: Abhay V Ashokan <35297280+AbhayVAshokan@users.noreply.github.com>

---------

Co-authored-by: Abhay V Ashokan <35297280+AbhayVAshokan@users.noreply.github.com>
---
 src/components/Alert.jsx                 |  2 +-
 src/components/Modal/MemoizedChildren.js | 10 ++++++++
 src/components/Modal/index.jsx           | 15 +++++++++---
 tests/Alert.test.jsx                     | 31 ++++++------------------
 types/Modal.d.ts                         |  2 +-
 5 files changed, 31 insertions(+), 29 deletions(-)
 create mode 100644 src/components/Modal/MemoizedChildren.js

diff --git a/src/components/Alert.jsx b/src/components/Alert.jsx
index 1a1d3d70d..3f2e3afb5 100644
--- a/src/components/Alert.jsx
+++ b/src/components/Alert.jsx
@@ -77,7 +77,7 @@ const Alert = ({
         )}
         <Button
           data-cy="alert-submit-button"
-          disabled={!isOpen}
+          disabled={isSubmitting}
           label={submitButtonLabel}
           loading={isSubmitting}
           ref={submitButtonRef}
diff --git a/src/components/Modal/MemoizedChildren.js b/src/components/Modal/MemoizedChildren.js
new file mode 100644
index 000000000..1832659b6
--- /dev/null
+++ b/src/components/Modal/MemoizedChildren.js
@@ -0,0 +1,10 @@
+import React from "react";
+
+const MemoizedChildren = React.memo(
+  ({ children }) => children,
+  (_, { shouldUpdate }) => !shouldUpdate
+);
+
+MemoizedChildren.displayName = "MemoizedChildren";
+
+export default MemoizedChildren;
diff --git a/src/components/Modal/index.jsx b/src/components/Modal/index.jsx
index c4c0a5e1d..c4794b8ac 100644
--- a/src/components/Modal/index.jsx
+++ b/src/components/Modal/index.jsx
@@ -13,6 +13,7 @@ import { useOverlayManager, useOverlay } from "hooks";
 import Body from "./Body";
 import Footer from "./Footer";
 import Header from "./Header";
+import MemoizedChildren from "./MemoizedChildren";
 
 const SIZES = {
   small: "small",
@@ -34,6 +35,7 @@ const Modal = ({
   backdropClassName = "",
   blockScrollOnMount = true,
   closeOnOutsideClick = true,
+  forceRender = false,
   ...otherProps
 }) => {
   const [hasTransitionCompleted, setHasTransitionCompleted] = useState(false);
@@ -107,9 +109,11 @@ const Modal = ({
                 onClick={handleOverlayClose}
               />
             )}
-            {typeof children === "function"
-              ? children({ setFocusField })
-              : children}
+            <MemoizedChildren shouldUpdate={isOpen || forceRender}>
+              {typeof children === "function"
+                ? children({ setFocusField })
+                : children}
+            </MemoizedChildren>
           </div>
         </Backdrop>
       </CSSTransition>
@@ -175,6 +179,11 @@ Modal.propTypes = {
    * To specify whether the scroll should be blocked when the Modal is opened.
    * */
   blockScrollOnMount: PropTypes.bool,
+  /**
+   * The modal children will be force re-rendered if the boolean is set to `true`. Ideally the modal won't update
+   * if the `isOpen` is `false`. You can use this prop to override that nature.
+   */
+  forceRender: PropTypes.bool,
 };
 
 Modal.Header = Header;
diff --git a/tests/Alert.test.jsx b/tests/Alert.test.jsx
index 79851eda7..371787302 100644
--- a/tests/Alert.test.jsx
+++ b/tests/Alert.test.jsx
@@ -26,10 +26,10 @@ describe("Alert", () => {
     const onClose = jest.fn();
     const { getByTestId } = render(
       <Alert
+        {...{ onClose }}
         isOpen
         message="Alert message"
         title="Alert title"
-        onClose={onClose}
       />
     );
     await userEvent.click(getByTestId("close-button"));
@@ -40,11 +40,11 @@ describe("Alert", () => {
     const onSubmit = jest.fn();
     const { getByText } = render(
       <Alert
+        {...{ onSubmit }}
         isOpen
         message="Alert message"
         submitButtonLabel="Submit"
         title="Alert title"
-        onSubmit={onSubmit}
       />
     );
     await userEvent.click(getByText("Submit"));
@@ -55,11 +55,11 @@ describe("Alert", () => {
     const onClose = jest.fn();
     const { getByText } = render(
       <Alert
+        {...{ onClose }}
         isOpen
         cancelButtonLabel="Cancel"
         message="Alert message"
         title="Alert title"
-        onClose={onClose}
       />
     );
     await userEvent.click(getByText("Cancel"));
@@ -70,11 +70,11 @@ describe("Alert", () => {
     const onClose = jest.fn();
     const { getAllByRole } = render(
       <Alert
+        {...{ onClose }}
         closeOnEsc
         isOpen
         message="Alert message"
         title="Alert title"
-        onClose={onClose}
       />
     );
     await userEvent.click(getAllByRole("button")[0]);
@@ -86,11 +86,11 @@ describe("Alert", () => {
     const onClose = jest.fn();
     const { container } = render(
       <Alert
+        {...{ onClose }}
         isOpen
         closeOnEsc={false}
         message="Alert message"
         title="Alert title"
-        onClose={onClose}
       />
     );
     await userEvent.type(container, "{esc}");
@@ -101,11 +101,11 @@ describe("Alert", () => {
     const onClose = jest.fn();
     const { getByTestId } = render(
       <Alert
+        {...{ onClose }}
         closeOnOutsideClick
         isOpen
         message="Alert message"
         title="Alert title"
-        onClose={onClose}
       />
     );
     await userEvent.click(getByTestId("backdrop"));
@@ -116,34 +116,17 @@ describe("Alert", () => {
     const onClose = jest.fn();
     const { getByTestId } = render(
       <Alert
+        {...{ onClose }}
         isOpen
         closeOnOutsideClick={false}
         message="Alert message"
         title="Alert title"
-        onClose={onClose}
       />
     );
     await userEvent.click(getByTestId("backdrop"));
     expect(onClose).not.toHaveBeenCalled();
   });
 
-  it("should not call onSubmit while alert is closing", async () => {
-    const onSubmit = jest.fn();
-
-    const { getByText, rerender } = render(
-      <Alert isOpen submitButtonLabel="Submit" onSubmit={onSubmit} />
-    );
-    await userEvent.click(getByText("Submit"));
-
-    rerender(
-      <Alert isOpen={false} submitButtonLabel="Submit" onSubmit={onSubmit} />
-    );
-
-    await userEvent.click(getByText("Submit"));
-
-    expect(onSubmit).toHaveBeenCalledTimes(1);
-  });
-
   it("should not show cancel button when hideCancelButton is true", () => {
     const { queryByText } = render(
       <Alert
diff --git a/types/Modal.d.ts b/types/Modal.d.ts
index 8661b2c3a..5b8ce818e 100644
--- a/types/Modal.d.ts
+++ b/types/Modal.d.ts
@@ -1,6 +1,6 @@
 import { PopupProps, PopupContentProps } from "./Popup";
 
-export type ModalProps = PopupProps & { size?: "small" | "medium" | "large" };
+export type ModalProps = PopupProps & { size?: "small" | "medium" | "large"; forceRender?: boolean; };
 
 const Modal: React.FC<ModalProps> & {
   Header: React.FC<PopupContentProps>;