Skip to content

Conversation

@dhrp-odoo
Copy link
Contributor

Description:

Steps to reproduce:

  • Apply “All” borders to B2:C3
  • Then merge C2:C3

Save the file, close the spreadsheet, and reopen it.

On import, the following happens:

  1. From data.borders, we call addBorder with:
    Zone: { top: 1, left: 2, bottom: 2, right: 2 } // C2:C3
    Border: { top, bottom, right, left (all thin black) }

    • This restores the stored borders for the merged range.
  2. Then we call addBorder for B2:B3 with:
    Zone: { top: 1, left: 1, bottom: 2, right: 1 } // B2:B3
    Border: { top, bottom, right, left, vertical, horizontal (all thin black) }

    • This reapplies the outer borders on B2:B3.
  3. Finally, import() calls addBordersToMerge(), which computes:
    Zone: { top: 1, left: 2, bottom: 2, right: 2 } // C2:C3
    Border: { top, bottom, right } // no left

    and then calls: this.addBorder(sheetId, zone, border, /* force = */ true);

The issue comes from how force was handled inside addBorder().

Because force = true for the merge, every entry in existingBorderSideToClear was set to true.
Thus, when the merged zone was adjacent to B2:B3, we cleared the border on the shared edge even if the new border did not define that side (in this case, there is no left on the merge). As a result, the right border of B2:B3 (the edge between B and C) disappeared after reopening.

We now compute existingBorderSideToClear so that it only depends on the sides actually defined on the new border.

Task: 5390397

review checklist

  • feature is organized in plugin, or UI components
  • support of duplicate sheet (deep copy)
  • in model/core: ranges are Range object, and can be adapted (adaptRanges)
  • in model/UI: ranges are strings (to show the user)
  • undo-able commands (uses this.history.update)
  • multiuser-able commands (has inverse commands and transformations where needed)
  • new/updated/removed commands are documented
  • exportable in excel
  • translations (_t("qmsdf %s", abc))
  • unit tested
  • clean commented code
  • track breaking changes
  • doc is rebuild (npm run doc)
  • status is correct in Odoo

- Apply “All” borders to B2:C3
- Then merge C2:C3

Save the file, close the spreadsheet, and reopen it.

On import, the following happens:

1. From `data.borders`, we call `addBorder` with:
   Zone:   { top: 1, left: 2, bottom: 2, right: 2 }   // C2:C3
   Border: { top, bottom, right, left (all thin black) }

   * This restores the stored borders for the merged range.

2. Then we call `addBorder` for B2:B3 with:
   Zone:   { top: 1, left: 1, bottom: 2, right: 1 }   // B2:B3
   Border: { top, bottom, right, left, vertical, horizontal (all thin black) }

   * This reapplies the outer borders on B2:B3.

3. Finally, `import()` calls `addBordersToMerge()`, which computes:
   Zone:   { top: 1, left: 2, bottom: 2, right: 2 }   // C2:C3
   Border: { top, bottom, right }                     // no left

   and then calls:
   `this.addBorder(sheetId, zone, border, /* force = */ true);`

The issue comes from how `force` was handled inside `addBorder()`.

Because `force = true` for the merge, *every* entry in
`existingBorderSideToClear` was set to `true`.
Thus, when the merged zone was adjacent to B2:B3, we cleared the border on the
shared edge even if the new border did not define that side (in this case,
there is no `left` on the merge). As a result, the right border of B2:B3 (the
edge between B and C) disappeared after reopening.

We now compute `existingBorderSideToClear` so that it only depends on the sides
actually defined on the new border.

Task: 5390397
@robodoo
Copy link
Collaborator

robodoo commented Dec 5, 2025

Pull request status dashboard

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants