Skip to content

LG-6308: Show in-person step indicator in initial steps#6833

Merged
aduth merged 5 commits intomainfrom
aduth-lg-6308-step-indicator
Aug 25, 2022
Merged

LG-6308: Show in-person step indicator in initial steps#6833
aduth merged 5 commits intomainfrom
aduth-lg-6308-step-indicator

Conversation

@aduth
Copy link
Contributor

@aduth aduth commented Aug 24, 2022

Why: It's expected that the step indicator should update to reflect in-person steps as soon as the user opts to verify in-person, which technically occurs while the user is still at the document capture step.

Screenshots:

Step Before After
Document Capture image image
Location image image
Prepare image image
Hybrid Switch Back image image

aduth added 2 commits August 24, 2022 11:41
**Why**: It's expected that the step indicator should update to reflect in-person steps as soon as the user opts to verify in-person, which technically occurs while the user is still at the document capture step.

changelog: Upcoming Features, In-person proofing, Show correct step indicator at each step
@aduth aduth requested review from a team and sheldon-b August 24, 2022 15:43
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

].filter(Boolean) as FormStep[]);

const stepIndicatorPath =
stepName && ['location', 'prepare', 'switch_back'].includes(stepName)
Copy link
Contributor

Choose a reason for hiding this comment

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

it would be cool if we had more static steps so we could do like LocationStep.name, PrepareStep.name, SwitchBackStep.name etc but looking at the step definitions, that is definitely a task for another time

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it would be cool if we had more static steps so we could do like LocationStep.name, PrepareStep.name, SwitchBackStep.name etc but looking at the step definitions, that is definitely a task for another time

Hm, yeah, I can see how that would be nice to have. Technically the step definitions are already an object from which we could reference step.name, and in the FSMv2 flow we do treat those objects as first-class exports, but we haven't really done that here. It could also be nice to collapse it so that the properties are assigned to the React component, but that may add some confusion, and historically I've had a hard time with TypeScript tolerating custom properties on functions.

@aduth aduth merged commit 601919c into main Aug 25, 2022
@aduth aduth deleted the aduth-lg-6308-step-indicator branch August 25, 2022 13:45
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.

Just a question

Comment on lines +44 to +46
password_confirm: 'secure_account',
personal_key: 'secure_account',
personal_key_confirm: 'secure_account',
Copy link
Contributor

Choose a reason for hiding this comment

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

Want to check my understanding here: when the user proceeds through the in-person pages and gets back to the password confirm page, when FSM v2 is enabled, the user will be back in the DEFAULT verify flow path?

Copy link
Contributor Author

@aduth aduth Aug 25, 2022

Choose a reason for hiding this comment

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

That's correct for now, yes, and would be something we want to change. I'll keep that in mind as I implement the remainder of the work for this ticket, since this pull request was primarily concerned with the first couple steps.

Good to call it out though, thanks 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We may be able to do something similar to what we did to implement the "pending" status for GPO in the FSMv2 flow, where we use the address proofing component. For example, we could consider the document proofing component to determine whether the default or in-person flow should be shown.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Circling back to this: The revisions needed here are included in #6846.

@aduth aduth mentioned this pull request Aug 30, 2022
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