-
Notifications
You must be signed in to change notification settings - Fork 356
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
Default screen height should be the documentHeight #318
Comments
We could quite easily set the viewport height to the full document height, but I wonder what effect that would have when diffing images with different heights? Feel free to experiment -- you should be able to do this by changing the following line in javascript/_getDimensions.js from
to
|
Hmm, this isn't an easy fix. These dimensions are passed to Phantom/Casper before it loads the page - so the height is unreliable. Not to mention that the height might change at runtime, or after you do something with the before_capture hook. I think what you'll want to do is put this in your YAML config:
And make a resizeHeight.js file something along the lines of:
|
Add Wraith so that regressions can be easily spotted. Runs against a set of supported examples. Run while pointing government-frontend at the dummy content store, and an instance of static. Usage: On master run: wraith history test/wraith/config.yaml On local branch run: wraith latest test/wraith/config.yaml Had to run `bundle update` to get a version of nokogiri that would work with `wraith`. Configuration generated with `wraith setup` and combined with our existing wraith usage, [1]. With modern versions of wraith you don't need a `snap.js` file, as wraith will take care of the `phantomjs` intergration, however we still need to set some cookies to prevent intermitent UI (like the survey, or cookie bar) from interfering with the diff results. [1] alphagov/design-principles#246 This config seems to work OK-ish, though starts having time out issues with many combinations of screen_widths, dogpiling the rails app and getting timeouts. I couldn't find an obvious way to limit this without forking wraith, so for now i've used a smaller set of screen_widths. I also had to specifcy explicit heights for each screen_width, this seems to be new to Wraith 3.x. Previous versions would capture the full page as a default, where as now it will default to 1500px unless you specificy a height. This isn't ideal as most of the page on a small screen_size is much further down than 1500px. For now i've set some heights that deal with some common cases, but i'm not sure this is a good approach long term. There are some related wraith notes on that here: - bbc/wraith#318 (suggestion doesn't work) - bbc/wraith#296
I've been running into the same issue, wanting to get the the full page height, while using wraith with Currently, i'm trying to put together a wraith config for a new app, using the latest version of wraith, and a modern config (which is 👍 better). The behaviour we're after is full page height capture and image diffs. I tried some variations of the It sounds like this isn't supported right now, and not straight forward to add? If I can find the time i'd be interested in looking into this in more detail, but i'd be good to get a steer first 😄 (edit: sorry about the commit reference spam, there was some excessive rebasing) |
I, too, consider this a huge failing of version 3.x, as capturing with an arbitrarily large height of, say, 10000px has a major performance impact. A job that used to take about 2-3 minutes with version 2.x now takes over 10 minutes to perform the same analysis. Why was this changed from version 2? Can the old behavior be restored? |
I hear you - it would definitely be great to fix this. Reopening the issue by popular demand, will try to look at this more closely in the coming weeks. In the meantime, pull requests are welcome! Wraith v3.1.1 has more resilient image cropping, so should cope with dynamic heights much better, so hopefully this issue can be fixed more easily now. |
When there is no height parameter set allow the full height instead of the 1500 default For bbc#318
From what I'm seeing in phantom.js there is no way to set the viewport dimensions via a before_capture script as they will always be overridden by the configured dimensions. @ChrisBAshton What needs to be done to move #439 forward? |
Hello, I currently use the version 4.0.0 with phantomjs but I still have the same issue when I didn't specify a height in the screen_widths attribute it is still resized to 1500px. Regards, Benjamin V. |
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... |
If this helps, I seem to have Wraith producing full-screen screenshots using Casper and the following:
|
@rithipooh That worked perfectly fine. Do we really have to change the |
If you're using phantomjs you can get full-page screenshots by commenting out a few lines in
I think this is the solution pointed out by @rithipooh in #318 (comment). |
Hi guys, |
Is there any news on this? |
All, There is a teribble, awful, awful way to workaround this without having to modify the core code so that it addresses this comment from @duartegarin. If it weren't for the immediate need to have this resolved, I would never do this but what you can do is copy the snap file Again, I caution this is a terrible solution as you won't pick up maintenance or enhancements to the core copies of these files, but it will get you a temporary solution that works around having to patch the core code. |
Here is a PR that attempts to integrate the suggestions above. |
I have tried to make the changes of #572 and I can not make the screenshots in full screen, using screen_widths: The ones suggested in this comment either. Am I the only one that doesn't work? |
Hey. I'm stuck with this issue. Was this issue resolved? |
We're considering using Wraith on a project and would also be interested in a resolution. |
The default document height for screenshots is set to 1500px, this causes issues with some pages which are longer than this. Current workaround is to set something like:
screen_widths:
in order to get the full page in the screenshot. This obviously isn't ideal if we end up with a very long page.
The text was updated successfully, but these errors were encountered: