Skip to content

Commit d0b1647

Browse files
committed
[IMP] model: avoid useless SET_FORMATTING
This commits adds a check for the command `SET_FORMATTING` to avoid useless revisions when the command doesn't change the format/style of any cell. Task: 3149088 Part-of: #7241 Signed-off-by: Pierre Rousseau (pro) <pro@odoo.com>
1 parent 1b3bac1 commit d0b1647

File tree

2 files changed

+56
-1
lines changed

2 files changed

+56
-1
lines changed

packages/o-spreadsheet-engine/src/plugins/core/style.ts

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,12 @@ import { PositionMap } from "../../helpers/cells/position_map";
33
import { getItemId } from "../../helpers/data_normalization";
44
import { recomputeZones } from "../../helpers/recompute_zones";
55
import { intersection, isInside, positionToZone, toZone, zoneToXc } from "../../helpers/zones";
6-
import { AddColumnsRowsCommand, CoreCommand } from "../../types/commands";
6+
import {
7+
AddColumnsRowsCommand,
8+
CommandResult,
9+
CoreCommand,
10+
SetFormattingCommand,
11+
} from "../../types/commands";
712
import {
813
ApplyRangeChange,
914
CellPosition,
@@ -47,6 +52,14 @@ export class StylePlugin extends CorePlugin<StylePluginState> implements StylePl
4752

4853
readonly styles: Record<UID, ZoneStyle[] | undefined> = {};
4954

55+
allowDispatch(cmd: CoreCommand): CommandResult | CommandResult[] {
56+
switch (cmd.type) {
57+
case "SET_FORMATTING":
58+
return this.checkUselessSetFormatting(cmd);
59+
}
60+
return CommandResult.Success;
61+
}
62+
5063
handle(cmd: CoreCommand) {
5164
switch (cmd.type) {
5265
case "ADD_MERGE":
@@ -329,4 +342,29 @@ export class StylePlugin extends CorePlugin<StylePluginState> implements StylePl
329342
exportForExcel(data: ExcelWorkbookData) {
330343
this.export(data);
331344
}
345+
346+
private checkUselessSetFormatting(cmd: SetFormattingCommand) {
347+
const { sheetId, target } = cmd;
348+
const hasStyle = "style" in cmd;
349+
const hasFormat = "format" in cmd;
350+
if (!hasStyle && !hasFormat) {
351+
return CommandResult.NoChanges;
352+
}
353+
for (const zone of recomputeZones(target)) {
354+
for (let col = zone.left; col <= zone.right; col++) {
355+
for (let row = zone.top; row <= zone.bottom; row++) {
356+
const position = { sheetId, col, row };
357+
const cell = this.getters.getCell(position);
358+
const style = this.getCellStyle(position);
359+
if (
360+
(hasStyle && !deepEquals(style, cmd.style)) ||
361+
(hasFormat && cell?.format !== cmd.format)
362+
) {
363+
return CommandResult.Success;
364+
}
365+
}
366+
}
367+
}
368+
return CommandResult.NoChanges;
369+
}
332370
}

tests/cells/style_plugin.test.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import {
55
} from "@odoo/o-spreadsheet-engine/constants";
66
import { Model } from "@odoo/o-spreadsheet-engine/model";
77
import { DEFAULT_STYLE_NO_ALIGN } from "@odoo/o-spreadsheet-engine/plugins/core/style";
8+
import { CommandResult } from "../../src";
89
import { fontSizeInPixels, toCartesian } from "../../src/helpers";
910
import {
1011
addDataValidation,
@@ -19,6 +20,22 @@ import { getCell, getCellContent, getCellStyle } from "../test_helpers/getters_h
1920
import { createEqualCF, target, toRangesData } from "../test_helpers/helpers";
2021

2122
describe("styles", () => {
23+
test("update formatting with the same format as before", () => {
24+
const model = new Model();
25+
expect(setFormat(model, "A1", "#,##0.0")).toBeSuccessfullyDispatched();
26+
expect(setFormat(model, "A1", "#,##0.0")).toBeCancelledBecause(CommandResult.NoChanges);
27+
expect(setFormat(model, "A1:A2", "#,##0.0")).toBeSuccessfullyDispatched();
28+
expect(setFormat(model, "A1:A2", "#,##0.0")).toBeCancelledBecause(CommandResult.NoChanges);
29+
});
30+
31+
test("update style with the same style as before", () => {
32+
const model = new Model();
33+
expect(setStyle(model, "A1", { bold: true })).toBeSuccessfullyDispatched();
34+
expect(setStyle(model, "A1", { bold: true })).toBeCancelledBecause(CommandResult.NoChanges);
35+
expect(setStyle(model, "A1:A2", { bold: true })).toBeSuccessfullyDispatched();
36+
expect(setStyle(model, "A1:A2", { bold: true })).toBeCancelledBecause(CommandResult.NoChanges);
37+
});
38+
2239
test("can undo and redo a setStyle operation on an empty cell", () => {
2340
const model = new Model();
2441
setStyle(model, "B1", { fillColor: "red" });

0 commit comments

Comments
 (0)