From ba13f88924e522ced9b9963a9e049e0fbd2f2e87 Mon Sep 17 00:00:00 2001
From: Christopher Chedeau <vjeuxx@gmail.com>
Date: Sat, 25 Jan 2020 14:11:26 -0800
Subject: [PATCH] Do not trigger modal actions when not visible (#562)

Right now we're running useEffect block which runs getExportCanvasPreview on every state update, even if the modal is not visible (eg: when moving the mouse around!). This puts the modal code in its own component so that it doesn't trigger useEffect when not visible.

The code isn't very elegant as we're forwarding all the props, there's likely a better way to handle it (if anyone is interested, PR would be appreciated), but at least now it no longer double renders the scene.

Fixes #559
---
 src/components/ExportDialog.tsx | 216 ++++++++++++++++++--------------
 1 file changed, 122 insertions(+), 94 deletions(-)

diff --git a/src/components/ExportDialog.tsx b/src/components/ExportDialog.tsx
index 4eba444c7..0acc30ca8 100644
--- a/src/components/ExportDialog.tsx
+++ b/src/components/ExportDialog.tsx
@@ -28,7 +28,7 @@ type ExportCB = (
   scale?: number,
 ) => void;
 
-export function ExportDialog({
+function ExportModal({
   elements,
   appState,
   exportPadding = 10,
@@ -37,6 +37,7 @@ export function ExportDialog({
   onExportToPng,
   onExportToClipboard,
   onExportToBackend,
+  onCloseRequest,
 }: {
   appState: AppState;
   elements: readonly ExcalidrawElement[];
@@ -46,10 +47,10 @@ export function ExportDialog({
   onExportToPng: ExportCB;
   onExportToClipboard: ExportCB;
   onExportToBackend: ExportCB;
+  onCloseRequest: () => void;
 }) {
   const { t } = useTranslation();
   const someElementIsSelected = elements.some(element => element.isSelected);
-  const [modalIsShown, setModalIsShown] = useState(false);
   const [scale, setScale] = useState(defaultScale);
   const [exportSelected, setExportSelected] = useState(someElementIsSelected);
   const previewRef = useRef<HTMLDivElement>(null);
@@ -76,7 +77,6 @@ export function ExportDialog({
       previewNode?.removeChild(canvas);
     };
   }, [
-    modalIsShown,
     exportedElements,
     exportBackground,
     exportPadding,
@@ -84,10 +84,113 @@ export function ExportDialog({
     scale,
   ]);
 
-  function handleClose() {
-    setModalIsShown(false);
-    setExportSelected(someElementIsSelected);
-  }
+  return (
+    <div className="ExportDialog__dialog">
+      <Island padding={4}>
+        <button className="ExportDialog__close" onClick={onCloseRequest}>
+          ╳
+        </button>
+        <h2>{t("buttons.export")}</h2>
+        <div className="ExportDialog__preview" ref={previewRef}></div>
+        <div className="ExportDialog__actions">
+          <Stack.Row gap={2}>
+            <ToolButton
+              type="button"
+              icon={downloadFile}
+              title={t("buttons.exportToPng")}
+              aria-label={t("buttons.exportToPng")}
+              onClick={() => onExportToPng(exportedElements, scale)}
+            />
+            {probablySupportsClipboard && (
+              <ToolButton
+                type="button"
+                icon={clipboard}
+                title={t("buttons.copyToClipboard")}
+                aria-label={t("buttons.copyToClipboard")}
+                onClick={() => onExportToClipboard(exportedElements, scale)}
+              />
+            )}
+            <ToolButton
+              type="button"
+              icon={link}
+              title={t("buttons.getShareableLink")}
+              aria-label={t("buttons.getShareableLink")}
+              onClick={() => onExportToBackend(exportedElements)}
+            />
+          </Stack.Row>
+
+          {actionManager.renderAction(
+            "changeProjectName",
+            elements,
+            appState,
+            syncActionResult,
+            t,
+          )}
+          <Stack.Col gap={1}>
+            <div className="ExportDialog__scales">
+              <Stack.Row gap={1} align="baseline">
+                {scales.map(s => (
+                  <ToolButton
+                    key={s}
+                    size="s"
+                    type="radio"
+                    icon={"x" + s}
+                    name="export-canvas-scale"
+                    aria-label="Export"
+                    id="export-canvas-scale"
+                    checked={scale === s}
+                    onChange={() => setScale(s)}
+                  />
+                ))}
+              </Stack.Row>
+            </div>
+            {actionManager.renderAction(
+              "changeExportBackground",
+              elements,
+              appState,
+              syncActionResult,
+              t,
+            )}
+            {someElementIsSelected && (
+              <div>
+                <label>
+                  <input
+                    type="checkbox"
+                    checked={exportSelected}
+                    onChange={e => setExportSelected(e.currentTarget.checked)}
+                  />{" "}
+                  {t("labels.onlySelected")}
+                </label>
+              </div>
+            )}
+          </Stack.Col>
+        </div>
+      </Island>
+    </div>
+  );
+}
+
+export function ExportDialog({
+  elements,
+  appState,
+  exportPadding = 10,
+  actionManager,
+  syncActionResult,
+  onExportToPng,
+  onExportToClipboard,
+  onExportToBackend,
+}: {
+  appState: AppState;
+  elements: readonly ExcalidrawElement[];
+  exportPadding?: number;
+  actionManager: ActionsManagerInterface;
+  syncActionResult: UpdaterFn;
+  onExportToPng: ExportCB;
+  onExportToClipboard: ExportCB;
+  onExportToBackend: ExportCB;
+}) {
+  const { t } = useTranslation();
+  const [modalIsShown, setModalIsShown] = useState(false);
 
   return (
     <>
@@ -99,93 +202,18 @@ export function ExportDialog({
         title={t("buttons.export")}
       />
       {modalIsShown && (
-        <Modal maxWidth={640} onCloseRequest={handleClose}>
-          <div className="ExportDialog__dialog">
-            <Island padding={4}>
-              <button className="ExportDialog__close" onClick={handleClose}>
-                ╳
-              </button>
-              <h2>{t("buttons.export")}</h2>
-              <div className="ExportDialog__preview" ref={previewRef}></div>
-              <div className="ExportDialog__actions">
-                <Stack.Row gap={2}>
-                  <ToolButton
-                    type="button"
-                    icon={downloadFile}
-                    title={t("buttons.exportToPng")}
-                    aria-label={t("buttons.exportToPng")}
-                    onClick={() => onExportToPng(exportedElements, scale)}
-                  />
-                  {probablySupportsClipboard && (
-                    <ToolButton
-                      type="button"
-                      icon={clipboard}
-                      title={t("buttons.copyToClipboard")}
-                      aria-label={t("buttons.copyToClipboard")}
-                      onClick={() =>
-                        onExportToClipboard(exportedElements, scale)
-                      }
-                    />
-                  )}
-                  <ToolButton
-                    type="button"
-                    icon={link}
-                    title={t("buttons.getShareableLink")}
-                    aria-label={t("buttons.getShareableLink")}
-                    onClick={() => onExportToBackend(exportedElements)}
-                  />
-                </Stack.Row>
-
-                {actionManager.renderAction(
-                  "changeProjectName",
-                  elements,
-                  appState,
-                  syncActionResult,
-                  t,
-                )}
-                <Stack.Col gap={1}>
-                  <div className="ExportDialog__scales">
-                    <Stack.Row gap={1} align="baseline">
-                      {scales.map(s => (
-                        <ToolButton
-                          key={s}
-                          size="s"
-                          type="radio"
-                          icon={"x" + s}
-                          name="export-canvas-scale"
-                          aria-label="Export"
-                          id="export-canvas-scale"
-                          checked={scale === s}
-                          onChange={() => setScale(s)}
-                        />
-                      ))}
-                    </Stack.Row>
-                  </div>
-                  {actionManager.renderAction(
-                    "changeExportBackground",
-                    elements,
-                    appState,
-                    syncActionResult,
-                    t,
-                  )}
-                  {someElementIsSelected && (
-                    <div>
-                      <label>
-                        <input
-                          type="checkbox"
-                          checked={exportSelected}
-                          onChange={e =>
-                            setExportSelected(e.currentTarget.checked)
-                          }
-                        />{" "}
-                        {t("labels.onlySelected")}
-                      </label>
-                    </div>
-                  )}
-                </Stack.Col>
-              </div>
-            </Island>
-          </div>
+        <Modal maxWidth={640} onCloseRequest={() => setModalIsShown(false)}>
+          <ExportModal
+            elements={elements}
+            appState={appState}
+            exportPadding={exportPadding}
+            actionManager={actionManager}
+            syncActionResult={syncActionResult}
+            onExportToPng={onExportToPng}
+            onExportToClipboard={onExportToClipboard}
+            onExportToBackend={onExportToBackend}
+            onCloseRequest={() => setModalIsShown(false)}
+          />
         </Modal>
       )}
     </>