fix: macOS, iPad Delete Action Fails and Causes Bottom Overflow Error in Confirmation Dialog #1326
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Reviewer's GuideThis PR refactors the delete confirmation dialog into a responsive, scrollable layout using ScreenUtil, connects the OK button to the badge deletion logic with a UI refresh, and simplifies the SnowFlakeAnimation by moving frame calculations out of the loop, handling empty grids, and condensing the canvas update. Sequence Diagram for Badge Deletion ProcesssequenceDiagram
actor User
participant Dialog as DeleteConfirmationDialog
participant Logic as BadgeDeletionService
participant UI as ClipartListUI
User->>Dialog: Clicks "OK" button
Dialog->>Logic: deleteBadge()
Logic->>Logic: Performs deletion
Logic->>UI: Trigger UI update
UI->>UI: Refreshes list
Updated Class Diagram for DeleteBadgeDialogclassDiagram
class DeleteBadgeDialog {
+Widget build(BuildContext context)
// build() method: UI structure refactored for responsiveness and to fix overflow:
// - Root widget changed from fixed-size Container to Padding.
// - Uses Column with MainAxisSize.min for intrinsic content sizing.
// - Leverages ScreenUtil for responsive dimensions and font sizes.
// - Confirmation message Text is wrapped with Expanded for better layout.
}
StatelessWidget <|-- DeleteBadgeDialog
Updated Class Diagram for SnowFlakeAnimationclassDiagram
class SnowFlakeAnimation {
+animate(int badgeHeight, int badgeWidth, int animationIndex, List<List<bool>> processGrid, List<List<bool>> canvas) void
// animate() method: Logic refactored for clarity and performance:
// - Frame count calculation (framesCount) is now outside the loops.
// - Added a safeguard: framesCount defaults to 1 if calculated as 0.
// - Current frame calculation (currentFrame) simplified.
// - Starting column (startCol) calculation based on currentFrame.
// - Canvas cell update logic is more direct.
}
BadgeAnimation <|-- SnowFlakeAnimation
File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
samruddhi-Rahegaonkar
changed the title
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @samruddhi-Rahegaonkar - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `lib/view/widgets/badge_delete_dialog.dart:38` </location>
<code_context>
- width: 20,
+ Expanded(
+ child: Text(
+ 'Are you sure want to delete this badge?',
+ style: TextStyle(fontSize: 14.sp),
+ ),
</code_context>
<issue_to_address>
Fix grammar in confirmation message
Change the prompt to: “Are you sure you want to delete this badge?”
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
child: Text(
'Are you sure want to delete this badge?',
style: TextStyle(fontSize: 14.sp),
),
=======
child: Text(
'Are you sure you want to delete this badge?',
style: TextStyle(fontSize: 14.sp),
),
>>>>>>> REPLACE
</suggested_fix>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
37
to
31
| child: Text( | ||
| 'Are you sure want to delete this badge?', | ||
| style: TextStyle(fontSize: 14.sp), | ||
| ), |
There was a problem hiding this comment.
nitpick (typo): Fix grammar in confirmation message
Change the prompt to: “Are you sure you want to delete this badge?”
Suggested change
| child: Text( | |
| 'Are you sure want to delete this badge?', | |
| style: TextStyle(fontSize: 14.sp), | |
| ), | |
| child: Text( | |
| 'Are you sure you want to delete this badge?', | |
| style: TextStyle(fontSize: 14.sp), | |
| ), |
samruddhi-Rahegaonkar
force-pushed
the
issue#1219
branch
from
ec31ae8 to
f20a1dd
Compare
Build StatusBuild successful. APKs to test: https://github.com/fossasia/badgemagic-app/actions/runs/16625096560/artifacts/3649326250. Screenshots (Android)
Screenshots (iPhone)
Screenshots (iPad)
|
||||||||||||||||||||||||
-1_home_screen.png?raw=true)
-2_text_badge.png?raw=true){kind=icon}
-3_emoji_badge.png?raw=true){kind=icon}
-4_inverted_emoji_badge.png?raw=true){kind=icon}
-5_saved_badges.png?raw=true)
-6_saved_badges_clicked.png?raw=true)
-7_draw_badge.png?raw=true){kind=icon}
nope3472
suggested changes
Jun 19, 2025
… in Confirmation Dialog fix: according to review
samruddhi-Rahegaonkar
force-pushed
the
issue#1219
branch
from
24bc1e2 to
f091233
Compare
|
@nope3472 please review this again |
|
@samruddhi-Rahegaonkar please update the branch so we can merge |
Done 👍 |
|
@Jhalakupadhyay we need an approval from you to merge this |
Jhalakupadhyay
approved these changes
Jul 31, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #1219
Changes
Dialogcontent in aSingleChildScrollView.ScreenUtilfor padding, spacing, and font sizes.Screenshots / Recordings
Checklist:
constants.dartwithout hard coding any value.Summary by Sourcery
Fix the delete confirmation dialog to prevent overflow and enable actual deletion with responsive layout, and simplify the snowflake animation grid logic
Bug Fixes:
Enhancements: