Skip to content

Commit 34d53fe

Browse files
committed
[IMP] topbar: improve clear formatting action
With this commit, the clear formatting action does a bit more than clearing the format/style: it also clears merges, conditional formats and table styles. We kept the old `CLEAR_FORMATTING` as it was so is doesn't break existing spreadsheet (and the command is still used once), and created a new `CLEAR_ALL_STYLING` command that clears everything else and dispatches a `CLEAR_FORMATTING`. Task: 5391157
1 parent 433fa7a commit 34d53fe

8 files changed

Lines changed: 171 additions & 4 deletions

File tree

src/actions/format_actions.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -398,7 +398,7 @@ export const clearFormat: ActionSpec = {
398398
name: _t("Clear formatting"),
399399
shortcut: "Ctrl+<",
400400
execute: (env) =>
401-
env.model.dispatch("CLEAR_FORMATTING", {
401+
env.model.dispatch("CLEAR_ALL_STYLING", {
402402
sheetId: env.model.getters.getActiveSheetId(),
403403
target: env.model.getters.getSelectedZones(),
404404
}),

src/components/grid/grid.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -797,7 +797,7 @@ export class Grid extends Component<SpreadsheetChildEnv> {
797797
}
798798

799799
private clearFormatting() {
800-
this.env.model.dispatch("CLEAR_FORMATTING", {
800+
this.env.model.dispatch("CLEAR_ALL_STYLING", {
801801
sheetId: this.env.model.getters.getActiveSheetId(),
802802
target: this.env.model.getters.getSelectedZones(),
803803
});

src/plugins/ui_feature/ui_sheet.ts

Lines changed: 65 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,8 @@ import {
88
import { isColorValid } from "../../helpers/color";
99
import { formatValue } from "../../helpers/format/format";
1010
import { localizeFormula } from "../../helpers/locale";
11-
import { groupConsecutive, largeMax, range } from "../../helpers/misc";
11+
import { deepCopy, groupConsecutive, largeMax, range } from "../../helpers/misc";
12+
import { recomputeZones } from "../../helpers/recompute_zones";
1213
import {
1314
computeMultilineTextSize,
1415
computeTextLinesHeight,
@@ -17,7 +18,7 @@ import {
1718
getCellContentHeight,
1819
splitTextToWidth,
1920
} from "../../helpers/text_helper";
20-
import { isEqual, positions } from "../../helpers/zones";
21+
import { isEqual, isZoneInside, overlap, positions } from "../../helpers/zones";
2122
import { CellValueType } from "../../types/cells";
2223
import { Command, CommandResult, LocalCommand } from "../../types/commands";
2324
import {
@@ -96,6 +97,9 @@ export class SheetUIPlugin extends UIPlugin {
9697
});
9798
this.dispatch("SET_SHEET_BACKGROUND_COLOR", { sheetId: cmd.sheetId, color: cmd.color });
9899
break;
100+
case "CLEAR_ALL_STYLING":
101+
this.clearAllStyling(cmd.sheetId, cmd.target);
102+
break;
99103
}
100104
}
101105

@@ -349,4 +353,63 @@ export class SheetUIPlugin extends UIPlugin {
349353
});
350354
}
351355
}
356+
357+
private clearAllStyling(sheetId: UID, target: Zone[]) {
358+
const tables = this.getters.getTables(sheetId);
359+
for (const table of tables) {
360+
if (target.some((zone) => isZoneInside(table.range.zone, zone))) {
361+
if (table.isPivotTable) {
362+
const position = { sheetId, col: table.range.zone.left, row: table.range.zone.top };
363+
const pivotId = this.getters.getPivotIdFromPosition(position);
364+
if (pivotId) {
365+
const definition = deepCopy(this.getters.getPivotCoreDefinition(pivotId));
366+
definition.style = { ...definition.style, tableStyleId: "None" };
367+
this.dispatch("UPDATE_PIVOT", { pivotId, pivot: definition });
368+
}
369+
} else {
370+
this.dispatch("UPDATE_TABLE", {
371+
sheetId,
372+
zone: table.range.zone,
373+
config: { ...table.config, styleId: "None" },
374+
});
375+
}
376+
}
377+
}
378+
379+
this.dispatch("CLEAR_FORMATTING", { sheetId, target });
380+
381+
const merges = this.getters.getMerges(sheetId);
382+
const mergesInsideTarget = merges.filter((merge) =>
383+
target.some((zone) => isZoneInside(merge, zone))
384+
);
385+
if (mergesInsideTarget.length > 0) {
386+
this.dispatch("REMOVE_MERGE", { sheetId, target: mergesInsideTarget });
387+
}
388+
389+
const conditionalFormats = this.getters.getConditionalFormats(sheetId);
390+
for (const cf of conditionalFormats) {
391+
const cfRanges = cf.ranges.map((range) => this.getters.getRangeFromSheetXC(sheetId, range));
392+
const hasOverlap = target.some((zone) =>
393+
cfRanges
394+
.map((range) => range.zone)
395+
.some((cfZone) => isZoneInside(cfZone, zone) || overlap(cfZone, zone))
396+
);
397+
if (!hasOverlap) {
398+
continue;
399+
}
400+
const newZones = recomputeZones(
401+
cfRanges.map((range) => range.unboundedZone),
402+
target
403+
);
404+
if (newZones.length === 0) {
405+
this.dispatch("REMOVE_CONDITIONAL_FORMAT", { sheetId, id: cf.id });
406+
} else {
407+
this.dispatch("ADD_CONDITIONAL_FORMAT", {
408+
sheetId,
409+
cf,
410+
ranges: newZones.map((zone) => this.getters.getRangeDataFromZone(sheetId, zone)),
411+
});
412+
}
413+
}
414+
}
352415
}

src/registries/repeat_commands_registry.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ repeatCommandTransformRegistry.add("REMOVE_MERGE", genericRepeat);
3333

3434
repeatCommandTransformRegistry.add("SET_FORMATTING", genericRepeat);
3535
repeatCommandTransformRegistry.add("CLEAR_FORMATTING", genericRepeat);
36+
repeatCommandTransformRegistry.add("CLEAR_ALL_STYLING", genericRepeat);
3637
repeatCommandTransformRegistry.add("SET_BORDER", genericRepeat);
3738

3839
repeatCommandTransformRegistry.add("CREATE_TABLE", genericRepeat);

src/types/commands.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -792,6 +792,10 @@ export interface ClearFormattingCommand extends TargetDependentCommand {
792792
type: "CLEAR_FORMATTING";
793793
}
794794

795+
export interface ClearAllStylingCommand extends TargetDependentCommand {
796+
type: "CLEAR_ALL_STYLING";
797+
}
798+
795799
export interface SetDecimalCommand extends TargetDependentCommand {
796800
type: "SET_DECIMAL";
797801
step: SetDecimalStep;
@@ -1392,6 +1396,7 @@ export type LocalCommand =
13921396
| DeleteFiguresCommand
13931397
| MergeIntoCarouselCommand
13941398
| CreateChartAndMergeIntoCarouselCommand
1399+
| ClearAllStylingCommand
13951400
| ColorAllCellsBackground;
13961401

13971402
export type Command = CoreCommand | LocalCommand;

tests/repeat_commands_plugin.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ describe("Repeat commands basics", () => {
7373
"ADD_MERGE",
7474
"REMOVE_MERGE",
7575
"SET_FORMATTING",
76+
"CLEAR_ALL_STYLING",
7677
"CLEAR_FORMATTING",
7778
"SET_BORDER",
7879
"CREATE_TABLE",

tests/sheet/ui_sheet.test.ts

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
import { Model, SpreadsheetPivotCoreDefinition, UID } from "../../src";
2+
import { zoneToXc } from "../../src/helpers/zones";
3+
import {
4+
addEqualCf,
5+
clearAllStyling,
6+
createTable,
7+
merge,
8+
setBordersOnTarget,
9+
setCellContent,
10+
setFormatting,
11+
} from "../test_helpers";
12+
import { createModelFromGrid, toCellPosition } from "../test_helpers/helpers";
13+
import { addPivot } from "../test_helpers/pivot_helpers";
14+
15+
let model: Model;
16+
let sheetId: UID;
17+
18+
beforeEach(() => {
19+
model = new Model();
20+
sheetId = model.getters.getActiveSheetId();
21+
});
22+
23+
describe("CLEAR_ALL_STYLING command", () => {
24+
test("Can clear the cell style", () => {
25+
setFormatting(model, "A1:A3", { bold: true });
26+
setBordersOnTarget(model, ["A1:A3"], { bottom: { color: "#ff0000", style: "thin" } });
27+
28+
clearAllStyling(model, "A1:A2");
29+
expect(model.getters.getCellStyle(toCellPosition(sheetId, "A1"))).toEqual({});
30+
expect(model.getters.getCellStyle(toCellPosition(sheetId, "A2"))).toEqual({});
31+
expect(model.getters.getCellStyle(toCellPosition(sheetId, "A3"))).toEqual({ bold: true });
32+
33+
expect(model.getters.getCellBorder(toCellPosition(sheetId, "A1"))).toEqual(null);
34+
expect(model.getters.getCellBorder(toCellPosition(sheetId, "A2"))).toEqual(null);
35+
expect(model.getters.getCellBorder(toCellPosition(sheetId, "A3"))).toEqual({
36+
bottom: { color: "#ff0000", style: "thin" },
37+
});
38+
});
39+
40+
test("Clear merges fully inside the target", () => {
41+
merge(model, "A1:B2");
42+
merge(model, "C1:D2");
43+
44+
clearAllStyling(model, "A1:C2");
45+
expect(model.getters.getMerges(sheetId).map(zoneToXc)).toEqual(["C1:D2"]);
46+
});
47+
48+
test("Clear table style for tables fully inside the target", () => {
49+
createTable(model, "A1:B2", { styleId: "TableStyleLight1" });
50+
createTable(model, "C1:D2", { styleId: "TableStyleLight2" });
51+
52+
clearAllStyling(model, "A1:C2");
53+
expect(model.getters.getTables(sheetId)).toMatchObject([
54+
{ config: { styleId: "None" } },
55+
{ config: { styleId: "TableStyleLight2" } },
56+
]);
57+
});
58+
59+
test("Clear pivot table style for pivot tables fully inside the target", () => {
60+
const grid = { A1: "Customer", B1: "Price", A2: "Alice", B2: "10" };
61+
const model = createModelFromGrid(grid);
62+
const pivotDefinition: Partial<SpreadsheetPivotCoreDefinition> = {
63+
columns: [],
64+
rows: [{ fieldName: "Customer" }],
65+
measures: [{ id: "Price:sum", fieldName: "Price", aggregator: "sum" }],
66+
style: { tableStyleId: "PivotTableStyleMedium9" },
67+
};
68+
addPivot(model, "A1:B2", pivotDefinition, "pivot1");
69+
addPivot(model, "A1:B2", pivotDefinition, "pivot2");
70+
71+
setCellContent(model, "A4", "=PIVOT(1)");
72+
setCellContent(model, "D4", "=PIVOT(2)");
73+
74+
clearAllStyling(model, "A1:D10");
75+
expect(model.getters.getPivotCoreDefinition("pivot1")?.style?.tableStyleId).toEqual("None");
76+
expect(model.getters.getPivotCoreDefinition("pivot2")?.style?.tableStyleId).toEqual(
77+
"PivotTableStyleMedium9"
78+
);
79+
});
80+
81+
test("Clear conditional formats", () => {
82+
addEqualCf(model, "A1:A7", { fillColor: "#FF0000" }, "5", "cf1");
83+
addEqualCf(model, "C2:C3", { fillColor: "#FF0000" }, "5", "cf2");
84+
85+
clearAllStyling(model, "A2:C3");
86+
expect(model.getters.getConditionalFormats(sheetId).map((cf) => cf.id)).toEqual(["cf1"]);
87+
expect(model.getters.getConditionalFormats(sheetId)[0].ranges).toEqual(["A1", "A4:A7"]);
88+
});
89+
});

tests/test_helpers/commands_helpers.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1332,6 +1332,14 @@ export function clearFormatting(
13321332
return model.dispatch("CLEAR_FORMATTING", { sheetId, target: target(targetXc) });
13331333
}
13341334

1335+
export function clearAllStyling(
1336+
model: Model,
1337+
targetXc: string,
1338+
sheetId: UID = model.getters.getActiveSheetId()
1339+
) {
1340+
return model.dispatch("CLEAR_ALL_STYLING", { sheetId, target: target(targetXc) });
1341+
}
1342+
13351343
/**
13361344
* Freeze a given number of rows on top of the sheet
13371345
*/

0 commit comments

Comments
 (0)