Ensure advance warning of links opening new window (LG-3528)

[mitchellhenke]

The ticket only mentions the footer, but there were other places as well so I fixed those. I'm not sure if in some of these cases we'd rather remove the target='_blank', but I didn't do that to any for now.

This adds a partial that includes the text describing the link as opening a new window for screen readers. I also added external-link to our styles because usa-external-link from USWDS includes a hover state (and we don't).

The language of (opens new window) was taken directly from https://www.w3.org/TR/WCAG20-TECHS/H83.html

New:

Old:

[aduth]

Was it considered to have these simply not open in a new window?

[mitchellhenke]

@aduth The issue is it would clear any inputs or forms a user has filled out, and weighing that risk depends on the page. It's probably not a big deal on pages like login or sign up where there's only one or two fields, but it's harder for the footer where a user may be in the middle of proofing or something.

[aduth]

@aduth The issue is it would clear any inputs or forms a user has filled out, and weighing that risk depends on the page. It's probably not a big deal on pages like login or sign up where there's only one or two fields, but it's harder for the footer where a user may be in the middle of proofing or something.

Works for me 👍

[aduth]

My expectation was that if we keep the links as opening in new tab, it would be communicated through aria-label that the link opens in a new window, and keep the visual text as-is (optionally with an icon).

[mitchellhenke]

@aduth Based on https://www.w3.org/TR/WCAG20-TECHS/H83.html, I'm not sure if aria-label is sufficient, as the warning could be helpful to people not using screen readers as well:

The objective of this technique is to avoid confusion that may be caused by the appearance of new windows that were not requested by the user. Suddenly opening new windows can disorient users or be missed completely by some. In HTML5, HTML 4.01 Transitional, and XHTML 1.0 Transitional, the target attribute can be used to open a new window, instead of automatic pop-ups. (The target attribute is deleted from HTML 4.01 Strict and XHTML 1.0 Strict.) Note that not using the target allows the user to decide whether a new window should be opened or not. Use of the target attribute provides an unambiguously machine-readable indication that a new window will open. User agents can inform the user, and can also be configured not to open the new window. For those not using assistive technology, the indication would also be available from the link text.

(Emphasis mine)

[aduth]

@mitchellhenke Hm, fair point. Also looking at G201, the procedure describes "there is a visual warning in text that this link opens to a new window". It could be possible that an icon as described in my previous comment could be sufficient for this. As currently proposed, there's enough visual change that I'd at least recommend soliciting some design feedback.

Alternatively, I'd also consider revisiting my earlier question, to the point of: Maybe we should just not open these in new windows. Per G200, this would be preferable in most cases. I don't know that navigating away would be disruptive to progress, at least in that (a) it should be expected from an explicit user action to navigate that in-page state could be lost, and (b) the primary concern described in G200 is of "multi-step" workflows. One which comes to mind is document capture in the React application, currently slated for improvement separately in LG-3613 (show prompt when navigating away from document capture).

[zachmargolis]

if we're going with this visual treatment, could we just apply this style to a[target=_blank] and skip the partial approach?

[mitchellhenke]

We could, but we'd have to override it to use the alternate white version for cases like the footer.

[zachmargolis]

Yeah that seems reasonable? Basically footer a[target=_blank] and be done with it?

[mitchellhenke]

Done in de18674, but I'm not sure if the file should be renamed.

[zachmargolis]

Cool and now we can get rid of the partial entirely, right?

[mitchellhenke]

We have to have both a visual notification and explicit text to let people using screen readers know the link will open a new window. The partial is needed for the latter part.

[mitchellhenke]

Also scss-lint is mad about using the attribute selectors with an element?

[zachmargolis]

Hmmmm I have one more idea to help us get rid of a partial and leverage CSS.... we could also do the text as ::after { content: '' } attribute

but we'd need to translate it per-local so we'd have to scope it by language. Luckily we sent <html lang="en"> on every page, so:

html[lang=en] {
  a[target=_blank]::after {
    content: 'opens in a new window';
  }
}

html[lang=es] {
  a[target=_blank]::after {
    content: 'opens in a new window but in Spanish';
  }
}

I don't know about the scss-lint thoughj

[mitchellhenke]

unfortunately pseudo-element content cannot be non-decorative: https://www.w3.org/TR/WCAG20-TECHS/F87.html

The CSS :before and :after pseudo-elements specify the location of content before and after an element's document tree content. The content property, in conjunction with these pseudo-elements, specifies what is inserted. For users who need to customize or turn off style information in order to view content according to their needs, assistive technologies may not be able to access the information that is inserted using CSS. Therefore, it is a failure to use these properties to insert non-decorative content.

[aduth]

Per discussion in #4310 (comment), I personally don't see it being much of a problem to have the partial, with there being two potential benefits:

• Acknowledging that, per guidelines, we want this to be an explicit and conscious decision, and that any potential inconvenience associated with using non-standard link rendering should be welcomed in promoting that consideration.
• Based on the USWDS links guidance that @mitchellhenke discovered, I think ideally we should be applying both usa-link and usa-link--external classes for these links.

Maybe an alternative is a helper link_to which adds any applicable classes and supplementary "(opens in new window)" text? I don't have a strong sense if one or the other of a helper or partial would be preferable.

[mitchellhenke]

@aduth if it looks good, can I get a ✅? 😀

[aduth]

Handful of syntax errors related to closing parentheses. Hard to tell based on the other lint failure, but hopefully this is something our automated testing can be catching?

Edit: I see the build failures now.

[mitchellhenke]

@aduth thank you for catching that, bad copy/paste job by me. fingers crossed the next build is it 🤞🏼

[aduth]

One question, but looks good 👍

[aduth]

How is the block form of this method expected to be called? With link_to, the arguments are shifted so make name optional:

https://github.com/rails/rails/blob/60ef07/actionview/lib/action_view/helpers/url_helper.rb#L197

Do we even need the block support? It doesn't appear we're using it?

Aside: Reminds me that test specs would be good as well.

[mitchellhenke]

We do use the block version in the footer here: https://github.com/18F/identity-idp/pull/4310/files#diff-727c1af48666bc6f26041f22d5bca5b0f0ab95014c24d020434389879134191cR32-R36

[aduth]

👍