Chrome full size screenshot without sticky header #38
Conversation
glibas
left a comment
glibas
left a comment
There was a problem hiding this comment.
Hi @galkina-anna ,
Thank you very much for your contribution. Could you please take a look at couple comments before PR can be merged.
Also could you please update docs with new usage examples.
Regards,
G
| /** | ||
| * Created by Anna Galkina on 23-06-2018. | ||
| */ | ||
| public class DriverCommandExecutor { |
There was a problem hiding this comment.
Lets move command execution methods to Browser class
src/main/resources/js/all-metrics.js
Outdated
| @@ -0,0 +1,3 @@ | |||
| ({width: Math.max(window.innerWidth,document.body.scrollWidth,document.documentElement.scrollWidth)|0, | |||
| height: Math.max(window.innerHeight,document.body.scrollHeight,document.documentElement.scrollHeight)|0, | |||
| deviceScaleFactor: window.devicePixelRatio || 1,mobile: typeof window.orientation !== 'undefined'}) No newline at end of file | |||
There was a problem hiding this comment.
New line added, but here in PR it is not showed:
| break; | ||
| case BOTH_DIRECTIONS: | ||
| pageScreenshot.setImage(browser.takeScreenshotEntirePage()); | ||
| case WITHOUT_STICKY_HEADER: |
| pageScreenshot.setImage(browser.takeScreenshotEntirePage()); | ||
| case WITHOUT_STICKY_HEADER: | ||
| pageScreenshot.setImage(browser.takeScreenshotEntirePage(true)); | ||
| pageScreenshot.setImage(browser.takeScreenshotEntirePageUsingChromeCommand()); |
There was a problem hiding this comment.
Shall we add broser.getName(); which will check which driver instance is currently used (instanceof presumably) and throw UnsupportedOperationException if current driver instance is other than Chrome
There was a problem hiding this comment.
Also, please add missing break to BOTH_DIRECTIONS case
| Object metrics = driver.evaluate(FileUtil.getJsScript(ALL_METRICS)); | ||
| driver.sendCommand("Emulation.setDeviceMetricsOverride", metrics); | ||
| Object result = driver.sendCommand("Page.captureScreenshot", ImmutableMap.of("format", "png", "fromSurface", true)); | ||
| driver.sendCommand("Emulation.clearDeviceMetricsOverride", ImmutableMap.of()); |
There was a problem hiding this comment.
It doesn't look like we need to clearDeviceMetricsOverride, as we don't really changing any initial device metrics just width and height which wont influence further test flow, moreover clearDeviceMetricsOverride for some reason spoins further highlighting/bluring etc. elements (coordinates are shifted).
|
Update from master before resolving. |
anna-shchurok
left a comment
anna-shchurok
left a comment
There was a problem hiding this comment.
Hi @glib-briia !
Thank you a lot for your review!
I have fixed my code following your comments. Only one left - I didn't change the doc as on README file this information would be inappropriate and wiki I have no access to amend.
Could you please do this for me or tell me how I find the way to change the doc?
Thank you,
2 participants
No description provided.