-
Notifications
You must be signed in to change notification settings - Fork 9.8k
Conversation
mravn-google
commented
Apr 4, 2018
•
edited
Loading
edited
- Add Dart API to move and animate map camera.
- Add Android implementation.
- Restructure plugin to use Dart library and parts.
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.
LGTM
googleMap.animateCamera(cameraUpdate); | ||
void moveCamera(final CameraUpdate cameraUpdate) { | ||
if (googleMap == null) { | ||
pendingOperations.add( |
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.
We might want to save the pre-initialized operations on the dart side to avoid duplicating this logic for ios.
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.
Done.
) | ||
], | ||
body: new NotificationListener<ScrollNotification>( | ||
onNotification: (ScrollNotification sn) { |
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.
Nit:
Don't abbreviate. sn => scrollNotification
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.
Done.
); | ||
} | ||
|
||
static CameraUpdate newLatLong(LatLng latLng) { |
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.
newLatLong => newLatLng ?
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.
Done.
/// | ||
/// Used for programmatically controlling a platform-specific | ||
/// GoogleMaps view, once it has been created and integrated | ||
/// into the Flutter application. |
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.
"once it has been created and integrated into the Flutter application." => "once it has been initialized."?
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.
Done.
|
||
part of google_mobile_maps; | ||
|
||
class Marker {} |
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.
Add TODO
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.
Class removed.
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.
Thanks for the review!
googleMap.animateCamera(cameraUpdate); | ||
void moveCamera(final CameraUpdate cameraUpdate) { | ||
if (googleMap == null) { | ||
pendingOperations.add( |
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.
Done.
); | ||
} | ||
|
||
static CameraUpdate newLatLong(LatLng latLng) { |
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.
Done.
) | ||
], | ||
body: new NotificationListener<ScrollNotification>( | ||
onNotification: (ScrollNotification sn) { |
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.
Done.
|
||
part of google_mobile_maps; | ||
|
||
class Marker {} |
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.
Class removed.
/// | ||
/// Used for programmatically controlling a platform-specific | ||
/// GoogleMaps view, once it has been created and integrated | ||
/// into the Flutter application. |
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.
Done.
This was done in flutter#463, not sure why, but I don't think we should export it (and it makes Dartdoc complain).
This was done in #463, not sure why, but I don't think we should export it (and it makes Dartdoc complain).
This was done in flutter#463, not sure why, but I don't think we should export it (and it makes Dartdoc complain).