feat: Implemented forgot password and verify OTP functionality #56
Conversation
This commit introduces the following changes: - Added a forgot password feature, allowing users to reset their passwords. - Implemented a verify OTP (One-Time Password) screen for enhanced security. - Updated the authentication repository to include methods for forgot password and OTP verification. - Added relevant translations for the new features. - Integrated the forgot password flow into the sign-in screen. - Updated API endpoints for forgot password and OTP verification. - Added necessary dependencies, including `pinput` for OTP input.
WalkthroughThis update introduces a complete "Forgot Password" and "Verify OTP" flow to the application. It adds new API endpoints, authentication model changes, repository methods, BLoC classes, events, and states for both features. Corresponding screens, routes, and UI elements are implemented, along with new localization keys and a dependency for OTP input handling. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SignInScreen
participant ForgotPasswordScreen
participant ForgotPasswordBloc
participant AuthRepository
participant VerifyOTPScreen
participant VerifyOTPBloc
User->>SignInScreen: Tap "Forgot Password"
SignInScreen->>ForgotPasswordScreen: Navigate
ForgotPasswordScreen->>ForgotPasswordBloc: Enter email & submit
ForgotPasswordBloc->>AuthRepository: forgotPassword(email)
AuthRepository-->>ForgotPasswordBloc: Success/Failure
ForgotPasswordBloc-->>ForgotPasswordScreen: State update (success)
ForgotPasswordScreen->>VerifyOTPScreen: Navigate with email
User->>VerifyOTPScreen: Enter OTP & submit
VerifyOTPScreen->>VerifyOTPBloc: Submit OTP
VerifyOTPBloc->>AuthRepository: verifyOTP(email, otp)
AuthRepository-->>VerifyOTPBloc: Success/Failure
VerifyOTPBloc-->>VerifyOTPScreen: State update (success/failure)
VerifyOTPScreen->>ChangePasswordScreen: Navigate on success
Suggested reviewers
Poem
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (5)
apps/app_core/lib/app/config/api_endpoints.dart (1)
4-5: Consider consistent endpoint naming and versioning.The new endpoints work functionally but consider consistency with existing patterns:
- Versioning: The
loginendpoint uses/api/v1/, consider if these should follow the same pattern- Naming convention: Consider using consistent naming (e.g.,
forgot-passwordvsforgotPassword)Consider this for better consistency:
- static const forgotPassword = '/api/forgotPassword'; - static const verifyOTP = '/api/verifyOTP'; + static const forgotPassword = '/api/v1/forgot-password'; + static const verifyOTP = '/api/v1/verify-otp';However, ensure backend endpoints match these changes if implemented.
apps/app_core/lib/modules/verify_otp/bloc/verify_otp_bloc.dart (1)
46-67: Consider making OTP length configurable.The OTP length is hard-coded to 6 digits. Consider making this configurable through a constant or configuration to improve maintainability.
+ static const int _otpLength = 6; + Future<Unit> _onVerifyOTPChanged(VerifyOTPChanged event, Emitter<VerifyOTPState> emit) async { - if (event.otp.length == 6) { + if (event.otp.length == _otpLength) {apps/app_core/lib/modules/auth/repository/auth_repository.dart (1)
123-138: Consider consistent error handling in forgotPassword method.The
forgotPasswordmethod maps the response tovoidbut doesn't handle specific error cases. Consider whether you need to handle specific HTTP status codes or API errors differently.The current implementation is functional but could benefit from more specific error handling if the API returns meaningful error messages for different failure scenarios.
apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart (2)
188-189: Extract hardcoded string to localization.The text "Didn't receive the verification OTP?" should be moved to localization files for consistency.
- text: "Didn't receive the verification OTP?", + text: context.t.didnt_receive_otp, // Add this key to translations
227-239: Remove unusedCountdownwidget.The
Countdownwidget is defined but never used in the screen. Consider removing it to reduce code clutter.If this widget is intended for future use, consider moving it to a separate file or adding a TODO comment explaining its purpose.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
apps/app_core/lib/app/config/api_endpoints.dart(1 hunks)apps/app_core/lib/app/routes/app_router.dart(3 hunks)apps/app_core/lib/modules/auth/model/auth_request_model.dart(4 hunks)apps/app_core/lib/modules/auth/repository/auth_repository.dart(5 hunks)apps/app_core/lib/modules/auth/sign_in/screens/sign_in_screen.dart(5 hunks)apps/app_core/lib/modules/forgot_password/bloc/forgot_password_bloc.dart(1 hunks)apps/app_core/lib/modules/forgot_password/bloc/forgot_password_event.dart(1 hunks)apps/app_core/lib/modules/forgot_password/bloc/forgot_password_state.dart(1 hunks)apps/app_core/lib/modules/forgot_password/screens/forgot_password_screen.dart(1 hunks)apps/app_core/lib/modules/verify_otp/bloc/verify_otp_bloc.dart(1 hunks)apps/app_core/lib/modules/verify_otp/bloc/verify_otp_event.dart(1 hunks)apps/app_core/lib/modules/verify_otp/bloc/verify_otp_state.dart(1 hunks)apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart(1 hunks)apps/app_core/pubspec.yaml(1 hunks)packages/app_translations/assets/i18n/en.i18n.json(1 hunks)
🔇 Additional comments (13)
packages/app_translations/assets/i18n/en.i18n.json (1)
57-65: Well-structured localization additions.The new translation keys comprehensively cover the forgot password and OTP verification flows with clear, user-friendly messages. The naming convention is consistent with existing translations.
apps/app_core/lib/modules/auth/sign_in/screens/sign_in_screen.dart (2)
68-83: Well-implemented forgot password navigation.The forgot password link is properly integrated with:
- Appropriate UI positioning and styling
- Correct navigation to
ForgotPasswordRoute- Proper use of translation strings
- Consistent animation delay with other elements
30-30: Good code formatting improvements.The formatting changes improve code readability by removing unnecessary line breaks and braces while maintaining functionality.
Also applies to: 93-93, 133-133, 150-150
apps/app_core/lib/app/routes/app_router.dart (3)
7-7: Proper imports for new screen modules.The imports correctly reference the new forgot password and verify OTP screen modules.
Also applies to: 13-13
27-28: Routes properly integrated into navigation flow.The new routes for
ForgotPasswordRouteandVerifyOTPRouteare correctly added to the routes list and follow the existing routing patterns.
39-39: Good formatting improvement.The single-line format for
ChangePasswordRouteimproves readability while maintaining all the original metadata.apps/app_core/lib/modules/forgot_password/bloc/forgot_password_event.dart (1)
3-21: LGTM! Well-structured event definitions following BLoC best practices.The use of sealed classes enables exhaustive pattern matching and the Equatable implementation ensures proper state comparison in the BLoC.
apps/app_core/lib/modules/auth/model/auth_request_model.dart (1)
13-15: LGTM! Clean model extensions following existing patterns.The new named constructors and serialization methods are well-implemented and maintain consistency with the existing codebase structure. The addition of the
tokenfield supports the OTP verification flow appropriately.Also applies to: 25-25, 36-41, 56-60
apps/app_core/lib/modules/forgot_password/screens/forgot_password_screen.dart (1)
1-108: LGTM! Well-structured forgot password screen implementation.The screen follows Flutter and BLoC best practices with proper:
- Repository and BLoC provider setup
- State management and error handling
- UI separation of concerns with dedicated widgets
- Navigation flow to OTP verification
- Localization and accessibility features
The implementation is clean and follows established patterns in the codebase.
apps/app_core/lib/modules/verify_otp/bloc/verify_otp_bloc.dart (1)
69-91: LGTM! Proper implementation of resend email functionality.The resend email logic correctly:
- Resets OTP state when resending
- Handles both success and error cases
- Uses proper state management with different status flags
apps/app_core/lib/modules/auth/repository/auth_repository.dart (2)
22-26: LGTM! Proper interface extension for new authentication methods.The interface correctly defines the new methods with appropriate return types using
TaskEitherfor functional error handling.
140-154: LGTM! Consistent verifyOTP implementation.The
verifyOTPmethod follows the same pattern as other authentication methods with proper response mapping and error handling.apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart (1)
64-82: LGTM! Well-implemented timer functionality.The timer implementation correctly:
- Manages the countdown state
- Prevents multiple timers from running
- Properly disposes of resources
- Updates the UI appropriately
apps/app_core/lib/modules/verify_otp/bloc/verify_otp_event.dart
Outdated
Show resolved
Hide resolved
apps/app_core/lib/modules/verify_otp/bloc/verify_otp_state.dart
Outdated
Show resolved
Hide resolved
apps/app_core/lib/modules/forgot_password/bloc/forgot_password_bloc.dart
Outdated
Show resolved
Hide resolved
apps/app_core/lib/modules/forgot_password/bloc/forgot_password_state.dart
Outdated
Show resolved
Hide resolved
apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart
Outdated
Show resolved
Hide resolved
apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart
Outdated
Show resolved
Hide resolved
- Updated ForgotPasswordBloc to handle success and failure cases. - Included errorMessage in ForgotPasswordState props. - Added static OTP validation in VerifyOTPScreen. - Updated VerifyOTPState and VerifyOTPEvent to include props for better state management and event handling. - Removed skeletonizer dependency override from widgetbook.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart (2)
149-155: Email field appears editable but serves as display-only.The email field is styled as a regular input field but only displays the initial value without proper state management or validation. This could confuse users who might expect to edit it.
Consider making it explicitly read-only or connecting it to state management:
AppTextField( initialValue: widget.emailAddress, + readOnly: true, label: context.t.email, - textInputAction: TextInputAction.done, - keyboardType: TextInputType.emailAddress, - autofillHints: const [AutofillHints.email], ),
228-240: Remove unused Countdown widget.The Countdown widget class is defined but never used in the screen implementation, making it dead code.
-class Countdown extends AnimatedWidget { - Countdown({super.key, this.animation}) : super(listenable: animation!); - final Animation<int>? animation; - - @override - Widget build(BuildContext context) { - final clockTimer = Duration(seconds: animation!.value); - - final timerText = '${clockTimer.inMinutes.remainder(60)}:${clockTimer.inSeconds.remainder(60).toString().padLeft(2, '0')}'; - - return AppText(text: timerText); - } -}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/app_core/lib/modules/forgot_password/bloc/forgot_password_bloc.dart(1 hunks)apps/app_core/lib/modules/forgot_password/bloc/forgot_password_state.dart(1 hunks)apps/app_core/lib/modules/verify_otp/bloc/verify_otp_event.dart(1 hunks)apps/app_core/lib/modules/verify_otp/bloc/verify_otp_state.dart(1 hunks)apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart(1 hunks)apps/app_core/pubspec.yaml(2 hunks)packages/widgetbook/pubspec_overrides.yaml(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/widgetbook/pubspec_overrides.yaml
🚧 Files skipped from review as they are similar to previous changes (5)
- apps/app_core/pubspec.yaml
- apps/app_core/lib/modules/verify_otp/bloc/verify_otp_state.dart
- apps/app_core/lib/modules/forgot_password/bloc/forgot_password_bloc.dart
- apps/app_core/lib/modules/forgot_password/bloc/forgot_password_state.dart
- apps/app_core/lib/modules/verify_otp/bloc/verify_otp_event.dart
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: tulsi-7span
PR: 7span/dashing-kit#56
File: apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart:98-112
Timestamp: 2025-06-26T11:54:23.384Z
Learning: In the verify OTP screen (apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart), the team uses static OTP validation with hardcoded value "222222" for testing success states during development, rather than calling the actual API verification.
apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart (1)
Learnt from: tulsi-7span
PR: 7span/dashing-kit#56
File: apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart:98-112
Timestamp: 2025-06-26T11:54:23.384Z
Learning: In the verify OTP screen (apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart), the team uses static OTP validation with hardcoded value "222222" for testing success states during development, rather than calling the actual API verification.
🔇 Additional comments (4)
apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart (4)
98-112: Static OTP validation is intentionally implemented for testing.Based on the team's previous feedback, this static validation with "222222" is intentionally kept for development/testing purposes rather than using actual API verification.
166-169: Static validator aligns with testing approach.The Pinput validator uses static validation consistent with the team's current testing strategy for the success flow.
64-82: Timer implementation is well-structured.The timer logic properly handles the 30-second countdown with appropriate state management and cleanup.
125-136: Error handling and success feedback are properly implemented.The BlocConsumer listener correctly handles both error states and success states for OTP operations with appropriate user feedback.
apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart
Outdated
Show resolved
Hide resolved
- Updated API endpoints for forgot password and verify OTP. - Set email field to read-only on the verify OTP screen. - Removed unused `Countdown` widget. - Standardized OTP length to 6 digits in `VerifyOTPBloc`. - Added `readOnly` property to `AppTextField` widget.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart (1)
64-82: Consider adding timer state check in _startTimer method.The timer management looks good overall, but consider adding a safety check to ensure the widget is still mounted before calling setState.
void _startTimer() { + if (!mounted) return; setState(() { _secondsRemaining = 30; _isTimerRunning = true; }); _timer?.cancel(); _timer = Timer.periodic(const Duration(seconds: 1), (timer) { if (_secondsRemaining > 0) { + if (!mounted) { + timer.cancel(); + return; + } setState(() { _secondsRemaining--; }); } else { + if (!mounted) { + timer.cancel(); + return; + } setState(() { _isTimerRunning = false; }); _timer?.cancel(); } }); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/app_core/lib/app/config/api_endpoints.dart(1 hunks)apps/app_core/lib/modules/verify_otp/bloc/verify_otp_bloc.dart(1 hunks)apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart(1 hunks)packages/app_ui/lib/src/widgets/molecules/app_textfield.dart(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/app_core/lib/app/config/api_endpoints.dart
- apps/app_core/lib/modules/verify_otp/bloc/verify_otp_bloc.dart
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: tulsi-7span
PR: 7span/dashing-kit#56
File: apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart:98-112
Timestamp: 2025-06-26T11:54:23.384Z
Learning: In the verify OTP screen (apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart), the team uses static OTP validation with hardcoded value "222222" for testing success states during development, rather than calling the actual API verification.
apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart (1)
Learnt from: tulsi-7span
PR: 7span/dashing-kit#56
File: apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart:98-112
Timestamp: 2025-06-26T11:54:23.384Z
Learning: In the verify OTP screen (apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart), the team uses static OTP validation with hardcoded value "222222" for testing success states during development, rather than calling the actual API verification.
🔇 Additional comments (4)
packages/app_ui/lib/src/widgets/molecules/app_textfield.dart (1)
12-12: LGTM! Clean implementation of readOnly functionality.The
readOnlyproperty is properly implemented across both constructors and correctly passed to the underlyingTextFormFieldwith appropriate null safety handling.Also applies to: 41-41, 51-51, 101-101
apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart (3)
149-149: Good integration with the enhanced AppTextField.The use of the new
readOnly: trueproperty effectively displays the email address as non-editable, which is perfect for the OTP verification context.
153-171: Well-implemented Pinput widget with proper integration.The OTP input implementation is clean with proper separation builder, validation, and BLoC integration for state updates.
84-96: Solid resend OTP implementation with proper validation.The method correctly validates email presence, clears the input, manages focus, triggers the BLoC event, and restarts the timer. Good defensive programming with the email null check.
Incremented the counter to 2.
# Conflicts: # packages/app_ui/lib/src/widgets/molecules/app_textfield.dart
- Removed unnecessary space between the left and right EdgeInsets.
This commit introduces a new `AppTimer` widget and integrates it into the OTP verification screen. Key changes: - Added `AppTimer` widget to `app_ui` for displaying a countdown timer. - Integrated `AppTimer` into `VerifyOTPScreen` to show the OTP resend countdown. - Refactored `VerifyOTPScreen` to use `AppTimer` and removed manual timer logic. - Updated `VerifyOTPBloc` to use `LengthValidator` for OTP validation. - Modified navigation from `ForgotPasswordScreen` to `VerifyOTPScreen` to use `replaceRoute` instead of `pushRoute`. - Ensured OTP input is cleared and focus is removed when resending OTP. - Displayed error messages using `SnackbarType.failed` for invalid OTP. - Updated Pinput widget to show error text based on `state.otp.error`. - Allowed only digits in OTP input using `FilteringTextInputFormatter.digitsOnly`.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/app_ui/lib/src/widgets/molecules/app_timer.dart (1)
17-17: Remove unused TickerProviderStateMixin.The widget includes
TickerProviderStateMixinbut doesn't use any animation controllers. This adds unnecessary overhead.-class _AppTimerState extends State<AppTimer> with TickerProviderStateMixin { +class _AppTimerState extends State<AppTimer> {apps/app_core/lib/modules/verify_otp/bloc/verify_otp_state.dart (1)
37-37: Consider validating email as well in the isValid getter.The current implementation only validates the OTP, but since this state also manages email validation, consider whether both should be validated for completeness.
-bool get isValid => otp.isValid; +bool get isValid => otp.isValid && email.isValid;However, if the email is always expected to be valid when reaching OTP verification (validated in previous step), the current implementation is acceptable.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/app_core/lib/modules/forgot_password/screens/forgot_password_screen.dart(1 hunks)apps/app_core/lib/modules/verify_otp/bloc/verify_otp_bloc.dart(1 hunks)apps/app_core/lib/modules/verify_otp/bloc/verify_otp_state.dart(1 hunks)apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart(1 hunks)packages/app_ui/lib/src/widgets/molecules/app_textfield.dart(5 hunks)packages/app_ui/lib/src/widgets/molecules/app_timer.dart(1 hunks)packages/app_ui/lib/src/widgets/molecules/molecules.dart(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/app_ui/lib/src/widgets/molecules/molecules.dart
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/app_ui/lib/src/widgets/molecules/app_textfield.dart
- apps/app_core/lib/modules/forgot_password/screens/forgot_password_screen.dart
- apps/app_core/lib/modules/verify_otp/bloc/verify_otp_bloc.dart
- apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: tulsi-7span
PR: 7span/dashing-kit#56
File: apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart:98-112
Timestamp: 2025-06-26T11:54:23.384Z
Learning: In the verify OTP screen (apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart), the team uses static OTP validation with hardcoded value "222222" for testing success states during development, rather than calling the actual API verification.
apps/app_core/lib/modules/verify_otp/bloc/verify_otp_state.dart (1)
Learnt from: tulsi-7span
PR: 7span/dashing-kit#56
File: apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart:98-112
Timestamp: 2025-06-26T11:54:23.384Z
Learning: In the verify OTP screen (apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart), the team uses static OTP validation with hardcoded value "222222" for testing success states during development, rather than calling the actual API verification.
🔇 Additional comments (4)
packages/app_ui/lib/src/widgets/molecules/app_timer.dart (2)
38-51: Well-implemented timer logic with proper state management.The timer implementation correctly handles:
- Periodic updates with proper state management
- Callback invocations for both tick and completion events
- Timer cancellation to prevent memory leaks
- Edge case handling when countdown reaches zero
28-36: Excellent handling of widget updates.The
didUpdateWidgetmethod properly restarts the timer when thesecondsparameter changes, ensuring the widget remains responsive to configuration updates.apps/app_core/lib/modules/verify_otp/bloc/verify_otp_state.dart (2)
6-13: Well-structured state class with sensible defaults.The constructor provides appropriate default values:
- Initial API statuses for both operations
- Pure validators for email and OTP
- Empty error message
This ensures the state is always in a valid initial state.
15-29: Correct copyWith implementation.The copyWith method now properly preserves existing values when parameters are null, including the previously problematic
errorMessagefield. This ensures immutable state updates work as expected.
- Removed unused `formKey` from `VerifyOTPScreen`. - Simplified `AppTimer` widget by removing `onTick` and `textStyle` parameters. - Updated `VerifyOTPState`'s `isValid` getter to include email validation.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/app_ui/lib/src/widgets/molecules/app_timer.dart (2)
37-49: Consider simplifying the callback invocation and adding input validation.The timer logic is generally sound, but there are a few areas for improvement:
- The callback check can be simplified
- Consider validating the input to prevent unexpected behavior
void _startTimer() { + if (widget.seconds <= 0) return; // Guard against invalid input _timer?.cancel(); _timer = Timer.periodic(const Duration(seconds: 1), (timer) { if (_secondsRemaining > 0) { setState(() { _secondsRemaining--; }); } else { timer.cancel(); - if (widget.onFinished != null) widget.onFinished?.call(); + widget.onFinished?.call(); } }); }
57-62: Consider the time format limitation for longer durations.The current format
"00:ss"only supports up to 59 seconds. While this is likely sufficient for OTP resend timers (typically 30 seconds), consider whether the component might be used for longer durations in the future.If you need to support minutes, consider this format:
Widget build(BuildContext context) { - final timerText = '00:${_secondsRemaining.toString().padLeft(2, '0')}'; + final minutes = _secondsRemaining ~/ 60; + final seconds = _secondsRemaining % 60; + final timerText = '${minutes.toString().padLeft(2, '0')}:${seconds.toString().padLeft(2, '0')}'; return AppText(text: timerText, style: context.textTheme?.sSemiBold.copyWith(color: context.colorScheme.primary400)); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/app_core/lib/modules/verify_otp/bloc/verify_otp_state.dart(1 hunks)apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart(1 hunks)packages/app_ui/lib/src/widgets/molecules/app_timer.dart(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/app_core/lib/modules/verify_otp/bloc/verify_otp_state.dart
- apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: tulsi-7span
PR: 7span/dashing-kit#56
File: apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart:98-112
Timestamp: 2025-06-26T11:54:23.384Z
Learning: In the verify OTP screen (apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart), the team uses static OTP validation with hardcoded value "222222" for testing success states during development, rather than calling the actual API verification.
🔇 Additional comments (3)
packages/app_ui/lib/src/widgets/molecules/app_timer.dart (3)
6-14: LGTM! Clean constructor with appropriate parameters.The widget constructor follows Flutter conventions with required
secondsparameter and optionalonFinishedcallback. The use ofsuper.keyis correct for modern Flutter development.
27-35: Good widget lifecycle management.Properly handles widget updates by canceling the existing timer and restarting with new duration. This ensures the timer stays synchronized when the parent widget updates the
secondsparameter.
51-55: Excellent resource management.Proper timer disposal prevents memory leaks. This is essential for widgets that manage resources like timers.
The `AppTimer` widget was modified to prevent the timer from starting if the initial `seconds` value is 0. Additionally, the `onFinished` callback is now always called when the timer finishes, even if it's null.
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
apps/app_core/pubspec.yaml (1)
92-92: Dependency correctly placed in production dependencies.The
pinputpackage is correctly placed in the regular dependencies section, which ensures it's available at runtime for OTP input functionality. This resolves the previous issue mentioned in past reviews.apps/app_core/lib/modules/forgot_password/bloc/forgot_password_state.dart (1)
26-26: Good - errorMessage is included in props list.The previous issue where
errorMessagewas missing from the props list has been properly addressed. All fields are now included for correct state comparison.
🧹 Nitpick comments (1)
apps/app_core/lib/modules/verify_otp/bloc/verify_otp_event.dart (1)
18-31: Improve consistency by using final class for all concrete events.While the sealed class pattern is correctly implemented, there's inconsistency in the concrete event classes. Some use
final classwhile others useclass. For sealed class hierarchies, it's recommended to usefinal classconsistently to prevent further inheritance.Apply this diff to improve consistency:
-class EmailAddressChanged extends VerifyOTPEvent { +final class EmailAddressChanged extends VerifyOTPEvent { const EmailAddressChanged(this.email); final String email; @override List<Object> get props => [email]; } -class VerifyButtonPressed extends VerifyOTPEvent { +final class VerifyButtonPressed extends VerifyOTPEvent { const VerifyButtonPressed(); } -class ResendEmailEvent extends VerifyOTPEvent { +final class ResendEmailEvent extends VerifyOTPEvent { const ResendEmailEvent(); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
BOILERPLATE/COUNTER(1 hunks)apps/app_core/lib/app/config/api_endpoints.dart(1 hunks)apps/app_core/lib/app/routes/app_router.dart(3 hunks)apps/app_core/lib/modules/auth/model/auth_request_model.dart(4 hunks)apps/app_core/lib/modules/auth/repository/auth_repository.dart(5 hunks)apps/app_core/lib/modules/auth/sign_in/screens/sign_in_screen.dart(5 hunks)apps/app_core/lib/modules/forgot_password/bloc/forgot_password_bloc.dart(1 hunks)apps/app_core/lib/modules/forgot_password/bloc/forgot_password_event.dart(1 hunks)apps/app_core/lib/modules/forgot_password/bloc/forgot_password_state.dart(1 hunks)apps/app_core/lib/modules/forgot_password/screens/forgot_password_screen.dart(1 hunks)apps/app_core/lib/modules/verify_otp/bloc/verify_otp_bloc.dart(1 hunks)apps/app_core/lib/modules/verify_otp/bloc/verify_otp_event.dart(1 hunks)apps/app_core/lib/modules/verify_otp/bloc/verify_otp_state.dart(1 hunks)apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart(1 hunks)apps/app_core/pubspec.yaml(2 hunks)packages/app_translations/assets/i18n/en.i18n.json(1 hunks)packages/app_ui/lib/src/widgets/molecules/app_textfield.dart(5 hunks)packages/app_ui/lib/src/widgets/molecules/app_timer.dart(1 hunks)packages/app_ui/lib/src/widgets/molecules/molecules.dart(1 hunks)packages/widgetbook/pubspec_overrides.yaml(1 hunks)
🧰 Additional context used
🧠 Learnings (10)
📓 Common learnings
Learnt from: tulsi-7span
PR: 7span/dashing-kit#56
File: apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart:98-112
Timestamp: 2025-06-26T11:54:23.384Z
Learning: In the verify OTP screen (apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart), the team uses static OTP validation with hardcoded value "222222" for testing success states during development, rather than calling the actual API verification.
apps/app_core/lib/app/config/api_endpoints.dart (1)
Learnt from: tulsi-7span
PR: 7span/dashing-kit#56
File: apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart:98-112
Timestamp: 2025-06-26T11:54:23.384Z
Learning: In the verify OTP screen (apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart), the team uses static OTP validation with hardcoded value "222222" for testing success states during development, rather than calling the actual API verification.
apps/app_core/lib/app/routes/app_router.dart (1)
Learnt from: tulsi-7span
PR: 7span/dashing-kit#56
File: apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart:98-112
Timestamp: 2025-06-26T11:54:23.384Z
Learning: In the verify OTP screen (apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart), the team uses static OTP validation with hardcoded value "222222" for testing success states during development, rather than calling the actual API verification.
apps/app_core/lib/modules/forgot_password/screens/forgot_password_screen.dart (1)
Learnt from: tulsi-7span
PR: 7span/dashing-kit#56
File: apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart:98-112
Timestamp: 2025-06-26T11:54:23.384Z
Learning: In the verify OTP screen (apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart), the team uses static OTP validation with hardcoded value "222222" for testing success states during development, rather than calling the actual API verification.
apps/app_core/lib/modules/verify_otp/bloc/verify_otp_event.dart (1)
Learnt from: tulsi-7span
PR: 7span/dashing-kit#56
File: apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart:98-112
Timestamp: 2025-06-26T11:54:23.384Z
Learning: In the verify OTP screen (apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart), the team uses static OTP validation with hardcoded value "222222" for testing success states during development, rather than calling the actual API verification.
apps/app_core/lib/modules/verify_otp/bloc/verify_otp_state.dart (1)
Learnt from: tulsi-7span
PR: 7span/dashing-kit#56
File: apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart:98-112
Timestamp: 2025-06-26T11:54:23.384Z
Learning: In the verify OTP screen (apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart), the team uses static OTP validation with hardcoded value "222222" for testing success states during development, rather than calling the actual API verification.
apps/app_core/lib/modules/auth/model/auth_request_model.dart (1)
Learnt from: tulsi-7span
PR: 7span/dashing-kit#56
File: apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart:98-112
Timestamp: 2025-06-26T11:54:23.384Z
Learning: In the verify OTP screen (apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart), the team uses static OTP validation with hardcoded value "222222" for testing success states during development, rather than calling the actual API verification.
apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart (1)
Learnt from: tulsi-7span
PR: 7span/dashing-kit#56
File: apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart:98-112
Timestamp: 2025-06-26T11:54:23.384Z
Learning: In the verify OTP screen (apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart), the team uses static OTP validation with hardcoded value "222222" for testing success states during development, rather than calling the actual API verification.
apps/app_core/lib/modules/verify_otp/bloc/verify_otp_bloc.dart (1)
Learnt from: tulsi-7span
PR: 7span/dashing-kit#56
File: apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart:98-112
Timestamp: 2025-06-26T11:54:23.384Z
Learning: In the verify OTP screen (apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart), the team uses static OTP validation with hardcoded value "222222" for testing success states during development, rather than calling the actual API verification.
apps/app_core/lib/modules/auth/repository/auth_repository.dart (1)
Learnt from: tulsi-7span
PR: 7span/dashing-kit#56
File: apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart:98-112
Timestamp: 2025-06-26T11:54:23.384Z
Learning: In the verify OTP screen (apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart), the team uses static OTP validation with hardcoded value "222222" for testing success states during development, rather than calling the actual API verification.
🔇 Additional comments (21)
BOILERPLATE/COUNTER (1)
2-2: Counter increment looks good.Simple counter increment from 1 to 2, likely for build or deployment purposes.
packages/widgetbook/pubspec_overrides.yaml (1)
1-4: Dependency override cleanup looks good.The removal of the
skeletonizeroverride simplifies the configuration while maintaining the necessaryapp_uioverride.packages/app_ui/lib/src/widgets/molecules/molecules.dart (1)
10-10: Timer widget export aligns with OTP functionality requirements.The
app_timer.dartexport is appropriately added to support the new OTP verification features, which typically require countdown timers for resend functionality.apps/app_core/lib/app/config/api_endpoints.dart (1)
4-5: New API endpoints properly structured and aligned with feature requirements.The
forgotPasswordandverifyOTPendpoints follow consistent naming conventions and URL patterns, properly supporting the new authentication flows implemented in this PR.packages/app_translations/assets/i18n/en.i18n.json (1)
57-66: LGTM! Translation keys are well-structured.The new localization keys properly support the forgot password and OTP verification features with clear, descriptive naming that follows the existing conventions.
apps/app_core/lib/modules/auth/sign_in/screens/sign_in_screen.dart (1)
68-84: LGTM! Forgot password link properly integrated.The implementation correctly adds the forgot password functionality with:
- Proper animation and gesture detection
- Correct navigation to
ForgotPasswordRoute- Appropriate styling and alignment
- Proper use of localization
packages/app_ui/lib/src/widgets/molecules/app_textfield.dart (1)
12-12: LGTM! ReadOnly property correctly implemented.The
readOnlyproperty is properly added to both constructors, correctly declared as a nullable field, and appropriately used in theTextFormFieldwith a sensible default value.Also applies to: 42-42, 53-53, 104-104
apps/app_core/lib/app/routes/app_router.dart (1)
7-7: LGTM! New routes properly configured.The
ForgotPasswordRouteandVerifyOTPRouteare correctly added with proper imports, enabling navigation to the new authentication flows.Also applies to: 13-13, 27-28
apps/app_core/lib/modules/forgot_password/bloc/forgot_password_event.dart (1)
1-21: LGTM! Event structure follows best practices.The event definitions correctly implement the bloc pattern with:
- Proper use of sealed classes for type safety
- Appropriate
Equatableimplementation for value equality- Well-named events that clearly represent user actions
- Clean separation between input change and submission events
packages/app_ui/lib/src/widgets/molecules/app_timer.dart (1)
1-63: Well-implemented countdown timer widget!The implementation follows Flutter best practices with proper timer management, state lifecycle handling, and memory leak prevention. The widget correctly handles dynamic updates and provides clean formatting.
apps/app_core/lib/modules/auth/model/auth_request_model.dart (3)
13-15: Good use of named constructors for specific use cases.The named constructors clearly express their intent and follow Dart best practices for factory-like patterns.
25-25: Appropriate addition of nullable token field.The token field is correctly added as nullable to support OTP verification while maintaining backward compatibility.
36-41: Serialization methods follow existing patterns.The new serialization methods are consistent with the existing
toMap()andtoSocialSignInMap()methods in structure and naming.Also applies to: 56-60
apps/app_core/lib/modules/forgot_password/bloc/forgot_password_bloc.dart (2)
29-42: Excellent error handling implementation.The submission handler now properly uses
.fold()to handle theEitherresult from the repository call, correctly setting success or failure states based on the API response. This addresses the previous concern about error handling.
24-27: Clean email validation implementation.The email change handler properly creates a dirty validator and updates the form validity state using Formz patterns.
apps/app_core/lib/modules/verify_otp/bloc/verify_otp_state.dart (2)
15-29: copyWith method correctly implemented.The copyWith method now properly preserves the existing
errorMessagevalue when not explicitly provided, fixing the previous issue where it would always reset to an empty string.
37-37: Clean validation logic.The
isValidgetter appropriately combines both OTP and email validation states for form validation.apps/app_core/lib/modules/verify_otp/bloc/verify_otp_event.dart (1)
3-8: Well-implemented sealed class pattern.The base event class correctly uses sealed class with Equatable, enabling exhaustive pattern matching and proper equality comparison.
apps/app_core/lib/modules/forgot_password/screens/forgot_password_screen.dart (1)
1-107: Well-implemented forgot password screen!The screen follows Flutter best practices with proper BLoC pattern implementation, state management, and UI composition. Good use of
AutoRouteWrapperfor dependency injection andBlocListenerfor handling state changes.apps/app_core/lib/modules/verify_otp/bloc/verify_otp_bloc.dart (1)
24-44: Well-structured OTP verification handler.The
_onVerifyButtonPressedmethod is properly implemented with appropriate error handling and state transitions. I understand from the context that the UI currently uses static validation for testing purposes rather than calling this API verification.apps/app_core/lib/modules/auth/repository/auth_repository.dart (1)
123-154: Well-implemented password reset and OTP verification methods.Both
forgotPasswordandverifyOTPmethods follow the established patterns in the repository with proper error handling and response mapping.
apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart
Outdated
Show resolved
Hide resolved
The `AppTimer` widget now correctly formats the remaining time to show both minutes and seconds (MM:SS) instead of just seconds.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/app_ui/lib/src/widgets/molecules/app_timer.dart (1)
6-10: Consider adding input validation for the seconds parameterThe widget doesn't validate that
secondsis non-negative. Negative values could lead to unexpected behavior in the timer logic.Consider adding validation in the constructor:
class AppTimer extends StatefulWidget { - const AppTimer({required this.seconds, super.key, this.onFinished}); + const AppTimer({required this.seconds, super.key, this.onFinished}) + : assert(seconds >= 0, 'seconds must be non-negative'); final int seconds;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/app_ui/lib/src/widgets/molecules/app_timer.dart(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: tulsi-7span
PR: 7span/dashing-kit#56
File: apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart:98-112
Timestamp: 2025-06-26T11:54:23.384Z
Learning: In the verify OTP screen (apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart), the team uses static OTP validation with hardcoded value "222222" for testing success states during development, rather than calling the actual API verification.
🔇 Additional comments (4)
packages/app_ui/lib/src/widgets/molecules/app_timer.dart (4)
37-50: Timer implementation is robust and well-structuredThe timer logic correctly handles:
- Early return for zero seconds
- Proper cancellation before starting new timer
- State updates with countdown logic
- Cleanup and callback execution when finished
The periodic timer implementation follows Flutter best practices.
27-35: Excellent widget update handlingThe
didUpdateWidgetimplementation properly handles dynamic updates to thesecondsparameter by canceling the existing timer and restarting with the new value. This ensures the widget remains responsive to property changes.
52-56: Proper resource cleanup in disposeGood implementation of timer cleanup to prevent memory leaks. The timer is properly canceled when the widget is disposed.
58-64: Clean time formatting and theme integrationThe time display logic correctly:
- Calculates minutes and seconds using integer division and modulo
- Formats with proper zero-padding for consistent "mm:ss" display
- Uses theme-based styling for consistent UI appearance
This integrates well with the app's design system.
- Added `pin_incorrect` localization. - Made `VerifyOTPEvent` subclasses final. - Added assertion for non-negative seconds in `AppTimer`. - Used localized error message for incorrect PIN in `VerifyOTPScreen`.
apps/app_core/lib/modules/forgot_password/bloc/forgot_password_bloc.dart
Outdated
Show resolved
Hide resolved
apps/app_core/lib/modules/forgot_password/bloc/forgot_password_state.dart
Show resolved
Hide resolved
apps/app_core/lib/modules/forgot_password/screens/forgot_password_screen.dart
Outdated
Show resolved
Hide resolved
Moved the `email` property declaration after the constructor in `SetEmailEvent` for consistency with other event classes.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/app_core/lib/modules/auth/repository/auth_repository.dart(5 hunks)apps/app_core/lib/modules/forgot_password/bloc/forgot_password_bloc.dart(1 hunks)apps/app_core/lib/modules/forgot_password/screens/forgot_password_screen.dart(1 hunks)apps/app_core/lib/modules/verify_otp/bloc/verify_otp_bloc.dart(1 hunks)apps/app_core/lib/modules/verify_otp/bloc/verify_otp_event.dart(1 hunks)apps/app_core/lib/modules/verify_otp/bloc/verify_otp_state.dart(1 hunks)apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- apps/app_core/lib/modules/verify_otp/bloc/verify_otp_event.dart
- apps/app_core/lib/modules/verify_otp/bloc/verify_otp_state.dart
- apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart
- apps/app_core/lib/modules/forgot_password/bloc/forgot_password_bloc.dart
- apps/app_core/lib/modules/forgot_password/screens/forgot_password_screen.dart
- apps/app_core/lib/modules/auth/repository/auth_repository.dart
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: tulsi-7span
PR: 7span/dashing-kit#56
File: apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart:98-112
Timestamp: 2025-06-26T11:54:23.384Z
Learning: In the verify OTP screen (apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart), the team uses static OTP validation with hardcoded value "222222" for testing success states during development, rather than calling the actual API verification.
apps/app_core/lib/modules/verify_otp/bloc/verify_otp_bloc.dart (1)
Learnt from: tulsi-7span
PR: 7span/dashing-kit#56
File: apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart:98-112
Timestamp: 2025-06-26T11:54:23.384Z
Learning: In the verify OTP screen (apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart), the team uses static OTP validation with hardcoded value "222222" for testing success states during development, rather than calling the actual API verification.
🔇 Additional comments (4)
apps/app_core/lib/modules/verify_otp/bloc/verify_otp_bloc.dart (4)
15-24: Well-structured BLoC implementation!The class follows the standard BLoC pattern with proper dependency injection and event handler registration.
25-27: Correct email state management.The method properly updates the email state using the validator pattern.
29-45: Robust OTP verification implementation.The method correctly handles the verification flow with proper loading states and error handling using the fold pattern.
54-74: Well-implemented OTP resend functionality.The method correctly handles the resend flow, properly resets the OTP field, and manages loading states appropriately.
| const VerifyOTPState({ | ||
| this.resendOtpStatus = ApiStatus.initial, | ||
| this.verifyOtpStatus = ApiStatus.initial, | ||
| this.email = const EmailValidator.pure(), |
There was a problem hiding this comment.
we can make this simple string variable
apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart
Outdated
Show resolved
Hide resolved
apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart
Outdated
Show resolved
Hide resolved
apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart
Outdated
Show resolved
Hide resolved
apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart
Outdated
Show resolved
Hide resolved
apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart
Outdated
Show resolved
Hide resolved
apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart
Outdated
Show resolved
Hide resolved
- Made `emailAddress` a required parameter in `VerifyOTPScreen`. - Removed local timer management from `VerifyOTPScreen` and moved timer logic to `VerifyOTPBloc`. - Simplified OTP verification logic in `VerifyOTPScreen` and `VerifyOTPBloc` to use a static OTP for now. - Replaced `SingleChildScrollView` with `ListView` in `VerifyOTPScreen`. - Updated `VerifyOTPState` to include `isTimerRunning` and changed `email` type to `String`. - Added `StartTimerEvent` and `TimerFinishedEvent` to `VerifyOTPEvent`. - Updated `VerifyOTPBloc` to handle new timer events and use the string email. - Renamed `readOnly` to `isReadOnly` in `AppTextField`.
apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart
Outdated
Show resolved
Hide resolved
apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart (1)
82-90: Move Pinput to App UI package for consistency.As discussed in previous reviews, UI components should be in the App UI package. Consider creating an
AppOtpInputwidget in the App UI package that wraps Pinput.Create a new widget in App UI package:
// In packages/app_ui/lib/src/widgets/molecules/app_otp_input.dart class AppOtpInput extends StatelessWidget { const AppOtpInput({ required this.onChanged, this.length = 6, this.errorText, super.key, }); final void Function(String) onChanged; final int length; final String? errorText; @override Widget build(BuildContext context) { return Pinput( length: length, separatorBuilder: (index) => HSpace.xxsmall4(), errorText: errorText, onChanged: onChanged, inputFormatters: [FilteringTextInputFormatter.digitsOnly], ); } }apps/app_core/lib/modules/verify_otp/bloc/verify_otp_bloc.dart (1)
53-53: Move _otpLength to class-level constant.The
_otpLengthvariable is a fixed value that should be declared as a class-level constant for better code organization.class VerifyOTPBloc extends Bloc<VerifyOTPEvent, VerifyOTPState> { + static const int _otpLength = 6; + VerifyOTPBloc(this.authenticationRepository) : super(const VerifyOTPState()) {And remove the instance variable:
- final int _otpLength = 6;
🧹 Nitpick comments (1)
apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart (1)
57-57: Consider using pushRoute instead of replaceRoute for better navigation flow.Using
replaceRouteremoves the verify OTP screen from the navigation stack. Users won't be able to go back if they need to re-enter OTP. Consider usingpushRouteto maintain navigation history.-context.replaceRoute(const ChangePasswordRoute()); +context.pushRoute(const ChangePasswordRoute());
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/app_core/lib/modules/verify_otp/bloc/verify_otp_bloc.dart(1 hunks)apps/app_core/lib/modules/verify_otp/bloc/verify_otp_event.dart(1 hunks)apps/app_core/lib/modules/verify_otp/bloc/verify_otp_state.dart(1 hunks)apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart(1 hunks)packages/app_ui/lib/src/widgets/molecules/app_textfield.dart(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/app_ui/lib/src/widgets/molecules/app_textfield.dart
- apps/app_core/lib/modules/verify_otp/bloc/verify_otp_event.dart
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: tulsi-7span
PR: 7span/dashing-kit#56
File: apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart:98-112
Timestamp: 2025-06-26T11:54:23.384Z
Learning: In the verify OTP screen (apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart), the team uses static OTP validation with hardcoded value "222222" for testing success states during development, rather than calling the actual API verification.
apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart (1)
Learnt from: tulsi-7span
PR: 7span/dashing-kit#56
File: apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart:98-112
Timestamp: 2025-06-26T11:54:23.384Z
Learning: In the verify OTP screen (apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart), the team uses static OTP validation with hardcoded value "222222" for testing success states during development, rather than calling the actual API verification.
apps/app_core/lib/modules/verify_otp/bloc/verify_otp_state.dart (1)
Learnt from: tulsi-7span
PR: 7span/dashing-kit#56
File: apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart:98-112
Timestamp: 2025-06-26T11:54:23.384Z
Learning: In the verify OTP screen (apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart), the team uses static OTP validation with hardcoded value "222222" for testing success states during development, rather than calling the actual API verification.
apps/app_core/lib/modules/verify_otp/bloc/verify_otp_bloc.dart (1)
Learnt from: tulsi-7span
PR: 7span/dashing-kit#56
File: apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart:98-112
Timestamp: 2025-06-26T11:54:23.384Z
Learning: In the verify OTP screen (apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart), the team uses static OTP validation with hardcoded value "222222" for testing success states during development, rather than calling the actual API verification.
🔇 Additional comments (4)
apps/app_core/lib/modules/verify_otp/bloc/verify_otp_state.dart (4)
5-13: Well-structured state class with appropriate field initialization.The class definition follows Flutter BLoC patterns correctly with sensible default values for all fields.
15-31: Excellent fix for the copyWith method.The copyWith method now correctly preserves existing field values when not explicitly provided, addressing the previous issue where
errorMessagewas being reset to an empty string. The implementation properly uses the null-aware operator to maintain state consistency.
40-40: Appropriate validation logic for OTP.The
isValidgetter correctly delegates to the LengthValidator, providing a clean interface for OTP validation state. Given the retrieved learning that the team uses static OTP validation with "222222" during development, this validator will work well for both testing and production scenarios.
42-44: Complete Equatable implementation.All state fields are properly included in the props list, ensuring accurate state comparison for the BLoC pattern.
apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart
Outdated
Show resolved
Hide resolved
apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart
Outdated
Show resolved
Hide resolved
apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart
Outdated
Show resolved
Hide resolved
The padding for the "Verify OTP" button on the `VerifyOTPScreen` has been updated.
Removed the `Padding` widget that was wrapping the `BlocConsumer` in the `VerifyOTPScreen` and moved the padding to the `ListView` instead.
- Added `pinput` dependency to `app_ui/pubspec.yaml`. - Removed `pinput` dependency from `app_core/pubspec.yaml`. - Created `AppOtpInput` widget in `app_ui` to encapsulate OTP input logic. - Replaced direct usage of `Pinput` with `AppOtpInput` in `VerifyOTPScreen`. - Updated various dependency versions in `widgetbook/pubspec.lock`. - Updated Flutter SDK constraint in `widgetbook/pubspec.lock`.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/app_ui/lib/src/widgets/molecules/app_otp_input.dart (1)
7-7: Consider adding parameter validation for better robustness.While the current implementation is functional, consider adding validation to ensure the length parameter is within reasonable bounds.
- const AppOtpInput({required this.onChanged, this.length = 6, this.errorText, super.key}); + const AppOtpInput({required this.onChanged, this.length = 6, this.errorText, super.key}) + : assert(length > 0 && length <= 10, 'OTP length must be between 1 and 10');
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
packages/widgetbook/pubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart(1 hunks)apps/app_core/pubspec.yaml(2 hunks)packages/app_ui/lib/src/widgets/molecules/app_otp_input.dart(1 hunks)packages/app_ui/lib/src/widgets/molecules/molecules.dart(1 hunks)packages/app_ui/pubspec.yaml(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- packages/app_ui/pubspec.yaml
- apps/app_core/pubspec.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: tulsi-7span
PR: 7span/dashing-kit#56
File: apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart:98-112
Timestamp: 2025-06-26T11:54:23.384Z
Learning: In the verify OTP screen (apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart), the team uses static OTP validation with hardcoded value "222222" for testing success states during development, rather than calling the actual API verification.
packages/app_ui/lib/src/widgets/molecules/molecules.dart (1)
Learnt from: tulsi-7span
PR: 7span/dashing-kit#56
File: apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart:98-112
Timestamp: 2025-06-26T11:54:23.384Z
Learning: In the verify OTP screen (apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart), the team uses static OTP validation with hardcoded value "222222" for testing success states during development, rather than calling the actual API verification.
packages/app_ui/lib/src/widgets/molecules/app_otp_input.dart (1)
Learnt from: tulsi-7span
PR: 7span/dashing-kit#56
File: apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart:98-112
Timestamp: 2025-06-26T11:54:23.384Z
Learning: In the verify OTP screen (apps/app_core/lib/modules/verify_otp/screens/verify_otp_screen.dart), the team uses static OTP validation with hardcoded value "222222" for testing success states during development, rather than calling the actual API verification.
🔇 Additional comments (2)
packages/app_ui/lib/src/widgets/molecules/app_otp_input.dart (1)
1-24: LGTM! Clean and well-structured OTP input widget implementation.The widget follows Flutter best practices with:
- Proper input validation using
FilteringTextInputFormatter.digitsOnly- Configurable length with sensible default (6 digits)
- Built-in error handling support
- Clean visual separation between input fields
- Appropriate use of required vs optional parameters
The implementation provides a solid foundation for the OTP verification feature.
packages/app_ui/lib/src/widgets/molecules/molecules.dart (1)
7-7: LGTM! Export statements properly organized.The new widget exports are correctly placed and maintain the alphabetical ordering, ensuring consistent module organization.
Also applies to: 11-11
Description
pinputfor OTP input.Type of Change
Summary by CodeRabbit
New Features
Style
Chores
pinputpackage for OTP input functionality.