keatonwilson
commented
2 years ago @yogat3ch - tagging you here for the easy follow.
Open keatonwilson opened 2 years ago
keatonwilson
commented
2 years ago @yogat3ch - tagging you here for the easy follow.
yogat3ch
commented
2 years ago I added wait_for_idle before each screenshot, but that did not fix it. The 1px margin difference made me consider the choice of screen dimensions for the AppDriver. The odd values of pixels for the screen width (1293) causes the CSS when determining 50% screen width to land on either 646 or 647 as the column width. It’s probably influenced by minor differences in floating point values, so between runs this value lands on a different width. I changed the screen dimensions to a more typical screen HxW (800x600) and the issue resolved. Hope this helps anyone who encounters this issue!
yogat3ch
commented
2 years ago A couple of ideas for addressing this:
Add a margin threshold argument on expect_screenshot that allows for elements to deviate within a radius of the specified pixel width before an error is thrown.
Thanks shinytest2 devs!
schloerke
commented
2 years ago
- Add a margin threshold argument on
expect_screenshotthat allows for elements to deviate within a radius of the specified pixel width before an error is thrown.
Related rstudio/shinytest2#220
Thanks
shinytest2devs!
😃
schloerke
commented
1 year ago So I can still reprex this issue with the latest of everything. Thank you for the reprex repo with {renv}.
The issue can be reproduced with a window that is larger than 992px wide and by calling app$get_screenshot(). (Kinda fun to see the bug live using app$view().)
For some really weird reason, Chrome DevTools makes jQuery believe the window size has changed, causing $(window).resize(FUNC) to trigger here: https://github.com/ColorlibHQ/AdminLTE/blob/75deb497b39c1a8547fe0f21a2478b066b223396/build/js/PushMenu.js#L160-L162
This calls: https://github.com/ColorlibHQ/AdminLTE/blob/75deb497b39c1a8547fe0f21a2478b066b223396/build/js/PushMenu.js#L107-L125 ... which believes the window width is 0, causing the sidebar to collapse, which moves the cards to the left. Immediately after this, the window is resized back to where it was and the resize method is called again, prompting the sidebar to expand and the cards to move back.
Work around:
If the AdminLTE3 can disable the auto collapse by setting the push menu option autoCollapseSize=false, then this odd behavior can be avoided. Docs: https://adminlte.io/docs/3.1/javascript/push-menu.html
This is not a fix, but there is no longer any {chromote} code that is making a resized viewport to trigger the jQuery resize. This is mind boggling to me.
I don't know of a permanent fix as there is nothing that we are doing. I'd like to point the finger at the Chrome DevTools.
@yogat3ch, please let me know if this autoCollapseSize can be disabled when shiny is in testing mode (getOption("shiny.testmode", FALSE)) (or however you see fit). Thank you!
schloerke
commented
1 year ago If the option can't be set, AdminLTE3 could be patched and have the resize method be debounced or throttled. This would avoid the really really small time domain where the window believes it has a width of 1px and a height of 1px.
schloerke
commented
1 year ago Small reprex app to get the resize to show up:
app <-
shinytest2::AppDriver$new(
shiny::shinyApp(
ui = shiny::fluidPage(
shiny::tags$h1("Does it resize?", style="height:400px;width:400px;background-color:lightblue;"),
shiny::tags$script("
$(function() {
$(window).resize(function(){
console.log('resize triggered! Current window size: ', $(window).height(), 'x', $(window).width())
})
})
")
),
server = function(...) {}
)
)
# app$view()
app$get_screenshot() # Ignoring result
app$get_logs()
#> .... (Previous logs removed for brevity)
#> {chromote} JS info 09:18:03.45 shinytest2; Shiny has been idle for 200ms
#> {shinytest2} R info 09:18:03.45 Shiny app started
#> {shinytest2} R info 09:18:03.78 Viewing chromote session
#> {shinytest2} R info 09:18:11.92 Taking screenshot
{chromote} JS log 10:14:30.34 resize triggered! Current window size: 1 x 1
(anonymous) @ :22:22
dispatch @ jquery-3.6.0/jquery.min.js:1:43063
v.handle @ jquery-3.6.0/jquery.min.js:1:41047
{chromote} JS log 10:14:30.35 resize triggered! Current window size: 1323 x 992
(anonymous) @ :22:22
dispatch @ jquery-3.6.0/jquery.min.js:1:43063
v.handle @ jquery-3.6.0/jquery.min.js:1:41047
{shiny} R stderr ----------- Loading required package: shiny
{shiny} R stderr ----------- Running application in test mode.
{shiny} R stderr -----------
{shiny} R stderr ----------- Listening on http://127.0.0.1:7415
resize triggered! Current window size: 1
^^ That's an issue since we do not believe {chromote} v0.1.1 is performing any resizing.
**Investigating
schloerke
commented
1 year ago Moving this issue as it is an underlying issue in {chromote}, not {shinytest2}.
schloerke
commented
1 year ago Displaying the commands with a dput(msg) inside $send_command(), I can see that the only command being sent to the protocol has the shape:
list(
method = "Page.captureScreenshot",
params = list(
clip = list(
x = 0, y = 0,
width = 992, height = 430,
scale = 1
),
fromSurface = TRUE,
captureBeyondViewport = TRUE
)
)Before the above command, no resize is triggered. After the command, two resizes are triggered: (HxW) 1x1, then the viewport size of 1323x992.
captureBeyondViewport=TRUE (current behavior)If captureBeyondViewport is set to FALSE, then no resize is triggered.
This seems like a bug in the Chrome DevTools.
captureBeyondViewport=FALSEschloerke
commented
1 year ago Link to repo that explores this bug in Chrome DevTools Protocol: https://github.com/schloerke/chrome-devtools-protocol-screenshot-bug
Link to bug filed with Chromium: https://bugs.chromium.org/p/chromium/issues/detail?id=1414460
schloerke
commented
1 year ago @wch I can repro this unexpected behavior with chrome devtools protocol, not just {chromote}. Currently any screenshot with {chromote} causes this rapid viewport resize.
Should we revert #83? Or adopt a approach similar to https://github.com/puppeteer/puppeteer/blob/abcc1756dd434dbe27d322aa9692b7fd9858a9ca/packages/puppeteer-core/src/common/Page.ts#L1400-L1419 ?
I believe the only clean solution is that Chrome DevTools fixes itself and we maintain our current code.
gadenbuie
commented
1 year ago Can we rename this issue? Something like: ChromoteSession$screenshot() triggers a page resize before screenshot.
I'm currently having to do a lot of gymnastics to avoid the consequences of the page refresh. I think we should consider one of these options for updating the screenshot method:
captureBeyondViewport = TRUE only when it's required.captureBeyondViewport = FALSE under certain conditions, e.g. the viewport was directly requested.captureBeyondViewport as an option in the method signature with a user beware message in the docs.
hadley
commented
9 months ago I'd suggest we take the puppeteer approach.
Hi there!
Was working through building out some new testing frameworks on some of our apps and ran into some trouble developing simple snapshots. We're currently using bs4Dash for a lot of our app layouts, and it looks like there may be some compatibility issues with the snapshotting features with using this layout. Wondering if anyone has had these same issues and if it might be possible to diagnose.
I've developed a small reprex to illustrate the issue, which can be viewed (and forked) here.
In short, it appears as though there are small differences in the margins/re-sizing of elements when snapshots are taken that are generating differences between the reference images and the new snapshots. We've tried a few workarounds using
Sys.sleep()to determine if it might be an animation wait-time issue, with little success. Any help anyone could provide would be great - thank you!