Skip to content

Conversation

thanhdang198
Copy link
Contributor

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:

  • Added a new onSingleTap(float x, float y) method to MapLibreMap, which processes single tap events by delegating to the annotation manager.

Unit tests for tap handling:

  • Added two tests in MapLibreMapTest.kt:
    • testOnSingleTapHandledByAnnotationManager verifies that taps handled by the annotation manager do not trigger marker deselection.
    • testOnSingleTapNotHandled_DeselectsAndNotifies verifies that if a tap is not handled, markers are deselected and the gestures manager is notified.

Reference

Copy link
Member

@louwers louwers left a 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?

@thanhdang198
Copy link
Contributor Author

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 onMapClick because the map is only rendered as a bitmap and displayed on the AA surface. With this function, we can notify the MapView when the user clicks on the AA surface, and the map controller will take care of the rest of the logic—such as triggering the onMapClick callback (returning latitude and longitude without calling queryFeature), handling annotation gestures, and more.

        override fun onClick(x: Float, y: Float) {
            mapLibreMap?.onSingleTap(x, y)
        }

@thanhdang198
Copy link
Contributor Author

This func meaning:
Hey MapView, the surface was clicked at position (x, y). Please handle all annotations for me.

@louwers
Copy link
Member

louwers commented Aug 19, 2025

It does not make sense to call such a method onSingleTap, because MapLibreMap is not a listener.

The method you added has almost the same contents as StandardGestureListener.onSingleTapConfirmed. Can't you call that method directly to simulate a click?

@thanhdang198
Copy link
Contributor Author

It does not make sense to call such a method onSingleTap, because MapLibreMap is not a listener.

The method you added has almost the same contents as StandardGestureListener.onSingleTapConfirmed. Can't you call that method directly to simulate a click?

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

@thanhdang198
Copy link
Contributor Author

@louwers Could you let us know if any changes are needed before we merge this PR?

@louwers
Copy link
Member

louwers commented Aug 21, 2025

  • It should be called something like simulateSingleTap. Not onSingleTap.
  • 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.

@frankkienl What do you think?

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.

2 participants