-
Notifications
You must be signed in to change notification settings - Fork 340
Verify user flow #196
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Verify user flow #196
Conversation
|
This pull request introduces 3 alerts when merging c3760a3 into 8e34ac7 - view on LGTM.com new alerts:
|
| }, | ||
| }); | ||
|
|
||
| I18n.putVocabularies(dict); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the aws-amplify-react package, we do this call ourselves when exporting the modules: https://github.com/aws-amplify/amplify-js/blob/main/packages/aws-amplify-react/src/index.tsx#L30
Is this something we expect customers to do in their applications?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For now, yes. Because this might look different in React vs Angular vs Vue. Suggestion proposed by Eric on my last PR - #189 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want this to be in https://github.com/aws-amplify/amplify-ui/blob/verify-user-flow/examples/next/pages/_app.tsx since most examples need it?
|
This pull request introduces 3 alerts when merging 231cca7 into af7de95 - view on LGTM.com new alerts:
|
231cca7 to
9b06e4e
Compare
|
This pull request introduces 3 alerts when merging d2980c0 into af7de95 - view on LGTM.com new alerts:
|
ericclemmons
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for sharing the video @eddiekeller! This is looking great.
There may be a minor tweak to be made, but I see several things that we'll come back to & polish as part of our implementation: send usage, shadowing APIs, dynamic buttons based on pending status, etc.
Again, great work!
| }, | ||
| }); | ||
|
|
||
| I18n.putVocabularies(dict); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want this to be in https://github.com/aws-amplify/amplify-ui/blob/verify-user-flow/examples/next/pages/_app.tsx since most examples need it?
Sync branch main to liveness-main
Issue #, if available:
n/a
Description of changes:
Adding verify user flow for when a user is registered and confirmed but does not have their contact info "verified." Weird state to get into.
Also added in the I18n setup for each of the react environments.
verify-user.mov
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.