Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 20 additions & 7 deletions src/plugins/core/borders.ts
Original file line number Diff line number Diff line change
Expand Up @@ -342,22 +342,35 @@ export class BordersPlugin extends CorePlugin<BordersPluginState> implements Bor
) {
const borders: ZoneBorder[] = [];
const plannedBorder = newBorder ? { zone, style: newBorder } : undefined;
const sideToClear = {
left: force || !!newBorder?.left,
right: force || !!newBorder?.right,
top: force || !!newBorder?.top,
bottom: force || !!newBorder?.bottom,

// For each side, decide if we must clear the border on the *adjacent*
// existing cell when we draw on the opposite side of the new zone.
//
// Example:
// - newBorder.right is set → we draw border on the RIGHT side of `zone`
// - the cell on the right may already have a LEFT border on that edge
// In that case we clear that LEFT border, so only the new RIGHT border
// remains on the shared edge.
//
// existingBorderSideToClear[side] = true means we should clear the border on that
// side of the existing adjacent zone before adding the new border.
const existingBorderSideToClear = {
left: force || !!newBorder?.right,
right: force || !!newBorder?.left,
top: force || !!newBorder?.bottom,
bottom: force || !!newBorder?.top,
};
let editingZone: Zone[] = [zone];
for (const existingBorder of this.borders[sheetId] ?? []) {
const inter = intersection(existingBorder.zone, zone);
if (!inter) {
// Clear adjacent borders on which you write
// Check if the existing border is adjacent to the new zone
const adjacentEdge = adjacent(existingBorder.zone, zone);
if (adjacentEdge && sideToClear[adjacentEdge.position]) {
if (adjacentEdge && existingBorderSideToClear[adjacentEdge.position]) {
for (const newZone of splitIfAdjacent(existingBorder.zone, zone)) {
const border = this.computeBorderFromZone(newZone, existingBorder);
const adjacentEdge = adjacent(newZone, zone);
// Clear the existing border on the side that touches the new zone
switch (adjacentEdge?.position) {
case "left":
border.style.left = undefined;
Expand Down
54 changes: 54 additions & 0 deletions tests/borders/border_plugin.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,60 @@ describe("borders", () => {
});
});

test("Preserves side borders when combining external and all via command", () => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should be very thorough here because it also affects top/bottom similarly., add some dispatch with "empty" border adjacents to C2:C4 and check that every single cell is still protected.

const model = new Model();
const defaultBorder = DEFAULT_BORDER_DESC;

setZoneBorders(model, { position: "all" }, ["C3"]);
setBordersOnTarget(model, ["C2"], {
top: defaultBorder,
right: defaultBorder,
left: defaultBorder,
});
setBordersOnTarget(model, ["C4"], {
bottom: defaultBorder,
left: defaultBorder,
right: defaultBorder,
});
setBordersOnTarget(model, ["B3"], {
top: defaultBorder,
bottom: defaultBorder,
left: defaultBorder,
});
setBordersOnTarget(model, ["D3"], {
top: defaultBorder,
bottom: defaultBorder,
right: defaultBorder,
});

expect(getBorder(model, "C3")).toEqual({
top: defaultBorder,
bottom: defaultBorder,
left: defaultBorder,
right: defaultBorder,
});
expect(getBorder(model, "C2")).toEqual({
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you should check every single cell and verify that they were not altered ;-)

top: defaultBorder,
left: defaultBorder,
right: defaultBorder,
});
expect(getBorder(model, "C4")).toEqual({
bottom: defaultBorder,
left: defaultBorder,
right: defaultBorder,
});
expect(getBorder(model, "B3")).toEqual({
top: defaultBorder,
left: defaultBorder,
bottom: defaultBorder,
});
expect(getBorder(model, "D3")).toEqual({
top: defaultBorder,
bottom: defaultBorder,
right: defaultBorder,
});
});

test("can set all borders in a zone", () => {
const model = new Model();

Expand Down