-
Notifications
You must be signed in to change notification settings - Fork 1k
USWDS - Links: External link labels #5166
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@amycole501 this is ready for your review. Here's the preview link. Interested in hearing if these labels are read and if that experience is good. |
I can confirm that using JAWS the words "external link" were read on all three link examples. From a usability standpoint, the third link was best. The first link example announces the brackets first, the second one mentions a colon but the third one only says "external link" along with the link wording and that's easiest to understand. In Narrator all three links announced the same as the third link. So if given the choice, I'd go with how you coded the third link. |
- Create a new theme setting for external links

There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this. Feels like a straightforward solution and it works well in VoiceOver.
One thing that might be worth considering is adding a comma to the end of the default content ($theme-external-link-sr-label: "External link,"
). It seems to give a tiny pause in between the pseudo text and the link text, which I found helpful. But it should be fine to move forward as-is.
Tested on VoiceOver in Chrome, Safari, Firefox, and Edge.
Also, the preview link is broken in the description. It looks like you removed the test story, so I went ahead and tested just on the main link component story.
@amyleadem thanks for the reminder on the preview link, I've added a comment and a new preview. I was hesitant to add any punctuation to affect RTL sites. That said, we don't have data on how common this use case is. Open to adding comma. |

There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do hear "external link". The only note I'd add is if the external link opens the visitor to a new window. That does also need to be announced. If it opens the external link in the same window nothing additional needs to be announced.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two things to consider:
- If someone is implementing the external link feature and the link opens in a new window, the person who adds the link needs to add some sort of way for a screen reader to announce it opens in a new window. Some guidance is here https://www.w3.org/TR/WCAG20-TECHS/G201.html
- Consider also if someone is implementing this link feature using a CMS-driven environment. Not all WYSIWYG editors allow the user to add link help text or descriptive hints that say "link opens in a new window". Not sure of the solution but wanted to raise it as a potential scenario.
These labels get combined in mixin to inform users about external links and whether they open in new tabs or not.
packages/usa-link/src/usa-link.twig
Outdated
</p> | ||
|
||
<p> | ||
This is a link that opens in a <em>new</em> tab and goes to an <a class="usa-link usa-link--external" rel="noreferrer noopener" target="_blank" href="https://i.giphy.com/media/WPzQF6ruiIIVzHNlwX/source.gif">external website</a>. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've explicitly added rel="noreferrer noopener"
as a best practice; even though most modern browsers implicitly add it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this is a best practice, would it be beneficial to include guidance on noreferrer noopener
in the documentation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've kept noreferrer
since only one is needed in [e348c59], but it might not be needed at all since modern browsers add it implicitly.
Either way, I've added it in guidance on site:
uswds/uswds-site#2056
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Listened in JAWS and used ANDI to verify the external link text was announced. Sounded good, clear, and was easy to understand.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this addition. The technical solution feels clean and the readout is helpful. I like that it doesn’t require any extra user action when setting up a link and that the readout copy is customizable.
Confirming that the VoiceOver readout is “link, external link, opens in current tab [link text]” or “link, external link, opens in new tab [link text]”, depending on the link target.
Added a few comments but no action is needed.
- Confirm that customizing the new theme settings updates the text as expected
- Confirm that VoiceOver identifies external links
- Confirm that VoiceOver identifies if the link opens in a new window
packages/usa-link/src/usa-link.twig
Outdated
</p> | ||
|
||
<p> | ||
This is a link that opens in a <em>new</em> tab and goes to an <a class="usa-link usa-link--external" rel="noreferrer noopener" target="_blank" href="https://i.giphy.com/media/WPzQF6ruiIIVzHNlwX/source.gif">external website</a>. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this is a best practice, would it be beneficial to include guidance on noreferrer noopener
in the documentation?
@@ -123,6 +123,11 @@ $theme-in-page-nav-margin-left: 4 !default; | |||
$theme-in-page-nav-margin-top: 2.5 !default; | |||
$theme-in-page-nav-top: 4 !default; | |||
|
|||
// Link | |||
$theme-external-link-sr-label: "External link" !default; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No action is required on this, but wondering if we can reduce some audio redundancy by removing the word “link” here so that it is just “External”. In Voiceover, that would mean the readout is “link, external, opens in a new tab [link text]”. I am not sure if other screen readers announce “link” in the same order though, so this might just reflect a VoiceOver bias.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@amycole501 did you notice this issue on NVDA/JAWS? If so, I can shorten it to just "External".
packages/usa-link/src/usa-link.twig
Outdated
@@ -2,7 +2,13 @@ | |||
|
|||
<p>This is <a class="usa-link usa-color-text-visited" href="javascript:void(0);">a visited link</a>.</p> | |||
|
|||
<p>This is a link that goes to an <a class="usa-link usa-link--external" href="https://i.giphy.com/media/WPzQF6ruiIIVzHNlwX/source.gif">external website</a>.</p> | |||
<p> | |||
This is a link that opens in <em>current</em> tab and goes to an <a class="usa-link usa-link--external" href="https://i.giphy.com/media/WPzQF6ruiIIVzHNlwX/source.gif">external website</a>. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No action needed, just noting that opening source.gif
is still a surprise every time
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now that @amyleadem mentions it yes, it does say "link" twice (which is tricky to discern since some of the links say "link" in them.
Additionally, I hear JAWS trying to say the word "tab" which runs into the phrase "external website" both in ANDI and in JAWS. I'm attaching how it looks in ANDI so you can get an idea of how it sounds. Like a run-on sentence.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@amycole501 @amyleadem I've added the following changes:
- Simplified the CSS
- Removed the base variable
$theme-external-link-sr-label
. - Added punctuation to the label ex:
"External, opens in a new tab."
to prevent screen readers from jumbling the words together.
- Remove base variable `$theme-external-link-sr-label` - Use punctuation to prevent run-on sentences in screen readers
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This sounds much better; the words aren't jumbled together and it's clear when listening in JAWS that the links open either in the same/current tab or a new tab.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good improvements!
FYI - I made a small commit (a00a4ed) to add a missing "the" to a sentence in the twig file. I also updated the PR description to reflect the new default values and descriptions for $theme-external-link-sr-label-tab-new
and $theme-external-link-sr-label-tab-same
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good
I'm going to draw up a changelog PR for this one |
packages/uswds-core/src/styles/settings/_settings-components.scss
Outdated
Show resolved
Hide resolved
Simplify default label. Co-authored-by: Dan O. Williams <[email protected]>
@thisisdano I tried to get a head start by creating a draft PR in site USWDS - Links: Update external link guidance #2056. |
I'm curious about the choice to announce the label before the link text. Most guidance I've seen (including WCAG G201) announces the label after the link text, which makes sense to me because it's a supplementary detail, where the primary detail is the link text itself. And I'd worry it might be slightly disorienting to announce that something will open in a new tab before even knowing what it is that'll be opened in a new tab. In terms of backwards-compatibility, I found my way here after realizing that our external links were announcing "opens in new tab" twice to screen readers, since we had already implemented this type of label before USWDS, so now we have a redundant label. This was somewhat unexpected. Related to my next point, we may just end up disabling the setting by setting it to an empty string, though this seems to work only by accident rather than being fully supported. Lastly, this seems to assume a site is English-only. Our site supports multiple languages, and it's not obvious how we'd be expected to use these labels. In our case, we may just "disable" the setting by configuring it to an empty string and continuing to use our custom implementation. The only alternative I could imagine is creating language-specific stylesheets, each with the Sass variable assigned to the label to be used in that language. |
@aduth good points. I've created #5942 to capture your suggestions. It makes sense to place the label after and allow devs to opt out. Another potential alternative is to use the attr() - MDN to allow users to pass their own strings via CSS. The only downside is it'd have to be manually added per link. We hope to address this type of feature the web component implementation. Initially, this was added to allow basic support of announcing links with further enhancements based on feedback. @jaclinec have we tested or are planning to test links? |
@mejiaj we tested links in 2021 (see research findings), but the research did not include participants who use screen readers so it may not be useful to this conversation. We don't have immediate plans to test links for accessibility but it is in the backlog. |
Summary
External links labeled for screen readers. Adds a label for screen readers that clearly marks external links and whether they open in a new tab or current.
Breaking change
This is not a breaking change.
Related issue
Closes #5164.
Preview link
External link test →Removed in 917b3e9Links [story mode]
Problem statement
Solution
This solution:
::before
pseudo element. Pseudo element was chosen to allow users to easily update and add this feature into their sites.Source: https://www.section508.gov/content/guide-accessible-web-design-development
Alternatives
JavaScript could be used to inject hidden span into each external link.
Major changes
Two new settings for Link component:
$theme-external-link-sr-label
"External link"
stringBase label indicating external link.$theme-external-link-sr-label-tab-new
"External, opens in a new tab."
$theme-external-link-sr-label-tab-same
"External, opens in current tab."
End result
The following text gets announced before an external link:
Testing and review
target="_blank")
or current (fallback).Previous test (click to expand)
Screen readers (JAWS/VoiceOver/NVDA) should read the hidden label identifying external links.Need to find the most clear and obvious label:❌[External link]
❌⚠️ Punctuation might negatively impact RTL languages]External link:
[✅External link
. This was found to provide the best experience across screen readers (JAWS, VoiceOver, and Narrator).I was able to get VoiceOver to read, Option 1 ([External]
) seemed like the best experience to me. Need to hear how these sound on other screen readers.Related PRs
Updated site guidance uswds/uswds-site#2056
npm run prettier:scss
to format any Sass updates.git pull origin [base branch]
to pull in the most recent updates from your base and check for merge conflicts. (Often, the base branch isdevelop
).