feat: Unsaved changes indicator with review dialog on Edit Task screen#524
feat: Unsaved changes indicator with review dialog on Edit Task screen#524mulikruchi07 wants to merge 4 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR implements an unsaved changes indicator on the Edit Task screen. When a user modifies any task attribute, a floating action button with a save icon appears in the bottom-right corner. Clicking it opens a review dialog showing the changes (old vs new values), where the user can either submit or cancel. The save button automatically disappears when all changes are reverted to their original values, ensuring accurate change tracking.
Key changes include:
- Enhanced priority widget to normalize null/empty values to display as 'X' and cycle through priority levels (X → H → M → L → X)
- Refactored the save flow by extracting the review dialog into a separate method with improved user feedback via SnackBar
- Updated change tracking logic to reflect real changes only by checking
modify.changes.isNotEmpty
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
lib/app/modules/detailRoute/views/priority_widget.dart |
Adds null/empty value normalization to 'X' for display, implements priority cycling logic with editable state handling |
lib/app/modules/detailRoute/views/detail_route_view.dart |
Refactors floating action button to use observable pattern, extracts review dialog to separate method, adds SnackBar feedback after save |
lib/app/modules/detailRoute/controllers/detail_route_controller.dart |
Updates onEdit flag logic to track real changes by checking modify.changes.isNotEmpty after setAttribute and saveChanges operations |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| onPressed: () { | ||
| Get.back(); | ||
| controller.saveChanges(); | ||
|
|
||
| ScaffoldMessenger.of(context).showSnackBar( | ||
| SnackBar( | ||
| content: Text(sentences.taskUpdated), | ||
| behavior: SnackBarBehavior.floating, | ||
| duration: const Duration(seconds: 2), | ||
| ), | ||
| ); |
There was a problem hiding this comment.
The navigation flow is broken. When the dialog is closed with Get.back() at line 225, and then controller.saveChanges() is called which contains another Get.back() at line 84 of detail_route_controller.dart, this will navigate back twice. The second Get.back() in saveChanges() will close the detail route view itself instead of just the dialog. Additionally, the SnackBar at lines 228-234 will be displayed after the view has already been popped, which may not work correctly. Consider removing the Get.back() call from the controller's saveChanges() method or restructuring this flow.
|
|
||
| ScaffoldMessenger.of(context).showSnackBar( | ||
| SnackBar( | ||
| content: Text(sentences.taskUpdated), | ||
| behavior: SnackBarBehavior.floating, | ||
| duration: const Duration(seconds: 2), | ||
| ), | ||
| ); |
There was a problem hiding this comment.
Duplicate SnackBar notifications will be shown to the user. The controller.saveChanges() method already displays a SnackBar (lines 76-81 in detail_route_controller.dart), and this code adds another SnackBar at lines 228-234. This will result in two "Task Updated" messages appearing to the user, creating a confusing experience. Consider either removing the SnackBar from the controller's saveChanges() method or removing it from this location.
| ScaffoldMessenger.of(context).showSnackBar( | |
| SnackBar( | |
| content: Text(sentences.taskUpdated), | |
| behavior: SnackBarBehavior.floating, | |
| duration: const Duration(seconds: 2), | |
| ), | |
| ); |
| modify.set(name, newValue); | ||
| onEdit.value = true; | ||
|
|
||
| // onEdit must reflect REAL changes only |
There was a problem hiding this comment.
The comment should be properly formatted with consistent spacing. Comments should start with a space after the double-slash to follow Dart style conventions.
| // onEdit must reflect REAL changes only | |
| // onEdit must reflect REAL changes only |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
📝 WalkthroughWalkthroughThe PR implements visual tracking of unsaved changes in the edit task screen. The controller now synchronizes the ChangesEdit task with unsaved changes tracking and review dialog
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/app/modules/detailRoute/views/detail_route_view.dart`:
- Around line 224-235: The dialog handler is showing a duplicate snackbar and
performing navigation that controller.saveChanges() already handles; remove the
explicit Get.back() call and the ScaffoldMessenger.of(context).showSnackBar(...)
block (including the SnackBar creation and sentences.taskUpdated usage) and
instead just invoke controller.saveChanges() so feedback and navigation come
solely from controller.saveChanges().
In `@lib/app/modules/detailRoute/views/priority_widget.dart`:
- Around line 31-33: The priority string is not canonicalized so values like "
low", "l", or "low" won't match the switch/case at the tap handler; update the
priority handling by normalizing the value right after it's computed (use trim()
and toUpperCase(), e.g., set priority = (value == null || value == '') ? 'X' :
value.toString().trim().toUpperCase()) and also add a safe default branch in the
tap-handling switch/mapping (the code around the onTap handler at lines ~70-83)
so any unmatched value falls back to a known priority or no-op handler,
preventing silent taps.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 016ceabe-fe3c-4352-8ac8-fe724a848e6f
📒 Files selected for processing (3)
lib/app/modules/detailRoute/controllers/detail_route_controller.dartlib/app/modules/detailRoute/views/detail_route_view.dartlib/app/modules/detailRoute/views/priority_widget.dart
| onPressed: () { | ||
| Get.back(); | ||
| controller.saveChanges(); | ||
|
|
||
| ScaffoldMessenger.of(context).showSnackBar( | ||
| SnackBar( | ||
| content: Text(sentences.taskUpdated), | ||
| behavior: SnackBarBehavior.floating, | ||
| duration: const Duration(seconds: 2), | ||
| ), | ||
| ); | ||
| }, |
There was a problem hiding this comment.
Remove duplicate success notification from the dialog submit path.
controller.saveChanges() already handles success feedback and navigation. On Line 228, showing another ScaffoldMessenger snackbar after popping can duplicate messages and rely on a dialog context that may no longer be valid.
Proposed fix
TextButton(
onPressed: () {
Get.back();
controller.saveChanges();
-
- ScaffoldMessenger.of(context).showSnackBar(
- SnackBar(
- content: Text(sentences.taskUpdated),
- behavior: SnackBarBehavior.floating,
- duration: const Duration(seconds: 2),
- ),
- );
},
child: Text(
sentences.submit,
style: const TextStyle(color: Colors.white),
),
),📝 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.
| onPressed: () { | |
| Get.back(); | |
| controller.saveChanges(); | |
| ScaffoldMessenger.of(context).showSnackBar( | |
| SnackBar( | |
| content: Text(sentences.taskUpdated), | |
| behavior: SnackBarBehavior.floating, | |
| duration: const Duration(seconds: 2), | |
| ), | |
| ); | |
| }, | |
| onPressed: () { | |
| Get.back(); | |
| controller.saveChanges(); | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/app/modules/detailRoute/views/detail_route_view.dart` around lines 224 -
235, The dialog handler is showing a duplicate snackbar and performing
navigation that controller.saveChanges() already handles; remove the explicit
Get.back() call and the ScaffoldMessenger.of(context).showSnackBar(...) block
(including the SnackBar creation and sentences.taskUpdated usage) and instead
just invoke controller.saveChanges() so feedback and navigation come solely from
controller.saveChanges().
| final String priority = | ||
| (value == null || value == '') ? 'X' : value.toString(); | ||
|
|
There was a problem hiding this comment.
Handle non-canonical priority values to avoid silent no-op taps.
At Line 31-32 and Line 70-83, unexpected values (e.g., lowercase or spaced strings) won’t match any case, so tapping does nothing even when editable. Canonicalize before switching or add a default fallback.
Proposed fix
- final String priority =
- (value == null || value == '') ? 'X' : value.toString();
+ final String priority = (() {
+ final raw = value?.toString().trim().toUpperCase();
+ return (raw == null || raw.isEmpty) ? 'X' : raw;
+ })();
...
switch (priority) {
case 'X':
callback('H');
break;
case 'H':
callback('M');
break;
case 'M':
callback('L');
break;
case 'L':
callback(null);
break;
+ default:
+ callback('H');
+ break;
}Also applies to: 70-83
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/app/modules/detailRoute/views/priority_widget.dart` around lines 31 - 33,
The priority string is not canonicalized so values like " low", "l", or "low"
won't match the switch/case at the tap handler; update the priority handling by
normalizing the value right after it's computed (use trim() and toUpperCase(),
e.g., set priority = (value == null || value == '') ? 'X' :
value.toString().trim().toUpperCase()) and also add a safe default branch in the
tap-handling switch/mapping (the code around the onTap handler at lines ~70-83)
so any unmatched value falls back to a known priority or no-op handler,
preventing silent taps.
Description
This PR adds a visual indicator for unsaved changes on the Edit Task screen.
When a user modifies any task attribute, a Save button appears at the bottom-right.
Clicking Save opens a review dialog that clearly shows previous vs updated values,
allowing the user to either submit or cancel the changes.
The Save indicator automatically disappears if all changes are reverted back to their original values, ensuring accurate change tracking and preventing empty review dialogs.
Fixes #523
Screenshots
enh_after_524.mp4
Checklist
Summary by CodeRabbit
Improvements
Bug Fixes