From fe7b9ff3654a8970586d617214a524f550852e7b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9mi=20Rahir=20=28rar=29?= Date: Thu, 2 Mar 2023 11:29:34 +0000 Subject: [PATCH] [REV] charts: Fix chart duplicated ids MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This change is not compatible with some commands in odoo ,which do not contain the sheet id. This reverts commit dbf469fc688990a6a7c724aaed50d5a09a429c92. closes odoo/o-spreadsheet#2150 Signed-off-by: Lucas Lefèvre (lul) --- src/components/figures/chart.ts | 27 +- src/components/figures/container.ts | 5 +- src/components/side_panel/chart_panel.ts | 6 +- src/plugins/core/chart.ts | 121 ++++----- src/plugins/core/figures.ts | 6 +- src/plugins/ui/evaluation_chart.ts | 58 ++--- src/registries/menus/menu_items_actions.ts | 2 +- src/types/commands.ts | 2 +- src/types/getters.ts | 2 +- tests/__snapshots__/xlsx.test.ts.snap | 30 ++- tests/components/charts.test.ts | 21 +- tests/plugins/chart.test.ts | 276 ++++++++------------- 12 files changed, 217 insertions(+), 339 deletions(-) diff --git a/src/components/figures/chart.ts b/src/components/figures/chart.ts index 6420cda4c3..a138405fc8 100644 --- a/src/components/figures/chart.ts +++ b/src/components/figures/chart.ts @@ -75,10 +75,7 @@ export class ChartFigure extends Component { mounted() { const figure = this.props.figure; - const chartData = this.env.getters.getChartRuntime( - this.env.getters.getActiveSheetId(), - figure.id - ); + const chartData = this.env.getters.getChartRuntime(figure.id); if (chartData) { this.createChart(chartData); } @@ -86,8 +83,7 @@ export class ChartFigure extends Component { patched() { const figure = this.props.figure; - const sheetId = this.env.getters.getActiveSheetId(); - const chartData = this.env.getters.getChartRuntime(sheetId, figure.id); + const chartData = this.env.getters.getChartRuntime(figure.id); if (chartData) { if (chartData.type !== this.chart!.config.type) { // Updating a chart type requires to update its options accordingly, if feasible at all. @@ -110,7 +106,7 @@ export class ChartFigure extends Component { } else { this.chart && this.chart.destroy(); } - const def = this.env.getters.getChartDefinition(sheetId, figure.id); + const def = this.env.getters.getChartDefinition(figure.id); if (def) { this.state.background = def.background; } @@ -120,10 +116,7 @@ export class ChartFigure extends Component { const canvas = this.canvas.el as HTMLCanvasElement; const ctx = canvas.getContext("2d")!; this.chart = new window.Chart(ctx, chartData); - const def = this.env.getters.getChartDefinition( - this.env.getters.getActiveSheetId(), - this.props.figure.id - ); + const def = this.env.getters.getChartDefinition(this.props.figure.id); if (def) { this.state.background = def.background; } @@ -134,11 +127,7 @@ export class ChartFigure extends Component { registry.add("edit", { name: _lt("Edit"), sequence: 1, - action: () => - this.env.openSidePanel("ChartPanel", { - sheetId: this.env.getters.getActiveSheetId(), - figure: this.props.figure, - }), + action: () => this.env.openSidePanel("ChartPanel", { figure: this.props.figure }), }); registry.add("delete", { name: _lt("Delete"), @@ -149,10 +138,7 @@ export class ChartFigure extends Component { id: this.props.figure.id, }); if (this.props.sidePanelIsOpen) { - this.env.toggleSidePanel("ChartPanel", { - sheetId: this.env.getters.getActiveSheetId(), - figure: this.props.figure, - }); + this.env.toggleSidePanel("ChartPanel", { figure: this.props.figure }); } this.trigger("figure-deleted"); }, @@ -162,7 +148,6 @@ export class ChartFigure extends Component { sequence: 11, action: () => { this.env.dispatch("REFRESH_CHART", { - sheetId: this.env.getters.getActiveSheetId(), id: this.props.figure.id, }); }, diff --git a/src/components/figures/container.ts b/src/components/figures/container.ts index 824c3ca989..50b35257de 100644 --- a/src/components/figures/container.ts +++ b/src/components/figures/container.ts @@ -251,10 +251,7 @@ export class FiguresContainer extends Component<{ sidePanelIsOpen: Boolean }, Sp return; } if (this.props.sidePanelIsOpen) { - this.env.openSidePanel("ChartPanel", { - sheetId: this.env.getters.getActiveSheetId(), - figure, - }); + this.env.openSidePanel("ChartPanel", { figure }); } const initialX = ev.clientX; const initialY = ev.clientY; diff --git a/src/components/side_panel/chart_panel.ts b/src/components/side_panel/chart_panel.ts index 4c74b049ed..309b05bc12 100644 --- a/src/components/side_panel/chart_panel.ts +++ b/src/components/side_panel/chart_panel.ts @@ -7,7 +7,6 @@ import { DispatchResult, Figure, SpreadsheetEnv, - UID, } from "../../types/index"; import { ColorPicker } from "../color_picker"; import * as icons from "../icons"; @@ -150,7 +149,6 @@ const STYLE = css/* scss */ ` `; interface Props { - sheetId: UID; figure: Figure; } @@ -171,7 +169,7 @@ export class ChartPanel extends Component { private state: ChartPanelState = useState(this.initialState(this.props.figure)); async willUpdateProps(nextProps: Props) { - if (!this.getters.getChartDefinition(nextProps.sheetId, nextProps.figure.id)) { + if (!this.getters.getChartDefinition(nextProps.figure.id)) { this.trigger("close-side-panel"); return; } @@ -241,7 +239,7 @@ export class ChartPanel extends Component { private updateChart(definition: ChartUIDefinitionUpdate): DispatchResult { return this.env.dispatch("UPDATE_CHART", { id: this.props.figure.id, - sheetId: this.props.sheetId, + sheetId: this.getters.getActiveSheetId(), definition, }); } diff --git a/src/plugins/core/chart.ts b/src/plugins/core/chart.ts index 8ebe8f1b90..bb435fd2ac 100644 --- a/src/plugins/core/chart.ts +++ b/src/plugins/core/chart.ts @@ -1,11 +1,5 @@ import { FIGURE_ID_SPLITTER, INCORRECT_RANGE_STRING } from "../../constants"; -import { - deepCopy, - isDefined, - rangeReference, - zoneToDimension, - zoneToXc, -} from "../../helpers/index"; +import { deepCopy, rangeReference, zoneToDimension, zoneToXc } from "../../helpers/index"; import { ApplyRangeChange, ChartDefinition, @@ -35,30 +29,23 @@ import { CorePlugin } from "../core_plugin"; * */ interface ChartState { - readonly chartFigures: { [sheetId: UID]: Record | undefined }; + readonly chartFigures: Record; } export class ChartPlugin extends CorePlugin implements ChartState { - static getters = ["getChartDefinition", "getChartDefinitionUI", "getChartDefinitionsBySheet"]; - readonly chartFigures: ChartState["chartFigures"] = {}; + static getters = ["getChartDefinition", "getChartDefinitionUI", "getChartsIdBySheet"]; + readonly chartFigures: Record = {}; adaptRanges(applyChange: ApplyRangeChange) { - for (const sheetId of Object.keys(this.chartFigures)) { - for (const [chartId, chart] of Object.entries(this.chartFigures[sheetId] || {})) { - if (chart) { - this.adaptDataSetRanges(sheetId, chart, chartId, applyChange); - this.adaptLabelRanges(sheetId, chart, chartId, applyChange); - } + for (let [chartId, chart] of Object.entries(this.chartFigures)) { + if (chart) { + this.adaptDataSetRanges(chart, chartId, applyChange); + this.adaptLabelRanges(chart, chartId, applyChange); } } } - private adaptDataSetRanges( - sheetId: UID, - chart: ChartDefinition, - chartId: UID, - applyChange: ApplyRangeChange - ) { + private adaptDataSetRanges(chart: ChartDefinition, chartId: UID, applyChange: ApplyRangeChange) { for (let ds of chart.dataSets) { if (ds.labelCell) { const labelCellChange = applyChange(ds.labelCell); @@ -66,7 +53,6 @@ export class ChartPlugin extends CorePlugin implements ChartState { case "REMOVE": this.history.update( "chartFigures", - sheetId, chartId, "dataSets", chart.dataSets.indexOf(ds), @@ -79,7 +65,6 @@ export class ChartPlugin extends CorePlugin implements ChartState { case "CHANGE": this.history.update( "chartFigures", - sheetId, chartId, "dataSets", chart.dataSets.indexOf(ds), @@ -92,7 +77,7 @@ export class ChartPlugin extends CorePlugin implements ChartState { switch (dataRangeChange.changeType) { case "REMOVE": const newDataSets = chart.dataSets.filter((dataset) => dataset !== ds); - this.history.update("chartFigures", sheetId, chartId, "dataSets", newDataSets); + this.history.update("chartFigures", chartId, "dataSets", newDataSets); break; case "RESIZE": case "MOVE": @@ -104,7 +89,6 @@ export class ChartPlugin extends CorePlugin implements ChartState { ) { this.history.update( "chartFigures", - sheetId, chartId, "dataSets", chart.dataSets.indexOf(ds), @@ -113,34 +97,23 @@ export class ChartPlugin extends CorePlugin implements ChartState { ); } else { const newDataSets = chart.dataSets.filter((dataset) => dataset !== ds); - this.history.update("chartFigures", sheetId, chartId, "dataSets", newDataSets); + this.history.update("chartFigures", chartId, "dataSets", newDataSets); } break; } } } - private adaptLabelRanges( - sheetId: UID, - chart: ChartDefinition, - chartId: UID, - applyChange: ApplyRangeChange - ) { + private adaptLabelRanges(chart: ChartDefinition, chartId: UID, applyChange: ApplyRangeChange) { if (chart.labelRange) { const labelRangeChange = applyChange(chart.labelRange); switch (labelRangeChange.changeType) { case "REMOVE": - this.history.update("chartFigures", sheetId, chartId, "labelRange", undefined); + this.history.update("chartFigures", chartId, "labelRange", undefined); break; case "RESIZE": case "MOVE": case "CHANGE": - this.history.update( - "chartFigures", - sheetId, - chartId, - "labelRange", - labelRangeChange.range - ); + this.history.update("chartFigures", chartId, "labelRange", labelRangeChange.range); break; } } @@ -180,7 +153,7 @@ export class ChartPlugin extends CorePlugin implements ChartState { }); break; case "UPDATE_CHART": { - this.updateChartDefinition(cmd.sheetId, cmd.id, cmd.definition); + this.updateChartDefinition(cmd.id, cmd.definition); break; } case "DUPLICATE_SHEET": { @@ -191,7 +164,7 @@ export class ChartPlugin extends CorePlugin implements ChartState { const duplicatedFigureId = `${cmd.sheetIdTo}${FIGURE_ID_SPLITTER}${figureIdBase}`; const chartDefinition = { - ...deepCopy(this.chartFigures[cmd.sheetId]![fig.id]!), + ...deepCopy(this.chartFigures[fig.id]), id: duplicatedFigureId, }; chartDefinition.sheetId = cmd.sheetIdTo; @@ -221,10 +194,14 @@ export class ChartPlugin extends CorePlugin implements ChartState { break; } case "DELETE_FIGURE": - this.history.update("chartFigures", cmd.sheetId, cmd.id, undefined); + this.history.update("chartFigures", cmd.id, undefined); break; case "DELETE_SHEET": - this.history.update("chartFigures", cmd.sheetId, undefined); + for (let id of Object.keys(this.chartFigures)) { + if (this.chartFigures[id]?.sheetId === cmd.sheetId) { + this.history.update("chartFigures", id, undefined); + } + } break; } } @@ -233,23 +210,22 @@ export class ChartPlugin extends CorePlugin implements ChartState { // Getters // --------------------------------------------------------------------------- - getChartDefinition(sheetId: UID, figureId: UID): ChartDefinition | undefined { - return this.chartFigures[sheetId]?.[figureId]; + getChartDefinition(figureId: UID): ChartDefinition | undefined { + return this.chartFigures[figureId]; } - getChartDefinitionsBySheet(sheetId: UID) { - return Object.values(this.chartFigures[sheetId] || {}).filter(isDefined); + getChartsIdBySheet(sheetId: UID) { + return Object.entries(this.chartFigures) + .filter((chart) => { + return chart[1].sheetId === sheetId; + }) + .map((chart) => chart[0]); } - getChartDefinitionUI( - sheetId: UID, - figureId: UID, - forceSheetName: boolean = false - ): ChartUIDefinition { - const data: ChartDefinition = this.chartFigures[sheetId]![figureId]!; - const rangeSheetId = forceSheetName ? "forceSheetReference" : sheetId; + getChartDefinitionUI(sheetId: UID, figureId: UID): ChartUIDefinition { + const data: ChartDefinition = this.chartFigures[figureId]; const dataSets: string[] = data.dataSets - .map((ds: DataSet) => (ds ? this.getters.getRangeString(ds.dataRange, rangeSheetId) : "")) + .map((ds: DataSet) => (ds ? this.getters.getRangeString(ds.dataRange, sheetId) : "")) .filter((ds) => { return ds !== ""; // && range !== INCORRECT_RANGE_STRING ? show incorrect #ref ? }); @@ -257,7 +233,7 @@ export class ChartPlugin extends CorePlugin implements ChartState { title: data && data.title ? data.title : "", dataSets, labelRange: data.labelRange - ? this.getters.getRangeString(data.labelRange, rangeSheetId) + ? this.getters.getRangeString(data.labelRange, sheetId) : undefined, type: data ? data.type : "bar", dataSetsHaveTitle: @@ -270,12 +246,12 @@ export class ChartPlugin extends CorePlugin implements ChartState { } private getChartDefinitionExcel(sheetId: UID, figureId: UID): ExcelChartDefinition { - const data: ChartDefinition = this.chartFigures[sheetId]![figureId]!; + const data: ChartDefinition = this.chartFigures[figureId]; const dataSets: ExcelChartDataset[] = data.dataSets .map((ds: DataSet) => this.toExcelDataset(ds)) .filter((ds) => ds.range !== ""); // && range !== INCORRECT_RANGE_STRING ? show incorrect #ref ? return { - ...this.getChartDefinitionUI(sheetId, figureId, true), + ...this.getChartDefinitionUI("forceSheetReference", figureId), backgroundColor: data.background, dataSets, }; @@ -313,17 +289,15 @@ export class ChartPlugin extends CorePlugin implements ChartState { import(data: WorkbookData) { for (let sheet of data.sheets) { if (sheet.figures) { - const charts = {}; for (let figure of sheet.figures) { if (figure.tag === "chart") { const figureData: ChartUIDefinition = { ...figure.data, }; - charts[figure.id] = this.createChartDefinition(figureData, sheet.id); + this.chartFigures[figure.id] = this.createChartDefinition(figureData, sheet.id); delete figure.data; } } - this.chartFigures[sheet.id] = charts; } } } @@ -378,45 +352,44 @@ export class ChartPlugin extends CorePlugin implements ChartState { * Update the chart definition linked to the given id with the attributes * given in the partial UI definition */ - private updateChartDefinition(sheetId: UID, id: UID, definition: ChartUIDefinitionUpdate) { - const chart = this.chartFigures[sheetId]![id]; + private updateChartDefinition(id: UID, definition: ChartUIDefinitionUpdate) { + const chart = this.chartFigures[id]; if (!chart) { throw new Error(`There is no chart with the given id: ${id}`); } if (definition.title !== undefined) { - this.history.update("chartFigures", sheetId, id, "title", definition.title); + this.history.update("chartFigures", id, "title", definition.title); } if (definition.type) { - this.history.update("chartFigures", sheetId, id, "type", definition.type); + this.history.update("chartFigures", id, "type", definition.type); } if (definition.dataSets) { const dataSetsHaveTitle = !!definition.dataSetsHaveTitle; const dataSets = this.createDataSets(definition.dataSets, chart.sheetId, dataSetsHaveTitle); - this.history.update("chartFigures", sheetId, id, "dataSets", dataSets); + this.history.update("chartFigures", id, "dataSets", dataSets); } if (definition.labelRange !== undefined) { const labelRange = definition.labelRange ? this.getters.getRangeFromSheetXC(chart.sheetId, definition.labelRange) : undefined; - this.history.update("chartFigures", sheetId, id, "labelRange", labelRange); + this.history.update("chartFigures", id, "labelRange", labelRange); } if (definition.background) { - this.history.update("chartFigures", sheetId, id, "background", definition.background); + this.history.update("chartFigures", id, "background", definition.background); } if (definition.verticalAxisPosition) { this.history.update( "chartFigures", - sheetId, id, "verticalAxisPosition", definition.verticalAxisPosition ); } if (definition.legendPosition) { - this.history.update("chartFigures", sheetId, id, "legendPosition", definition.legendPosition); + this.history.update("chartFigures", id, "legendPosition", definition.legendPosition); } if (definition.stackedBar !== undefined) { - this.history.update("chartFigures", sheetId, id, "stackedBar", definition.stackedBar); + this.history.update("chartFigures", id, "stackedBar", definition.stackedBar); } } @@ -487,7 +460,7 @@ export class ChartPlugin extends CorePlugin implements ChartState { sheetId, figure, }); - this.history.update("chartFigures", sheetId, figure.id, data); + this.history.update("chartFigures", figure.id, data); } private createDataSet(sheetId: UID, fullZone: Zone, titleZone: Zone | undefined): DataSet { diff --git a/src/plugins/core/figures.ts b/src/plugins/core/figures.ts index 93d31c4456..1e51e5fc80 100644 --- a/src/plugins/core/figures.ts +++ b/src/plugins/core/figures.ts @@ -10,12 +10,14 @@ import { import { CorePlugin } from "../core_plugin"; interface FigureState { - readonly figures: { [sheet: UID]: Record | undefined }; + readonly figures: { [sheet: string]: Record | undefined }; } export class FigurePlugin extends CorePlugin implements FigureState { static getters = ["getFigures", "getFigure"]; - readonly figures: FigureState["figures"] = {}; + readonly figures: { + [sheet: string]: Record | undefined; + } = {}; // --------------------------------------------------------------------------- // Command Handling // --------------------------------------------------------------------------- diff --git a/src/plugins/ui/evaluation_chart.ts b/src/plugins/ui/evaluation_chart.ts index 24dbf61628..a811680dce 100644 --- a/src/plugins/ui/evaluation_chart.ts +++ b/src/plugins/ui/evaluation_chart.ts @@ -23,8 +23,7 @@ export class EvaluationChartPlugin extends UIPlugin { static modes: Mode[] = ["normal"]; // contains the configuration of the chart with it's values like they should be displayed, // as well as all the options needed for the chart library to work correctly - readonly chartRuntime: { [sheetId: UID]: { [figureId: UID]: ChartConfiguration } | undefined } = - {}; + readonly chartRuntime: { [figureId: string]: ChartConfiguration } = {}; private outOfDate: Set = new Set(); handle(cmd: Command) { @@ -33,36 +32,33 @@ export class EvaluationChartPlugin extends UIPlugin { cmd.type === "EVALUATE_CELLS" || (cmd.type === "UPDATE_CELL" && "content" in cmd) ) { - for (const sheetId of Object.keys(this.chartRuntime)) { - for (const chartId of Object.keys(this.chartRuntime[sheetId] || {})) { - this.outOfDate.add(chartId); - } + for (let chartId of Object.keys(this.chartRuntime)) { + this.outOfDate.add(chartId); } } switch (cmd.type) { case "UPDATE_CHART": - case "CREATE_CHART": { - const chartDefinition = this.getters.getChartDefinition(cmd.sheetId, cmd.id)!; - if (!this.chartRuntime[cmd.sheetId]) this.chartRuntime[cmd.sheetId] = {}; - this.chartRuntime[cmd.sheetId]![cmd.id] = this.mapDefinitionToRuntime(chartDefinition); + case "CREATE_CHART": + const chartDefinition = this.getters.getChartDefinition(cmd.id)!; + this.chartRuntime[cmd.id] = this.mapDefinitionToRuntime(chartDefinition); break; - } case "DELETE_FIGURE": - delete this.chartRuntime[cmd.sheetId]?.[cmd.id]; + delete this.chartRuntime[cmd.id]; break; - case "REFRESH_CHART": { - const chartDefinition = this.getters.getChartDefinition(cmd.sheetId, cmd.id)!; - this.evaluateUsedSheets([chartDefinition]); + case "REFRESH_CHART": + this.evaluateUsedSheets([cmd.id]); this.outOfDate.add(cmd.id); break; - } - case "ACTIVATE_SHEET": { - const chartsDefinitions = this.getters.getChartDefinitionsBySheet(cmd.sheetIdTo); - this.evaluateUsedSheets(chartsDefinitions); + case "ACTIVATE_SHEET": + const chartsIds = this.getters.getChartsIdBySheet(cmd.sheetIdTo); + this.evaluateUsedSheets(chartsIds); break; - } case "DELETE_SHEET": - delete this.chartRuntime[cmd.sheetId]; + for (let chartId of Object.keys(this.chartRuntime)) { + if (!this.getters.getChartDefinition(chartId)) { + delete this.chartRuntime[chartId]; + } + } break; } } @@ -71,19 +67,14 @@ export class EvaluationChartPlugin extends UIPlugin { // Getters // --------------------------------------------------------------------------- - getChartRuntime(sheetId: UID, figureId: string): ChartConfiguration | undefined { - if ( - this.outOfDate.has(figureId) || - !(sheetId in this.chartRuntime) || - !(figureId in this.chartRuntime[sheetId]!) - ) { - const chartDefinition = this.getters.getChartDefinition(sheetId, figureId); + getChartRuntime(figureId: string): ChartConfiguration | undefined { + if (this.outOfDate.has(figureId) || !(figureId in this.chartRuntime)) { + const chartDefinition = this.getters.getChartDefinition(figureId); if (chartDefinition === undefined) return; - if (!this.chartRuntime[sheetId]) this.chartRuntime[sheetId] = {}; - this.chartRuntime[sheetId]![figureId] = this.mapDefinitionToRuntime(chartDefinition); + this.chartRuntime[figureId] = this.mapDefinitionToRuntime(chartDefinition); this.outOfDate.delete(figureId); } - return this.chartRuntime[sheetId]![figureId]; + return this.chartRuntime[figureId]; } private truncateLabel(label: string | undefined): string { @@ -194,9 +185,10 @@ export class EvaluationChartPlugin extends UIPlugin { return sheetIds; } - private evaluateUsedSheets(chartsDefinitions: ChartDefinition[]) { + private evaluateUsedSheets(chartsIds: UID[]) { const usedSheetsId: Set = new Set(); - for (const chartDefinition of chartsDefinitions) { + for (let chartId of chartsIds) { + const chartDefinition = this.getters.getChartDefinition(chartId); const sheetsIds = chartDefinition !== undefined ? this.getSheetIdsUsedInChart(chartDefinition) : []; sheetsIds.forEach((sheetId) => { diff --git a/src/registries/menus/menu_items_actions.ts b/src/registries/menus/menu_items_actions.ts index 9f6cbc5bdc..6de575d8d3 100644 --- a/src/registries/menus/menu_items_actions.ts +++ b/src/registries/menus/menu_items_actions.ts @@ -556,7 +556,7 @@ export const CREATE_CHART = (env: SpreadsheetEnv) => { }, }); const figure = env.getters.getFigure(sheetId, id); - env.openSidePanel("ChartPanel", { sheetId, figure }); + env.openSidePanel("ChartPanel", { figure }); }; //------------------------------------------------------------------------------ diff --git a/src/types/commands.ts b/src/types/commands.ts index 12cc4533ff..7086262514 100644 --- a/src/types/commands.ts +++ b/src/types/commands.ts @@ -367,7 +367,7 @@ export interface UpdateChartCommand extends BaseCommand, SheetDependentCommand { definition: ChartUIDefinitionUpdate; } -export interface RefreshChartCommand extends BaseCommand, SheetDependentCommand { +export interface RefreshChartCommand extends BaseCommand { type: "REFRESH_CHART"; id: UID; } diff --git a/src/types/getters.ts b/src/types/getters.ts index 4b365970e2..454d945e11 100644 --- a/src/types/getters.ts +++ b/src/types/getters.ts @@ -91,7 +91,7 @@ export interface CoreGetters { getCellBorder: BordersPlugin["getCellBorder"]; getChartDefinition: ChartPlugin["getChartDefinition"]; - getChartDefinitionsBySheet: ChartPlugin["getChartDefinitionsBySheet"]; + getChartsIdBySheet: ChartPlugin["getChartsIdBySheet"]; getChartDefinitionUI: ChartPlugin["getChartDefinitionUI"]; getRangeString: RangeAdapter["getRangeString"]; diff --git a/tests/__snapshots__/xlsx.test.ts.snap b/tests/__snapshots__/xlsx.test.ts.snap index 8de6c778af..acdad69ed2 100644 --- a/tests/__snapshots__/xlsx.test.ts.snap +++ b/tests/__snapshots__/xlsx.test.ts.snap @@ -1220,17 +1220,16 @@ Object { - + + + + + - - - - - @@ -1239,11 +1238,13 @@ Object { - + + + + - @@ -1267,11 +1268,6 @@ Object { - - - - - @@ -1280,11 +1276,13 @@ Object { - + + + + - @@ -1307,7 +1305,7 @@ Object { - + diff --git a/tests/components/charts.test.ts b/tests/components/charts.test.ts index ca69ccc76c..dfc85c4ff1 100644 --- a/tests/components/charts.test.ts +++ b/tests/components/charts.test.ts @@ -138,8 +138,7 @@ describe("figures", () => { }); test("Click on Delete button will delete the chart", async () => { - const sheetId = model.getters.getActiveSheetId(); - expect(model.getters.getChartDefinition(sheetId, "someuuid")).toMatchObject({ + expect(model.getters.getChartDefinition("someuuid")).toMatchObject({ dataSets: [ { dataRange: { @@ -183,7 +182,7 @@ describe("figures", () => { const deleteButton = fixture.querySelectorAll(".o-menu-item")[1]; expect(deleteButton.textContent).toBe("Delete"); await simulateClick(".o-menu div[data-name='delete']"); - expect(model.getters.getChartRuntime(sheetId, "someuuid")).toBeUndefined(); + expect(model.getters.getChartRuntime("someuuid")).toBeUndefined(); }); test("Click on Edit button will prefill sidepanel", async () => { @@ -285,22 +284,20 @@ describe("figures", () => { const model = parent.model; const sheetId = model.getters.getActiveSheetId(); const figure = model.getters.getFigure(sheetId, chartId); - expect( - parent.model.getters.getChartDefinition(sheetId, chartId)?.labelRange - ).not.toBeUndefined(); - parent.env.openSidePanel("ChartPanel", { sheetId, figure }); + expect(parent.model.getters.getChartDefinition(chartId)?.labelRange).not.toBeUndefined(); + parent.env.openSidePanel("ChartPanel", { figure }); await nextTick(); await simulateClick(".o-data-labels input"); setInputValueAndTrigger(".o-data-labels input", "", "change"); await simulateClick(".o-data-labels .o-selection-ok"); - expect(parent.model.getters.getChartDefinition(sheetId, chartId)?.labelRange).toBeUndefined(); + expect(parent.model.getters.getChartDefinition(chartId)?.labelRange).toBeUndefined(); }); test("empty dataset and invalid label range display both errors", async () => { const model = parent.model; const sheetId = model.getters.getActiveSheetId(); const figure = model.getters.getFigure(sheetId, chartId); - parent.env.openSidePanel("ChartPanel", { sheetId, figure }); + parent.env.openSidePanel("ChartPanel", { figure }); await nextTick(); // empty dataset @@ -344,7 +341,6 @@ describe("figures", () => { }); test("deleting chart will close sidePanel", async () => { - const sheetId = model.getters.getActiveSheetId(); expect(fixture.querySelector(".o-sidePanel .o-sidePanelBody .o-chart")).toBeFalsy(); await simulateClick(".o-figure"); await simulateClick(".o-chart-menu"); @@ -354,7 +350,7 @@ describe("figures", () => { await simulateClick(".o-figure"); await simulateClick(".o-chart-menu"); await simulateClick(".o-menu div[data-name='delete']"); - expect(model.getters.getChartRuntime(sheetId, "someuuid")).toBeUndefined(); + expect(model.getters.getChartRuntime("someuuid")).toBeUndefined(); await nextTick(); expect(fixture.querySelector(".o-sidePanel .o-sidePanelBody .o-chart")).toBeFalsy(); }); @@ -372,7 +368,6 @@ describe("figures", () => { await simulateClick(".o-menu div[data-name='refresh']"); expect(parent.env.dispatch).toHaveBeenCalledWith("REFRESH_CHART", { id: "someuuid", - sheetId: model.getters.getActiveSheetId(), }); }); @@ -534,7 +529,7 @@ describe("charts with multiple sheets", () => { test("delete sheet containing chart data does not crash", async () => { expect(model.getters.getSheetName(model.getters.getActiveSheetId())).toBe("Sheet1"); model.dispatch("DELETE_SHEET", { sheetId: model.getters.getActiveSheetId() }); - const runtimeChart = model.getters.getChartRuntime("Sheet2", "1"); + const runtimeChart = model.getters.getChartRuntime("1"); expect(runtimeChart).toBeDefined(); await nextTick(); expect(fixture.querySelector(".o-chart-container")).toBeDefined(); diff --git a/tests/plugins/chart.test.ts b/tests/plugins/chart.test.ts index 9ced421267..365964418c 100644 --- a/tests/plugins/chart.test.ts +++ b/tests/plugins/chart.test.ts @@ -75,7 +75,7 @@ describe("datasource tests", function () { }, "1" ); - expect(model.getters.getChartDefinition(sheetId, "1")).toMatchObject({ + expect(model.getters.getChartDefinition("1")).toMatchObject({ dataSets: [ { dataRange: { @@ -112,7 +112,7 @@ describe("datasource tests", function () { title: "test", type: "line", }); - expect(model.getters.getChartRuntime(sheetId, "1")).toMatchSnapshot(); + expect(model.getters.getChartRuntime("1")).toMatchSnapshot(); }); test("create chart with rectangle dataset", () => { @@ -126,7 +126,7 @@ describe("datasource tests", function () { }, "1" ); - expect(model.getters.getChartDefinition(sheetId, "1")).toMatchObject({ + expect(model.getters.getChartDefinition("1")).toMatchObject({ dataSets: [ { dataRange: { @@ -163,7 +163,7 @@ describe("datasource tests", function () { title: "test", type: "line", }); - expect(model.getters.getChartRuntime(sheetId, "1")).toMatchSnapshot(); + expect(model.getters.getChartRuntime("1")).toMatchSnapshot(); }); test("create chart with column datasets without series title", () => { @@ -178,7 +178,7 @@ describe("datasource tests", function () { }, "1" ); - expect(model.getters.getChartDefinition(sheetId, "1")).toMatchObject({ + expect(model.getters.getChartDefinition("1")).toMatchObject({ dataSets: [ { dataRange: { @@ -206,7 +206,7 @@ describe("datasource tests", function () { title: "test", type: "line", }); - expect(model.getters.getChartRuntime(sheetId, "1")).toMatchSnapshot(); + expect(model.getters.getChartRuntime("1")).toMatchSnapshot(); }); test("create chart with row datasets", () => { @@ -220,7 +220,7 @@ describe("datasource tests", function () { }, "1" ); - expect(model.getters.getChartDefinition(sheetId, "1")).toMatchObject({ + expect(model.getters.getChartDefinition("1")).toMatchObject({ dataSets: [ { dataRange: { @@ -256,7 +256,7 @@ describe("datasource tests", function () { title: "test", type: "line", }); - expect(model.getters.getChartRuntime(sheetId, "1")).toMatchSnapshot(); + expect(model.getters.getChartRuntime("1")).toMatchSnapshot(); }); test("create chart with row datasets without series title", () => { @@ -271,7 +271,7 @@ describe("datasource tests", function () { }, "1" ); - expect(model.getters.getChartDefinition(sheetId, "1")).toMatchObject({ + expect(model.getters.getChartDefinition("1")).toMatchObject({ dataSets: [ { dataRange: { @@ -299,7 +299,7 @@ describe("datasource tests", function () { title: "test", type: "line", }); - expect(model.getters.getChartRuntime(sheetId, "1")).toMatchSnapshot(); + expect(model.getters.getChartRuntime("1")).toMatchSnapshot(); }); test("create chart with only the dataset title (no data)", () => { @@ -313,7 +313,7 @@ describe("datasource tests", function () { }, "1" ); - expect(model.getters.getChartDefinition(sheetId, "1")).toMatchObject({ + expect(model.getters.getChartDefinition("1")).toMatchObject({ dataSets: [], labelRange: { prefixSheet: true, @@ -324,7 +324,7 @@ describe("datasource tests", function () { title: "test", type: "line", }); - expect(model.getters.getChartRuntime(sheetId, "1")).toMatchSnapshot(); + expect(model.getters.getChartRuntime("1")).toMatchSnapshot(); }); test("create chart with a dataset of one cell (no title)", () => { @@ -339,7 +339,7 @@ describe("datasource tests", function () { }, "1" ); - expect(model.getters.getChartDefinition(sheetId, "1")).toMatchObject({ + expect(model.getters.getChartDefinition("1")).toMatchObject({ dataSets: [ { dataRange: { @@ -359,11 +359,10 @@ describe("datasource tests", function () { title: "test", type: "line", }); - expect(model.getters.getChartRuntime(sheetId, "1")).toMatchSnapshot(); + expect(model.getters.getChartRuntime("1")).toMatchSnapshot(); }); test("create a chart with stacked bar", () => { - const sheetId = model.getters.getActiveSheetId(); createChart( model, { @@ -374,11 +373,10 @@ describe("datasource tests", function () { }, "1" ); - expect(model.getters.getChartRuntime(sheetId, "1")).toMatchSnapshot(); + expect(model.getters.getChartRuntime("1")).toMatchSnapshot(); }); test("ranges in definition change automatically", () => { - const sheetId = model.getters.getActiveSheetId(); createChart( model, { @@ -389,7 +387,7 @@ describe("datasource tests", function () { "1" ); addColumns(model, "before", "A", 2); - const chart = model.getters.getChartDefinition(sheetId, "1")!; + const chart = model.getters.getChartDefinition("1")!; expect(chart.dataSets[0].dataRange.zone).toStrictEqual(toZone("D1:D4")); expect(chart.dataSets[0].labelCell!.zone).toStrictEqual(toZone("D1:D1")); expect(chart.dataSets[1].dataRange.zone).toStrictEqual(toZone("E1:E4")); @@ -398,26 +396,24 @@ describe("datasource tests", function () { }); test("pie chart tooltip title display the correct dataset", () => { - const sheetId = model.getters.getActiveSheetId(); createChart( model, { dataSets: ["B7:B8"], dataSetsHaveTitle: true, labelRange: "B7", type: "pie" }, "1" ); - const title = model.getters.getChartRuntime(sheetId, "1")!.options!.tooltips!.callbacks!.title!; + const title = model.getters.getChartRuntime("1")!.options!.tooltips!.callbacks!.title!; const chartData = { datasets: [{ label: "dataset 1" }, { label: "dataset 2" }] }; expect(title([{ datasetIndex: 0 }], chartData)).toBe("dataset 1"); expect(title([{ datasetIndex: 1 }], chartData)).toBe("dataset 2"); }); test.each(["bar", "line"] as const)("chart %s tooltip title is not dynamic", (chartType) => { - const sheetId = model.getters.getActiveSheetId(); createChart( model, { dataSets: ["B7:B8"], dataSetsHaveTitle: true, labelRange: "B7", type: chartType }, "1" ); - const title = model.getters.getChartRuntime(sheetId, "1")?.options?.tooltips?.callbacks?.title; + const title = model.getters.getChartRuntime("1")?.options?.tooltips?.callbacks?.title; expect(title).toBeUndefined(); }); @@ -435,14 +431,13 @@ describe("datasource tests", function () { const exportedData = model.exportData(); const newModel = new Model(exportedData); expect(newModel.getters.getVisibleFigures(sheetId)).toHaveLength(1); - expect(newModel.getters.getChartRuntime(sheetId, "1")).toBeTruthy(); + expect(newModel.getters.getChartRuntime("1")).toBeTruthy(); newModel.dispatch("DELETE_FIGURE", { sheetId: model.getters.getActiveSheetId(), id: "1" }); expect(newModel.getters.getVisibleFigures(sheetId)).toHaveLength(0); - expect(newModel.getters.getChartRuntime(sheetId, "1")).toBeUndefined(); + expect(newModel.getters.getChartRuntime("1")).toBeUndefined(); }); test("update dataset of imported chart", () => { - const sheetId = model.getters.getActiveSheetId(); createChart( model, { @@ -453,10 +448,10 @@ describe("datasource tests", function () { "1" ); const newModel = new Model(model.exportData()); - let chart = newModel.getters.getChartRuntime(sheetId, "1")!; + let chart = newModel.getters.getChartRuntime("1")!; expect(chart.data!.datasets![0].data).toEqual([10, 11, 12]); setCellContent(newModel, "B2", "99"); - chart = newModel.getters.getChartRuntime(sheetId, "1")!; + chart = newModel.getters.getChartRuntime("1")!; expect(chart.data!.datasets![0].data).toEqual([99, 11, 12]); }); @@ -471,7 +466,7 @@ describe("datasource tests", function () { }, "1" ); - let chart = model.getters.getChartRuntime(sheetId, "1")!; + let chart = model.getters.getChartRuntime("1")!; expect(chart.data!.datasets![0].data).toEqual([10, 11, 12]); expect(chart.type).toEqual("line"); updateChart(model, "1", { @@ -481,8 +476,8 @@ describe("datasource tests", function () { type: "bar", title: "hello1", }); - chart = model.getters.getChartRuntime(sheetId, "1")!; - expect(model.getters.getChartDefinition(sheetId, "1")).toMatchObject({ + chart = model.getters.getChartRuntime("1")!; + expect(model.getters.getChartDefinition("1")).toMatchObject({ dataSets: [ { dataRange: { @@ -524,7 +519,6 @@ describe("datasource tests", function () { }); test("remove labels from existing chart", () => { - const sheetId = model.getters.getActiveSheetId(); createChart( model, { @@ -535,7 +529,7 @@ describe("datasource tests", function () { "1" ); updateChart(model, "1", { labelRange: null }); - expect(model.getters.getChartDefinition(sheetId, "1")?.labelRange).toBeUndefined(); + expect(model.getters.getChartDefinition("1")?.labelRange).toBeUndefined(); }); test("deleting a random sheet does not affect a chart", () => { @@ -575,7 +569,6 @@ describe("datasource tests", function () { }); test("delete a data source column", () => { - const sheetId = model.getters.getActiveSheetId(); createChart( model, { @@ -586,14 +579,13 @@ describe("datasource tests", function () { "1" ); deleteColumns(model, ["B"]); - const chart = model.getters.getChartRuntime(sheetId, "1")!; + const chart = model.getters.getChartRuntime("1")!; expect(chart.data!.datasets![0].data).toEqual([20, 19, 18]); expect(chart.data!.datasets![1]).toBe(undefined); expect(chart.data!.labels).toEqual(["P1", "P2", "P3"]); }); test("delete a data set labels column", () => { - const sheetId = model.getters.getActiveSheetId(); createChart( model, { @@ -605,15 +597,14 @@ describe("datasource tests", function () { ); deleteColumns(model, ["A"]); // dataset in col B becomes labels in col A - expect(model.getters.getChartRuntime(sheetId, "1")!.data!.labels).toEqual(["0", "1", "2"]); - const chart = model.getters.getChartRuntime(sheetId, "1")!; + expect(model.getters.getChartRuntime("1")!.data!.labels).toEqual(["0", "1", "2"]); + const chart = model.getters.getChartRuntime("1")!; expect(chart.data!.datasets![0].data).toEqual([10, 11, 12]); expect(chart.data!.datasets![1].data).toEqual([20, 19, 18]); expect(chart.data!.labels).toEqual(["0", "1", "2"]); }); test("delete last row of dataset", () => { - const sheetId = model.getters.getActiveSheetId(); createChart( model, { @@ -625,14 +616,13 @@ describe("datasource tests", function () { "1" ); deleteRows(model, [4]); - const chart = model.getters.getChartRuntime(sheetId, "1")!; + const chart = model.getters.getChartRuntime("1")!; expect(chart.data!.datasets![0].data).toEqual([10, 11, 12]); expect(chart.data!.datasets![1].data).toEqual([20, 19, 18]); expect(chart.data!.labels).toEqual(["P1", "P2", "P3"]); }); test("delete last col of dataset", () => { - const sheetId = model.getters.getActiveSheetId(); createChart( model, { @@ -644,14 +634,13 @@ describe("datasource tests", function () { "1" ); deleteColumns(model, ["C"]); - const chart = model.getters.getChartRuntime(sheetId, "1")!; + const chart = model.getters.getChartRuntime("1")!; expect(chart.data!.datasets![0].data).toEqual([10, 11, 12, 13]); expect(chart.data!.datasets![1]).toBeUndefined(); expect(chart.data!.labels).toEqual(["P1", "P2", "P3", "P4"]); }); test("add row in dataset", () => { - const sheetId = model.getters.getActiveSheetId(); createChart( model, { @@ -663,7 +652,7 @@ describe("datasource tests", function () { "1" ); addRows(model, "before", 2, 1); - const chart = model.getters.getChartRuntime(sheetId, "1")!; + const chart = model.getters.getChartRuntime("1")!; expect(chart.data!.datasets![0].data).toEqual([10, undefined, 11, 12, 13]); expect(chart.data!.datasets![1].data).toEqual([20, undefined, 19, 18, 17]); expect(chart.data!.labels).toEqual(["P1", "", "P2", "P3", "P4"]); @@ -688,7 +677,6 @@ describe("datasource tests", function () { }); test("delete all the dataset except for the title", () => { - const sheetId = model.getters.getActiveSheetId(); createChart( model, { @@ -700,14 +688,13 @@ describe("datasource tests", function () { "1" ); deleteRows(model, [1, 2, 3, 4]); - const chart = model.getters.getChartRuntime(sheetId, "1")!; + const chart = model.getters.getChartRuntime("1")!; expect(chart.data!.datasets![0].data).toEqual([]); expect(chart.data!.datasets![1].data).toEqual([]); expect(chart.data!.labels).toEqual([]); }); test("update dataset cell updates chart runtime", () => { - const sheetId = model.getters.getActiveSheetId(); createChart( model, { @@ -717,12 +704,12 @@ describe("datasource tests", function () { }, "1" ); - let chart = model.getters.getChartRuntime(sheetId, "1")!; + let chart = model.getters.getChartRuntime("1")!; expect(chart.data!.datasets![0].data).toEqual([10, 11, 12]); expect(chart.data!.datasets![0].label).toEqual("first column dataset"); setCellContent(model, "B2", "99"); setCellContent(model, "B1", "new dataset label"); - chart = model.getters.getChartRuntime(sheetId, "1")!; + chart = model.getters.getChartRuntime("1")!; expect(chart.data!.datasets![0].data).toEqual([99, 11, 12]); expect(chart.data!.datasets![0].label).toEqual("new dataset label"); }); @@ -786,8 +773,8 @@ describe("datasource tests", function () { }, "1" ); - const chart = model.getters.getChartRuntime(sheetId, "1")!; - expect(model.getters.getChartDefinition(sheetId, "1")).toMatchObject({ + const chart = model.getters.getChartRuntime("1")!; + expect(model.getters.getChartDefinition("1")).toMatchObject({ dataSets: [ { dataRange: { @@ -902,7 +889,6 @@ describe("datasource tests", function () { ]); }); test("extend data source to new values manually", () => { - const sheetId = model.getters.getActiveSheetId(); createChart( model, { @@ -916,12 +902,11 @@ describe("datasource tests", function () { labelRange: "Sheet1!A2:A5", dataSetsHaveTitle: true, }); - const chart = model.getters.getChartRuntime(sheetId, "1")!; + const chart = model.getters.getChartRuntime("1")!; expect(chart.data!.datasets![0].data).toEqual([10, 11, 12, 13]); expect(chart.data!.datasets![1].data).toEqual([20, 19, 18, 17]); }); test("extend data set labels to new values manually", () => { - const sheetId = model.getters.getActiveSheetId(); createChart( model, { @@ -934,7 +919,7 @@ describe("datasource tests", function () { dataSets: ["Sheet1!B1:B5", "Sheet1!C1:C5"], labelRange: "Sheet1!A2:A5", }); - const chart = model.getters.getChartRuntime(sheetId, "1")!; + const chart = model.getters.getChartRuntime("1")!; expect(chart.data!.labels).toEqual(["P1", "P2", "P3", "P4"]); }); @@ -950,9 +935,9 @@ describe("datasource tests", function () { "1", "2" ); - expect(model.getters.getChartRuntime("2", "1")).not.toBeUndefined(); + expect(model.getters.getChartRuntime("1")).not.toBeUndefined(); model.dispatch("DELETE_SHEET", { sheetId: "2" }); - expect(model.getters.getChartRuntime("2", "1")).toBeUndefined(); + expect(model.getters.getChartRuntime("1")).toBeUndefined(); }); test("Chart is copied on sheet duplication", () => { @@ -974,10 +959,7 @@ describe("datasource tests", function () { expect(model.getters.getFigures(secondSheetId)).toHaveLength(1); const duplicatedFigure = model.getters.getFigures(secondSheetId)[0]; - const duplicatedChartDefinition = model.getters.getChartDefinition( - secondSheetId, - duplicatedFigure.id - ); + const duplicatedChartDefinition = model.getters.getChartDefinition(duplicatedFigure.id); const expectedDuplicatedChartDefinition = { dataSets: [ { @@ -1000,7 +982,7 @@ describe("datasource tests", function () { deleteSheet(model, firstSheetId); expect(model.getters.getSheets()).toHaveLength(1); expect(model.getters.getFigures(secondSheetId)).toEqual([duplicatedFigure]); - expect(model.getters.getChartDefinition(secondSheetId, duplicatedFigure.id)).toMatchObject( + expect(model.getters.getChartDefinition(duplicatedFigure.id)).toMatchObject( expectedDuplicatedChartDefinition ); }); @@ -1037,17 +1019,17 @@ describe("datasource tests", function () { expect(figuresSh2.length).toEqual(1); expect(figuresSh3.length).toEqual(1); - expect(newModel.getters.getChartDefinitionsBySheet(firstSheetId).length).toEqual(1); - expect(newModel.getters.getChartDefinitionsBySheet(secondSheetId).length).toEqual(1); - expect(newModel.getters.getChartDefinitionsBySheet(thirdSheetId).length).toEqual(1); + expect(newModel.getters.getChartsIdBySheet(firstSheetId).length).toEqual(1); + expect(newModel.getters.getChartsIdBySheet(secondSheetId).length).toEqual(1); + expect(newModel.getters.getChartsIdBySheet(thirdSheetId).length).toEqual(1); expect(figuresSh1[0].id).toEqual("myChart"); expect(figuresSh2[0].id).toEqual(secondSheetId + FIGURE_ID_SPLITTER + "myChart"); expect(figuresSh3[0].id).toEqual(thirdSheetId + FIGURE_ID_SPLITTER + "myChart"); - const chartSh1 = newModel.getters.getChartDefinition(firstSheetId, figuresSh1[0].id); - const chartSh2 = newModel.getters.getChartDefinition(secondSheetId, figuresSh2[0].id); - const chartSh3 = newModel.getters.getChartDefinition(thirdSheetId, figuresSh3[0].id); + const chartSh1 = newModel.getters.getChartDefinition(figuresSh1[0].id); + const chartSh2 = newModel.getters.getChartDefinition(figuresSh2[0].id); + const chartSh3 = newModel.getters.getChartDefinition(figuresSh3[0].id); expect(chartSh1?.sheetId).toBe(firstSheetId); expect(chartSh2?.sheetId).toBe(secondSheetId); @@ -1077,10 +1059,7 @@ describe("datasource tests", function () { sheetId: firstSheetId, }); const duplicatedFigure = model.getters.getFigures(thirdSheetId)[0]; - const duplicatedChartDefinition = model.getters.getChartDefinition( - thirdSheetId, - duplicatedFigure.id - ); + const duplicatedChartDefinition = model.getters.getChartDefinition(duplicatedFigure.id); expect(duplicatedChartDefinition).toMatchObject({ dataSets: [ { @@ -1115,7 +1094,6 @@ describe("datasource tests", function () { describe("title", function () { test("change title manually", () => { - const sheetId = model.getters.getActiveSheetId(); createChart( model, { @@ -1125,16 +1103,15 @@ describe("title", function () { }, "1" ); - let chart = model.getters.getChartRuntime(sheetId, "1")!; + let chart = model.getters.getChartRuntime("1")!; expect(chart.options!.title!.text).toEqual("title"); updateChart(model, "1", { title: "newTitle" }); - chart = model.getters.getChartRuntime(sheetId, "1")!; + chart = model.getters.getChartRuntime("1")!; expect(chart.options!.title!.text).toEqual("newTitle"); }); test("Title is not displayed if empty", () => { - const sheetId = model.getters.getActiveSheetId(); createChart( model, { @@ -1144,16 +1121,15 @@ describe("title", function () { }, "1" ); - expect(model.getters.getChartRuntime(sheetId, "1")?.options?.title?.display).toBe(true); + expect(model.getters.getChartRuntime("1")?.options?.title?.display).toBe(true); updateChart(model, "1", { title: "" }); - expect(model.getters.getChartRuntime(sheetId, "1")?.options?.title?.display).toBe(false); + expect(model.getters.getChartRuntime("1")?.options?.title?.display).toBe(false); }); }); describe("multiple sheets", function () { test("create a chart with data from another sheet", () => { - const newSheetId = "42"; - createSheet(model, { sheetId: newSheetId, activate: true }); + createSheet(model, { sheetId: "42", activate: true }); createChart( model, { @@ -1162,8 +1138,8 @@ describe("multiple sheets", function () { }, "1" ); - const chart = model.getters.getChartRuntime(newSheetId, "1")!; - const chartDefinition = model.getters.getChartDefinition(newSheetId, "1"); + const chart = model.getters.getChartRuntime("1")!; + const chartDefinition = model.getters.getChartDefinition("1"); expect(chart.data!.datasets![0].data).toEqual([10, 11, 12]); expect(chart.data!.datasets![1].data).toEqual([20, 19, 18]); expect(chartDefinition).toMatchObject({ @@ -1194,12 +1170,11 @@ describe("multiple sheets", function () { }, }, ], - sheetId: newSheetId, + sheetId: "42", }); }); test("create a chart with dataset label from another sheet", () => { - const newSheetId = "42"; - createSheet(model, { sheetId: newSheetId, activate: true }); + createSheet(model, { sheetId: "42", activate: true }); createChart( model, { @@ -1208,8 +1183,8 @@ describe("multiple sheets", function () { }, "1" ); - const chart = model.getters.getChartRuntime(newSheetId, "1")!; - const chartDefinition = model.getters.getChartDefinition(newSheetId, "1"); + const chart = model.getters.getChartRuntime("1")!; + const chartDefinition = model.getters.getChartDefinition("1"); expect(chart.data!.labels).toEqual(["P1", "P2", "P3"]); expect(chartDefinition).toMatchObject({ labelRange: { @@ -1217,10 +1192,9 @@ describe("multiple sheets", function () { sheetId: "Sheet1", zone: toZone("A2:A4"), }, - sheetId: newSheetId, + sheetId: "42", }); }); - test("change source data then activate the chart sheet (it should be up-to-date)", () => { createSheet(model, { sheetId: "42", activate: true }); createChart( @@ -1239,7 +1213,7 @@ describe("multiple sheets", function () { content: "99", }); model.dispatch("ACTIVATE_SHEET", { sheetIdFrom: "Sheet1", sheetIdTo: "42" }); - const chart = model.getters.getChartRuntime("42", "28")!; + const chart = model.getters.getChartRuntime("28")!; expect(chart.data!.datasets![0].data).toEqual([99, 11, 12]); }); test("change dataset label then activate the chart sheet (it should be up-to-date)", () => { @@ -1260,12 +1234,11 @@ describe("multiple sheets", function () { content: "miam", }); model.dispatch("ACTIVATE_SHEET", { sheetIdFrom: "Sheet1", sheetIdTo: "42" }); - const chart = model.getters.getChartRuntime("42", "28")!; + const chart = model.getters.getChartRuntime("28")!; expect(chart.data!.labels).toEqual(["P1", "miam", "P3"]); }); test("create a chart with data from another sheet", () => { - const newSheetId = "42"; - createSheet(model, { sheetId: newSheetId, activate: true }); + createSheet(model, { sheetId: "42", activate: true }); createChart( model, { @@ -1274,8 +1247,8 @@ describe("multiple sheets", function () { }, "28" ); - const chart = model.getters.getChartRuntime(newSheetId, "28")!; - const chartDefinition = model.getters.getChartDefinition(newSheetId, "28"); + const chart = model.getters.getChartRuntime("28")!; + const chartDefinition = model.getters.getChartDefinition("28"); expect(chart.data!.datasets![0].data).toEqual([10, 11, 12]); expect(chart.data!.datasets![1].data).toEqual([20, 19, 18]); expect(chartDefinition).toMatchObject({ @@ -1306,7 +1279,7 @@ describe("multiple sheets", function () { }, }, ], - sheetId: newSheetId, + sheetId: "42", }); }); describe("multiple sheets with formulas", function () { @@ -1350,22 +1323,21 @@ describe("multiple sheets", function () { }); }); test("new model with chart with formulas from another sheet (not evaluated yet)", () => { - const chart = model.getters.getChartRuntime(model.getters.getActiveSheetId(), "1")!; + const chart = model.getters.getChartRuntime("1")!; expect(chart.data!.datasets![0].data).toEqual([2, 4]); }); test("refresh chart to update it with new data", () => { - const sheetId = model.getters.getActiveSheetId(); model.dispatch("UPDATE_CELL", { sheetId: "Sheet2", col: 0, row: 0, content: "=Sheet1!B1*3", }); - let chart = model.getters.getChartRuntime(sheetId, "1")!; + let chart = model.getters.getChartRuntime("1")!; expect(chart.data!.datasets![0].data).toEqual(["Loading...", 4]); // data has not been updated :( - model.dispatch("REFRESH_CHART", { sheetId, id: "1" }); - chart = model.getters.getChartRuntime(sheetId, "1")!; + model.dispatch("REFRESH_CHART", { id: "1" }); + chart = model.getters.getChartRuntime("1")!; expect(chart.data!.datasets![0].data).toEqual([3, 4]); model.dispatch("UPDATE_CELL", { @@ -1374,19 +1346,18 @@ describe("multiple sheets", function () { row: 1, content: "5", }); - chart = model.getters.getChartRuntime(sheetId, "1")!; + chart = model.getters.getChartRuntime("1")!; expect(chart.data!.datasets![0].data).toEqual([3, 4]); // data has not been updated :( - model.dispatch("REFRESH_CHART", { sheetId, id: "1" }); - chart = model.getters.getChartRuntime(sheetId, "1")!; + model.dispatch("REFRESH_CHART", { id: "1" }); + chart = model.getters.getChartRuntime("1")!; expect(chart.data!.datasets![0].data).toEqual([3, 10]); }); }); test("export with chart data from a sheet that was deleted, than import data does not crash", () => { const originSheet = model.getters.getActiveSheetId(); - const newSheetId = "42"; - createSheet(model, { sheetId: newSheetId, activate: true }); + createSheet(model, { sheetId: "42", activate: true }); createChart( model, { @@ -1398,7 +1369,7 @@ describe("multiple sheets", function () { model.dispatch("DELETE_SHEET", { sheetId: originSheet }); const exportedData = model.exportData(); const newModel = new Model(exportedData); - const chart = newModel.getters.getChartRuntime(newSheetId, "28")!; + const chart = newModel.getters.getChartRuntime("28")!; expect(chart).toBeDefined(); }); }); @@ -1414,7 +1385,6 @@ describe("undo/redo", () => { expect(model).toExport(after); }); test("undo/redo chart dataset rebuild the chart runtime", () => { - const sheetId = model.getters.getActiveSheetId(); createChart( model, { @@ -1424,16 +1394,16 @@ describe("undo/redo", () => { }, "27" ); - let chart = model.getters.getChartRuntime(sheetId, "27")!; + let chart = model.getters.getChartRuntime("27")!; expect(chart.data!.datasets![0].data).toEqual([10, 11, 12]); setCellContent(model, "B2", "99"); - chart = model.getters.getChartRuntime(sheetId, "27")!; + chart = model.getters.getChartRuntime("27")!; expect(chart.data!.datasets![0].data).toEqual([99, 11, 12]); undo(model); - chart = model.getters.getChartRuntime(sheetId, "27")!; + chart = model.getters.getChartRuntime("27")!; expect(chart.data!.datasets![0].data).toEqual([10, 11, 12]); redo(model); - chart = model.getters.getChartRuntime(sheetId, "27")!; + chart = model.getters.getChartRuntime("27")!; expect(chart.data!.datasets![0].data).toEqual([99, 11, 12]); }); }); @@ -1451,36 +1421,34 @@ describe("Chart without labels", () => { }; test("The legend is not displayed when there is only one dataSet and no label", () => { - const sheetId = model.getters.getActiveSheetId(); createChart(model, defaultChart, "42"); - expect(model.getters.getChartRuntime(sheetId, "42")?.options?.legend?.display).toBe(false); + expect(model.getters.getChartRuntime("42")?.options?.legend?.display).toBe(false); createChart(model, { ...defaultChart, dataSets: ["A1:A2", "A3:A4"] }, "43"); - expect(model.getters.getChartRuntime(sheetId, "43")?.options?.legend?.display).toBeUndefined(); + expect(model.getters.getChartRuntime("43")?.options?.legend?.display).toBeUndefined(); createChart(model, { ...defaultChart, labelRange: "B1:B2" }, "44"); - expect(model.getters.getChartRuntime(sheetId, "44")?.options?.legend?.display).toBeUndefined(); + expect(model.getters.getChartRuntime("44")?.options?.legend?.display).toBeUndefined(); }); test("Labels are empty if there is only one dataSet and no label", () => { - const sheetId = model.getters.getActiveSheetId(); setCellContent(model, "A1", "1"); setCellContent(model, "A2", "2"); createChart(model, defaultChart, "42"); - expect(model.getters.getChartRuntime(sheetId, "42")?.data?.labels).toEqual(["", ""]); + expect(model.getters.getChartRuntime("42")?.data?.labels).toEqual(["", ""]); createChart(model, { ...defaultChart, dataSets: ["A1:A2", "A3:A4"] }, "43"); - expect(model.getters.getChartRuntime(sheetId, "43")?.data?.datasets![0].label).toEqual( + expect(model.getters.getChartRuntime("43")?.data?.datasets![0].label).toEqual( `${chartTerms.Series.toString()} 1` ); - expect(model.getters.getChartRuntime(sheetId, "43")?.data?.datasets![1].label).toEqual( + expect(model.getters.getChartRuntime("43")?.data?.datasets![1].label).toEqual( `${chartTerms.Series.toString()} 2` ); setCellContent(model, "B1", "B1"); setCellContent(model, "B2", "B2"); createChart(model, { ...defaultChart, labelRange: "B1:B2" }, "44"); - expect(model.getters.getChartRuntime(sheetId, "44")?.data?.labels).toEqual(["B1", "B2"]); + expect(model.getters.getChartRuntime("44")?.data?.labels).toEqual(["B1", "B2"]); }); }); @@ -1498,18 +1466,17 @@ describe("Chart design configuration", () => { }; test("Legend position", () => { - const sheetId = model.getters.getActiveSheetId(); createChart(model, defaultChart, "42"); - expect(model.getters.getChartRuntime(sheetId, "42")?.options?.legend?.position).toBe("top"); + expect(model.getters.getChartRuntime("42")?.options?.legend?.position).toBe("top"); updateChart(model, "42", { legendPosition: "left" }); - expect(model.getters.getChartRuntime(sheetId, "42")?.options?.legend?.position).toBe("left"); + expect(model.getters.getChartRuntime("42")?.options?.legend?.position).toBe("left"); updateChart(model, "42", { legendPosition: "right" }); - expect(model.getters.getChartRuntime(sheetId, "42")?.options?.legend?.position).toBe("right"); + expect(model.getters.getChartRuntime("42")?.options?.legend?.position).toBe("right"); updateChart(model, "42", { legendPosition: "bottom" }); - expect(model.getters.getChartRuntime(sheetId, "42")?.options?.legend?.position).toBe("bottom"); + expect(model.getters.getChartRuntime("42")?.options?.legend?.position).toBe("bottom"); }); test("Background is correctly updated", () => { @@ -1525,59 +1492,33 @@ describe("Chart design configuration", () => { }); test("Stacked bar", () => { - const sheetId = model.getters.getActiveSheetId(); createChart(model, defaultChart, "42"); - expect( - model.getters.getChartRuntime(sheetId, "42")?.options?.scales?.xAxes![0].stacked - ).toBeUndefined(); - expect( - model.getters.getChartRuntime(sheetId, "42")?.options?.scales?.yAxes![0].stacked - ).toBeUndefined(); + expect(model.getters.getChartRuntime("42")?.options?.scales?.xAxes![0].stacked).toBeUndefined(); + expect(model.getters.getChartRuntime("42")?.options?.scales?.yAxes![0].stacked).toBeUndefined(); updateChart(model, "42", { stackedBar: true }); - expect(model.getters.getChartRuntime(sheetId, "42")?.options?.scales?.xAxes![0].stacked).toBe( - true - ); - expect(model.getters.getChartRuntime(sheetId, "42")?.options?.scales?.yAxes![0].stacked).toBe( - true - ); + expect(model.getters.getChartRuntime("42")?.options?.scales?.xAxes![0].stacked).toBe(true); + expect(model.getters.getChartRuntime("42")?.options?.scales?.yAxes![0].stacked).toBe(true); updateChart(model, "42", { type: "line" }); - expect( - model.getters.getChartRuntime(sheetId, "42")?.options?.scales?.xAxes![0].stacked - ).toBeUndefined(); - expect( - model.getters.getChartRuntime(sheetId, "42")?.options?.scales?.yAxes![0].stacked - ).toBeUndefined(); + expect(model.getters.getChartRuntime("42")?.options?.scales?.xAxes![0].stacked).toBeUndefined(); + expect(model.getters.getChartRuntime("42")?.options?.scales?.yAxes![0].stacked).toBeUndefined(); updateChart(model, "42", { type: "bar" }); - expect(model.getters.getChartRuntime(sheetId, "42")?.options?.scales?.xAxes![0].stacked).toBe( - true - ); - expect(model.getters.getChartRuntime(sheetId, "42")?.options?.scales?.yAxes![0].stacked).toBe( - true - ); + expect(model.getters.getChartRuntime("42")?.options?.scales?.xAxes![0].stacked).toBe(true); + expect(model.getters.getChartRuntime("42")?.options?.scales?.yAxes![0].stacked).toBe(true); updateChart(model, "42", { stackedBar: false }); - expect( - model.getters.getChartRuntime(sheetId, "42")?.options?.scales?.xAxes![0].stacked - ).toBeUndefined(); - expect( - model.getters.getChartRuntime(sheetId, "42")?.options?.scales?.yAxes![0].stacked - ).toBeUndefined(); + expect(model.getters.getChartRuntime("42")?.options?.scales?.xAxes![0].stacked).toBeUndefined(); + expect(model.getters.getChartRuntime("42")?.options?.scales?.yAxes![0].stacked).toBeUndefined(); }); test("Vertical axis position", () => { - const sheetId = model.getters.getActiveSheetId(); createChart(model, defaultChart, "42"); - expect(model.getters.getChartRuntime(sheetId, "42")?.options?.scales?.yAxes![0].position).toBe( - "left" - ); + expect(model.getters.getChartRuntime("42")?.options?.scales?.yAxes![0].position).toBe("left"); updateChart(model, "42", { verticalAxisPosition: "right" }); - expect(model.getters.getChartRuntime(sheetId, "42")?.options?.scales?.yAxes![0].position).toBe( - "right" - ); + expect(model.getters.getChartRuntime("42")?.options?.scales?.yAxes![0].position).toBe("right"); }); }); @@ -1596,13 +1537,10 @@ describe("Chart evaluation", () => { }, "1" ); - const sheetId = model.getters.getActiveSheetId(); - expect(model.getters.getChartRuntime(sheetId, "1")!.data!.datasets![0]!.data![0]).toBeNull(); + expect(model.getters.getChartRuntime("1")!.data!.datasets![0]!.data![0]).toBeNull(); setCellContent(model, "C3", "1"); - expect(model.getters.getChartRuntime(sheetId, "1")!.data!.datasets![0]!.data![0]).toBe(1); + expect(model.getters.getChartRuntime("1")!.data!.datasets![0]!.data![0]).toBe(1); deleteColumns(model, ["C"]); - expect(model.getters.getChartRuntime(sheetId, "1")!.data!.datasets![0]!.data![0]).toBe( - "#ERROR" - ); + expect(model.getters.getChartRuntime("1")!.data!.datasets![0]!.data![0]).toBe("#ERROR"); }); });