Skip to content
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

Issues using vh units in CSS #785

Open
justlevine opened this issue May 26, 2018 · 14 comments
Open

Issues using vh units in CSS #785

justlevine opened this issue May 26, 2018 · 14 comments

Comments

@justlevine
Copy link

How do I get backstopJS to respect height: [num]vh in my css?

Theyre getting comically stretched out.

Eg. (viewport: 1366x768, selectors: 'document')

Actual Chrome Browser (1366x768) - showing just the second section, for reference:
capture

Those two stretched-out sections have a height:90vh.

@garris
Copy link
Owner

garris commented May 26, 2018

What happens if your selectors prop is set to body?

@justlevine
Copy link
Author

@garris then I just get the viewport instead of the fullPage....

@garris
Copy link
Owner

garris commented May 26, 2018

please post your config. and version info.

@matthew-dean
Copy link

matthew-dean commented Jun 13, 2018

Getting the same issue. Viewport units (maybe just vh) are (more than?) twice what they should be.

@matthew-dean
Copy link

As far as I can tell this may be a Chromy issue: OnetapInc/chromy#117

@justlevine are you also using Chromy?

I'd love to just switch to Puppeteer, but it causes a host of other capturing issues that the Chromy plugin doesn't have. (Puppeteer captures incorrect section of the page, rendering the correct dimensions of the element but for the completely wrong coordinates.)

@garris
Copy link
Owner

garris commented Jun 13, 2018

Has anyone tried bumping the puppeteer version in backstop? Perhaps there will be some improvements?

@JamieMcNaught
Copy link

See my bug #820 - this seems to be an issue with the way Chrome(y) takes the screenshot by resizing the page - you can reproduce the issue in desktop Chrome by using "Capture Full size screenshot" in the "Developer Tools + mobile view". What I think we need to do is screenshot, scroll, screenshot, scroll and then stitch together.

I've no idea of any current + working workaround, but without a work around unfortunately I can't use backstep :( :( :(

@matthew-dean
Copy link

@JamieMcNaught Why doesn't Puppeteer have this issue? Only Chromy does, but I can't use the Puppeteer Backstop engine because it has other bugs.

@AdrienLemaire
Copy link

AdrienLemaire commented Dec 3, 2018

I have the same issue, using latest versions of BackstopJS and Puppeteer.
BackstopJS v3.8.5 with dependency puppeteer ^1.2.0-next.1523485686787

  "viewports": [
    {
      "label": "mobile",
      "width": 320,
      "height": 480
    },
  ],
  "engine": "puppeteer",
  "onBeforeScript": "puppet/onBefore.js",
  "onReadyScript": "puppet/onReady.js"

style.less

    @media @xs-only, @sm-only {
      margin: 0 -1rem;
      height: 61.5vh;
      border-radius: 0;
      …
    }

@martyfmelb
Copy link

martyfmelb commented Dec 8, 2018

I've made a just-good-enough fix for this so that I can snapshot toyota.com.au, which has CSS that heavily uses VH units:

https://github.com/ie/snapsite-vrt/blob/feature/client-agnostic-replace-toyota-com-au/engine_scripts/chromy/onReady.js

(See fixVhHeights for the vh unit fix, and fixPercentageHeights for the % height unit fix.)

Unfortunately this fix crashes on some other sites like lexus.com.au (i.e. no screenshot taken), so your mileage may vary.

@offroadkev
Copy link

offroadkev commented Feb 10, 2021

This issue is from 2018, we're now in 2021, should we abandon hope that backstop can be used as a visual regression tool in real life projects, or is this going to be resolved soon? This whole project is super awesome, but this issue is a stopper for myself and countless others unfortunately.

If this was fixed, backstop would be an amazing tool for all of us front end engineers, however, with this vh issue, I don't see how anyone could use backstop.

Sure there are websites that don't use vh (but most do these days, I would imagine), but imagine you spent the time to add backstop to your first 20+ templates on a project and then you got to one that had vh, boom, you just realized that all of your efforts with backstop were wasted....which is what happened to me.

Could someone from the backstop project chime in please and let us know if this is going to be resolved? Just need to know so that we can decide if we should rip backstop out of our project, or continue to wait for a resolution.

Please advise.

[Edit] @garris I see that you are the maintainer. I personally don't have time to contribute code to the project, but I am happy to do PR reviews and locally test any branches you have laying around that might solve this. I know you work your butt off for this project and like I said it's awesome (except for this blocker), so if you need another set of hands to help review code, please let me know, I'm more than capable and willing to do so.

@garris
Copy link
Owner

garris commented Feb 10, 2021

@vegaskev thanks for the help.

This product is 100% free -- it depends on community for support. I would love if the community would self-organize and solve this problem in a non-breaking way.

Please review the PRs and provide feedback. The question I would have is... What solution is the best to resolve this issue and why?

@kwaves
Copy link

kwaves commented Feb 10, 2021

Here's how I get around this issue:

CSS variables to the rescue

Step 1: Set all vh values with a CSS variable

:root {
  --viewport-height: 100vh;
}

Example usage:

.full-viewport-height {
    min-height: var(--viewport-height, 100vh);
}

.half-viewport-height {
    min-height: calc(var(--viewport-height, 100vh) * .5);
}

Step 2: Hard-code a px-based --viewport-height for your tests

Update the top line of overrideCSS.js to something like this:

const BACKSTOP_TEST_CSS_OVERRIDE = `html { --viewport-height: 1000px !important; } @media screen and (max-width: 361px) { html { --viewport-height: 740px !important; } }`;

The values above should match the dimensions of your viewports in backstop.json. In that example, the viewports were:

{
  "id": "whatever",
  "viewports": [
    {
      "label": "desktop",
      "width": 1440,
      "height": 1000
    },
    {
      "label": "mobile",
      "width": 360,
      "height": 740
    }
  ]
}

The above also assumes you're using onReady.js with the default overrideCSS reference intact.

For Chromy folks, you'd need to adapt overrideCSS.js from the Puppeteer folder.

Side benefit for 100vh mobile UI patterns

This idea occurred to me when I was trying to fix another problem:

As you may know, there's a long-running "feature-not-a-bug" on iPhone (and now Android too) where 100vh ignores the top and bottom browser bars. Those bars jump around as you scroll (and is also directionally-dependent, to my memory). The result is this:

viewport-units-mobile-crop

(Graphic: CSS Tricks)

More example code of the fix here.

Side note: While most people seem to be calculating 1vh then multiplying with calc(), I've found it's usually cleaner to set 100vh and divide, at least for the sorts of problems I'm usually trying to solve.

This doesn't seem like a Backstop problem

Given we're seeing this vh behavior with Chrome's full page screenshot feature in DevTools (as a commenter above pointed out), perhaps this should be addressed in Chromium (or whatever's responsible for this feature)? I'd be surprised if this wasn't a deliberate choice–even off the top of my head I can think of some advantages to handling it the way they currently are–but maybe this could be a candidate for a flag.

@offroadkev
Copy link

Thank you @garris and @kwaves

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

No branches or pull requests

8 participants