Draw a uniform filled shape on a single tap - #591
Conversation
Tapping the drawing view (a touch with no drag) left a half-formed shape that endShape() discarded, so a single tap drew nothing. Detect a tap at the view level (a touch that never moves beyond TAP_TOLERANCE) and replace it with a uniform filled version of the selected shape, sized from the shape thickness: a circle for an oval, a square for a rectangle. Shapes with no single-point form (line, arrow, freehand) and taps while erasing draw nothing. Adds instrumented tests covering each case.
fd74254 to
5203ec1
Compare
There was a problem hiding this comment.
Pull request overview
This PR makes single taps on DrawingView produce a visible result for supported shapes by detecting taps at the view level (movement within a small tolerance) and, on tap, drawing a uniform filled circle/square sized from the current shape thickness. It also adds instrumented tests covering the new tap behavior.
Changes:
- Add view-level tap detection (tap vs drag) and route taps to a new
handleTap(...)implementation. - On tap, draw a filled uniform circle (Oval) or square (Rectangle); do nothing for line/arrow/brush and while erasing.
- Add instrumented tests for tap behavior for oval/rectangle, pointless shapes, and erasing.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| photoeditor/src/main/java/ja/burhanrashid52/photoeditor/shape/RectangleShape.kt | Adds drawSquare(...) helper for tap-created square paths. |
| photoeditor/src/main/java/ja/burhanrashid52/photoeditor/shape/OvalShape.kt | Adds drawCircle(...) helper for tap-created circle paths. |
| photoeditor/src/main/java/ja/burhanrashid52/photoeditor/DrawingView.kt | Implements tap detection and tap-to-filled-shape behavior. |
| photoeditor/src/androidTest/java/ja/burhanrashid52/photoeditor/DrawingViewTouchEventTest.kt | Adds new instrumented tests validating tap outcomes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@stephanepechard Have a look at this PR agent review. And feel free to close the conversation if its not relevant. Please merge it and release it once that is done. |
|
@burhanrashid52 I cannot merge, I need an approving review. AI comments have been resolved. |
Problem
A single tap on the drawing view (a touch that ends without dragging) produced nothing for most shapes:
endShape()detected the empty/half-formed shape viahasBeenTapped()and removed it, with thehandleTap(...)call left commented out.Change
TAP_TOLERANCEis treated as a tap rather than a drag (more reliable than the per-shapehasBeenTapped()bounds check, which never fired forBrushShape).OvalShape.drawCircle(...)/RectangleShape.drawSquare(...)build the centered shape path.Tests
Adds instrumented tests in
DrawingViewTouchEventTest:testTapWithOvalDrawsUniformFilledCircletestTapWithRectangleDrawsUniformFilledSquaretestTapWithPointlessShapeDrawsNothing(line/brush/arrow)testTapWhileErasingDrawsNothing