-
Notifications
You must be signed in to change notification settings - Fork 244
fix: fixed the drawclipart function to follow the finger movements #1521
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?
fix: fixed the drawclipart function to follow the finger movements #1521
Conversation
Reviewer's GuideThis PR refactors the badge drawing widget to compute grid coordinates dynamically via BadgeUtils (replacing the old static cell-size logic), ensures the first freehand touch point is rendered immediately, updates rendering aspect ratio to 4.0 for the badge, and removes an extraneous line in the CI push workflow. Sequence diagram for freehand drawing with immediate touch renderingsequenceDiagram
participant User as actor User
participant BMBadge
participant DrawBadgeProvider
User->>BMBadge: Touches badge (pan start)
BMBadge->>DrawBadgeProvider: pushToUndoStack()
alt Freehand shape selected
BMBadge->>BMBadge: _localToGrid(dragStart)
BMBadge->>DrawBadgeProvider: setCell(x, y, isDrawing, preview: false)
end
User->>BMBadge: Moves finger (pan update)
BMBadge->>BMBadge: _localToGrid(localPosition)
BMBadge->>DrawBadgeProvider: clearPreviewGrid()
BMBadge->>DrawBadgeProvider: setCell(x, y, isDrawing, preview: false)
Class diagram for updated BMBadge and BadgeUtils usageclassDiagram
class BMBadge {
+void Function(DrawBadgeProvider provider)? providerInit
}
class _BMBadgeState {
-DrawBadgeProvider drawProvider
-BadgeUtils badgeUtils
-Offset? dragStart
+_getBadgeRenderSize(): Size
+_getLocalPosition(globalPosition: Offset): Offset
+_localToGrid(localPosition: Offset): (int x, int y)
+_handlePanStart(details: DragStartDetails)
+_handlePanUpdate(details: DragUpdateDetails)
+gridWidth: int
+gridHeight: int
}
class BadgeUtils {
+getBadgeOffsetBackground(size: Size): MapEntry<double, double>
+getBadgeSize(offsetHeight: double, offsetWidth: double, size: Size): MapEntry<double, double>
+getCellStartCoordinate(offsetWidth: double, offsetHeight: double, badgeWidth: double, badgeHeight: double): MapEntry<double, double>
}
BMBadge "1" *-- "1" _BMBadgeState
_BMBadgeState "1" -- "1" BadgeUtils
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 and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `lib/virtualbadge/view/draw_badge.dart:79-80` </location>
<code_context>
+ double relativeX = localPosition.dx - cellStartX;
+ double relativeY = localPosition.dy - cellStartY;
+
+ int col = (relativeX / (cellSize * 0.93)).floor().clamp(0, gridWidth - 1);
+ int row = (relativeY / cellSize).floor().clamp(0, gridHeight - 1);
+
+ return (x: row, y: col);
</code_context>
<issue_to_address>
**suggestion:** The horizontal compression factor (0.93) is hardcoded; consider centralizing or documenting its origin.
Defining the compression factor as a constant or sourcing it from a shared configuration will improve maintainability and prevent inconsistencies if the value changes or is reused.
Suggested implementation:
```
// Convert touch position to grid coordinates
// Accounting for the horizontal compression factor used in rendering
const double _horizontalCompressionFactor = 0.93;
double relativeX = localPosition.dx - cellStartX;
double relativeY = localPosition.dy - cellStartY;
int col = (relativeX / (cellSize * _horizontalCompressionFactor)).floor().clamp(0, gridWidth - 1);
int row = (relativeY / cellSize).floor().clamp(0, gridHeight - 1);
return (x: row, y: col);
```
If this code is inside a class, consider moving the constant definition (`const double _horizontalCompressionFactor = 0.93;`) to the top of the class for better visibility and reuse. If the compression factor is used in multiple files, you may want to define it in a shared configuration or constants file and import it where needed.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
bb55023 to
c8c02ab
Compare
|
@mariobehling @hpdang Corrected finger-to-grid coordinate mapping so drawing follows finger movements accurately. |
Build StatusBuild successful. APKs to test: https://github.com/fossasia/badgemagic-app/actions/runs/18929357777/artifacts/4413444683. Screenshots |







-1_home_screen.png?raw=true)
-2_text_badge.png?raw=true)
-3_emoji_badge.png?raw=true)
-4_inverted_emoji_badge.png?raw=true)
-5_saved_badges.png?raw=true)
-6_saved_badges_clicked.png?raw=true)
-7_draw_badge.png?raw=true)
Fixes #1520
Checklist:
constants.dartwithout hard coding any value.Summary by Sourcery
Map finger movements correctly onto the badge grid by centralizing dimension and offset calculations in BadgeUtils, replacing inline grid conversion logic in pan handlers, and updating the badge aspect ratio; also remove an extraneous line in the CI workflow.
Bug Fixes:
Enhancements:
CI: