[google_maps_flutter]ChangeNotifier is replaced with granular callbacks #1302
Conversation
This facilitates better interaction with the map than adding a listener to the controller.
cyanglaz
changed the title
| zoom: 11.0, | ||
| ); | ||
|
|
||
| GoogleMapController mapController; |
|
|
||
| /// Callback that tracks camera position. | ||
| /// | ||
| /// Will be null if trackCameraPosition on [GoogleMap] is false. |
|
|
||
| typedef void MapCreatedCallback(GoogleMapController controller); | ||
|
|
||
| /// Callback that tracks camera position. |
There was a problem hiding this comment.
Callback for getting updates on a new camera position?
| typedef void MapCreatedCallback(GoogleMapController controller); | ||
|
|
||
| /// Callback that tracks camera position. | ||
| /// |
There was a problem hiding this comment.
Probably worth adding "Used by: " and listing the relevant GoogleMap fields.
| /// Markers to be placed on the map. | ||
| final Set<Marker> markers; | ||
|
|
||
| /// Callback for when the camera move started. |
There was a problem hiding this comment.
This doesn't add much information.
https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo#avoid-useless-documentation
Documentation is definitely hard 😄 I sometimes find the prompts here helpful:
https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo#writing-prompts-for-good-documentation
Some questions I might have been looking for answers in the docs are: is this called on the first time the map is loaded? is this called when the position is updated programmatically?
(similar comments to the docs of the other new fields below)
There was a problem hiding this comment.
Great read. Will revise the docs based on this.
| /// Callback for when the camera move started. | ||
| final VoidCallback onCameraMoveStarted; | ||
|
|
||
| /// Callback for when the camera is moving. |
There was a problem hiding this comment.
This should probably say something about [GoogleMap.trackCameraPosition]
| final VoidCallback onCameraIdle; | ||
|
|
||
| /// Callback when the [GoogleMap] options have been applied on the platform. | ||
| final CameraPositionCallback onMapOptionsUpdate; |
There was a problem hiding this comment.
I wonder if the onOptionsUpdate and onMarkersUpdate callback are really needed? do you have some concrete use-cases in mind?
We can always add fields later, but removing will be a breaking API change.
There was a problem hiding this comment.
They provide a way for users to know when the platform has "committed" the update. It might be useful to take an action based on it. We could remove them, I put them here to maintain compatibility with the behavior before.
|
|
||
| /// Callback that receives updates to the camera position. | ||
| /// | ||
| /// This callback is triggered when the platform google map |
| /// This can be initiated by the following: | ||
| /// 1. Non-gesture animation initiated in response to user actions. | ||
| /// For example: zoom buttons, my location button, or marker clicks. | ||
| /// 2. Developer initiated animation. |
There was a problem hiding this comment.
nit: I'd call it "Programmatically initiated animation."
| /// This may be called as often as once every frame and should | ||
| /// not perform expensive operations. | ||
| /// | ||
| /// This is callback is not called when trackCameraPosition is false. |
| /// This may be called as often as once every frame and should | ||
| /// not perform expensive operations. | ||
| /// | ||
| /// This is callback is not called when trackCameraPosition is false. |
There was a problem hiding this comment.
nit: since the default for trackCameraPosition is false, I'll reverse the negation (This is only called if [trackCameraPosition] is false).
BTW does it never gets called if trackCameraPosition is false? in that case maybe it would make sense to get rid of trackCameraPosition and use the presence of this callback as the flag?
There was a problem hiding this comment.
I am pro getting rid of trackCameraPosition entirely, it's a notion introduced by the plugin and not something that is native to Google Map platform implementations. Cool if i do it in a follow up PR?
|
merging since these are successes on https://cirrus-ci.com/build/5658597900943360 |
…ks (flutter#1302) * ChangeNotifier is replaced with granular callbacks This facilitates better interaction with the map than adding a listener to the controller. * Make docs better * Remove camera update call backs for map and marker update * Fix up some links and caps. * Update changelog and pubspec. * fix unused var
* commit '9017d6e7f867af278edd7e8e584d52524f37443f': (408 commits) [webview_flutter]Allow specifying a navigation delegate(Android and Dart). (flutter#1236) Allow specifying a navigation delegate (iOS implementation). (flutter#1323) Change build link in contributors site to cirrus (flutter#1327) [image_picker] Update versioning for flutter#1268 (flutter#1326) [image_picker] remove unnecessary camera permmision (flutter#1268) Exclude longPress from semantics (flutter#1324) [in_app_purchase] refactoring and tests (flutter#1322) [in_app_purchase] Adds Dart BillingClient APIs for loading purchases (flutter#1286) [connectivity] Update README.md (flutter#1201) [camera] Fixes #28350 (flutter#1261) [cloud_functions] Specify version for CocoaPod and handle null regions gracefully (flutter#1316) [in_app_purchase]retrieve receipt (flutter#1303) [firebase_analytics] Add resetAnalyticsData method (flutter#1311) trackCameraPosition is inferred from GoogleMap.onCameraMove (flutter#1314) [google_maps_flutter]ChangeNotifier is replaced with granular callbacks (flutter#1302) [video_player]Do not divide by zero (flutter#793) [firebase_dynamic_links] Version bump for firebase_dynamic_links PR flutter#1142 (flutter#1309) [firebase_dynamic_links] fix dynamic link crash when creating shortlink if warnings are null (flutter#1142) Fix typo in RewardedVideoAdd sample (flutter#927) Add my name to firebase_performance and firebase_dynamic_links owners (flutter#1300) ... # Conflicts: # packages/camera/android/src/main/java/io/flutter/plugins/camera/CameraPlugin.java # packages/camera/ios/Classes/CameraPlugin.m # packages/firebase_auth/ios/Classes/FirebaseAuthPlugin.m # packages/image_picker/android/build.gradle # packages/video_player/android/src/main/java/io/flutter/plugins/videoplayer/VideoPlayerPlugin.java
3 participants
This facilitates better interaction with the map
than adding a listener to the controller.