-
-
Notifications
You must be signed in to change notification settings - Fork 439
feat: add onSingleTap function support for Android Auto
#3730
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
base: main
Are you sure you want to change the base?
feat: add onSingleTap function support for Android Auto
#3730
Conversation
platform/android/MapLibreAndroid/src/main/java/org/maplibre/android/maps/MapLibreMap.java
Show resolved
Hide resolved
platform/android/MapLibreAndroid/src/main/java/org/maplibre/android/maps/MapLibreMap.java
Outdated
Show resolved
Hide resolved
louwers
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't really understand this pull request.
This pull request introduces a new method for handling single tap events
What I see is a method onSingleTap(float x, float y) on MapLibreMap. You would not override that class, so it's not a method for handling single tap events. It is a method to simulate single tap events, correct?
On Android Auto, we can’t add a listener for override fun onClick(x: Float, y: Float) {
mapLibreMap?.onSingleTap(x, y)
} |
|
This func meaning: |
|
It does not make sense to call such a method The method you added has almost the same contents as |
No, I couldn’t find a way to directly call this function. I believe it was created to be handled from the Android surface as well. This logic introduces fewer changes and has minimal impact on the current flow, since this code block is only used by Android Auto and does not affect anything else |
|
@louwers Could you let us know if any changes are needed before we merge this PR? |
@frankkienl What do you think? |
|
the "onX" naming convention is very normal for Android; |
|
"I am not sure if MapLibreMap is the right place for it." |
|
"If possible we should try to deduplicate the logic between this method and StandardGestureListener.onSingleTapConfirmed." |
Thanks for reply. Cause I'm not familiar with java or kotlin native, so I think keep it independent of anything else is better than change current logic of MaplibreMap and other class @@ |
This pull request introduces a new method for handling single tap events on the map and adds comprehensive unit tests to ensure correct behavior. The main focus is on improving marker selection/deselection logic and interaction with gesture management when a tap occurs.
Map tap event handling:
onSingleTap(float x, float y)method toMapLibreMap, which processes single tap events by delegating to the annotation manager.Unit tests for tap handling:
MapLibreMapTest.kt:testOnSingleTapHandledByAnnotationManagerverifies that taps handled by the annotation manager do not trigger marker deselection.testOnSingleTapNotHandled_DeselectsAndNotifiesverifies that if a tap is not handled, markers are deselected and the gestures manager is notified.Reference
MapGestureDetector.javain MapLibre