Skip to content

LG-12670: handoff ipp option#10260

Closed
dawei-nava wants to merge 10 commits intomainfrom
dwang/LG-12670_handoff_ipp_option
Closed

LG-12670: handoff ipp option#10260
dawei-nava wants to merge 10 commits intomainfrom
dwang/LG-12670_handoff_ipp_option

Conversation

@dawei-nava
Copy link
Contributor

@dawei-nava dawei-nava commented Mar 18, 2024

🎫 Ticket

Replaced with new PR #10267

Link to the relevant ticket:
LG-12670

🛠 Summary of changes

WIP

IPP options on hybrid handoff page when IPP is available.

👀 Screenshots

If relevant, include a screenshot or screen capture of the changes.

Details
After:

IPP available and selfie required

English Spanish French
En-Ipp-Selfie ES-Ipp-Selfie Fr-Ipp-Selfie

IPP available and NO selfie required

English Spanish French
En-Ipp-NonSelfie ES-Ipp-Nonselfie Fr-Ipp-NonSelfie

IPP NOT available and Selfie required

English Spanish French
EN-NonIpp-Selfie ES-NonIpp-Selfie Fr-NonIpp-Selfie

IPP NOT available and NO Selfie required

English Spanish French
EN-Noipp-NonSelfie ES-Nonipp-NonSelfie Fr-NonIpp-NonSelfie

@dawei-nava dawei-nava force-pushed the dwang/LG-12670_handoff_ipp_option branch from d35b79f to d38dd75 Compare March 18, 2024 17:52
@dawei-nava dawei-nava changed the title Dwang/lg 12670 handoff ipp option LG-12670: handoff ipp option Mar 18, 2024
Copy link
Contributor

@JackRyan1989 JackRyan1989 left a comment

Choose a reason for hiding this comment

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

Is LG-12670 the correct ticket for this work? I don't see any changes to analytics_events or anything about capturing the SDK version number. This seems more related to setting up the infrastructure to handle routing in the React app for a new feature?

Copy link
Contributor

Choose a reason for hiding this comment

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

Why this addition when this check already exists on line 67?

Copy link
Contributor

Choose a reason for hiding this comment

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

This variable name is confusing to me. The code in the selected_remote method is checking config values to determine if opt in IPP is enabled, however we are checking a different set of config values here. So I'm not totally sure I understand what this var is checking for? I'm also unclear on how it's being used.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@JackRyan1989 , yes it is a "general" opt in for IPP and may drift away the existing "Optin" meaning. I will rename it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Why are we using this value as a precondition when it's not set to true until the document capture page?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@JackRyan1989 , it's for the "back" button to work from document capture(IPP) page.

So the user can go from handoff (select ipp)-> document catpure -> Back -> handoff.

This is a new flag to control this behavior similar to skip_doc_auth.

Copy link
Contributor

Choose a reason for hiding this comment

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

It seems like this value is referring to a new page for document capture rather than opt in IPP, so I think we should rename it.

@dawei-nava dawei-nava force-pushed the dwang/LG-12670_handoff_ipp_option branch from 28e6abd to 527e65a Compare March 19, 2024 15:42
@dawei-nava dawei-nava mentioned this pull request Mar 19, 2024
8 tasks
@dawei-nava dawei-nava closed this Mar 19, 2024
@dawei-nava dawei-nava deleted the dwang/LG-12670_handoff_ipp_option branch March 19, 2024 16:03
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.

2 participants