Skip to content

Keep borders of merged cells after rewriting an Excel workbook #1102

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Feb 6, 2020

Conversation

kigh-ota
Copy link
Contributor

@kigh-ota kigh-ota commented Jan 31, 2020

This PR is intended to fix #623.

Cause of the bug

Currently, styles of cells included in a merged cell are merged and always the same after reading from file, because Worksheet#set model() calls Cell#merge(), which overrides the cell's style with the master (left-top) cell.

this._parseMergeCells(value);

this.style = master.style;

However, in the original xml, each cell in a merged cell may have different styles.
For example, this file, which I created with Excel, has sheet1.xml like this:

  <sheetData>
    <row r="2" spans="2:3">
      <c r="B2" s="1"/>
      <c r="C2" s="2"/>
    </row>
    <row r="3" spans="2:3">
      <c r="B3" s="3"/>
      <c r="C3" s="4"/>
    </row>
  </sheetData>
  <mergeCells count="1">
    <mergeCell ref="B2:C3"/>
  </mergeCells>

Thus, merging the styles drops intended cell styles, which causes #623.

Solution

Worksheet#set model() is changed such that it parses merged cells without overriding the styles of included cells.

Note

I keep the behavior of Worksheet#mergeCells() unchanged, since this is an API that can be used programmatically. https://github.com/exceljs/exceljs#merged-cells
Another related issue #635 seems to correspond to this programmatic use case, but I think it can be tackled separately.

@npmDude
Copy link

npmDude commented Feb 4, 2020

Can I know when is the scheduled merge for this PR?

@Siemienik
Copy link
Member

in the time when @guyonroche has a minute :)

@alubbe alubbe merged commit fc8c138 into exceljs:master Feb 6, 2020
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.

Issue with borders for merged cell when rewriting an excel workbook
4 participants