Add aria-describedby in the featured image preview #14593
Conversation
Jackie6
requested review from
noisysocks,
talldan and
youknowriad
as code owners
gziolo
added
[Focus] Accessibility (a11y)
Jackie6
force-pushed
the
update/featured-img
branch
from
dc253d0 to
5fafe97
Compare
## Description Related in #14416 we have a similar issue but for the featured image that issue is being addressed on #14593. Here the solution is not as perfect, as we don't have the alt text of the poster image only the URL was saved in the attributes. ## How has this been tested? I used the voice over utility and I checked that a descriptive message is announced after the select/replace poster image button gains focus.
…14752) ## Description Related in WordPress#14416 we have a similar issue but for the featured image that issue is being addressed on WordPress#14593. Here the solution is not as perfect, as we don't have the alt text of the poster image only the URL was saved in the attributes. ## How has this been tested? I used the voice over utility and I checked that a descriptive message is announced after the select/replace poster image button gains focus.
| if ( media ) { | ||
| const mediaSize = applyFilters( 'editor.PostFeaturedImage.imageSize', 'post-thumbnail', media.id, currentPostId ); | ||
| if ( has( media, [ 'media_details', 'sizes', mediaSize ] ) ) { | ||
| mediaWidth = media.media_details.sizes[ mediaSize ].width; | ||
| mediaHeight = media.media_details.sizes[ mediaSize ].height; | ||
| mediaSourceUrl = media.media_details.sizes[ mediaSize ].source_url; | ||
| mediaAltText = media.media_details.sizes[ mediaSize ].alt_text; |
There was a problem hiding this comment.
I'm not sure I understand how the PostFeaturedImage works (@gziolo is this still working?), however seems to me alt_text is never a property of media.media_details.sizes[ mediaSize ] and instead it's always media.alt_text. If I'm correct, this will fail and will use the wrong description for images that actually do have an alt text.
| hidden | ||
| > | ||
| { mediaAltText ? | ||
| sprintf( __( 'Current image: %s' ), mediaAltText ) : |
There was a problem hiding this comment.
Should use a translators comment /* translators: %s: image alt text */
| > | ||
| { mediaAltText ? | ||
| sprintf( __( 'Current image: %s' ), mediaAltText ) : | ||
| __( 'The current image has no alternative text. The file name is: ' + imageFilename ) |
There was a problem hiding this comment.
Instead of concatenation, should use sprintf() and a translator comment /* translators: %s: image file name */
There was a problem hiding this comment.
Thanks @Jackie6 and sorry for the late review.
I left some comments regarding a few points that seems to me should be addressed.
Also, a couple more issues:
1
When setting an image and the media modal closes, focus should be moved back to the button that was previously focused. This can be either the Set featured image button (which turns in the preview button) or the Replace Image one, depending on the workflow. On master, this seems to work correctly. On this branch seems it isn't.
I guess that instead of adding one more MediaUploadCheck, we should try to reduce their number to avoid the component is unmounted and remounted. See for example a similar issue in #14415
2
On master, there's some margin between the image and the Replace button (see also the focus indication around the image when the modal closes):
On this branch, there's no margin (and focus is not moved back):
|
@afercia Thanks for the comments. Will do it next week! |
|
Closing as superseded by #24888. |
Description
Fix #14416
How has this been tested?
Types of changes
Add
aria-describedbyin the image button of the feature image preview.aria-describedbyshould beCurrent image: altTextaria-describedbyshould beThe current image has no alternative text. The file name is: fileNameChecklist: