Skip to content

LG-5213: IAL2 document capture tips prior to submission#5534

Merged
aduth merged 47 commits intomainfrom
aduth-lg-5213-docauth-tips
Oct 27, 2021
Merged

LG-5213: IAL2 document capture tips prior to submission#5534
aduth merged 47 commits intomainfrom
aduth-lg-5213-docauth-tips

Conversation

@aduth
Copy link
Contributor

@aduth aduth commented Oct 22, 2021

Why: As an end user, if I am attempting to take images of my ID with the Acuant SDK, I want to see messaging to help me troubleshoot if I try multiple times, so that I get additional help when I need it.

Implementation Notes:

Screenshot:

screenshot

@aduth aduth marked this pull request as ready for review October 25, 2021 13:48
Comment on lines +148 to +152
const {
documentCaptureTipsUrl: documentCaptureTipsURL,
maxCaptureAttemptsBeforeTips,
appName,
} = /** @type {AppRootData} */ (appRoot.dataset);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Something to consider following-up on, since there's a lot of inconsistency with how we pull attributes from the root element. This was also sparked from discussion point at #5535 (comment) .

A few observations:

  • Renaming properties from naive dataset auto-conversion (e.g. documentCaptureTipsUrl -> documentCaptureTipsURL)
  • Casting type from dataset string to target type (e.g. Number(maxCaptureAttemptsBeforeTips))

I'm familiar with some related validation libraries (e.g. joi, JSON Schema implementations like ajv). On closer inspection, Ajv has a "type coercion" feature which could be quite nice 🤔 A shame it's a very large library.

Maybe we could do some minimal utility to pick / format / rename:

const {
  documentCaptureTipsURL,
  maxCaptureAttemptsBeforeTips,
  appName,
} = pick(appRoot.dataset, {
  documentCaptureTipsUrl: compose(renameTo('documentCaptureTipsURL'), castAs(Number)),
  maxCaptureAttemptsBeforeTips: castAs(Number),
  appName: identity,
});

aduth added 4 commits October 27, 2021 08:23
This reverts commit eadf947.
**Why**: Maintain as close to the current implementation as we can until further research and user testing can give us insight into optimal navigation experience.
Copy link
Contributor

@amathews-fs amathews-fs 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.

Reviewed sandbox (iOS, Chrome and Safari) and tested glare errors. The main issue is that the tips screen appears on the second error, not the third? Also noticed that the scroll position does not appear to reset which can obscure the icon/heading at the top - would make sense to reset it to the top, I think.

Visually, couple tiny things:

  1. A little more space before and after the Try Again button (2.5 rem total top/bottom)
  2. Thanks for catching that the Try Again button should be full width on mobile, I actually had that wrong in my mockups but it's fixed now! Is the button the big or regular size, though? (should be big)
  3. Remove Start Over and Cancel here

Lastly, I didn't check that this doesn't appear on desktop/manual upload or other scenarios outside of blur and glare, anything like that worth manually testing? Thanks!

@aduth
Copy link
Contributor Author

aduth commented Oct 27, 2021

Thanks @anniehirshman-gsa ! Most of this looks like it should be relatively straight-forward to fix up.

Also noticed that the scroll position does not appear to reset which can obscure the icon/heading at the top - would make sense to reset it to the top, I think.

The intention is that the title of the warning screen should be focused, which should at least reset the scroll to bring that element into view, even if not to the very top. I'll check to make sure that's working. Though based on what you're suggesting, maybe it would be better to reset it to the top regardless?

Lastly, I didn't check that this doesn't appear on desktop/manual upload or other scenarios outside of blur and glare, anything like that worth manually testing? Thanks!

Correct, we're only counting these attempts in response to photos submitted through the Acuant SDK, so neither desktop nor manual upload would be a factor.

@anniehirshman-gsa
Copy link
Contributor

Great thanks! Re: scroll position, I think setting it to the very top is fine, but it at least should show the main content area in view (if the heading is the first thing in view, you miss the warning icon/design element).

Glad to hear we're all set for desktop and manual testing... I'm assuming other front-end scenarios that are not blur and glare (like image size) are unaffected as well. In that case, with these changes should be all set 👍

@aduth
Copy link
Contributor Author

aduth commented Oct 27, 2021

Is the button the big or regular size, though? (should be big)

Missed this question: It's big, yes.

@aduth
Copy link
Contributor Author

aduth commented Oct 27, 2021

I just pushed the following revisions:

As discussed on Slack, I'm going to leave the following items for quick follow-on pull requests, since they're non-trivial and this pull request is already quite large and blocking other effort:

  • Reset scroll position to the top of the page on form step transitions
  • Hide "Start Over" and "Cancel" links from troubleshooting tips

@aduth aduth merged commit ce0683b into main Oct 27, 2021
@aduth aduth deleted the aduth-lg-5213-docauth-tips branch October 27, 2021 19:29
aduth added a commit that referenced this pull request Oct 28, 2021
Follow-up from #5534

**Why**: Per design, the "Start Over" and "Cancel" links are not intended to be shown in the troubleshooting tips. Because this was rendered outside the React application, the approach to hiding them is to absorb the logic of the links into the React application, rendering them only where applicable.
aduth added a commit that referenced this pull request Oct 29, 2021
* LG-5213: Hide "Start Over" and "Cancel" in capture tips

Follow-up from #5534

**Why**: Per design, the "Start Over" and "Cancel" links are not intended to be shown in the troubleshooting tips. Because this was rendered outside the React application, the approach to hiding them is to absorb the logic of the links into the React application, rendering them only where applicable.

* Add component description for ButtonTo

Portal-ing is non-obvious

* Import FlowPath type from document-capture

#5553 (comment)
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.

3 participants