-
Notifications
You must be signed in to change notification settings - Fork 124
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
Render the draw vertex to be open to prevent occluding the target point. #2241
Conversation
<path | ||
android:pathData="M12,12m-10.5,0a10.5,10.5 0,1 1,21 0a10.5,10.5 0,1 1,-21 0" | ||
android:strokeWidth="0" | ||
android:fillColor="@color/drawAreaLineStringStrokeColor"/> |
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.
Sorry to ask, but we just finalized the designs here:
Any chance you could update the styling to match? Just asked @rawbzz to confirm whether the color should match the polygon (job color) or be fixed. Thanks for your patience!
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.
Yes! No worries, happy to refactor!
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.
And I'm assuming that we'll be using the white circle to indicate the vertices, including the endcap, but the plus-sign is going to move independently on its own when panning, to minimize occlusion of the base layer?
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.
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.
And I'm assuming that we'll be using the white circle to indicate the vertices, including the endcap, but the plus-sign is going to move independently on its own when panning, to minimize occlusion of the base layer?
Hey I hadn't thought of that, but it's a great idea. The only this is that this will only work because of a bug / incomplete FR where the edge only snaps to the map center when the camera stops moving. But the way you've done it makes this a feature rather than a bug, so LGTM. @rawbzz FYI
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.
A few suggestions. Thanks!
...main/java/com/google/android/ground/ui/datacollection/tasks/polygon/DrawAreaTaskViewModel.kt
Outdated
Show resolved
Hide resolved
ground/src/main/java/com/google/android/ground/ui/util/ColorUtil.kt
Outdated
Show resolved
Hide resolved
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.
P.S. Run Gradle target ktfmtFormat
to fix formatting issues.
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.
⭕ LGTM!
Some code quality checks are still failing. You can run these checks locally via gradle target |
Thank you! Turns out the whole file can be deleted, as it is now unused. Done. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2241 +/- ##
=========================================
Coverage 52.42% 52.42%
Complexity 1222 1222
=========================================
Files 317 317
Lines 6264 6264
Branches 659 659
=========================================
Hits 3284 3284
Misses 2617 2617
Partials 363 363 ☔ View full report in Codecov by Sentry. |
Fixes #2110
Sets the draw tool to a static color and sets the draw target to be an open circle, allowing for better view of the data underneath.
@gino-m