Skip to content

Commit

Permalink
[FIX] borders: wrong borders on remove rows
Browse files Browse the repository at this point in the history
When removing multiple headers, the borders are not shifted
correctly, leading to incorrect borders.

This happens because we modify the borders for each deleted rows
separately. But we don't sort the rows before modifying the borders.
So we first delete the row 1, then the row 2. But the row 2 is now
the row 1, since the old row 1 has been deleted. So we end up with wrong
operations.

This is solved by simply sorting the rows in reverse order before
modifying the borders.

closes #4121

Task: 3884112
X-original-commit: 0dbad31
Signed-off-by: Adrien Minne (adrm) <adrm@odoo.com>
Signed-off-by: Lucas Lefèvre (lul) <lul@odoo.com>
  • Loading branch information
hokolomopo committed Apr 24, 2024
1 parent a54fa7f commit d28b9b4
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 1 deletion.
2 changes: 1 addition & 1 deletion src/plugins/core/borders.ts
Expand Up @@ -100,7 +100,7 @@ export class BordersPlugin extends CorePlugin<BordersPluginState> implements Bor
this.clearBorders(cmd.sheetId, cmd.target);
break;
case "REMOVE_COLUMNS_ROWS":
for (let el of cmd.elements) {
for (let el of [...cmd.elements].sort((a, b) => b - a)) {
if (cmd.dimension === "COL") {
this.shiftBordersHorizontally(cmd.sheetId, el + 1, -1);
} else {
Expand Down
17 changes: 17 additions & 0 deletions tests/borders/border_plugin.test.ts
Expand Up @@ -6,6 +6,8 @@ import {
addRows,
cut,
deleteCells,
deleteColumns,
deleteRows,
paste,
selectCell,
setAnchorCorner,
Expand Down Expand Up @@ -573,6 +575,21 @@ describe("Grid manipulation", () => {
expect(getBorder(model, "B4")).toEqual({ top: DEFAULT_BORDER_DESC });
});

test("Remove multiple headers before the borders", () => {
const b = DEFAULT_BORDER_DESC;
setZoneBorders(model, { position: "external" }, ["C3"]);
deleteRows(model, [0, 1]);
expect(getBorder(model, "B1")).toEqual({ right: b });
expect(getBorder(model, "C1")).toEqual({ top: b, left: b, right: b, bottom: b });
expect(getBorder(model, "D1")).toEqual({ left: b });
expect(getBorder(model, "C2")).toEqual({ top: b });

deleteColumns(model, ["A", "B"]);
expect(getBorder(model, "A1")).toEqual({ top: b, left: b, right: b, bottom: b });
expect(getBorder(model, "B1")).toEqual({ left: b });
expect(getBorder(model, "A2")).toEqual({ top: b });
});

test("Borders are correctly duplicated on sheet dup", () => {
setZoneBorders(model, { position: "external" }, ["B2"]);
const sheetId = model.getters.getActiveSheetId();
Expand Down

0 comments on commit d28b9b4

Please sign in to comment.