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

feat(google-maps): Separate mapId for Google Maps Cloud IDs #1750

Merged
merged 11 commits into from
Oct 23, 2023

Conversation

theproducer
Copy link
Contributor

@theproducer theproducer commented Aug 22, 2023

This PR adds support for a separate optional mapId that is used for cloud based map styling and features:
https://developers.google.com/maps/documentation/get-map-id

closes: #1708
closes ionic-team/capacitor#7011

@theproducer theproducer marked this pull request as ready for review August 22, 2023 17:55
Copy link
Member

@jcesarmobile jcesarmobile left a comment

Choose a reason for hiding this comment

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

It looks good, but should we use different config options for Android and iOS?
the mapId is unique per platform, so having a single mapId option will mean users will have to detect the platform and pass the id for the specific platform, a single mapId can't be used, it will error if you try to use a javascript map id in Android (on iOS it surprisingly works, but it shouldn't according to the docs)

@giralte-ionic
Copy link
Contributor

I agree with Julio that the platform should be part of the params passed here to avoid the problem. I also don't believe we should assume that iOS will always accept a javascript platform id.

Comment on lines 97 to 98
MapsInitializer.Renderer.LATEST -> Log.d("Capacitor Google Maps", "Latest Google Maps renderer enabled")
MapsInitializer.Renderer.LEGACY -> Log.d("Capacitor Google Maps", "Legacy Google Maps renderer enabled - Cloud based map styling and advanced drawing not available")
Copy link
Member

Choose a reason for hiding this comment

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

Use Logger.debug instead of android Log class so it can be globally disabled

@theproducer theproducer merged commit 88a633c into main Oct 23, 2023
10 of 11 checks passed
@theproducer theproducer deleted the google-maps/map-id branch October 23, 2023 16:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
5 participants