From 80de593eeac3596a84a8f6402fd60b72f0a6c1db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9mi=20Rahir=20=28rar=29?= Date: Thu, 9 Jan 2025 13:47:53 +0100 Subject: [PATCH] [REF] Border: Change border internal mapping some of the current behaviour is "meh" due to the fact taht the borders are shared between adjacent positions and not defined per position. see task Task: 4458245 --- src/plugins/core/borders.ts | 200 +++++++++--------- src/stores/grid_renderer_store.ts | 4 +- src/types/misc.ts | 2 - tests/borders/border_plugin.test.ts | 112 ++++------ tests/clipboard/clipboard_plugin.test.ts | 4 +- tests/sheet/sheet_manipulation_plugin.test.ts | 107 +++++----- .../__snapshots__/xlsx_export.test.ts.snap | 178 +++------------- 7 files changed, 217 insertions(+), 390 deletions(-) diff --git a/src/plugins/core/borders.ts b/src/plugins/core/borders.ts index 8dedfa46ef..fde8d6d85b 100644 --- a/src/plugins/core/borders.ts +++ b/src/plugins/core/borders.ts @@ -14,7 +14,6 @@ import { AddColumnsRowsCommand, Border, BorderDescr, - BorderDescription, BorderPosition, CellPosition, Color, @@ -30,7 +29,7 @@ import { import { CorePlugin } from "../core_plugin"; interface BordersPluginState { - readonly borders: Record; + readonly borders: Record; } /** * Formatting plugin. @@ -105,53 +104,15 @@ export class BordersPlugin extends CorePlugin implements Bor const elements = [...cmd.elements].sort((a, b) => b - a); for (const group of groupConsecutive(elements)) { if (cmd.dimension === "COL") { - if (group[0] >= this.getters.getNumberCols(cmd.sheetId)) { - for (let row: HeaderIndex = 0; row < this.getters.getNumberRows(cmd.sheetId); row++) { - this.history.update( - "borders", - cmd.sheetId, - group[0] + 1, - row, - "vertical", - undefined - ); - } - } - if (group[group.length - 1] === 0) { - for (let row: HeaderIndex = 0; row < this.getters.getNumberRows(cmd.sheetId); row++) { - this.history.update("borders", cmd.sheetId, 0, row, "vertical", undefined); - } - } - const zone = this.getters.getColsZone( - cmd.sheetId, - group[group.length - 1] + 1, - group[0] - ); + const start = Math.min(group[0], group[group.length - 1]); + const end = Math.max(group[0], group[group.length - 1]); + const zone = this.getters.getColsZone(cmd.sheetId, start, end); this.clearInsideBorders(cmd.sheetId, [zone]); this.shiftBordersHorizontally(cmd.sheetId, group[0] + 1, -group.length); } else { - if (group[0] >= this.getters.getNumberRows(cmd.sheetId)) { - for (let col = 0; col < this.getters.getNumberCols(cmd.sheetId); col++) { - this.history.update( - "borders", - cmd.sheetId, - col, - group[0] + 1, - "horizontal", - undefined - ); - } - } - if (group[group.length - 1] === 0) { - for (let col = 0; col < this.getters.getNumberCols(cmd.sheetId); col++) { - this.history.update("borders", cmd.sheetId, col, 0, "horizontal", undefined); - } - } - const zone = this.getters.getRowsZone( - cmd.sheetId, - group[group.length - 1] + 1, - group[0] - ); + const start = Math.min(group[0], group[group.length - 1]); + const end = Math.max(group[0], group[group.length - 1]); + const zone = this.getters.getRowsZone(cmd.sheetId, start, end); this.clearInsideBorders(cmd.sheetId, [zone]); this.shiftBordersVertically(cmd.sheetId, group[0] + 1, -group.length); } @@ -182,7 +143,7 @@ export class BordersPlugin extends CorePlugin implements Bor colLeftOfInsertion = cmd.base - 1; colRightOfInsertion = cmd.base + cmd.quantity; } else { - this.shiftBordersHorizontally(cmd.sheetId, cmd.base + 1, cmd.quantity, { + this.shiftBordersHorizontally(cmd.sheetId, cmd.base, cmd.quantity, { moveFirstLeftBorder: false, }); colLeftOfInsertion = cmd.base; @@ -206,7 +167,7 @@ export class BordersPlugin extends CorePlugin implements Bor rowAboveInsertion = cmd.base - 1; rowBelowInsertion = cmd.base + cmd.quantity; } else { - this.shiftBordersVertically(cmd.sheetId, cmd.base + 1, cmd.quantity, { + this.shiftBordersVertically(cmd.sheetId, cmd.base, cmd.quantity, { moveFirstTopBorder: false, }); rowAboveInsertion = cmd.base; @@ -220,16 +181,8 @@ export class BordersPlugin extends CorePlugin implements Bor // --------------------------------------------------------------------------- getCellBorder({ sheetId, col, row }: CellPosition): Border | null { - const border = { - top: this.borders[sheetId]?.[col]?.[row]?.horizontal, - bottom: this.borders[sheetId]?.[col]?.[row + 1]?.horizontal, - left: this.borders[sheetId]?.[col]?.[row]?.vertical, - right: this.borders[sheetId]?.[col + 1]?.[row]?.vertical, - }; - if (!border.bottom && !border.left && !border.right && !border.top) { - return null; - } - return border; + const border = this.borders[sheetId]?.[col]?.[row]; + return border && Object.keys(border).length > 0 ? border : null; } getBordersColors(sheetId: UID): Color[] { @@ -238,11 +191,8 @@ export class BordersPlugin extends CorePlugin implements Bor if (sheetBorders) { for (const borders of sheetBorders.filter(isDefined)) { for (const cellBorder of borders) { - if (cellBorder?.horizontal) { - colors.push(cellBorder.horizontal.color); - } - if (cellBorder?.vertical) { - colors.push(cellBorder.vertical.color); + if (cellBorder) { + colors.push(...Object.values(cellBorder).map((b) => b?.color)); } } } @@ -358,7 +308,7 @@ export class BordersPlugin extends CorePlugin implements Bor const borders = this.borders[sheetId]; if (!borders) return; if (delta < 0) { - this.moveBordersOfColumn(sheetId, start, delta, "vertical", { + this.moveBordersOfColumn(sheetId, start, delta, { destructive: false, }); } @@ -367,9 +317,8 @@ export class BordersPlugin extends CorePlugin implements Bor .sort((a, b) => (delta < 0 ? a - b : b - a)) // start by the end when moving up .forEach((col) => { if ((col === start && moveFirstLeftBorder) || col !== start) { - this.moveBordersOfColumn(sheetId, col, delta, "vertical"); + this.moveBordersOfColumn(sheetId, col, delta); } - this.moveBordersOfColumn(sheetId, col, delta, "horizontal"); }); } @@ -388,7 +337,7 @@ export class BordersPlugin extends CorePlugin implements Bor const borders = this.borders[sheetId]; if (!borders) return; if (delta < 0) { - this.moveBordersOfRow(sheetId, start, delta, "horizontal", { + this.moveBordersOfRow(sheetId, start, delta, { destructive: false, }); } @@ -397,9 +346,8 @@ export class BordersPlugin extends CorePlugin implements Bor .sort((a, b) => (delta < 0 ? a - b : b - a)) // start by the end when moving up .forEach((row) => { if ((row === start && moveFirstTopBorder) || row !== start) { - this.moveBordersOfRow(sheetId, row, delta, "horizontal"); + this.moveBordersOfRow(sheetId, row, delta); } - this.moveBordersOfRow(sheetId, row, delta, "vertical"); }); } @@ -418,23 +366,21 @@ export class BordersPlugin extends CorePlugin implements Bor sheetId: UID, row: HeaderIndex, delta: number, - borderDirection: "vertical" | "horizontal", { destructive }: { destructive: boolean } = { destructive: true } ) { const borders = this.borders[sheetId]; if (!borders) return; this.getColumnsWithBorders(sheetId).forEach((col) => { - const targetBorder = borders[col]?.[row + delta]?.[borderDirection]; - const movedBorder = borders[col]?.[row]?.[borderDirection]; + const targetBorder = borders[col]?.[row + delta]; + const movedBorder = borders[col]?.[row]; this.history.update( "borders", sheetId, col, row + delta, - borderDirection, destructive ? movedBorder : movedBorder || targetBorder ); - this.history.update("borders", sheetId, col, row, borderDirection, undefined); + this.history.update("borders", sheetId, col, row, undefined); }); } @@ -453,23 +399,23 @@ export class BordersPlugin extends CorePlugin implements Bor sheetId: UID, col: HeaderIndex, delta: number, - borderDirection: "vertical" | "horizontal", { destructive }: { destructive: boolean } = { destructive: true } ) { const borders = this.borders[sheetId]; if (!borders) return; this.getRowsRange(sheetId).forEach((row) => { - const targetBorder = borders[col + delta]?.[row]?.[borderDirection]; - const movedBorder = borders[col]?.[row]?.[borderDirection]; + const targetBorder = borders[col + delta]?.[row]; + const movedBorder = borders[col]?.[row]; this.history.update( "borders", sheetId, col + delta, row, - borderDirection, destructive ? movedBorder : movedBorder || targetBorder ); - this.history.update("borders", sheetId, col, row, borderDirection, undefined); + if (destructive) { + this.history.update("borders", sheetId, col, row, undefined); + } }); } @@ -484,34 +430,62 @@ export class BordersPlugin extends CorePlugin implements Bor border?: Border, override = true ) { - if (override || !this.borders?.[sheetId]?.[col]?.[row]?.vertical) { - this.history.update("borders", sheetId, col, row, "vertical", border?.left); + if (override || !this.borders[sheetId]?.[col]?.[row]?.left) { + this.history.update("borders", sheetId, col, row, "left", border?.left); + if ( + border?.left && + !deepEquals(this.getCellBorder({ sheetId, col: col - 1, row })?.right, border?.left) + ) { + this.history.update("borders", sheetId, col - 1, row, "right", undefined); + } } - if (override || !this.borders?.[sheetId]?.[col]?.[row]?.horizontal) { - this.history.update("borders", sheetId, col, row, "horizontal", border?.top); + if (override || !this.borders[sheetId]?.[col]?.[row]?.top) { + this.history.update("borders", sheetId, col, row, "top", border?.top); + if ( + border?.top && + !deepEquals(this.getCellBorder({ sheetId, col, row: row - 1 })?.bottom, border?.top) + ) { + this.history.update("borders", sheetId, col, row - 1, "bottom", undefined); + } } - if (override || !this.borders?.[sheetId]?.[col + 1]?.[row]?.vertical) { - this.history.update("borders", sheetId, col + 1, row, "vertical", border?.right); + if (override || !this.borders[sheetId]?.[col]?.[row]?.right) { + this.history.update("borders", sheetId, col, row, "right", border?.right); + if ( + border?.right && + !deepEquals(this.getCellBorder({ sheetId, col: col + 1, row })?.left, border?.right) + ) { + this.history.update("borders", sheetId, col + 1, row, "left", undefined); + } } - if (override || !this.borders?.[sheetId]?.[col]?.[row + 1]?.horizontal) { - this.history.update("borders", sheetId, col, row + 1, "horizontal", border?.bottom); + if (override || !this.borders[sheetId]?.[col]?.[row]?.bottom) { + this.history.update("borders", sheetId, col, row, "bottom", border?.bottom); + if ( + border?.bottom && + !deepEquals(this.getCellBorder({ sheetId, col, row: row + 1 })?.top, border?.bottom) + ) { + this.history.update("borders", sheetId, col, row + 1, "top", undefined); + } } } /** * Remove the borders of a zone */ - private clearBorders(sheetId: UID, zones: Zone[]) { + private clearBorders(sheetId: UID, zones: Zone[], eraseBoundaries = false) { for (let zone of recomputeZones(zones)) { for (let row = zone.top; row <= zone.bottom; row++) { - this.history.update("borders", sheetId, zone.right + 1, row, "vertical", undefined); + if (eraseBoundaries) { + this.history.update("borders", sheetId, zone.left - 1, row, "right", undefined); + this.history.update("borders", sheetId, zone.right + 1, row, "left", undefined); + } for (let col = zone.left; col <= zone.right; col++) { this.history.update("borders", sheetId, col, row, undefined); + if (eraseBoundaries) { + this.history.update("borders", sheetId, col, zone.top - 1, "bottom", undefined); + this.history.update("borders", sheetId, col, zone.bottom + 1, "top", undefined); + } } } - for (let col = zone.left; col <= zone.right; col++) { - this.history.update("borders", sheetId, col, zone.bottom + 1, "horizontal", undefined); - } } } @@ -549,41 +523,57 @@ export class BordersPlugin extends CorePlugin implements Bor border: BorderDescr | undefined ) { if (position === "clear") { - return this.clearBorders(sheetId, zones); + return this.clearBorders(sheetId, zones, true); } for (let zone of recomputeZones(zones)) { - if (position === "h" || position === "hv" || position === "all") { - for (let row = zone.top + 1; row <= zone.bottom; row++) { + if (position === "all") { + for (let row = zone.top; row <= zone.bottom; row++) { for (let col = zone.left; col <= zone.right; col++) { - this.addBorder(sheetId, col, row, { top: border }); + this.addBorder(sheetId, col, row, { + top: border, + right: border, + bottom: border, + left: border, + }); + } + } + } + if (position === "h" || position === "hv") { + for (let col = zone.left; col <= zone.right; col++) { + this.addBorder(sheetId, col, zone.top, { bottom: border }); + for (let row = zone.top + 1; row < zone.bottom; row++) { + this.addBorder(sheetId, col, row, { top: border, bottom: border }); } + this.addBorder(sheetId, col, zone.bottom, { top: border }); } } - if (position === "v" || position === "hv" || position === "all") { + if (position === "v" || position === "hv") { for (let row = zone.top; row <= zone.bottom; row++) { - for (let col = zone.left + 1; col <= zone.right; col++) { - this.addBorder(sheetId, col, row, { left: border }); + this.addBorder(sheetId, zone.left, row, { right: border }); + for (let col = zone.left + 1; col < zone.right; col++) { + this.addBorder(sheetId, col, row, { left: border, right: border }); } + this.addBorder(sheetId, zone.right, row, { left: border }); } } - if (position === "left" || position === "all" || position === "external") { + if (position === "left" || position === "external") { for (let row = zone.top; row <= zone.bottom; row++) { this.addBorder(sheetId, zone.left, row, { left: border }); } } - if (position === "right" || position === "all" || position === "external") { + if (position === "right" || position === "external") { for (let row = zone.top; row <= zone.bottom; row++) { - this.addBorder(sheetId, zone.right + 1, row, { left: border }); + this.addBorder(sheetId, zone.right, row, { right: border }); } } - if (position === "top" || position === "all" || position === "external") { + if (position === "top" || position === "external") { for (let col = zone.left; col <= zone.right; col++) { this.addBorder(sheetId, col, zone.top, { top: border }); } } - if (position === "bottom" || position === "all" || position === "external") { + if (position === "bottom" || position === "external") { for (let col = zone.left; col <= zone.right; col++) { - this.addBorder(sheetId, col, zone.bottom + 1, { top: border }); + this.addBorder(sheetId, col, zone.bottom, { bottom: border }); } } } diff --git a/src/stores/grid_renderer_store.ts b/src/stores/grid_renderer_store.ts index 4766b277f9..dfd3310b3e 100644 --- a/src/stores/grid_renderer_store.ts +++ b/src/stores/grid_renderer_store.ts @@ -672,8 +672,8 @@ export class GridRenderer { nextColIndex = this.getters.getMerge(position)!.right; previousColIndex = col; } else { - nextColIndex = this.findNextEmptyCol(col, right, row); - previousColIndex = this.findPreviousEmptyCol(col, left, row); + nextColIndex = box.border?.right ? zone.right : this.findNextEmptyCol(col, right, row); + previousColIndex = box.border?.left ? zone.left : this.findPreviousEmptyCol(col, left, row); box.isOverflow = true; } diff --git a/src/types/misc.ts b/src/types/misc.ts index df4214c252..c8ba8f30bb 100644 --- a/src/types/misc.ts +++ b/src/types/misc.ts @@ -274,8 +274,6 @@ export type BorderPosition = | "bottom" | "clear"; -export type BorderDescription = { vertical?: BorderDescr; horizontal?: BorderDescr } | undefined; - export const enum DIRECTION { UP = "up", DOWN = "down", diff --git a/tests/borders/border_plugin.test.ts b/tests/borders/border_plugin.test.ts index 30dbede5bc..0fb48475b0 100644 --- a/tests/borders/border_plugin.test.ts +++ b/tests/borders/border_plugin.test.ts @@ -406,14 +406,12 @@ describe("Grid manipulation", () => { test("ADD_COLUMNS_ROWS with dimension col before with external borders", () => { setZoneBorders(model, { position: "external" }, ["B2"]); addColumns(model, "before", "B", 1); - expect(getBorder(model, "B2")).toEqual({ right: DEFAULT_BORDER_DESC }); expect(getBorder(model, "C2")).toEqual({ top: DEFAULT_BORDER_DESC, left: DEFAULT_BORDER_DESC, right: DEFAULT_BORDER_DESC, bottom: DEFAULT_BORDER_DESC, }); - expect(getBorder(model, "D2")).toEqual({ left: DEFAULT_BORDER_DESC }); }); test("move duplicated border when col is inserted before", () => { @@ -438,7 +436,6 @@ describe("Grid manipulation", () => { right: DEFAULT_BORDER_DESC, bottom: DEFAULT_BORDER_DESC, }); - expect(getBorder(model, "B2", secondSheetId)).toEqual({ right: DEFAULT_BORDER_DESC }); expect(getBorder(model, "C2", secondSheetId)).toEqual({ top: DEFAULT_BORDER_DESC, left: DEFAULT_BORDER_DESC, @@ -463,7 +460,6 @@ describe("Grid manipulation", () => { right: DEFAULT_BORDER_DESC, bottom: DEFAULT_BORDER_DESC, }); - expect(getBorder(model, "B2", secondSheetId)).toEqual({ bottom: DEFAULT_BORDER_DESC }); expect(getBorder(model, "B3", secondSheetId)).toEqual({ top: DEFAULT_BORDER_DESC, left: DEFAULT_BORDER_DESC, @@ -472,127 +468,69 @@ describe("Grid manipulation", () => { }); }); - test.skip("[ok] ADD_COLUMNS_ROWS with dimension col before with external borders in the column before", () => { - setZoneBorders(model, { position: "external" }, ["B2"]); - addColumns(model, "before", "C", 1); - expect(getBorder(model, "A2")).toEqual({ right: DEFAULT_BORDER_DESC }); - expect(getBorder(model, "B2")).toEqual({ - top: DEFAULT_BORDER_DESC, - left: DEFAULT_BORDER_DESC, - right: DEFAULT_BORDER_DESC, - bottom: DEFAULT_BORDER_DESC, - }); - expect(getBorder(model, "C2")).toEqual({ left: DEFAULT_BORDER_DESC }); - expect(getBorder(model, "D2")).toBeNull(); - }); - test("ADD_COLUMNS_ROWS with dimension col before with external borders in the column before", () => { setZoneBorders(model, { position: "external" }, ["B2"]); addColumns(model, "before", "C", 1); - expect(getBorder(model, "A2")).toEqual({ right: DEFAULT_BORDER_DESC }); expect(getBorder(model, "B2")).toEqual({ top: DEFAULT_BORDER_DESC, left: DEFAULT_BORDER_DESC, right: DEFAULT_BORDER_DESC, bottom: DEFAULT_BORDER_DESC, }); - // because of border continuity - expect(getBorder(model, "C2")).toEqual({ - left: DEFAULT_BORDER_DESC, - right: DEFAULT_BORDER_DESC, - }); - expect(getBorder(model, "D2")).toEqual({ left: DEFAULT_BORDER_DESC }); }); test("ADD_COLUMNS_ROWS with dimension col before with external borders in the column after", () => { setZoneBorders(model, { position: "external" }, ["B2"]); addColumns(model, "before", "A", 1); - expect(getBorder(model, "A2")).toBeNull(); - expect(getBorder(model, "B2")).toEqual({ right: DEFAULT_BORDER_DESC }); expect(getBorder(model, "C2")).toEqual({ top: DEFAULT_BORDER_DESC, left: DEFAULT_BORDER_DESC, right: DEFAULT_BORDER_DESC, bottom: DEFAULT_BORDER_DESC, }); - expect(getBorder(model, "D2")).toEqual({ left: DEFAULT_BORDER_DESC }); }); test("ADD_COLUMNS_ROWS with dimension row before with external borders", () => { setZoneBorders(model, { position: "external" }, ["B2"]); addRows(model, "before", 1, 1); - expect(getBorder(model, "B2")).toEqual({ bottom: DEFAULT_BORDER_DESC }); expect(getBorder(model, "B3")).toEqual({ top: DEFAULT_BORDER_DESC, left: DEFAULT_BORDER_DESC, right: DEFAULT_BORDER_DESC, bottom: DEFAULT_BORDER_DESC, }); - expect(getBorder(model, "C2")).toBeNull(); - expect(getBorder(model, "B4")).toEqual({ top: DEFAULT_BORDER_DESC }); - }); - - test.skip("[ok] ADD_COLUMNS_ROWS with dimension row before with external borders in the column before", () => { - setZoneBorders(model, { position: "external" }, ["B2"]); - addRows(model, "before", 2, 1); - expect(getBorder(model, "A2")).toEqual({ right: DEFAULT_BORDER_DESC }); - expect(getBorder(model, "B2")).toEqual({ - top: DEFAULT_BORDER_DESC, - left: DEFAULT_BORDER_DESC, - right: DEFAULT_BORDER_DESC, - bottom: DEFAULT_BORDER_DESC, - }); - expect(getBorder(model, "C2")).toEqual({ left: DEFAULT_BORDER_DESC }); - expect(getBorder(model, "B3")).toEqual({ top: DEFAULT_BORDER_DESC }); - expect(getBorder(model, "B4")).toBeNull(); }); test("ADD_COLUMNS_ROWS with dimension row before with external borders in the column before", () => { setZoneBorders(model, { position: "external" }, ["B2"]); addRows(model, "before", 2, 1); - expect(getBorder(model, "A2")).toEqual({ right: DEFAULT_BORDER_DESC }); expect(getBorder(model, "B2")).toEqual({ top: DEFAULT_BORDER_DESC, left: DEFAULT_BORDER_DESC, right: DEFAULT_BORDER_DESC, bottom: DEFAULT_BORDER_DESC, }); - expect(getBorder(model, "C2")).toEqual({ left: DEFAULT_BORDER_DESC }); - // because of border continuity - expect(getBorder(model, "B3")).toEqual({ - top: DEFAULT_BORDER_DESC, - bottom: DEFAULT_BORDER_DESC, - }); - expect(getBorder(model, "B4")).toEqual({ top: DEFAULT_BORDER_DESC }); }); test("ADD_COLUMNS_ROWS with dimension row before with external borders in the column after", () => { setZoneBorders(model, { position: "external" }, ["B2"]); addRows(model, "before", 0, 1); - expect(getBorder(model, "B2")).toEqual({ bottom: DEFAULT_BORDER_DESC }); expect(getBorder(model, "B3")).toEqual({ top: DEFAULT_BORDER_DESC, left: DEFAULT_BORDER_DESC, right: DEFAULT_BORDER_DESC, bottom: DEFAULT_BORDER_DESC, }); - expect(getBorder(model, "C2")).toBeNull(); - 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", () => { @@ -612,13 +550,11 @@ describe("Grid manipulation", () => { test("Delete cell correctly move borders on shift up", () => { setZoneBorders(model, { position: "external" }, ["C3:D4"]); deleteCells(model, "C1", "up"); - expect(getBorder(model, "C1")).toEqual({ bottom: DEFAULT_BORDER_DESC }); expect(getBorder(model, "C2")).toEqual({ top: DEFAULT_BORDER_DESC, left: DEFAULT_BORDER_DESC }); expect(getBorder(model, "C3")).toEqual({ bottom: DEFAULT_BORDER_DESC, left: DEFAULT_BORDER_DESC, }); - expect(getBorder(model, "D2")).toEqual({ bottom: DEFAULT_BORDER_DESC }); expect(getBorder(model, "D3")).toEqual({ top: DEFAULT_BORDER_DESC, right: DEFAULT_BORDER_DESC, @@ -632,26 +568,22 @@ describe("Grid manipulation", () => { test("Delete a cell correctly move all the borders on shift up", () => { setZoneBorders(model, { position: "external" }, ["C3"]); deleteCells(model, "C1", "up"); - expect(getBorder(model, "C1")).toEqual({ bottom: DEFAULT_BORDER_DESC }); expect(getBorder(model, "C2")).toEqual({ bottom: DEFAULT_BORDER_DESC, top: DEFAULT_BORDER_DESC, left: DEFAULT_BORDER_DESC, right: DEFAULT_BORDER_DESC, }); - expect(getBorder(model, "C3")).toEqual({ top: DEFAULT_BORDER_DESC }); }); test("Delete cell correctly move borders on shift left", () => { setZoneBorders(model, { position: "external" }, ["C3:D4"]); deleteCells(model, "A3", "left"); - expect(getBorder(model, "A3")).toEqual({ right: DEFAULT_BORDER_DESC }); expect(getBorder(model, "B3")).toEqual({ left: DEFAULT_BORDER_DESC, top: DEFAULT_BORDER_DESC }); expect(getBorder(model, "C3")).toEqual({ right: DEFAULT_BORDER_DESC, top: DEFAULT_BORDER_DESC, }); - expect(getBorder(model, "B4")).toEqual({ right: DEFAULT_BORDER_DESC }); expect(getBorder(model, "C4")).toEqual({ left: DEFAULT_BORDER_DESC, bottom: DEFAULT_BORDER_DESC, @@ -665,14 +597,12 @@ describe("Grid manipulation", () => { test("Delete a cell correctly move all the borders on shift left", () => { setZoneBorders(model, { position: "external" }, ["C3"]); deleteCells(model, "A3", "left"); - expect(getBorder(model, "A3")).toEqual({ right: DEFAULT_BORDER_DESC }); expect(getBorder(model, "B3")).toEqual({ bottom: DEFAULT_BORDER_DESC, top: DEFAULT_BORDER_DESC, left: DEFAULT_BORDER_DESC, right: DEFAULT_BORDER_DESC, }); - expect(getBorder(model, "C3")).toEqual({ left: DEFAULT_BORDER_DESC }); }); test("Remove multiple rows before borders at the bottom of the sheet starting from the first column", () => { @@ -744,6 +674,48 @@ describe("Grid manipulation", () => { deleteRows(model, [98, 99]); expect(model.exportData().borders).toEqual({}); }); + + test("Adding a border on a cell removes it on the adjacent cells if it differs", () => { + const model = new Model(); + const b = DEFAULT_BORDER_DESC; + setZoneBorders(model, { position: "bottom", color: "red", style: "dashed" }, ["B2"]); + setZoneBorders(model, { position: "right", color: "red", style: "dashed" }, ["A3"]); + setZoneBorders(model, { position: "left", color: "red", style: "dashed" }, ["C3"]); + setZoneBorders(model, { position: "top" }, ["D2"]); + setZoneBorders(model, { position: "external" }, ["B3"]); + expect(getBorder(model, "B3")).toEqual({ top: b, bottom: b, left: b, right: b }); + // deleted as the borders are different + expect(getBorder(model, "B2")).toBeNull(); + expect(getBorder(model, "A3")).toBeNull(); + expect(getBorder(model, "C3")).toBeNull(); + // untouched as the border are the same + expect(getBorder(model, "D2")).toEqual({ top: DEFAULT_BORDER_DESC }); + }); + + test("Setting a 'clear' border on a cell removes the adjacent border cell", () => { + const model = new Model(); + const b = DEFAULT_BORDER_DESC; + setZoneBorders(model, { position: "all" }, ["A1:C3"]); + setZoneBorders(model, { position: "clear" }, ["B2"]); + expect(getBorder(model, "A1")).toEqual({ top: b, bottom: b, left: b, right: b }); + expect(getBorder(model, "A2")).toEqual({ top: b, bottom: b, left: b }); + expect(getBorder(model, "A3")).toEqual({ top: b, bottom: b, left: b, right: b }); + expect(getBorder(model, "B1")).toEqual({ top: b, left: b, right: b }); + expect(getBorder(model, "B2")).toBeNull(); + expect(getBorder(model, "B3")).toEqual({ bottom: b, left: b, right: b }); + expect(getBorder(model, "C1")).toEqual({ top: b, bottom: b, left: b, right: b }); + expect(getBorder(model, "C2")).toEqual({ top: b, bottom: b, right: b }); + expect(getBorder(model, "C3")).toEqual({ top: b, bottom: b, left: b, right: b }); + }); + + // need pr of florian ? + test("no lingering border when deleting a cell with a border", () => { + const sheetId = model.getters.getActiveSheetId(); + setZoneBorders(model, { position: "top", color: "#123456" }, ["C3"]); + expect(model.getters.getBordersColors(sheetId)).toEqual(["#123456"]); + deleteRows(model, [2]); + expect(model.getters.getBordersColors(sheetId)).toEqual([]); + }); }); test("Cells that have undefined borders don't override borders of neighboring cells at import", () => { diff --git a/tests/clipboard/clipboard_plugin.test.ts b/tests/clipboard/clipboard_plugin.test.ts index 199beefb95..e7785a357b 100644 --- a/tests/clipboard/clipboard_plugin.test.ts +++ b/tests/clipboard/clipboard_plugin.test.ts @@ -1250,10 +1250,8 @@ describe("clipboard", () => { const model = new Model(); setCellContent(model, "B2", "b2"); setCellContent(model, "C3", "c3"); - selectCell(model, "C3"); - setZoneBorders(model, { position: "bottom" }); + setZoneBorders(model, { position: "bottom" }, ["C3"]); expect(getBorder(model, "C3")).toEqual({ bottom: DEFAULT_BORDER_DESC }); - expect(getBorder(model, "C4")).toEqual({ top: DEFAULT_BORDER_DESC }); copy(model, "B2"); paste(model, "C3", "asValue"); diff --git a/tests/sheet/sheet_manipulation_plugin.test.ts b/tests/sheet/sheet_manipulation_plugin.test.ts index 9bf6897032..a2b04d7b73 100644 --- a/tests/sheet/sheet_manipulation_plugin.test.ts +++ b/tests/sheet/sheet_manipulation_plugin.test.ts @@ -329,19 +329,19 @@ describe("Columns", () => { ], borders: { 1: { top: s } }, }); - expect(getBorder(model, "A1")).toEqual({ bottom: s }); + expect(getBorder(model, "A1")).toBeNull(); expect(getBorder(model, "A2")).toEqual({ top: s }); addColumns(model, "before", "A", 1); expect(getBorder(model, "A1")).toBeNull(); expect(getBorder(model, "A2")).toBeNull(); - expect(getBorder(model, "B1")).toEqual({ bottom: s }); + expect(getBorder(model, "B1")).toBeNull(); expect(getBorder(model, "B2")).toEqual({ top: s }); expect(getBorder(model, "C1")).toBeNull(); expect(getBorder(model, "C2")).toBeNull(); addColumns(model, "after", "B", 1); expect(getBorder(model, "A1")).toBeNull(); expect(getBorder(model, "A2")).toBeNull(); - expect(getBorder(model, "B1")).toEqual({ bottom: s }); + expect(getBorder(model, "B1")).toBeNull(); expect(getBorder(model, "B2")).toEqual({ top: s }); expect(getBorder(model, "C1")).toBeNull(); expect(getBorder(model, "C2")).toBeNull(); @@ -362,32 +362,32 @@ describe("Columns", () => { ], borders: { 1: { top: s }, 2: { left: s } }, }); - expect(getBorder(model, "A1")).toEqual({ bottom: s }); + expect(getBorder(model, "A1")).toBeNull(); expect(getBorder(model, "A2")).toEqual({ top: s }); - expect(getBorder(model, "B1")).toEqual({ bottom: s }); + expect(getBorder(model, "B1")).toBeNull(); expect(getBorder(model, "B2")).toEqual({ top: s }); - expect(getBorder(model, "A4")).toEqual({ left: s, right: s }); - expect(getBorder(model, "B4")).toEqual({ left: s, right: s }); + expect(getBorder(model, "A4")).toEqual({ left: s }); + expect(getBorder(model, "B4")).toEqual({ left: s }); expect(getBorder(model, "C4")).toEqual({ left: s }); addColumns(model, "before", "A", 1); expect(getBorder(model, "A1")).toBeNull(); expect(getBorder(model, "A2")).toBeNull(); - expect(getBorder(model, "B1")).toEqual({ bottom: s }); + expect(getBorder(model, "B1")).toBeNull(); expect(getBorder(model, "B2")).toEqual({ top: s }); - expect(getBorder(model, "C1")).toEqual({ bottom: s }); + expect(getBorder(model, "C1")).toBeNull(); expect(getBorder(model, "C2")).toEqual({ top: s }); - expect(getBorder(model, "A4")).toEqual({ right: s }); - expect(getBorder(model, "B4")).toEqual({ left: s, right: s }); - expect(getBorder(model, "C4")).toEqual({ left: s, right: s }); - expect(getBorder(model, "C4")).toEqual({ left: s, right: s }); + expect(getBorder(model, "A4")).toBeNull(); + expect(getBorder(model, "B4")).toEqual({ left: s }); + expect(getBorder(model, "C4")).toEqual({ left: s }); + expect(getBorder(model, "D4")).toEqual({ left: s }); addColumns(model, "after", "B", 1); expect(getBorder(model, "A1")).toBeNull(); expect(getBorder(model, "A2")).toBeNull(); - expect(getBorder(model, "B1")).toEqual({ bottom: s }); + expect(getBorder(model, "B1")).toBeNull(); expect(getBorder(model, "B2")).toEqual({ top: s }); - expect(getBorder(model, "C1")).toEqual({ bottom: s }); + expect(getBorder(model, "C1")).toBeNull(); expect(getBorder(model, "C2")).toEqual({ top: s }); - expect(getBorder(model, "D1")).toEqual({ bottom: s }); + expect(getBorder(model, "D1")).toBeNull(); expect(getBorder(model, "D2")).toEqual({ top: s }); }); @@ -401,7 +401,6 @@ describe("Columns", () => { left: DEFAULT_BORDER_DESC, right: DEFAULT_BORDER_DESC, }); - expect(getBorder(model, "C2")).toEqual({ left: DEFAULT_BORDER_DESC }); }); test("insert column before cell with external border", () => { @@ -414,7 +413,6 @@ describe("Columns", () => { left: DEFAULT_BORDER_DESC, right: DEFAULT_BORDER_DESC, }); - expect(getBorder(model, "B2")).toEqual({ right: DEFAULT_BORDER_DESC }); }); test("delete column after cell with external border", () => { @@ -478,24 +476,24 @@ describe("Columns", () => { C1: { style }, C3: { style }, }); - expect(getBorder(model, "A2")).toEqual({ top: s, bottom: s }); + expect(getBorder(model, "A2")).toEqual({ top: s }); expect(getBorder(model, "A3")).toEqual({ top: s }); expect(getBorder(model, "B4")).toEqual({ top: s }); - expect(getBorder(model, "C2")).toEqual({ top: s, bottom: s }); + expect(getBorder(model, "C2")).toEqual({ top: s }); expect(getBorder(model, "C3")).toEqual({ top: s }); }); test("On addition", () => { const s = { style: "thin", color: "#000000" }; const style = { textColor: "#fe0000" }; - expect(getBorder(model, "A1")).toEqual({ bottom: s }); - expect(getBorder(model, "A2")).toEqual({ top: s, bottom: s }); + expect(getBorder(model, "A1")).toBeNull(); + expect(getBorder(model, "A2")).toEqual({ top: s }); expect(getBorder(model, "A3")).toEqual({ top: s }); - expect(getBorder(model, "B1")).toEqual({ bottom: s }); - expect(getBorder(model, "B2")).toEqual({ top: s, bottom: s }); - expect(getBorder(model, "B3")).toEqual({ top: s, bottom: s }); + expect(getBorder(model, "B1")).toBeNull(); + expect(getBorder(model, "B2")).toEqual({ top: s }); + expect(getBorder(model, "B3")).toEqual({ top: s }); expect(getBorder(model, "B4")).toEqual({ top: s }); - expect(getBorder(model, "D1")).toEqual({ bottom: s }); - expect(getBorder(model, "D2")).toEqual({ top: s, bottom: s }); + expect(getBorder(model, "D1")).toBeNull(); + expect(getBorder(model, "D2")).toEqual({ top: s }); expect(getBorder(model, "D3")).toEqual({ top: s }); addColumns(model, "before", "B", 1); addColumns(model, "after", "C", 2); @@ -510,27 +508,27 @@ describe("Columns", () => { C4: { style }, E1: { style }, }); - expect(getBorder(model, "A2")).toEqual({ top: s, bottom: s }); + expect(getBorder(model, "A2")).toEqual({ top: s }); expect(getBorder(model, "A3")).toEqual({ top: s }); - expect(getBorder(model, "B2")).toEqual({ top: s, bottom: s }); + expect(getBorder(model, "B2")).toEqual({ top: s }); expect(getBorder(model, "B3")).toEqual({ top: s }); expect(getBorder(model, "B4")).toBeNull(); - expect(getBorder(model, "C2")).toEqual({ top: s, bottom: s }); - expect(getBorder(model, "C3")).toEqual({ top: s, bottom: s }); + expect(getBorder(model, "C2")).toEqual({ top: s }); + expect(getBorder(model, "C3")).toEqual({ top: s }); expect(getBorder(model, "C4")).toEqual({ top: s }); expect(getBorder(model, "D1")).toBeNull(); expect(getBorder(model, "D2")).toBeNull(); - expect(getBorder(model, "D3")).toEqual({ bottom: s }); + expect(getBorder(model, "D3")).toBeNull(); expect(getBorder(model, "D4")).toEqual({ top: s }); expect(getBorder(model, "E1")).toBeNull(); expect(getBorder(model, "E2")).toBeNull(); - expect(getBorder(model, "E3")).toEqual({ bottom: s }); + expect(getBorder(model, "E3")).toBeNull(); expect(getBorder(model, "E4")).toEqual({ top: s }); expect(getBorder(model, "F2")).toBeNull(); - expect(getBorder(model, "F3")).toEqual({ bottom: s }); + expect(getBorder(model, "F3")).toBeNull(); expect(getBorder(model, "F4")).toEqual({ top: s }); - expect(getBorder(model, "G1")).toEqual({ bottom: s }); - expect(getBorder(model, "G2")).toEqual({ top: s, bottom: s }); + expect(getBorder(model, "G1")).toBeNull(); + expect(getBorder(model, "G2")).toEqual({ top: s }); expect(getBorder(model, "G3")).toEqual({ top: s }); expect(Object.values(getMerges(model))[0]).toMatchObject({ left: 2, @@ -1086,31 +1084,31 @@ describe("Rows", () => { expect(Object.values(model.getters.getCells(sheetId))).toHaveLength(5); // 4 NumberCells +1 emptyCell with no merge, but with style expect(getCell(model, "A1")).toMatchObject({ style }); expect(getCell(model, "A3")).toMatchObject({ style }); - expect(getBorder(model, "B1")).toEqual({ top: s, bottom: s }); - expect(getBorder(model, "B2")).toEqual({ top: s, bottom: s }); + expect(getBorder(model, "B1")).toEqual({ top: s }); + expect(getBorder(model, "B2")).toBeNull(); expect(getBorder(model, "B3")).toEqual({ top: s }); expect(getCell(model, "C1")).toMatchObject({ style }); expect(getCell(model, "C3")).toMatchObject({ style }); expect(getCell(model, "D2")).toMatchObject({ style }); - expect(getBorder(model, "C1")).toEqual({ top: s, bottom: s }); - expect(getBorder(model, "C2")).toEqual({ top: s, bottom: s }); + expect(getBorder(model, "C1")).toEqual({ top: s }); + expect(getBorder(model, "C2")).toBeNull(); expect(getBorder(model, "C3")).toEqual({ top: s }); - expect(getBorder(model, "D2")).toEqual({ top: s }); + expect(getBorder(model, "D2")).toBeNull(); }); test("On addition", () => { const s = { style: "thin", color: "#000000" }; addRows(model, "before", 1, 1); const style = { textColor: "#fe0000" }; - expect(getBorder(model, "B1")).toEqual({ top: s, bottom: s }); - expect(getBorder(model, "B2")).toEqual({ top: s, bottom: s }); + expect(getBorder(model, "B1")).toEqual({ top: s }); + expect(getBorder(model, "B2")).toEqual({ top: s }); expect(getBorder(model, "B3")).toEqual({ top: s }); - expect(getBorder(model, "B4")).toEqual({ bottom: s }); + expect(getBorder(model, "B4")).toBeNull(); expect(getBorder(model, "B5")).toEqual({ top: s }); - expect(getBorder(model, "C1")).toEqual({ top: s, bottom: s }); - expect(getBorder(model, "C2")).toEqual({ top: s, bottom: s }); + expect(getBorder(model, "C1")).toEqual({ top: s }); + expect(getBorder(model, "C2")).toEqual({ top: s }); expect(getBorder(model, "C3")).toEqual({ top: s }); - expect(getBorder(model, "C4")).toEqual({ bottom: s }); + expect(getBorder(model, "C4")).toBeNull(); expect(getBorder(model, "C5")).toEqual({ top: s }); addRows(model, "after", 2, 2); expect(getCellsObject(model, "sheet1")).toMatchObject({ @@ -1124,19 +1122,19 @@ describe("Rows", () => { D3: { style }, A5: { style }, }); - expect(getBorder(model, "B1")).toEqual({ top: s, bottom: s }); - expect(getBorder(model, "B2")).toEqual({ top: s, bottom: s }); + expect(getBorder(model, "B1")).toEqual({ top: s }); + expect(getBorder(model, "B2")).toEqual({ top: s }); expect(getBorder(model, "B3")).toEqual({ top: s }); expect(getBorder(model, "B4")).toBeNull(); expect(getBorder(model, "B5")).toBeNull(); - expect(getBorder(model, "B6")).toEqual({ bottom: s }); + expect(getBorder(model, "B6")).toBeNull(); expect(getBorder(model, "B7")).toEqual({ top: s }); - expect(getBorder(model, "C1")).toEqual({ top: s, bottom: s }); - expect(getBorder(model, "C2")).toEqual({ top: s, bottom: s }); + expect(getBorder(model, "C1")).toEqual({ top: s }); + expect(getBorder(model, "C2")).toEqual({ top: s }); expect(getBorder(model, "C3")).toEqual({ top: s }); expect(getBorder(model, "C4")).toBeNull(); expect(getBorder(model, "C5")).toBeNull(); - expect(getBorder(model, "C6")).toEqual({ bottom: s }); + expect(getBorder(model, "C6")).toBeNull(); expect(getBorder(model, "C7")).toEqual({ top: s }); expect(Object.values(getMerges(model))[0]).toMatchObject({ top: 2, @@ -1150,7 +1148,6 @@ describe("Rows", () => { setZoneBorders(model, { position: "external" }, ["B2"]); addRows(model, "after", 1, 1); expect(getBorder(model, "B2")).toEqual({ top: s, bottom: s, left: s, right: s }); - expect(getBorder(model, "B3")).toEqual({ top: s }); }); test("insert row before cell with external border", () => { @@ -1158,7 +1155,7 @@ describe("Rows", () => { const s = DEFAULT_BORDER_DESC; setZoneBorders(model, { position: "external" }, ["B2"]); addRows(model, "before", 1, 1); - expect(getBorder(model, "B2")).toEqual({ bottom: s }); + expect(getBorder(model, "B2")).toBeNull(); expect(getBorder(model, "B3")).toEqual({ top: s, bottom: s, left: s, right: s }); }); diff --git a/tests/xlsx/__snapshots__/xlsx_export.test.ts.snap b/tests/xlsx/__snapshots__/xlsx_export.test.ts.snap index 924364d40b..5eb3493d3b 100644 --- a/tests/xlsx/__snapshots__/xlsx_export.test.ts.snap +++ b/tests/xlsx/__snapshots__/xlsx_export.test.ts.snap @@ -23607,64 +23607,46 @@ exports[`Test XLSX export Generic sheets (style, hidden, size, cf) Simple model 2 - - - + 3 - - - - - + - + 4 - + 5 - - - - - + - + 6 - - - - - + - + 7 - - - - - + @@ -23673,21 +23655,15 @@ exports[`Test XLSX export Generic sheets (style, hidden, size, cf) Simple model 8 - - - - - + - + 9 - - @@ -23695,11 +23671,7 @@ exports[`Test XLSX export Generic sheets (style, hidden, size, cf) Simple model 10 - - - - - + @@ -23708,8 +23680,6 @@ exports[`Test XLSX export Generic sheets (style, hidden, size, cf) Simple model 11 - - @@ -23718,34 +23688,22 @@ exports[`Test XLSX export Generic sheets (style, hidden, size, cf) Simple model - - - - - - - - - + 13 - + 14 - - - + 15 - - @@ -23753,25 +23711,23 @@ exports[`Test XLSX export Generic sheets (style, hidden, size, cf) Simple model 16 - - - + 0.43 - + 10 - + 10.123 @@ -23999,7 +23955,7 @@ exports[`Test XLSX export Generic sheets (style, hidden, size, cf) Simple model - + @@ -24012,19 +23968,6 @@ exports[`Test XLSX export Generic sheets (style, hidden, size, cf) Simple model - - - - - - - - - - - - - @@ -24038,19 +23981,6 @@ exports[`Test XLSX export Generic sheets (style, hidden, size, cf) Simple model - - - - - - - - - - - - - @@ -24080,19 +24010,6 @@ exports[`Test XLSX export Generic sheets (style, hidden, size, cf) Simple model - - - - - - - - - - - - - @@ -24122,19 +24039,6 @@ exports[`Test XLSX export Generic sheets (style, hidden, size, cf) Simple model - - - - - - - - - - - - - @@ -24151,45 +24055,16 @@ exports[`Test XLSX export Generic sheets (style, hidden, size, cf) Simple model - - - - - - - - - - - - - - - - - - - - - - - - - - - + - - - - + - + @@ -24199,13 +24074,10 @@ exports[`Test XLSX export Generic sheets (style, hidden, size, cf) Simple model - - - - - - + + +