Remove `visibility: hidden` from the image component #23278

shuding · 2021-03-22T16:54:38Z

This PR removes the visibility style property change from next/image. It was previously added in #18195 to fix a bug that when no src is set, and that bug is not valid anymore as all images will always have src (and a fallback too).

It also fixes the problem that screen readers ignore elements with visibility: hidden.

Fixes #23201.

Bug

Related issues VoiceOver support in next/image #23201
Integration tests added

Feature

Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR.
Related issues linked using fixes #number
Integration tests added
Documentation added
Telemetry added. In case of a feature if it's used or not.

Documentation / Examples

Make sure the linting passes

ijjk · 2021-03-22T17:26:24Z

Stats from current PR

Default Server Mode (Increase detected ⚠️)

General Overall decrease ✓

	vercel/next.js canary	shuding/next.js fix-image-with-screen-reader	Change
buildDuration	10.9s	9.9s	-952ms
nodeModulesSize	43.6 MB	43.6 MB	-153 B

Page Load Tests Overall increase ✓

	vercel/next.js canary	shuding/next.js fix-image-with-screen-reader	Change
/ failed reqs	0	0	✓
/ total time (seconds)	1.866	1.776	-0.09
/ avg req/sec	1340.1	1407.86	+67.76
/error-in-render failed reqs	0	0	✓
/error-in-render total time (seconds)	1.065	1.052	-0.01
/error-in-render avg req/sec	2346.69	2376.6	+29.91

Client Bundles (main, webpack, commons)

	vercel/next.js canary	shuding/next.js fix-image-with-screen-reader	Change
677f882d2ed8..a2e7.js gzip	13.4 kB	13.4 kB	✓
framework.HASH.js gzip	39 kB	39 kB	✓
main-HASH.js gzip	7.14 kB	7.14 kB	✓
webpack-HASH.js gzip	751 B	751 B	✓
Overall change	60.3 kB	60.3 kB	✓

Legacy Client Bundles (polyfills)

	vercel/next.js canary	shuding/next.js fix-image-with-screen-reader	Change
polyfills-HASH.js gzip	31.3 kB	31.3 kB	✓
Overall change	31.3 kB	31.3 kB	✓

Client Pages

	vercel/next.js canary	shuding/next.js fix-image-with-screen-reader	Change
_app-fde3324..9dd1.js gzip	1.28 kB	1.28 kB	✓
_error-af59f..582f.js gzip	3.46 kB	3.46 kB	✓
amp-9716187d..0aa8.js gzip	536 B	536 B	✓
hooks-107e90..74c7.js gzip	888 B	888 B	✓
index-ac435c..ecf2.js gzip	227 B	227 B	✓
link-e6cee42..49eb.js gzip	1.64 kB	1.64 kB	✓
routerDirect..dc9d.js gzip	303 B	303 B	✓
withRouter-6..0e02.js gzip	302 B	302 B	✓
Overall change	8.64 kB	8.64 kB	✓

Client Build Manifests

	vercel/next.js canary	shuding/next.js fix-image-with-screen-reader	Change
_buildManifest.js gzip	346 B	346 B	✓
Overall change	346 B	346 B	✓

Rendered Page Sizes

	vercel/next.js canary	shuding/next.js fix-image-with-screen-reader	Change
index.html gzip	612 B	612 B	✓
link.html gzip	621 B	621 B	✓
withRouter.html gzip	608 B	608 B	✓
Overall change	1.84 kB	1.84 kB	✓

Serverless Mode (Decrease detected ✓)

General Overall decrease ✓

	vercel/next.js canary	shuding/next.js fix-image-with-screen-reader	Change
buildDuration	11.5s	11.4s	-107ms
nodeModulesSize	43.6 MB	43.6 MB	-153 B

Client Bundles (main, webpack, commons)

	vercel/next.js canary	shuding/next.js fix-image-with-screen-reader	Change
677f882d2ed8..a2e7.js gzip	13.4 kB	13.4 kB	✓
framework.HASH.js gzip	39 kB	39 kB	✓
main-HASH.js gzip	7.14 kB	7.14 kB	✓
webpack-HASH.js gzip	751 B	751 B	✓
Overall change	60.3 kB	60.3 kB	✓

Legacy Client Bundles (polyfills)

	vercel/next.js canary	shuding/next.js fix-image-with-screen-reader	Change
polyfills-HASH.js gzip	31.3 kB	31.3 kB	✓
Overall change	31.3 kB	31.3 kB	✓

Client Pages

	vercel/next.js canary	shuding/next.js fix-image-with-screen-reader	Change
_app-fde3324..9dd1.js gzip	1.28 kB	1.28 kB	✓
_error-af59f..582f.js gzip	3.46 kB	3.46 kB	✓
amp-9716187d..0aa8.js gzip	536 B	536 B	✓
hooks-107e90..74c7.js gzip	888 B	888 B	✓
index-ac435c..ecf2.js gzip	227 B	227 B	✓
link-e6cee42..49eb.js gzip	1.64 kB	1.64 kB	✓
routerDirect..dc9d.js gzip	303 B	303 B	✓
withRouter-6..0e02.js gzip	302 B	302 B	✓
Overall change	8.64 kB	8.64 kB	✓

Client Build Manifests

	vercel/next.js canary	shuding/next.js fix-image-with-screen-reader	Change
_buildManifest.js gzip	346 B	346 B	✓
Overall change	346 B	346 B	✓

Serverless bundles

	vercel/next.js canary	shuding/next.js fix-image-with-screen-reader	Change
_error.js	1.02 MB	1.02 MB	✓
404.html	2.67 kB	2.67 kB	✓
500.html	2.65 kB	2.65 kB	✓
amp.amp.html	10.6 kB	10.6 kB	✓
amp.html	1.86 kB	1.86 kB	✓
hooks.html	1.92 kB	1.92 kB	✓
index.js	1.02 MB	1.02 MB	✓
link.js	1.08 MB	1.08 MB	✓
routerDirect.js	1.07 MB	1.07 MB	✓
withRouter.js	1.07 MB	1.07 MB	✓
Overall change	5.27 MB	5.27 MB	✓

Webpack 5 Mode (Increase detected ⚠️)

General Overall decrease ✓

	vercel/next.js canary	shuding/next.js fix-image-with-screen-reader	Change
buildDuration	11.4s	11.6s	⚠️ +144ms
nodeModulesSize	43.6 MB	43.6 MB	-153 B

Page Load Tests Overall increase ✓

	vercel/next.js canary	shuding/next.js fix-image-with-screen-reader	Change
/ failed reqs	0	0	✓
/ total time (seconds)	1.758	1.723	-0.03
/ avg req/sec	1422.36	1450.9	+28.54
/error-in-render failed reqs	0	0	✓
/error-in-render total time (seconds)	1.093	1.095	0
/error-in-render avg req/sec	2288.08	2283.18	⚠️ -4.9

Client Bundles (main, webpack, commons)

	vercel/next.js canary	shuding/next.js fix-image-with-screen-reader	Change
597-12fee761..569d.js gzip	13.3 kB	13.3 kB	✓
framework.HASH.js gzip	39.3 kB	39.3 kB	✓
main-HASH.js gzip	7.11 kB	7.11 kB	✓
webpack-HASH.js gzip	993 B	993 B	✓
Overall change	60.7 kB	60.7 kB	✓

Legacy Client Bundles (polyfills)

	vercel/next.js canary	shuding/next.js fix-image-with-screen-reader	Change
polyfills-HASH.js gzip	31.1 kB	31.1 kB	✓
Overall change	31.1 kB	31.1 kB	✓

Client Pages

	vercel/next.js canary	shuding/next.js fix-image-with-screen-reader	Change
_app-b5edc60..e5c2.js gzip	1.29 kB	1.29 kB	✓
_error-b58c1..9b8e.js gzip	3.4 kB	3.4 kB	✓
amp-89a5460c..567f.js gzip	558 B	558 B	✓
hooks-8c2e74..be37.js gzip	924 B	924 B	✓
index-fec729..83b2.js gzip	243 B	243 B	✓
link-3631090..4cab.js gzip	1.65 kB	1.65 kB	✓
routerDirect..20d7.js gzip	328 B	328 B	✓
withRouter-b..a4b9.js gzip	326 B	326 B	✓
Overall change	8.72 kB	8.72 kB	✓

Client Build Manifests

	vercel/next.js canary	shuding/next.js fix-image-with-screen-reader	Change
_buildManifest.js gzip	325 B	325 B	✓
Overall change	325 B	325 B	✓

Rendered Page Sizes

	vercel/next.js canary	shuding/next.js fix-image-with-screen-reader	Change
index.html gzip	585 B	585 B	✓
link.html gzip	594 B	594 B	✓
withRouter.html gzip	582 B	582 B	✓
Overall change	1.76 kB	1.76 kB	✓

Diffs

Diff for index.html

@@ -43,7 +43,7 @@
         "props": { "pageProps": {} },
         "page": "/",
         "query": {},
-        "buildId": "fdbrIgF_p7InaqGdKlCTG",
+        "buildId": "7We7k9taV3UmmVa58KO6r",
         "isFallback": false,
         "gip": true
       }
@@ -77,11 +77,11 @@
       async=""
     ></script>
     <script
-      src="/_next/static/fdbrIgF_p7InaqGdKlCTG/_buildManifest.js"
+      src="/_next/static/7We7k9taV3UmmVa58KO6r/_buildManifest.js"
       async=""
     ></script>
     <script
-      src="/_next/static/fdbrIgF_p7InaqGdKlCTG/_ssgManifest.js"
+      src="/_next/static/7We7k9taV3UmmVa58KO6r/_ssgManifest.js"
       async=""
     ></script>
   </body>

Diff for link.html

@@ -48,7 +48,7 @@
         "props": { "pageProps": {} },
         "page": "/link",
         "query": {},
-        "buildId": "fdbrIgF_p7InaqGdKlCTG",
+        "buildId": "7We7k9taV3UmmVa58KO6r",
         "isFallback": false,
         "gip": true
       }
@@ -82,11 +82,11 @@
       async=""
     ></script>
     <script
-      src="/_next/static/fdbrIgF_p7InaqGdKlCTG/_buildManifest.js"
+      src="/_next/static/7We7k9taV3UmmVa58KO6r/_buildManifest.js"
       async=""
     ></script>
     <script
-      src="/_next/static/fdbrIgF_p7InaqGdKlCTG/_ssgManifest.js"
+      src="/_next/static/7We7k9taV3UmmVa58KO6r/_ssgManifest.js"
       async=""
     ></script>
   </body>

Diff for withRouter.html

@@ -43,7 +43,7 @@
         "props": { "pageProps": {} },
         "page": "/withRouter",
         "query": {},
-        "buildId": "fdbrIgF_p7InaqGdKlCTG",
+        "buildId": "7We7k9taV3UmmVa58KO6r",
         "isFallback": false,
         "gip": true
       }
@@ -77,11 +77,11 @@
       async=""
     ></script>
     <script
-      src="/_next/static/fdbrIgF_p7InaqGdKlCTG/_buildManifest.js"
+      src="/_next/static/7We7k9taV3UmmVa58KO6r/_buildManifest.js"
       async=""
     ></script>
     <script
-      src="/_next/static/fdbrIgF_p7InaqGdKlCTG/_ssgManifest.js"
+      src="/_next/static/7We7k9taV3UmmVa58KO6r/_ssgManifest.js"
       async=""
     ></script>
   </body>

Commit: 276b285

This is a follow-up PR of #19052, where `visibility: inherit` was mistakenly added back. It was removed in #23278. ## Bug - [ ] Related issues linked using `fixes #number` - [ ] Integration tests added ## Feature - [ ] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR. - [ ] Related issues linked using `fixes #number` - [ ] Integration tests added - [ ] Documentation added - [ ] Telemetry added. In case of a feature if it's used or not. ## Documentation / Examples - [ ] Make sure the linting passes

This PR removes the `visibility` style property change from next/image. It was previously added in vercel#18195 to fix a bug that when no `src` is set, and that bug is not valid anymore as all images will always have `src` (and a fallback too). It also fixes the problem that screen readers ignore elements with `visibility: hidden`. Fixes vercel#23201. ## Bug - [x] Related issues vercel#23201 - [ ] Integration tests added ## Feature - [ ] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR. - [ ] Related issues linked using `fixes #number` - [ ] Integration tests added - [ ] Documentation added - [ ] Telemetry added. In case of a feature if it's used or not. ## Documentation / Examples - [ ] Make sure the linting passes

This is a follow-up PR of vercel#19052, where `visibility: inherit` was mistakenly added back. It was removed in vercel#23278. ## Bug - [ ] Related issues linked using `fixes #number` - [ ] Integration tests added ## Feature - [ ] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR. - [ ] Related issues linked using `fixes #number` - [ ] Integration tests added - [ ] Documentation added - [ ] Telemetry added. In case of a feature if it's used or not. ## Documentation / Examples - [ ] Make sure the linting passes

This PR removes the `visibility` style property change from next/image. It was previously added in vercel#18195 to fix a bug that when no `src` is set, and that bug is not valid anymore as all images will always have `src` (and a fallback too). It also fixes the problem that screen readers ignore elements with `visibility: hidden`. Fixes vercel#23201. ## Bug - [x] Related issues vercel#23201 - [ ] Integration tests added ## Feature - [ ] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR. - [ ] Related issues linked using `fixes #number` - [ ] Integration tests added - [ ] Documentation added - [ ] Telemetry added. In case of a feature if it's used or not. ## Documentation / Examples - [ ] Make sure the linting passes

This is a follow-up PR of vercel#19052, where `visibility: inherit` was mistakenly added back. It was removed in vercel#23278. ## Bug - [ ] Related issues linked using `fixes #number` - [ ] Integration tests added ## Feature - [ ] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR. - [ ] Related issues linked using `fixes #number` - [ ] Integration tests added - [ ] Documentation added - [ ] Telemetry added. In case of a feature if it's used or not. ## Documentation / Examples - [ ] Make sure the linting passes

remove visibility-hidden

26164bc

ijjk added the type: next label Mar 22, 2021

This comment has been minimized.

Sign in to view

shuding marked this pull request as ready for review March 22, 2021 17:07

shuding requested review from divmain, ijjk, lfades and timneutkens as code owners March 22, 2021 17:07

shuding requested a review from styfle March 22, 2021 17:07

This comment has been minimized.

Sign in to view

shuding marked this pull request as draft March 22, 2021 17:11

styfle previously approved these changes Mar 22, 2021

View reviewed changes

remove visibility tests

276b285

shuding dismissed styfle’s stale review via 276b285 March 22, 2021 17:17

shuding marked this pull request as ready for review March 22, 2021 17:17

styfle approved these changes Mar 22, 2021

View reviewed changes

ijjk approved these changes Mar 22, 2021

View reviewed changes

kodiakhq bot merged commit c8680a3 into vercel:canary Mar 22, 2021

shuding deleted the fix-image-with-screen-reader branch March 23, 2021 09:09

styfle mentioned this pull request Mar 23, 2021

Add fallback for <Image /> component when JavaScript is disabled in browser #19052

Merged

shuding mentioned this pull request Mar 23, 2021

Remove visibility: inherit from the image component #23329

Merged

8 tasks

vaypoldev mentioned this pull request Jun 26, 2021

Problem with next/image component in NextJS 10.0.10+ and Preact #26621

Closed

vercel locked as resolved and limited conversation to collaborators Jan 28, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove `visibility: hidden` from the image component #23278

Remove `visibility: hidden` from the image component #23278

shuding commented Mar 22, 2021

This comment has been minimized.

This comment has been minimized.

ijjk commented Mar 22, 2021

Diffs

Remove visibility: hidden from the image component #23278

Remove visibility: hidden from the image component #23278

Conversation

shuding commented Mar 22, 2021

Bug

Feature

Documentation / Examples

This comment has been minimized.

This comment has been minimized.

ijjk commented Mar 22, 2021

Stats from current PR

Diffs

Remove `visibility: hidden` from the image component #23278

Remove `visibility: hidden` from the image component #23278