Skip to content

fix issue #1045 horizontalCentered & verticalCentered in page not working #1073

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Jan 17, 2020
Merged

fix issue #1045 horizontalCentered & verticalCentered in page not working #1073

merged 4 commits into from
Jan 17, 2020

Conversation

Alanscut
Copy link
Member

Changes

  1. This PR solves the problem that horizontalCentered & verticalCentered is not working.

  2. Fix for horizontalCentered & verticalCentered in page not working #1045

@Alanscut Alanscut requested a review from Siemienik January 15, 2020 11:37
@Siemienik
Copy link
Member

@Alanscut thanks for that PR, it's a bit of great work here:)
Could I ask you to write at least one test which checks this logic? (for example, write and read file + asserts for these properties).

@Alanscut
Copy link
Member Author

hi @Siemienik
Thank you very much for your comment.
See https://github.com/exceljs/exceljs/blob/master/spec/unit/xlsx/xform/sheet/print-options-xform.spec.js#L23

@Siemienik
Copy link
Member

hmm.. so how the tests work ok before and now after source code was changed? Are both versions correct?

@Alanscut
Copy link
Member Author

@Siemienik Having researched the unit test, I found I have misunderstood it. Actually the testcases for it lies in file worksheet-xform.spec.js, property horizontalCentered are not tested before, so I have add the test case and fixed issue #1079 also.

exceljs/spec/unit/xlsx/xform/sheet/worksheet-xform.spec.js

Lines 53 to 70 in 9054c32

{
title: 'Sheet 1',
create: () => new WorksheetXform(),
initialModel: fixDate(require('./data/sheet.1.0.json')),
preparedModel: fixDate(require('./data/sheet.1.1.json')),
xml: fs.readFileSync(`${__dirname}/data/sheet.1.2.xml`).toString(),
parsedModel: require('./data/sheet.1.3.json'),
reconciledModel: fixDate(require('./data/sheet.1.4.json')),
tests: ['prepare', 'render', 'parse'],
options: {
sharedStrings: new SharedStringsXform(),
hyperlinks: [],
hyperlinkMap: fakeHyperlinkMap,
styles: fakeStyles,
formulae: {},
siFormulae: 0,
},
},

@Siemienik
Copy link
Member

great :)

@Siemienik Siemienik merged commit 207ba4e into exceljs:master Jan 17, 2020
@Alanscut Alanscut deleted the issue_1045 branch May 5, 2020 07:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Siemienik Siemienik Awaiting requested review from Siemienik

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Alanscut @Siemienik