[google_maps_flutter] Adds point of interest event handler for iOS and Android #4052

bndos · 2023-05-22T02:56:50Z

Issue #60695 mentions the lack of a point of interest (POI) click handler in the google_maps_flutter package. This pull request adds support for the point of interest (POI) event handler in the google_maps_flutter package. The POI event handler allows users to interact with points of interest on the map by detecting when a POI is clicked.

The changes in this pull request include:

Updated dependencies to the latest versions that include the POI event handler functionality:
- google_maps_flutter: 2.2.9
- google_maps_flutter_platform_interface: 2.2.8
- google_maps_flutter_android: 2.4.16
- google_maps_flutter_ios: 2.2.4
Implemented the POI event handler for Android and iOS:
- google_maps_flutter_platform_interface: Added the interface methods for the POI event handler to be implemented in platform-specific code.
- google_maps_flutter_android: Implemented the POI event handler for Android devices.
- google_maps_flutter_ios: Implemented the POI event handler for iOS devices.

Additionally, the google_maps_flutter package now includes an example in the example directory (map_click.dart) that demonstrates the usage of the POI event handler.

Fixes issue: flutter/flutter#60695

… handler interface This commit addresses issue #60695, which mentions the lack of an on point of interest event listener. The google_maps_flutter_platform_interface package is being released first to enable other Google Maps platform-specific packages to implement the onPoiClick feature. See: flutter/flutter#60695

…for point of interest event handler This commit adds the Android implementation for the point of interest event handler in the google_maps_flutter_platform_interface package. It addresses issue #60695, which raised the need for this functionality. By including the Android implementation, other Google Maps platform-specific packages can now utilize the onPoiClick feature on Android devices. Issue link: flutter/flutter#60695

…vent handler in ios This commit adds the necessary code to the `google_maps_flutter_ios` package to handle point of interest (POI) events. It includes the implementation for registering the POI click callback and handling the corresponding events on iOS. This functionality is in line with the recent addition to the `google_maps_flutter_platform_interface` package, allowing users to interact with points of interest on the map. The changes are currently untested and require further validation on macOS. Issue: #60695

…o example This commit fixes a compilation issue in the `google_maps_flutter_ios` package caused by my previous commit. The issue has been resolved, and the package now builds successfully on iOS. Additionally, iOS testing support has been added to the example app in google_maps_flutter to ensure proper functionality. Issue: #60695

This commit updates the versions of the `google_maps_flutter` packages. The new versions include the relevant code for point of interest clicked event handling for Android and iOS. The updated packages are as follows: - `google_maps_flutter`: 2.2.7 - `google_maps_flutter_platform_interface`: 2.2.7 - `google_maps_flutter_android`: 2.4.15 - `google_maps_flutter_ios`: 2.2.3 Issue: flutter/flutter#60695

This commit updates the dependencies of the `google_maps_flutter` packages to point to the new package versions mentioned in the previous commit. By updating the dependencies, the packages now utilize the latest versions that include the implementation of the point of interest event functionality. Updated dependencies: - `google_maps_flutter`: 2.2.7 - `google_maps_flutter_platform_interface`: 2.2.7 - `google_maps_flutter_android`: 2.4.15 - `google_maps_flutter_ios`: 2.2.3 Issue: flutter/flutter#60695

reidbaker · 2023-05-22T14:53:08Z

It looks like the federation safety checks are failing. https://github.com/flutter/packages/pull/4052/checks?check_run_id=13646626910

bndos · 2023-05-22T18:42:21Z

Looks like I need to make a separate PR for google_maps_flutter_platform_interface. Thank you for pointing that out.

…rest event interface Issue: flutter/flutter#60695

…t of Interest event interface Issue: flutter/flutter#60695

bndos · 2023-05-22T21:51:04Z

@reidbaker I have added the pull request for the only the google_maps_flutter_platform_interface (#4063).

stuartmorgan-g · 2023-05-23T12:06:03Z

Please see https://github.com/flutter/flutter/wiki/Contributing-to-Plugins-and-Packages#changing-federated-plugins. The sub-PRs should only be made once the overall PR has been reviewed.

You just need to revert the constraint version bumps for now so that version solving actually works and the CI can run.

stuartmorgan-g

Thanks for the contribution!

stuartmorgan-g · 2023-06-14T15:53:30Z

packages/google_maps_flutter/google_maps_flutter/example/lib/map_click.dart

  GoogleMapController? mapController;
  LatLng? _lastTap;
  LatLng? _lastLongPress;
+  PointOfInterest? _lastPoiTap;


The abbreviation hurts readability; we should call it _lastPointOfInterestTap.

stuartmorgan-g · 2023-06-14T15:53:50Z

packages/google_maps_flutter/google_maps_flutter/example/lib/map_click.dart

    if (mapController != null) {
      final String lastTap = 'Tap:\n${_lastTap ?? ""}\n';
      final String lastLongPress = 'Long press:\n${_lastLongPress ?? ""}';
+      final String lastPoiTap = 'Point of interest tap:\n${_lastPoiTap ?? ""}';


stuartmorgan-g · 2023-06-14T15:54:28Z

packages/google_maps_flutter/google_maps_flutter/example/pubspec.yaml

    # the parent directory to use the current plugin's version.
    path: ../
  google_maps_flutter_android: ^2.1.10
+  google_maps_flutter_ios: ^2.2.3


Why have you adde this? I don't see any use of it.

I have removed the dependency.

stuartmorgan-g · 2023-06-14T15:56:15Z

...maps_flutter_platform_interface/lib/src/platform_interface/google_maps_flutter_platform.dart

    throw UnimplementedError('onLongPress() has not been implemented.');
  }

+  /// A Map has been tapped on a POI [LatLng, String, String].


What does the array there represent? I'm not sure how to read this.

I have added a better comment:
/// A Map has been tapped on a POI with a position, place name and place id
/// [LatLng, String, String].

stuartmorgan-g · 2023-06-14T15:57:03Z

...maps_flutter_platform_interface/lib/src/platform_interface/google_maps_flutter_platform.dart

  }

+  /// A Map has been tapped on a POI [LatLng, String, String].
+  Stream<MapPoiClickEvent> onPoiClick({required int mapId}) {


This should also not be abbreviated. Also, why is it called "click" when everything else is "tap"? I would expect this to be called onPointOfInterestTap.

stuartmorgan-g · 2023-06-14T16:03:01Z

...gle_maps_flutter/google_maps_flutter_platform_interface/lib/src/types/point_of_interest.dart

+import 'types.dart';
+
+/// A pair of latitude and longitude coordinates, stored as degrees.
+class PointOfInterest {


This should be @immutable.

@ditman mentioned the need of being able to retrieve name afterwards for the web implementation. What do you suggest doing? #4052 (comment)

If it's not a Future I don't see how that would work. What would update it later, and how would any client know when that had happened?

If it's going to be updated asynchronously, it should be a Future; if not, then the object can be immutable anyway. @ditman Am I misunderstanding the discussion above, or did you just want to leave this null on web?

stuartmorgan-g · 2023-06-14T16:04:01Z

...e_maps_flutter/google_maps_flutter_platform_interface/test/types/point_of_interest_test.dart

+void main() {
+  TestWidgetsFlutterBinding.ensureInitialized();
+
+  group('$PointOfInterest', () {


No $; that pattern is slowly being removed everywhere as it interferes with IDE test UI.

I have removed the $.

stuartmorgan-g · 2023-06-14T16:08:41Z

...maps_flutter_platform_interface/lib/src/platform_interface/google_maps_flutter_platform.dart


+  /// A Map has been tapped on a POI [LatLng, String, String].
+  Stream<MapPoiClickEvent> onPoiClick({required int mapId}) {
+    throw UnimplementedError('onPoiClick() has not been implemented.');


You are adding unconditional calls to this new method, which will immediately crash anyone who is using a third-party implementation. If the app-facing package is going to use this unconditionally, then the default implementation needs to be to return a stream that never has any events.

I have changed it to return const Stream<MapPointOfInterestTapEvent>.empty();

stuartmorgan-g · 2023-06-14T16:09:28Z

packages/google_maps_flutter/google_maps_flutter/lib/src/google_map.dart

    this.onCameraIdle,
    this.onTap,
    this.onLongPress,
+    this.onPoiClick,


See naming notes in my other comments.

stuartmorgan-g · 2023-06-14T16:10:27Z

packages/google_maps_flutter/google_maps_flutter/CHANGELOG.md

@@ -1,3 +1,7 @@
+## 2.2.9


New features increment the minor version, not the bugfix version (applies throughout the PR).

I have fixed it.

…apEvent

…ent for POI tap

bndos · 2023-06-20T04:04:34Z

Hi @stuartmorgan thank you for the detailed review. I have address every point besides this one: #4052 (comment). Because I am not sure about the preferred implementation to take.

reidbaker · 2023-07-18T17:18:46Z

It looks like there are some pretty simple formatting changes that are required.

reidbaker

The android implementation code looks ok to me but I dont see android tests. Am I missing something?

stuartmorgan-g · 2023-07-18T19:40:57Z

This will definitely need integration test and/or unit tests that cover the implementations on all platforms. I'm not sure how I missed that in the initial review.

ditman · 2023-07-19T00:54:26Z

...gle_maps_flutter/google_maps_flutter_platform_interface/lib/src/types/point_of_interest.dart

@stuartmorgan About the nullable "name" on the web:

As opposed to iOS' didTapPOIWithPlaceID:name:location: and Android's onPoiClick's PointOfInterest object, retrieving the "name" of a POI from its placeId on the web seems to always have an additional cost. Pricing. On the web, the POI tap event only gets latLng and placeId for free. Tinker here.

If we make the plugin always return a POI with "name", we'd be forcing the web implementation to always perform an additional (metered) request to the Places API to fetch the "name" of the clicked POI (and throw away a bunch of additional information that could be retrieved by the same price). This is not super user-friendly :P

Rethinking my String? name suggestion, I agree this doesn't match what we do in the plugin very well. The issue I see now is that this PR is mixing the PointOfInterest (hydrated object, costs money, can have more info than "name") with the PointOfInterestId (what Maps gives us across all platforms).

Considering the plugin as a whole, I think this PR should propagate the tap events on POIs by their PointOfInterestId, rather than the hydrated version, just like it does onCircleTap(CircleId circleId).

I think the plugin should NOT auto-hydrate any POI Data, unless the programmer explicitly requests it to, via a separate method (something like: Future<Place> getPlaceData(PlaceId id);)

Hydrating a PointOfInterest later lets the user pick what data they want to retrieve (data is priced in 3 tiers, so we could have tiered factories to construct a POI from its placeId), and not drop any data to the floor. (Similarly we need to find a way to keep the free information that is offered by Android and iOS, so it can be used later!)

In addition to the "please do not force web to hydrate PointOfInterests", I'd suggest to rename the new data structures to Place and PlaceId. That's the name used in the Maps SDK; after all a POI is just a Place that has some special "prominence" that gets it automatically rendered on the map, and make the Place object support as much data as possible coming from the Places API, not just the "name". This should enable the plugin to evolve and maybe provide other functionality of the Places API.

See available data for a Place.

Apologies for not having written my reasoning more thoroughly before!

(On dart we may be able to know if a Stream has listeners or not, so we turn on/off the hydration as needed, but I think this has further implications than that)

stuartmorgan-g · 2023-08-10T18:49:55Z

@bndos Are you planning on adding tests, per the review comments above?

reidbaker · 2023-08-24T18:11:46Z

Thank you for your contribution. I'm going to close this PR for now since there are outstanding comments, just to get this off our PR review queue. Please don't hesitate to submit a new PR if you have the time to address the review comments. Thanks!

bndos and others added 6 commits May 20, 2023 03:34

bndos requested review from cyanglaz, ditman, reidbaker and stuartmorgan-g as code owners May 22, 2023 02:56

github-actions bot added p: google_maps_flutter platform-android platform-ios platform-web labels May 22, 2023

bndos mentioned this pull request May 22, 2023

[google_maps_flutter] Add callback for click on google poi's on the map flutter/flutter#60695

Open

bndos and others added 5 commits May 22, 2023 15:38

Merge branch 'main' into feature/point-of-interest-interface

f0d0e28

Merge branch 'main' into feature/point-of-interest-interface

f55ec72

[google_maps_flutter_platform_interface] Adds tests for Point of Inte…

2ac3190

…rest event interface Issue: flutter/flutter#60695

[google_maps_flutter_platform_interface] Adds version change for Poin…

7dc7bca

…t of Interest event interface Issue: flutter/flutter#60695

[google_maps_flutter_platform_interface] Adds version change for Poin…

12f6b54

…t of Interest event interface Issue: flutter/flutter#60695

bndos added 4 commits May 23, 2023 11:13

Merge branch 'main' into feature/point-of-interest

45aa702

Merge branch 'main' into feature/point-of-interest

b0c77ad

Removes unnecesary null check on point of interest

87e074f

[google_maps_flutter] Reverts constraint version bumps

06408d5

github-actions bot removed the platform-web label May 23, 2023

[google_maps_flutter] Update version for google_maps_platform_interface

4ac5907

bndos added 2 commits May 30, 2023 19:34

[google_maps_flutter] Fixes point of interest test

3219832

[google_maps_flutter] Makes PointOfInterest mutable

e0a5157

stuartmorgan-g requested changes Jun 14, 2023

View reviewed changes

bndos added 13 commits June 19, 2023 11:20

Merge branch 'main' into feature/point-of-interest

2730ae2

[google_maps_flutter] Changes Poi to point of interest in example

197b1e9

[google_maps_flutter] Removes ios dependency in example

01d6dd0

[google_maps_flutter] Adds clearer explanation on poi arguments

f9ec090

[google_maps_flutter] Fixes missing period on PointOfInterest

48017a9

[google_maps_flutter] Removes unnecessary toJson for POI

4d755df

[google_maps_flutter] Adds better description for the POI name

70fa2ee

[google_maps_flutter] Fixes comment for the location of the POI

ed46cda

[google_maps_flutter] Removes dollar sign from poi test

5fd9edd

[google_maps_flutter] Renames MapPoiClickEvent to MapPointOfInterestT…

e8ab53d

…apEvent

[google_maps_flutter] Renames onPoiClick to onPointOfInterestTap

eeae9eb

[google_maps_flutter] Removes unimplemented error and return empty ev…

eb6a351

…ent for POI tap

[google_maps_flutter] Updates minor version for feature POI

9236c6f

github-actions bot added the platform-web label Jun 19, 2023

bndos added 2 commits June 19, 2023 15:37

[google_maps_flutter] Removes version change for web

29fe8de

[google_maps_flutter] Removes version change for web

b41f299

github-actions bot removed the platform-web label Jun 19, 2023

bndos requested a review from stuartmorgan-g June 20, 2023 04:04

reidbaker requested changes Jul 18, 2023

View reviewed changes

ditman reviewed Jul 19, 2023

View reviewed changes

reidbaker closed this Aug 24, 2023

stuartmorgan-g mentioned this pull request Feb 4, 2026

[google_maps_flutter] Add onPoiTap callback to GoogleMap #10963

Open

11 tasks

[google_maps_flutter] Adds point of interest event handler for iOS and Android #4052

[google_maps_flutter] Adds point of interest event handler for iOS and Android #4052

Uh oh!

Conversation

bndos commented May 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

reidbaker commented May 22, 2023

Uh oh!

bndos commented May 22, 2023

Uh oh!

bndos commented May 22, 2023

Uh oh!

stuartmorgan-g commented May 23, 2023

Uh oh!

stuartmorgan-g left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bndos Jun 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bndos Jun 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bndos Jun 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bndos Jun 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bndos Jun 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bndos commented Jun 20, 2023

Uh oh!

reidbaker commented Jul 18, 2023

Uh oh!

reidbaker left a comment

Choose a reason for hiding this comment

Uh oh!

stuartmorgan-g commented Jul 18, 2023

Uh oh!

ditman Jul 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

bndos commented May 22, 2023 •

edited

Loading

bndos Jun 19, 2023 •

edited

Loading

bndos Jun 19, 2023 •

edited

Loading

bndos Jun 19, 2023 •

edited

Loading

bndos Jun 19, 2023 •

edited

Loading

bndos Jun 19, 2023 •

edited

Loading

ditman Jul 19, 2023 •

edited

Loading