Skip to content

Conversation

@ChrisBAshton
Copy link
Contributor

@ChrisBAshton ChrisBAshton commented May 22, 2016

This PR significantly refactors the snapfile JavaScript (particularly for Phantom) and addresses issue #318.

Once merged, this should allow users to configure Wraiths without having to specify a height, whether they use PhantomJS or CasperJS.

Note: users with Wraith configurations where JavaScript is disabled will still have the issue, since Wraith has no way of determining the height of the document.

Before merging:

  • double check config-level and path-level before_capture hooks are still applied correctly, in both Phantom and Casper

@ChrisBAshton ChrisBAshton changed the title [in progress] Snapfile refactor [ready for review] Snapfile refactor May 22, 2016
@micros123
Copy link

@ChrisBAshton Dynamic heights would be a great addition! But despite having built a Gem from your 'refactor' branch, screenshots keep defaulting to 1500px height. Any ideas on why it might not be able to detect the height? (We are using PhantomJS)

@ChrisBAshton
Copy link
Contributor Author

Thanks for the feedback @micros123 , that's really useful. To help debug this, could you add verbose: true to your config, run your command and paste the output here?

@chrowe
Copy link

chrowe commented Jan 11, 2017

I tested this and it seemed to be working. I didn't have to change anything in my configs yaml file. All I did was install the a new gem based on the branch and run wraith with my current config.

Install steps

gem uninstall wraith //all versions
git clone git@github.com:BBC-News/wraith.git
cd wraith/
git checkout remotes/origin/refactor
gem build wraith.gemspec
gem install ./wraith-3.2.0.gem

@rithipooh
Copy link

Hello,

My friend and colleague found a wonderful solution to this issue. All we had to do was reduce the default height in helper.js to 100 instead of 1500 px in the function getWidthAndHeight(), and comment the line page.clipRect in Phantom.js to prevent setting the height. The code then takes the rendered height of the page for the screenshot!!! Try out and let us know how it works out...

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.

5 participants