Skip to content
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

Marker drag event callback #10

Closed
chibatching opened this issue Feb 6, 2022 · 16 comments · Fixed by #11
Closed

Marker drag event callback #10

chibatching opened this issue Feb 6, 2022 · 16 comments · Fixed by #11
Assignees
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Comments

@chibatching
Copy link
Contributor

I want to get marker drag event callback and manipulate marker-related work.

So, Marker composable have to have dragging callback parameter like onMarkerDrag onMarkerDragStart onMarkerDragEnd similar with onClick and onInfoWindow~

It is an alternative to OnMarkerDragListener of Android view version.

@chibatching chibatching added triage me I really want to be triaged. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. labels Feb 6, 2022
@jpoehnelt
Copy link
Contributor

@chibatching Thank you for opening this issue. 🙏
Please check out these other resources that might be applicable:

This is an automated message, feel free to ignore.

@arriolac
Copy link
Contributor

arriolac commented Feb 7, 2022

This is what MarkerDragState is for. You should be able to do the following:

val markerDragState = rememberMarkerDragState()

GoogleMap() {
    Marker(
        draggable = true,
        markerDragStae = markerDragState,
        // ...
    )
}
Text(text = "The marker drag state is $markerDragState.dragState")

Definitely open to reevaluate this although I think marker drag state should only be observed either using the State-based approach or the callback-based approach, not both. Please provide a bit more context if for some reason the latter works better for your use case.

@arriolac arriolac added needs more info This issue needs more information from the customer to proceed. and removed triage me I really want to be triaged. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. labels Feb 7, 2022
@chibatching
Copy link
Contributor Author

@arriolac
I have to know marker LatLng after drag to execute some process using the position after the move.
It is not enough to observe only the drag state.

My usecase is like below

var markerPoint by remember { mutableStateOf(defaultLatLng) }

GoogleMap() {
    Marker(
        position = markerPoint,
        draggable = true,
        onMarkerDragEnd = { marker ->
            markerPoint = maker.position
            viewModel.updateUserSelectedPosition(marker.position)
        },
    )
}

@chibatching
Copy link
Contributor Author

I think it doesn't need to be a callback style if the drag state has marker object or position info.

@arriolac
Copy link
Contributor

arriolac commented Feb 7, 2022

I think it doesn't need to be a callback style if the drag state has marker object or position info.

Yeah, this is what I'm thinking as well. There should be a Marker property in the MarkerDragState object. Will add that.

@chibatching
Copy link
Contributor Author

chibatching commented Feb 8, 2022

I updated my PR #11 to add Marker property.
Would you check it?

@arriolac
Copy link
Contributor

arriolac commented Feb 10, 2022

Please see my comment on #11 (comment) for how this feature should be implemented.

@arriolac arriolac added type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. and removed needs more info This issue needs more information from the customer to proceed. labels Feb 10, 2022
@arriolac arriolac added the priority: p2 Moderately-important priority. Fix may not be included in next release. label Mar 1, 2022
@arriolac arriolac added this to the 2.0 milestone Mar 14, 2022
@bubenheimer
Copy link
Contributor

bubenheimer commented Jun 4, 2022

AFAICT the current implementation (in 2.2.0) does not take into account the difference between drag state and drag events. The onMarkerDrag... callbacks reflect events, while the current 'MarkerState' approach reflects state. This implies information loss; some or all drag events may never be seen by an observer. An approach like this may be reasonable when there is only a single type of event, DRAG, but there are 3 different types, and an observer may never notice that the marker was dragged, as START, and DRAG, may fall through the cracks; even a distinct END may not be seen by a slow observer when another drag event starts quickly. Really, the only thing I can observe here is whether I am currently dragging or not, essentially binary drag state.

For example, I have a use case where a user's marker drag events are not committed to the data model when the marker is dragged to an invalid position. This looks hard to implement reliably at the moment, if possible at all; I need to save the prior marker position at drag START, verify the final position at drag END and commit or roll back, and update dependent visualizations upon DRAG. I don't see a way to access the raw events via a listener, either. This use case causes zero problems with the standard GoogleMap event listener approach.

What you want to expose here is a Flow or possibly a ReceiveChannel of drag events. The consumer will have to determine the buffering strategy, not the compose wrapper.

@bubenheimer
Copy link
Contributor

bubenheimer commented Jun 5, 2022

There also seems to be a misconception about what state hoisting means. When I hoist state, I control the state at a higher level of the composition, and the state object is not present at deeper levels of the composition, which cannot directly change the state. Yet here I am expected to pass the "hoisted" state object down into the composition, and it will change under my nose when the user drags the marker. That is the exact opposite of state hoisting. It means a lot of trouble for my outside data model.

https://googlemaps.github.io/android-maps-compose/maps-compose/com.google.maps.android.compose/-marker-state/

A state object that can be hoisted to control and observe the marker state.

@arriolac
Copy link
Contributor

arriolac commented Jun 6, 2022

@bubenheimer thanks for the feedback on the current marker drag API. I'm a bit confused what you mean by information loss as all marker drag transition states should be reported in MarkerState.dragState—maybe I'm missing something?

From your feedback, seems like it is desirable to have an event-based API to accomplish validation type actions as the current state-based approach for drag state is difficult to work with. If you can capture some suggestions you have on a separate issue we can reevaluate and improve upon the existing API.

@bubenheimer
Copy link
Contributor

bubenheimer commented Jun 6, 2022

Thanks. In general there is no way to guarantee that an observer will see transitory states of a MutableState, the observer only gets to see snapshot state (not events), and it may not even see all of the snapshot states. This is a fundamental characteristic of snapshot state. For background, see the bottom of the snapshotFlow documentation:

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 will propose something in a separate issue.

Edit: removed comment about "state object hoisting" in favor of broader discussion in new issue.

@zaheeroz
Copy link

Hope we will see this event soon, both drag start and drag end. I could do with drag start but drag end is critical.

@bubenheimer
Copy link
Contributor

bubenheimer commented Sep 23, 2022

@zaheeroz You may be able to "reconstruct" drag start & end to some degree from the information that is available, but you need to take into account that you may miss some ends and/or starts, when a new start-drag-end cycle starts too quickly after the last one ends.

@zaheeroz
Copy link

Thanks @bubenheimer that's great. Could you provide a link that can help in this regard. I can start a drag by first clicking on the marker to register a 'drag start' then drag the marker. But the problem is even click fires on 'pressed' and not ' on release' so I cant get the location where the marker was dragged (end of drag). So any help in this regard will be greatly appreciated.

@bubenheimer
Copy link
Contributor

@zaheeroz I don't have a link, sorry.

@wangela
Copy link
Member

wangela commented Feb 7, 2023

I believe the initial request here was addressed in v2.0, so closing this issue. Conversation continues about improving marker drag state in #149.

@wangela wangela closed this as completed Feb 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants