-
Notifications
You must be signed in to change notification settings - Fork 62
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
Mostly updates for phantomjs 2.0 by DEfusion #32
base: master
Are you sure you want to change the base?
Conversation
For some reason it wasn't loading for me and thus the callback was never called. We don't need jQuery to do those calculations anyway so clean it up by removing it.
Happens when div loading is dependent on resources loading.
im happy to merge. can we get the test fixed? |
- Instead of modifying the height of the body (and html) after render we just set the mask to take care of it. The image we were getting was of the full size page cropped down to the height and the rest filled with white. I think this is because the larger initial page size caused the viewport to expand but then it didn’t contract. - Since args.div will supersede the mask eventually, don’t set the mask up front if args.div is present. Set the mask solely on the height arg as we can default top and left to 0. This fixes the failing unit tests when running locally.
Not really sure how the build is set up. Looks like ruby 2 passed where as before it was failing but the other 3 versions are dead and I don't think it has anything to do with what I did. |
Good work with the upgrade 👍 The problem might be with change of version of the gem from 0.1.4 to 0.2.1. |
Yeah, I thought it might be that also but why is it only affecting one of the build versions? Makes me think it is a setting somewhere that I don't have access to. |
See PR#35 for a fix to this build issue. |
Also, a fix for deferring finding the screencap element until all resources have loaded. This fixes issues with client side rendering that depends on resource loading.
We are fine with having this as a fork but might help others to get it merged back into the main repo.