Conversation
Forward GMSMapView POI taps to PointOfInterestTapEvent for sdk9/sdk10 (and shared source), with Dart and native unit coverage, targeting platform interface 2.17.0.
953c4fe to
446ac1d
Compare
| /// | ||
| /// This exists to add AnyObject to the requirements, so that references to it can be weak. | ||
| protocol MapEventDelegate: AnyObject, MapsCallbackApiProtocol {} | ||
| protocol MapEventDelegate: AnyObject, MapsCallbackApiProtocol, TileProviderDelegate {} |
There was a problem hiding this comment.
Why was this changed? Providing tiles has nothing to do with event delegation.
If these changes were made so that you only have to inject one object instead of two, please don't do that. The fact that the default implementation is the same object is irrelevant to testing.
There was a problem hiding this comment.
Agreed — that was a mistake. I had merged TileProviderDelegate into MapEventDelegate so a single test fake could cover both roles; that’s the wrong tradeoff and unrelated to POI.
Reverted: MapEventDelegate is only MapsCallbackApiProtocol again, and MapsCallbackApi: TileProviderDelegate is restored as on main.
| assetProvider: AssetProvider, | ||
| binaryMessenger: FlutterBinaryMessenger | ||
| binaryMessenger: FlutterBinaryMessenger, | ||
| callbackHandler: MapEventDelegate? = nil |
There was a problem hiding this comment.
Please follow the pattern all the other dependency injection uses, where it's non-optional, and the default value is set from the convenience initializer.
There was a problem hiding this comment.
Thanks — updated to match the existing DI pattern: callbackHandler is required on the designated initializer, and the convenience initializers supply a MapsCallbackApi default (same approach as assetProvider).
|
Hey, sorry, still draft, just rebased with quick fixes but not yet ready, I should have been explicit. |
|
Superseded by #12880 (consolidated Android/iOS/web implementations PR). |
…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
2 participants
Summary
sdk9/sdk10+ shared source) implementation ofonPointOfInterestTap, following the published platform interface from [google_maps_flutter_platform_interface] Add onPointOfInterestTap support #12752GMSMapViewdidTapPOIWithPlaceIDtoPointOfInterestTapEventMapEventDelegate) so coverage matches [google_maps_flutter] Add onPointOfInterestTap #11872Divergence from #11872 (intentional):
GoogleMapControlleronmain(same behavior; not a replay of the old ObjC files)google_maps_flutter_ios— that package no longer takes feature updates (see its README); use sdk9/sdk10 insteadMapsCallbackApiProtocol/MapEventDelegateinstead of the ObjC-onlyFGMMapsCallbackApiProtocolwrapper from the combined PRPart of #11872 / flutter/flutter#60695.
Depends on published
google_maps_flutter_platform_interface2.17.0.Pre-Review Checklist
[shared_preferences]///).