Skip to content

Commit 4e27d00

Browse files
committed
[FIX] borders: preserve side borders when combining External and All
When applying an External border on a range and then applying All borders on an inner/overlapping range, the side borders (left/right) of the inner cells were lost after re-opening the spreadsheet. The exported border data was correct, but the BordersPlugin recomputation logic in addBorder incorrectly cleared adjacent borders on import/update. The adjacency check (adjacent(existingBorder.zone, zone)) reported the side of the existing zone, but we were deciding whether to clear it based on the same side key in the new border, instead of the opposite side (shared edge) on the new zone. As a result, importing the combination of: C2:C4 with All borders B2:B4 with left/top/bottom D2:D4 with right/top/bottom ended up clearing the left and right borders of C2:C4. This commit adjusts the adjacent clearing logic to: Map the adjacent side of the existing zone to the opposite side on the new zone (i.e. left → right, right → left, top → bottom, bottom → top). Only clear the existing side if the corresponding opposite side is actually being written on the new border. Task: 5270171
1 parent d59fcbf commit 4e27d00

File tree

2 files changed

+74
-7
lines changed

2 files changed

+74
-7
lines changed

src/plugins/core/borders.ts

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -342,22 +342,35 @@ export class BordersPlugin extends CorePlugin<BordersPluginState> implements Bor
342342
) {
343343
const borders: ZoneBorder[] = [];
344344
const plannedBorder = newBorder ? { zone, style: newBorder } : undefined;
345-
const sideToClear = {
346-
left: force || !!newBorder?.left,
347-
right: force || !!newBorder?.right,
348-
top: force || !!newBorder?.top,
349-
bottom: force || !!newBorder?.bottom,
345+
346+
// For each side, decide if we must clear the border on the *adjacent*
347+
// existing cell when we draw on the opposite side of the new zone.
348+
//
349+
// Example:
350+
// - newBorder.right is set → we draw border on the RIGHT side of `zone`
351+
// - the cell on the right may already have a LEFT border on that edge
352+
// In that case we clear that LEFT border, so only the new RIGHT border
353+
// remains on the shared edge.
354+
//
355+
// existingBorderSideToClear[side] = true means we should clear the border on that
356+
// side of the existing adjacent zone before adding the new border.
357+
const existingBorderSideToClear = {
358+
left: force || !!newBorder?.right,
359+
right: force || !!newBorder?.left,
360+
top: force || !!newBorder?.bottom,
361+
bottom: force || !!newBorder?.top,
350362
};
351363
let editingZone: Zone[] = [zone];
352364
for (const existingBorder of this.borders[sheetId] ?? []) {
353365
const inter = intersection(existingBorder.zone, zone);
354366
if (!inter) {
355-
// Clear adjacent borders on which you write
367+
// Check if the existing border is adjacent to the new zone
356368
const adjacentEdge = adjacent(existingBorder.zone, zone);
357-
if (adjacentEdge && sideToClear[adjacentEdge.position]) {
369+
if (adjacentEdge && existingBorderSideToClear[adjacentEdge.position]) {
358370
for (const newZone of splitIfAdjacent(existingBorder.zone, zone)) {
359371
const border = this.computeBorderFromZone(newZone, existingBorder);
360372
const adjacentEdge = adjacent(newZone, zone);
373+
// Clear the existing border on the side that touches the new zone
361374
switch (adjacentEdge?.position) {
362375
case "left":
363376
border.style.left = undefined;

tests/borders/border_plugin.test.ts

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,60 @@ describe("borders", () => {
147147
});
148148
});
149149

150+
test("Preserves side borders when combining external and all via command", () => {
151+
const model = new Model();
152+
const defaultBorder = DEFAULT_BORDER_DESC;
153+
154+
setZoneBorders(model, { position: "all" }, ["C3"]);
155+
setBordersOnTarget(model, ["C2"], {
156+
top: defaultBorder,
157+
right: defaultBorder,
158+
left: defaultBorder,
159+
});
160+
setBordersOnTarget(model, ["C4"], {
161+
bottom: defaultBorder,
162+
left: defaultBorder,
163+
right: defaultBorder,
164+
});
165+
setBordersOnTarget(model, ["B3"], {
166+
top: defaultBorder,
167+
bottom: defaultBorder,
168+
left: defaultBorder,
169+
});
170+
setBordersOnTarget(model, ["D3"], {
171+
top: defaultBorder,
172+
bottom: defaultBorder,
173+
right: defaultBorder,
174+
});
175+
176+
expect(getBorder(model, "C3")).toEqual({
177+
top: defaultBorder,
178+
bottom: defaultBorder,
179+
left: defaultBorder,
180+
right: defaultBorder,
181+
});
182+
expect(getBorder(model, "C2")).toEqual({
183+
top: defaultBorder,
184+
left: defaultBorder,
185+
right: defaultBorder,
186+
});
187+
expect(getBorder(model, "C4")).toEqual({
188+
bottom: defaultBorder,
189+
left: defaultBorder,
190+
right: defaultBorder,
191+
});
192+
expect(getBorder(model, "B3")).toEqual({
193+
top: defaultBorder,
194+
left: defaultBorder,
195+
bottom: defaultBorder,
196+
});
197+
expect(getBorder(model, "D3")).toEqual({
198+
top: defaultBorder,
199+
bottom: defaultBorder,
200+
right: defaultBorder,
201+
});
202+
});
203+
150204
test("can set all borders in a zone", () => {
151205
const model = new Model();
152206

0 commit comments

Comments
 (0)