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

VoiceOver support in next/image #23201

Closed
shanly-suepaul opened this issue Mar 18, 2021 · 3 comments · Fixed by #23278
Closed

VoiceOver support in next/image #23201

shanly-suepaul opened this issue Mar 18, 2021 · 3 comments · Fixed by #23278
Assignees
@shuding
Milestone
Iteration 18

Comments

@shanly-suepaul
Copy link

What version of Next.js are you using?

10.0.6

What version of Node.js are you using?

12.20.1

What browser are you using?

Chrome

What operating system are you using?

macOS 11

How are you deploying your application?

next start + Vercel

Describe the Bug

When using next/image images that are below the fold, the images are loaded using visibility: none. This causes screenreaders to ignore the element and not read it, even though scrolling the element into the viewport would display the element for sighted users.

Expected Behavior

VoiceOver should read and scroll to next/image images below the fold.

To Reproduce

See repo here: https://github.com/myplanetdigital/voiceover-next-image-bug and deployment here: https://voiceover-next-image-shanly-myplanet.vercel.app/

Steps to reproduce:

  1. Go to the deployment with VoiceOver on
  2. Try to navigate using the voiceover cursor (VO+left, VO+right)
  3. Note that the UL is announced as having four items, but only one (the one above the fold) is read.

I believe this is due to the use of visibility: hidden, which prevents screen readers from parsing an element. I don't understand the purpose of that rule as the images are already defaulting to lazy loading.
@shanly-suepaul shanly-suepaul added the bug Issue was opened via the bug report template. label Mar 18, 2021
@shuding shuding self-assigned this Mar 19, 2021
@timneutkens timneutkens added kind: bug and removed bug Issue was opened via the bug report template. labels Mar 19, 2021
@timneutkens timneutkens added this to the Iteration 18 milestone Mar 19, 2021
@nersoh
Copy link

nersoh commented Mar 19, 2021

Maybe opacity property could be applied instead?

@shuding
Copy link
Member

shuding commented Mar 22, 2021

This is indeed caused by visibility: hidden which was introduced in the old logic and it's safe to remove it now.

@shuding shuding mentioned this issue Mar 22, 2021
Merged
8 tasks
@kodiakhq kodiakhq bot closed this as completed in #23278 Mar 22, 2021
kodiakhq bot pushed a commit that referenced this issue Mar 22, 2021
@shuding
Remove visibility: hidden from the image component (#23278)
c8680a3 
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

- [x] Related issues #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
SokratisVidros pushed a commit to SokratisVidros/next.js that referenced this issue Apr 20, 2021
@shuding @SokratisVidros
Remove visibility: hidden from the image component (vercel#23278)
e0f3b70 
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
flybayer pushed a commit to blitz-js/next.js that referenced this issue Apr 29, 2021
@shuding
Remove visibility: hidden from the image component (vercel#23278)
e8bb164 
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
@balazsorban44
Copy link
Member

This issue has been automatically locked due to no recent activity. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@vercel vercel locked as resolved and limited conversation to collaborators Jan 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@shuding shuding

Labels
None yet
Projects
None yet
Milestone
Iteration 18
Development

Successfully merging a pull request may close this issue.

Remove visibility: hidden from the image component shuding/next.js
5 participants
@shanly-suepaul @nersoh @shuding @timneutkens @balazsorban44