Google Auth - Implement Basic Auth Screens (Part 3) - #3
Conversation
…ign out, minor change in empty HomeScreen
There was a problem hiding this comment.
Pull Request Overview
This PR implements basic authentication screens for Google Sign-In functionality, adding SignIn and Profile screens to test authentication flow.
- Added new SignInScreen with Google Sign-In button and error handling
- Added ProfileScreen displaying user information and sign-out functionality
- Updated dependencies to include Firebase authentication and credential manager libraries
Reviewed Changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| gradle/libs.versions.toml | Added Firebase, credential manager, and Google ID dependencies |
| app/src/main/res/values/strings.xml | Added Google OAuth client ID configuration |
| app/src/main/java/com/cornellappdev/hustle/ui/screens/profile/ProfileScreen.kt | New profile screen with user info display and sign-out functionality |
| app/src/main/java/com/cornellappdev/hustle/ui/screens/onboarding/SignInScreen.kt | New sign-in screen with Google authentication button |
| app/src/main/java/com/cornellappdev/hustle/ui/screens/home/HomeScreen.kt | Added missing import for HomeScreenViewModel |
| app/build.gradle.kts | Added Firebase and authentication dependencies with organized comments |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| @@ -1,3 +1,4 @@ | |||
| <resources> | |||
| <string name="app_name">Hustle</string> | |||
| <string name="default_web_client_id">1054767037583-oh008pkkqev5veggoa896b8lj3e4hd86.apps.googleusercontent.com</string> | |||
There was a problem hiding this comment.
The Google OAuth client ID should not be hardcoded in the repository. Consider using build variants or environment variables to manage sensitive configuration data, especially for production builds.
| <string name="default_web_client_id">1054767037583-oh008pkkqev5veggoa896b8lj3e4hd86.apps.googleusercontent.com</string> | |
| <string name="default_web_client_id">${GOOGLE_OAUTH_CLIENT_ID}</string> |
zachseidner1
left a comment
zachseidner1
left a comment
There was a problem hiding this comment.
Nice work overall, but some questions about ProfileScreen parameters
|
|
||
| @Composable | ||
| fun SignInScreen( | ||
| signInWithGoogle: () -> Unit, |
There was a problem hiding this comment.
nit: the name onGoogleSignInButtonClick sort of makes the UI dumber, and just more closely follows standard naming conventions
| signInWithGoogle: () -> Unit, | ||
| isLoading: Boolean, | ||
| errorMessage: String?, | ||
| clearError: () -> Unit, |
There was a problem hiding this comment.
nit: same thing here this could be called onDismissError or something
| class SignInErrorMessageProvider : PreviewParameterProvider<String?> { | ||
| override val values = sequenceOf( | ||
| null, | ||
| "Sign in failed. Please sign in with your Cornell email" | ||
| ) |
There was a problem hiding this comment.
this makes me happy to see you are using this!
|
|
||
| @Composable | ||
| fun ProfileScreen( | ||
| user: User?, |
There was a problem hiding this comment.
in what case would we navigate to the profile screen with a null user? I feel like this should be non-null?
There was a problem hiding this comment.
Oh yeah, good point. I forgot that for hustle, we're enforcing people to sign up unlike some of our other apps.
| fun ProfileScreen( | ||
| user: User?, | ||
| onSignOut: () -> Unit, | ||
| isLoading: Boolean, |
There was a problem hiding this comment.
This name is unclear, I think ifSignOutLoading would be much better. At first I thought you meant that the whole screen could be loading here.
There was a problem hiding this comment.
Good point, will change
Overview
Implemented Basic Auth Screens (Sign In, Profile) for testing
Changes Made
Test Coverage
Next Steps
Related PRs or Issues
Screenshots
Screen.Recording.2025-09-26.at.3.32.38.PM.mov
Notes