Skip to content
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

Allow setting page.paperSize from options explicitly #134

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

xelez
Copy link

@xelez xelez commented May 16, 2016

Motivation

http://phantomjs.org/api/webpage/property/paper-size.html

If no paperSize is defined, the size is defined by the web page.

Currently paperSize is always defined, so size auto detection is unusable, although I personally sometimes need it.

Problem

I couldn't find a way in PhantomJS to turn on size auto detection and use headers/footers at the same time. So, adding options like format: 'auto' seems unreasonable because it would break other options.

Variant of solution

Well.. allowing users to set PhantomJS pageSize to what they exactly want seems to work just find and it must be quite obvious that setting it manually will overwrite other options. And setting it to
empty object ({}) does the work and turns on page size auto detection, so it's a kind of solution.

Maybe having something like pageSize: 'auto' is better, but I'm not really sure.

Comments appreciated.
P.S. If I missed something like contributing guidelines, I'll be glad if you point me to them.

@@ -54,7 +54,7 @@ setTimeout(function () {
// ----------------------------------
page.onLoadFinished = function (status) {
// The paperSize object must be set at once
page.paperSize = definePaperSize(getContent(page), options)
page.paperSize = options.paperSize || definePaperSize(getContent(page), options)
Copy link
Owner

Choose a reason for hiding this comment

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

What about null? Do you know whether that might work?
I'd use

if (typeof options.paperSize === 'undefined') options.paperSize = definePaperSize(getContent(page), options) 

@jhwheeler
Copy link

@marcbachmann Has this feature been implemented? It took me a while to discover that this package doesn't allow PhantomJS's auto sizing to work, but it's a feature that I really need. Would love to hear of a solution. Thank you!

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.

3 participants