From 0a4d3c4e22eecacbbedb2b8164db139bb3dcf095 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Micha=C3=ABl=20Zasso?= <targos@protonmail.com>
Date: Tue, 8 Aug 2023 10:10:07 +0200
Subject: [PATCH] fix: only render modal children after calling showModal
 (#530)

This is important when the children measure their size dynamically when
they are mounted. The OCL editor does it.
---
 src/components/hooks/useOnOff.ts      |  3 +-
 src/components/modal/ConfirmModal.tsx | 65 ++++++++++++++-------------
 src/components/modal/Modal.tsx        | 28 ++++++------
 src/components/modal/useDialog.ts     | 46 ++++++++++++++-----
 stories/components/modal.stories.tsx  | 18 ++++++++
 5 files changed, 105 insertions(+), 55 deletions(-)

diff --git a/src/components/hooks/useOnOff.ts b/src/components/hooks/useOnOff.ts
index 20159728..4849d70e 100644
--- a/src/components/hooks/useOnOff.ts
+++ b/src/components/hooks/useOnOff.ts
@@ -4,9 +4,8 @@ export function useOnOff(
   initialValue = false,
 ): [isOn: boolean, setOn: () => void, setOff: () => void, toggle: () => void] {
   const [isOn, setOnOff] = useState(initialValue);
-
   const setOn = useCallback(() => setOnOff(true), []);
   const setOff = useCallback(() => setOnOff(false), []);
-  const toggle = useCallback(() => setOnOff(!isOn), [isOn]);
+  const toggle = useCallback(() => setOnOff((isOn) => !isOn), []);
   return [isOn, setOn, setOff, toggle];
 }
diff --git a/src/components/modal/ConfirmModal.tsx b/src/components/modal/ConfirmModal.tsx
index 51b25ab4..2f23d8cb 100644
--- a/src/components/modal/ConfirmModal.tsx
+++ b/src/components/modal/ConfirmModal.tsx
@@ -79,7 +79,7 @@ export function ConfirmModal(props: ConfirmModalProps) {
   } = props;
 
   const dialogRef = useRef<HTMLDialogElement>(null);
-  const dialogProps = useDialog({
+  const { dialogProps, isModalShown } = useDialog({
     dialogRef,
     isOpen,
     requestCloseOnEsc,
@@ -104,36 +104,41 @@ export function ConfirmModal(props: ConfirmModalProps) {
   return (
     <Portal>
       <ConfirmModalDialog {...dialogProps} ref={dialogRef}>
-        <RootLayoutProvider innerRef={portalDomNode}>
-          <ConfirmModalContents headerColor={headerColor} style={{ maxWidth }}>
-            <ConfirmModalChildrenRoot headerColor={headerColor}>
-              {children}
-            </ConfirmModalChildrenRoot>
+        {isModalShown && (
+          <RootLayoutProvider innerRef={portalDomNode}>
+            <ConfirmModalContents
+              headerColor={headerColor}
+              style={{ maxWidth }}
+            >
+              <ConfirmModalChildrenRoot headerColor={headerColor}>
+                {children}
+              </ConfirmModalChildrenRoot>
 
-            <ConfirmModalFooter>
-              <Button
-                onClick={onConfirm}
-                backgroundColor={{
-                  basic: 'hsla(243deg, 75%, 58%, 1)',
-                  hover: 'hsla(245deg, 58%, 50%, 1)',
-                }}
-                color={{ basic: 'white' }}
-              >
-                {saveText}
-              </Button>
-              <Button
-                onClick={onCancel}
-                backgroundColor={{
-                  basic: 'hsla(0deg, 72%, 50%, 1)',
-                  hover: 'hsla(0deg, 73%, 42%, 1)',
-                }}
-                color={{ basic: 'white' }}
-              >
-                {cancelText}
-              </Button>
-            </ConfirmModalFooter>
-          </ConfirmModalContents>
-        </RootLayoutProvider>
+              <ConfirmModalFooter>
+                <Button
+                  onClick={onConfirm}
+                  backgroundColor={{
+                    basic: 'hsla(243deg, 75%, 58%, 1)',
+                    hover: 'hsla(245deg, 58%, 50%, 1)',
+                  }}
+                  color={{ basic: 'white' }}
+                >
+                  {saveText}
+                </Button>
+                <Button
+                  onClick={onCancel}
+                  backgroundColor={{
+                    basic: 'hsla(0deg, 72%, 50%, 1)',
+                    hover: 'hsla(0deg, 73%, 42%, 1)',
+                  }}
+                  color={{ basic: 'white' }}
+                >
+                  {cancelText}
+                </Button>
+              </ConfirmModalFooter>
+            </ConfirmModalContents>
+          </RootLayoutProvider>
+        )}
       </ConfirmModalDialog>
     </Portal>
   );
diff --git a/src/components/modal/Modal.tsx b/src/components/modal/Modal.tsx
index e9c96e08..6d46e864 100644
--- a/src/components/modal/Modal.tsx
+++ b/src/components/modal/Modal.tsx
@@ -76,7 +76,7 @@ export function Modal(props: ModalProps) {
   } = props;
 
   const dialogRef = useRef<HTMLDialogElement>(null);
-  const dialogProps = useDialog({
+  const { dialogProps, isModalShown } = useDialog({
     dialogRef,
     isOpen,
     requestCloseOnEsc,
@@ -101,18 +101,20 @@ export function Modal(props: ModalProps) {
   return (
     <Portal>
       <DialogRoot {...dialogProps} ref={dialogRef}>
-        <RootLayoutProvider innerRef={portalDomNode}>
-          <DialogContents
-            style={{
-              maxWidth,
-              height: height || 'max-content',
-              width: width || '100%',
-            }}
-          >
-            {children}
-            {hasCloseButton && <ModalCloseButton onClick={onRequestClose} />}
-          </DialogContents>
-        </RootLayoutProvider>
+        {isModalShown && (
+          <RootLayoutProvider innerRef={portalDomNode}>
+            <DialogContents
+              style={{
+                maxWidth,
+                height: height || 'max-content',
+                width: width || '100%',
+              }}
+            >
+              {children}
+              {hasCloseButton && <ModalCloseButton onClick={onRequestClose} />}
+            </DialogContents>
+          </RootLayoutProvider>
+        )}
       </DialogRoot>
     </Portal>
   );
diff --git a/src/components/modal/useDialog.ts b/src/components/modal/useDialog.ts
index edd32c94..9a5bcbf2 100644
--- a/src/components/modal/useDialog.ts
+++ b/src/components/modal/useDialog.ts
@@ -4,31 +4,49 @@ import {
   RefObject,
   useCallback,
   useEffect,
+  useMemo,
 } from 'react';
 import { useKbsDisableGlobal } from 'react-kbs';
 
+import { useOnOff } from '../index';
+
+interface UseDialogOptions {
+  dialogRef: RefObject<HTMLDialogElement>;
+  isOpen: boolean;
+  requestCloseOnEsc: boolean;
+  requestCloseOnBackdrop: boolean;
+  onRequestClose?: () => void;
+}
+
+interface UseDialogReturn {
+  dialogProps: {
+    onClick: MouseEventHandler<HTMLDialogElement>;
+    onCancel: ReactEventHandler<HTMLDialogElement>;
+  };
+  isModalShown: boolean;
+}
+
 export function useDialog({
   dialogRef,
   isOpen,
   requestCloseOnEsc,
   requestCloseOnBackdrop,
   onRequestClose,
-}: {
-  dialogRef: RefObject<HTMLDialogElement>;
-  isOpen: boolean;
-  requestCloseOnEsc: boolean;
-  requestCloseOnBackdrop: boolean;
-  onRequestClose?: () => void;
-}) {
+}: UseDialogOptions): UseDialogReturn {
   useKbsDisableGlobal(isOpen);
+  const [isModalShown, showModal, hideModal] = useOnOff(false);
 
   useEffect(() => {
     const dialog = dialogRef.current;
     if (dialog && isOpen) {
+      showModal();
       dialog.showModal();
-      return () => dialog.close();
+      return () => {
+        hideModal();
+        dialog.close();
+      };
     }
-  }, [dialogRef, isOpen]);
+  }, [dialogRef, isOpen, showModal, hideModal]);
 
   const onCancel = useCallback<ReactEventHandler<HTMLDialogElement>>(
     (event) => {
@@ -65,5 +83,13 @@ export function useDialog({
     [dialogRef, requestCloseOnBackdrop, onRequestClose],
   );
 
-  return { onClick, onCancel };
+  const dialogProps = useMemo<UseDialogReturn['dialogProps']>(
+    () => ({ onClick, onCancel }),
+    [onClick, onCancel],
+  );
+
+  return {
+    dialogProps,
+    isModalShown,
+  };
 }
diff --git a/stories/components/modal.stories.tsx b/stories/components/modal.stories.tsx
index c85e5f17..567e43dc 100644
--- a/stories/components/modal.stories.tsx
+++ b/stories/components/modal.stories.tsx
@@ -10,6 +10,7 @@ import {
   FaNpm,
   FaNodeJs,
 } from 'react-icons/fa';
+import { StructureEditor } from 'react-ocl/full';
 
 import {
   Accordion,
@@ -275,6 +276,23 @@ WithComplexContents.args = {
 
 WithComplexContents.argTypes = actions;
 
+export function DynamicallySizedChildren() {
+  const [isOpen, open, close] = useOnOff(false);
+  return (
+    <div style={{ margin: '2em' }}>
+      <Button onClick={open}>Open editor modal</Button>
+      <Modal width={700} height={500} isOpen={isOpen} onRequestClose={close}>
+        <Modal.Header>Test OCL editor in modal</Modal.Header>
+        <Modal.Body>
+          <StructureEditor width={600} height={400} svgMenu />
+        </Modal.Body>
+      </Modal>
+    </div>
+  );
+}
+
+DynamicallySizedChildren.storyName = 'With dynamically sized children';
+
 function DemoPage(props: { openModal: () => void }) {
   return (
     <>