Shs 5134 photo album aria #1404
Conversation
There was a problem hiding this comment.
I think this approach works, but there are a couple of things that can be improved:
-
The variable
attributes['data-cbox-img-attrs']is a JSON string, it contains special chars and extra text that is included in the alt text and read by the screen reader. This can confuse end users:
It should be possible to parse this string and extract the alt text itself.
-
I think that the labels can be more descriptive. The purpose of the links is not clear, there's not descriptive text anywhere indicating that this is a gallery. While this is also a content issue, we could create links that provide a better description of the purpose. For example instead of adding just the alt text in the
aria-label, we could set the text to be "Open [ALT] image in the gallery" (or something similar). We could just use "Open image in the gallery" for images without an alt text.
Also, it would be good to check with Tori if the goal is to have users to set correct alt text for all images. Editoria11y reports the missing alt texts, but only if the aria-label is not present. Maybe it would be better to leave the aria-label out and use the error report as an incentive for editors to fix all the missing alt texts.
docroot/themes/humsci/humsci_basic/templates/contrib/colorbox-formatter.html.twig
Outdated
Show resolved
Hide resolved
|
@cienvaras I've made those changes -- should I ask Tori about her preference now or after we send them the changes to review? |
|
Also, I removed the "caption/credit" possibility. Describing what will happen if clicked seems to trump a description that might end up just being a photo credit. |
docroot/themes/humsci/humsci_basic/templates/contrib/colorbox-formatter.html.twig
Outdated
Show resolved
Hide resolved
| {% set link_label = "Open " ~ attributes['data-cbox-img-attrs'] ~ " image in the gallery" %} | ||
| {% endif %} | ||
| {% set link_label = link_label|raw %} | ||
| {% set link_label = link_label|replace({'{"alt":"':'', '"}':''}) %} |
There was a problem hiding this comment.
This works, but checking the colorbox module code I found that data-cbox-img-attrs can include more attributes, not only alt. To prevent weird values, you can get the alt text from the image variable:
{% set alt_text = image['#alt'] %}
That would also make the use of the |raw filter unnecessary.
There was a problem hiding this comment.
Oh my, you are right, we don't need that stuff anymore. Thanks!
READY FOR REVIEW
Summary
This PR makes adjustments to how the colorbox gallery pulls in attributes for identifying link purposes.
Urgency
medium
Steps to Test
aria-titleattribute. This attribute should be (in this order): the alt text of the image if it isn't blank, the caption if it isn't blank, or generic text "Thumbnail Image" if neither of those exist.I realize this strays from the original AC. But I'm realizing that the problem here is that the aria-title exists because sometimes the images don't have alt text. And that's a problem for describing the to the user what they are clicking on and why. At the same time, there's some sort of bug with the colorbox templates where even if there's no alt text on the image, it pulls in some weird additional markup (
{"alt":""}or something like that), so there won't ever be an error saying "hey, there's no alt text here."I tried to approach this with giving best to worst aria-title text possible to the link. Maybe the generic text could be improved. Please let me know what you think!
PR Checklist