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

Viewport option doesn't work #269

Open
JakubBlazity opened this issue Jun 6, 2023 · 8 comments · May be fixed by #273
Open

Viewport option doesn't work #269

JakubBlazity opened this issue Jun 6, 2023 · 8 comments · May be fixed by #273

Comments

@JakubBlazity
Copy link

Bug description

Viewport option doesn't have impact on generated image size

How to reproduce

  1. Use lost-pixel v3.4.1
  2. Create simple setup in example next app:
export const config: CustomProjectConfig = {
  pageShots: {
    pages: [
      { path: "/", name: "Home", viewport: { width: 375, height: 720 } },
    ],
    baseUrl: "http://localhost:3000",
  },
  waitBetweenFlakynessRetries: 5000,
  threshold: 0.0005,
  waitBeforeScreenshot: 10000,
  generateOnly: true,
  shotConcurrency: 5,
  failOnDifference: true,
};
  1. Run npx lost-pixel
  2. Generated image has default size

Expected behavior

Generated image should have specified size

Lost Pixel information

No response

lost-pixel logs from CI

No response

@d-ivashchuk
Copy link
Collaborator

Hey @JakubBlazity! What's your exact use case for the viewport? Do you want to make the screenshots for specific size of browser tab always or it's more in the direction of making shots with different breakpoints?

We have a feature for breakpoints, but the viewport as you noticed might not work as expected. We need to work on this!

@JakubBlazity
Copy link
Author

@d-ivashchuk
I want to be able create one screenshot for 1920x1080 browser size, and another, lets say for 375x720. Is it possible?

@d-ivashchuk
Copy link
Collaborator

with our breakpoint shots implementation, we went in the direction of horizontal breakpoints, meaning that you can set that up horizontally.

module.exports = {
  storybookShots: {
    storybookUrl: './storybook-static',
    breakpoints: [640, 1024],
  },
  lostPixelProjectId: 'xxx',
  apiKey: process.env.LOST_PIXEL_API_KEY,
};
uses: lost-pixel/lost-pixel@feature/breakpoint-shots

I don't think setting the viewport in page mode is possible now(we missed something in the implementation), but it should be quite easy to fix. If you want to make a contribution I'd be happy to assist!

@jerico-wf
Copy link

I had a look at the codebase and there's a condition that checks if configureBrowser exists. Adding this line made it work for me. Feels a bit strange though.

module.exports = {
  configureBrowser: (browser) => browser,
};

@d-ivashchuk
Copy link
Collaborator

Hey @jerico-wf! What exactly started to work with configureBrowser present? Might help us fixing it, thanks! I'd also be glad to accept a PR 🙌🏼

@jericopingul
Copy link

Hey @d-ivashchuk, just responding from my main account. I found this condition in the code which appears to require browserConfig to be defined for the code to accept page.viewport. Which is why I tried passing in configureBrowser and the viewport started working. Why the condition is there, I'm not so sure..

@d-ivashchuk
Copy link
Collaborator

Awesome work finding this out, I don't think there is some good reasoning behind this! Would appreciate the PR that fixes this so you don't need to use the hack 🙌🏼

@jericopingul
Copy link

@d-ivashchuk sure thing - let me know what you think about this 🙏 #273

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

Successfully merging a pull request may close this issue.

4 participants