mapbox / mapbox-maps-android

Interactive, thoroughly customizable maps in native Android powered by vector tiles and OpenGL.
https://www.mapbox.com/mobile-maps-sdk
Other
468 stars 131 forks source link

[Android Auto] Support for onTap event in MapboxCarMapGestureHandler #1490

Closed narko closed 2 years ago

narko commented 2 years ago

This feature is about allowing users to tap on the map similarly to how mapSurface.getMapboxMap().addOnMapClickListener works.

This would allow users to tap on a specific POI and do things like opening a new Screen.

Not sure if this is supported somehow. I have tried to build something similar by using a MapboxObserver and adding the following on the onAttach method, but it didn't work for me:

override fun onAttached(mapboxCarMapSurface: MapboxCarMapSurface) {
        this.mapboxCarMapSurface = mapboxCarMapSurface
        mapboxCarMapSurface.mapSurface.getMapboxMap().addOnMapClickListener(onMapClickListener)
}

androidx.car.app.SurfaceCallback supports onClick events already:

    @RequiresCarApi(5)
    @ExperimentalCarApi
    default void onClick(float x, float y) {
    }

That means that it would be quite straightforward to support that functionality in MapboxCarMapGestureHandler (unless I missed something down the line).

narko commented 2 years ago

I just realized I should have posted this rather here: https://github.com/mapbox/mapbox-maps-android

kmadsen commented 2 years ago

Excited that support for this is moving. But it's not quite ready for production. Currently, it is part of 1.2.0 which is a release candidate. The api is experimental, and it requires a special configuration in order to get api level 5 and there are only 4 levels supported by the desktop-head-unit. For the reason, considering this issue blocked.

Screen Shot 2022-07-11 at 9 53 19 AM
kmadsen commented 2 years ago

A related issue that could be looked into, is that the SDK does not support upgrading to the experimental api. Essentially, the CarMapSurfaceCallback needs to add the onClick in order to forward the new method to the MapboxCarMapGestureHandler

This issue was known in development, because the car library does not allow for multiple subscriptions to the surface callback https://issuetracker.google.com/u/0/issues/200592099. It also looks like that is something google is addressing so we may have better support for this in the future.

kmadsen commented 2 years ago

That means that it would be quite straightforward to support that functionality in MapboxCarMapGestureHandler (unless I missed something down the line).

Agree with this! Once the api is stable this should be a simple update.

I was considering a bigger refactor to decouple the sdk from experimental apis in the SurfaceCallback, but the solutions i tried would not help because the onClick method does not seem to work yet. Best to follow this issue instead of taking any action here https://issuetracker.google.com/issues/171715013

kmadsen commented 2 years ago

This would allow users to tap on a specific POI and do things like opening a new Screen.

@narko the navigation SDK includes a screen that allows you to select POI from the screen. https://docs.mapbox.com/android/navigation/guides/androidauto/search/#placeslistonmapscreen-to-show-results-on-the-map

Take a look at PlacesListOnMapScreen

narko commented 2 years ago

This would allow users to tap on a specific POI and do things like opening a new Screen.

@narko the navigation SDK includes a screen that allows you to select POI from the screen. https://docs.mapbox.com/android/navigation/guides/androidauto/search/#placeslistonmapscreen-to-show-results-on-the-map

Take a look at PlacesListOnMapScreen

Thanks @kmadsen! As far as I understood that example is setting the onClickListener on the Row.Builder using the list of POIs instead: https://github.com/mapbox/mapbox-navigation-android/blob/e778f67bda1d2f7f2f5b3cf3e78b7b8075e57c7d/libnavui-androidauto/src/main/java/com/mapbox/androidauto/car/placeslistonmap/PlacesListItemMapper.kt#L80 Unfortunately in my use case I really need to tap directly on the map.

kmadsen commented 2 years ago

As far as I understood that example is setting the onClickListener on the Row.Builder using the list of POIs instead:

@narko Yeah that is correct. That has been the android auto solution that google provided as an alternative to tapping directly on the map. as you can see, basic map panning and zooming was added a year ago in 1.1

Unfortunately in my use case I really need to tap directly on the map.

As far as I can tell, this is not supported by android auto head units at the moment. I spoke with some of the CarPlay developers, and this is also a limitation on their end.

It does look like api support is coming, but you will need to wait for a stable api in order to use it (my guess is ~6 months but that is just my guess and maybe it is less or more). We will be adding support for it when it is available

This issue can track when we add support for it after google provides stable support for surface taps

narko commented 2 years ago

@kmadsen API 5 seems to be supported now, which includes the onClick in the SurfaceCallback. See https://developer.android.com/jetpack/androidx/releases/car-app#version_13_2

API Level 5: new onClick method in the SurfaceCallback interface to allow for tap on map interactivity ([Ia9777](https://android-review.googlesource.com/q/Ia9777690c21f2d211e7b8277130ae5aaffe075b4))

Features annotated with API level 5 are compatible with Android Auto 7.9 and above.

narko commented 2 years ago

@kmadsen I have created a WIP PR, where I am trying to add support for this using the androidx.car.app version 1.3.0-alpha01.

I know that AA is not on 7.9 (in Google Play), but I'd like to give this a try using some AA betas. Would you mind if I ask a question directly on the PR changes?

narko commented 2 years ago

@kmadsen added this feature in https://github.com/mapbox/mapbox-maps-android/releases/tag/extension-androidauto-v0.3.0 :tada: