Skip to content

In-Person-Proofing "Beta" Tag (LG-6073)#6192

Merged
zachmargolis merged 16 commits intomainfrom
margolis-ipp-beta-tag
Apr 13, 2022
Merged

In-Person-Proofing "Beta" Tag (LG-6073)#6192
zachmargolis merged 16 commits intomainfrom
margolis-ipp-beta-tag

Conversation

@zachmargolis
Copy link
Contributor

@zachmargolis zachmargolis commented Apr 12, 2022

Adds a "new feature" tag that separates this new option, to both client (for doc auth errors) and server (for other errors)

I've deployed this to my personal env to click around on (https://idp.zmargolis.identitysandbox.gov/)

  • get translations
  • add release notes
URL screenshot
doc auth after one error submission Screen Shot 2022-04-12 at 1 31 49 PM
/verify/session/errors/failure Screen Shot 2022-04-12 at 2 34 36 PM
/verify/session/errors/throttled Screen Shot 2022-04-12 at 2 35 02 PM
/verify/session/errors/exception Screen Shot 2022-04-12 at 2 35 22 PM
/verify/session/errors/warning (removed IPP link, since the error message indicates Resolution failed, but that's required to pass for this iteration of IPP) Screen Shot 2022-04-12 at 2 36 09 PM

changelog: Upcoming Features, In-Person Proofing, Add "new features" badge for links
</p>
<% end %>

<%= render TroubleshootingOptionsComponent.new(new_features: true) do |c| %>
Copy link
Contributor

Choose a reason for hiding this comment

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

since this is repeated 3 times should it be broken out into its own file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

like a partial for just the IPP-specific options? sure will give it a shot

Copy link
Contributor Author

Choose a reason for hiding this comment

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

gave that a shot in f2f759c, PTAL!

Copy link
Contributor

@stevegsa stevegsa left a comment

Choose a reason for hiding this comment

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

Excellent. My only concern is that user hasn't given their best effort before presenting the in-person option. ie image blurry. However, it seems there would be no reliable way to determine that.

@zachmargolis zachmargolis requested a review from aduth April 12, 2022 21:44
Copy link
Contributor

@aduth aduth left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Copy link
Contributor

@gsa-manish gsa-manish left a comment

Choose a reason for hiding this comment

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

LGTM

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.

Can remove margin-top-3 from the tag, since the top margin on the section (2 rem) is enough space.

And the caveat: users won't actually see the option to proof in-person from the /verify/session/errors/failure i.e. "We could not find records matching your personal information" screen after the SSN/personal details verification, right? I think you mentioned there was a reason to maintain the different versions.

For translations: I hadn't included those yet because I planned to get everything translated after usability testing and a final content review from @Danielle-Lee, since things are still in flux (for example, the heading changed yesterday). OK to ticket separately, or would you prefer to keep up with translations now? If so, let's loop in Danielle who may want to handle the translation request.

Otherwise LGTM thank you!

@zachmargolis
Copy link
Contributor Author

Can remove margin-top-3 from the tag, since the top margin on the section (2 rem) is enough space.

removed in 5aafbd9

And the caveat: users won't actually see the option to proof in-person from the /verify/session/errors/failure i.e. "We could not find records matching your personal information" screen after the SSN/personal details verification, right? I think you mentioned there was a reason to maintain the different versions.

I'm not 100% certain, but I can remove the links from that page if we want?

For translations: I hadn't included those yet because I planned to get everything translated after usability testing and a final content review from @Danielle-Lee, since things are still in flux (for example, the heading changed yesterday). OK to ticket separately, or would you prefer to keep up with translations now? If so, let's loop in Danielle who may want to handle the translation request.

I ran with what was in the Figma, so long as we remember to update later, copy changes are pretty easy to incorporate later

@zachmargolis zachmargolis merged commit 7b7be84 into main Apr 13, 2022
@zachmargolis zachmargolis deleted the margolis-ipp-beta-tag branch April 13, 2022 16:06
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.

6 participants