From e1ae03dfa8256f24e4a7669ed91a254a18fa217d Mon Sep 17 00:00:00 2001 From: Tal Koren Date: Wed, 5 Jun 2024 18:10:21 +0300 Subject: [PATCH 1/4] feat(Modal): don't render when modal is closed --- packages/core/src/components/Modal/Modal.tsx | 7 +++++- .../Modal/__stories__/Modal.stories.tsx | 8 ++++++- .../src/components/Modal/useShowHideModal.ts | 24 +++++++++++-------- 3 files changed, 27 insertions(+), 12 deletions(-) diff --git a/packages/core/src/components/Modal/Modal.tsx b/packages/core/src/components/Modal/Modal.tsx index 4b81f23585..c7d5528814 100644 --- a/packages/core/src/components/Modal/Modal.tsx +++ b/packages/core/src/components/Modal/Modal.tsx @@ -76,6 +76,7 @@ export interface ModalProps { * z-index attribute of the container */ zIndex?: number; + unmountOnClose?: boolean; } const Modal: FC & { width?: typeof ModalWidth } = ({ @@ -95,6 +96,7 @@ const Modal: FC & { width?: typeof ModalWidth } = ({ closeButtonAriaLabel = "Close", contentSpacing = false, zIndex = 10000, + unmountOnClose, "data-testid": dataTestId }) => { const childrenArray: ReactElement[] = useMemo( @@ -118,7 +120,7 @@ const Modal: FC & { width?: typeof ModalWidth } = ({ useBodyScrollLock({ instance }); // show/hide and animate the modal - useShowHideModal({ + const { shouldShow } = useShowHideModal({ instance, show, triggerElement, @@ -187,6 +189,9 @@ const Modal: FC & { width?: typeof ModalWidth } = ({ document.body ); + if (unmountOnClose && !shouldShow) { + return null; + } return ReactDOM.createPortal(dialog, document.body); }; diff --git a/packages/core/src/components/Modal/__stories__/Modal.stories.tsx b/packages/core/src/components/Modal/__stories__/Modal.stories.tsx index f561885287..f10bb3153b 100644 --- a/packages/core/src/components/Modal/__stories__/Modal.stories.tsx +++ b/packages/core/src/components/Modal/__stories__/Modal.stories.tsx @@ -67,7 +67,13 @@ export default { export const Overview = { render: modalTemplate.bind({}), - name: "Overview" + name: "Overview", + parameters: { + controls: { + // TODO: remove exclusion when prop is removed in next major + exclude: ["unmountOnClose"] + } + } }; export const WidthVariantsNormal = { diff --git a/packages/core/src/components/Modal/useShowHideModal.ts b/packages/core/src/components/Modal/useShowHideModal.ts index 0498c64d5a..29e61dddfd 100644 --- a/packages/core/src/components/Modal/useShowHideModal.ts +++ b/packages/core/src/components/Modal/useShowHideModal.ts @@ -1,4 +1,4 @@ -import React, { useCallback, useEffect } from "react"; +import React, { useState, useCallback, useEffect } from "react"; import useAnimationProps from "./useAnimationProps"; import useKeyEvent from "../../hooks/useKeyEvent/index"; import { A11yDialogType } from "./ModalHelper"; @@ -19,6 +19,7 @@ export default function useShowHideModal({ onClose: () => void; alertDialog: boolean; }) { + const [isAnimating, setIsAnimating] = useState(false); const getAnimationProps = useAnimationProps(triggerElement, instance); const closeOnEsc = useCallback( @@ -37,22 +38,25 @@ export default function useShowHideModal({ keys: KEYS }); - // show/hide and animate the modal useEffect(() => { - if (!instance) { - return; - } - const animate = instance?.$el.childNodes[1].animate; if (show) { + setIsAnimating(true); instance?.show(); - if (animate) instance?.$el.childNodes[1].animate?.(...getAnimationProps()); + const animate = instance?.$el.childNodes[1].animate; + if (animate) { + instance?.$el.childNodes[1].animate?.(...getAnimationProps()); + } } else { + const animate = instance?.$el.childNodes[1].animate; if (animate) { const animation = instance?.$el.childNodes[1]?.animate(...getAnimationProps(true)); - animation?.finished.then(() => instance?.hide()); - } else { - instance?.hide(); + animation.finished.then(() => { + setIsAnimating(false); + instance?.hide(); + }); } } }, [show, instance, getAnimationProps]); + + return { shouldShow: isAnimating }; } From 272662e0eea70a7bef639974061498a2b3ca0fe2 Mon Sep 17 00:00:00 2001 From: Tal Koren Date: Wed, 5 Jun 2024 18:12:23 +0300 Subject: [PATCH 2/4] docs: add jsdoc --- packages/core/src/components/Modal/Modal.tsx | 3 +++ .../src/components/Modal/__stories__/Modal.stories.tsx | 8 +------- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/packages/core/src/components/Modal/Modal.tsx b/packages/core/src/components/Modal/Modal.tsx index c7d5528814..cebedb77b2 100644 --- a/packages/core/src/components/Modal/Modal.tsx +++ b/packages/core/src/components/Modal/Modal.tsx @@ -76,6 +76,9 @@ export interface ModalProps { * z-index attribute of the container */ zIndex?: number; + /** + * If true, the modal will unmount when it's not shown + */ unmountOnClose?: boolean; } diff --git a/packages/core/src/components/Modal/__stories__/Modal.stories.tsx b/packages/core/src/components/Modal/__stories__/Modal.stories.tsx index f10bb3153b..f561885287 100644 --- a/packages/core/src/components/Modal/__stories__/Modal.stories.tsx +++ b/packages/core/src/components/Modal/__stories__/Modal.stories.tsx @@ -67,13 +67,7 @@ export default { export const Overview = { render: modalTemplate.bind({}), - name: "Overview", - parameters: { - controls: { - // TODO: remove exclusion when prop is removed in next major - exclude: ["unmountOnClose"] - } - } + name: "Overview" }; export const WidthVariantsNormal = { From b627255efcb4ec563cce2cf3feafd271aa8b198a Mon Sep 17 00:00:00 2001 From: Tal Koren Date: Wed, 5 Jun 2024 18:43:59 +0300 Subject: [PATCH 3/4] fix: add needed fallback --- packages/core/src/components/Modal/useShowHideModal.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/core/src/components/Modal/useShowHideModal.ts b/packages/core/src/components/Modal/useShowHideModal.ts index 29e61dddfd..e06ea06485 100644 --- a/packages/core/src/components/Modal/useShowHideModal.ts +++ b/packages/core/src/components/Modal/useShowHideModal.ts @@ -54,6 +54,9 @@ export default function useShowHideModal({ setIsAnimating(false); instance?.hide(); }); + } else { + setIsAnimating(false); + instance?.hide(); } } }, [show, instance, getAnimationProps]); From 8616f05f3a808ad8c0da1ba093898a9bcbd543a0 Mon Sep 17 00:00:00 2001 From: Tal Koren Date: Sun, 9 Jun 2024 11:26:53 +0300 Subject: [PATCH 4/4] fix: add optional chaining --- packages/core/src/components/Modal/useShowHideModal.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/src/components/Modal/useShowHideModal.ts b/packages/core/src/components/Modal/useShowHideModal.ts index e06ea06485..c75cd05103 100644 --- a/packages/core/src/components/Modal/useShowHideModal.ts +++ b/packages/core/src/components/Modal/useShowHideModal.ts @@ -50,7 +50,7 @@ export default function useShowHideModal({ const animate = instance?.$el.childNodes[1].animate; if (animate) { const animation = instance?.$el.childNodes[1]?.animate(...getAnimationProps(true)); - animation.finished.then(() => { + animation?.finished.then(() => { setIsAnimating(false); instance?.hide(); });