-
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] style password complexity on sign in #39
Conversation
…om:calblueprint/girls-write-now into inaya/auth-style-login-signup
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.
Really great work! Very impressed by your handling of all the complexity associated with this task. Most of the comments I left are just styling nit picks, but please Slack me if you have any questions about any of the changes I requested!
src/app/auth/signup.tsx
Outdated
import { Button, Input } from 'react-native-elements'; | ||
|
||
import Icon from '../../../assets/icons'; | ||
import color from '../../styles/colors'; |
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.
Right now your PR is failing checks because you're accidentally importing from the same file twice. Delete line 7 (the wrong import) and you should be good!
src/app/auth/signup.tsx
Outdated
value={password} | ||
secureTextEntry | ||
placeholder="Password" | ||
autoCapitalize="none" | ||
/> | ||
</View> | ||
<View> |
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 outermost View is unnecessary
src/app/auth/signup.tsx
Outdated
</View> | ||
<View> | ||
{password !== '' && ( | ||
<View style={globalStyles.passwordComplexity}> |
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 don't want to add the styles for passwordComplexity to globalStyles, because this style is only going to be used on this screen. Create a styles sheet at the bottom of this file (look at verify.tsx as an example) and move this set of styles there instead.
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.
Also looking at the designs, we have 8 pixels of padding between each password complexity line of text. If you add paddingBottom: 8 to your passwordComplexity styles, this should fix it.
src/app/auth/signup.tsx
Outdated
: { color: colors.textRed } | ||
} | ||
> | ||
At least 1 uppercase letter. |
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 pick, but to match the designs exactly, let's remove periods from the ends of all of the complexity text lines
src/app/auth/signup.tsx
Outdated
<Text | ||
style={ | ||
hasLowercase | ||
? { color: colors.textGreen } |
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: looking at the designs, we want this fontSize to be 12 and there to be a gap of 8 between the icon and the text. To do this, we can replace these lines with
style={[ styles.errorText, hasLength ? { color: colors.textGreen } : { color: colors.textRed }, ]}
where the styles for errorText include fontSize: 12 and marginLeft: 8
src/styles/globalStyles.ts
Outdated
passwordComplexity: { | ||
display: 'flex', | ||
flexDirection: 'row', | ||
}, |
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.
Delete from globalStyles once moved into the local styles sheet
…nt/girls-write-now into inaya/auth-style-login-signup
What's new in this PR
on sign in screen, now checks for the 4 requirements for password complexity (minimum 1 uppercase, 1 lowercase, 1 number, and 8 characters total) and updates it beneath the password input with a check/x icon and green/red. also disables sign up button until complexity is met.
Relevant Links
n/a
Notion Sprint Task
https://www.notion.so/calblueprint/auth-Style-login-signup-screens-726c755253824e5bb89498c578e2e103
Online sources
n/a
Related PRs
styling that adi is working on?
How to review
n/a
Next steps
dependent on adi's styling
Tests Performed, Edge Cases
n/a
Screenshots
CC: @akshaynthakur