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.
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
Gallery: Add labels to img, figure and figcaption elements for accessibility #25560
Conversation
|
Size Change: +499 B (0%) Total Size: 1.17 MB
ℹ️ View Unchanged
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested with VoiceOver on Safari and tried various combinations of images with and without alt, and with and without caption.
In all cases I felt enough information was being announced 👍
|
Tested this PR with both VoiceOver/Safari and NVDA/Firefox and found that I've pushed a commit replacing it with a conditional |
|
Thanks, @tellthemachines! Tested on VoiceOver and this is what I found: 1. If the image is a link and only has a caption 2. If the image is a link and only has alt text 3. If the image is a link and has both alt text and a caption 4. If the image is a link and has no alt text nor a caption I think this is feeling in good shape with the exception of #1. I don't think the caption should be announced twice. What do you think? |
Hmm, I tried adding I can't think of any other way to avoid the repetition in VoiceOver, and having to choose between the two, I'd rather optimise for NVDA as it's the more popular screen reader. |
Co-authored-by: Zebulan Stanphill <zebulanstanphill@protonmail.com>
Co-authored-by: Zebulan Stanphill <zebulanstanphill@protonmail.com>
This reverts commit d72429a.
f0f9a35
to
b34cfed
Compare
|
@tellthemachines I agree, definitively not a blocker and much better than what we had before. Thank you! |
|
I'm not able to review at the moment, but I'd like to dismiss my previous review. How do I do that? I can't find any button to do so. |
It has been addressed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code looks good and the block migration worked in my testing 👍
|
Did we consider adding these server side instead of serializing? |
|
Thanks for this PR! Did we consider to not use On the left: broken image with no alt text and with aria-label The alt text is always preferable as it can help also sighted users. Also, we should consider the first rule of ARIA use. /Cc @joedolson |
@afercia I did not think of that scenario, thanks for raising it. Wouldn't the presence of the caption be enough? |
|
If we aim to revert this can we do it before 9.2 is released to avoid extra deprecations? |
@enriquesanchez my point is more about why to use an aria-label in that scenario when it's possible to populate the alt text? Usage of ARIA should be avoided when there's an equivalent HTML element or attribute that can be used. |
…ibility (#25560) * Add labels to img, figure and figcaption elements to make captions more accessible * change aria-label to aria-describedby * Update packages/block-library/src/gallery/gallery-image.js Co-authored-by: Zebulan Stanphill <zebulanstanphill@protonmail.com> * Update packages/block-library/src/gallery/gallery-image.js Co-authored-by: Zebulan Stanphill <zebulanstanphill@protonmail.com> * Replace aria-describedby with conditional aria-label. * Avoid caption repetition. * Revert "Avoid caption repetition." This reverts commit d72429a. * Add deprecation. * Remove unnecessary Id. Co-authored-by: Zebulan Stanphill <zebulanstanphill@protonmail.com> Co-authored-by: tellthemachines <isabel@tellthemachines.com>
…ibility (#25560) * Add labels to img, figure and figcaption elements to make captions more accessible * change aria-label to aria-describedby * Update packages/block-library/src/gallery/gallery-image.js Co-authored-by: Zebulan Stanphill <zebulanstanphill@protonmail.com> * Update packages/block-library/src/gallery/gallery-image.js Co-authored-by: Zebulan Stanphill <zebulanstanphill@protonmail.com> * Replace aria-describedby with conditional aria-label. * Avoid caption repetition. * Revert "Avoid caption repetition." This reverts commit d72429a. * Add deprecation. * Remove unnecessary Id. Co-authored-by: Zebulan Stanphill <zebulanstanphill@protonmail.com> Co-authored-by: tellthemachines <isabel@tellthemachines.com>





Fixes #13164
Description
This adds some attributes to the Gallery block markup for accessibility reasons:
aria-describedbyattribute to thefigureelement to declare what element contains the captionidto the correspondingfigcaptionto associate it with the figurearia-labelattribute on the image, as a fallback in case the describedby attribute doesn't worktitleattribute on the link, as a fallback in case the describedby attribute doesn't workHow has this been tested?
Screenshots
Types of changes
Checklist: