Skip to content

[google_maps_flutter_web] Adds missing MapOptions parameters. #4553

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

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from all 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
Copy link
Contributor

Choose a reason for hiding this comment

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

Why aren't there any actual changes in this package? Isn't the intent of the PR to allow clients of google_maps_flutter to configure these options?

Original file line number Diff line number Diff line change
Expand Up @@ -33,3 +33,8 @@ 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_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 @@ -32,3 +32,8 @@ flutter:
uses-material-design: true
assets:
- assets/

# FOR TESTING AND INITIAL REVIEW ONLY. DO NOT MERGE.
Copy link
Contributor

Choose a reason for hiding this comment

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

Please revert the changes to google_maps_flutter_android and google_maps_flutter_ios.

# See https://github.com/flutter/flutter/wiki/Contributing-to-Plugins-and-Packages#changing-federated-plugins
dependency_overrides:
{google_maps_flutter_platform_interface: {path: ../../../google_maps_flutter/google_maps_flutter_platform_interface}}
Original file line number Diff line number Diff line change
Expand Up @@ -32,3 +32,8 @@ 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_platform_interface: {path: ../../../../google_maps_flutter/google_maps_flutter_platform_interface}}
Original file line number Diff line number Diff line change
Expand Up @@ -32,3 +32,8 @@ 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_platform_interface: {path: ../../../../google_maps_flutter/google_maps_flutter_platform_interface}}
Original file line number Diff line number Diff line change
Expand Up @@ -32,3 +32,8 @@ 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_platform_interface: {path: ../../../../google_maps_flutter/google_maps_flutter_platform_interface}}
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,8 @@ dependencies:

flutter:
uses-material-design: true

# 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_platform_interface: {path: ../../../../../google_maps_flutter/google_maps_flutter_platform_interface}}
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 2.4.1
Copy link
Contributor

Choose a reason for hiding this comment

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

New functionality is a minor update, not a bugfix update.


* Adds `mapType`, `fullscreen` and `streetview` controls to google maps configuration.

## 2.4.0

* Adds options for gesture handling and tilt controls on web.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ class MapConfiguration {
this.indoorViewEnabled,
this.trafficEnabled,
this.buildingsEnabled,
this.mapTypeControl,
this.fullscreenControl,
this.streetViewControl,
this.cloudMapId,
});

Expand Down Expand Up @@ -107,6 +110,16 @@ class MapConfiguration {
/// True if 3D building display should be enabled.
final bool? buildingsEnabled;

/// True if the control to toggle between map types should be displayed.
final bool? mapTypeControl;
Copy link
Contributor

Choose a reason for hiding this comment

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

These should all end with Enabled, like the existing comparable properties.


/// True if the control to open the map on full screen should be displayed.
final bool? fullscreenControl;

/// True if the Pegman control, that lets the user to activate the street
/// view panorama, should be displayed.
final bool? streetViewControl;

/// Identifier that's associated with a specific cloud-based map style.
///
/// See https://developers.google.com/maps/documentation/get-map-id
Expand Down Expand Up @@ -173,6 +186,14 @@ class MapConfiguration {
trafficEnabled != other.trafficEnabled ? trafficEnabled : null,
buildingsEnabled:
buildingsEnabled != other.buildingsEnabled ? buildingsEnabled : null,
mapTypeControl:
mapTypeControl != other.mapTypeControl ? mapTypeControl : null,
fullscreenControl: fullscreenControl != other.fullscreenControl
? fullscreenControl
: null,
streetViewControl: streetViewControl != other.streetViewControl
? streetViewControl
: null,
cloudMapId: cloudMapId != other.cloudMapId ? cloudMapId : null,
);
}
Expand Down Expand Up @@ -205,6 +226,9 @@ class MapConfiguration {
indoorViewEnabled: diff.indoorViewEnabled ?? indoorViewEnabled,
trafficEnabled: diff.trafficEnabled ?? trafficEnabled,
buildingsEnabled: diff.buildingsEnabled ?? buildingsEnabled,
mapTypeControl: diff.mapTypeControl ?? mapTypeControl,
fullscreenControl: diff.fullscreenControl ?? fullscreenControl,
streetViewControl: diff.streetViewControl ?? streetViewControl,
cloudMapId: diff.cloudMapId ?? cloudMapId,
);
}
Expand All @@ -231,6 +255,9 @@ class MapConfiguration {
indoorViewEnabled == null &&
trafficEnabled == null &&
buildingsEnabled == null &&
mapTypeControl == null &&
fullscreenControl == null &&
streetViewControl == null &&
cloudMapId == null;

@override
Expand Down Expand Up @@ -262,6 +289,9 @@ class MapConfiguration {
indoorViewEnabled == other.indoorViewEnabled &&
trafficEnabled == other.trafficEnabled &&
buildingsEnabled == other.buildingsEnabled &&
mapTypeControl == other.mapTypeControl &&
fullscreenControl == other.fullscreenControl &&
streetViewControl == other.streetViewControl &&
cloudMapId == other.cloudMapId;
}

Expand All @@ -287,6 +317,9 @@ class MapConfiguration {
indoorViewEnabled,
trafficEnabled,
buildingsEnabled,
mapTypeControl,
fullscreenControl,
streetViewControl,
cloudMapId,
]);
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ repository: https://github.com/flutter/packages/tree/main/packages/google_maps_f
issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+maps%22
# NOTE: We strongly prefer non-breaking changes, even at the expense of a
# less-clean API. See https://flutter.dev/go/platform-interface-breaking-changes
version: 2.4.0
version: 2.4.1

environment:
sdk: ">=2.18.0 <4.0.0"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ void main() {
indoorViewEnabled: false,
trafficEnabled: false,
buildingsEnabled: false,
mapTypeControl: false,
fullscreenControl: false,
streetViewControl: false,
);

test('only include changed fields', () async {
Expand Down Expand Up @@ -393,6 +396,54 @@ void main() {
expect(empty.hashCode, isNot(diff.hashCode));
});

test('handle mapTypeControl', () async {
const MapConfiguration diff = MapConfiguration(mapTypeControl: true);

const MapConfiguration empty = MapConfiguration();
final MapConfiguration updated = diffBase.applyDiff(diff);

// A diff applied to empty options should be the diff itself.
expect(empty.applyDiff(diff), diff);
// The diff from empty options should be the diff itself.
expect(diff.diffFrom(empty), diff);
// A diff applied to non-empty options should update that field.
expect(updated.mapTypeControl, true);
// The hash code should change.
expect(empty.hashCode, isNot(diff.hashCode));
});

test('handle fullscreenControl', () async {
const MapConfiguration diff = MapConfiguration(fullscreenControl: true);

const MapConfiguration empty = MapConfiguration();
final MapConfiguration updated = diffBase.applyDiff(diff);

// A diff applied to empty options should be the diff itself.
expect(empty.applyDiff(diff), diff);
// The diff from empty options should be the diff itself.
expect(diff.diffFrom(empty), diff);
// A diff applied to non-empty options should update that field.
expect(updated.fullscreenControl, true);
// The hash code should change.
expect(empty.hashCode, isNot(diff.hashCode));
});

test('handle streetViewControl', () async {
const MapConfiguration diff = MapConfiguration(streetViewControl: true);

const MapConfiguration empty = MapConfiguration();
final MapConfiguration updated = diffBase.applyDiff(diff);

// A diff applied to empty options should be the diff itself.
expect(empty.applyDiff(diff), diff);
// The diff from empty options should be the diff itself.
expect(diff.diffFrom(empty), diff);
// A diff applied to non-empty options should update that field.
expect(updated.streetViewControl, true);
// The hash code should change.
expect(empty.hashCode, isNot(diff.hashCode));
});

test('handle cloudMapId', () async {
const MapConfiguration diff = MapConfiguration(cloudMapId: _kCloudMapId);

Expand Down Expand Up @@ -536,6 +587,24 @@ void main() {
expect(diff.isEmpty, false);
});

test('is false with mapTypeControl', () async {
const MapConfiguration diff = MapConfiguration(mapTypeControl: true);

expect(diff.isEmpty, false);
});

test('is false with fullscreenControl', () async {
const MapConfiguration diff = MapConfiguration(fullscreenControl: true);

expect(diff.isEmpty, false);
});

test('is false with streetViewControl', () async {
const MapConfiguration diff = MapConfiguration(streetViewControl: true);

expect(diff.isEmpty, false);
});

test('is false with cloudMapId', () async {
const MapConfiguration diff = MapConfiguration(cloudMapId: _kCloudMapId);

Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 0.5.2+1

* Adds `mapType`, `fullscreen` and `streetview` controls to google maps configuration.

## 0.5.2

* Adds options for gesture handling and tilt controls.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -403,6 +403,30 @@ void main() {
expect(capturedOptions!.tilt, 0);
});

testWidgets(
'translates mapTypeControl, fullscreenControl, streetViewControl configurations',
Copy link
Contributor

Choose a reason for hiding this comment

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

Three unrelated things should not be tested in the same test.

(WidgetTester tester) async {
controller = createController(
mapConfiguration: const MapConfiguration(
mapTypeControl: true,
fullscreenControl: true,
streetViewControl: true,
));

controller.debugSetOverrides(
createMap: (_, gmaps.MapOptions options) {
capturedOptions = options;
return map;
});

controller.init();

expect(capturedOptions, isNotNull);
expect(capturedOptions!.mapTypeControl, true);
expect(capturedOptions!.fullscreenControl, true);
expect(capturedOptions!.streetViewControl, true);
});

testWidgets('translates fortyFiveDegreeImageryEnabled option',
(WidgetTester tester) async {
controller = createController(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,15 @@ dev_dependencies:
sdk: flutter
mockito: 5.4.1


# 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:

# Override the google_maps_flutter dependency on google_maps_flutter_web.
# TODO(ditman): Unwind the circular dependency. This will create problems
# if we need to make a breaking change to google_maps_flutter_web.
google_maps_flutter_platform_interface:
path: ../../../google_maps_flutter/google_maps_flutter_platform_interface
google_maps_flutter_web:
path: ../
path: ../../../google_maps_flutter/google_maps_flutter_web
Original file line number Diff line number Diff line change
Expand Up @@ -74,11 +74,9 @@ gmaps.MapOptions _configurationAndStyleToGmapsOptions(
options.rotateControl = configuration.fortyFiveDegreeImageryEnabled;
}

// These don't have any configuration entries, but they seem to be off in the
// native maps.
options.mapTypeControl = false;
options.fullscreenControl = false;
options.streetViewControl = false;
options.mapTypeControl = configuration.mapTypeControl ?? false;
options.fullscreenControl = configuration.fullscreenControl ?? false;
options.streetViewControl = configuration.streetViewControl ?? false;

options.styles = styles;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ name: google_maps_flutter_web
description: Web platform implementation of google_maps_flutter
repository: https://github.com/flutter/packages/tree/main/packages/google_maps_flutter/google_maps_flutter_web
issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+maps%22
version: 0.5.2
version: 0.5.2+1
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here; this should be 0.5.3.


environment:
sdk: ">=2.18.0 <4.0.0"
Expand Down Expand Up @@ -33,3 +33,8 @@ dev_dependencies:
# 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_platform_interface: {path: ../../google_maps_flutter/google_maps_flutter_platform_interface}}