-
Notifications
You must be signed in to change notification settings - Fork 242
feat: Added Support for the shapes in draw clipart #1375
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
feat: Added Support for the shapes in draw clipart #1375
Conversation
Reviewer's GuideThis PR adds predefined shape support to the draw clipart feature by refactoring the draw badge screen layout to include a scrollable shape selector, modularizing button builders, unifying gesture handling in BMBadge for freehand and shape drawing, and extending the DrawBadgeProvider to manage selected shapes and rendering logic. Class diagram for updated BMBadge gesture handlingclassDiagram
class BMBadge {
+ void _handlePanStart(DragStartDetails details)
+ void _handlePanUpdate(DragUpdateDetails details)
+ void _handlePanEnd(DragEndDetails details)
+ void _drawLine(int r1, int c1, int r2, int c2)
+ void _drawSquare(int row, int col, int radius, bool state)
+ void _drawRectangle(int row, int col, int halfWidth, int halfHeight, bool state)
+ void _drawCircle(int row, int col, int radius, bool state)
+ void _drawTriangle(int row, int col, int height, bool state)
+ void _safeSet(int row, int col, bool value)
}
BMBadge --> DrawBadgeProvider
Class diagram for updated DrawBadge UI structureclassDiagram
class DrawBadge {
+ Widget _buildDrawEraseButton(bool isDraw, IconData icon, String label)
+ Widget _buildResetButton()
+ Widget _buildSaveButton(FileHelper fileHelper)
+ Widget _buildShapesToggleButton()
+ Widget _buildShapeCard(BuildContext context, DrawShape shape, IconData icon, String label)
}
DrawBadge --> DrawBadgeProvider
DrawBadge --> BMBadge
DrawBadge --> DrawShape
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Build StatusBuild successful. APKs to test: https://github.com/fossasia/badgemagic-app/actions/runs/16964279189/artifacts/3764458814. Screenshots (Android)
Screenshots (iPhone)
Screenshots (iPad)
|
9916d64
to
2ca8cc1
Compare
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 @samruddhi-Rahegaonkar - I've reviewed your changes - here's some feedback:
- After drawing shapes via the BMBadge shape methods you’re mutating the grid directly but not notifying listeners—add notifyListeners() (or equivalent setState) after shape updates so the UI stays in sync.
- The code for converting touch positions into grid rows/columns (with clamping and cell size math) is duplicated—extract that into a shared helper to reduce repetition and improve readability.
- Currently each pan update draws a new shape on top of the last, leading to overlapping previews—consider clearing or redrawing the previous preview shape before rendering the updated one to avoid artifacts.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- After drawing shapes via the BMBadge shape methods you’re mutating the grid directly but not notifying listeners—add notifyListeners() (or equivalent setState) after shape updates so the UI stays in sync.
- The code for converting touch positions into grid rows/columns (with clamping and cell size math) is duplicated—extract that into a shared helper to reduce repetition and improve readability.
- Currently each pan update draws a new shape on top of the last, leading to overlapping previews—consider clearing or redrawing the previous preview shape before rendering the updated one to avoid artifacts.
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
b21bbc1
to
ae08ced
Compare
@samruddhi-Rahegaonkar Good idea, but the implementation needs improvement. The moving preview screen makes drawing difficult, compared to the previous version where the preview/drawing screen stayed stable. |
@hpdang How about reducing the preview size little bit and adding the tools to right of the preview vertically ? or keeping it strictly vertical without scrolling ? |
358b2f6
to
ae08ced
Compare
@nope3472 Please Share your insights what you were talking in todays meeting. |
@samruddhi-Rahegaonkar yeah just need to recheck will update you |
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.
Update all shape-drawing routines (circle, rectangle, triangle, etc.) so that if a shape extends beyond a grid edge, the overflowing part wraps around to the opposite edge.
aba5e48
to
22aa1e8
Compare
@nope3472 please review this |
Fixes #1374
Changes
Screenshots / Recordings
WhatsApp.Video.2025-07-18.at.18.51.27.mp4
Checklist:
constants.dart
without hard coding any value.Summary by Sourcery
Enable drawing of predefined shapes in the clipart drawing screen by adding a shapes toggle and selection panel, implement shape-based drawing algorithms in the badge view, and refactor UI and provider logic to support the new modes.
New Features:
Enhancements: