search

christopherwk210 / site-scan

CLI for capturing website screenshots, powered by puppeteer.
MIT License
136 stars 13 forks source link

Add option to change resolution density #5

Closed adius closed 6 years ago

adius commented 6 years ago

The screenshots are really low density. No comparison to a normal screenshot taken on a retina display. There should definitely be an option to set the density (e.g. --density 100ppmm)

christopherwk210 commented 6 years ago

I'm open to a PR, but I'm not sure if that's possible.

A retina display mac renders everything at a larger size to accommodate the display. Because headless chrome is, well, headless, it doesn't render anything to a screen at all. It seems there is no way to "force" it to render like it would on a retina screen either, which makes sense because an option like that wouldn't really have a purpose.

The only solution I can think of is to scale the image after the screenshot is taken, and a feature like that doesn't really belong in this tool.

adius commented 6 years ago

I'm open to a PR, but I'm not sure if that's possible.

Pageres supports it: https://github.com/sindresorhus/pageres#scale

It seems there is no way to "force" it to render like it would on a retina screen either

I think what should work is zooming into the website with the same factor as scaling the viewport. I'd also guess that's how pageres does it...

adius commented 6 years ago

And scale is probably better than density. Not sure if anyone is interested in the exact density (which makes even less sense as it's not clear what the baseline size is as different screens have different densities in the first place)

christopherwk210 commented 6 years ago

Ah yes, that makes much more sense! You're right then, it's definitely possible and I've added the feature in d20bfa26372dc237bf6aa104d7a34be31f6726d4, good suggestion. Updated the npm package as well, closing :)

adius commented 6 years ago

Nice, thanks! (Btw: You messed up the version number a little ... after 1.2.7 comes 1.3.0 and not 1.3.7 😅)

adius commented 6 years ago

@christopherwk210 To cite the official specification (https://semver.org/#spec-item-7):

Patch version MUST be reset to 0 when minor version is incremented.

christopherwk210 commented 6 years ago

Hey you're right, thank you for the heads up! I had no idea, will do this going forward.