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

✨ Capture different pixel densities in asset discovery #970

Merged
merged 4 commits into from
Jun 27, 2022

Conversation

Robdel12
Copy link
Contributor

@Robdel12 Robdel12 commented Jun 22, 2022

What is this?

With the addition of real native devices in our rendering infrastructure, we now need to capture 2x++ pixel images for pages to render properly (since the real devices have high pixel density screens).

With the new devicePixelRatio config option, we now will resize the browser to each specified width at 1x and also whatever the passed value is for devicePixelRatio. We also run both beforeResize and afterResize events with the additional scale resizes. These assets are discovered in their own tab, separate from 1x devicePixelRatio assets to ensure that the right scaled assets are loaded (rather than trying to resize and reset the devicePixelRatio)

With the addition of real native devices in our rendering infrastructure, we now
need to capture 2x++ pixel images for pages to render properly (since the real
devices have high pixel density screens).

With the new `deviceScaleFactor` config option, we now will resize the browser
to each specified width at 1x and also whatever the passed value is for
`deviceScaleFactor`. We also run both `beforeResize` and `afterResize` events
with the additional scale resizes.
@Robdel12 Robdel12 added the ✨ enhancement New feature or request label Jun 22, 2022
packages/core/src/config.js Outdated Show resolved Hide resolved
packages/core/src/snapshot.js Outdated Show resolved Hide resolved
packages/core/src/snapshot.js Outdated Show resolved Hide resolved
packages/core/src/snapshot.js Outdated Show resolved Hide resolved
packages/core/src/snapshot.js Outdated Show resolved Hide resolved
packages/core/src/snapshot.js Outdated Show resolved Hide resolved
packages/core/src/page.js Outdated Show resolved Hide resolved
@Robdel12 Robdel12 force-pushed the rd/device-scale-factor branch 2 times, most recently from b6c915d to bb4e064 Compare June 27, 2022 18:29
Also renamed the new deviceScaleFactor option to devicePixelRatio to better reflect web standards
rather than the API option name.
@wwilsman
Copy link
Contributor

I can better articulate my suggestions with code, so after talking about it outside of this thread, I added a commit to change how the pixel ratio is handled.

We also decided to rename the option to better reflect web standards rather than the underlying API option name.

Copy link
Contributor

@wwilsman wwilsman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me as long as you approve of my minimal changes

@Robdel12 Robdel12 marked this pull request as ready for review June 27, 2022 19:55
@Robdel12 Robdel12 merged commit b73cb10 into master Jun 27, 2022
@Robdel12 Robdel12 deleted the rd/device-scale-factor branch June 27, 2022 23:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✨ enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants