Improve accessibility of video block select poster image. #14752
Conversation
jorgefilipecosta
added
[Focus] Accessibility (a11y)
jorgefilipecosta
requested review from
ajitbohra,
gziolo,
noisysocks,
notnownikki,
Soean,
talldan and
youknowriad
as code owners
| @@ -232,11 +240,21 @@ class VideoEdit extends Component { | |||
| isDefault | |||
| onClick={ open } | |||
| ref={ this.posterImageButton } | |||
| aria-describedby={ `video-block__poster-image-description-${ instanceId }` } | |||
There was a problem hiding this comment.
For simpler maintainability, this id could be assigned to a variable and used in both places.
| > | ||
| { ! this.props.attributes.poster ? __( 'Select Poster Image' ) : __( 'Replace image' ) } | ||
| </Button> | ||
| ) } | ||
| /> | ||
| <p | ||
| id={ `video-block__poster-image-description-${ instanceId }` } | ||
| hidden |
There was a problem hiding this comment.
What's the purpose of using hidden prop here? In other places, we use screen-reader-text class name if the message exists only for screen readers.
There was a problem hiding this comment.
Hi @gziolo, nice question!
Like you said we use screen-reader-text class name if the message exists only for screen readers. When we use screen-reader-text the screen readers still announce the text of the element. In this case, we also want to hide this element from screen readers. This element should not appear on the a11y tree by itself. It is only used when the other element that references it is focused and in that case the description is announced as part of the other element because it references it with aria-describedby.
There was a problem hiding this comment.
I didn’t articulate it good enough. This special class hides an element completely unless it receives focus. In this case this class would be applied to p element which never gets focused through tabbing.
There was a problem hiding this comment.
@joedolson, could you advise what is the best approach? I saw your name in related docs 😃
There was a problem hiding this comment.
Just trying to figure out exactly what this change is doing. It seems like it's adding a description of the image that's currently selected as the poster image - is that correct?
If so, this should be fine. If a hidden element is the target of an aria-describedby or aria-labelledby attribute, that text should be read out by screen readers when they focus on the element that carries the labelling attribute. (In short: being hidden doesn't matter to aria-describedby).
Do I correctly understand what's being done here?
There was a problem hiding this comment.
Yes, it’s correct. Thanks for a quick answer 👍
jorgefilipecosta
force-pushed
the
update/improve-accessibility-of-select-poster-image
branch
from
aaba291
to
cc840ba
Compare
jorgefilipecosta
force-pushed
the
update/improve-accessibility-of-select-poster-image
branch
from
cc840ba
to
d61b99c
Compare
jorgefilipecosta
deleted the
update/improve-accessibility-of-select-poster-image
branch
…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.
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.