Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Prev Previous commit
Next Next commit
web implementation
  • Loading branch information
stuartmorgan-g committed Feb 23, 2024
commit 7721166a65fd60a03e93d672612a0a13ce56ff47
Original file line number Diff line number Diff line change
Expand Up @@ -444,6 +444,46 @@ void main() {
expect(capturedOptions!.zoom, 12);
expect(capturedOptions!.center, isNotNull);
});

testWidgets('translates style option', (WidgetTester tester) async {
const String style = '''
[{
"featureType": "poi.park",
"elementType": "labels.text.fill",
"stylers": [{"color": "#6b9a76"}]
}]''';
controller = createController(
mapConfiguration: const MapConfiguration(style: style));
controller.debugSetOverrides(
createMap: (_, gmaps.MapOptions options) {
capturedOptions = options;
return map;
});

controller.init();

expect(capturedOptions, isNotNull);
expect(capturedOptions!.styles?.length, 1);
});

testWidgets('stores style errors for later querying',
(WidgetTester tester) async {
controller = createController(
mapConfiguration: const MapConfiguration(
style: '[[invalid style', zoomControlsEnabled: true));
controller.debugSetOverrides(
createMap: (_, gmaps.MapOptions options) {
capturedOptions = options;
return map;
});

controller.init();

expect(controller.lastStyleError, isNotNull);
// Style failures should not prevent other options from being set.
expect(capturedOptions, isNotNull);
expect(capturedOptions!.zoomControl, true);
});
});

group('Traffic Layer', () {
Expand Down Expand Up @@ -481,7 +521,7 @@ void main() {
..init();
});

group('updateRawOptions', () {
group('updateMapConfiguration', () {
testWidgets('can update `options`', (WidgetTester tester) async {
controller.updateMapConfiguration(const MapConfiguration(
mapType: MapType.satellite,
Expand All @@ -505,6 +545,27 @@ void main() {

expect(controller.trafficLayer, isNull);
});

testWidgets('can update style', (WidgetTester tester) async {
const String style = '''
[{
"featureType": "poi.park",
"elementType": "labels.text.fill",
"stylers": [{"color": "#6b9a76"}]
}]''';
controller
.updateMapConfiguration(const MapConfiguration(style: style));

expect(controller.styles.length, 1);
});

testWidgets('can update style', (WidgetTester tester) async {
controller.updateMapConfiguration(
const MapConfiguration(style: '[[[invalid style'));

expect(controller.styles, isEmpty);
expect(controller.lastStyleError, isNotNull);
});
});

group('viewport getters', () {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ gmaps.MapOptions _configurationAndStyleToGmapsOptions(
options.fullscreenControl = false;
options.streetViewControl = false;

// See updateMapConfiguration for why this is not using configuration.style.
options.styles = styles;

options.mapId = configuration.cloudMapId;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ class GoogleMapController {
_polylinesController = PolylinesController(stream: _streamController);
_markersController = MarkersController(stream: _streamController);
_tileOverlaysController = TileOverlaysController();
_updateStylesFromConfiguration(mapConfiguration);

// Register the view factory that will hold the `_div` that holds the map in the DOM.
// The `_div` needs to be created outside of the ViewFactory (and cached!) so we can
Expand Down Expand Up @@ -60,6 +61,8 @@ class GoogleMapController {
// Caching this allows us to re-create the map faithfully when needed.
MapConfiguration _lastMapConfiguration = const MapConfiguration();
List<gmaps.MapTypeStyle> _lastStyles = const <gmaps.MapTypeStyle>[];
// The last error resulting from providing a map style, if any.
String? _lastStyleError;

/// Configuration accessor for the [GoogleMapsInspectorWeb].
///
Expand Down Expand Up @@ -279,15 +282,36 @@ class GoogleMapController {
return _lastMapConfiguration;
}

// TODO(stuartmorgan): Refactor so that _lastMapConfiguration.style is the
// source of truth for style info. Currently it's tracked and handled
// separately since style didn't used to be part of the configuration.
List<gmaps.MapTypeStyle> _updateStylesFromConfiguration(
MapConfiguration update) {
if (update.style != null) {
// Provide async access to the error rather than throwing, to match the
// behavior of other platforms where there's no mechanism to return errors
// from configuration updates.
try {
_lastStyles = _mapStyles(update.style);
Copy link
Member

Choose a reason for hiding this comment

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

Do we want a test that asserts that _lastStyles remain unchanged when update.style is wrong? (not sure if we want that to be a feature or is a side-effect of this particular implementation)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That does seem worth testing. I couldn't see a way to add it without adding another kind of ugly test hook, so I did that; let me know if you have a better option. (Long term we may want to wrap that opaque maps object instead of having to add these hooks at this layer.)

Copy link
Member

Choose a reason for hiding this comment

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

The new hook is fine. There seems to exist a way to fetch styles from the gmaps.GMap object through its MVCObject interface:

map.get('styles')

(get DartDocs)

But I'm not sure if whatever that returns is directly comparable with what we pass in!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We shouldn't need to compare with what we pass in, just the before/after, but I'm getting null from that getter. Maybe this is related to needing to pump things differently in the maps integration tests?

For now I'm just going to land this since it's a pattern we already have, and we can revisit the structure of these tests later if necessary.

_lastStyleError = null;
} on MapStyleException catch (e) {
_lastStyleError = e.cause;
}
}
return _lastStyles;
}

/// Updates the map options from a [MapConfiguration].
///
/// This method converts the map into the proper [gmaps.MapOptions].
void updateMapConfiguration(MapConfiguration update) {
assert(_googleMap != null, 'Cannot update options on a null map.');

final List<gmaps.MapTypeStyle> styles =
_updateStylesFromConfiguration(update);
final MapConfiguration newConfiguration = _mergeConfigurations(update);
final gmaps.MapOptions newOptions =
_configurationAndStyleToGmapsOptions(newConfiguration, _lastStyles);
_configurationAndStyleToGmapsOptions(newConfiguration, styles);

_setOptions(newOptions);
_setTrafficLayer(_googleMap!, newConfiguration.trafficEnabled ?? false);
Expand All @@ -300,6 +324,13 @@ class GoogleMapController {
_configurationAndStyleToGmapsOptions(_lastMapConfiguration, styles));
}

/// A getter for the current styles. Only for tests.
@visibleForTesting
List<gmaps.MapTypeStyle> get styles => _lastStyles;

/// Returns the last error from setting the map's style, if any.
String? get lastStyleError => _lastStyleError;

// Sets new [gmaps.MapOptions] on the wrapped map.
// ignore: use_setters_to_change_properties
void _setOptions(gmaps.MapOptions options) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,11 @@ class GoogleMapsPlugin extends GoogleMapsFlutterPlatform {
return _events(mapId).whereType<MapLongPressEvent>();
}

@override
Future<String?> getStyleError({required int mapId}) async {
return _map(mapId).lastStyleError;
}

/// Disposes of the current map. It can't be used afterwards!
@override
void dispose({required int mapId}) {
Expand Down