WordPress / gutenberg

The Block Editor project for WordPress and beyond. Plugin is available from the official repository.
https://wordpress.org/gutenberg/
Other
10.54k stars 4.21k forks source link

Image block lightbox: do not hide the figcaption with display none #55088

Open afercia opened 1 year ago

afercia commented 1 year ago

Description

Noticed while working on https://github.com/WordPress/gutenberg/pull/55010

When an image has an empty alt attribute and the wrapping figure element does have a figcaption element, the actual image description is determined, both visually and semantically, by the figcaption content.

Screen readers announce the figcaption even when the image is marked as 'decorative' by the means ot an empty alt="". Screenshot when testing with Safari and VoiceOver:

image no alt with caption

When the lightbox is enabled and its modal dialog opens, the figcaption element is hidden via CSS display: none. As such, important information related to the image is not available for all users:

In this case, screen readers don't have anything meaningful to announce, they will only announce the figure element. Screenshot:

image no alt with hidden caption

This is clearly not the best experience for screen reader users. They will be informed that a modal dialog has opened. They will not have any clue what the modal dialog content is other than a figure element.

I'd argue that also visually the figcaption represents important information that should always be visible, also in the modal dialog.

At the very least, the figcaption element should not be hidden with display: none. Instead, it should be visually hidden via the screen-reader-text class or by other means to keep it perceivable by assistive technology.

Step-by-step reproduction instructions

Screenshots, screen recording, code snippet

No response

Environment info

No response

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

artemiomorales commented 1 year ago

It'd be good to get another perspective on this one. Could @joedolson and/or @alexstine could take a look?

Perhaps it might be better in this case to not show the caption in the lightbox so as to prevent redundant information for folks who use screen readers.

In any case, I've tested adding the screen-reader-text class to the caption in the lightbox, but in that scenario, one can't just tab to the caption in Safari or Firefox — one needs to use the key combo control + option + arrow, which passes over both the responsive image and the enlarged image in the process, which also isn't a great experience but I'm not sure how we'd resolve it. Is that what you're seeing @afercia?

If we do want to make the caption available in the lightbox, I think adding the screen-reader-text class would probably be the best approach, but as per the point above, I'm not sure how it could receive focus in order to become visible.

I'll also note that neither VoiceOver on Mac nor the NVDA browser extension announces the caption at all in Chrome.

afercia commented 1 year ago

In any case, I've tested adding the screen-reader-text class to the caption in the lightbox, but in that scenario, one can't just tab to the caption

I'm not sure I fully understand. The caption is just text, it's not expected to be able to tab to it. To navigate content, arrows navigation is the right way.

I'll also note that neither VoiceOver on Mac nor the NVDA browser extension announces the caption at all in Chrome.

You should never test VoiceOver with Chrome. VoiceOver is expected to work best with Safari. I have no idea what the NVDA browser extension is and how it works. Surely it's not a real screen reader and I wouldn't recommend using it. I'd also like to remind everyone that NVDA and other Windows screen reader have a pretty different interaction odel compare to VoiveOver. As such, they must be tested on Windows.

hich passes over both the responsive image and the enlarged image in the process, which also isn't a great experience

The double image is a problem. Instead of overlaying the two images, one of them should be hidden and only revealed when necessary (whilehiding the other one).

afercia commented 1 year ago

While it's a minor issue, the double image is also the reason why Safari + VoiceOver announce 'web dialog with 3 items' when the modal dialog opens:

  1. The close button
  2. The first image
  3. The second image
Screenshot 2023-10-10 at 11 00 54
joedolson commented 1 year ago

One thing I'll observe is that there shouldn't be any legal case for an image with an empty alt and a caption. If an image merits a caption, then it should also merit a description. If the caption is being used to describe the image, then that's misusing the caption.

If the modal shouldn't show the caption visibly, then the simplest resolution is to add the related figcaption text into the alt attribute if the alt attribute is empty.

If the figcaption is going to be made available to a screen reader while the lightbox is open, I think it should also be visible.

Also agree that if an image is hidden, then it should be completely hidden, not just visually hidden.

artemiomorales commented 1 year ago

The double image is a problem. Instead of overlaying the two images, one of them should be hidden and only revealed when necessary (whilehiding the other one).

While it's a minor issue, the double image is also the reason why Safari + VoiceOver announce 'web dialog with 3 items' when the modal dialog opens:

  1. The close button
  2. The first image
  3. The second image

Ok got it, I've created an issue for this.

artemiomorales commented 1 year ago

If the modal shouldn't show the caption visibly, then the simplest resolution is to add the related figcaption text into the alt attribute if the alt attribute is empty.

This approach sounds good to me, although it seems this solution seems somewhat at odds with https://github.com/WordPress/gutenberg/pull/43059, which uses aria-describedby if an image has no alt but does have a caption.

We could try taking the aria-describedby approach instead, but I believe we should avoid showing the caption visibly in the lightbox if possible — and as @joedolson points out, if we reference the figcaption using aria-describedby, then perhaps it should be visible.

What if we were to use aria-describedby in the body content, but put the caption into the image alt inside the lightbox?

Pinging @carolinan for feedback.

CC @afercia

afercia commented 1 year ago

but I believe we should avoid showing the caption visibly in the lightbox if possible

I think that would be a little too much opinionated. On one hand, the editor aims to give users full power on building their site and posts exactly how they want them to look. On the other hand, the Lighbox look and feel is opinionated and there are no settings to change its appearance. The Navigation component, at least, has a setting to change icons to text. The lighbox should have settings to let users decide whether they want the caption to be visible in the dialog or not.

afercia commented 1 year ago

One thing I'll observe is that there shouldn't be any legal case for an image with an empty alt and a caption. If an image merits a caption, then it should also merit a description. If the caption is being used to describe the image, then that's misusing the caption.

@joedolson should we revise any of the work done a few years ago in core with the image caption? See https://core.trac.wordpress.org/ticket/27402 https://core.trac.wordpress.org/ticket/34595