Repository navigation
[google_maps_flutter] Add onPointOfInterestTap - #11872
Conversation
336fb13 to
e125948
Compare
There was a problem hiding this comment.
Code Review
This pull request adds support for tapping points of interest (POIs) on the map across Android, iOS, and Web platforms in the google_maps_flutter plugin. It introduces the PointOfInterestId type and PointOfInterestTapEvent in the platform interface, exposes the onPointOfInterestTap callback on the GoogleMap widget, and implements the platform-specific event handling and Pigeon messaging. The review feedback suggests adding a defensive null check for the PointOfInterest parameter in the Android onPoiClick handler to prevent a potential NullPointerException.
| public void onPoiClick(PointOfInterest pointOfInterest) { | ||
| if (pointOfInterest.placeId != null) { | ||
| flutterApi.onPointOfInterestTap( | ||
| pointOfInterest.placeId, (Result<Unit> result) -> Unit.INSTANCE); | ||
| } | ||
| } |
There was a problem hiding this comment.
Defensively checking pointOfInterest for null before accessing its properties is a good practice to prevent potential NullPointerExceptions, especially since the parameter is not annotated with @NonNull.
| public void onPoiClick(PointOfInterest pointOfInterest) { | |
| if (pointOfInterest.placeId != null) { | |
| flutterApi.onPointOfInterestTap( | |
| pointOfInterest.placeId, (Result<Unit> result) -> Unit.INSTANCE); | |
| } | |
| } | |
| public void onPoiClick(PointOfInterest pointOfInterest) { | |
| if (pointOfInterest != null && pointOfInterest.placeId != null) { | |
| flutterApi.onPointOfInterestTap( | |
| pointOfInterest.placeId, (Result<Unit> result) -> Unit.INSTANCE); | |
| } | |
| } |
There was a problem hiding this comment.
The placeId null guard is the important part, and it’s already there. The Maps SDK only calls onPoiClick with a non-null PointOfInterest when a POI is tapped, and the other click handlers in this class don’t null-check their parameters either. We already have a test for the realistic case (placeId == null), which matches iOS. An extra pointOfInterest != null check would be harmless but unnecessary.
|
Hi @stuartmorgan-g when you're ready to review ping me and I'll do it right away. I'd rather fix the conflicts onces and for all instead of doing it every commit on that part of the code ;). |
|
No need to resolve changelog/pubspec comments until the whole review process is essentially complete (per the FAQ) since the conflicts in those files don't affect the ability to review the changes. |
| - (void)didLongPressAtPosition:(FGMPlatformLatLng *)position; | ||
|
|
||
| /// Called when a point of interest is tapped. | ||
| - (void)didTapPointOfInterestWithPlaceId:(NSString *)placeId; |
There was a problem hiding this comment.
...WithPlaceIdentifier:
There was a problem hiding this comment.
Renamed. I also moved the declaration up so it sits in the same spot as the matching method in MapsCallbackApi.
| void onCircleTap(String circleId); | ||
|
|
||
| /// Called when a point of interest is tapped. | ||
| @ObjCSelector('didTapPointOfInterestWithPlaceId:') |
There was a problem hiding this comment.
Done, it's didTapPointOfInterestWithPlaceIdentifier: now.
2247d7a to
1215581
Compare
Would rather have me update this PR? (I can still close the others if it is more convenient for you guys.) |
|
Please see https://github.com/flutter/flutter/blob/master/docs/ecosystem/contributing/README.md#changing-federated-plugins. This PR contains both implementation packages and the app-facing package, so cannot land this way. The flow we recommend for a change like this is the one described there, which involves three total PRs. |
Got it, I closed the 2 draft PRs and used the following for the implementations: #12880 |
…ns (#12880) ## Summary - Platform implementations of `onPointOfInterestTap` for **Android**, **iOS** (`sdk9` / `sdk10` + shared source), and **web** - Depends on published `google_maps_flutter_platform_interface` 2.17.0 from #12752 - Federated implementations PR (per [Changing federated plugins](https://github.com/flutter/flutter/blob/master/docs/ecosystem/contributing/README.md#changing-federated-plugins)); app-facing package will follow in a separate PR **Notes** - Frozen `google_maps_flutter_ios` is intentionally unchanged (no new features per that package’s README); iOS support is in `google_maps_flutter_ios_sdk9` / `google_maps_flutter_ios_sdk10` - iOS DI follows the Swift controller conventions (required `callbackHandler` on the designated initializer) Supersedes the separate drafts #12881 (web) and #12882 (iOS). Part of #11872 / flutter/flutter#60695. ## Pre-Review Checklist
|
@tenninebt could you resolved the conflict and rebase? thanks |
Expose the POI tap callback on GoogleMap now that platform implementations from flutter#12880 are published.
954c841 to
f71e8e8
Compare
| /// Called when a point of interest on the map is tapped. | ||
| /// | ||
| /// Supported on Android and web, and on iOS when using | ||
| /// `google_maps_flutter_ios_sdk9` or `google_maps_flutter_ios_sdk10`. |
There was a problem hiding this comment.
This comment does not belong in this package; it will become incorrect when, inevitably, new _sdk* variants are introduced that also support this feature.
It should simply say that it may not be supported on all implementations. A note about google_maps_flutter_ios not supporting a feature belongs in the google_maps_flutter_ios README, not here.
| /// | ||
| /// Supported on Android and web, and on iOS when using | ||
| /// `google_maps_flutter_ios_sdk9` or `google_maps_flutter_ios_sdk10`. | ||
| /// The default `google_maps_flutter_ios` package does not receive new |
There was a problem hiding this comment.
When the endorsed implementation is changed, there is essentially no chance that the person making that change would remember the existence of this comment, so this also does not belong here.
| * Adds support for tapping points of interest on the map. | ||
| On iOS, this requires `google_maps_flutter_ios_sdk9` or | ||
| `google_maps_flutter_ios_sdk10` (the default `google_maps_flutter_ios` | ||
| package does not receive new features). |
There was a problem hiding this comment.
(This is fine, since CHANGELOG entries are not expected to be evergreen.)
Keep app-facing dartdoc generic, and document that the frozen iOS implementation does not support the callback in its own README.
| @@ -1,5 +1,7 @@ | |||
| ## NEXT | |||
There was a problem hiding this comment.
README changes need versioning, as noted in the contributing docs.
| For example, `GoogleMap.onPointOfInterestTap` is not supported here; use | ||
| [`google_maps_flutter_ios_sdk9`](https://pub.dev/packages/google_maps_flutter_ios_sdk9) | ||
| or | ||
| [`google_maps_flutter_ios_sdk10`](https://pub.dev/packages/google_maps_flutter_ios_sdk10). |
There was a problem hiding this comment.
The previous sentence already linked down to the section of this README that has the same links; we don't want two copies of the same content in one README.
README changes must be published. Drop the duplicate SDK links and release the note as 2.18.7, folding the existing NEXT entry in.
Roll Packages from a1ed23c to da40eaf (33 revisions) flutter/packages@a1ed23c...da40eaf 2026-10-15 [email protected] [pigeon]add support for sealed classes in native interop (flutter/packages#13175) 2026-10-15 [email protected] [pigeon] Replace full JNI bindings with stubs for API classes that aren't needed (flutter/packages#13173) 2026-10-14 [email protected] [material_ui] Fix self-comparison and assert bugs in semantics_tester (flutter/packages#12722) 2026-10-14 [email protected] [camera_avfoundation] Replace the deprecated 'videoOrientation' with 'videoRotationAngle' on iOS 17+ (flutter/packages#13247) 2026-10-14 [email protected] [material_ui] Fix GridTile crash when the incoming constraints are unbounded (flutter/packages#12586) 2026-10-14 [email protected] [in_app_purchase_storekit] Remove unused and deprecated "SKDownload" hosted content (flutter/packages#13222) 2026-10-14 [email protected] [material_ui] Add supportingTextPadding to InputDecorator (flutter/packages#12769) 2026-10-14 [email protected] [vector_graphics_codec] Add support for encoding and decoding paint blur commands (flutter/packages#13148) 2026-10-14 [email protected] [go_router] Fix redirect zone async error handling (flutter/packages#12836) 2026-10-14 [email protected] Sync release-go_router-18.1.0 to main (flutter/packages#13260) 2026-10-14 [email protected] Sync release-cupertino_ui-1.1.3 to main (flutter/packages#13258) 2026-10-14 [email protected] [material_ui] add enableInlinePrediction parameter to TextFormField (flutter/packages#12793) 2026-10-14 [email protected] Sync release-material_ui-1.7.0 to main (flutter/packages#13259) 2026-10-14 [email protected] [material_ui] Add the Material shapes. (flutter/packages#12771) 2026-10-14 [email protected] [material_ui] Add tertiary tap callbacks to InkResponse and InkWell (flutter/packages#12676) 2026-10-14 [email protected] Roll Flutter from 4249ccb to 426be96 (44 revisions) (flutter/packages#13256) 2026-10-14 [email protected] [material_ui] fixes expanded state in the a11y_assessment home page dropdown (flutter/packages#13219) 2026-10-14 [email protected] [VPAT] Fix semantics in a11y_assessments drawer, radio, and rail demos (flutter/packages#13159) 2026-10-14 [email protected] [google_maps_flutter] Add onPointOfInterestTap (flutter/packages#11872) 2026-10-14 [email protected] [go_router] Fix shell route key collisions and dynamic branch updates (flutter/packages#12549) 2026-10-14 [email protected] Manual roll Flutter from d03768e to 4249ccb (158 revisions) (flutter/packages#13220) 2026-10-14 [email protected] [image_picker_ios] Require Flutter 3.47 / iOS 15 and drop deprecated kUTType code paths (flutter/packages#13224) 2026-10-13 [email protected] [in_app_purchase_storekit] Fixes Storekit 2 flaky tests (flutter/packages#13213) 2026-10-13 [email protected] [local_auth] Convert Android to new Pigeon suspend support (flutter/packages#12894) 2026-10-13 [email protected] [material_ui] Add enabled parameter to DropdownButtonFormField (flutter/packages#12610) 2026-10-13 [email protected] [go_router] Link ShellRoute docs to StatefulShellRoute (flutter/packages#13120) 2026-10-13 [email protected] FadeForwards page transition performance (flutter/packages#13161) 2026-10-13 [email protected] [in_app_purchase_android] Guard async billing callbacks against duplicate listener responses (flutter/packages#12640) 2026-10-13 [email protected] [google_sign_in] Remove the Objective-C exception catcher (flutter/packages#13156) 2026-10-13 [email protected] [camera_avfoundation] Remove availability checks below the package minimum iOS 13 (flutter/packages#13214) 2026-10-13 [email protected] [google_maps_flutter_ios] Remove iOS 14 availability check from example UI tests (flutter/packages#13218) 2026-10-13 [email protected] [google_sign_in] Convert implementation to Kotlin (flutter/packages#13005) 2026-10-13 [email protected] [image_picker_ios] PR 2/5 Add a PHImageManager image-data seam and migrate plugin native tests to Swift Testing (flutter/packages#13079) If this roll has caused a breakage, revert this CL and set the roller to dry run mode using the controls here: https://autoroll.skia.org/r/flutter-packages-flutter-autoroll Please CC [email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: ...
Roll Packages from a1ed23c to da40eaf (33 revisions) flutter/packages@a1ed23c...da40eaf 2026-10-15 [email protected] [pigeon]add support for sealed classes in native interop (flutter/packages#13175) 2026-10-15 [email protected] [pigeon] Replace full JNI bindings with stubs for API classes that aren't needed (flutter/packages#13173) 2026-10-14 [email protected] [material_ui] Fix self-comparison and assert bugs in semantics_tester (flutter/packages#12722) 2026-10-14 [email protected] [camera_avfoundation] Replace the deprecated 'videoOrientation' with 'videoRotationAngle' on iOS 17+ (flutter/packages#13247) 2026-10-14 [email protected] [material_ui] Fix GridTile crash when the incoming constraints are unbounded (flutter/packages#12586) 2026-10-14 [email protected] [in_app_purchase_storekit] Remove unused and deprecated "SKDownload" hosted content (flutter/packages#13222) 2026-10-14 [email protected] [material_ui] Add supportingTextPadding to InputDecorator (flutter/packages#12769) 2026-10-14 [email protected] [vector_graphics_codec] Add support for encoding and decoding paint blur commands (flutter/packages#13148) 2026-10-14 [email protected] [go_router] Fix redirect zone async error handling (flutter/packages#12836) 2026-10-14 [email protected] Sync release-go_router-18.1.0 to main (flutter/packages#13260) 2026-10-14 [email protected] Sync release-cupertino_ui-1.1.3 to main (flutter/packages#13258) 2026-10-14 [email protected] [material_ui] add enableInlinePrediction parameter to TextFormField (flutter/packages#12793) 2026-10-14 [email protected] Sync release-material_ui-1.7.0 to main (flutter/packages#13259) 2026-10-14 [email protected] [material_ui] Add the Material shapes. (flutter/packages#12771) 2026-10-14 [email protected] [material_ui] Add tertiary tap callbacks to InkResponse and InkWell (flutter/packages#12676) 2026-10-14 [email protected] Roll Flutter from 4249ccb to 426be96 (44 revisions) (flutter/packages#13256) 2026-10-14 [email protected] [material_ui] fixes expanded state in the a11y_assessment home page dropdown (flutter/packages#13219) 2026-10-14 [email protected] [VPAT] Fix semantics in a11y_assessments drawer, radio, and rail demos (flutter/packages#13159) 2026-10-14 [email protected] [google_maps_flutter] Add onPointOfInterestTap (flutter/packages#11872) 2026-10-14 [email protected] [go_router] Fix shell route key collisions and dynamic branch updates (flutter/packages#12549) 2026-10-14 [email protected] Manual roll Flutter from d03768e to 4249ccb (158 revisions) (flutter/packages#13220) 2026-10-14 [email protected] [image_picker_ios] Require Flutter 3.47 / iOS 15 and drop deprecated kUTType code paths (flutter/packages#13224) 2026-10-13 [email protected] [in_app_purchase_storekit] Fixes Storekit 2 flaky tests (flutter/packages#13213) 2026-10-13 [email protected] [local_auth] Convert Android to new Pigeon suspend support (flutter/packages#12894) 2026-10-13 [email protected] [material_ui] Add enabled parameter to DropdownButtonFormField (flutter/packages#12610) 2026-10-13 [email protected] [go_router] Link ShellRoute docs to StatefulShellRoute (flutter/packages#13120) 2026-10-13 [email protected] FadeForwards page transition performance (flutter/packages#13161) 2026-10-13 [email protected] [in_app_purchase_android] Guard async billing callbacks against duplicate listener responses (flutter/packages#12640) 2026-10-13 [email protected] [google_sign_in] Remove the Objective-C exception catcher (flutter/packages#13156) 2026-10-13 [email protected] [camera_avfoundation] Remove availability checks below the package minimum iOS 13 (flutter/packages#13214) 2026-10-13 [email protected] [google_maps_flutter_ios] Remove iOS 14 availability check from example UI tests (flutter/packages#13218) 2026-10-13 [email protected] [google_sign_in] Convert implementation to Kotlin (flutter/packages#13005) 2026-10-13 [email protected] [image_picker_ios] PR 2/5 Add a PHImageManager image-data seam and migrate plugin native tests to Swift Testing (flutter/packages#13079) If this roll has caused a breakage, revert this CL and set the roller to dry run mode using the controls here: https://autoroll.skia.org/r/flutter-packages-flutter-autoroll Please CC [email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: ...
Adds
GoogleMap.onPointOfInterestTap, a callback that fires when the user taps a built-in map point of interest. The callback receives aPointOfInterestIdcontaining the place ID only, matching maintainer feedback on #4052 and #10963.The change is wired through the federated plugin stack: platform_interface (type + event stream), Android (
OnPoiClickListener), iOS including sdk9/sdk10/shared (didTapPOIWithPlaceID), web (IconMouseEvent.placeIdon map click), and the app-facinggoogle_maps_flutterpackage. Tests cover Dart unit tests, Android Robolectric, iOS native, and web integration tests in the web example.Fixes flutter/flutter#60695
Pre-Review Checklist
[shared_preferences]///).