Improve UX #433
Improve UX #433
Conversation
…onfirmation dialog
WalkthroughAdds exit-confirmation localizations (EN/AR/HI) and wires an exit dialog into CVLandingView back navigation. Refactors router to use a centralized fade+scale transition. Converts primary/outline/flat buttons to animated, stateful interactions. Adjusts GroupDetailsView’s email dialog for scrollability and validation. All changes are additive; no public route APIs altered. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches🧪 Generate unit tests
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. CodeRabbit Commands (Invoked using PR/Issue comments)Type 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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/ui/components/cv_flat_button.dart (1)
6-10: Removerequiredmodifier fromkeyparameter.Keys should be optional parameters in Flutter widgets. The
requiredmodifier onkeyis unconventional and may cause issues.const CVFlatButton({ - required Key key, + Key? key, required this.triggerFunction, required this.buttonText, this.context, }) : super(key: key);
🧹 Nitpick comments (12)
lib/ui/views/cv_landing_view.dart (1)
105-116: Consider enhancing the exit dialog UX with an icon.The exit confirmation dialog implementation is solid, but adding a warning icon could improve visual communication of the action's significance.
Consider updating the DialogService to support an optional icon parameter for better visual feedback in confirmation dialogs.
lib/utils/router.dart (2)
102-105: Consider making transition durations configurable.While the current durations work well, consider extracting them as constants for easier adjustment based on user feedback or accessibility needs.
Add constants at the class level:
class CVRouter { + static const Duration _forwardTransitionDuration = Duration(milliseconds: 350); + static const Duration _reverseTransitionDuration = Duration(milliseconds: 300); + static Route<dynamic> generateRoute(RouteSettings settings) {Then use them in _buildRoute:
static PageRouteBuilder _buildRoute(Widget page, {RouteSettings? settings}) { return PageRouteBuilder( settings: settings, - transitionDuration: const Duration(milliseconds: 350), - reverseTransitionDuration: const Duration(milliseconds: 300), + transitionDuration: _forwardTransitionDuration, + reverseTransitionDuration: _reverseTransitionDuration,
113-115: Consider accessibility for reduced motion preferences.Users with motion sensitivity might prefer reduced or no animations. Consider checking for reduced motion preferences.
Would you like me to help implement a check for the system's reduced motion accessibility setting to conditionally apply simpler transitions for users who prefer less motion?
lib/ui/components/cv_outline_button.dart (4)
31-32: Consider making the state class private.The state class
CVOutlineButtonStateis exposed as public, but there's no apparent need for external access to the state. Consider making it private by prefixing with an underscore.@override - CVOutlineButtonState createState() => CVOutlineButtonState(); + _CVOutlineButtonState createState() => _CVOutlineButtonState();-class CVOutlineButtonState extends State<CVOutlineButton> +class _CVOutlineButtonState extends State<CVOutlineButton>
63-63: UsewithValuesinstead of deprecatedwithAlphamethod.The
withAlphamethod is a legacy API. UsewithValuesfor better forward compatibility with Material 3.- end: primaryColor.withAlpha(153), // ~60% opacity + end: primaryColor.withValues(alpha: 153), // ~60% opacity
70-70: UsewithValuesinstead of deprecatedwithAlphamethod.The
withAlphamethod is a legacy API. UsewithValuesfor better forward compatibility.- end: primaryColor.withAlpha(179), // ~70% opacity + end: primaryColor.withValues(alpha: 179), // ~70% opacity
140-140: Remove null overlay color to maintain default ripple effect.Setting
overlayColortonullexplicitly disables the Material ripple effect. This might not be intended since you're adding custom animations on top of the default Material behavior.Consider removing the
overlayColor: nullline to preserve the default Material ripple effect which provides additional visual feedback:foregroundColor: _textColorAnimation.value ?? (widget.isPrimaryDark ? CVTheme.primaryColorDark : CVTheme.primaryColor), - overlayColor: null, ),lib/ui/components/cv_primary_button.dart (3)
44-45: Consider making the state class private.The state class
CVPrimaryButtonStateis exposed as public, but there's no apparent need for external access to the state. Consider making it private.@override - CVPrimaryButtonState createState() => CVPrimaryButtonState(); + _CVPrimaryButtonState createState() => _CVPrimaryButtonState();-class CVPrimaryButtonState extends State<CVPrimaryButton> +class _CVPrimaryButtonState extends State<CVPrimaryButton>
80-80: UsewithValuesinstead of deprecatedwithAlphamethod.The
withAlphamethod is a legacy API. UsewithValuesfor better forward compatibility.- end: primaryColor.withAlpha(204), // ~80% opacity for pressed state + end: primaryColor.withValues(alpha: 204), // ~80% opacity for pressed state
138-138: Remove null overlay color to maintain default ripple effect.Setting
overlayColortonullexplicitly disables the Material ripple effect, which might not be intended.Consider removing the
overlayColor: nullline:elevation: _elevationAnimation.value, - overlayColor: null, ),lib/ui/components/cv_flat_button.dart (2)
40-40: UsewithValuesinstead of deprecatedwithAlphamethod.The
withAlphamethod is a legacy API. UsewithValuesfor better forward compatibility.- end: Colors.grey.withAlpha(179), + end: Colors.grey.withValues(alpha: 179),isActive - ? CVTheme.primaryColor.withAlpha(179) - : Colors.grey.withAlpha(179), + ? CVTheme.primaryColor.withValues(alpha: 179) + : Colors.grey.withValues(alpha: 179),Also applies to: 62-63
107-107: Remove null overlay color to maintain default ripple effect.Setting
overlayColortonullexplicitly disables the Material ripple effect.Consider removing the
overlayColorproperty:foregroundColor: WidgetStateProperty.all( _colorAnimation.value, ), - overlayColor: WidgetStateProperty.all(null), ),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (13)
lib/gen_l10n/app_localizations.dart(1 hunks)lib/gen_l10n/app_localizations_ar.dart(1 hunks)lib/gen_l10n/app_localizations_en.dart(1 hunks)lib/gen_l10n/app_localizations_hi.dart(1 hunks)lib/l10n/app_ar.arb(1 hunks)lib/l10n/app_en.arb(1 hunks)lib/l10n/app_hi.arb(1 hunks)lib/ui/components/cv_flat_button.dart(1 hunks)lib/ui/components/cv_outline_button.dart(2 hunks)lib/ui/components/cv_primary_button.dart(2 hunks)lib/ui/views/cv_landing_view.dart(2 hunks)lib/ui/views/groups/group_details_view.dart(2 hunks)lib/utils/router.dart(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-12T14:34:41.433Z
Learnt from: JatsuAkaYashvant
PR: CircuitVerse/mobile-app#421
File: lib/viewmodels/simulator/simulator_viewmodel.dart:244-244
Timestamp: 2025-08-12T14:34:41.433Z
Learning: In Flutter projects, `Uint8List` is re-exported by `flutter/services.dart`, so importing `dart:typed_data` directly is redundant when `flutter/services.dart` is already imported. The Dart analyzer will confirm such imports as unnecessary.
Applied to files:
lib/ui/views/cv_landing_view.dart
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Test & Analyze (ubuntu-latest)
- GitHub Check: Test & Analyze (macos-latest)
- GitHub Check: Test & Analyze (windows-latest)
🔇 Additional comments (13)
lib/l10n/app_en.arb (1)
434-437: LGTM! New exit confirmation localization keys added correctly.The new localization keys for exit confirmation dialog are well-structured and follow consistent naming patterns. The placement after
cv_languagemaintains the logical grouping within thecv_landing_viewsection.lib/l10n/app_hi.arb (1)
432-435: LGTM! Hindi translations are accurate and culturally appropriate.The Hindi translations properly convey the intended meaning:
- "ऐप बंद करें" (Exit App)
- "क्या आप वाकई ऐप बंद करना चाहते हैं?" (Are you sure you want to exit the application?)
- "बंद करें" (Exit)
- "रद्द करें" (Cancel)
These translations are natural and consistent with common Hindi UI terminology.
lib/l10n/app_ar.arb (1)
435-438: LGTM! Arabic translations are accurate and properly localized.The Arabic translations are contextually appropriate and follow proper Arabic UI conventions:
- "الخروج من التطبيق" (Exit App)
- "هل أنت متأكد أنك تريد الخروج من التطبيق؟" (Are you sure you want to exit the application?)
- "خروج" (Exit)
- "إلغاء" (Cancel)
The translations maintain consistency with the existing Arabic localization style in the file.
lib/ui/views/groups/group_details_view.dart (2)
166-166: Good improvement for dialog layout.Adding
mainAxisSize: MainAxisSize.minto the Column helps reduce the dialog's vertical footprint, making it more compact and visually appealing.
183-212: Excellent enhancement for email input dialog usability.The refactoring improves the dialog in several ways:
- Scrollability: Wrapping
SimpleChipsInputwithSingleChildScrollViewprevents overflow issues when many email chips are added- Better organization: Moving all
SimpleChipsInputconfiguration parameters inside the widget makes the code more maintainable- Consistent validation: The email validation logic with localized error messages enhances user experience
The dynamic button state management via
onChangedcallback ensures the "Add" button is only enabled when valid input is present.lib/gen_l10n/app_localizations.dart (1)
2353-2376: LGTM! Generated localization getters follow Flutter conventions.The four new getters (
cv_exit_app_title,cv_exit_app_message,cv_exit_app,cv_cancel) are properly generated with:
- Consistent documentation format
- Proper English translations in comments
- Standard getter signature pattern
- Correct placement in the class structure
This aligns with Flutter's localization code generation best practices from the intl package.
lib/gen_l10n/app_localizations_ar.dart (1)
1192-1203: LGTM! Arabic translations for exit dialog are correct.The Arabic translations are properly implemented and linguistically accurate. The translations follow the standard Arabic UI conventions.
lib/gen_l10n/app_localizations_en.dart (1)
1201-1213: LGTM! English translations are clear and consistent.The English translations for the exit confirmation dialog follow standard UX patterns with clear messaging.
lib/ui/views/cv_landing_view.dart (2)
124-137: Good implementation of back navigation with exit confirmation.The back navigation logic correctly handles both navigation to home and app exit scenarios. The async handling and PopScope configuration are properly implemented.
3-3: Keep flutter/services.dart import
SystemNavigator is only available via package:flutter/services.dart and isn’t re-exported by other imports, so this import is required.Likely an incorrect or invalid review comment.
lib/gen_l10n/app_localizations_hi.dart (1)
1210-1221: LGTM! Hindi translations are appropriate.The Hindi translations are correctly implemented and use appropriate formal language for the exit confirmation dialog.
lib/utils/router.dart (2)
100-123: Excellent implementation of smooth page transitions!The fade + scale transition implementation provides a premium feel with well-chosen animation parameters:
- The 350ms forward and 300ms reverse durations create natural-feeling transitions
- The subtle scale from 0.96 to 1.0 adds depth without being distracting
- Using
Curves.easeInOutQuartprovides smooth acceleration and decelerationThis significantly enhances the user experience compared to default MaterialPageRoute transitions.
41-45: Good preservation of route settings for SimulatorView.The implementation correctly maintains route metadata by passing RouteSettings to _buildRoute, ensuring proper navigation state management.
Pull Request Test Coverage Report for Build 17354405759Details
💛 - Coveralls |
3 participants
Describe the changes you have made in this PR
Note: Please check Allow edits from maintainers if you would like us to assist in the PR.
Summary by CodeRabbit