Marker dragging: fix loss of drag events and difficulty observing marker positions

[bubenheimer]

android-maps-compose 2.2.1

The current API design of Marker and MarkerState poses significant challenges. This issue is a continuation of comments I made in #10; @arriolac asked to move discussion to a new issue.

1. 'DragState' events are lost, in particular START, but also DRAG, and, theoretically, even END.

This is due to the characteristics of snapshot state in relation to events:

Observable state is a lossy compression of the events that produced that state.

https://developer.android.com/reference/kotlin/androidx/compose/runtime/package-summary#snapshotFlow(kotlin.Function0)

I have a use case where I need to reliably detect START and END to temporarily keep a backup of the old marker location to restore in case the final marker drag location is deemed invalid by business logic.

2. Hoisted state objects are not suitable for representing collections of app marker data.

Hoisted state objects can be a suitable vessel for maintaining UI state. However, they are an anti-pattern for representing data that is ultimately maintained in an app data model. Observing requires using snapshotFlow for each indivdual state object, and updating implies feeding model data back to the state object in some manner.

An app commonly maintains a collection of markers, so a collection of MarkerState objects is needed, one for each marker. Markers may be added or removed, adding significant complexity to the task of keeping separate collections of marker data in sync. Typically one will opt to recreate the entire collection if one element changes in any way.

I will add more detail and proposals in comments below.

Clarification: In my main use case for this feature, markers define the corners of a polyline/polygon, so order is significant, and the marker collection is a list, where updates, inserts, and deletions can occur at any position. Keeping lists of variable size with changing marker content synchronized is a more challenging problem than for example maps of markers where order is not significant; I use the latter in another case, and it is easier to deal with.

[bubenheimer]

Regarding the loss of DragState events: the MarkerState.dragState property needs to be replaced with a boolean MarkerState.dragging property. This is the only thing that can be validly represented as state.

To make DragState events available in a reliable manner, a callback on the Marker class should be made available, which is passed a DragState, and the current marker position. This callback can come directly from the Play Maps SDK dragging callbacks. Additional detail below.

[bubenheimer]

Regarding problems with hoisted state objects: remove position from MarkerState and replace by the standard Compose approach of hoisting state: pass position from the user's data model into the Marker composable as a parameter, and pass back updated position via the DragState callback mentioned above.

I believe it is possible to completely separate the SDK's position changes and app-controlled position changes. I'm assuming that the only time the map SDK impl changes the marker position on its own is dragging. Dragging is reliably demarcated by START and END events.

maps-compose needs to box the SDK's dragging events. Dragging position should not generally be reflected in maps-compose marker position, but only be updated in response to Marker recomposition with a new position value (which would typically be triggered from the marker dragging callback in some indirect way).

To avoid visual artifacts, compose-maps needs to delay effectuating visual changes in Marker's position parameter until drag END (a.k.a. wait until drag END before informing the Maps SDK about whatever position maps-compose has at that point). Essentially maps-compose needs to treat marker position changes from the SDK as tentative, and "restore" or "impose" whatever position it has at the time of drag END.

Typically dragging callbacks will update Marker's position parameter with whatever position is passed as a parameter, via hoisted state or app data model. This is the common way of structuring a compose API with hoisted state (pass in state value as parameter, receive changed values via callback, update parameter via recomposition), so the only thing that needs explanation is the SDK-controlled marker position during a drag, requiring deferred visual updates for the compose-controlled position parameter.

The boolean Marker.draggable parameter should be replaced with the dragging callback lambda, with null passed for dragging disabled.

The above proposal turns Map SDK's marker drag design on its head, replacing it with the default API a compose user would expect, and it can do it very cleanly, I believe.

The only gotcha I can think of is an app's inability to influence the actual marker position on the map while the marker is being dragged. This is generally a bad idea due to visual artifacts, but there is one case I can think of where an app may choose to do it despite artifacts; more in a future comment.

[bubenheimer]

The use case I was alluding to in the last comment is marker dragging and keeping the marker from jumping out of place. When the marker starts dragging, Maps SDK moves the marker up and out of place a good amount to make it visible from underneath a user's "finger". This approach can cause a lot of problems, so in some cases one might choose to manually override the marker position upon each START/DRAG/END event to put the marker back in place. This causes visual artifacts, and is a grossly unsupported hack of map's data structures, but depending on the use case one might choose to try it anyway.

Overriding marker drag position manually in this fashion would not work with the approach I sketched. Marker position set via composition during dragging will take effect only after dragging is finished.

The issues with marker jump on drag were previously brought up with Maps SDK and dismissed. The main issue I saw is this: https://issuetracker.google.com/issues/35824023.

If this is a concern then the best approach may be to open a new issue with Maps SDK. Perhaps it would be addressed with added leverage from @arriolac. The marker jump problem affects one of my use cases as well, where the marker needs precise repositioning upon dragging.

Alternatively, hacky use cases like this could likely be implemented via the new direct access to Maps SDK's GoogleMap from maps-compose.

[bubenheimer]

N.B. I've reviewed the use cases and concerns about prior implementations and API designs from #10 and #11. I believe the API I am proposing in this issue addresses them.

[bubenheimer]

Upon implementing this new API, a public MarkerState object is no longer particularly meaningful and should be refactored. The only remaining stateful element is the dragging boolean, which by itself does not look valuable enough to carry the weight of a whole state object.

[bubenheimer]

Thoughts?

[arriolac]

Thanks @bubenheimer for creating this issue. This is very detailed and helps us improve the API for marker dragging. I agree with a lot of the proposed changes—it will be a breaking change though and I'll label it as such. We may want to release this with any other breaking change requests/issues in the repo.

[bubenheimer]

Thanks @arriolac, makes sense. I made one clarification in the original comment that my primary use case is changing lists of markers, which is a more challenging problem than unordered collections.