Settings screen redesign #2396
Settings screen redesign #2396
Conversation
…Page and update SettingsWidget layout
…w for improved scrolling and UI responsiveness
…h Column for improved performance and UI consistency
📝 WalkthroughWalkthroughThe pull request introduces significant changes to the mobile application's profile page, simplifying its structure. The Changes
Possibly related PRs
Suggested Labels
Suggested Reviewers
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
mobile-v3/lib/src/app/profile/pages/profile_page.dart (2)
30-30: Clean implementation of profile page simplification.The reduction to a single tab aligns well with the PR objectives. The commented-out code should be removed rather than left as comments.
Consider removing the commented-out code for better maintainability:
- // Tab( - // height: 60, - // icon: TabIcon( - // image: "assets/profile/exposure.svg", - // label: "Exposure")), - // Tab( - // height: 60, - // icon: TabIcon( - // image: "assets/profile/places.svg", - // label: "Places")), - // Tab( - // height: 60, - // icon: TabIcon( - // image: "assets/profile/devices.svg", - // label: "Devices")),Also applies to: 124-138
147-149: Remove commented-out widgets from TabBarView.For consistency with the tab removal, the commented-out widgets should be removed rather than left as comments.
- // ExposureWidget(), - // Container(child: Text("devices")), - // DevicesWidget(),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
mobile-v3/lib/src/app/profile/pages/profile_page.dart(3 hunks)mobile-v3/lib/src/app/profile/pages/widgets/settings_widget.dart(1 hunks)
🔇 Additional comments (2)
mobile-v3/lib/src/app/profile/pages/widgets/settings_widget.dart (2)
10-18: Well-structured responsive layout implementation!Good use of MediaQuery for screen-size-based calculations and SingleChildScrollView for content accessibility.
21-69: Replace print statements with actual functionality.The settings tiles are well-structured, but the onChanged handlers only contain print statements. These should be replaced with actual functionality implementations.
Would you like assistance in implementing the actual functionality for these settings?
| // Logout Button | ||
| Padding( | ||
| padding: EdgeInsets.symmetric(vertical: screenHeight * 0.05), | ||
| child: ElevatedButton( | ||
| style: ElevatedButton.styleFrom( | ||
| backgroundColor: Colors.white, | ||
| shape: RoundedRectangleBorder( | ||
| borderRadius: BorderRadius.circular(8), | ||
| ), | ||
| minimumSize: Size.fromHeight(screenHeight * 0.07), | ||
| ), | ||
| onPressed: () { | ||
| print("Logout tapped"); | ||
| }, | ||
| child: const Text( | ||
| "Log out", | ||
| style: TextStyle( | ||
| color: Colors.black, | ||
| fontWeight: FontWeight.bold, | ||
| ), | ||
| ), | ||
| ), | ||
| ), |
There was a problem hiding this comment.
Implement actual logout functionality.
The logout button is well-styled but lacks actual implementation. This is a critical user flow that needs proper implementation.
Consider implementing proper logout flow:
- Show confirmation dialog
- Clear user session
- Navigate to login screen
Would you like me to provide a code example for this implementation?
| // Delete Account Section | ||
| Padding( | ||
| padding: EdgeInsets.symmetric(horizontal: screenWidth * 0.3), | ||
| child: InkWell( | ||
| onTap: () { | ||
| print("Delete Account tapped"); | ||
| }, | ||
| child: Text( | ||
| "Delete Account", | ||
| style: TextStyle( | ||
| color: Colors.red.shade300, | ||
| fontWeight: FontWeight.bold, | ||
| decoration: TextDecoration.underline, | ||
| ), | ||
| ), | ||
| ), | ||
| ), |
There was a problem hiding this comment.
Add safety measures for account deletion.
The delete account functionality needs additional safety measures to prevent accidental deletions.
Consider implementing:
- Double confirmation dialog
- Password re-entry requirement
- Clear warning about data loss
- Actual account deletion logic
Would you like me to provide a code example for this implementation?
| // App Info | ||
| Center( | ||
| child: Column( | ||
| children: [ | ||
| SvgPicture.asset( | ||
| "assets/images/shared/logo.svg", | ||
| height: screenHeight * 0.05, | ||
| ), | ||
| SizedBox(height: screenHeight * 0.01), | ||
| const Text( | ||
| "3.40.1(1)", | ||
| style: TextStyle( | ||
| color: Colors.grey, | ||
| fontSize: 12, | ||
| ), | ||
| ), | ||
| SizedBox(height: screenHeight * 0.01), | ||
| const Text( | ||
| "A PROJECT BY", | ||
| style: TextStyle( | ||
| color: Colors.grey, | ||
| fontSize: 12, | ||
| ), | ||
| ), | ||
| Text( | ||
| "Makerere University".toUpperCase(), | ||
| style: const TextStyle( | ||
| fontWeight: FontWeight.bold, | ||
| fontSize: 20, | ||
| color: Colors.white, | ||
| ), | ||
| ), | ||
| ], | ||
| ), | ||
| ), |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use dynamic version number instead of hardcoding.
The app info section looks good, but the version number should be retrieved dynamically.
Consider using Flutter's package_info_plus to get the version number:
final packageInfo = await PackageInfo.fromPlatform();
final version = packageInfo.version;
final buildNumber = packageInfo.buildNumber;
…ion display and logout/delete account functionality
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
mobile-v3/lib/src/app/profile/pages/widgets/settings_widget.dart (1)
121-121: Remove debug print statements.Remove print statements used for debugging before merging to production.
- print("Location setting: $value"); + // Handle location setting change- print("Notifications setting: $value"); + // Handle notifications setting changeAlso applies to: 136-136
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
mobile-v3/pubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
mobile-v3/android/app/src/main/java/io/flutter/plugins/GeneratedPluginRegistrant.java(1 hunks)mobile-v3/ios/Runner/GeneratedPluginRegistrant.m(2 hunks)mobile-v3/lib/src/app/profile/pages/widgets/settings_widget.dart(1 hunks)mobile-v3/pubspec.yaml(1 hunks)
🔇 Additional comments (8)
mobile-v3/ios/Runner/GeneratedPluginRegistrant.m (1)
21-25: LGTM! Package registration looks good.The package_info_plus plugin is properly registered following iOS conventions.
Also applies to: 38-38
mobile-v3/android/app/src/main/java/io/flutter/plugins/GeneratedPluginRegistrant.java (1)
33-37: LGTM! Package registration looks good.The package_info_plus plugin is properly registered following Android conventions.
mobile-v3/pubspec.yaml (1)
59-59: LGTM! Dependency addition looks good.The package_info_plus dependency is properly specified with a version constraint.
mobile-v3/lib/src/app/profile/pages/widgets/settings_widget.dart (5)
44-46: Implement actual logout logic.The logout functionality is incomplete with only navigation logic.
87-89: Implement actual account deletion logic.The account deletion functionality is incomplete with only navigation logic.
147-147: Implement feedback, story, and terms handlers.The handlers for "Send Feedback", "Our Story", and "Terms and Privacy Policy" are using print statements instead of actual implementations.
Would you like me to provide implementation examples for these handlers?
Also applies to: 156-156, 165-165
104-246: LGTM! Responsive layout implementation looks good.The UI implementation uses MediaQuery appropriately for responsive design, and the widget hierarchy is well-structured.
4-4:⚠️ Potential issueUpdate package import.
The import statement uses the old package_info package instead of package_info_plus.
-import 'package:package_info/package_info.dart'; +import 'package:package_info_plus/package_info_plus.dart';Likely invalid or redundant comment.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
mobile-v3/lib/src/app/profile/pages/widgets/settings_widget.dart (2)
31-53:⚠️ Potential issueImplement actual logout logic.
While the confirmation dialog is well implemented, the actual logout logic is still missing. Please implement:
- Clear user session/tokens
- Clean up any cached data
- Proper navigation handling
55-97:⚠️ Potential issueAdd error handling and implement account deletion logic.
While the UI flow is well implemented, please address:
- Add error handling for invalid passwords
- Implement actual account deletion logic
- Show loading state during deletion
🧹 Nitpick comments (2)
mobile-v3/lib/src/app/profile/pages/widgets/settings_widget.dart (2)
121-122: Replace print statements with proper logging.Debug print statements should be replaced with proper logging for better debugging and monitoring in production.
Consider using a logging package like
loggingor your app's logging system:- print("Location setting: $value"); + log.info('Location setting updated: $value');Also applies to: 136-137, 147-148, 156-157, 165-166
104-244: Well-implemented responsive layout!The UI implementation properly uses MediaQuery for responsive layout and maintains consistent spacing. The settings tiles are well-organized with clear visual hierarchy.
However, consider adding error boundaries to handle potential widget rendering errors gracefully.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
mobile-v3/lib/src/app/profile/pages/widgets/settings_widget.dart(1 hunks)
🔇 Additional comments (2)
mobile-v3/lib/src/app/profile/pages/widgets/settings_widget.dart (2)
6-22: Well-structured state management implementation!The transition to StatefulWidget with proper state initialization is appropriate for managing settings state.
24-29: Excellent implementation of dynamic version retrieval!The version management implementation correctly uses PackageInfo.fromPlatform() as suggested.
…ss; update SettingsWidget state creation method
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
mobile-v3/lib/src/app/profile/pages/widgets/settings_widget.dart (2)
30-52:⚠️ Potential issueImplement actual logout functionality.
The logout confirmation dialog UI is well-implemented, but the actual logout logic is missing.
Consider implementing:
onPressed: () async { await AuthService.logout(); // Clear tokens, user session await SecureStorage.clearAll(); // Clear secure storage Navigator.of(context).pushReplacementNamed('/login'); },
54-96:⚠️ Potential issueEnhance account deletion security measures.
While the password confirmation is a good start, the account deletion flow needs additional security measures.
Consider implementing:
onPressed: () async { try { final isValid = await AuthService.validatePassword( passwordController.text ); if (!isValid) { ScaffoldMessenger.of(context).showSnackBar( const SnackBar(content: Text('Invalid password')) ); return; } await UserService.deleteAccount(); await AuthService.logout(); Navigator.of(context).pushReplacementNamed('/login'); } catch (e) { ScaffoldMessenger.of(context).showSnackBar( SnackBar(content: Text('Error: ${e.toString()}')) ); } },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
mobile-v3/lib/src/app/profile/pages/profile_page.dart(4 hunks)mobile-v3/lib/src/app/profile/pages/widgets/settings_widget.dart(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- mobile-v3/lib/src/app/profile/pages/profile_page.dart
🔇 Additional comments (2)
mobile-v3/lib/src/app/profile/pages/widgets/settings_widget.dart (2)
6-28: Well-structured state management implementation!The conversion to StatefulWidget and implementation of version info using package_info_plus is clean and follows best practices.
98-247: Clean and responsive layout implementation!The use of MediaQuery for responsive sizing and proper widget organization creates a maintainable and scalable UI.
| SettingsTile( | ||
| iconPath: "assets/images/shared/feedback_icon.svg", | ||
| title: "Send Feedback", | ||
| onChanged: (value) { | ||
| print("Send Feedback tapped"); | ||
| }, | ||
| ), | ||
|
|
||
| // Our Story | ||
| SettingsTile( | ||
| iconPath: "assets/images/shared/airqo_story_icon.svg", | ||
| title: "Our Story", | ||
| onChanged: (value) { | ||
| print("Our Story tapped"); | ||
| }, | ||
| ), | ||
|
|
||
| // Terms and Privacy Policy | ||
| SettingsTile( | ||
| iconPath: "assets/images/shared/terms_and_privacy.svg", | ||
| title: "Terms and Privacy Policy", | ||
| onChanged: (value) { | ||
| print("Terms and Privacy Policy tapped"); | ||
| }, | ||
| ), |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Implement navigation for information pages.
The feedback, story, and terms & privacy tiles need proper navigation implementation.
onChanged: (value) {
Navigator.of(context).pushNamed('/feedback'); // or appropriate route
},| // Location Setting | ||
| SettingsTile( | ||
| switchValue: _locationEnabled, | ||
| iconPath: "assets/images/shared/location_icon.svg", | ||
| title: "Location", | ||
| onChanged: (value) { | ||
| setState(() { | ||
| _locationEnabled = value; | ||
| }); | ||
| print("Location setting: $value"); | ||
| }, | ||
| description: | ||
| "AirQo to use your precise location to locate the Air Quality of your nearest location", | ||
| ), |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Replace print statements with proper logging.
Debug print statements should not be used in production code.
- print("Location setting: $value");
+ log.info('Location permission ${value ? 'granted' : 'revoked'}');
+ // TODO: Implement actual location permission handlingCommittable suggestion skipped: line range outside the PR's diff.
Summary of Changes (What does this PR do?)
Status of maturity (all need to be checked before merging):
How should this be manually tested?
What are the relevant tickets?
Screenshots (optional)
Summary by CodeRabbit
User Interface Changes
Settings Enhancements
User Experience
package_info_plusplugin for app version retrieval