Skip to content

LG-6874: Keep doc auth errors available on "Back" for first couple IPP pages#6801

Merged
NavaTim merged 4 commits intomainfrom
tbradley/lg-6874-retain-doc-capture-errors
Aug 19, 2022
Merged

LG-6874: Keep doc auth errors available on "Back" for first couple IPP pages#6801
NavaTim merged 4 commits intomainfrom
tbradley/lg-6874-retain-doc-capture-errors

Conversation

@NavaTim
Copy link
Contributor

@NavaTim NavaTim commented Aug 19, 2022

GIF Demo

back_preserve_error_doc_auth3

Testing Steps

  1. Start doc auth workflow
  2. Use an ID that causes an IPP-compatible error (e.g. image resolution)
  3. Select the option to verify your identity in-person
  4. Select "Back" link or use browser back button
  5. The specific error should remain visible

@NavaTim NavaTim requested review from a team and sheldon-b August 19, 2022 01:56
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.

This is a clever solution, nice!

const [previousErrors, setPreviousErrors] = useState<PreviousErrorsLookup>({});

useEffect(() => {
const prevErrs = previousErrors[stepName || steps[0]?.name];
Copy link
Contributor

@aduth aduth Aug 19, 2022

Choose a reason for hiding this comment

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

In at least one other place in the file we reference the step name by step?.name, since step would be the result of validating stepName in the set of steps. We could move this useEffect a bit down (or conversely, move the step assignment a bit up) to use it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Still works as intended, so the useEffect has been moved.

const [previousErrors, setPreviousErrors] = useState<PreviousErrorsLookup>({});

useEffect(() => {
const prevErrs = previousErrors[stepName || steps[0]?.name];
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor: It could be useful to disambiguate the two different "previous errors" here by incorporating "step" into the name, e.g.

Suggested change
const prevErrs = previousErrors[stepName || steps[0]?.name];
const previousStepErrors = previousErrors[stepName || steps[0]?.name];

Copy link
Contributor Author

@NavaTim NavaTim Aug 19, 2022

Choose a reason for hiding this comment

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

I decided to rename the state value instead of the one in useEffect. Given that the const in useEffect is used only on the following line and nowhere else, I don't think a rename of that const is particularly helpful.

Copy link
Contributor

@sheldon-b sheldon-b left a comment

Choose a reason for hiding this comment

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

LGTM

@NavaTim NavaTim merged commit 2611878 into main Aug 19, 2022
@NavaTim NavaTim deleted the tbradley/lg-6874-retain-doc-capture-errors branch August 19, 2022 18:33
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