From d2ba34166b90a59332c351f8ecc932c38b858648 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Nicol=C3=A1s=20Hatcher?= <nicolas@theuniverse.today>
Date: Wed, 1 Jan 2025 13:55:42 +0100
Subject: [PATCH] FIX: Refactor model out of the dialogs

---
 .../NameManagerDialog/NameManagerDialog.tsx   | 92 +++++++++++--------
 .../NameManagerDialog/NamedRangeActive.tsx    | 60 ++++--------
 .../NameManagerDialog/NamedRangeInactive.tsx  | 36 ++------
 3 files changed, 80 insertions(+), 108 deletions(-)

diff --git a/webapp/src/components/NameManagerDialog/NameManagerDialog.tsx b/webapp/src/components/NameManagerDialog/NameManagerDialog.tsx
index a10cec8..0b84a09 100644
--- a/webapp/src/components/NameManagerDialog/NameManagerDialog.tsx
+++ b/webapp/src/components/NameManagerDialog/NameManagerDialog.tsx
@@ -48,19 +48,6 @@ function NameManagerDialog(properties: NameManagerDialogProperties) {
     }
   }, [editingNameIndex]);
 
-  const handleNewName = () => {
-    setEditingNameIndex(-1);
-  };
-
-  const handleSave = () => {
-    setEditingNameIndex(-2);
-    onNamesChanged();
-  };
-
-  const handleDelete = () => {
-    onNamesChanged();
-  };
-
   const formatFormula = (): string => {
     const worksheetNames = model.getWorksheetsProperties().map((s) => s.name);
     const selectedView = model.getSelectedView();
@@ -84,31 +71,49 @@ function NameManagerDialog(properties: NameManagerDialogProperties) {
         </StyledRangesHeader>
         <NameListWrapper>
           {definedNameList.map((definedName, index) => {
+            const scopeName = definedName.scope
+              ? worksheets[definedName.scope].name
+              : "[global]";
             if (index === editingNameIndex) {
               return (
                 <NamedRangeActive
                   model={model}
                   worksheets={worksheets}
                   name={definedName.name}
-                  scope={definedName.scope}
+                  scope={scopeName}
                   formula={definedName.formula}
                   key={definedName.name + definedName.scope}
-                  onSave={handleSave}
+                  onSave={(newName, newScope, newFormula) => {
+                    const scope_index = worksheets.findIndex(
+                      (s) => s.name === newScope,
+                    );
+                    const scope = scope_index > 0 ? scope_index : undefined;
+                    model.updateDefinedName(
+                      definedName.name,
+                      definedName.scope,
+                      newName,
+                      scope,
+                      newFormula,
+                    );
+                    setEditingNameIndex(-2);
+                    onNamesChanged();
+                  }}
                   onCancel={() => setEditingNameIndex(-2)}
                 />
               );
             }
             return (
               <NamedRangeInactive
-                model={model}
-                worksheets={worksheets}
                 name={definedName.name}
-                scope={definedName.scope}
+                scope={scopeName}
                 formula={definedName.formula}
                 key={definedName.name + definedName.scope}
                 showOptions={showOptions}
                 onEdit={() => setEditingNameIndex(index)}
-                onDelete={handleDelete}
+                onDelete={() => {
+                  model.deleteDefinedName(definedName.name, definedName.scope);
+                  onNamesChanged();
+                }}
               />
             );
           })}
@@ -119,7 +124,14 @@ function NameManagerDialog(properties: NameManagerDialogProperties) {
             worksheets={worksheets}
             name={""}
             formula={formatFormula()}
-            onSave={handleSave}
+            scope={t("name_manager_dialog.global")}
+            onSave={(name, scope, formula) => {
+              const scope_index = worksheets.findIndex((s) => s.name === scope);
+              const scope_value = scope_index > 0 ? scope_index : undefined;
+              model.newDefinedName(name, scope_value, formula);
+              setEditingNameIndex(-2);
+              onNamesChanged();
+            }}
             onCancel={() => setEditingNameIndex(-2)}
           />
         )}
@@ -132,7 +144,7 @@ function NameManagerDialog(properties: NameManagerDialogProperties) {
           </span>
         </Box>
         <Button
-          onClick={handleNewName}
+          onClick={() => setEditingNameIndex(-1)}
           variant="contained"
           disableElevation
           sx={{ textTransform: "none" }}
@@ -155,13 +167,13 @@ const StyledDialog = styled(Dialog)(() => ({
 }));
 
 const StyledDialogTitle = styled(DialogTitle)`
-padding: 12px 20px;
-height: 20px;
-font-size: 14px;
-font-weight: 600;
-display: flex;
-align-items: center;
-justify-content: space-between;
+  padding: 12px 20px;
+  height: 20px;
+  font-size: 14px;
+  font-weight: 600;
+  display: flex;
+  align-items: center;
+  justify-content: space-between;
 `;
 
 const NameListWrapper = styled(Stack)`
@@ -170,14 +182,14 @@ const NameListWrapper = styled(Stack)`
 `;
 
 const StyledBox = styled(Box)`
-width: 161.67px;
+  width: 161.67px;
 `;
 
 const StyledDialogContent = styled(DialogContent)`
-display: flex;
-flex-direction: column;
-gap: 12px;
-padding: 20px 12px 20px 20px;
+  display: flex;
+  flex-direction: column;
+  gap: 12px;
+  padding: 20px 12px 20px 20px;
 `;
 
 const StyledRangesHeader = styled(Stack)(({ theme }) => ({
@@ -191,13 +203,13 @@ const StyledRangesHeader = styled(Stack)(({ theme }) => ({
 }));
 
 const StyledDialogActions = styled(DialogActions)`
-padding: 12px 20px;
-height: 40px;
-display: flex;
-align-items: center;
-justify-content: space-between;
-font-size: 12px;
-color: #757575;
+  padding: 12px 20px;
+  height: 40px;
+  display: flex;
+  align-items: center;
+  justify-content: space-between;
+  font-size: 12px;
+  color: #757575;
 `;
 
 export default NameManagerDialog;
diff --git a/webapp/src/components/NameManagerDialog/NamedRangeActive.tsx b/webapp/src/components/NameManagerDialog/NamedRangeActive.tsx
index 33f8610..887ecbe 100644
--- a/webapp/src/components/NameManagerDialog/NamedRangeActive.tsx
+++ b/webapp/src/components/NameManagerDialog/NamedRangeActive.tsx
@@ -15,44 +15,22 @@ interface NamedRangeProperties {
   model: Model;
   worksheets: WorksheetProperties[];
   name: string;
-  scope?: number;
+  scope: string;
   formula: string;
-  onSave: () => void;
-  onDelete: () => void;
+  onSave: (name: string, scope: string, formula: string) => void;
   onCancel: () => void;
 }
 
 function NamedRangeActive(properties: NamedRangeProperties) {
-  const { model, worksheets, name, scope, formula, onSave, onCancel } =
-    properties;
-  const [newName, setNewName] = useState(name);
-  const [newScope, setNewScope] = useState(scope);
-  const [newFormula, setNewFormula] = useState(formula);
+  const { worksheets, onSave, onCancel } = properties;
+  const [name, setName] = useState(properties.name);
+  const [scope, setScope] = useState(properties.scope);
+  const [formula, setFormula] = useState(properties.formula);
 
   // TODO: add error messages for validations
   const [nameError, setNameError] = useState(false);
   const [formulaError, setFormulaError] = useState(false);
 
-  // TODO: move logic to NameManagerDialog
-  const handleSaveUpdate = () => {
-    const definedNamesModel = model.getDefinedNameList();
-
-    if (definedNamesModel.find((n) => n.name === name && n.scope === scope)) {
-      try {
-        model.updateDefinedName(name, scope, newName, newScope, newFormula);
-      } catch (error) {
-        console.log("DefinedName update failed", error);
-      }
-    } else {
-      try {
-        model.newDefinedName(newName, newScope, newFormula);
-      } catch (error) {
-        console.log("DefinedName save failed", error);
-      }
-    }
-    onSave();
-  };
-
   return (
     <>
       <StyledBox>
@@ -63,8 +41,8 @@ function NamedRangeActive(properties: NamedRangeProperties) {
           margin="none"
           fullWidth
           error={nameError}
-          value={newName}
-          onChange={(event) => setNewName(event.target.value)}
+          value={name}
+          onChange={(event) => setName(event.target.value)}
           onKeyDown={(event) => {
             event.stopPropagation();
           }}
@@ -77,20 +55,18 @@ function NamedRangeActive(properties: NamedRangeProperties) {
           size="small"
           margin="none"
           fullWidth
-          value={newScope ?? "global"}
+          value={scope}
           onChange={(event) => {
-            event.target.value === "global"
-              ? setNewScope(undefined)
-              : setNewScope(+event.target.value);
+            setScope(event.target.value);
           }}
         >
-          <MenuItem value={"global"}>
+          <MenuItem value={"[global]"}>
             {`${t("name_manager_dialog.workbook")} ${t(
               "name_manager_dialog.global",
             )}`}
           </MenuItem>
-          {worksheets.map((option, index) => (
-            <MenuItem key={option.name} value={index}>
+          {worksheets.map((option) => (
+            <MenuItem key={option.name} value={option.name}>
               {option.name}
             </MenuItem>
           ))}
@@ -102,15 +78,19 @@ function NamedRangeActive(properties: NamedRangeProperties) {
           margin="none"
           fullWidth
           error={formulaError}
-          value={newFormula}
-          onChange={(event) => setNewFormula(event.target.value)}
+          value={formula}
+          onChange={(event) => setFormula(event.target.value)}
           onKeyDown={(event) => {
             event.stopPropagation();
           }}
           onClick={(event) => event.stopPropagation()}
         />
         <IconsWrapper>
-          <IconButton onClick={handleSaveUpdate}>
+          <IconButton
+            onClick={() => {
+              onSave(name, scope, formula);
+            }}
+          >
             <StyledCheck size={12} />
           </IconButton>
           <StyledIconButton onClick={onCancel}>
diff --git a/webapp/src/components/NameManagerDialog/NamedRangeInactive.tsx b/webapp/src/components/NameManagerDialog/NamedRangeInactive.tsx
index 355fd37..2f664fa 100644
--- a/webapp/src/components/NameManagerDialog/NamedRangeInactive.tsx
+++ b/webapp/src/components/NameManagerDialog/NamedRangeInactive.tsx
@@ -1,13 +1,10 @@
-import type { Model, WorksheetProperties } from "@ironcalc/wasm";
 import { Box, Divider, IconButton, styled } from "@mui/material";
 import { t } from "i18next";
 import { PencilLine, Trash2 } from "lucide-react";
 
 interface NamedRangeInactiveProperties {
-  model: Model;
-  worksheets: WorksheetProperties[];
   name: string;
-  scope?: number;
+  scope: string;
   formula: string;
   onDelete: () => void;
   onEdit: () => void;
@@ -15,31 +12,14 @@ interface NamedRangeInactiveProperties {
 }
 
 function NamedRangeInactive(properties: NamedRangeInactiveProperties) {
-  const {
-    model,
-    worksheets,
-    name,
-    scope,
-    formula,
-    onDelete,
-    onEdit,
-    showOptions,
-  } = properties;
+  const { name, scope, formula, onDelete, onEdit, showOptions } = properties;
 
-  // TODO: move logic to NameManagerDialog
-  const handleDelete = () => {
-    try {
-      model.deleteDefinedName(name, scope);
-    } catch (error) {
-      console.log("DefinedName delete failed", error);
-    }
-    onDelete();
-  };
-
-  // TODO: pass the name, avoid logic
   const scopeName =
-    worksheets.find((sheet, index) => index === scope)?.name ||
-    `${t("name_manager_dialog.workbook")} ${t("name_manager_dialog.global")}`;
+    scope === "[global]"
+      ? `${t("name_manager_dialog.workbook")} ${t(
+          "name_manager_dialog.global",
+        )}`
+      : scope;
 
   return (
     <>
@@ -51,7 +31,7 @@ function NamedRangeInactive(properties: NamedRangeInactiveProperties) {
           <StyledIconButtonBlack onClick={onEdit} disabled={!showOptions}>
             <PencilLine size={12} />
           </StyledIconButtonBlack>
-          <StyledIconButtonRed onClick={handleDelete} disabled={!showOptions}>
+          <StyledIconButtonRed onClick={onDelete} disabled={!showOptions}>
             <Trash2 size={12} />
           </StyledIconButtonRed>
         </IconsWrapper>