From b2744efeb53effbce97f4f7f8eba868a8986dc28 Mon Sep 17 00:00:00 2001 From: Daniel Gonzalez Albo Date: Tue, 11 Nov 2025 23:24:22 +0100 Subject: [PATCH] fix: copilot and nicos comments --- .../NamedRanges/EditNamedRange.tsx | 34 +++++------- .../RightDrawer/NamedRanges/NamedRanges.tsx | 53 ++++++++++++------- .../components/RightDrawer/RightDrawer.tsx | 46 +++++++--------- .../src/components/Workbook/Workbook.tsx | 22 +++++--- webapp/IronCalc/src/locale/en_us.json | 3 +- 5 files changed, 82 insertions(+), 76 deletions(-) diff --git a/webapp/IronCalc/src/components/RightDrawer/NamedRanges/EditNamedRange.tsx b/webapp/IronCalc/src/components/RightDrawer/NamedRanges/EditNamedRange.tsx index 74a2dc5..e3d3428 100644 --- a/webapp/IronCalc/src/components/RightDrawer/NamedRanges/EditNamedRange.tsx +++ b/webapp/IronCalc/src/components/RightDrawer/NamedRanges/EditNamedRange.tsx @@ -12,7 +12,6 @@ import { import { t } from "i18next"; import { Check, Tag } from "lucide-react"; import { useEffect, useState } from "react"; -import type React from "react"; import { theme } from "../../../theme"; import { Footer, NewButton } from "./NamedRanges"; @@ -26,27 +25,22 @@ interface EditNamedRangeProps { name: string; scope: string; formula: string; - onSave: ( - name: string, - scope: string, - formula: string, - ) => SaveError | undefined; + onSave: (name: string, scope: string, formula: string) => SaveError; onCancel: () => void; - definedNameList?: DefinedName[]; - editingDefinedName?: DefinedName | null; + definedNameList: DefinedName[]; + editingDefinedName: DefinedName | null; } -const EditNamedRange: React.FC = ({ +function EditNamedRange({ worksheets, name: initialName, scope: initialScope, formula: initialFormula, onSave, onCancel, - definedNameList = [], - editingDefinedName = null, -}) => { - // Generate default name if empty + definedNameList, + editingDefinedName, +}: EditNamedRangeProps) { const getDefaultName = () => { if (initialName) return initialName; let counter = 1; @@ -253,13 +247,11 @@ const EditNamedRange: React.FC = ({ startIcon={} onClick={() => { const error = onSave(name.trim(), scope, formula); - if (error) { - if (error.nameError) { - setNameError(error.nameError); - } - if (error.formulaError) { - setFormulaError(error.formulaError); - } + if (error.nameError) { + setNameError(error.nameError); + } + if (error.formulaError) { + setFormulaError(error.formulaError); } }} > @@ -268,7 +260,7 @@ const EditNamedRange: React.FC = ({ ); -}; +} const Container = styled("div")({ height: "100%", diff --git a/webapp/IronCalc/src/components/RightDrawer/NamedRanges/NamedRanges.tsx b/webapp/IronCalc/src/components/RightDrawer/NamedRanges/NamedRanges.tsx index 4a70b92..1c44cd9 100644 --- a/webapp/IronCalc/src/components/RightDrawer/NamedRanges/NamedRanges.tsx +++ b/webapp/IronCalc/src/components/RightDrawer/NamedRanges/NamedRanges.tsx @@ -25,21 +25,21 @@ interface NamedRangesProps { worksheets?: WorksheetProperties[]; updateDefinedName?: ( name: string, - scope: number | undefined, + scope: number | null, newName: string, - newScope: number | undefined, + newScope: number | null, newFormula: string, ) => void; newDefinedName?: ( name: string, - scope: number | undefined, + scope: number | null, formula: string, ) => void; - deleteDefinedName?: (name: string, scope: number | undefined) => void; + deleteDefinedName?: (name: string, scope: number | null) => void; selectedArea?: () => string; } -const NamedRanges: React.FC = ({ +function NamedRanges({ title, onClose, definedNameList = [], @@ -48,7 +48,7 @@ const NamedRanges: React.FC = ({ newDefinedName, deleteDefinedName, selectedArea, -}) => { +}: NamedRangesProps) { const [editingDefinedName, setEditingDefinedName] = useState(null); const [isCreatingNew, setIsCreatingNew] = useState(false); @@ -72,35 +72,37 @@ const NamedRanges: React.FC = ({ name: string, scope: string, formula: string, - ): SaveError | undefined => { + ): SaveError => { if (isCreatingNew) { - if (!newDefinedName) return undefined; + if (!newDefinedName) return {}; const scope_index = worksheets.findIndex((s) => s.name === scope); - const newScope = scope_index >= 0 ? scope_index : undefined; + const newScope = scope_index >= 0 ? scope_index : null; try { newDefinedName(name, newScope, formula); setIsCreatingNew(false); - return undefined; + return {}; } catch (e) { // Since name validation is done client-side, errors from model are formula errors return { formulaError: `${e}` }; } } else { - if (!editingDefinedName || !updateDefinedName) return undefined; + if (!editingDefinedName || !updateDefinedName) return {}; const scope_index = worksheets.findIndex((s) => s.name === scope); - const newScope = scope_index >= 0 ? scope_index : undefined; + const newScope = scope_index >= 0 ? scope_index : null; try { updateDefinedName( editingDefinedName.name, - editingDefinedName.scope, + editingDefinedName.scope !== undefined + ? editingDefinedName.scope + : null, name, newScope, formula, ); setEditingDefinedName(null); - return undefined; + return {}; } catch (e) { // Since name validation is done client-side, errors from model are formula errors return { formulaError: `${e}` }; @@ -140,7 +142,7 @@ const NamedRanges: React.FC = ({ handleCancel(); } }} - aria-label="Back to list" + aria-label={t("name_manager_dialog.back")} tabIndex={0} > @@ -170,7 +172,7 @@ const NamedRanges: React.FC = ({ onClose(); } }} - aria-label="Close drawer" + aria-label={t("right_drawer.close")} tabIndex={0} > @@ -223,7 +225,7 @@ const NamedRanges: React.FC = ({ onClose(); } }} - aria-label="Close drawer" + aria-label={t("right_drawer.close")} tabIndex={0} > @@ -257,6 +259,13 @@ const NamedRanges: React.FC = ({ key={`${definedName.name}-${definedName.scope}`} tabIndex={0} $isSelected={isSelected} + onClick={() => handleListItemClick(definedName)} + onKeyDown={(e) => { + if (e.key === "Enter" || e.key === " ") { + e.preventDefault(); + handleListItemClick(definedName); + } + }} > {definedName.name} @@ -291,7 +300,9 @@ const NamedRanges: React.FC = ({ if (deleteDefinedName) { deleteDefinedName( definedName.name, - definedName.scope, + definedName.scope !== undefined + ? definedName.scope + : null, ); } }} @@ -302,7 +313,9 @@ const NamedRanges: React.FC = ({ if (deleteDefinedName) { deleteDefinedName( definedName.name, - definedName.scope, + definedName.scope !== undefined + ? definedName.scope + : null, ); } } @@ -340,7 +353,7 @@ const NamedRanges: React.FC = ({ ); -}; +} const Container = styled("div")({ height: "100%", diff --git a/webapp/IronCalc/src/components/RightDrawer/RightDrawer.tsx b/webapp/IronCalc/src/components/RightDrawer/RightDrawer.tsx index 0840acb..f6033e7 100644 --- a/webapp/IronCalc/src/components/RightDrawer/RightDrawer.tsx +++ b/webapp/IronCalc/src/components/RightDrawer/RightDrawer.tsx @@ -1,7 +1,8 @@ import type { DefinedName, WorksheetProperties } from "@ironcalc/wasm"; import { styled } from "@mui/material/styles"; -import type { MouseEvent as ReactMouseEvent, ReactNode } from "react"; +import type { MouseEvent as ReactMouseEvent } from "react"; import { useCallback, useEffect, useRef, useState } from "react"; +import { useTranslation } from "react-i18next"; import { theme } from "../../theme"; import { TOOLBAR_HEIGHT } from "../constants"; import NamedRanges from "./NamedRanges/NamedRanges"; @@ -13,38 +14,29 @@ const MAX_DRAWER_WIDTH = 500; interface RightDrawerProps { isOpen: boolean; onClose: () => void; - width?: number; - onWidthChange?: (width: number) => void; - children?: ReactNode; - showCloseButton?: boolean; - backgroundColor?: string; - title?: string; - definedNameList?: DefinedName[]; - worksheets?: WorksheetProperties[]; - updateDefinedName?: ( + width: number; + onWidthChange: (width: number) => void; + title: string; + definedNameList: DefinedName[]; + worksheets: WorksheetProperties[]; + updateDefinedName: ( name: string, - scope: number | undefined, + scope: number | null, newName: string, - newScope: number | undefined, + newScope: number | null, newFormula: string, ) => void; - newDefinedName?: ( - name: string, - scope: number | undefined, - formula: string, - ) => void; - deleteDefinedName?: (name: string, scope: number | undefined) => void; - selectedArea?: () => string; + newDefinedName: (name: string, scope: number | null, formula: string) => void; + deleteDefinedName: (name: string, scope: number | null) => void; + selectedArea: () => string; } const RightDrawer = ({ isOpen, onClose, - width = DEFAULT_DRAWER_WIDTH, + width, onWidthChange, - children, - showCloseButton = true, - title = "Named Ranges", + title, definedNameList, worksheets, updateDefinedName, @@ -52,6 +44,7 @@ const RightDrawer = ({ deleteDefinedName, selectedArea, }: RightDrawerProps) => { + const { t } = useTranslation(); const [drawerWidth, setDrawerWidth] = useState(width); const [isResizing, setIsResizing] = useState(false); const resizeHandleRef = useRef(null); @@ -80,7 +73,7 @@ const RightDrawer = ({ Math.min(MAX_DRAWER_WIDTH, newWidth), ); setDrawerWidth(clampedWidth); - onWidthChange?.(clampedWidth); + onWidthChange(clampedWidth); }; const handleMouseUp = () => { @@ -108,14 +101,13 @@ const RightDrawer = ({ ref={resizeHandleRef} onMouseDown={handleMouseDown} $isResizing={isResizing} - aria-label="Resize drawer" + aria-label={t("right_drawer.resize_drawer")} /> - {children} { onClose={() => setDrawerOpen(false)} width={drawerWidth} onWidthChange={setDrawerWidth} + title={t("name_manager_dialog.title")} definedNameList={model.getDefinedNameList()} worksheets={worksheets} updateDefinedName={( name: string, - scope: number | undefined, + scope: number | null, newName: string, - newScope: number | undefined, + newScope: number | null, newFormula: string, ) => { - model.updateDefinedName(name, scope, newName, newScope, newFormula); + model.updateDefinedName( + name, + scope ?? undefined, + newName, + newScope ?? undefined, + newFormula, + ); setRedrawId((id) => id + 1); }} newDefinedName={( name: string, - scope: number | undefined, + scope: number | null, formula: string, ) => { - model.newDefinedName(name, scope, formula); + model.newDefinedName(name, scope ?? undefined, formula); setRedrawId((id) => id + 1); }} - deleteDefinedName={(name: string, scope: number | undefined) => { - model.deleteDefinedName(name, scope); + deleteDefinedName={(name: string, scope: number | null) => { + model.deleteDefinedName(name, scope ?? undefined); setRedrawId((id) => id + 1); }} selectedArea={() => { diff --git a/webapp/IronCalc/src/locale/en_us.json b/webapp/IronCalc/src/locale/en_us.json index d95e7ca..59c0944 100644 --- a/webapp/IronCalc/src/locale/en_us.json +++ b/webapp/IronCalc/src/locale/en_us.json @@ -162,6 +162,7 @@ "custom": "Custom" }, "right_drawer": { - "close": "Close" + "close": "Close", + "resize_drawer": "Resize drawer" } }