[google_maps_flutter] Add Advanced Markers support

[aednlaxer]

trafficstars

This PR adds Advanced Markers support to google_maps_flutter as discussed in #155526. The document from the issue offers several options to implement Advanced Markers support, this PR uses option 1 (Advanced Marker Dart class is a subclass Marker class).

Summary of changes:

• Add map capability check
• Add AdvancedMarker class
• Add MarkerCollisionBehavior enum to control Advanced Marker's behavior when it collides with another marker
• Add PinConfig bitmap descriptor for customizing Advanced Marker's pin and icon
• Add markerType parameter to indicate that Advanced Markers should be used
• Rename cloudMapId to mapId

Notes:

• native implementations need to know what kind of marker should be used, this is needed to use correct marker options, cluster manager and marker controller. Indicating marker type is done by using a markerType option when creating a GoogleMap (could be marker or advancedMarker). Default option is marker
• cloudMapId is deprecated in favor of mapId. New name follows SDKs, documentation and Cloud Console naming.
• web implementation uses generics because gmaps.Marker and gmaps.AdvancedMarkerElement are not related to each other and should be handled differently
• legacy markers are deprecated but still supported in Maps Javascript API. google_maps_flutter still uses them by default in this PR because of backward-compatibility. #130472 is related, package users will be able to use Advanced Markers to fix the deprecation warning
• Advanced Markers example on iOS doesn't show advanced markers, seems to be a known issue
• Using Flutter widget as Advanced Marker icon (so called View Marker) is not part of this PR, this was discussed in the document and later removed as we agreed that it should be a separate issue

Resolves #155526

Pre-launch Checklist

• [x] I read the Contributor Guide and followed the process outlined there for submitting PRs.
• [x] I read the Tree Hygiene page, which explains my responsibilities.
• [x] I read and followed the relevant style guides and ran the auto-formatter. (Unlike the flutter/flutter repo, the flutter/packages repo does use dart format.)
• [x] I signed the CLA.
• [x] The title of the PR starts with the name of the package surrounded by square brackets, e.g. [shared_preferences]
• [x] I linked to at least one issue that this PR fixes in the description above.
• [ ] I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.
• [ ] I updated CHANGELOG.md to add a description of the change, following repository CHANGELOG style, or this PR is exempt from CHANGELOG changes.
• [x] I updated/added relevant documentation (doc comments with ///).
• [x] I added new tests to check the change I am making, or this PR is test-exempt.
• [x] All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

[jmagman]

@aednlaxer it looks like you've pulled in far more changes than your original PR intended, can you fix that up and rebase on main, or whatever you would need to do?

[aednlaxer]

@aednlaxer it looks like you've pulled in far more changes than your original PR intended, can you fix that up and rebase on main, or whatever you would need to do?

Sorry for that, it should now contain new changes only

[stuartmorgan-g]

Looks like some platform reviewers were dropped at some point on accident.

[rajubhusani]

I see Advanced merker icons are not rendering properly on the map. Unhandled Exception: PlatformException(java.lang.IllegalArgumentException: Unable to interpret pin config as a valid image., IllegalArgumentException, Cause: java.lang.NullPointerException: null reference, Stacktrace: java.lang.IllegalArgumentException: Unable to interpret pin config as a valid image.

[aednlaxer]

@rajubhusani

I see Advanced merker icons are not rendering properly on the map. Unhandled Exception: PlatformException(java.lang.IllegalArgumentException: Unable to interpret pin config as a valid image., IllegalArgumentException, Cause: java.lang.NullPointerException: null reference, Stacktrace: java.lang.IllegalArgumentException: Unable to interpret pin config as a valid image.

Hi, thanks for checking it out. Could please provide more details?

• Did you use one of the new examples from this PR? Which one?
• Did you add your API key and map ID before running the app?
• Did you build your own sample to test advanced markers? Can you share it or provide code that could help reproducing the issue?
• What kind of image do you use? Would it be possible to test if this image works with legacy (non-advanced) markers?
• Did you stop the Android app before running the code? Running app should be stopped and started again after checking out this PR's code. The reason is the platform-specific code that won't work using Flutter's hot-restart and should only work if native (Android) app is restarted.

[rajubhusani]

@rajubhusani

I see Advanced merker icons are not rendering properly on the map. Unhandled Exception: PlatformException(java.lang.IllegalArgumentException: Unable to interpret pin config as a valid image., IllegalArgumentException, Cause: java.lang.NullPointerException: null reference, Stacktrace: java.lang.IllegalArgumentException: Unable to interpret pin config as a valid image.

Hi, thanks for checking it out. Could please provide more details?

• Did you use one of the new examples from this PR? Which one?
• Did you add your API key and map ID before running the app?
• Did you build your own sample to test advanced markers? Can you share it or provide code that could help reproducing the issue?
• What kind of image do you use? Would it be possible to test if this image works with legacy (non-advanced) markers?
• Did you stop the Android app before running the code? Running app should be stopped and started again after checking out this PR's code. The reason is the platform-specific code that won't work using Flutter's hot-restart and should only work if native (Android) app is restarted.

@aednlaxer, Please find the responses, I just cloned the code from your branch "CodemateLtd:feature/google-maps-advanced-markers-support". And directly ran the Android app from the google_maps_flutter_android/example project. I have not made any changes to that. I have used my own API key setup. All the options are working on the example except the Custom Marker & Cluster.

[aednlaxer]

@aednlaxer, Please find the responses, I just cloned the code from your branch "CodemateLtd:feature/google-maps-advanced-markers-support". And directly ran the Android app from the google_maps_flutter_android/example project. I have not made any changes to that. I have used my own API key setup. All the options are working on the example except the Custom Marker & Cluster.

@rajubhusani I'm still not able to reproduce it :( Can you debug it a little to find what the actual exception is? It seems to be happening in Convert.java file. If you remove the try-catch block in getBitmapFromPinConfig() function and run the example app again, the actual exception should be thrown and shown in the logs. Please note that there are quite many changes since your last comment

[ditman]

PS, I think the web mocks need updating:

Launching integration_test/google_maps_controller_test.dart on Web Server in debug mode...
org-dartlang-app:/google_maps_controller_test.mocks.dart:282:16: Error: Type argument 'Object?' doesn't conform to
the bound 'JSObject' of the type variable 'T' on 'MarkersController'.
 - 'Object' is from 'dart:core'.
Try changing type arguments so that they conform to the bounds.
    implements _i2.MarkersController<Object?, Object> {
               ^
../lib/src/markers.dart:13:34: Context: This is the type variable whose bound isn't conformed to.
abstract class MarkersController<T extends JSObject, O>
                                 ^

And:

Launching integration_test/google_maps_plugin_test.dart on Web Server in debug mode...
org-dartlang-app:/google_maps_plugin_test.mocks.dart:113:21: Error: The return type of the method
'MockGoogleMapController.styles' is 'List<dynamic>', which does not match the return type, 'List<MapTypeStyle>',
of the overridden method, 'GoogleMapController.styles'.
 - 'List' is from 'dart:core'.
Change to a subtype of 'List<MapTypeStyle>'.
  List<dynamic> get styles =>
                    ^
../lib/src/google_maps_controller.dart:415:32: Context: This is the overridden method ('styles').
  List<gmaps.MapTypeStyle> get styles => _lastStyles;
                               ^
org-dartlang-app:/google_maps_plugin_test.mocks.dart:125:50: Error: The parameter 'markers' of the method
'MockGoogleMapController.debugSetOverrides' has type 'MarkersController<JSObject, Object>?', which does not match
the corresponding type, 'MarkersController<Object?, Object>?', in the overridden method,
'GoogleMapController.debugSetOverrides'.
 - 'MarkersController' is from 'package:google_maps_flutter_web/google_maps_flutter_web.dart'
 ('../lib/google_maps_flutter_web.dart').
 - 'Object' is from 'dart:core'.
Change to a supertype of 'MarkersController<Object?, Object>?', or, for a covariant parameter, a subtype.
    _i4.MarkersController<_i5.JSObject, Object>? markers,
                                                 ^
../lib/src/google_maps_controller.dart:191:8: Context: This is the overridden method ('debugSetOverrides').
  void debugSetOverrides({
       ^

If you're in linux, you can run the web integration tests with something like:

dart run /work/flutter/packages/script/tool/bin/flutter_plugin_tools.dart drive-examples --current-package --web --run-chromedriver

(Needs to have chrome and chromedriver in $PATH)

Or more directly, start chromedriver in port 4444, and run command: flutter drive -d web-server --web-port=7357 --browser-name=chrome --driver test_driver/integration_test.dart --target integration_test/TEST FILE NAME.dart inside the example directory.

[ditman]

@aednlaxer I tried to push a fix for the mocks, but:

$ git push
ERROR: Permission to CodemateLtd/packages.git denied to ditman.
fatal: Could not read from remote repository.

I've sent this PR instead:

• https://github.com/CodemateLtd/packages/pull/18

(The PR should heal the Linux_web web_platform_tests_shard_3 checks, please check the other failing tests!)

[vashworth]

Advanced Markers example on iOS doesn't show advanced markers, seems to be a known issue

You can use Advanced Markers in example map. You set mapId to DEMO_MAP_ID

Update: It appears that you can't use GMSPinImage in the example. So the default example in PlaceAdvancedMarkerPage doesn't show because it uses a pinConfig

[aednlaxer]

@reidbaker @vashworth could you please give this another round of review? All comments should be resolved now

[aednlaxer]

Linux_web web_platform_tests_wasm_shard_3 master is failing because innerHtml returns a different object type on Flutter main:

• Flutter stable: String?
• Flutter main: JSAny

@ditman is this something that should be type-checked in tests or is there a better way to deal with it?

[ditman]

@aednlaxer this has to do with (possible) differences in implementation of those types in JS vs WASM (the failing tests are WASM ones, it seems).

Have you tried compiling your demo app with --wasm and seeing if it actually fails, or is it a problem with the tests? If we're using dart:html in tests that's a problem-ish, we should move away from that and use package:web and dart:js_interop instead.

I can't take a look right now, but will try to figure out a patch later this week.

(PS: I suspect this needs a strategic .toDart/.toJS in the failing assertion, when the type is JSAny? probably/maybe :))

[bridgein]

Is there any roadmap for the feature?

[stuartmorgan-g]

@aednlaxer This has high-level LGs for all components now, so please feel free to split out the platform interface PR. I'll re-review that piece here in the meantime, but any comments can be addressed in the sub-PR.

[Piinks]

Greetings from stale PR triage! 👋 Is this change still on your radar? It looks like it has had quite the journey and a fair number of stamps, is it ready? :)

[stuartmorgan-g]

This is waiting on the sub-PR at https://github.com/flutter/packages/pull/9773

[stuartmorgan-g]

@aednlaxer It looks like only web was split out; is there a reason the Android and iOS implementations weren't included in that PR (or a parallel split-out PR)?

[jokerttu]

@aednlaxer It looks like only web was split out; is there a reason the Android and iOS implementations weren't included in that PR (or a parallel split-out PR)?

@stuartmorgan-g Discussed with @aednlaxer and I'll take over @aednlaxer 's work, and continue splitting the next PR's. Already rebased to the main (not pushed yet), and currently fixing issues I found with iOS implementation before creating the PR's. Should be ready soon.

[jokerttu]

Created split PR for Android. I will create split-PR for iOS implementation later as draft as this known issue seems to be still visible. I have a separate discussion open about this issue, and hopefully, I’ll get a resolution soon.

[stuartmorgan-g]

@jokerttu Any update here? The referenced iOS issue appears to be have been closed for quite some time.

This intermediate state is (due to some unfortunate re-exports of platform-interface-package APIs in the app-facing package) causing some user confusion, so I would like to make sure we are on a clear path to getting the rest of this landed.

[jokerttu]

@jokerttu Any update here? The referenced iOS issue appears to be have been closed for quite some time.

This intermediate state is (due to some unfortunate re-exports of platform-interface-package APIs in the app-facing package) causing some user confusion, so I would like to make sure we are on a clear path to getting the rest of this landed.

@stuartmorgan-g

The Android implementation is already split here: https://github.com/flutter/packages/pull/10381

iOS:

A PR for iOS has not been created yet due to an issue with the GMSPinImage not rendering on the map. Does @caio1985 have an update on whether this issue is under investigation?

Would it be helpful if I created a draft PR for iOS now to make the issue easier to track?

I also created a native iOS demo app demonstrating this issue: google-maps-advanced-markers-test-app

Do you think it is worth moving forward with the app-facing package for Android and Web now, or should we wait for the iOS fix? I worry that releasing partial support (where iOS throws UnimplementedError) will confuse developers even when well documented.

[stuartmorgan-g]

A PR for iOS has not been created yet due to an issue with the GMSPinImage not rendering on the map. Does @caio1985 have an update on whether this issue is under investigation?

I don't have any information on this, but I'm also confused because as I mentioned above, the issue you linked to and that you seem to be referring to here was closed as obsolete in April. I would not expect any active investigation to be happening on an issue that was closed 7 months ago.

Would it be helpful if I created a draft PR for iOS now to make the issue easier to track?

A draft PR would be useful, but also a live upstream blocking issue would be helpful.

Do you think it is worth moving forward with the app-facing package for Android and Web now, or should we wait for the iOS fix?

For now we should wait for iOS, but I also want to make sure there is some clear path forward for iOS and that it's being actively investigated, so we can make a more informed decision on that.

[raphael-bmec-co]

Do you think it is worth moving forward with the app-facing package for Android and Web now, or should we wait for the iOS fix? I worry that releasing partial support (where iOS throws UnimplementedError) will confuse developers even when well documented.

Please excuse my jumping in here - I have no idea what internal policy dictates. That said, the current production implementation has several unimplemented platform interfaces that fail silently. So while partial support is not ideal, my personal view is that it is better than the current status. Looking forward to having this landed. Thanks all.

[cedvdb]

@jokerttu I was also confused, I believe you linked the wrong issue above.