-
Notifications
You must be signed in to change notification settings - Fork 27k
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
Add experimental blurry placeholder to image component #24153
Changes from 8 commits
274f8d0
c74a188
ffa2db7
71380e3
40b83db
7049942
7a78a4d
44a5e86
0356aa1
e0558b4
3dc7d15
6b9c407
1cfdea1
8e0ae65
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
import React from 'react' | ||
import Image from 'next/image' | ||
|
||
export default function Page() { | ||
return ( | ||
<div> | ||
<p>Blurry Placeholder</p> | ||
<Image | ||
priority | ||
id="blurry-placeholder" | ||
src="/test.jpg" | ||
width="400" | ||
height="400" | ||
placeholder="blur" | ||
blurDataURL="data:image/svg+xml,%3Csvg xmlns='http://www.w3.org/2000/svg' width='400' height='400' viewBox='0 0 400 400'%3E%3Cfilter id='blur' filterUnits='userSpaceOnUse' color-interpolation-filters='sRGB'%3E%3CfeGaussianBlur stdDeviation='20' edgeMode='duplicate' /%3E%3CfeComponentTransfer%3E%3CfeFuncA type='discrete' tableValues='1 1' /%3E%3C/feComponentTransfer%3E%3C/filter%3E%3Cimage filter='url(%23blur)' href='data:image/jpeg;base64,/9j/4AAQSkZJRgABAQAAAQABAAD/2wBDAAMDAwMDAwQEBAQFBQUFBQcHBgYHBwsICQgJCAsRCwwLCwwLEQ8SDw4PEg8bFRMTFRsfGhkaHyYiIiYwLTA+PlT/wAALCAAKAAoBAREA/8QAMwABAQEAAAAAAAAAAAAAAAAAAAcJEAABAwUAAwAAAAAAAAAAAAAFAAYRAQMEEyEVMlH/2gAIAQEAAD8Az1bLPaxhiuk0QdeCOLDtHixN2dmd2bsc5FPX7VTREX//2Q==' x='0' y='0' height='100%25' width='100%25'/%3E%3C/svg%3E" | ||
Joonpark13 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
/> | ||
</div> | ||
) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -587,7 +587,10 @@ describe('Image Component Tests', () => { | |
nextConfig, | ||
` | ||
module.exports = { | ||
target: 'serverless' | ||
target: 'serverless', | ||
experimental: { | ||
enableBlurryPlaceholder: true, | ||
}, | ||
} | ||
` | ||
) | ||
|
@@ -600,6 +603,29 @@ describe('Image Component Tests', () => { | |
await killApp(app) | ||
}) | ||
|
||
it('should have blurry placeholder when enabled', async () => { | ||
const html = await renderViaHTTP(appPort, '/blurry-placeholder') | ||
expect(html).toContain( | ||
'background-image:url("data:image/svg+xml,%3Csvg xmlns='http://www.w3.org/2000/svg' width='400' height='400' viewBox='0 0 400 400'%3E%3Cfilter id='blur' filterUnits='userSpaceOnUse' color-interpolation-filters='sRGB'%3E%3CfeGaussianBlur stdDeviation='20' edgeMode='duplicate' /%3E%3CfeComponentTransfer%3E%3CfeFuncA type='discrete' tableValues='1 1' /%3E%3C/feComponentTransfer%3E%3C/filter%3E%3Cimage filter='url(%23blur)' href='data:image/jpeg;base64,/9j/4AAQSkZJRgABAQAAAQABAAD/2wBDAAMDAwMDAwQEBAQFBQUFBQcHBgYHBwsICQgJCAsRCwwLCwwLEQ8SDw4PEg8bFRMTFRsfGhkaHyYiIiYwLTA+PlT/wAALCAAKAAoBAREA/8QAMwABAQEAAAAAAAAAAAAAAAAAAAcJEAABAwUAAwAAAAAAAAAAAAAFAAYRAQMEEyEVMlH/2gAIAQEAAD8Az1bLPaxhiuk0QdeCOLDtHixN2dmd2bsc5FPX7VTREX//2Q==' x='0' y='0' height='100%25' width='100%25'/%3E%3C/svg%3E")' | ||
) | ||
}) | ||
|
||
it('should remove blurry placeholder after image loads', async () => { | ||
let browser | ||
try { | ||
browser = await webdriver(appPort, '/blurry-placeholder') | ||
const id = 'blurry-placeholder' | ||
const backgroundImage = await browser.eval( | ||
`window.getComputedStyle(document.getElementById('${id}')).getPropertyValue('background-image')` | ||
) | ||
expect(backgroundImage).toBe('none') | ||
} finally { | ||
if (browser) { | ||
await browser.close() | ||
} | ||
} | ||
}) | ||
|
||
Comment on lines
+606
to
+628
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As suggested by @atcastle I'm adding the integration tests to the serverless suite only in order to avoid changing the config and having to rebuild. This should be fine for now since there's no functional difference for this feature between the modes and when this feature graduates out of experimental, we can come back and place this where it should live with the right config. |
||
runTests('serverless') | ||
}) | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@timneutkens @Timer Do you think we should use
empty
ornone
? Do we have any precedence for this?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think
none
would be a good name since it may imply that we don't placeholder the image in any way, causing layout shift.empty
was used in an effort to promote the understanding that there will at least be empty space.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another option here is
blank
. I agree thatnone
might imply that we don't have a placeholder here.