Added pagination start page #227
Conversation
Added an option to the generation script to dictate on which page the page numbering should start. Option name is paginationStartPage and is 1 based (to start pagination at page 2, the value would be 2)
| contents: phantom.callback(function (pageNum, numPages) { | ||
| var html = o[pageNum] || c[pageNum] | ||
|
|
||
| var pageNumFinal = pageNum, numPagesFinal = numPages; |
There was a problem hiding this comment.
Please split up multiple assignments onto multiple lines.
This didn't throw an error in the linting script because only bin/index.js was tested.
I've created a pr here that fixes the config https://github.com/marcbachmann/node-html-pdf/pull/230/files
Can you please rebase and apply those changes.
|
Thanks for your pr. This is definitely something we can take in. |
| var pageNumFinal = pageNum, numPagesFinal = numPages; | ||
| if(opts.paginationStartPage && opts.paginationStartPage > 1) { | ||
| var pageOffset = opts.paginationStartPage - 1; | ||
| pageNumFinal = (pageNumFinal - pageOffset > 0 ? pageNumFinal - pageOffset : ''); |
There was a problem hiding this comment.
That's strange that you have an empty string as fallback.
The ternary also looks strange. Please use (pageNumFinal - pageOffset > 0) ? pageNumFinal - pageOffset : ''.
There was a problem hiding this comment.
Will do. The empty string is to actualy print out a blank for cover pages and such
There was a problem hiding this comment.
Maybe it would look nicer if we have that fallback in the replace method.
| if(opts.paginationStartPage && opts.paginationStartPage > 1) { | ||
| var pageOffset = opts.paginationStartPage - 1; | ||
| pageNumFinal = (pageNumFinal - pageOffset > 0 ? pageNumFinal - pageOffset : ''); | ||
| numPagesFinal -= pageOffset; |
There was a problem hiding this comment.
I suggest to never use -=. More explicit is better.
Would be nice if we can calculate those numbers differently.
e.g.
var paginationStartPage = (opts.paginationStartPage > 1) ? opts.paginationStartPage : 1
var pageNumFinal = (pageNum - pageOffset > 0) ? pageNumFinal - pageOffset : ''
var numPagesFinal = numPages - (paginationStartPage - 1)There was a problem hiding this comment.
Agreed, will make those changes.
Also would be nice if we could have header and footer for the current page. E.g. if you have, in a .page element a .header or .footet, it be used as the header/footer for that specific page.
Oh, do you know how to rebase? If there's no conflict, you can force push to overwite your changes with |
|
Hi, guys. Could you tell please status this pr now? |
|
I'll merge that in here: #258 |
3 participants
Added an option to the generation script to dictate on which page the page numbering should start. Option name is paginationStartPage and is 1 based (to start pagination at page 2, the value would be 2)
Sorry if this pull request is made the wrong way, it is my first one and made to the best of my knowlege.