-
-
Notifications
You must be signed in to change notification settings - Fork 974
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
docs(firebase_login): add firebase login example app #3946
base: master
Are you sure you want to change the base?
docs(firebase_login): add firebase login example app #3946
Conversation
Warning Rate limit exceeded@thisissandipp has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 17 minutes and 7 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughThis pull request introduces a comprehensive Firebase login example application using Flutter and Riverpod. The project establishes a complete authentication flow with sign-up, sign-in, and home pages, integrating Firebase authentication with Google Sign-In. The implementation includes robust error handling, state management, and a structured approach to managing user authentication across different screens. Additionally, it enhances project configuration with new metadata, documentation, and dependency management. Changes
Possibly related PRs
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
@@ -0,0 +1 @@ | |||
{"flutter":{"platforms":{"android":{"default":{"projectId":"fb-login-riverpod","appId":"1:359355392679:android:942c58c04ec3c1f105f182","fileOutput":"android/app/google-services.json"}},"ios":{"default":{"projectId":"fb-login-riverpod","appId":"1:359355392679:ios:3e31b15579c4b6e005f182","uploadDebugSymbols":false,"fileOutput":"ios/Runner/GoogleService-Info.plist"}},"dart":{"lib/firebase_options.dart":{"projectId":"fb-login-riverpod","configurations":{"android":"1:359355392679:android:942c58c04ec3c1f105f182","ios":"1:359355392679:ios:3e31b15579c4b6e005f182"}}}}}} |
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.
Don't commit this
// gestures. You can also use WidgetTester to find child widgets in the widget | ||
// tree, read text, and verify that the values of widget properties are correct. | ||
|
||
import 'package:flutter/material.dart'; |
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.
Either write proper tests or delete this
I don't like the proposed folder architecture. Please don't use kind-based folders such as Instead use feature-based folders like |
@@ -0,0 +1,70 @@ | |||
// File generated by FlutterFire CLI. |
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.
Don't commit this
@@ -0,0 +1,40 @@ | |||
import 'dart:developer'; | |||
|
|||
import 'package:flutter_riverpod/flutter_riverpod.dart'; |
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 isn't useful in the context of a Firebase example
colorScheme: ColorScheme.fromSeed(seedColor: Colors.indigo), | ||
useMaterial3: false, | ||
), | ||
home: authState.when( |
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.
Don't use .when
Instead use switch (authState)
Thanks for this! I gave a very quick look at the current state, and suggested broad changes. I'll look move closely at the details once we agree on the basics :) Note: It'd be useful to update the README with:
|
Thanks for the quick review and feedback, Remi. I'll make the changes as suggested, and I'll make sure to verify the architecture with you before I start moving things around, so we’re on the same page. I'll add instructions to the README as well. |
Cool :) If you have any doubt, feel free to ask questions! |
@rrousselGit I have considered your suggestions and come up with this structure, what do you think about it? lib/
├── auth/
│ ├── auth_repository.dart
│ ├── user.dart
├── app/
│ ├── app_state.dart // e.g., app state provider (auth state can be renamed)
│ ├── app.dart // MaterialApp widget, route initialization
├── sign_in/
│ ├── sign_in_page.dart
│ ├── sign_in_state.dart // Providers related to sign-in
├── sign_up/
│ ├── sign_up_page.dart
│ ├── sign_up_state.dart // Providers related to sign-up
├── home/
│ ├── home_page.dart
├── main.dart
├── provider_observer.dart Also, I noticed your comment regarding If you feel it doesn't add enough value here, I'm happy to remove it! I just thought it might be beneficial to keep it for the debugging phase and as a teaching point for others reviewing the example. |
Must better!
I think it's better to remove it. IMO, less is more. The more noise we add, the harder it is for people to find what they are looking for. Demonstrating how to do those things is valuable. But it should likely be a separate example instead. This one is about Firebase auth. We can have another example for demonstrating logging (including Crashlytics interaction). If this example contains something, it should be closely related to Firebase Auth. |
Actually about the folder structure, I'd still change a few things. What I'd personally do is:
For UI, I usually go with "one file per route". It's good to keep the provider and its view in the same file. No need to separate them |
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.
Actionable comments posted: 9
🧹 Nitpick comments (10)
examples/firebase_login/lib/auth/auth_repository.dart (4)
17-55
: Sufficient coverage of sign-up error codes.The class neatly enumerates common Firebase Auth errors like
'invalid-email'
,'email-already-in-use'
, etc. However, consider including additional codes such as'network-request-failed'
or'too-many-requests'
in the future, especially if your flow needs to handle them.
167-168
: Use a consistent error message style.All other failures include a user-friendly message. For consistency and user-friendliness, store an error message in
SignOutFailure
or consider mirroring the pattern of other exception classes.
170-194
: Consider usingidTokenChanges()
rather thanauthStateChanges()
.
authStateChanges()
doesn’t always capture changes with token refreshes, whileidTokenChanges()
listens for more granular auth events. Evaluate whether your use case needs these additional updates.
281-286
: Check null-safety for displayName and photoURL.Currently
displayName
andphotoURL
can benull
. Consider fallback defaults if needed by the UI to avoid rendering issues or null references.examples/firebase_login/test/home_test.dart (1)
19-28
: Enhance test coverage for HomePage.While the sign-out test is good, consider adding tests for:
- User data display (ID, name, email)
- Error states when user data is null
- Loading states while data is being fetched
Example test case:
testWidgets('displays user information correctly', (tester) async { final user = User( id: 'test-id', name: 'Test User', email: '[email protected]', isAuthenticated: true, ); when(() => authRepository.user).thenAnswer( (_) => Stream.value(user), ); await tester.pumpApp( MaterialApp(home: HomePage()), authRepository: authRepository, ); expect(find.text('User ID: test-id'), findsOneWidget); expect(find.text('Name: Test User'), findsOneWidget); expect(find.text('Email: [email protected]'), findsOneWidget); });examples/firebase_login/lib/home/home.dart (1)
19-23
: Add confirmation dialog before sign-out.Consider adding a confirmation dialog before signing out to prevent accidental sign-outs.
IconButton( - onPressed: authProvider.signOut, + onPressed: () async { + final confirm = await showDialog<bool>( + context: context, + builder: (context) => AlertDialog( + title: const Text('Sign Out'), + content: const Text('Are you sure you want to sign out?'), + actions: [ + TextButton( + onPressed: () => Navigator.pop(context, false), + child: const Text('Cancel'), + ), + TextButton( + onPressed: () => Navigator.pop(context, true), + child: const Text('Sign Out'), + ), + ], + ), + ); + if (confirm == true) { + await authProvider.signOut(); + } + }, icon: const Icon(Icons.logout), ),examples/firebase_login/lib/main.dart (2)
42-46
: Simplify authentication flow logic.The nested ternary operators make the code harder to read. Consider simplifying the condition since both
user == null
and!user.isAuthenticated
lead toSignInPage
.- home: user == null - ? const SignInPage() - : user.isAuthenticated - ? const HomePage() - : const SignInPage(), + home: user?.isAuthenticated == true + ? const HomePage() + : const SignInPage(),
38-41
: Document Material 3 decision.The code explicitly disables Material 3. Consider adding a comment explaining why, or enable it for a more modern UI.
theme: ThemeData( colorScheme: ColorScheme.fromSeed(seedColor: Colors.indigo), - useMaterial3: false, + // TODO: Consider enabling Material 3 for a more modern UI + useMaterial3: true, ),examples/firebase_login/test/app_test.dart (1)
21-53
: Add tests for edge cases in authentication state handling.The tests cover basic authentication states well. Consider adding:
- Tests for error states in the user stream
- Tests for state transitions (e.g., sign-out)
- Tests for error handling during state changes
Example test case for error state:
testWidgets( 'handles error in user stream gracefully', (tester) async { when(() => authRepository.user).thenAnswer( (_) => Stream.error(Exception('Network error')), ); await tester.pumpApp(App(), authRepository: authRepository); await tester.pumpAndSettle(); expect(find.byType(ErrorWidget), findsOneWidget); }, );examples/firebase_login/lib/sign_up/sign_up.dart (1)
147-149
: Improve error handling for unexpected errors.The catch-all error handler provides a generic message. Consider:
- Logging the error for debugging
- Providing more specific error messages based on error types
- } catch (_) { - state = const SignUpState.failure('An unexpected error occured!'); + } catch (e, stackTrace) { + debugPrint('Unexpected error during sign up: $e\n$stackTrace'); + state = SignUpState.failure( + 'Unable to create account. Please try again later.', + );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
examples/firebase_login/samples/demo.gif
is excluded by!**/*.gif
examples/firebase_login/samples/error.png
is excluded by!**/*.png
examples/firebase_login/samples/home.png
is excluded by!**/*.png
examples/firebase_login/samples/signup.png
is excluded by!**/*.png
📒 Files selected for processing (22)
examples/firebase_login/.gitignore
(1 hunks)examples/firebase_login/.metadata
(1 hunks)examples/firebase_login/README.md
(1 hunks)examples/firebase_login/analysis_options.yaml
(1 hunks)examples/firebase_login/lib/auth/auth_repository.dart
(1 hunks)examples/firebase_login/lib/auth/auth_repository.g.dart
(1 hunks)examples/firebase_login/lib/auth/user.dart
(1 hunks)examples/firebase_login/lib/auth/user.freezed.dart
(1 hunks)examples/firebase_login/lib/home/home.dart
(1 hunks)examples/firebase_login/lib/main.dart
(1 hunks)examples/firebase_login/lib/main.g.dart
(1 hunks)examples/firebase_login/lib/sign_in/sign_in.dart
(1 hunks)examples/firebase_login/lib/sign_in/sign_in.freezed.dart
(1 hunks)examples/firebase_login/lib/sign_up/sign_up.dart
(1 hunks)examples/firebase_login/lib/sign_up/sign_up.freezed.dart
(1 hunks)examples/firebase_login/pubspec.yaml
(1 hunks)examples/firebase_login/pubspec_overrides.yaml
(1 hunks)examples/firebase_login/test/app_test.dart
(1 hunks)examples/firebase_login/test/home_test.dart
(1 hunks)examples/firebase_login/test/pump_app.dart
(1 hunks)examples/firebase_login/test/sign_in_test.dart
(1 hunks)examples/firebase_login/test/sign_up_test.dart
(1 hunks)
✅ Files skipped from review due to trivial changes (6)
- examples/firebase_login/analysis_options.yaml
- examples/firebase_login/README.md
- examples/firebase_login/pubspec.yaml
- examples/firebase_login/.metadata
- examples/firebase_login/.gitignore
- examples/firebase_login/lib/auth/auth_repository.g.dart
👮 Files not reviewed due to content moderation or server errors (4)
- examples/firebase_login/lib/sign_in/sign_in.dart
- examples/firebase_login/lib/auth/user.freezed.dart
- examples/firebase_login/lib/sign_in/sign_in.freezed.dart
- examples/firebase_login/lib/sign_up/sign_up.freezed.dart
🔇 Additional comments (15)
examples/firebase_login/lib/auth/auth_repository.dart (7)
1-7
: Imports look good.Using
package:firebase_auth/firebase_auth.dart
with an alias and referencingkIsWeb
fromfoundation.dart
is an appropriate approach for conditional web logic. No immediate issues here.
8-8
: Appropriate use of part file.It's standard practice to separate generated code. Ensure the corresponding
.g.dart
file is properly generated and version-controlled if required.
108-166
: Validate presence of relevant Google error codes.While most Google sign-in errors are handled, confirm that
'wrong-password'
truly arises with Google sign-in flows. If it's not relevant to this provider, removing it reduces confusion.
195-213
: Sign-up flow looks correct.Ensure that your UI layer properly handles
SignUpWithEmailAndPasswordFailure
. The rethrowing approach is straightforward, and the code is well-encapsulated.
215-233
: Robust sign-in logic.The error-handling mirrors the sign-up flow. This is good for consistency. Just be mindful of your UI responding appropriately to exceptions.
265-279
: Multiple awaits in sign-out.Using
Future.wait
is a good approach to sign out from Firebase and Google simultaneously. No issues found here.
288-301
: Riverpod providers are properly separated.You’re cleanly injecting dependencies and simplifying testing. This fosters maintainability. No concerns here.
examples/firebase_login/test/pump_app.dart (3)
1-6
: Imports are concise and relevant.No extraneous imports spotted. Using the appropriate testing, mocking, and Flutter libraries.
7-7
: MockAuthRepository is well-defined.Implementing the authentication interface with a mock is a standard approach for testing. Looks good.
9-22
: Reusable test harness withpumpApp
.Overriding
authRepositoryProvider
in aProviderScope
is an excellent choice for controlled testing. Ensure to set up method mocks forMockAuthRepository
to handle any method calls your tests rely on.examples/firebase_login/lib/auth/user.dart (1)
1-4
: Importingfreezed_annotation
is optimal for immutability.Ensure you’ve run the code generation to produce
user.freezed.dart
.examples/firebase_login/lib/main.g.dart (1)
1-26
: LGTM!The generated code follows Riverpod's patterns correctly and includes proper deprecation notices.
examples/firebase_login/lib/sign_up/sign_up.dart (1)
30-31
:⚠️ Potential issueFix the Column widget usage.
The
spacing
parameter is not a valid property ofColumn
. UsemainAxisSpacing
in aColumn
wrapped with aListView
for better scrolling support.- child: Column( - spacing: 16, + child: ListView( + children: [ + Column( + mainAxisSpacing: 16,Likely invalid or redundant comment.
examples/firebase_login/pubspec_overrides.yaml (2)
1-1
: LGTM! Melos management is properly configured.The comment correctly indicates that Melos manages the dependency overrides for the core Riverpod packages.
2-14
: Verify the relative paths to the Riverpod packages.The dependency overrides look correct for local development. However, let's verify that all the specified paths exist and are accessible.
Also, consider adding a comment explaining that these overrides are for development purposes and should not be committed to production.
# melos_managed_dependency_overrides: flutter_riverpod,riverpod,riverpod_lint,riverpod_analyzer_utils +# Note: These overrides are for development purposes only and should not be committed to production. dependency_overrides:
✅ Verification successful
All Riverpod package paths verified and accessible
All relative paths in the dependency overrides are correct and point to existing package directories.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that all Riverpod package paths exist # Navigate to the example directory and check each package path cd examples/firebase_login || exit 1 packages=( "../../packages/flutter_riverpod" "../../packages/riverpod" "../../packages/riverpod_analyzer_utils" "../../packages/riverpod_lint" "../../packages/riverpod_annotation" "../../packages/riverpod_generator" ) for package in "${packages[@]}"; do if [ ! -d "$package" ]; then echo "❌ Package path not found: $package" else echo "✅ Package path exists: $package" fi doneLength of output: 1461
/// {@template sign_in_with_email_and_password_failure} | ||
/// Thrown during the sign in process if a failure occurs. | ||
/// | ||
/// * Check the [Reference API][ref link] for updated error codes. | ||
/// | ||
/// [ref link]: https://pub.dev/documentation/firebase_auth/latest/firebase_auth/FirebaseAuth/signInWithEmailAndPassword.html | ||
/// {@endtemplate} | ||
class SignInWithEmailAndPasswordFailure implements Exception { | ||
/// {@macro sign_in_with_email_and_password_failure} | ||
const SignInWithEmailAndPasswordFailure([ | ||
this.message = 'An unknown error occurred.', | ||
]); | ||
|
||
/// Creates a failure message from a [firebase_auth.FirebaseAuthException] code. | ||
/// | ||
/// {@macro sign_in_with_email_and_password_failure} | ||
factory SignInWithEmailAndPasswordFailure.fromCode(String code) { | ||
switch (code) { | ||
case 'invalid-email': | ||
return const SignInWithEmailAndPasswordFailure( | ||
'The email is not valid or badly formatted.', | ||
); | ||
case 'user-disabled': | ||
return const SignInWithEmailAndPasswordFailure( | ||
'This user account has been disabled. Please contact support.', | ||
); | ||
case 'user-not-found': | ||
return const SignInWithEmailAndPasswordFailure( | ||
'No user found with this email. Please check the email or sign up.', | ||
); | ||
case 'wrong-password': | ||
return const SignInWithEmailAndPasswordFailure( | ||
'Incorrect password. Please try again.', | ||
); | ||
case 'invalid-credential': | ||
return const SignInWithEmailAndPasswordFailure( | ||
'The credential provided is invalid. Please try again or use a different method.', | ||
); | ||
|
||
default: | ||
return const SignInWithEmailAndPasswordFailure(); | ||
} | ||
} | ||
|
||
/// The associated error message. | ||
final String message; | ||
|
||
@override | ||
String toString() => message; | ||
} |
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.
🛠️ Refactor suggestion
Comprehensive sign-in error handling.
Similar to the sign-up class, this covers a wide range of errors. Consider factoring out a common base class or utility method to avoid duplication across authentication failure classes.
/// Triggers the google sign in flow. | ||
/// | ||
/// Throws [SignInWithGoogleFailure] when an exception occurs | ||
/// during the operation. | ||
Future<void> signInWithGoogle() async { | ||
try { | ||
late final firebase_auth.AuthCredential credential; | ||
if (kIsWeb) { | ||
final googleProvider = firebase_auth.GoogleAuthProvider(); | ||
final userCredential = await _firebaseAuth.signInWithPopup( | ||
googleProvider, | ||
); | ||
credential = userCredential.credential!; | ||
} else { | ||
final googleAccount = await _googleSignIn.signIn(); | ||
final googleAuthentication = await googleAccount!.authentication; | ||
credential = firebase_auth.GoogleAuthProvider.credential( | ||
accessToken: googleAuthentication.accessToken, | ||
idToken: googleAuthentication.idToken, | ||
); | ||
} | ||
|
||
await _firebaseAuth.signInWithCredential(credential); | ||
} on firebase_auth.FirebaseAuthException catch (e) { | ||
throw SignInWithGoogleFailure.fromCode(e.code); | ||
} catch (_) { | ||
throw const SignInWithGoogleFailure(); | ||
} | ||
} |
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.
Handle cancellation or null from GoogleSignIn.
If the user closes the Google sign-in dialog (line 249), googleAccount
can be null
. Force-unwrapping googleAccount!
can cause a runtime error. Consider throwing a specific exception or returning early if googleAccount
is null
.
Proposed safe check:
249-250
- final googleAccount = await _googleSignIn.signIn();
- final googleAuthentication = await googleAccount!.authentication;
+ final googleAccount = await _googleSignIn.signIn();
+ if (googleAccount == null) {
+ throw const SignInWithGoogleFailure('User canceled Google sign-in.');
+ }
+ final googleAuthentication = await googleAccount.authentication;
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
/// Triggers the google sign in flow. | |
/// | |
/// Throws [SignInWithGoogleFailure] when an exception occurs | |
/// during the operation. | |
Future<void> signInWithGoogle() async { | |
try { | |
late final firebase_auth.AuthCredential credential; | |
if (kIsWeb) { | |
final googleProvider = firebase_auth.GoogleAuthProvider(); | |
final userCredential = await _firebaseAuth.signInWithPopup( | |
googleProvider, | |
); | |
credential = userCredential.credential!; | |
} else { | |
final googleAccount = await _googleSignIn.signIn(); | |
final googleAuthentication = await googleAccount!.authentication; | |
credential = firebase_auth.GoogleAuthProvider.credential( | |
accessToken: googleAuthentication.accessToken, | |
idToken: googleAuthentication.idToken, | |
); | |
} | |
await _firebaseAuth.signInWithCredential(credential); | |
} on firebase_auth.FirebaseAuthException catch (e) { | |
throw SignInWithGoogleFailure.fromCode(e.code); | |
} catch (_) { | |
throw const SignInWithGoogleFailure(); | |
} | |
} | |
/// Triggers the google sign in flow. | |
/// | |
/// Throws [SignInWithGoogleFailure] when an exception occurs | |
/// during the operation. | |
Future<void> signInWithGoogle() async { | |
try { | |
late final firebase_auth.AuthCredential credential; | |
if (kIsWeb) { | |
final googleProvider = firebase_auth.GoogleAuthProvider(); | |
final userCredential = await _firebaseAuth.signInWithPopup( | |
googleProvider, | |
); | |
credential = userCredential.credential!; | |
} else { | |
final googleAccount = await _googleSignIn.signIn(); | |
if (googleAccount == null) { | |
throw const SignInWithGoogleFailure('User canceled Google sign-in.'); | |
} | |
final googleAuthentication = await googleAccount.authentication; | |
credential = firebase_auth.GoogleAuthProvider.credential( | |
accessToken: googleAuthentication.accessToken, | |
idToken: googleAuthentication.idToken, | |
); | |
} | |
await _firebaseAuth.signInWithCredential(credential); | |
} on firebase_auth.FirebaseAuthException catch (e) { | |
throw SignInWithGoogleFailure.fromCode(e.code); | |
} catch (_) { | |
throw const SignInWithGoogleFailure(); | |
} | |
} |
child: Column( | ||
mainAxisSize: MainAxisSize.min, | ||
crossAxisAlignment: CrossAxisAlignment.start, | ||
spacing: 16, | ||
children: [ | ||
Text('User ID: ${user?.id ?? ''}'), | ||
Text('Name: ${user?.name ?? ''}'), | ||
Text('Email: ${user?.email ?? ''}'), | ||
], | ||
), |
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.
Fix Column properties and add spacing.
The Column
widget doesn't have a spacing
property. Use a combination of children
and SizedBox
for spacing.
child: Column(
mainAxisSize: MainAxisSize.min,
crossAxisAlignment: CrossAxisAlignment.start,
- spacing: 16,
children: [
Text('User ID: ${user?.id ?? ''}'),
+ const SizedBox(height: 16),
Text('Name: ${user?.name ?? ''}'),
+ const SizedBox(height: 16),
Text('Email: ${user?.email ?? ''}'),
],
),
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
child: Column( | |
mainAxisSize: MainAxisSize.min, | |
crossAxisAlignment: CrossAxisAlignment.start, | |
spacing: 16, | |
children: [ | |
Text('User ID: ${user?.id ?? ''}'), | |
Text('Name: ${user?.name ?? ''}'), | |
Text('Email: ${user?.email ?? ''}'), | |
], | |
), | |
child: Column( | |
mainAxisSize: MainAxisSize.min, | |
crossAxisAlignment: CrossAxisAlignment.start, | |
children: [ | |
Text('User ID: ${user?.id ?? ''}'), | |
const SizedBox(height: 16), | |
Text('Name: ${user?.name ?? ''}'), | |
const SizedBox(height: 16), | |
Text('Email: ${user?.email ?? ''}'), | |
], | |
), |
Widget build(BuildContext context, WidgetRef ref) { | ||
final authProvider = ref.read(authRepositoryProvider); | ||
final user = ref.watch(userProvider).value; | ||
|
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.
🛠️ Refactor suggestion
Add loading and error states.
The current implementation doesn't handle loading states or errors when fetching user data.
Widget build(BuildContext context, WidgetRef ref) {
final authProvider = ref.read(authRepositoryProvider);
- final user = ref.watch(userProvider).value;
+ final userAsync = ref.watch(userProvider);
+
+ return userAsync.when(
+ loading: () => const Scaffold(
+ body: Center(child: CircularProgressIndicator()),
+ ),
+ error: (error, stack) => Scaffold(
+ body: Center(child: Text('Error: $error')),
+ ),
+ data: (user) => Scaffold(
Committable suggestion skipped: line range outside the PR's diff.
Future<void> main() async { | ||
WidgetsFlutterBinding.ensureInitialized(); | ||
await Firebase.initializeApp( | ||
options: DefaultFirebaseOptions.currentPlatform, | ||
); | ||
|
||
runApp( | ||
const ProviderScope( | ||
child: App(), | ||
), | ||
); | ||
} |
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.
🛠️ Refactor suggestion
Add error handling for Firebase initialization.
The Firebase initialization should handle potential errors that might occur during setup.
Future<void> main() async {
WidgetsFlutterBinding.ensureInitialized();
+ try {
await Firebase.initializeApp(
options: DefaultFirebaseOptions.currentPlatform,
);
+ } catch (e, stack) {
+ debugPrint('Failed to initialize Firebase: $e\n$stack');
+ // Consider showing an error screen or retrying
+ rethrow;
+ }
runApp(
const ProviderScope(
child: App(),
),
);
}
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Future<void> main() async { | |
WidgetsFlutterBinding.ensureInitialized(); | |
await Firebase.initializeApp( | |
options: DefaultFirebaseOptions.currentPlatform, | |
); | |
runApp( | |
const ProviderScope( | |
child: App(), | |
), | |
); | |
} | |
Future<void> main() async { | |
WidgetsFlutterBinding.ensureInitialized(); | |
try { | |
await Firebase.initializeApp( | |
options: DefaultFirebaseOptions.currentPlatform, | |
); | |
} catch (e, stack) { | |
debugPrint('Failed to initialize Firebase: $e\n$stack'); | |
// Consider showing an error screen or retrying | |
rethrow; | |
} | |
runApp( | |
const ProviderScope( | |
child: App(), | |
), | |
); | |
} |
group('SignUp', () { | ||
testWidgets( | ||
'triggers signUpWithEmailAndPassword on submit button click', | ||
(tester) async { | ||
await tester.pumpApp( | ||
MaterialApp(home: SignUpPage()), | ||
authRepository: authRepository, | ||
); | ||
|
||
await tester.enterText(find.byKey(emailKey), '[email protected]'); | ||
await tester.enterText(find.byKey(passwordKey), 's3cRe7'); | ||
|
||
await tester.tap(find.byType(ElevatedButton)); | ||
verify( | ||
() => authRepository.signUpWithEmailAndPassword( | ||
email: '[email protected]', | ||
password: 's3cRe7', | ||
), | ||
).called(1); | ||
}, | ||
); | ||
}); |
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.
🛠️ Refactor suggestion
Enhance test coverage for sign-up functionality.
The current test only covers the happy path. Consider adding tests for:
- Error scenarios (invalid email, weak password)
- Form validation
- Loading state verification
- Error message display
Example test case for error scenario:
testWidgets(
'displays error message when sign up fails',
(tester) async {
when(
() => authRepository.signUpWithEmailAndPassword(
email: any(named: 'email'),
password: any(named: 'password'),
),
).thenThrow(SignUpWithEmailAndPasswordFailure('Invalid email'));
await tester.pumpApp(
MaterialApp(home: SignUpPage()),
authRepository: authRepository,
);
await tester.enterText(find.byKey(emailKey), 'invalid-email');
await tester.enterText(find.byKey(passwordKey), 'password');
await tester.tap(find.byType(ElevatedButton));
await tester.pump();
expect(find.text('Invalid email'), findsOneWidget);
},
);
group('SignIn', () { | ||
testWidgets( | ||
'triggers signInWithEmailAndPassword on submit button click', | ||
(tester) async { | ||
await tester.pumpApp( | ||
MaterialApp(home: SignInPage()), | ||
authRepository: authRepository, | ||
); | ||
|
||
await tester.enterText(find.byKey(emailKey), '[email protected]'); | ||
await tester.enterText(find.byKey(passwordKey), 's3cRe7'); | ||
|
||
await tester.tap(find.byKey(emailSignInKey)); | ||
verify( | ||
() => authRepository.signInWithEmailAndPassword( | ||
email: '[email protected]', | ||
password: 's3cRe7', | ||
), | ||
).called(1); | ||
}, | ||
); | ||
|
||
testWidgets( | ||
'triggers signInWithGoogle on google button click', | ||
(tester) async { | ||
await tester.pumpApp( | ||
MaterialApp(home: SignInPage()), | ||
authRepository: authRepository, | ||
); | ||
|
||
await tester.tap(find.byKey(googleSignInKey)); | ||
verify( | ||
() => authRepository.signInWithGoogle(), | ||
).called(1); | ||
}, | ||
); | ||
}); |
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.
🛠️ Refactor suggestion
Enhance test coverage for sign-in functionality.
The tests only cover successful sign-in scenarios. Consider adding:
- Tests for authentication failures
- Tests for loading states during sign-in
- Tests for form validation
- Tests for navigation after successful sign-in
Example test case for loading state:
testWidgets(
'shows loading indicator during sign in',
(tester) async {
final completer = Completer<void>();
when(
() => authRepository.signInWithEmailAndPassword(
email: any(named: 'email'),
password: any(named: 'password'),
),
).thenAnswer((_) => completer.future);
await tester.pumpApp(
MaterialApp(home: SignInPage()),
authRepository: authRepository,
);
await tester.enterText(find.byKey(emailKey), '[email protected]');
await tester.enterText(find.byKey(passwordKey), 'password');
await tester.tap(find.byKey(emailSignInKey));
await tester.pump();
expect(find.byType(CircularProgressIndicator), findsOneWidget);
},
);
TextFormField( | ||
key: const Key('email_textFormField'), | ||
controller: _emailController, | ||
decoration: const InputDecoration( | ||
labelText: 'Email address', | ||
border: OutlineInputBorder(), | ||
), | ||
), | ||
TextFormField( | ||
key: const Key('password_textFormField'), | ||
controller: _passwordController, | ||
obscureText: true, | ||
decoration: const InputDecoration( | ||
labelText: 'Password', | ||
border: OutlineInputBorder(), | ||
), | ||
), |
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.
🛠️ Refactor suggestion
Add form validation and accessibility improvements.
The form fields lack:
- Input validation (email format, password strength)
- Accessibility labels
- Password requirements display
Apply this diff to enhance the form fields:
TextFormField(
key: const Key('email_textFormField'),
controller: _emailController,
+ autovalidateMode: AutovalidateMode.onUserInteraction,
+ validator: (value) {
+ if (value == null || !value.contains('@')) {
+ return 'Please enter a valid email';
+ }
+ return null;
+ },
decoration: const InputDecoration(
labelText: 'Email address',
border: OutlineInputBorder(),
+ semanticsLabel: 'Email address input field',
),
),
TextFormField(
key: const Key('password_textFormField'),
controller: _passwordController,
obscureText: true,
+ autovalidateMode: AutovalidateMode.onUserInteraction,
+ validator: (value) {
+ if (value == null || value.length < 8) {
+ return 'Password must be at least 8 characters';
+ }
+ return null;
+ },
decoration: const InputDecoration(
labelText: 'Password',
border: OutlineInputBorder(),
+ semanticsLabel: 'Password input field',
+ helperText: 'Password must be at least 8 characters long',
),
),
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
TextFormField( | |
key: const Key('email_textFormField'), | |
controller: _emailController, | |
decoration: const InputDecoration( | |
labelText: 'Email address', | |
border: OutlineInputBorder(), | |
), | |
), | |
TextFormField( | |
key: const Key('password_textFormField'), | |
controller: _passwordController, | |
obscureText: true, | |
decoration: const InputDecoration( | |
labelText: 'Password', | |
border: OutlineInputBorder(), | |
), | |
), | |
TextFormField( | |
key: const Key('email_textFormField'), | |
controller: _emailController, | |
autovalidateMode: AutovalidateMode.onUserInteraction, | |
validator: (value) { | |
if (value == null || !value.contains('@')) { | |
return 'Please enter a valid email'; | |
} | |
return null; | |
}, | |
decoration: const InputDecoration( | |
labelText: 'Email address', | |
border: OutlineInputBorder(), | |
semanticsLabel: 'Email address input field', | |
), | |
), | |
TextFormField( | |
key: const Key('password_textFormField'), | |
controller: _passwordController, | |
obscureText: true, | |
autovalidateMode: AutovalidateMode.onUserInteraction, | |
validator: (value) { | |
if (value == null || value.length < 8) { | |
return 'Password must be at least 8 characters'; | |
} | |
return null; | |
}, | |
decoration: const InputDecoration( | |
labelText: 'Password', | |
border: OutlineInputBorder(), | |
semanticsLabel: 'Password input field', | |
helperText: 'Password must be at least 8 characters long', | |
), | |
), |
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.
Actionable comments posted: 1
🧹 Nitpick comments (2)
examples/firebase_login/README.md (2)
1-4
: Enhance the introduction section.Consider expanding the description to better highlight the purpose and tech stack:
# Firebase Login -Example flutter app built with Riverpod that demonstrates authentication with Firebase. +A comprehensive example Flutter application demonstrating Firebase Authentication integration using Riverpod state management. This educational example showcases best practices for implementing user authentication in Flutter applications.
14-20
: Expand the features section to include all implemented functionality.Based on the PR objectives, consider adding these additional features to the list:
## Features - Sign up with Email and Password - Sign in with Email and Password - Sign in with Google - Error handling +- State management with Riverpod +- Automatic navigation based on authentication state +- Error handling with SnackBar notifications +- Comprehensive error messages for authentication failures
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
examples/firebase_login/README.md
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (19)
- GitHub Check: build (master, packages/riverpod_generator/integration/build_yaml)
- GitHub Check: build (master, packages/riverpod_generator)
- GitHub Check: build (master, packages/riverpod_annotation)
- GitHub Check: build (master, packages/hooks_riverpod/example)
- GitHub Check: build (master, packages/hooks_riverpod)
- GitHub Check: build (master, packages/flutter_riverpod/example)
- GitHub Check: build (master, packages/flutter_riverpod)
- GitHub Check: build (master, examples/todos)
- GitHub Check: build (master, examples/stackoverflow)
- GitHub Check: build (master, examples/random_number)
- GitHub Check: build (master, examples/pub)
- GitHub Check: build (master, examples/marvel)
- GitHub Check: build (master, examples/counter)
- GitHub Check: riverpod_lint (stable, packages/riverpod_lint_flutter_test)
- GitHub Check: riverpod_lint (stable, packages/riverpod_analyzer_utils_tests)
- GitHub Check: riverpod_lint (master, packages/riverpod_lint_flutter_test)
- GitHub Check: riverpod_lint (master, packages/riverpod_lint)
- GitHub Check: riverpod_lint (master, packages/riverpod_analyzer_utils_tests)
- GitHub Check: check_generation
🔇 Additional comments (1)
examples/firebase_login/README.md (1)
5-12
: Improve image accessibility and add descriptive captions.The images need more descriptive alt text and captions to explain what each screenshot demonstrates.
<table> <tr> - <td><img src="./samples/demo.gif" alt="demo" width="320"></td> - <td><img src="./samples/error.png" alt="error" width="320"></td> - <td><img src="./samples/signup.png" alt="signup" width="320"></td> - <td><img src="./samples/home.png" alt="home" width="320"></td> + <td> + <img src="./samples/demo.gif" alt="Demo showing the complete authentication flow" width="320"> + <p align="center">Complete Auth Flow</p> + </td> + <td> + <img src="./samples/error.png" alt="Error handling demonstration with SnackBar" width="320"> + <p align="center">Error Handling</p> + </td> + <td> + <img src="./samples/signup.png" alt="Sign up screen with email and password fields" width="320"> + <p align="center">Sign Up Screen</p> + </td> + <td> + <img src="./samples/home.png" alt="Home screen showing authenticated user information" width="320"> + <p align="center">Home Screen</p> + </td> </tr> </table>Also, let's verify the existence of these images:
✅ Verification successful
All referenced images exist, proceed with accessibility improvements
The verification confirms that all images referenced in the README table exist in the correct location. The suggested improvements for better alt text and captions remain valid for enhanced accessibility.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if the referenced images exist in the samples directory fd -t f -e png -e gif . examples/firebase_login/samples/Length of output: 225
## Getting Started | ||
|
||
1. Generate the native folders first by running `flutter create --platforms=android,ios,web,windows,macos .` | ||
2. Create your firebase project, and enable email/password authentication and google. | ||
3. Use [FlutterFire CLI](https://firebase.google.com/docs/flutter/setup?platform=ios) to connect the app with the firebase project. | ||
4. Follow the [platform integration steps](https://pub.dev/packages/google_sign_in#platform-integration) for the `google_sign_in` package. | ||
5. Run the app. | ||
6. To run the tests, run `flutter test`. |
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.
🛠️ Refactor suggestion
Enhance the getting started section with prerequisites and detailed setup steps.
Consider expanding this section to include:
## Getting Started
+### Prerequisites
+
+- Flutter SDK (3.0.0 or higher)
+- Firebase CLI and FlutterFire CLI installed
+- A Firebase account
+
+### Setup Steps
+
1. Generate the native folders first by running `flutter create --platforms=android,ios,web,windows,macos .`
2. Create your firebase project, and enable email/password authentication and google.
+ - Go to Firebase Console
+ - Create a new project
+ - Navigate to Authentication section
+ - Enable Email/Password provider
+ - Enable Google Sign-in provider
+ - Configure OAuth consent screen
3. Use [FlutterFire CLI](https://firebase.google.com/docs/flutter/setup?platform=ios) to connect the app with the firebase project.
+ ```bash
+ flutterfire configure
+ ```
4. Follow the [platform integration steps](https://pub.dev/packages/google_sign_in#platform-integration) for the `google_sign_in` package.
5. Run the app.
6. To run the tests, run `flutter test`.
+
+### Troubleshooting
+
+If you encounter any issues:
+- Ensure all Firebase configuration files are properly generated
+- Verify SHA-1 fingerprint is correctly added to Firebase project (Android)
+- Check iOS bundle ID matches Firebase configuration
Description
Add a Flutter app example demonstrating how to handle Firebase authentication using Riverpod.
Checklist
Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]
).I have updated the
CHANGELOG.md
of the relevant packages.Changelog files must be edited under the form:
If this contains new features or behavior changes,
I have updated the documentation to match those changes.
Summary by CodeRabbit
Based on the comprehensive summary, here are the updated release notes:
New Features
Documentation
Testing
Development Tools