-
Notifications
You must be signed in to change notification settings - Fork 1
Auth0 integration and Login Success page #7
Conversation
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.
This PR looks solid! Just left one small nitpick about the digit handling logic, but it can wait until the next PR.
🚢
</div> | ||
<> | ||
<div className={styles.inputContainer}> | ||
{ |
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.
Nit: This component is getting pretty packed with logic. It’s fine for now, but if we keep adding features, we might want to think about breaking some of this out into its own component.
I don’t want to mess with the “insertFromPaste,” so feel free to just leave a “TODO” comment for now.
Just keeping an eye on this complexity because React makes it so easy to get these things dirty and unmanageable!
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.
Yeah you're right. I'll add a TODO comment.
Thanks!
() => setSubmittedEmail(inputValue), | ||
() => { | ||
setSubmittedEmail('') | ||
// TODO better error showing for the user |
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.
👍 we'll mention this to our designer.
Integration between extension and the login token is made here