LG-7760: Increase visibility of in-person proofing call-to-action#7211
Merged
LG-7760: Increase visibility of in-person proofing call-to-action#7211
Conversation
aduth
requested review from
a team,
benjaminchait,
kellular,
sumiat and
tomas-nava
aduth
force-pushed
the
aduth-lg-7760-ipp-cta
branch
from
07795f9 to
6c62200
Compare
svalexander
reviewed
Oct 25, 2022
Contributor
Author
|
Going to flip this back to draft while I sort out what's going on with the failing spec. |
kellular
approved these changes
Oct 25, 2022
kellular
left a comment
kellular
left a comment
There was a problem hiding this comment.
From a design and content perspective, this LGTM
NavaTim
approved these changes
Oct 25, 2022
Contributor
NavaTim
left a comment
NavaTim
left a comment
There was a problem hiding this comment.
Overall looks good, but per the story:
Wait to merge changes...
If you want to prevent code drift we could move forward with a feature flag instead, but TBH it's not that long to wait.
app/javascript/packages/document-capture/components/in-person-call-to-action.tsx
Outdated
Show resolved
Hide resolved
allthesignals
approved these changes
Oct 25, 2022
Contributor
There was a problem hiding this comment.
LGTM. Love seeing the IPP translation entries being moved into the in_person_proofing file. What I'm seeing in general:
- Extract the tag markup into its own component and invoke in
<InPersonCalltoAction/> - De-bloat troubleshooting-options by removing isNewFeatures and divider
- Promote isNewFeatures flag to a new component
<InPersonCalltoAction/> - Divider visiblity is back to "implicit" CSS pseudo-element styling
app/javascript/packages/components/troubleshooting-options.spec.tsx
Outdated
Show resolved
Hide resolved
...cript/packages/document-capture/components/document-capture-troubleshooting-options.spec.tsx
Outdated
Show resolved
Hide resolved
...cript/packages/document-capture/components/document-capture-troubleshooting-options.spec.tsx
Outdated
Show resolved
Hide resolved
app/javascript/packages/document-capture/components/in-person-call-to-action.spec.tsx
Outdated
Show resolved
Hide resolved
sumiat
approved these changes
Oct 25, 2022
aduth
force-pushed
the
aduth-lg-7760-ipp-cta
branch
from
62f80fb to
f3a5955
Compare
changelog: Improvements, In-person proofing, Increase visibility of in-person proofing call-to-action
Since ID includes dynamic, incrementing value, and is not deterministic
1. The tests run with async document uploads, which don't actually assign the value to the field, they upload directly to S3
2. We're already indirectly asserting the presence of a value by checking that the label name includes the file name ("logo.png")
aduth
force-pushed
the
aduth-lg-7760-ipp-cta
branch
from
f3a5955 to
6274fc8
Compare
Contributor
Author
|
Next production deploy is on November 3rd, so this is ready to merge now. |
Merged
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🎫 Ticket
LG-7760
🛠 Summary of changes
Revises the appearance of the in-person proofing call-to-action to appear as an alternative option to "Try again", instead of as a troubleshooting option.
📜 Testing Plan
Verify that you see and can opt-in to in-person proofing.
👀 Screenshots
Mobile:
Desktop: