Skip to content

Commit 1f80887

Browse files
committed
[FIX] borders: fix border on column/row deletion
- Fix issue when deleting the first/last row/colum not deleting the exterior borders - Fix issue with wrong border movement when deleting row/col - Fix not removing internal border when deleting row/col closes #5300 Task: 4089281 Signed-off-by: Rémi Rahir (rar) <[email protected]>
1 parent 9d0e335 commit 1f80887

File tree

2 files changed

+103
-2
lines changed

2 files changed

+103
-2
lines changed

src/plugins/core/borders.ts

Lines changed: 61 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,9 +90,55 @@ export class BordersPlugin extends CorePlugin<BordersPluginState> implements Bor
9090
const elements = [...cmd.elements].sort((a, b) => b - a);
9191
for (const group of groupConsecutive(elements)) {
9292
if (cmd.dimension === "COL") {
93-
this.shiftBordersHorizontally(cmd.sheetId, group[group.length - 1] + 1, -group.length);
93+
if (group[0] >= this.getters.getNumberCols(cmd.sheetId)) {
94+
for (let row: HeaderIndex = 0; row < this.getters.getNumberRows(cmd.sheetId); row++) {
95+
this.history.update(
96+
"borders",
97+
cmd.sheetId,
98+
group[0] + 1,
99+
row,
100+
"vertical",
101+
undefined
102+
);
103+
}
104+
}
105+
if (group[group.length - 1] === 0) {
106+
for (let row: HeaderIndex = 0; row < this.getters.getNumberRows(cmd.sheetId); row++) {
107+
this.history.update("borders", cmd.sheetId, 0, row, "vertical", undefined);
108+
}
109+
}
110+
const zone = this.getters.getColsZone(
111+
cmd.sheetId,
112+
group[group.length - 1] + 1,
113+
group[0]
114+
);
115+
this.clearInsideBorders(cmd.sheetId, [zone]);
116+
this.shiftBordersHorizontally(cmd.sheetId, group[0] + 1, -group.length);
94117
} else {
95-
this.shiftBordersVertically(cmd.sheetId, group[group.length - 1] + 1, -group.length);
118+
if (group[0] >= this.getters.getNumberRows(cmd.sheetId)) {
119+
for (let col = 0; col < this.getters.getNumberCols(cmd.sheetId); col++) {
120+
this.history.update(
121+
"borders",
122+
cmd.sheetId,
123+
col,
124+
group[0] + 1,
125+
"horizontal",
126+
undefined
127+
);
128+
}
129+
}
130+
if (group[group.length - 1] === 0) {
131+
for (let col = 0; col < this.getters.getNumberCols(cmd.sheetId); col++) {
132+
this.history.update("borders", cmd.sheetId, col, 0, "horizontal", undefined);
133+
}
134+
}
135+
const zone = this.getters.getRowsZone(
136+
cmd.sheetId,
137+
group[group.length - 1] + 1,
138+
group[0]
139+
);
140+
this.clearInsideBorders(cmd.sheetId, [zone]);
141+
this.shiftBordersVertically(cmd.sheetId, group[0] + 1, -group.length);
96142
}
97143
}
98144
break;
@@ -436,6 +482,19 @@ export class BordersPlugin extends CorePlugin<BordersPluginState> implements Bor
436482
}
437483
}
438484

485+
/**
486+
* Remove the borders inside of a zone
487+
*/
488+
private clearInsideBorders(sheetId: UID, zones: Zone[]) {
489+
for (let zone of zones) {
490+
for (let row = zone.top; row <= zone.bottom; row++) {
491+
for (let col = zone.left; col <= zone.right; col++) {
492+
this.history.update("borders", sheetId, col, row, undefined);
493+
}
494+
}
495+
}
496+
}
497+
439498
/**
440499
* Add a border to the existing one to a cell
441500
*/

tests/plugins/borders.test.ts

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -533,6 +533,48 @@ describe("Grid manipulation", () => {
533533
expect(getBorder(model, "C96")).toEqual({ bottom: b });
534534
expect(getBorder(model, "D96")).toEqual({ right: b, bottom: b });
535535
});
536+
537+
test("Removing multiple rows removes internal borders", () => {
538+
setBorder(model, "hv", "B42:E45");
539+
deleteRows(model, [41, 42, 43, 44]);
540+
expect(model.exportData().borders).toEqual({});
541+
});
542+
543+
test("Removing multiple columns removes internal borders", () => {
544+
setBorder(model, "hv", "B42:E45");
545+
deleteColumns(model, ["B", "C", "D", "E"]);
546+
expect(model.exportData().borders).toEqual({});
547+
});
548+
549+
test("Removing top row removes top border", () => {
550+
setBorder(model, "top", "A1:J1");
551+
deleteRows(model, [0]);
552+
expect(model.exportData().borders).toEqual({});
553+
});
554+
555+
test("Removing bottom row removes bottom border", () => {
556+
setBorder(model, "bottom", "A100:J100");
557+
deleteRows(model, [99]);
558+
expect(model.exportData().borders).toEqual({});
559+
});
560+
561+
test("Removing left-most cost removes left-most border", () => {
562+
setBorder(model, "left", "A1:A6");
563+
deleteColumns(model, ["A"]);
564+
expect(model.exportData().borders).toEqual({});
565+
});
566+
567+
test("Removing right-most row removes right-most border", () => {
568+
setBorder(model, "right", "Z1:Z6");
569+
deleteColumns(model, ["Z"]);
570+
expect(model.exportData().borders).toEqual({});
571+
});
572+
573+
test("Removing bottom two row removes bottom border", () => {
574+
setBorder(model, "bottom", "A100:J100");
575+
deleteRows(model, [98, 99]);
576+
expect(model.exportData().borders).toEqual({});
577+
});
536578
});
537579

538580
test("Cells that have undefined borders don't override borders of neighboring cells at import", () => {

0 commit comments

Comments
 (0)