-
Notifications
You must be signed in to change notification settings - Fork 55
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
feat: enhance login screen with social login options #373
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: kshitij79 <[email protected]>
Signed-off-by: kshitij79 <[email protected]>
Signed-off-by: kshitij79 <[email protected]>
…to-52 chore: bump expo to 52
Signed-off-by: kshitij79 <[email protected]>
…mponent feat: implement user information component
…cripts chore: add missing workspace scripts in package.json
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.
Great job on these social buttons 🎉 I have a few improvement suggestions:
1- Refactor for Reusability – The social buttons share a similar structure. Could you extract a reusable SocialButton component that accepts props for provider, iconName, onPress? This would make the code more DRY (Don't Repeat Yourself).
2- Library Consistency – I noticed react-native-vector-icons is being used, but @expo/vector-icons is already installed. Any specific reason for this choice?
9173085
to
fa00544
Compare
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.
Suggestions & Fixes
1. Avoid Hardcoding Uppercase Titles
Issue: Hardcoding uppercase titles, like "LOG IN WITH GMAIL", reduces flexibility and maintainability.
Suggestion: Apply uppercase styling through CSS or styles, not directly in the text.
2. Rename Styles for Better Clarity and Consistency
Suggestion
socialButtonText → buttonText: Generalizes the name to make it more reusable.
socialButton → button: Simplifies the name while keeping it clear and concise.
forgotPasswordContainer → forgotPasswordSection: "Section" better represents the grouping of elements.
forgotPasswordText → forgotPasswordLabel: "Label" is more suitable for descriptive text.
resetText → resetLink: This reflects its purpose as a clickable link.
orText → dividerText: Makes it clear that this text divides sections.
signupContainer → signupSection: Consistent naming with other section elements.
signupText → signupPrompt: "Prompt" makes it clearer that it’s encouraging action.
signupLink → signupActionLink: "ActionLink" makes it clear that it’s a clickable action.
socialButton → socialButtonContainer: More descriptive and consistent with naming.
hey, i think every thing is good now😅 |
Description
Please provide a clear and concise description of the changes made, including
the purpose and context.
resolves #352
Changes Made
Changes in
apps
folder (specify the app and briefly describe thechanges):
Web
Native
Changes in
packages
folder (specify the package and briefly describethe changes):
Core
Type of Change
functionality to not work as expected)
Screenshots
How Has This Been Tested?
Checklist:
Additional Comments
(Optional) Add any additional comments or notes for reviewers here.