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

[Android Auto] Add onClick example #1682

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Conversation

kmadsen
Copy link
Contributor

@kmadsen kmadsen commented Sep 21, 2022

Blocked

Status is that this example is blocked until kotlin and the compile version are upgraded. So this change is built on top of this one #1683. The other can be merged while this one could be merged after repo upgrades

Summary of changes

We would like to build things that use the new "onClick" function that is available in 1.3.0-beta.1.

Unfortunately, this feature is only available in beta and if we upgrade the mapbox extension library, we will also block our ability to create a stable release.

Fortunately, there is an easy solution that provides a mechanism for downstream developers to use our existing extension and adopt this new feature. The idea is to temporarily (or maybe not temporary) expose the CarMapSurfaceOwner as a public experimental class. And then a downstream developer can use MapboxCarMap.setupWithCustomCallback() to inject their own version of the SurfaceCallback. After 1.3.0 is rolled out to stable, we will be able to add the onClick to the core sdk and we may no longer need to surface this internal class.

This updates the android-auto-app to have the latest versions to showcase this ability.

support-onclick.mov

User impact (optional)

Pull request checklist:

  • Briefly describe the changes in this PR.
  • Include before/after visuals or gifs if this PR includes visual changes.
  • Write tests for all new functionality. If tests were not written, please explain why.
  • Optimize code for java consumption (@JvmOverloads, @file:JvmName, etc).
  • Add example if relevant.
  • Document any changes to public APIs.
  • Run make update-api to update generated api files, if there's public API changes, otherwise the verify-api-* CI steps might fail.
  • Update CHANGELOG.md or use the label 'skip changelog', otherwise check changelog CI step will fail.
  • If this PR is a v10.[version] release branch fix / enhancement, merge it to main firstly and then port to v10.[version] release branch.

Fixes: < Link to related issues that will be fixed by this pull request, if they exist >

PRs must be submitted under the terms of our Contributor License Agreement CLA.

@kmadsen kmadsen requested a review from a team as a code owner September 21, 2022 17:55
@kmadsen kmadsen assigned ypenglyn and pengdev and unassigned ypenglyn and pengdev Sep 21, 2022
@kmadsen
Copy link
Contributor Author

kmadsen commented Sep 21, 2022

In order to support 1.3.0-beta.1 in an example, this repository needs to be upgraded to a new version of compileSdkVersion=33, which appears to require a newer version of kotlin. Which requires a few upgrades unrelated to this pull request

For example

* What went wrong:
Execution failed for task ':sdk-base:apiBuild'.
> Incompatible version of Kotlin metadata.
  Maximal supported Kotlin metadata version: 1.5.1,
  com/mapbox/common/MapboxCommonLogger Kotlin metadata version: 1.7.1.
  As a workaround, it is possible to manually update 'kotlinx-metadata-jvm' version in your project.
> Incompatible version of Kotlin metadata.
  Maximal supported Kotlin metadata version: 1.5.1,
  com/mapbox/maps/extension/androidauto/CarMapSurfaceOwner Kotlin metadata version: 1.7.1.
  As a workaround, it is possible to manually update 'kotlinx-metadata-jvm' version in your project.

An option, is to remove the upgrades from the android-auto-app and allow downstread to handle the upgrades. Or we can upgrade this repository to the new compileSdkVersion to unblock this change.

cc: @tobrun

@kmadsen kmadsen force-pushed the km-add-support-for-onclick branch 3 times, most recently from 212034b to 6d5fe47 Compare September 21, 2022 18:22
@kmadsen kmadsen changed the base branch from main to km-allow-callback-surface-override September 21, 2022 19:17
@kmadsen kmadsen changed the title [Android Auto] Add support for onClick [Android Auto] Add onClick example Sep 21, 2022
@kmadsen kmadsen marked this pull request as draft September 21, 2022 19:21
@kmadsen kmadsen force-pushed the km-allow-callback-surface-override branch from 7ea29d8 to 334ef1d Compare September 21, 2022 19:34
@tobrun
Copy link
Member

tobrun commented Sep 22, 2022

Thanks for flagging @kmadsen, I'm having the team look more into above requirements

@kmadsen kmadsen force-pushed the km-allow-callback-surface-override branch from ed5686e to 3613a29 Compare September 22, 2022 17:05
@kmadsen kmadsen force-pushed the km-allow-callback-surface-override branch from 3613a29 to b12ba1c Compare September 22, 2022 17:18
@kmadsen kmadsen force-pushed the km-allow-callback-surface-override branch from 712ec5f to 4f1eb41 Compare September 22, 2022 17:51
Base automatically changed from km-allow-callback-surface-override to main September 23, 2022 16:02
Comment on lines +50 to +65
val carContext = mapboxCarMap.carContext
if (customCallbackEnabled) {
val surfaceCallback = mapboxCarMap.prepareSurfaceCallback(
carContext, initializer.onCreate(carContext)
)
carContext.getCarService(AppManager::class.java)
.setSurfaceCallback(object : SurfaceCallbackInterceptor(surfaceCallback) {
override fun onClick(x: Float, y: Float) {
super.onClick(x, y)
onMapSurfaceClick(x, y)
}
})
} else {
mapboxCarMap.setup(carContext, initializer.onCreate(carContext))
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can toggle between these at runtime and everything works.

All observers that are attached, will be detached and then attached with a new map surface

@kmadsen kmadsen force-pushed the km-add-support-for-onclick branch 3 times, most recently from 2f0dc6f to 9903822 Compare October 7, 2022 16:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants