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.

Sign up for GitHub

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

Jump to bottom

Allow smaller sizes in srcset for image with fill layout and sizes prop #21670

Merged
merged 9 commits into from
Feb 24, 2021
Merged

Allow smaller sizes in srcset for image with fill layout and sizes prop #21670

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
2edfc74
Allow smaller-than-viewport sizes in srcset for image with fill layou…
atcastle Jan 29, 2021
7cae0a2
Adjust integration test values
atcastle Jan 29, 2021
d7bfe91
Separate image fill sizing integration test into separate expect stat…
atcastle Jan 29, 2021
130c8fb
Update optimization to take into account more complex sizes values
atcastle Jan 29, 2021
3317dbb
Add layout fill sizing integration test case for px values
atcastle Jan 29, 2021
562f7f4
Don't apply new image component sizes rule to responsive layout, and …
atcastle Jan 29, 2021
72320f0
Refactor support for responsive layout with sizes prop and clean up i…
atcastle Jan 30, 2021
3b03fc2
Merge remote-tracking branch 'origin' into image-component-update-fil…
atcastle Feb 24, 2021
fa82398
Update base-path image component test to reflect new sizes behavior
atcastle Feb 24, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 19 additions & 2 deletions packages/next/client/image.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,25 @@ allSizes.sort((a, b) => a - b)

function getWidths(
width: number | undefined,
layout: LayoutValue
layout: LayoutValue,
sizes: string | undefined
): { widths: number[]; kind: 'w' | 'x' } {
if (sizes && (layout === 'fill' || layout === 'responsive')) {
// Find all the "vw" percent sizes used in the sizes prop
const percentSizes = [...sizes.matchAll(/(^|\s)(1?\d?\d)vw/g)].map((m) =>
parseInt(m[2])
)
if (percentSizes.length) {
const smallestRatio = Math.min(...percentSizes) * 0.01
return {
widths: allSizes.filter(
(s) => s >= configDeviceSizes[0] * smallestRatio
),
kind: 'w',
}
}
return { widths: allSizes, kind: 'w' }
}
if (
typeof width !== 'number' ||
layout === 'fill' ||
Expand Down Expand Up @@ -146,7 +163,7 @@ function generateImgAttrs({
return { src, srcSet: undefined, sizes: undefined }
}

const { widths, kind } = getWidths(width, layout)
const { widths, kind } = getWidths(width, layout, sizes)
const last = widths.length - 1

return {
Expand Down
2 changes: 1 addition & 1 deletion test/integration/image-component/base-path/test/index.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -371,7 +371,7 @@ function runTests(mode) {
'/docs/_next/image?url=%2Fdocs%2Fwide.png&w=3840&q=75'
)
expect(await browser.elementById(id).getAttribute('srcset')).toBe(
'/docs/_next/image?url=%2Fdocs%2Fwide.png&w=640&q=75 640w, /docs/_next/image?url=%2Fdocs%2Fwide.png&w=750&q=75 750w, /docs/_next/image?url=%2Fdocs%2Fwide.png&w=828&q=75 828w, /docs/_next/image?url=%2Fdocs%2Fwide.png&w=1080&q=75 1080w, /docs/_next/image?url=%2Fdocs%2Fwide.png&w=1200&q=75 1200w, /docs/_next/image?url=%2Fdocs%2Fwide.png&w=1920&q=75 1920w, /docs/_next/image?url=%2Fdocs%2Fwide.png&w=2048&q=75 2048w, /docs/_next/image?url=%2Fdocs%2Fwide.png&w=3840&q=75 3840w'
'/docs/_next/image?url=%2Fdocs%2Fwide.png&w=16&q=75 16w, /docs/_next/image?url=%2Fdocs%2Fwide.png&w=32&q=75 32w, /docs/_next/image?url=%2Fdocs%2Fwide.png&w=48&q=75 48w, /docs/_next/image?url=%2Fdocs%2Fwide.png&w=64&q=75 64w, /docs/_next/image?url=%2Fdocs%2Fwide.png&w=96&q=75 96w, /docs/_next/image?url=%2Fdocs%2Fwide.png&w=128&q=75 128w, /docs/_next/image?url=%2Fdocs%2Fwide.png&w=256&q=75 256w, /docs/_next/image?url=%2Fdocs%2Fwide.png&w=384&q=75 384w, /docs/_next/image?url=%2Fdocs%2Fwide.png&w=640&q=75 640w, /docs/_next/image?url=%2Fdocs%2Fwide.png&w=750&q=75 750w, /docs/_next/image?url=%2Fdocs%2Fwide.png&w=828&q=75 828w, /docs/_next/image?url=%2Fdocs%2Fwide.png&w=1080&q=75 1080w, /docs/_next/image?url=%2Fdocs%2Fwide.png&w=1200&q=75 1200w, /docs/_next/image?url=%2Fdocs%2Fwide.png&w=1920&q=75 1920w, /docs/_next/image?url=%2Fdocs%2Fwide.png&w=2048&q=75 2048w, /docs/_next/image?url=%2Fdocs%2Fwide.png&w=3840&q=75 3840w'
)
expect(await browser.elementById(id).getAttribute('sizes')).toBe(
'(max-width: 2048px) 1200px, 3840px'
Expand Down
23 changes: 23 additions & 0 deletions test/integration/image-component/default/pages/layout-fill.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,29 @@ const Page = () => {
/>
</div>
<p>Layout Fill</p>
<div style={{ position: 'relative', width: '50vw', height: '50vh' }}>
<Image
id="fill3"
src="/wide.png"
layout="fill"
objectFit="cover"
objectPosition="left center"
sizes="(min-width: 1200px) 90vw,
(min-width: 800px) 30vw,
100vw"
/>
</div>
<p>Layout Fill</p>
<div style={{ position: 'relative', width: '50vw', height: '50vh' }}>
<Image
id="fill4"
src="/wide.png"
layout="fill"
objectFit="cover"
objectPosition="left center"
sizes="500px"
/>
</div>
</div>
)
}
Expand Down
10 changes: 9 additions & 1 deletion test/integration/image-component/default/test/index.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -410,6 +410,14 @@ function runTests(mode) {
)
expect(objectFit).toBe('cover')
expect(objectPosition).toBe('left center')
await browser.eval(`document.getElementById("fill3").scrollIntoView()`)
expect(await browser.elementById('fill3').getAttribute('srcset')).toBe(
'/_next/image?url=%2Fwide.png&w=256&q=75 256w, /_next/image?url=%2Fwide.png&w=384&q=75 384w, /_next/image?url=%2Fwide.png&w=640&q=75 640w, /_next/image?url=%2Fwide.png&w=750&q=75 750w, /_next/image?url=%2Fwide.png&w=828&q=75 828w, /_next/image?url=%2Fwide.png&w=1080&q=75 1080w, /_next/image?url=%2Fwide.png&w=1200&q=75 1200w, /_next/image?url=%2Fwide.png&w=1920&q=75 1920w, /_next/image?url=%2Fwide.png&w=2048&q=75 2048w, /_next/image?url=%2Fwide.png&w=3840&q=75 3840w'
)
await browser.eval(`document.getElementById("fill4").scrollIntoView()`)
expect(await browser.elementById('fill4').getAttribute('srcset')).toBe(
'/_next/image?url=%2Fwide.png&w=16&q=75 16w, /_next/image?url=%2Fwide.png&w=32&q=75 32w, /_next/image?url=%2Fwide.png&w=48&q=75 48w, /_next/image?url=%2Fwide.png&w=64&q=75 64w, /_next/image?url=%2Fwide.png&w=96&q=75 96w, /_next/image?url=%2Fwide.png&w=128&q=75 128w, /_next/image?url=%2Fwide.png&w=256&q=75 256w, /_next/image?url=%2Fwide.png&w=384&q=75 384w, /_next/image?url=%2Fwide.png&w=640&q=75 640w, /_next/image?url=%2Fwide.png&w=750&q=75 750w, /_next/image?url=%2Fwide.png&w=828&q=75 828w, /_next/image?url=%2Fwide.png&w=1080&q=75 1080w, /_next/image?url=%2Fwide.png&w=1200&q=75 1200w, /_next/image?url=%2Fwide.png&w=1920&q=75 1920w, /_next/image?url=%2Fwide.png&w=2048&q=75 2048w, /_next/image?url=%2Fwide.png&w=3840&q=75 3840w'
)
} finally {
if (browser) {
await browser.close()
Expand All @@ -429,7 +437,7 @@ function runTests(mode) {
'/_next/image?url=%2Fwide.png&w=3840&q=75'
)
expect(await browser.elementById(id).getAttribute('srcset')).toBe(
'/_next/image?url=%2Fwide.png&w=640&q=75 640w, /_next/image?url=%2Fwide.png&w=750&q=75 750w, /_next/image?url=%2Fwide.png&w=828&q=75 828w, /_next/image?url=%2Fwide.png&w=1080&q=75 1080w, /_next/image?url=%2Fwide.png&w=1200&q=75 1200w, /_next/image?url=%2Fwide.png&w=1920&q=75 1920w, /_next/image?url=%2Fwide.png&w=2048&q=75 2048w, /_next/image?url=%2Fwide.png&w=3840&q=75 3840w'
'/_next/image?url=%2Fwide.png&w=16&q=75 16w, /_next/image?url=%2Fwide.png&w=32&q=75 32w, /_next/image?url=%2Fwide.png&w=48&q=75 48w, /_next/image?url=%2Fwide.png&w=64&q=75 64w, /_next/image?url=%2Fwide.png&w=96&q=75 96w, /_next/image?url=%2Fwide.png&w=128&q=75 128w, /_next/image?url=%2Fwide.png&w=256&q=75 256w, /_next/image?url=%2Fwide.png&w=384&q=75 384w, /_next/image?url=%2Fwide.png&w=640&q=75 640w, /_next/image?url=%2Fwide.png&w=750&q=75 750w, /_next/image?url=%2Fwide.png&w=828&q=75 828w, /_next/image?url=%2Fwide.png&w=1080&q=75 1080w, /_next/image?url=%2Fwide.png&w=1200&q=75 1200w, /_next/image?url=%2Fwide.png&w=1920&q=75 1920w, /_next/image?url=%2Fwide.png&w=2048&q=75 2048w, /_next/image?url=%2Fwide.png&w=3840&q=75 3840w'
)
expect(await browser.elementById(id).getAttribute('sizes')).toBe(
'(max-width: 2048px) 1200px, 3840px'
Expand Down