Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[google_maps_flutter] Add style to widget #6192

Merged
merged 19 commits into from
Feb 28, 2024
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
## NEXT
## 2.6.0

* Adds `style` to the GoogleMap widget constructor. This allows setting the map
style during creation, avoiding the possibility of the default style being
displayed briefly.
* Deprecates `GoogleMapController.setMapStyle` in favor of setting the style via
the new widget `style` parameter.
* Updates minimum supported SDK version to Flutter 3.13/Dart 3.1.

## 2.5.3
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -448,6 +448,28 @@ void runTests() {
await mapIdCompleter.future;
},
);

testWidgets('getStyleError reports last error', (WidgetTester tester) async {
final Key key = GlobalKey();
final Completer<GoogleMapController> controllerCompleter =
Completer<GoogleMapController>();

await tester.pumpWidget(Directionality(
textDirection: TextDirection.ltr,
child: GoogleMap(
key: key,
initialCameraPosition: kInitialCameraPosition,
style: '[[[this is an invalid style',
onMapCreated: (GoogleMapController controller) {
controllerCompleter.complete(controller);
},
),
));

final GoogleMapController controller = await controllerCompleter.future;
final String? error = await controller.getStyleError();
expect(error, isNotNull);
});
}

/// Repeatedly checks an asynchronous value against a test condition.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

// ignore_for_file: public_member_api_docs

import 'package:flutter/foundation.dart';
import 'package:flutter/material.dart';
import 'package:flutter/services.dart' show rootBundle;
import 'package:google_maps_flutter/google_maps_flutter.dart';
Expand Down Expand Up @@ -59,6 +60,7 @@ class MapUiBodyState extends State<MapUiBody> {
bool _myLocationButtonEnabled = true;
late GoogleMapController _controller;
bool _nightMode = false;
String _mapStyle = '';

@override
void initState() {
Expand Down Expand Up @@ -243,27 +245,18 @@ class MapUiBodyState extends State<MapUiBody> {
return rootBundle.loadString(path);
}

void _setMapStyle(String mapStyle) {
setState(() {
_nightMode = true;
_controller.setMapStyle(mapStyle);
});
}

// Should only be called if _isMapCreated is true.
Widget _nightModeToggler() {
assert(_isMapCreated);
return TextButton(
child: Text('${_nightMode ? 'disable' : 'enable'} night mode'),
onPressed: () {
if (_nightMode) {
setState(() {
_nightMode = false;
_controller.setMapStyle(null);
});
} else {
_getFileData('assets/night_mode.json').then(_setMapStyle);
}
onPressed: () async {
_nightMode = !_nightMode;
final String style =
_nightMode ? await _getFileData('assets/night_mode.json') : '';
setState(() {
_mapStyle = style;
});
},
);
}
Expand All @@ -278,6 +271,7 @@ class MapUiBodyState extends State<MapUiBody> {
cameraTargetBounds: _cameraTargetBounds,
minMaxZoomPreference: _minMaxZoomPreference,
mapType: _mapType,
style: _mapStyle,
rotateGesturesEnabled: _rotateGesturesEnabled,
scrollGesturesEnabled: _scrollGesturesEnabled,
tiltGesturesEnabled: _tiltGesturesEnabled,
Expand Down Expand Up @@ -352,5 +346,13 @@ class MapUiBodyState extends State<MapUiBody> {
_controller = controller;
_isMapCreated = true;
});
// Log any style errors to the console for debugging.
if (kDebugMode) {
_controller.getStyleError().then((String? error) {
if (error != null) {
debugPrint(error);
}
});
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -33,3 +33,11 @@ flutter:
uses-material-design: true
assets:
- assets/

# FOR TESTING AND INITIAL REVIEW ONLY. DO NOT MERGE.
# See https://github.com/flutter/flutter/wiki/Contributing-to-Plugins-and-Packages#changing-federated-plugins
dependency_overrides:
google_maps_flutter_android: {path: ../../../google_maps_flutter/google_maps_flutter_android}
google_maps_flutter_ios: {path: ../../../google_maps_flutter/google_maps_flutter_ios}
google_maps_flutter_platform_interface: {path: ../../../google_maps_flutter/google_maps_flutter_platform_interface}
google_maps_flutter_web: {path: ../../../google_maps_flutter/google_maps_flutter_web}
Original file line number Diff line number Diff line change
Expand Up @@ -190,11 +190,17 @@ class GoogleMapController {
/// Also, refer [iOS](https://developers.google.com/maps/documentation/ios-sdk/style-reference)
/// and [Android](https://developers.google.com/maps/documentation/android-sdk/style-reference)
/// style reference for more information regarding the supported styles.
@Deprecated('Use GoogleMap.style instead.')
Future<void> setMapStyle(String? mapStyle) {
return GoogleMapsFlutterPlatform.instance
.setMapStyle(mapStyle, mapId: mapId);
}

/// Returns the last style error, if any.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this say that it will get errors for the current mapid only?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MapController is a per-map object; it's created with this ID.

Future<String?> getStyleError() {
return GoogleMapsFlutterPlatform.instance.getStyleError(mapId: mapId);
}

/// Return [LatLngBounds] defining the region that is visible in a map.
Future<LatLngBounds> getVisibleRegion() {
return GoogleMapsFlutterPlatform.instance.getVisibleRegion(mapId: mapId);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ class GoogleMap extends StatefulWidget {
const GoogleMap({
super.key,
required this.initialCameraPosition,
this.style,
this.onMapCreated,
this.gestureRecognizers = const <Factory<OneSequenceGestureRecognizer>>{},
this.webGestureHandling,
Expand Down Expand Up @@ -136,6 +137,19 @@ class GoogleMap extends StatefulWidget {
/// The initial position of the map's camera.
final CameraPosition initialCameraPosition;

/// The style for the map.
///
/// Set to null to clear any previous custom styling.
///
/// If problems were detected with the [mapStyle], including un-parsable
/// styling JSON, unrecognized feature type, unrecognized element type, or
/// invalid styler keys, the style is left unchanged, and the error can be
/// retrieved with [GoogleMapController.getStyleError].
///
/// The style string can be generated using the
/// [map style tool](https://mapstyle.withgoogle.com/).
final String? style;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any way at all to make this a type safe object?

If not can we at least point users to https://developers.google.com/maps/documentation/cloud-customization which does not require app updates and a stringly typed api.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any way at all to make this a type safe object?

Not in any useful way. The native objects are opaque objects created from a string that is supposed to be a JSON string. So the options are:

  • Make a do-nothing MapStyle object that just wraps a String, but in practice that would almost certainly just convert every call from style: someString to style: MapStyle(someString)
  • Pre-parse the JSON on the Dart side, only to convert it back to a string and then have the Maps SDK re-parse it anyway.

If not can we at least point users to https://developers.google.com/maps/documentation/cloud-customization which does not require app updates and a stringly typed api.

If you follow the link there in this comment for generating the JSON style, you'll see that we don't really need to do that at the plugin level, as the SDK's docs and tool are very aggressive about this. (Also, there are significant caveats, like cloud style requires a per-load charge, and doesn't work on all Android devices, which is more than I want to get into in the comments here. I'd rather err on the side of being a neutral wrapper of the SDKs, and let the SDK docs steer people.)

Copy link
Member

@ditman ditman Feb 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It can definitely be made a type-safe object (we have a "type-safe-ish version" in the web version); the value of the String version of this property is that it can be generated/tweaked (and then copy-pasted) from here: https://mapstyle.withgoogle.com (and probably others)

HOWEVER, one thing that Google Maps SDK seems to be doing is to call this feature "legacy styling", so maybe this property could be called: legacyStyle and in the documentation point them to the link that you're proposing? That way people can learn about cloud styling, the mapId property and the new way (and cost) of doing things?

There's even documentation on how to migrate from this legacyStyle value to the new cloud-based one: https://developers.google.com/maps/documentation/javascript/cloud-customization/map-styles#import-json-styling

Copy link
Contributor Author

@stuartmorgan stuartmorgan Feb 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It can definitely be made a type-safe object

Sorry, technically there is a third option: we could—in our wrapper that is already under-resourced—reverse-engineer a complex object graph from docs, thus building and maintaining a much more complex API than what the SDKs provide, creating a ton of work for us, only to throw away all that structure and re-encode it as a string anyway at the SDK boundary. And thereby make the existing tools for these styles harder for our clients to use. And also being inconsistent with our setMapStyle, making it significantly harder to migrate to this new API.

It's theoretically possible, but I don't see any actual upsides.

HOWEVER, one thing that Google Maps SDK seems to be doing is to call this feature "legacy styling", so maybe this property could be called: legacyStyle

Their high-level docs call it legacy style. The actual SDKs, and the code-level API docs for those SDKs, do not use that language anywhere that I have seen.

I do not want to be in the business of being significantly more opinionated in the code about what APIs people should or should not use than the people actually maintaining the SDK we are wrapping.

I also don't want setMapStyle (the existing method) and legacyStyle (the new, preferred approach), since that will make the deprecated one seem preferable and almost certainly confuse everyone.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SGTM, legacyStyle seems to be only mentioned in the Cloud Styling docs.


/// True if the map should show a compass when rotated.
final bool compassEnabled;

Expand Down Expand Up @@ -556,5 +570,8 @@ MapConfiguration _configurationFromMapWidget(GoogleMap map) {
trafficEnabled: map.trafficEnabled,
buildingsEnabled: map.buildingsEnabled,
cloudMapId: map.cloudMapId,
// A null style in the widget means no style, which is expressed as '' in
// the configuration to distinguish from no change (null).
style: map.style ?? '',
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ name: google_maps_flutter
description: A Flutter plugin for integrating Google Maps in iOS and Android applications.
repository: https://github.com/flutter/packages/tree/main/packages/google_maps_flutter/google_maps_flutter
issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+maps%22
version: 2.5.3
version: 2.6.0

environment:
sdk: ^3.1.0
Expand Down Expand Up @@ -40,3 +40,11 @@ topics:
# The example deliberately includes limited-use secrets.
false_secrets:
- /example/web/index.html

# FOR TESTING AND INITIAL REVIEW ONLY. DO NOT MERGE.
# See https://github.com/flutter/flutter/wiki/Contributing-to-Plugins-and-Packages#changing-federated-plugins
dependency_overrides:
google_maps_flutter_android: {path: ../../google_maps_flutter/google_maps_flutter_android}
google_maps_flutter_ios: {path: ../../google_maps_flutter/google_maps_flutter_ios}
google_maps_flutter_platform_interface: {path: ../../google_maps_flutter/google_maps_flutter_platform_interface}
google_maps_flutter_web: {path: ../../google_maps_flutter/google_maps_flutter_web}
Original file line number Diff line number Diff line change
Expand Up @@ -551,4 +551,32 @@ void main() {

expect(map.mapConfiguration.buildingsEnabled, true);
});

testWidgets('Can update style', (WidgetTester tester) async {
const String initialStyle = '[]';
await tester.pumpWidget(
const Directionality(
textDirection: TextDirection.ltr,
child: GoogleMap(
initialCameraPosition: CameraPosition(target: LatLng(10.0, 15.0)),
style: initialStyle,
),
),
);

final PlatformMapStateRecorder map = platform.lastCreatedMap;

expect(map.mapConfiguration.style, initialStyle);

await tester.pumpWidget(
const Directionality(
textDirection: TextDirection.ltr,
child: GoogleMap(
initialCameraPosition: CameraPosition(target: LatLng(10.0, 15.0)),
),
),
);

expect(map.mapConfiguration.style, '');
});
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
## NEXT
## 2.7.0

* Adds support for `MapConfiguration.style`.
* Adds support for `getStyleError`.
* Updates minimum supported SDK version to Flutter 3.13/Dart 3.1.
* Updates compileSdk version to 34.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -377,6 +377,10 @@ static void interpretGoogleMapOptions(Object o, GoogleMapOptionsSink sink) {
if (buildingsEnabled != null) {
sink.setBuildingsEnabled(toBoolean(buildingsEnabled));
}
final Object style = data.get("style");
if (style != null) {
sink.setMapStyle(toString(style));
}
}

/** Returns the dartMarkerId of the interpreted marker. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

import android.content.Context;
import android.graphics.Rect;
import androidx.annotation.Nullable;
import com.google.android.gms.maps.GoogleMapOptions;
import com.google.android.gms.maps.model.CameraPosition;
import com.google.android.gms.maps.model.LatLngBounds;
Expand All @@ -27,6 +28,7 @@ class GoogleMapBuilder implements GoogleMapOptionsSink {
private Object initialCircles;
private List<Map<String, ?>> initialTileOverlays;
private Rect padding = new Rect(0, 0, 0, 0);
private @Nullable String style;

GoogleMapController build(
int id,
Expand All @@ -48,6 +50,7 @@ GoogleMapController build(
controller.setInitialCircles(initialCircles);
controller.setPadding(padding.top, padding.left, padding.bottom, padding.right);
controller.setInitialTileOverlays(initialTileOverlays);
controller.setMapStyle(style);
return controller;
}

Expand Down Expand Up @@ -178,4 +181,9 @@ public void setInitialCircles(Object initialCircles) {
public void setInitialTileOverlays(List<Map<String, ?>> initialTileOverlays) {
this.initialTileOverlays = initialTileOverlays;
}

@Override
public void setMapStyle(@Nullable String style) {
this.style = style;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;

/** Controller of a single GoogleMaps MapView instance. */
final class GoogleMapController
Expand Down Expand Up @@ -87,6 +88,9 @@ final class GoogleMapController
private List<Object> initialPolylines;
private List<Object> initialCircles;
private List<Map<String, ?>> initialTileOverlays;
// Null except between initialization and onMapReady.
private @Nullable String initialMapStyle;
private @Nullable String lastStyleError;
@VisibleForTesting List<Float> initialPadding;

GoogleMapController(
Expand Down Expand Up @@ -169,6 +173,10 @@ public void onMapReady(GoogleMap googleMap) {
initialPadding.get(2),
initialPadding.get(3));
}
if (initialMapStyle != null) {
updateMapStyle(initialMapStyle);
initialMapStyle = null;
}
}

// Returns the first TextureView found in the view hierarchy.
Expand Down Expand Up @@ -459,26 +467,22 @@ public void onSnapshotReady(Bitmap bitmap) {
}
case "map#setStyle":
{
boolean mapStyleSet;
if (call.arguments instanceof String) {
String mapStyle = (String) call.arguments;
if (mapStyle == null) {
mapStyleSet = googleMap.setMapStyle(null);
} else {
mapStyleSet = googleMap.setMapStyle(new MapStyleOptions(mapStyle));
}
} else {
mapStyleSet = googleMap.setMapStyle(null);
}
Object arg = call.arguments;
final String style = arg instanceof String ? (String) arg : null;
final boolean mapStyleSet = updateMapStyle(style);
ArrayList<Object> mapStyleResult = new ArrayList<>(2);
mapStyleResult.add(mapStyleSet);
if (!mapStyleSet) {
mapStyleResult.add(
"Unable to set the map style. Please check console logs for errors.");
mapStyleResult.add(lastStyleError);
}
result.success(mapStyleResult);
break;
}
case "map#getStyleError":
{
result.success(lastStyleError);
break;
}
case "tileOverlays#update":
{
List<Map<String, ?>> tileOverlaysToAdd = call.argument("tileOverlaysToAdd");
Expand Down Expand Up @@ -926,4 +930,22 @@ public void setTrafficEnabled(boolean trafficEnabled) {
public void setBuildingsEnabled(boolean buildingsEnabled) {
this.buildingsEnabled = buildingsEnabled;
}

public void setMapStyle(@Nullable String style) {
if (googleMap == null) {
initialMapStyle = style;
} else {
updateMapStyle(style);
}
}

private boolean updateMapStyle(String style) {
// Dart passes an empty string to indicate that the style should be cleared.
final MapStyleOptions mapStyleOptions =
style == null || style.isEmpty() ? null : new MapStyleOptions(style);
final boolean set = Objects.requireNonNull(googleMap).setMapStyle(mapStyleOptions);
lastStyleError =
set ? null : "Unable to set the map style. Please check console logs for errors.";
return set;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

package io.flutter.plugins.googlemaps;

import androidx.annotation.Nullable;
import com.google.android.gms.maps.model.LatLngBounds;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -55,4 +56,6 @@ interface GoogleMapOptionsSink {
void setInitialCircles(Object initialCircles);

void setInitialTileOverlays(List<Map<String, ?>> initialTileOverlays);

void setMapStyle(@Nullable String style);
}
Loading