-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
Refactor login form into a single component #2363
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 is not my area of expertise, but this looks OK to me.
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.
Leaving a few comments mostly LGTM
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.
auto focus feature still has 2 problems
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.
Looks great thanks!
🚀 Deployed to staging 🚀
|
🚀 Deployed to production in version: 1.0.39-5🚀
|
Details
Before, the
LoginForm
component had narrow and wide counterparts (sub-components), which made the logic confusing.In lieu of the changes in #2196, we can now refactor this into a single component without these counterparts and clean up the code.
Fixed Issues
Expensify/Expensify#157816
Tests
QA Steps
Tested On
Screenshots
Web
Mobile Web
Desktop
iOS
Android