fix: copilot and nicos comments

This commit is contained in:
Daniel Gonzalez Albo
2025-11-11 23:24:22 +01:00
committed by Nicolás Hatcher Andrés
parent ef6849e822
commit b2744efeb5
5 changed files with 82 additions and 76 deletions

View File

@@ -12,7 +12,6 @@ import {
import { t } from "i18next"; import { t } from "i18next";
import { Check, Tag } from "lucide-react"; import { Check, Tag } from "lucide-react";
import { useEffect, useState } from "react"; import { useEffect, useState } from "react";
import type React from "react";
import { theme } from "../../../theme"; import { theme } from "../../../theme";
import { Footer, NewButton } from "./NamedRanges"; import { Footer, NewButton } from "./NamedRanges";
@@ -26,27 +25,22 @@ interface EditNamedRangeProps {
name: string; name: string;
scope: string; scope: string;
formula: string; formula: string;
onSave: ( onSave: (name: string, scope: string, formula: string) => SaveError;
name: string,
scope: string,
formula: string,
) => SaveError | undefined;
onCancel: () => void; onCancel: () => void;
definedNameList?: DefinedName[]; definedNameList: DefinedName[];
editingDefinedName?: DefinedName | null; editingDefinedName: DefinedName | null;
} }
const EditNamedRange: React.FC<EditNamedRangeProps> = ({ function EditNamedRange({
worksheets, worksheets,
name: initialName, name: initialName,
scope: initialScope, scope: initialScope,
formula: initialFormula, formula: initialFormula,
onSave, onSave,
onCancel, onCancel,
definedNameList = [], definedNameList,
editingDefinedName = null, editingDefinedName,
}) => { }: EditNamedRangeProps) {
// Generate default name if empty
const getDefaultName = () => { const getDefaultName = () => {
if (initialName) return initialName; if (initialName) return initialName;
let counter = 1; let counter = 1;
@@ -253,14 +247,12 @@ const EditNamedRange: React.FC<EditNamedRangeProps> = ({
startIcon={<Check size={16} />} startIcon={<Check size={16} />}
onClick={() => { onClick={() => {
const error = onSave(name.trim(), scope, formula); const error = onSave(name.trim(), scope, formula);
if (error) {
if (error.nameError) { if (error.nameError) {
setNameError(error.nameError); setNameError(error.nameError);
} }
if (error.formulaError) { if (error.formulaError) {
setFormulaError(error.formulaError); setFormulaError(error.formulaError);
} }
}
}} }}
> >
{t("name_manager_dialog.apply")} {t("name_manager_dialog.apply")}
@@ -268,7 +260,7 @@ const EditNamedRange: React.FC<EditNamedRangeProps> = ({
</Footer> </Footer>
</Container> </Container>
); );
}; }
const Container = styled("div")({ const Container = styled("div")({
height: "100%", height: "100%",

View File

@@ -25,21 +25,21 @@ interface NamedRangesProps {
worksheets?: WorksheetProperties[]; worksheets?: WorksheetProperties[];
updateDefinedName?: ( updateDefinedName?: (
name: string, name: string,
scope: number | undefined, scope: number | null,
newName: string, newName: string,
newScope: number | undefined, newScope: number | null,
newFormula: string, newFormula: string,
) => void; ) => void;
newDefinedName?: ( newDefinedName?: (
name: string, name: string,
scope: number | undefined, scope: number | null,
formula: string, formula: string,
) => void; ) => void;
deleteDefinedName?: (name: string, scope: number | undefined) => void; deleteDefinedName?: (name: string, scope: number | null) => void;
selectedArea?: () => string; selectedArea?: () => string;
} }
const NamedRanges: React.FC<NamedRangesProps> = ({ function NamedRanges({
title, title,
onClose, onClose,
definedNameList = [], definedNameList = [],
@@ -48,7 +48,7 @@ const NamedRanges: React.FC<NamedRangesProps> = ({
newDefinedName, newDefinedName,
deleteDefinedName, deleteDefinedName,
selectedArea, selectedArea,
}) => { }: NamedRangesProps) {
const [editingDefinedName, setEditingDefinedName] = const [editingDefinedName, setEditingDefinedName] =
useState<DefinedName | null>(null); useState<DefinedName | null>(null);
const [isCreatingNew, setIsCreatingNew] = useState(false); const [isCreatingNew, setIsCreatingNew] = useState(false);
@@ -72,35 +72,37 @@ const NamedRanges: React.FC<NamedRangesProps> = ({
name: string, name: string,
scope: string, scope: string,
formula: string, formula: string,
): SaveError | undefined => { ): SaveError => {
if (isCreatingNew) { if (isCreatingNew) {
if (!newDefinedName) return undefined; if (!newDefinedName) return {};
const scope_index = worksheets.findIndex((s) => s.name === scope); 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 { try {
newDefinedName(name, newScope, formula); newDefinedName(name, newScope, formula);
setIsCreatingNew(false); setIsCreatingNew(false);
return undefined; return {};
} catch (e) { } catch (e) {
// Since name validation is done client-side, errors from model are formula errors // Since name validation is done client-side, errors from model are formula errors
return { formulaError: `${e}` }; return { formulaError: `${e}` };
} }
} else { } else {
if (!editingDefinedName || !updateDefinedName) return undefined; if (!editingDefinedName || !updateDefinedName) return {};
const scope_index = worksheets.findIndex((s) => s.name === scope); 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 { try {
updateDefinedName( updateDefinedName(
editingDefinedName.name, editingDefinedName.name,
editingDefinedName.scope, editingDefinedName.scope !== undefined
? editingDefinedName.scope
: null,
name, name,
newScope, newScope,
formula, formula,
); );
setEditingDefinedName(null); setEditingDefinedName(null);
return undefined; return {};
} catch (e) { } catch (e) {
// Since name validation is done client-side, errors from model are formula errors // Since name validation is done client-side, errors from model are formula errors
return { formulaError: `${e}` }; return { formulaError: `${e}` };
@@ -140,7 +142,7 @@ const NamedRanges: React.FC<NamedRangesProps> = ({
handleCancel(); handleCancel();
} }
}} }}
aria-label="Back to list" aria-label={t("name_manager_dialog.back")}
tabIndex={0} tabIndex={0}
> >
<ArrowLeft /> <ArrowLeft />
@@ -170,7 +172,7 @@ const NamedRanges: React.FC<NamedRangesProps> = ({
onClose(); onClose();
} }
}} }}
aria-label="Close drawer" aria-label={t("right_drawer.close")}
tabIndex={0} tabIndex={0}
> >
<X /> <X />
@@ -223,7 +225,7 @@ const NamedRanges: React.FC<NamedRangesProps> = ({
onClose(); onClose();
} }
}} }}
aria-label="Close drawer" aria-label={t("right_drawer.close")}
tabIndex={0} tabIndex={0}
> >
<X /> <X />
@@ -257,6 +259,13 @@ const NamedRanges: React.FC<NamedRangesProps> = ({
key={`${definedName.name}-${definedName.scope}`} key={`${definedName.name}-${definedName.scope}`}
tabIndex={0} tabIndex={0}
$isSelected={isSelected} $isSelected={isSelected}
onClick={() => handleListItemClick(definedName)}
onKeyDown={(e) => {
if (e.key === "Enter" || e.key === " ") {
e.preventDefault();
handleListItemClick(definedName);
}
}}
> >
<ListItemText> <ListItemText>
<NameText>{definedName.name}</NameText> <NameText>{definedName.name}</NameText>
@@ -291,7 +300,9 @@ const NamedRanges: React.FC<NamedRangesProps> = ({
if (deleteDefinedName) { if (deleteDefinedName) {
deleteDefinedName( deleteDefinedName(
definedName.name, definedName.name,
definedName.scope, definedName.scope !== undefined
? definedName.scope
: null,
); );
} }
}} }}
@@ -302,7 +313,9 @@ const NamedRanges: React.FC<NamedRangesProps> = ({
if (deleteDefinedName) { if (deleteDefinedName) {
deleteDefinedName( deleteDefinedName(
definedName.name, definedName.name,
definedName.scope, definedName.scope !== undefined
? definedName.scope
: null,
); );
} }
} }
@@ -340,7 +353,7 @@ const NamedRanges: React.FC<NamedRangesProps> = ({
</Footer> </Footer>
</Container> </Container>
); );
}; }
const Container = styled("div")({ const Container = styled("div")({
height: "100%", height: "100%",

View File

@@ -1,7 +1,8 @@
import type { DefinedName, WorksheetProperties } from "@ironcalc/wasm"; import type { DefinedName, WorksheetProperties } from "@ironcalc/wasm";
import { styled } from "@mui/material/styles"; 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 { useCallback, useEffect, useRef, useState } from "react";
import { useTranslation } from "react-i18next";
import { theme } from "../../theme"; import { theme } from "../../theme";
import { TOOLBAR_HEIGHT } from "../constants"; import { TOOLBAR_HEIGHT } from "../constants";
import NamedRanges from "./NamedRanges/NamedRanges"; import NamedRanges from "./NamedRanges/NamedRanges";
@@ -13,38 +14,29 @@ const MAX_DRAWER_WIDTH = 500;
interface RightDrawerProps { interface RightDrawerProps {
isOpen: boolean; isOpen: boolean;
onClose: () => void; onClose: () => void;
width?: number; width: number;
onWidthChange?: (width: number) => void; onWidthChange: (width: number) => void;
children?: ReactNode; title: string;
showCloseButton?: boolean; definedNameList: DefinedName[];
backgroundColor?: string; worksheets: WorksheetProperties[];
title?: string; updateDefinedName: (
definedNameList?: DefinedName[];
worksheets?: WorksheetProperties[];
updateDefinedName?: (
name: string, name: string,
scope: number | undefined, scope: number | null,
newName: string, newName: string,
newScope: number | undefined, newScope: number | null,
newFormula: string, newFormula: string,
) => void; ) => void;
newDefinedName?: ( newDefinedName: (name: string, scope: number | null, formula: string) => void;
name: string, deleteDefinedName: (name: string, scope: number | null) => void;
scope: number | undefined, selectedArea: () => string;
formula: string,
) => void;
deleteDefinedName?: (name: string, scope: number | undefined) => void;
selectedArea?: () => string;
} }
const RightDrawer = ({ const RightDrawer = ({
isOpen, isOpen,
onClose, onClose,
width = DEFAULT_DRAWER_WIDTH, width,
onWidthChange, onWidthChange,
children, title,
showCloseButton = true,
title = "Named Ranges",
definedNameList, definedNameList,
worksheets, worksheets,
updateDefinedName, updateDefinedName,
@@ -52,6 +44,7 @@ const RightDrawer = ({
deleteDefinedName, deleteDefinedName,
selectedArea, selectedArea,
}: RightDrawerProps) => { }: RightDrawerProps) => {
const { t } = useTranslation();
const [drawerWidth, setDrawerWidth] = useState(width); const [drawerWidth, setDrawerWidth] = useState(width);
const [isResizing, setIsResizing] = useState(false); const [isResizing, setIsResizing] = useState(false);
const resizeHandleRef = useRef<HTMLDivElement>(null); const resizeHandleRef = useRef<HTMLDivElement>(null);
@@ -80,7 +73,7 @@ const RightDrawer = ({
Math.min(MAX_DRAWER_WIDTH, newWidth), Math.min(MAX_DRAWER_WIDTH, newWidth),
); );
setDrawerWidth(clampedWidth); setDrawerWidth(clampedWidth);
onWidthChange?.(clampedWidth); onWidthChange(clampedWidth);
}; };
const handleMouseUp = () => { const handleMouseUp = () => {
@@ -108,14 +101,13 @@ const RightDrawer = ({
ref={resizeHandleRef} ref={resizeHandleRef}
onMouseDown={handleMouseDown} onMouseDown={handleMouseDown}
$isResizing={isResizing} $isResizing={isResizing}
aria-label="Resize drawer" aria-label={t("right_drawer.resize_drawer")}
/> />
{children}
<Divider /> <Divider />
<DrawerContent> <DrawerContent>
<NamedRanges <NamedRanges
title={title} title={title}
onClose={showCloseButton ? onClose : undefined} onClose={onClose}
definedNameList={definedNameList} definedNameList={definedNameList}
worksheets={worksheets} worksheets={worksheets}
updateDefinedName={updateDefinedName} updateDefinedName={updateDefinedName}

View File

@@ -5,6 +5,7 @@ import type {
WorksheetProperties, WorksheetProperties,
} from "@ironcalc/wasm"; } from "@ironcalc/wasm";
import { styled } from "@mui/material/styles"; import { styled } from "@mui/material/styles";
import { t } from "i18next";
import { useCallback, useEffect, useRef, useState } from "react"; import { useCallback, useEffect, useRef, useState } from "react";
import FormulaBar from "../FormulaBar/FormulaBar"; import FormulaBar from "../FormulaBar/FormulaBar";
import RightDrawer, { DEFAULT_DRAWER_WIDTH } from "../RightDrawer/RightDrawer"; import RightDrawer, { DEFAULT_DRAWER_WIDTH } from "../RightDrawer/RightDrawer";
@@ -742,28 +743,35 @@ const Workbook = (props: { model: Model; workbookState: WorkbookState }) => {
onClose={() => setDrawerOpen(false)} onClose={() => setDrawerOpen(false)}
width={drawerWidth} width={drawerWidth}
onWidthChange={setDrawerWidth} onWidthChange={setDrawerWidth}
title={t("name_manager_dialog.title")}
definedNameList={model.getDefinedNameList()} definedNameList={model.getDefinedNameList()}
worksheets={worksheets} worksheets={worksheets}
updateDefinedName={( updateDefinedName={(
name: string, name: string,
scope: number | undefined, scope: number | null,
newName: string, newName: string,
newScope: number | undefined, newScope: number | null,
newFormula: string, newFormula: string,
) => { ) => {
model.updateDefinedName(name, scope, newName, newScope, newFormula); model.updateDefinedName(
name,
scope ?? undefined,
newName,
newScope ?? undefined,
newFormula,
);
setRedrawId((id) => id + 1); setRedrawId((id) => id + 1);
}} }}
newDefinedName={( newDefinedName={(
name: string, name: string,
scope: number | undefined, scope: number | null,
formula: string, formula: string,
) => { ) => {
model.newDefinedName(name, scope, formula); model.newDefinedName(name, scope ?? undefined, formula);
setRedrawId((id) => id + 1); setRedrawId((id) => id + 1);
}} }}
deleteDefinedName={(name: string, scope: number | undefined) => { deleteDefinedName={(name: string, scope: number | null) => {
model.deleteDefinedName(name, scope); model.deleteDefinedName(name, scope ?? undefined);
setRedrawId((id) => id + 1); setRedrawId((id) => id + 1);
}} }}
selectedArea={() => { selectedArea={() => {

View File

@@ -162,6 +162,7 @@
"custom": "Custom" "custom": "Custom"
}, },
"right_drawer": { "right_drawer": {
"close": "Close" "close": "Close",
"resize_drawer": "Resize drawer"
} }
} }