-
Notifications
You must be signed in to change notification settings - Fork 1
Google Auth - Implement Auth Components and Add Icon Assets (Part 2) #2
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
Conversation
…o andrew/google-auth
…o andrew/google-auth
…o andrew/google-auth
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.
Pull Request Overview
This PR implements Google authentication components and adds supporting icon assets for the Hustle Android app. It sets up the foundation for Google sign-in functionality with UI components and required dependencies.
- Adds Firebase authentication dependencies and Google sign-in libraries
- Creates reusable UI components for authentication (GoogleSignInButton and ErrorMessage)
- Adds vector drawable icons for navigation and UI elements
Reviewed Changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| gradle/libs.versions.toml | Adds Firebase, credential manager, and Google sign-in library dependencies |
| app/build.gradle.kts | Includes Firebase BOM, authentication, and credential management dependencies |
| app/src/main/res/values/strings.xml | Adds Google OAuth client ID for authentication configuration |
| app/src/main/AndroidManifest.xml | Adds internet and network access permissions required for authentication |
| GoogleSignInButton.kt | Implements reusable Google sign-in button component with loading states |
| ErrorMessage.kt | Creates dismissible error message component for authentication errors |
| Multiple drawable files | Adds vector icons for Google, warning, profile, messages, home, and close actions |
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> | |||
Copilot
AI
Sep 26, 2025
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.
The Google OAuth client ID should not be hardcoded in the strings.xml file as it could be exposed in the repository. Consider moving this to a secure configuration file or environment variable that is not committed to version control.
| <string name="default_web_client_id">1054767037583-oh008pkkqev5veggoa896b8lj3e4hd86.apps.googleusercontent.com</string> |
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.
Is fixed in related PR (part 1)
zachseidner1
left a comment
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.
LGTM 🚀
| xmlns:tools="http://schemas.android.com/tools"> | ||
|
|
||
| <uses-permission android:name="android.permission.INTERNET" /> | ||
| <uses-permission android:name="android.permission.ACCESS_NETWORK_STATE" /> |
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.
Curious about why you need ACCESS_NETWORK_STATE? Haven't seen this permission before
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.
Tbh, I thought it was just internet, but in their documentation https://developer.android.com/develop/connectivity/network-ops/connecting, they included access network state. I think it is mainly to allow us to check if the device is connected to the internet or not
| verticalAlignment = Alignment.CenterVertically, | ||
| horizontalArrangement = Arrangement.spacedBy(12.dp) | ||
| ) { | ||
| if (isLoading) { |
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: if you want you could do an animated content with isLoading as the key to make this look a little cooler
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.
Sounds like a great idea, and I would normally add this, but I think I will hold off for now just until I get the actual auth screen designs in case the button + loading state is different.
Overview
Implemented Auth Components and Add Icon Assets
Changes Made
Test Coverage
Next Steps
Related PRs or Issues
Screenshots
Screen.Recording.2025-09-26.at.3.32.38.PM.mov
Notes