Skip to content

Commit

Permalink
Fix next/future/image alt text (#39366)
Browse files Browse the repository at this point in the history
In many browsers (other than Chrome), the `alt` text is visible while the image is loading. This causes a sense layout shift since you'll see a flash of text and then the image (although lighthouse measures 0 CLS, likely because Chrome doesn't have this problem). This PR updates `next/future/image` to hide the alt text, unless there is an error while loading the image in which case the `alt` text because relevant as the fallback.

Example:

<img width="115" alt="image" src="https://user-images.githubusercontent.com/229881/183128008-0660c50c-18aa-4e64-872e-ada9a652130f.png">

Unfortunately, Safari also shows a border while lazy loading images and it cannot be styled.

See upstream issue here: https://bugs.webkit.org/show_bug.cgi?id=243601
  • Loading branch information
styfle committed Aug 6, 2022
1 parent a679a1e commit 59a01ec
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 6 deletions.
7 changes: 7 additions & 0 deletions packages/next/client/future/image.tsx
Expand Up @@ -130,6 +130,7 @@ type ImageElementProps = Omit<ImageProps, 'src' | 'loader'> & {
placeholder: PlaceholderValue
onLoadingCompleteRef: React.MutableRefObject<OnLoadingComplete | undefined>
setBlurComplete: (b: boolean) => void
setShowAltText: (b: boolean) => void
noscriptSizes: string | undefined
}

Expand Down Expand Up @@ -403,6 +404,7 @@ export default function Image({
}

const [blurComplete, setBlurComplete] = useState(false)
const [showAltText, setShowAltText] = useState(false)
let widthInt = getInt(width)
let heightInt = getInt(height)
const qualityInt = getInt(quality)
Expand Down Expand Up @@ -576,6 +578,7 @@ export default function Image({
width: '100%',
}
: {},
showAltText || placeholder === 'blur' ? {} : { color: 'transparent' },
style
)
const svgBlurPlaceholder = `url("data:image/svg+xml;charset=utf-8,%3Csvg xmlns='http%3A//www.w3.org/2000/svg' viewBox='0 0 ${widthInt} ${heightInt}'%3E%3Cfilter id='b' color-interpolation-filters='sRGB'%3E%3CfeGaussianBlur stdDeviation='50'/%3E%3CfeComponentTransfer%3E%3CfeFuncA type='discrete' tableValues='1 1'/%3E%3C/feComponentTransfer%3E%3C/filter%3E%3Cimage filter='url(%23b)' x='0' y='0' height='100%25' width='100%25' href='${blurDataURL}'/%3E%3C/svg%3E")`
Expand Down Expand Up @@ -655,6 +658,7 @@ export default function Image({
srcString,
onLoadingCompleteRef,
setBlurComplete,
setShowAltText,
noscriptSizes: sizes,
...rest,
}
Expand Down Expand Up @@ -704,6 +708,7 @@ const ImageElement = ({
loader,
onLoadingCompleteRef,
setBlurComplete,
setShowAltText,
onLoad,
onError,
noscriptSizes,
Expand Down Expand Up @@ -756,6 +761,8 @@ const ImageElement = ({
}
}}
onError={(event) => {
// if the real image fails to load, this will ensure "alt" is visible
setShowAltText(true)
if (placeholder === 'blur') {
// If the real image fails to load, this will still remove the placeholder.
setBlurComplete(true)
Expand Down
28 changes: 22 additions & 6 deletions test/integration/image-future/default/test/index.test.js
Expand Up @@ -361,18 +361,30 @@ function runTests(mode) {
() => browser.eval(`document.getElementById("msg1").textContent`),
'no error occured for img1'
)
await check(
() => browser.eval(`document.getElementById("img1").style.color`),
'transparent'
)
await browser.eval(
`document.getElementById("img2").scrollIntoView({behavior: "smooth"})`
)
await check(
() => browser.eval(`document.getElementById("msg2").textContent`),
'no error occured for img2'
)
await check(
() => browser.eval(`document.getElementById("img2").style.color`),
'transparent'
)
await browser.eval(`document.getElementById("toggle").click()`)
await check(
() => browser.eval(`document.getElementById("msg2").textContent`),
'error occured while loading img2'
)
await check(
() => browser.eval(`document.getElementById("img2").style.color`),
''
)
})

it('should work with image with blob src', async () => {
Expand Down Expand Up @@ -444,7 +456,9 @@ function runTests(mode) {
)
expect(childElementType).toBe('IMG')

expect(await browser.elementById('img1').getAttribute('style')).toBeNull()
expect(await browser.elementById('img1').getAttribute('style')).toBe(
'color:transparent'
)
expect(await browser.elementById('img1').getAttribute('height')).toBe(
'700'
)
Expand All @@ -459,7 +473,7 @@ function runTests(mode) {
)

expect(await browser.elementById('img2').getAttribute('style')).toBe(
'padding-left:4rem;width:100%;object-position:30% 30%'
'color:transparent;padding-left:4rem;width:100%;object-position:30% 30%'
)
expect(await browser.elementById('img2').getAttribute('height')).toBe(
'700'
Expand All @@ -474,7 +488,9 @@ function runTests(mode) {
'lazy'
)

expect(await browser.elementById('img3').getAttribute('style')).toBeNull()
expect(await browser.elementById('img3').getAttribute('style')).toBe(
'color:transparent'
)
expect(await browser.elementById('img3').getAttribute('srcset')).toBe(
`/_next/image?url=%2Ftest.png&w=640&q=75 1x, /_next/image?url=%2Ftest.png&w=828&q=75 2x`
)
Expand Down Expand Up @@ -591,14 +607,14 @@ function runTests(mode) {
const browser = await webdriver(appPort, '/style-prop')

expect(await browser.elementById('with-styles').getAttribute('style')).toBe(
'border-radius:10px;padding:10px'
'color:transparent;border-radius:10px;padding:10px'
)
expect(
await browser.elementById('with-overlapping-styles').getAttribute('style')
).toBe('width:10px;border-radius:10px;margin:15px')
).toBe('color:transparent;width:10px;border-radius:10px;margin:15px')
expect(
await browser.elementById('without-styles').getAttribute('style')
).toBeNull()
).toBe('color:transparent')
})

if (mode === 'dev') {
Expand Down

0 comments on commit 59a01ec

Please sign in to comment.