-
Notifications
You must be signed in to change notification settings - Fork 242
feat: Add Custom Frame Animation Creator with Save & Preview #1455
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: development
Are you sure you want to change the base?
Conversation
Reviewer's GuideThis PR implements a custom frame-based animation creator by extending FileHelper for saving JSON-encoded frames, adds two new navigation entries and routes for creating and browsing saved animations, introduces a full CreateFramesScreen with frame navigation, drawing history, preview and save dialog (with name sanitization and speed selection), and adds a SavedFramesScreen to list, preview and delete saved animations from storage. Sequence diagram for saving a custom frame animationsequenceDiagram
actor User
participant CreateFramesScreen
participant FileHelper
participant Storage
User->>CreateFramesScreen: Clicks Save
CreateFramesScreen->>FileHelper: saveFrameAnimationWithName(name, frames, speed)
FileHelper->>Storage: Write frames_{sanitized_name}.json
Storage-->>FileHelper: Success
FileHelper-->>CreateFramesScreen: Save complete
CreateFramesScreen-->>User: Show "Frames saved" toast
Sequence diagram for previewing a saved frame animationsequenceDiagram
actor User
participant SavedFramesScreen
participant Storage
participant AnimationBadgeProvider
User->>SavedFramesScreen: Clicks Play on saved animation
SavedFramesScreen->>Storage: Read frames_{name}.json
Storage-->>SavedFramesScreen: Return frame data
SavedFramesScreen->>AnimationBadgeProvider: setNewGrid(stitched frames)
SavedFramesScreen->>AnimationBadgeProvider: calculateDuration(speed)
AnimationBadgeProvider-->>SavedFramesScreen: Animation ready
SavedFramesScreen-->>User: Show animation preview
Class diagram for new and updated classes related to frame animationclassDiagram
class FileHelper {
+saveFrameAnimation(frames)
+saveFrameAnimationWithName(name, frames, speed)
-_generateFramesFilename()
}
class CreateFramesScreen {
-_frames: List<List<List<int>>>
-_currentFrame: int
-_history: List<List<List<bool>>>
+_saveFrames()
+_nextFrame()
+_prevFrame()
+_undo()
+_redo()
}
class SavedFramesScreen {
-_saved: List<MapEntry<String, List<List<List<int>>>>>
+_load()
}
FileHelper <.. CreateFramesScreen : uses
SavedFramesScreen <.. FileHelper : uses
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey there - I've reviewed your changes - here's some feedback:
- The CreateFramesScreen class is over 400 lines and mixes UI, state management, and file logic—consider extracting parts (toolbar, frame navigator, save dialog) into smaller widgets or controllers to improve readability and testability.
- The logic for ‘stitching’ frames into a single grid is duplicated in both save and load screens—extract that into a shared utility method to avoid code duplication and ease future updates.
- Currently saveFrameAnimationWithName uses only the sanitized name as the filename, which could overwrite existing files if reused—consider appending a timestamp or unique ID to ensure filenames stay unique.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The CreateFramesScreen class is over 400 lines and mixes UI, state management, and file logic—consider extracting parts (toolbar, frame navigator, save dialog) into smaller widgets or controllers to improve readability and testability.
- The logic for ‘stitching’ frames into a single grid is duplicated in both save and load screens—extract that into a shared utility method to avoid code duplication and ease future updates.
- Currently saveFrameAnimationWithName uses only the sanitized name as the filename, which could overwrite existing files if reused—consider appending a timestamp or unique ID to ensure filenames stay unique.
## Individual Comments
### Comment 1
<location> `lib/view/create_frames_screen.dart:275` </location>
<code_context>
+
+ if (name == null || name.isEmpty) return;
+
+ await _fileHelper.saveFrameAnimationWithName(name, _frames, selectedSpeed);
+ ToastUtils().showToast("Frames saved");
+ }
</code_context>
<issue_to_address>
No error handling for save failures.
Add error handling to notify the user if saveFrameAnimationWithName fails due to an exception.
</issue_to_address>
### Comment 2
<location> `lib/view/saved_frames_screen.dart:30` </location>
<code_context>
+ for (final f in files) {
</code_context>
<issue_to_address>
Silent catch block may hide file loading errors.
Please log the exception or notify the user when file loading fails to aid debugging.
</issue_to_address>
### Comment 3
<location> `lib/view/saved_frames_screen.dart:157` </location>
<code_context>
+ },
+ ),
+ IconButton(
+ icon: const Icon(Icons.delete_outline),
+ onPressed: () async {
+ final dir =
+ await getApplicationDocumentsDirectory();
+ final path = '${dir.path}/${item.key}';
+ final f = File(path);
+ if (await f.exists()) await f.delete();
+ await _load();
+ },
+ ),
</code_context>
<issue_to_address>
No confirmation before deleting saved frames.
Consider implementing a confirmation dialog to prevent accidental deletion of saved frames.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
IconButton(
icon: const Icon(Icons.delete_outline),
onPressed: () async {
final dir =
await getApplicationDocumentsDirectory();
final path = '${dir.path}/${item.key}';
final f = File(path);
if (await f.exists()) await f.delete();
await _load();
},
),
=======
IconButton(
icon: const Icon(Icons.delete_outline),
onPressed: () async {
final confirm = await showDialog<bool>(
context: context,
builder: (BuildContext context) {
return AlertDialog(
title: const Text('Delete Frame'),
content: const Text('Are you sure you want to delete this saved frame? This action cannot be undone.'),
actions: [
TextButton(
onPressed: () => Navigator.of(context).pop(false),
child: const Text('Cancel'),
),
TextButton(
onPressed: () => Navigator.of(context).pop(true),
child: const Text('Delete'),
),
],
);
},
);
if (confirm == true) {
final dir =
await getApplicationDocumentsDirectory();
final path = '${dir.path}/${item.key}';
final f = File(path);
if (await f.exists()) await f.delete();
await _load();
}
},
),
>>>>>>> REPLACE
</suggested_fix>
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
||
if (name == null || name.isEmpty) return; | ||
|
||
await _fileHelper.saveFrameAnimationWithName(name, _frames, selectedSpeed); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue: No error handling for save failures.
Add error handling to notify the user if saveFrameAnimationWithName fails due to an exception.
for (final f in files) { | ||
try { | ||
final content = await f.readAsString(); | ||
final data = jsonDecode(content); | ||
List<List<List<int>>> frames; | ||
if (data is List) { | ||
// backward compatibility (no speed) | ||
frames = (data) | ||
.map((frame) => (frame as List<dynamic>) | ||
.map((row) => (row as List<dynamic>).cast<int>()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue (bug_risk): Silent catch block may hide file loading errors.
Please log the exception or notify the user when file loading fails to aid debugging.
IconButton( | ||
icon: const Icon(Icons.delete_outline), | ||
onPressed: () async { | ||
final dir = | ||
await getApplicationDocumentsDirectory(); | ||
final path = '${dir.path}/${item.key}'; | ||
final f = File(path); | ||
if (await f.exists()) await f.delete(); | ||
await _load(); | ||
}, | ||
), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion: No confirmation before deleting saved frames.
Consider implementing a confirmation dialog to prevent accidental deletion of saved frames.
IconButton( | |
icon: const Icon(Icons.delete_outline), | |
onPressed: () async { | |
final dir = | |
await getApplicationDocumentsDirectory(); | |
final path = '${dir.path}/${item.key}'; | |
final f = File(path); | |
if (await f.exists()) await f.delete(); | |
await _load(); | |
}, | |
), | |
IconButton( | |
icon: const Icon(Icons.delete_outline), | |
onPressed: () async { | |
final confirm = await showDialog<bool>( | |
context: context, | |
builder: (BuildContext context) { | |
return AlertDialog( | |
title: const Text('Delete Frame'), | |
content: const Text('Are you sure you want to delete this saved frame? This action cannot be undone.'), | |
actions: [ | |
TextButton( | |
onPressed: () => Navigator.of(context).pop(false), | |
child: const Text('Cancel'), | |
), | |
TextButton( | |
onPressed: () => Navigator.of(context).pop(true), | |
child: const Text('Delete'), | |
), | |
], | |
); | |
}, | |
); | |
if (confirm == true) { | |
final dir = | |
await getApplicationDocumentsDirectory(); | |
final path = '${dir.path}/${item.key}'; | |
final f = File(path); | |
if (await f.exists()) await f.delete(); | |
await _load(); | |
} | |
}, | |
), |
cafad2d
to
1c18576
Compare
Build StatusBuild successful. APKs to test: https://github.com/fossasia/badgemagic-app/actions/runs/17548240363/artifacts/3952722329. Screenshots |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you. Please address sourcery-ai reviews.
There is often a distortion of the images you upload on the PRs. This is due to a wrong ratio of the height and width. Could you please fix this and avoid these issues in future? Already pointed it out previously. It confuses in reviews. |
@mariobehling okay will use the correct ratios for uploading images |
Fixes #1454
Changes
Introduced a feature to create animations by drawing 8 frames, preview them, save as animations, and transfer to badges.
Screenshots / Recordings
Checklist:
constants.dart
without hard coding any value.Summary by Sourcery
Add custom multi-frame animation capabilities including a creator interface, file persistence, and a management screen
New Features:
Enhancements: