Skip to content

LG-4951: Link relevant help articles in the IAL2 flow#5300

Merged
aduth merged 7 commits intomainfrom
aduth-lg-4951-ial2-help-articles
Aug 20, 2021
Merged

LG-4951: Link relevant help articles in the IAL2 flow#5300
aduth merged 7 commits intomainfrom
aduth-lg-4951-ial2-help-articles

Conversation

@aduth
Copy link
Contributor

@aduth aduth commented Aug 17, 2021

Why:

As an end user, I want a way to learn more about the requirements to complete the process on the IAL2 welcome screen, if I have questions about the required documents or information so that I know what I will need to complete identity proofing with login.gov before I get started.

As an end-user, I want a way to learn more about the requirements to complete the phone verification step, so that I can get answers to my questions and know if I will be able to complete identity proofing with login.gov.

Note: Phone OTP screen revisions are currently pending design finalization.

Screenshots:

Screen Before After
Welcome welcome-before localhost_3000_verify_doc_auth_welcome
Phone phone-before localhost_3000_verify_phone
Phone OTP otp-before otp-after
GPO gpo-before localhost_3000_verify_usps

**Why**:

As an end user, I want a way to learn more about the requirements to complete the process on the IAL2 welcome screen, if I have questions about the required documents or information so that I know what I will need to complete identity proofing with login.gov before I get started.

As an end-user, I want a way to learn more about the requirements to complete the phone verification step, so that I can get answers to my questions and know if I will be able to complete identity proofing with login.gov.
Copy link
Contributor

@zachmargolis zachmargolis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@aduth
Copy link
Contributor Author

aduth commented Aug 18, 2021

I pushed some updates in ca85b69 based on accessibility feedback to open external links in a new window and show an icon. I've also updated screenshots in the original comment.

This was a bit more sizable work than I'd initially anticipated, due to:

  • Inferring a link as external seemed like an easy prospect, until encountering the fact that our "Get help at SP" redirects go through an internal route before redirecting the user to their external destination. This is why there's both implicit and explicit external options.
  • Our "Block Link" component is implemented both as a Rails partial and as a React component.

url_host_with_port.present? && url_host_with_port != "#{request.host}:#{request.port}"
end
tag_attrs = { href: url, class: 'usa-link block-link' }
tag_attrs[:target] = '_blank' if external
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought our whole thing for external links was that we prefix them with text but we still open them in the same window?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The argument being made in the accessibility discussion and related Slack discussion is that navigating to an external link in the IAL2 flow satisfies some exceptions to the rule, notably "significantly disrupt a multi-step workflow, such as filling in and submitting a form, if the page is opened in the same window or tab" and "provides extensive context-sensitive help [...] in new windows or tabs to prevent the loss of any form data that has already been entered."

Though as I write it out, and considering that the usage of a partial like this shouldn't be exclusive to IAL2, maybe we should at least drop the implicit "new tab if different host" behavior and pass the option explicitly everywhere it's relevant within the IAL2 flow.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Though as I write it out, and considering that the usage of a partial like this shouldn't be exclusive to IAL2, maybe we should at least drop the implicit "new tab if different host" behavior and pass the option explicitly everywhere it's relevant within the IAL2 flow.

Updated in bd5a8e2, with additional clarification around "new tab" not being strictly relevant to the links being external, and should be used within guidelines mentioned above.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to me! Thx 👍

@aduth
Copy link
Contributor Author

aduth commented Aug 18, 2021

It looks like the specs don't like new tabs, so I'll need to dig into that.

Copy link
Contributor

@anniehirshman-gsa anniehirshman-gsa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aduth reviewed the sandbox and looks great, just a couple small comments about the external link icon:

  1. When the external link is included in the troubleshooting options, the preceding word should break to the next line along with the icon, so the icon is never on its own line. Appears to work with the "Learn more" links currently, but not the troubleshooting options.
    Screen Shot 2021-08-19 at 2 33 29 PM

  2. Is it possible to add 2px right and left (so margin-left: 4px and margin-right: 2px), by default to give the icon just a little more breathing room? Here's what it would look like in-line (revised):
    Screen Shot 2021-08-19 at 2 40 07 PM

@aduth
Copy link
Contributor Author

aduth commented Aug 19, 2021

Thanks for the feedback @anniehirshman-gsa !

  1. Yes, good call. I'll try replacing the standard space with a non-breaking space to help with this.
  2. Sure, I can increase the margins. This is a style inherited from the design system, so what I'll probably end up doing is make a temporary patch here, separately propose a change to the design system, and then we can reconcile the redundancy in a future IdP design system update. Let me know if that sounds good to you.

Comment on lines +17 to +28
<svg
xmlns="http://www.w3.org/2000/svg"
height="10"
viewBox="0 0 64 55"
focusable="false"
aria-hidden="true"
>
<path
d="M35.226 5c.333 0 .605.106.818.32s.319.485.319.816v2.273c0 .332-.107.604-.32.817s-.484.32-.817.32H10.227c-1.562 0-2.9.555-4.013 1.668s-1.669 2.45-1.669 4.013v29.545c0 1.563.556 2.9 1.669 4.013s2.45 1.668 4.013 1.668h29.546c1.562 0 2.9-.555 4.013-1.668a5.47 5.47 0 0 0 1.668-4.013V33.41c0-.332.106-.604.32-.817s.484-.32.816-.32h2.273c.331 0 .604.107.817.32s.32.485.32.818v11.363C50 47.591 49 50 46.999 52s-4.41 3.001-7.226 3.001H10.227C7.41 55 5.001 54 3 51.999s-3-4.41-3-7.226V15.227c0-2.817 1-5.226 3-7.226S7.41 5 10.227 5zm26.46-5c.627 0 1.17.229 1.627.686s.687 1 .687 1.626v18.501c0 .626-.229 1.168-.687 1.626a2.22 2.22 0 0 1-1.626.687c-.626 0-1.168-.23-1.626-.687l-6.36-6.36-23.558 23.56c-.24.24-.518.36-.831.36s-.59-.12-.831-.36l-4.12-4.12a1.142 1.142 0 0 1-.361-.831c0-.313.12-.59.361-.83l23.559-23.56-6.36-6.36c-.457-.457-.686-1-.686-1.626s.229-1.168.687-1.626 1-.686 1.625-.686z"
fill="currentColor"
/>
</svg>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than rendering this SVG inline, we should probably just include the usa-link--external class to the wrapping link, to be sure the icon styling is consistent and to automatically fix the line breaking behavior.

The downside is that we don't get automatic text color inheriting via currentColor, as in case of a visited link:

image

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know USWDS has been using CSS mask as a workaround for coloring icons in other similar situations (e.g. the "official USA website" banner arrow icon), so maybe an opportunity for an upstream enhancement.

@aduth
Copy link
Contributor Author

aduth commented Aug 20, 2021

@anniehirshman-gsa I pushed a few updates yesterday afternoon to address your feedback, improving how the icon breaks onto new lines and giving it a bit more space from surrounding content.

localhost_3000_verify_doc_auth_welcome (3)

Copy link
Contributor

@anniehirshman-gsa anniehirshman-gsa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for the updates! And lmk if you need help ticketing the design system improvement for the icon color (so it matches the text color for visited links).

@aduth
Copy link
Contributor Author

aduth commented Aug 20, 2021

lmk if you need help ticketing the design system improvement for the icon color (so it matches the text color for visited links).

I discovered a related issue in the USWDS repository which was closed as out-of-roadmap. Might still be something they'd accept a contribution for though, especially since the mask workaround is a relatively new one AFAIK. Otherwise, we can go the LGDS route.

@aduth aduth merged commit 439d270 into main Aug 20, 2021
@aduth aduth deleted the aduth-lg-4951-ial2-help-articles branch August 20, 2021 13:50
@aduth
Copy link
Contributor Author

aduth commented Aug 25, 2021

Visited link icon color follow-up at uswds/uswds#4297

aduth added a commit that referenced this pull request Oct 14, 2021
**Why**: Because I had mistakenly neglected to get these professionally translated as part of the work in #5300.
aduth added a commit that referenced this pull request Oct 14, 2021
**Why**: Because I had mistakenly neglected to get these professionally translated as part of the work in #5300.
Comment on lines -94 to +93
city: Ville
no_alternate_phone_html: '<strong>%{link}</strong>. Nous vous ferons parvenir
une lettre contenant un code.'
city: Ville une lettre contenant un code.'
Copy link
Contributor

@sheldon-b sheldon-b Jun 28, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aduth looks like this got a little fudged -- I ran into it consolidating some i18n keys for an IPP ticket. Any concerns with me updating this like so? Is there any particular approval process I should go through?

city: Ville

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whoops! Thanks for catching that slip-up @sheldon-b . I think it'd be fine to fix that directly, since the text had already existed and it was only by mistake that it had been combined with the removed text string.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants