Umami Theme - a11y context is needed for read more links

Don't proceed with this for now. We're in some doubt as to whether this is required for WCAG 2.0 SC 2.4.4 Link Purpose (in context) level A, or not.

The WCAG 2.0 "known failures" document F63: Failure of Success Criterion 2.4.4 due to providing link context only in content that is not related to the link strongly suggests that what we have already is sufficient, because a preceding heading provides context for the link (emphasis mine):

Locate links where some additional link context is needed to understand the purpose of the link. For each link:

1.Check whether the context is contained in the same sentence, paragraph, list item, table cell, associated table headers, or preceding heading.

However the WCAG 2.0 recommendation does NOT say that a preceeding heading is sufficient, so we have a discrepancy in the WCAG documents that could do with clarification.

Note that the standard D8 core "read more" links rendered on node teasers DO have a longer accessible name ("Read more about my amazing article") using a visually-hidden span. In that case, we're using the generic node template, and there is assumed to be quite a lot of content between the heading and read-more link (e.g. the node body>summary, tags, image, etc). By the time a reader reaches the read-more link, the heading was quite a way back. I think the extra context makes sense here.

However in the Umami profile's recipe listing view, the read-more link is immediately after the heading. Adding extra context to the link could be needless verbosity.

Aside: when filing these OOTB follow-up issues, please be careful to check if we already have an issue open over at GitHub. I'm worried the discussion will become confusing if it is split over two places, and harder for new contributors to get on board. Once the OOTB MVP is in core, I'd prefer to see D.O issues take over.

@DuaelFr needs a credit here for reporting it in the GitHub issue originally.

Yes, I'm also really not sure why there is this extra link given that the title is immediately above it. There in fact 3 links for every image which kinda makes this annoying for any keyboard only user. There's the yummy image, the article title & View recipe links.

Shouldn't we strive to just put it all in one link sorta like this:

<article data-quickedit-entity-id="node/11" role="article" class="contextual-region node node--type-recipe node--promoted node--view-mode-highlighted-bottom" about="/recipes/thai-green-curry" data-quickedit-entity-instance-id="1">
  <a href="/recipes/thai-green-curry">
    <div class="node__content">
        <div data-quickedit-field-id="node/11/field_image/en/highlighted_bottom" class="field field--name-field-image field--type-image field--label-hidden field__item"><img srcset="/sites/default/files/styles/medium_3_2_600x400/public/thai-green-curry-umami.jpg?itok=CboNznCi 600w, /sites/default/files/styles/large_3_2_768x512/public/thai-green-curry-umami.jpg?itok=bFpAYv3I 768w, /sites/default/files/styles/medium_3_2_2x/public/thai-green-curry-umami.jpg?itok=wxY9YGKi 1200w, /sites/default/files/styles/large_3_2_2x/public/thai-green-curry-umami.jpg?itok=NjcQ--fQ 1536w" sizes="100vw" src="/sites/default/files/styles/large_3_2_768x512/public/thai-green-curry-umami.jpg?itok=bFpAYv3I" alt="A traditional bowl of creamy, aromatic Thai green curry with chunks of chicken and a small bowl of jasmine rice." typeof="foaf:Image">
      </div>
     <div class="node__meta">      
        <h2 class="node__title">Thai green curry</h2>
        <div class="read-more">View recipe</div>
    </div>
  </a>
</article>

Then we don't have to worry about inserting the additional context. Heck, that alt text sounds better than the picture looks. Great work! I'm hungry now.

There are a few approaches to reduce the number of links here. @duaelfr has an approach on one of the Umami github issues, we're still moving across to d.o

I reported and expanded upon the duplicate link issue here: https://github.com/lauriii/umami/issues/146 which is slightly different from the original "read more" issue that this ticket covers.

The card teasers on this site include three links to the same URL. This is considered an alert by WCAG (2.4.4 Link Purpose - Level A).

When adjacent links go to the same location (such as a linked product image and an adjacent linked product name that go to the same product page) this results in additional navigation and repetition for keyboard and screen reader users.

If possible, combine the redundant links into one link and remove any redundant text or alternative text (for example, if a product image and product name are in the same link, the image can usually be given alt="").

See http://a11y-style-guide.com/style-guide/section-general.html#kssref-gene... for some "read more" examples to avoid one redundancy issue. Might consider making the entire card teaser a link or removing the link off of the image.

Update:

Then {{ 'View'|t }} {{ node.bundle }} construct identified as problematic in node--highlighted-bottom.html.twig is now in node--card-common.html .

The image is no longer a link so there are not three links to tab through for each listed node.

We need to decide whether to keep the View bundle link - if so we need to consider accessibility context, and wrapping the entire card as a single item. Or if this is redundant due to the hyperlink to the article in close proximity as suggested in #5 by @mgifford, we should simply remove it.

Discussed with @markconroy and @eli-t at DrupalEurope. Propose:

Remove link around titles.
Keep the View bundle link
For accessibility, use the following markup on each of the View bundle link’s in order to add context for screen reader users only:

<a class="read-more__link" href="{{ url }}">{{ 'View'|t }} {{ node.bundle }}<span class="visually-hidden">{{ label }}</span></a>

@kjay yes this issue is still worth addressing. I think it would be good to have the link purpose differentiated for these. That's good for WCAG "Link purpose" (success criteria 2.4.4 and 2.4.9, at level A and AAA respectively). Setting status back to active.

WCAG is ambiguous (annoyingly) about whether a standalone link gets programmatic context from the preceding heading, which I said in #2. So I think it's best to do this, and consider it necessary for WCAG level A.

Where are we with the wider plan of reducing template customization, so the view mode "manage display" settings are better respected? Because we could get the link context for free if we can use the existing read-more links provided by node module. If we can find a way to customize that link text in a preprocess hook (per bundle/view mode), that would be better than putting a custom link in a Twig template IMO.

Review of the snippet in #10:

1. Markup-wise, this is fine for accessibility.
2. The phrasing could be improved. Currently this will result in "View Recipe Fiery Chilli Sauce" as one utterance, without punctuation or prepositions. I reckon a colon/dash in the hidden text could improve this, say "VIew Recipe - Fiery Chilli Sauce". Then text-to-speech engines can put a more appropriate pause in, according to their own language rules.
3. Currently "View" is the only translatable word. It's better to treat the whole phrase as a translatable. So twig can do something like this:
   

   <a class="read-more__link" href="{{ url }}">{%- trans -%}
      View {{ node.bundle }} - <span class="visually-hidden">{{ label }}</span>
   {%- end trans -%}
   </a>

Important: Under WCAG 2.1, this needs to satisfy the new SC 2.5.3 Label in Name too. Basically, if the visible link name is "View recipe", the visually-hidden text can't come in between these two visible words. Speech control users would suffer. The snippet in #10 passes, as does the read-more link from node module. However View <span class="visually-hidden">Fiery Chilli Sauce</span> Recipe would fail. I'm catching this one early - I still need to do an assessment of this for every bit of visually-hidden text in the whole of of core!

Removing the needs accessibility review, now there's a plan from #10-11. Please re-tag if the plan changes!

I updated the issue summary, now this looks good for a novice contributor at a code sprint.

1. Proposed resolution, option 1 looks like short work,
2. Proposed resolution, option 2 needs a bit more investigation, I'm not sure which preprocess hook it would need.

I will apply updates for the code.

@andrewmacpherson There was an issue https://www.drupal.org/project/drupal/issues/2982781 Support display setting changes in Demo Umami's card view modes, where the card templates were converted to the new one:

1. highlighted-top -> card-common
2. highlighted-medium -> card-common-alt
3. highlighted-bottom -> card-common
4. highlighted-small -> card

And appropriate files for those cards were deleted

1. highlighted-bottom.css Deleted
2. highlighted-medium.css Deleted
3. highlighted-small.css Deleted
4. highlighted-top.css Deleted
5. node--highlighted-bottom.html.twig Deleted
6. node--highlighted-medium.html.twig Deleted
7. node--highlighted-small.html.twig Deleted
8. node--highlighted-top.html.twig Deleted

So as I understand now we should care about this template:

1. highlighted-bottom -> card-common

And the highlighted-small -> card template should be respected as well.
Am I right?

@andrewmacpherson Using of such construction with translation placeholder:
{% trans %} View {{ node.bundle }} <span class="visually-hidden">&ndash;&nbsp;{{ label }}</span> {% endtrans %}
Will give such a string for translation:
View @node.bundle <span class="visually-hidden">&ndash;&nbsp;@label</span>
And in current situation @node.bundle will always passed as a variable value with no translation provided, as far as @node.bundle gives as machine name of the node type, we will not able translate it.
But using of such construction:
{{ 'View'|t }}&nbsp;{{ node.bundle|t }}&nbsp;<span class="visually-hidden">&ndash;&nbsp;{{ label }}</span>
Where we use string translation filter for both will give us possibility handle translation for a node type as a string value.
So I used this approach in the path above.

Tagging for upcoming contribution days.

@volkswagenchick - thanks for tagging, it's a good one to work on. I'll be at the Leeds UK sprint, and we are expecting @eli-t (OOTB maintainer) too. I'll point this one out to him.

@hiway - thanks for working on this. Here is feedback for the patch in #17:

1. "node.bundle gives as machine name of the node type" - GOOD CATCH :-) In that case it's the wrong variable (that was my mistake, sorry). We should use the human-readable name instead.
2. Treating the 3 parts as separate translatable phrases will hinder translators, because it forces a particular word order. Phrasing in some languages may need the equivalent of "Recipe View - Crema catalana" The approach of #11.3 is still preferred, I think?
3. Patch #17 uses non-breaking spaces, without explaining why. I don't know if this is better or worse for screen readers using text-to-speech. However it does cause issues for screen readers using a braille display. It's safer to use normal spaces here - we should change those back.

Re: #5 - a bit late, it occurs to me that wrapping the whole card as a link will give us very long link names, with the visible recipe name somewhere in the middle. I don't think that's going to be easy to understand. Remember, WCAG "link purpose" is about making it clear what the link leads to, and burying the recipe name in the middle of a long link name makes that harder IMO.

Updating title. If #10 is still the proposed direction, then the answer is yes - the read-more links DO need the extra context.

there's a lot of new information, since the last summary update.

also, is the list of templates up to date? I know the OOTB team were trying to reduce the number of these.

Re #13, I think using the readmore links provided by the node module is still worth looking into. The numbering of proposals 1 + 2 is not an order of preference.

The links in
/core/profiles/demo_umami/themes/umami/templates/content/node--card-common.html.twig
/core/profiles/demo_umami/themes/umami/templates/content/node--card.html.twig

<a class="read-more__link" href="{{ url }}">{{ 'View'|t }} {{ node.bundle }}</a>

This should be written like this to pass WCAG 2.0 standards

<a class="read-more__link" href="{{ url }}" aria-label="{{ 'Go to the @label page'|t({'@label': label }}">{{ 'View'|t }} {{ node.bundle }}</a>

This solution will give sighted users the visual text of "View Bundle" while giving screen readers a more unique and descriptive label of "Go to the Node page".

NOTE: There is another issue with there being 2 links within the card with it's not a page that go to the same place. The fact that these 2 links have different link text makes this issue less confusing to screen reader users, but it is not the most ideal use case. In the future it would be nice to add a tabindex="-1" and role="presentation" to the link within the H2. This would keep this link from keyboard tabbing and screen reader shortcuts, while allowing sighted users to interact with this link still.

Thanks for the comments @kbeck303

This solution will give sighted users the visual text of "View Bundle" while giving screen readers a more unique and descriptive label of "Go to the Node page."

Unfortunately, that will fail WCAG 2.1 success criterion 2.5.3 Label In Name. A sighted speech-control user will not be able to activate the link by saying "Click View Bundle". For more details, see Example 1 in F96: Failure due to "accessible name" not containing the visible label text.

Granted, our current target is WCAG 2.0, but we aim to raise that to WCAG 2.1 in due course, so we don't want to knowingly introduce something at this stage which passes WCAG 2.0 but fails WCAG 2.1.

View Recipe <span class="visually-hidden">Crema catalana</span> does satisfy Label in Name, so the approach in #10-11 is still the preferred one.

There is another issue with there being 2 links within the card with it's not a page that go to the same place. The fact that these 2 links have different link text makes this issue less confusing to screen reader users, but it is not the most ideal use case.

That was discussed earlier in the issue, and @kjay proposes in #10 to reduce it to one link per card. The workaround with role="presentation" and tabindex="-1" would indeed work, but it won't be necessary if we're following #10.

@andrewmacpherson

I just created a patch and an interdiff implementing the WCAG 2.0 compliant solution suggested by @kbeck303.

Based on your comments, I will also create a patch implementing the WCAG 2.1 compliant approach in #10 - #11.

I'll upload that, also with an interdiff to #17, as an alternate approach.

Happy Drupal Global Contribution Weekend!

@andrewmacpherson here's the patch and interdiff based on your WCAG 2.1 compliant approach.

Hopefully one of these will resolve the issue.

@hotwebmatter I suppose you patches are failing because the actual code for card node templates in 8.7.x branch is:

<div class="read-more">
    <a class="read-more__link" href="{{ url }}">{% trans %}View {{ node.type.entity.label() }}{% endtrans %}</a>
  </div>

I've updated you pathc against 8.7.x and may some small changes. As far as node label is already translated we don't need pass it through the {% trans %} function, and "-" should also be a part of visually hidden element.

Tested with English and Hebrew, a11y context works as intended (I can see it when inspecting the "View Recipe / View Article link elements)

(seems like automated tests failed for an unrelated reason)

I agree with the RTBC, but we still need to think about this part of the proposal from #10 (and discussed in #22-23,) concerning the multiple links per card:

Remove link around titles.

That task can be done in a follow up I think. The patch here is a big a11y win, regardless of the multiple links issue.

FIled a follow up issue to fix the duplicate links.

Sent for a re-test since there was no proof this passes tests now.

Assigning credits.

Thanks all!

Fixing the accessibility tag.