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

Fix next/future/image alt text #39366

Merged
merged 10 commits into from Aug 6, 2022
Merged

Fix next/future/image alt text #39366

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
040d2ed
Fix `next/future/image` alt text
styfle Aug 5, 2022
e2768ac
Fix static tests
styfle Aug 5, 2022
1e63b5c
Update onError test
styfle Aug 5, 2022
e4900ec
Fix style so its not present on blur placeholder
styfle Aug 6, 2022
137c4db
Fix other tests checking styles
styfle Aug 6, 2022
f49ea91
Revert static blur test
styfle Aug 6, 2022
dfe85ea
Merge branch 'canary' into show-hide-alt-text-future-image
styfle Aug 6, 2022
7422838
Add missing await
styfle Aug 6, 2022
604cb7b
uno mas
styfle Aug 6, 2022
d34773b
Merge branch 'canary' into show-hide-alt-text-future-image
kodiakhq[bot] Aug 6, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 7 additions & 0 deletions packages/next/client/future/image.tsx
View file
Open in desktop
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
View file
Open in desktop
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