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

Reconsider default deviceSizes configuration to handle 2x/3x DPI #18420

Closed
styfle opened this issue Oct 28, 2020 · 3 comments · Fixed by #18717
Closed

Reconsider default deviceSizes configuration to handle 2x/3x DPI #18420

styfle opened this issue Oct 28, 2020 · 3 comments · Fixed by #18717
Assignees
Milestone

Comments

@styfle
Copy link
Member

styfle commented Oct 28, 2020

Currently, deviceWidths is [320, 420, 768, 1024, 1200], however we did not consider DPI.

For example, an iPhone 5/SE is 320px wide but it requests the a 640px image from the srcset because its 2x DPI.

However, because we don't have 640w as an option, this means the 768w image is used because it is the next largest.

Apple Docs

@styfle styfle added this to the 10.x.x milestone Oct 28, 2020
@styfle styfle self-assigned this Oct 28, 2020
@Timer Timer modified the milestones: 10.x.x, iteration 12 Nov 2, 2020
@styfle
Copy link
Member Author

styfle commented Nov 2, 2020

Suggested defaults:

{
  deviceSizes: [640, 750, 828, 1080, 1200, 1920, 2048, 3840],
  imageSizes: [16, 32, 48, 64, 96, 128, 256, 384]
}

We can also use 1x, 2x, 3x variants for layout=fixed and layout=intrinsic.

@Timer Timer removed the priority: p1 label Nov 2, 2020
@styfle styfle changed the title Reconsider default deviceWidths configuration Reconsider default deviceWidths configuration to handle 2x/3x DPI Nov 2, 2020
@styfle styfle added point: 5 and removed point: 3 labels Nov 3, 2020
@styfle styfle changed the title Reconsider default deviceWidths configuration to handle 2x/3x DPI Reconsider default deviceSizes configuration to handle 2x/3x DPI Nov 3, 2020
@dohomi
Copy link

dohomi commented Nov 3, 2020

Suggested defaults:

{
  deviceSizes: [640, 750, 828, 1080, 1200, 1920, 2048, 3840],
  imageSizes: [16, 32, 48, 64, 96, 128, 256, 384]
}

We can also use 1x, 2x, 3x variants for layout=fixed and layout=intrinsic.

that would be awesome

@kodiakhq kodiakhq bot closed this as completed in #18717 Nov 3, 2020
kodiakhq bot pushed a commit that referenced this issue Nov 3, 2020
- Update default `deviceSizes`
- Add default `imageSizes`
- Use `layout` value to determine which `srcset` to use

Fixes #18420 
Closes #18714
@Timer Timer assigned styfle and Timer and unassigned styfle Nov 3, 2020
@balazsorban44
Copy link
Member

This issue has been automatically locked due to no recent activity. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@vercel vercel locked as resolved and limited conversation to collaborators Jan 29, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants