-
Notifications
You must be signed in to change notification settings - Fork 1
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
[auth] Styled and implemented forgotPassword screen #60
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.
bro reading your code is so readable keep it up. couple of small bugs to fix!
|
||
subtext: { | ||
paddingVertical: 18, | ||
color: '#797979', |
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.
You can replace with with colors.darkGrey
(from styles/colors.tsx
) just to keep everything standardized.
<Text style={[globalStyles.h1]}>Forgot Password?</Text> | ||
<UserStringInput | ||
placeholder="Email or account username" | ||
placeholderTextColor="#797979" |
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.
You can replace with with colors.darkGrey
(from styles/colors.tsx
).
leftIcon={{ type: 'font-awesome', name: 'envelope' }} | ||
onChangeText={text => setEmail(text)} | ||
<View> | ||
<Text style={[globalStyles.h1]}>Forgot Password?</Text> |
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.
I think you need to add a paddingBottom of 8 here based on the designs
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.
couple things i forgot the first pass of the review
const { error } = await verifyOtp(email, verificationCode); | ||
const setAndCheckEmailOrUsername = async (newEmail: string) => { | ||
setEmail(newEmail); | ||
const release = await mutex.current.acquire(); |
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.
So I found a much better way to fix this issue (sorry about making you implement the mutex, but this makes the code a lot easier to understand). Using this library (https://github.com/xnimorz/use-debounce?tab=readme-ov-file), we can debounce a value. This means the state will only update when the user stops updating the text field (aka, stops typing). So we only need to run database requests whenever the user stops typing and not everytime the text box updates. It should be really easy to implement and you can play around with what you want the debounce value to be.
What's new in this PR
Relevant Links
Notion Sprint Task
Sprint Page
Online sources
Mutex documentation
Related PRs
How to review
Next steps
Tests Performed, Edge Cases
Screenshots
CC: @adityapawar1