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

@@ -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>
       )}
     </>