Use job color in geometry tasks and refactor feature renderers #1951
Conversation
…o gino-m/1910/marker-render-refactor # Conflicts: # ground/src/main/java/com/google/android/ground/ui/IconFactory.kt # ground/src/main/java/com/google/android/ground/ui/map/gms/FeatureClusterRenderer.kt # ground/src/main/java/com/google/android/ground/ui/map/gms/GoogleMapsFragment.kt # ground/src/main/res/drawable/cluster_marker.xml
…o gino-m/1907/refactor-feature-renders # Conflicts: # ground/src/main/java/com/google/android/ground/persistence/remote/firebase/schema/LoiCollectionReference.kt # ground/src/main/java/com/google/android/ground/ui/datacollection/tasks/location/CaptureLocationTaskViewModel.kt # ground/src/main/java/com/google/android/ground/ui/datacollection/tasks/point/DropAPinTaskViewModel.kt # ground/src/main/java/com/google/android/ground/ui/map/gms/FeatureClusterRenderer.kt # ground/src/main/java/com/google/android/ground/ui/map/gms/GoogleMapsFragment.kt
gino-m
changed the title
...c/main/java/com/google/android/ground/ui/datacollection/tasks/point/DropAPinTaskViewModel.kt
Show resolved
Hide resolved
...in/java/com/google/android/ground/ui/datacollection/tasks/polygon/PolygonDrawingViewModel.kt
Show resolved
Hide resolved
| @@ -55,6 +65,8 @@ constructor(resources: Resources, private val uuidGenerator: OfflineUuidGenerato | |||
| id = uuidGenerator.generateUuid(), | |||
| type = FeatureType.USER_POINT.ordinal, | |||
| geometry = point, | |||
| // TODO: Set correct pin color. | |||
| style = Feature.Style(pinColor), | |||
There was a problem hiding this comment.
As this is the only usage, should we consider inlining the code? Then we'd have to set the job to a class var in the super class. Thoughts?
style = Feature.Style(job.getDefaultColor())
There was a problem hiding this comment.
The superclass is scoped more widely that geometry tasks, and each geometry task may require slightly different styling treatments. Also, I'd prefer to avoid too much indirection to keep things locally comprehensible.
Sorry I missed this TODO, removing it.
| loiRepository.getLocationsOfInterest(survey).map { | ||
| it.map { loi -> loi.toFeature() }.toPersistentSet() | ||
| } |
There was a problem hiding this comment.
nit: Can we get rid of the nesting by using flatMap?
There was a problem hiding this comment.
flatMap pulls the elements of a list or flow up a level. That would change the output to Flow<Feature>, which is not what we want here; we want a full snapshot of all the displayed features on each emission.
| submissionCount + submissionRepository.getPendingCreateCount(id) - | ||
| submissionRepository.getPendingDeleteCount(id) > 0, |
There was a problem hiding this comment.
Can we move these to separate lines for better readability?
There was a problem hiding this comment.
Sure, coming right up
|
@shobhitagarwal1612 PTAL? |
Fixes #1907
Fixes #1952
@JSunde @shobhitagarwal1612 PTAL?