Problem/Motivation
Follow-up from #2809635-178: Create experimental installation profile:
This link needs context for accessibility, 'View Recipe' or similar isn't specific enough for screen reader users.
+++ b/core/profiles/demo_umami/themes/umami/templates/content/node--highlighted-bottom.html.twig
@@ -0,0 +1,107 @@
+ <a class="read-more__link" href="{{ url }}">{{ 'View'|t }} {{ node.bundle }}</a>
To satisfy WCAG Link Purpose (In Context) at level A, we should distinguish multiple "View recipe" links from each other.
Proposed resolution
Add the recipe node title as visually-hidden text in the "view recipe" link. Either:
- Update exiting custom template in Umami. Suggested twig code was discussed in comments #10-11:
<a class="read-more__link" href="{{ url }}">{%- trans -%} View {{ node.bundle }} - <span class="visually-hidden">{{ label }}</span> {%- end trans -%} </a>
- Alternatively, change the recipe card display to use the read-more links provided by node module. Can we update the phrasing in a preprocess hook? This has the advantage of respecting "manage display" settings for recipe nodes.
Either of these approaches will satisfy WCAG Link Purpose at level A. The relevant WCAG technique document is C7: Using CSS to hide a portion of the link text.
Important: whichever approach we follow, the link text must also satisfy WCAG 2.5.3 Label in Name. There must not be any visually-hidden text between the words "view" and "recipe". Explained in comment #11.
Remaining tasks
Patch!
User interface changes
No visible changes. Some visually hidden text provides extra context for accessibility, screen reader users especially.
API changes
None.
Data model changes
None.
Original report by [username]
Reported in TWO places already.
- Reported by @DuaelFr in the lauriii/umami Gitub repo: #147: [a11y][AAA] Link Purpose (Link Only)
- Also noticed during code review at : #2809635-178: Create experimental installation profile
I'm not sure i agree with this request. At the moment in Drupal core, we just have a "Read more" string. We are being more explicit here in letting readers know that it's an article or a recipe they are going to read. I'm happy to be corrected.
Comment | File | Size | Author |
---|---|---|---|
#29 | 2937639-29.patch | 1.54 KB | hiway |
#25 | interdiff_17-25.txt | 1.49 KB | hotwebmatter |
#25 | 2937639-25.patch | 1.49 KB | hotwebmatter |
#24 | interdiff_17-24.txt | 1.52 KB | hotwebmatter |
#24 | 2937639-24.patch | 1.42 KB | hotwebmatter |
Comments
Comment #2
andrewmacpherson CreditAttribution: andrewmacpherson as a volunteer and at Annertech commentedDon'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):
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.
Comment #3
andrewmacpherson CreditAttribution: andrewmacpherson as a volunteer and at Annertech commented@DuaelFr needs a credit here for reporting it in the GitHub issue originally.
Comment #4
markconroy CreditAttribution: markconroy as a volunteer and at Annertech commentedComment #5
mgiffordYes, 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:
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.
Comment #6
andrewmacpherson CreditAttribution: andrewmacpherson as a volunteer and at Annertech commentedThere 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
Comment #7
cehfisher CreditAttribution: cehfisher at Hook 42 commentedI 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.
Comment #9
Eli-TUpdate:
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.
Comment #10
kjay CreditAttribution: kjay commentedDiscussed 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>
Comment #11
andrewmacpherson CreditAttribution: andrewmacpherson as a volunteer and at Annertech commented@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:
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!Comment #12
andrewmacpherson CreditAttribution: andrewmacpherson as a volunteer and at Annertech commentedComment #13
andrewmacpherson CreditAttribution: andrewmacpherson as a volunteer and at Annertech commentedRemoving 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.
Comment #14
andrewmacpherson CreditAttribution: andrewmacpherson as a volunteer and at Annertech commentedComment #15
hiway CreditAttribution: hiway at FFW commentedI will apply updates for the code.
Comment #16
hiway CreditAttribution: hiway at FFW commented@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:
And appropriate files for those cards were deleted
So as I understand now we should care about this template:
And the highlighted-small -> card template should be respected as well.
Am I right?
Comment #17
hiway CreditAttribution: hiway at FFW commented@andrewmacpherson Using of such construction with translation placeholder:
{% trans %} View {{ node.bundle }} <span class="visually-hidden">– {{ label }}</span> {% endtrans %}
Will give such a string for translation:
View @node.bundle <span class="visually-hidden">– @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 }} {{ node.bundle|t }} <span class="visually-hidden">– {{ 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.
Comment #18
hiway CreditAttribution: hiway at FFW commentedComment #19
volkswagenchickTagging for upcoming contribution days.
Comment #20
andrewmacpherson CreditAttribution: andrewmacpherson as a volunteer and at Annertech commented@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:
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.
Comment #21
andrewmacpherson CreditAttribution: andrewmacpherson as a volunteer and at Annertech commentedthere'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.
Comment #22
kbeck303 CreditAttribution: kbeck303 at Oomph, Inc. commentedThe 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
This should be written like this to pass WCAG 2.0 standards
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"
androle="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.Comment #23
andrewmacpherson CreditAttribution: andrewmacpherson as a volunteer and at Annertech commentedThanks for the comments @kbeck303
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.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"
andtabindex="-1"
would indeed work, but it won't be necessary if we're following #10.Comment #24
hotwebmatter CreditAttribution: hotwebmatter at Oomph, Inc. commented@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!
Comment #25
hotwebmatter CreditAttribution: hotwebmatter at Oomph, Inc. commented@andrewmacpherson here's the patch and interdiff based on your WCAG 2.1 compliant approach.
Hopefully one of these will resolve the issue.
Comment #26
BrightBoldComment #29
hiway CreditAttribution: hiway at FFW commented@hotwebmatter I suppose you patches are failing because the actual code for card node templates in 8.7.x branch is:
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.Comment #30
hiway CreditAttribution: hiway at FFW commentedComment #31
shaalTested with English and Hebrew, a11y context works as intended (I can see it when inspecting the "View Recipe / View Article link elements)
Comment #33
shaal(seems like automated tests failed for an unrelated reason)
Comment #34
andrewmacpherson CreditAttribution: andrewmacpherson as a volunteer and at Annertech commentedI 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:
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.
Comment #35
andrewmacpherson CreditAttribution: andrewmacpherson as a volunteer and at Annertech commentedFIled a follow up issue to fix the duplicate links.
Comment #36
Gábor HojtsySent for a re-test since there was no proof this passes tests now.
Comment #37
Gábor HojtsyAssigning credits.
Comment #39
Gábor HojtsyThanks all!
Comment #42
mgiffordFixing the accessibility tag.