[google_maps_flutter] Add support for styling google maps #1697
Conversation
- supports both Android and iOS - fixes: flutter/flutter#24147
| if (_nightMode) { | ||
| setState(() { | ||
| _nightMode = false; | ||
| _controller.setMapStyle(null); |
There was a problem hiding this comment.
Can _controller be null here?
There was a problem hiding this comment.
No, this is only called from _nightModeToggler where we first check for _isMapCreated which is set alongside with _controller.
| } | ||
|
|
||
| - (BOOL)setMapStyle:(NSString*)mapStyle { | ||
| if (mapStyle == (id)[NSNull null] || mapStyle.length == 0) { |
There was a problem hiding this comment.
nit: we don't seem to cast to (id) elsewhere, do we need it?
There was a problem hiding this comment.
I get an IDE warning about comparison between incompatible pointer types, we could either cast it to (NSString*) or (id) to address that and I chose the latter. It shouldn't be a big deal if we choose to remove the cast but seems reasonable to leave it.
There was a problem hiding this comment.
Totally reasonable was just wondering why this is different from the other similar checks in this file. Taking another look I guess it's because the other instances here are access NSArray elements.
| return YES; | ||
| } | ||
| NSError* error; | ||
| GMSMapStyle* style = [GMSMapStyle styleWithJSONString:mapStyle error:&error]; |
There was a problem hiding this comment.
is there any useful information in error we should send back to Dart?
There was a problem hiding this comment.
The error has some more details about what the issue with the JSON is. Though it is useful to know what the error is for fixing the style, I don't think we can take any action based on the specifics of the error on the dart side in this case.
Typical action on error would be to keep the existing style (which is the default) or change to an empty/alternate style, which the user can currently do by checking the return value.
There was a problem hiding this comment.
We should pass back whatever information we have to the Dart side. e.g it's possible that I'm building an app for editing and previewing map styles, in which case I would even want to show the error to the user.
| /// feature type, unrecognized element type, or invalid styler keys. If the return | ||
| /// value is `false`, the current style is left unchanged. | ||
| /// | ||
| /// The style string can be generated using https://mapstyle.withgoogle.com/ |
There was a problem hiding this comment.
Is there any documentation for the json format we can refer to?
There was a problem hiding this comment.
Here's the iOS style reference: https://developers.google.com/maps/documentation/ios-sdk/style-reference
I believe it is identical to Android: https://developers.google.com/maps/documentation/android-sdk/style-reference
There was a problem hiding this comment.
Thanks @domesticmouse , will add these to the docs.
|
landing after a verbal LGTM from @amirh 👍 |
|
Is there any date to publish a new version with this changes? |
|
@pedromassango it's published |
|
Thanks for great improvement guys |
|
Is there any documentation on how to use this? |
|
@nephix the dart docs on the method over at https://github.com/flutter/plugins/pull/1697/files#diff-41940e5867e958815f51ef18747727edR189 should answer most questions, also look at the example app for how this is used. |
|
is it intended behavior that you can only style the "normal" map? mapType: MapType.hybrid, // No styling applied. :( Or am I doing something wrong? |
This comment has been minimized.
This comment has been minimized.
9 participants
Checklist
///).flutter analyze) does not report any problems on my PR.Breaking Change
Does your PR require plugin users to manually update their apps to accommodate your change?