[google_maps_flutter] Add onPoiTap callback to GoogleMap #10963
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,96 @@ | ||
| // Copyright 2013 The Flutter Authors | ||
| // Use of this source code is governed by a BSD-style license that can be | ||
| // found in the LICENSE file. | ||
|
|
||
| import 'package:flutter/material.dart'; | ||
| import 'package:google_maps_flutter/google_maps_flutter.dart'; | ||
|
|
||
| import 'page.dart'; | ||
|
|
||
| /// Page for demonstrating Point of Interest (POI) tapping. | ||
| class PlacePoiPage extends GoogleMapExampleAppPage { | ||
| /// Default constructor. | ||
| const PlacePoiPage({super.key}) | ||
| : super(const Icon(Icons.business), 'Place POI'); | ||
|
|
||
| @override | ||
| Widget build(BuildContext context) { | ||
| return const PlacePoiBody(); | ||
| } | ||
| } | ||
|
|
||
| /// Body of the POI page. | ||
| class PlacePoiBody extends StatefulWidget { | ||
| /// Default constructor. | ||
| const PlacePoiBody({super.key}); | ||
|
|
||
| @override | ||
| State<StatefulWidget> createState() => PlacePoiBodyState(); | ||
| } | ||
|
|
||
| /// State for [PlacePoiBody]. | ||
| class PlacePoiBodyState extends State<PlacePoiBody> { | ||
| /// The controller for the map. | ||
| /// | ||
| /// This is public to match the example pattern, but marked with a doc comment. | ||
| GoogleMapController? controller; | ||
| PointOfInterest? _lastPoi; | ||
|
|
||
| final CameraPosition _kKolkata = const CameraPosition( | ||
| target: LatLng(22.54222641620606, 88.34560669761545), | ||
| zoom: 16.0, | ||
| ); | ||
|
|
||
| // ignore: use_setters_to_change_properties | ||
| void _onMapCreated(GoogleMapController controller) { | ||
| this.controller = controller; | ||
| } | ||
|
|
||
| void _onPoiTap(PointOfInterest poi) { | ||
| setState(() { | ||
| _lastPoi = poi; | ||
| }); | ||
|
|
||
| controller?.animateCamera(CameraUpdate.newLatLng(poi.position)); | ||
| } | ||
|
|
||
| @override | ||
| Widget build(BuildContext context) { | ||
| return Column( | ||
| children: <Widget>[ | ||
| Expanded( | ||
| child: GoogleMap( | ||
| onMapCreated: _onMapCreated, | ||
| initialCameraPosition: _kKolkata, | ||
| onPoiTap: _onPoiTap, | ||
| myLocationButtonEnabled: false, | ||
| ), | ||
| ), | ||
| Container( | ||
| color: Colors.white, | ||
| width: double.infinity, | ||
| padding: const EdgeInsets.all(16.0), | ||
| child: Column( | ||
| crossAxisAlignment: CrossAxisAlignment.start, | ||
| mainAxisSize: MainAxisSize.min, | ||
| children: <Widget>[ | ||
| const Text( | ||
| 'Last Tapped POI:', | ||
| style: TextStyle(fontWeight: FontWeight.bold, fontSize: 16), | ||
| ), | ||
| const SizedBox(height: 8), | ||
| if (_lastPoi != null) ...<Widget>[ | ||
| Text('Name: ${_lastPoi!.name ?? "Unknown"}'), | ||
| Text('Place ID: ${_lastPoi!.placeId}'), | ||
| Text( | ||
| 'Lat/Lng: ${_lastPoi!.position.latitude.toStringAsFixed(5)}, ${_lastPoi!.position.longitude.toStringAsFixed(5)}', | ||
| ), | ||
| ] else | ||
| const Text('Tap on a business or landmark icon...'), | ||
| ], | ||
| ), | ||
| ), | ||
| ], | ||
| ); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -133,6 +133,7 @@ class GoogleMap extends StatefulWidget { | |
| this.onCameraIdle, | ||
| this.onTap, | ||
| this.onLongPress, | ||
| this.onPoiTap, | ||
| this.cloudMapId, | ||
| }); | ||
|
|
||
|
|
@@ -235,6 +236,9 @@ class GoogleMap extends StatefulWidget { | |
| /// See https://pub.dev/packages/google_maps_flutter_web. | ||
| final Set<ClusterManager> clusterManagers; | ||
|
|
||
| /// Callback for Point of Interests tap | ||
| final ArgumentCallback<PointOfInterest>? onPoiTap; | ||
|
Collaborator
There was a problem hiding this comment. My previous comment has not been addressed. This is still returning an inflated POI rather than an ID. |
||
|
|
||
| /// Ground overlays to be initialized for the map. | ||
| /// | ||
| /// Support table for Ground Overlay features: | ||
|
|
@@ -612,6 +616,12 @@ class _GoogleMapState extends State<GoogleMap> { | |
| } | ||
| } | ||
|
|
||
| void onPoiTap(PointOfInterest poi) { | ||
| if (widget.onPoiTap != null) { | ||
| widget.onPoiTap!(poi); | ||
| } | ||
|
There was a problem hiding this comment. For conciseness, you can use the null-aware widget.onPoiTap?.call(poi);
Author
There was a problem hiding this comment. @stuartmorgan-g can you please see the code changes again. I have removed the name param from the onPoiTap. As you said that it sent inflated POI values in the web version. In the web version the name is not there in the native implementation which is why the inflated values were returned that's why I have removed the name param from all platform implementations and kept only the placeId and position. As the unimplemented error was thrown which would have resulted in a breaking change so I have changed it to return an empty string instead of throwing. I also applied the make-deps-path-based tooling but it was resulting in blank dependency overrides so I had to add the dependency overrides manually. Please help me in understanding this if I am using the tool incorrectly. Above all thank you for your constant valuable feedback on the errors I was doing continuously, it would be great if you explain more about the errors if those happen again, and I have read the documentation regarding the federated plugin change and did all what I understood, please guide me if I have left something unaddressed.
Collaborator
There was a problem hiding this comment.
It appears that you have removed the name from the The issue is not that POI doesn't have a name on web, it's that the tap event callback doesn't return a POI on web. That can't be fixed by removing information from POI itself. It needs to be solved by having the tap handler receive something that is not the POI class.
Author
There was a problem hiding this comment. @stuartmorgan-g Thank you for the clarification. That makes perfect sense regarding not removing data from the core PointOfInterest class. Before I write the code and push another combination PR, I want to make sure my planned architecture aligns exactly with your expectations:
Does this separation between the core POI class and the tap event payload resolve the issue?
Collaborator
There was a problem hiding this comment. My expectation is that you follow the architecture laid out in the comment I have referred you to several times, which is not what you just described. Before I spend more of my time on this review, I would like a clear, explicit answer to my previous question about how you are using AI here, and how much human involvement there is.
Author
There was a problem hiding this comment. @stuartmorgan-g Thank you for your time and patience. I am trying to grasp on the architecture that you have referred in the comments. I have used AI to learn about the CI errors I was going through and therefore trying my best to contribute to the package.
Collaborator
There was a problem hiding this comment.
A key point is this section of the comment:
An event payload class that has auto-hydrated POI data (such as a |
||
| } | ||
|
|
||
| void onPolygonTap(PolygonId polygonId) { | ||
| final Polygon? polygon = _polygons[polygonId]; | ||
| if (polygon == null) { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs to be grouped with, and following the same comment style as, all the other callbacks.