-
Notifications
You must be signed in to change notification settings - Fork 189
Feature/map view types #9
Feature/map view types #9
Conversation
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 PR! I just have a few small changes we'd like to see before merging.
lib/map_options.dart
Outdated
|
||
MapOptions( | ||
{this.showUserLocation: false, | ||
this.initialCameraPosition: _defaultCamera, | ||
this.title: ""}); | ||
this.title: "", | ||
this.mapViewType}); |
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.
mapViewType should have a default value of Normal.
lib/map_view_type.dart
Outdated
@@ -0,0 +1,8 @@ | |||
|
|||
class MapViewType { | |||
static const int MAP_TYPE_HYBRID = 4; |
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.
Should use dart style here:
none,
hybrid,
satellite,
terrain.
Also, are you confident these int values map the same on iOS and Android?
ios/Classes/MapViewPlugin.m
Outdated
@@ -31,6 +31,8 @@ - (void)handleMethodCall:(FlutterMethodCall *)call result:(FlutterResult)result | |||
NSDictionary *mapOptions = args[@"mapOptions"]; | |||
NSDictionary *cameraDict = mapOptions[@"cameraPosition"]; | |||
self.mapTitle = mapOptions[@"title"]; | |||
|
|||
self.mapViewType = [mapOptions[@"mapViewType"] intValue]; |
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.
current mapViewType could be null or 0 because it's not a required param for MapOptions. You should make sure this is a valid enum value before setting it.
@@ -95,6 +96,7 @@ class MapViewPlugin(val activity: Activity) : MethodCallHandler { | |||
toolbarActions = getToolbarActions(call.argument<List<Map<String, Any>>>("actions")) | |||
showUserLocation = mapOptions["showUserLocation"] as Boolean | |||
mapTitle = mapOptions["title"] as String | |||
mapViewType = mapOptions["mapViewType"] as Int |
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.
Not guarantee to not be null currently. You should cast it to Int? and check to make sure it's not null before setting it on mapViewType
.
@@ -35,6 +35,9 @@ class MapActivity : AppCompatActivity(), | |||
override fun onMapReady(map: GoogleMap) { | |||
googleMap = map | |||
|
|||
//set mapViewType |
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.
Remove comment. Code is clear enough
Hi,
I have added fucntionality for configuring
map type
, which I raised the issue about changing to Satellite view #7P.S: I will add the same functionality to IOS later next week.
Thanks
Helen