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

Prevent annotation changes from invalidating unrelated drag actions #1159

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

TroyStopera
Copy link

@TroyStopera TroyStopera commented Sep 29, 2020

Relates to, but does not resolve, #1029

Motivation for change

I have a use case where markers on the map are 'linked' together via lines. As the user drags a marker, it is necessary that the line is programmatically updated as well. Due to the current logic which ensures data integrity, updates to the associated lines invalidate/stop the drag action.

This simple change adds a new method to the DraggableAnnotationController which allows the AnnotationManager to notify the controller of more fine-grain changes to the map source. If the change wasn't to the dragged annotation, the drag will continue.

Future work

As stated above, this does not solve 1029, which asks that changes to certain attributes of a dragged annotation don't stop the drag action. For example, a circle should be able to change size while being dragged, as this does not logically interfere with the annotation moving.

Although not included in this PR, a solution to that could be to add a field/method to the Annotation class which computes a 'drag integrity hash' which hashes all fields that must remain static during a drag. When a drag starts, the DraggableAnnotationManager can take note of this hash. When notified of an update, the controller can check the current integrity hash of the annotation, and if it doesn't match the original stop the drag.

If that solution is agreeable, I can update this PR to include that.

@LukasPaczos LukasPaczos requested a review from a team October 7, 2020 13:24
@troy-skydio
Copy link

@LukasPaczos Note sure what I need to do to get eyes on this. I'd really like to get this working in master, so that I can update to the latest versions released by Mapbox.

@LukasPaczos
Copy link
Contributor

Tagging @mapbox/maps-android here.

@tobrun
Copy link
Member

tobrun commented May 27, 2021

CI needs to be restored for this repo before we will be able to merge

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants