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 <[email protected]> --------- Co-authored-by: Abhay V Ashokan <[email protected]>
bigbinary · Oct 4, 2024 · 6f88e0c · 6f88e0c
1 parent be30d03
commit 6f88e0c
Show file tree

Hide file tree

Showing 5 changed files with 31 additions and 29 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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>;