Improve selecting shapes/traces #5463
Conversation
| markers.add(markerDescription.point) | ||
| markerIcons.add(markerDescription.iconDescription) | ||
| return markers.size - 1 | ||
| val featureId = generateFeatureId() |
There was a problem hiding this comment.
We want to generate random IDs to prevent our tests from allowing implementations that rely on sequential ones.
|
|
||
| private lateinit var map: MapFragment | ||
| private lateinit var summarySheetBehavior: BottomSheetBehavior<*> | ||
| private lateinit var summarySheet: SelectionSummarySheet | ||
| private lateinit var bottomSheetCallback: BottomSheetCallback | ||
|
|
||
| private val itemsByFeatureId: MutableMap<Int, MappableSelectItem> = mutableMapOf() | ||
| private val featureIdsByItemId: MutableMap<Long, Int> = mutableMapOf() |
There was a problem hiding this comment.
It maybe feels excessive to have two Map indexes here, but otherwise we'd need to do linear searches on several click events, and we build these maps in a loop we're already performing.
| map.zoomToPoint(MapPoint(point.latitude, point.longitude), map.zoom, true) | ||
| if (!skipSummary) { | ||
| if (item.points.size > 1) { | ||
| map.zoomToBoundingBox(item.points, 0.8, true) |
There was a problem hiding this comment.
We're doing a "zoom to fit" when selecting a shape/trace rather than maintaining the user zoom. I think that addresses some of the feedback around it being hard to see which item is selected (without having to deal with color/shape changes), but as @lognaturel has pointed out to me, it isn't great in situations where the user taps on the wrong item and then loses context.
|
There is possibility to cover one of the points in a trace by the Select Point window in Google and OSM map source. For Mapbox zoom works correctly but after rotating the screen almost 50% of the trace is covered by Select Window. XRecorder_07032023_130807.1.mp4Rotated screen:
|
|
@srujner yeah, I think this is fine. |
|
Tested with Success! Verified on Android 10 Verified Cases:
|
|
Tested with Success! Verified on Android 13 |
Closes #5447
As well as improving the zoom behaviour on selection and fixing Mapbox, this PR fixes a bug we hadn't found that would only crop up in cases where traces and shapes were interleaved in the data.
What has been done to verify that this works as intended?
New tests and verified manually.
Why is this the best possible solution? Were any other approaches considered?
Comments inline.
How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?
As well as the changes described in the issue, it'd be good to test select one from map with both points and traces/shapes in a single question. There were some problems we hadn't caught in this scenario if the points and traces are interleaved in the form data set or the GeoJSON file that this PR should address.
Before submitting this PR, please make sure you have:
./gradlew checkAlland confirmed all checks still pass OR confirm CircleCI build passes and run./gradlew connectedDebugAndroidTestlocally.