Skip to content

Fix the error that comment does not delete at spliceColumn #1334

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
Jun 20, 2020

Conversation

sdg9670
Copy link
Contributor

@sdg9670 sdg9670 commented Jun 17, 2020

Summary

Fix the error that comment does not delete at spliceColumn

Test plan

Test Source Code

const Excel = require('./lib/exceljs.nodejs');

(async () => {
    const wb = new Excel.Workbook();
    const ws = wb.addWorksheet('testSheet');

    ws.addRow(['test1', 'test2', 'test3', 'test4', 'test5', 'test6', 'test7', 'test8']);

    const row = ws.getRow(1);
    row.getCell(1).note = 'test1';
    row.getCell(2).note = 'test2';
    row.getCell(3).note = 'test3';
    row.getCell(4).note = 'test4';

    ws.spliceColumns(2, 1);

    await wb.xlsx.writeFile('data.xlsx');
})();

Existing Results
image

Result
image

@sdg9670 sdg9670 changed the title Corrected the error that comment does not delete during spliceColumn operation. Fix the error that comment does not delete during spliceColumn operation. Jun 17, 2020
@sdg9670 sdg9670 changed the title Fix the error that comment does not delete during spliceColumn operation. Fix the error that comment does not delete at spliceColumn Jun 17, 2020
Copy link
Member

@Siemienik Siemienik left a comment

Choose a reason for hiding this comment

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

Thanks for your pull request :)

Before I can merge it into master It's required to add test case which checks if this bug is fixed.

Could I please you to write any test which covers this fix?

Copy link
Member

@Siemienik Siemienik left a comment

Choose a reason for hiding this comment

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

thanks a lot

@Siemienik Siemienik merged commit e2aab24 into exceljs:master Jun 20, 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.

2 participants