From 544002d3d3d0b1077c26b7b3e12c2cbaa30eb40c Mon Sep 17 00:00:00 2001 From: Ross Wang Date: Tue, 22 Nov 2022 19:29:05 -0800 Subject: [PATCH 1/6] [google_maps_flutter_web] Allow marker position updates Unconditionally convert the current marker position in `convert.dart:_markerOptionsFromMarker`, to allow for position updates. Also adds position changes to `marker_test.dart/MarkerController/update` and `markers_test.dart/MarkersController/changeMarkers`. The `MarkersController` case is fixed by this patch. --- .../google_maps_flutter_web/CHANGELOG.md | 3 ++- .../example/integration_test/marker_test.dart | 5 ++++- .../example/integration_test/markers_test.dart | 14 ++++++++++---- .../google_maps_flutter_web/lib/src/convert.dart | 10 ++++------ .../google_maps_flutter_web/pubspec.yaml | 2 +- 5 files changed, 21 insertions(+), 13 deletions(-) diff --git a/packages/google_maps_flutter/google_maps_flutter_web/CHANGELOG.md b/packages/google_maps_flutter/google_maps_flutter_web/CHANGELOG.md index 0a712b44eba..077d012e5b8 100644 --- a/packages/google_maps_flutter/google_maps_flutter_web/CHANGELOG.md +++ b/packages/google_maps_flutter/google_maps_flutter_web/CHANGELOG.md @@ -1,6 +1,7 @@ -## NEXT +## 0.4.0+8 * Updates minimum Flutter version to 3.3. +* Allows marker position updates. Issue [#83467](https://github.com/flutter/flutter/issues/83467). ## 0.4.0+7 diff --git a/packages/google_maps_flutter/google_maps_flutter_web/example/integration_test/marker_test.dart b/packages/google_maps_flutter/google_maps_flutter_web/example/integration_test/marker_test.dart index 6591b0ca08d..2e2d77b71de 100644 --- a/packages/google_maps_flutter/google_maps_flutter_web/example/integration_test/marker_test.dart +++ b/packages/google_maps_flutter/google_maps_flutter_web/example/integration_test/marker_test.dart @@ -100,13 +100,16 @@ void main() { testWidgets('update', (WidgetTester tester) async { final MarkerController controller = MarkerController(marker: marker); final gmaps.MarkerOptions options = gmaps.MarkerOptions() - ..draggable = true; + ..draggable = true + ..position = gmaps.LatLng(42, 54); expect(marker.draggable, isNull); controller.update(options); expect(marker.draggable, isTrue); + expect(marker.position?.lat, equals(42)); + expect(marker.position?.lng, equals(54)); }); testWidgets('infoWindow null, showInfoWindow.', diff --git a/packages/google_maps_flutter/google_maps_flutter_web/example/integration_test/markers_test.dart b/packages/google_maps_flutter/google_maps_flutter_web/example/integration_test/markers_test.dart index e4c4dd7c0cf..e997b07f30d 100644 --- a/packages/google_maps_flutter/google_maps_flutter_web/example/integration_test/markers_test.dart +++ b/packages/google_maps_flutter/google_maps_flutter_web/example/integration_test/markers_test.dart @@ -55,15 +55,21 @@ void main() { expect( controller.markers[const MarkerId('1')]?.marker?.draggable, isFalse); - // Update the marker with radius 10 + // Update the marker with draggability and position final Set updatedMarkers = { - const Marker(markerId: MarkerId('1'), draggable: true), + const Marker( + markerId: MarkerId('1'), + draggable: true, + position: LatLng(42, 54), + ), }; controller.changeMarkers(updatedMarkers); expect(controller.markers.length, 1); - expect( - controller.markers[const MarkerId('1')]?.marker?.draggable, isTrue); + final marker = controller.markers[const MarkerId('1')]?.marker; + expect(marker?.draggable, isTrue); + expect(marker?.position?.lat, equals(42)); + expect(marker?.position?.lng, equals(54)); }); testWidgets('removeMarkers', (WidgetTester tester) async { diff --git a/packages/google_maps_flutter/google_maps_flutter_web/lib/src/convert.dart b/packages/google_maps_flutter/google_maps_flutter_web/lib/src/convert.dart index 25cba849475..dcce8d35699 100644 --- a/packages/google_maps_flutter/google_maps_flutter_web/lib/src/convert.dart +++ b/packages/google_maps_flutter/google_maps_flutter_web/lib/src/convert.dart @@ -290,17 +290,15 @@ gmaps.Icon? _gmIconFromBitmapDescriptor(BitmapDescriptor bitmapDescriptor) { // Computes the options for a new [gmaps.Marker] from an incoming set of options // [marker], and the existing marker registered with the map: [currentMarker]. -// Preserves the position from the [currentMarker], if set. gmaps.MarkerOptions _markerOptionsFromMarker( Marker marker, gmaps.Marker? currentMarker, ) { return gmaps.MarkerOptions() - ..position = currentMarker?.position ?? - gmaps.LatLng( - marker.position.latitude, - marker.position.longitude, - ) + ..position = gmaps.LatLng( + marker.position.latitude, + marker.position.longitude, + ) ..title = sanitizeHtml(marker.infoWindow.title ?? '') ..zIndex = marker.zIndex ..visible = marker.visible diff --git a/packages/google_maps_flutter/google_maps_flutter_web/pubspec.yaml b/packages/google_maps_flutter/google_maps_flutter_web/pubspec.yaml index 83f4971a268..67df16f0016 100644 --- a/packages/google_maps_flutter/google_maps_flutter_web/pubspec.yaml +++ b/packages/google_maps_flutter/google_maps_flutter_web/pubspec.yaml @@ -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.4.0+7 +version: 0.4.0+8 environment: sdk: ">=2.18.0 <4.0.0" From 3423b739b23011b214101b62c9296c15c4c4f290 Mon Sep 17 00:00:00 2001 From: David Iglesias Teixeira Date: Wed, 12 Apr 2023 15:30:48 -0700 Subject: [PATCH 2/6] Make position update optional. Add tests. Fix analyzer. --- .../integration_test/markers_test.dart | 106 ++++++++++++++++-- .../lib/src/convert.dart | 19 +++- 2 files changed, 114 insertions(+), 11 deletions(-) diff --git a/packages/google_maps_flutter/google_maps_flutter_web/example/integration_test/markers_test.dart b/packages/google_maps_flutter/google_maps_flutter_web/example/integration_test/markers_test.dart index e997b07f30d..f1d7f0e957f 100644 --- a/packages/google_maps_flutter/google_maps_flutter_web/example/integration_test/markers_test.dart +++ b/packages/google_maps_flutter/google_maps_flutter_web/example/integration_test/markers_test.dart @@ -47,15 +47,19 @@ void main() { }); testWidgets('changeMarkers', (WidgetTester tester) async { + gmaps.Marker? marker; + final Set markers = { const Marker(markerId: MarkerId('1')), }; controller.addMarkers(markers); - expect( - controller.markers[const MarkerId('1')]?.marker?.draggable, isFalse); + marker = controller.markers[const MarkerId('1')]?.marker; + expect(marker, isNotNull); + expect(marker!.draggable, isFalse); + expect(marker.position, isNull); - // Update the marker with draggability and position + // Update the marker with draggable and position final Set updatedMarkers = { const Marker( markerId: MarkerId('1'), @@ -64,12 +68,100 @@ void main() { ), }; controller.changeMarkers(updatedMarkers); + expect(controller.markers.length, 1); + + marker = controller.markers[const MarkerId('1')]?.marker; + expect(marker, isNotNull); + expect(marker!.draggable, isTrue); + + final gmaps.LatLng? position = marker.position; + expect(position, isNotNull); + expect(position!.lat, equals(42)); + expect(position.lng, equals(54)); + }); + + testWidgets( + 'changeMarkers can update a marker without resetting its position', + (WidgetTester tester) async { + gmaps.Marker? marker; + gmaps.LatLng? position; + + final Set markers = { + const Marker( + markerId: MarkerId('1'), + position: LatLng(42, 54), + ), + }; + controller.addMarkers(markers); + + marker = controller.markers[const MarkerId('1')]?.marker; + expect(marker, isNotNull); + expect(marker!.draggable, isFalse); + + position = marker.position; + expect(position, isNotNull); + expect(position!.lat, equals(42)); + expect(position.lng, equals(54)); + + // Update the marker without position + final Set updatedMarkers = { + const Marker( + markerId: MarkerId('1'), + draggable: true, + ), + }; + controller.changeMarkers(updatedMarkers); + expect(controller.markers.length, 1); + + marker = controller.markers[const MarkerId('1')]?.marker; + expect(marker, isNotNull); + expect(marker!.draggable, isTrue); + + position = marker.position; + expect(position, isNotNull); + expect(position!.lat, equals(42)); + expect(position.lng, equals(54)); + }); + + testWidgets( + 'changeMarkers can place a marker really close to Null Island', + (WidgetTester tester) async { + gmaps.Marker? marker; + gmaps.LatLng? position; + final Set markers = { + const Marker( + markerId: MarkerId('1'), + position: LatLng(42, 54), + ), + }; + controller.addMarkers(markers); + + marker = controller.markers[const MarkerId('1')]?.marker; + expect(marker, isNotNull); + + position = marker!.position; + expect(position, isNotNull); + expect(position!.lat, equals(42)); + expect(position.lng, equals(54)); + + // Update the marker without position + final Set updatedMarkers = { + const Marker( + markerId: MarkerId('1'), + position: LatLng(0, 1e-10), + ), + }; + controller.changeMarkers(updatedMarkers); expect(controller.markers.length, 1); - final marker = controller.markers[const MarkerId('1')]?.marker; - expect(marker?.draggable, isTrue); - expect(marker?.position?.lat, equals(42)); - expect(marker?.position?.lng, equals(54)); + + marker = controller.markers[const MarkerId('1')]?.marker; + expect(marker, isNotNull); + + position = marker!.position; + expect(position, isNotNull); + expect(position!.lat, equals(0)); + expect(position.lng, moreOrLessEquals(0)); }); testWidgets('removeMarkers', (WidgetTester tester) async { diff --git a/packages/google_maps_flutter/google_maps_flutter_web/lib/src/convert.dart b/packages/google_maps_flutter/google_maps_flutter_web/lib/src/convert.dart index dcce8d35699..e8b88f0b8a0 100644 --- a/packages/google_maps_flutter/google_maps_flutter_web/lib/src/convert.dart +++ b/packages/google_maps_flutter/google_maps_flutter_web/lib/src/convert.dart @@ -294,11 +294,22 @@ gmaps.MarkerOptions _markerOptionsFromMarker( Marker marker, gmaps.Marker? currentMarker, ) { + // The incoming Marker position always defaults to LatLng(0,0), so it's + // not possible to distinguish if users want to update the position of the + // marker or not. + // Since "Null Island" is an unlikely position for anything to happen, we + // assume that LatLng(0,0) means "do not change the current position", so + // Marker updates don't need to always specify a location. + // A workaround to position a Marker on "Null Island" is to pass an + // non-exactly-zero value (like Number.EPSILON) to either lat/lng of the + // update. + gmaps.LatLng? newPosition = + (marker.position.latitude != 0 || marker.position.longitude != 0) + ? gmaps.LatLng(marker.position.latitude, marker.position.longitude) + : currentMarker?.position; + return gmaps.MarkerOptions() - ..position = gmaps.LatLng( - marker.position.latitude, - marker.position.longitude, - ) + ..position = newPosition ..title = sanitizeHtml(marker.infoWindow.title ?? '') ..zIndex = marker.zIndex ..visible = marker.visible From d5117dd2e1f3fd1c3ec0f528bd1cc4ac139eedf2 Mon Sep 17 00:00:00 2001 From: David Iglesias Teixeira Date: Wed, 12 Apr 2023 15:35:59 -0700 Subject: [PATCH 3/6] Make variable final. --- .../google_maps_flutter_web/lib/src/convert.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/google_maps_flutter/google_maps_flutter_web/lib/src/convert.dart b/packages/google_maps_flutter/google_maps_flutter_web/lib/src/convert.dart index e8b88f0b8a0..3afe23904a4 100644 --- a/packages/google_maps_flutter/google_maps_flutter_web/lib/src/convert.dart +++ b/packages/google_maps_flutter/google_maps_flutter_web/lib/src/convert.dart @@ -303,7 +303,7 @@ gmaps.MarkerOptions _markerOptionsFromMarker( // A workaround to position a Marker on "Null Island" is to pass an // non-exactly-zero value (like Number.EPSILON) to either lat/lng of the // update. - gmaps.LatLng? newPosition = + final gmaps.LatLng? newPosition = (marker.position.latitude != 0 || marker.position.longitude != 0) ? gmaps.LatLng(marker.position.latitude, marker.position.longitude) : currentMarker?.position; From 9e5181eab8be91100b82aa379a08e432da38b788 Mon Sep 17 00:00:00 2001 From: David Iglesias Teixeira Date: Wed, 12 Apr 2023 15:36:37 -0700 Subject: [PATCH 4/6] dart format . --- .../example/integration_test/markers_test.dart | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/google_maps_flutter/google_maps_flutter_web/example/integration_test/markers_test.dart b/packages/google_maps_flutter/google_maps_flutter_web/example/integration_test/markers_test.dart index f1d7f0e957f..0ae62567084 100644 --- a/packages/google_maps_flutter/google_maps_flutter_web/example/integration_test/markers_test.dart +++ b/packages/google_maps_flutter/google_maps_flutter_web/example/integration_test/markers_test.dart @@ -123,8 +123,7 @@ void main() { expect(position.lng, equals(54)); }); - testWidgets( - 'changeMarkers can place a marker really close to Null Island', + testWidgets('changeMarkers can place a marker really close to Null Island', (WidgetTester tester) async { gmaps.Marker? marker; gmaps.LatLng? position; From cead485326251d4f2d9edf4e1916905b732ece45 Mon Sep 17 00:00:00 2001 From: David Iglesias Teixeira Date: Tue, 18 Apr 2023 17:00:20 -0700 Subject: [PATCH 5/6] Do not attempt to preserve marker position across updates at all. --- .../integration_test/markers_test.dart | 54 ++++--------------- .../lib/src/convert.dart | 19 ++----- 2 files changed, 14 insertions(+), 59 deletions(-) diff --git a/packages/google_maps_flutter/google_maps_flutter_web/example/integration_test/markers_test.dart b/packages/google_maps_flutter/google_maps_flutter_web/example/integration_test/markers_test.dart index 0ae62567084..6a264064a3c 100644 --- a/packages/google_maps_flutter/google_maps_flutter_web/example/integration_test/markers_test.dart +++ b/packages/google_maps_flutter/google_maps_flutter_web/example/integration_test/markers_test.dart @@ -48,6 +48,7 @@ void main() { testWidgets('changeMarkers', (WidgetTester tester) async { gmaps.Marker? marker; + gmaps.LatLng? position; final Set markers = { const Marker(markerId: MarkerId('1')), @@ -57,7 +58,12 @@ void main() { marker = controller.markers[const MarkerId('1')]?.marker; expect(marker, isNotNull); expect(marker!.draggable, isFalse); - expect(marker.position, isNull); + + // By default, markers fall in LatLng(0, 0) + position = marker.position; + expect(position, isNotNull); + expect(position!.lat, equals(0)); + expect(position.lng, equals(0)); // Update the marker with draggable and position final Set updatedMarkers = { @@ -74,14 +80,14 @@ void main() { expect(marker, isNotNull); expect(marker!.draggable, isTrue); - final gmaps.LatLng? position = marker.position; + position = marker.position; expect(position, isNotNull); expect(position!.lat, equals(42)); expect(position.lng, equals(54)); }); testWidgets( - 'changeMarkers can update a marker without resetting its position', + 'changeMarkers resets marker position if not passed when updating!', (WidgetTester tester) async { gmaps.Marker? marker; gmaps.LatLng? position; @@ -119,48 +125,8 @@ void main() { position = marker.position; expect(position, isNotNull); - expect(position!.lat, equals(42)); - expect(position.lng, equals(54)); - }); - - testWidgets('changeMarkers can place a marker really close to Null Island', - (WidgetTester tester) async { - gmaps.Marker? marker; - gmaps.LatLng? position; - - final Set markers = { - const Marker( - markerId: MarkerId('1'), - position: LatLng(42, 54), - ), - }; - controller.addMarkers(markers); - - marker = controller.markers[const MarkerId('1')]?.marker; - expect(marker, isNotNull); - - position = marker!.position; - expect(position, isNotNull); - expect(position!.lat, equals(42)); - expect(position.lng, equals(54)); - - // Update the marker without position - final Set updatedMarkers = { - const Marker( - markerId: MarkerId('1'), - position: LatLng(0, 1e-10), - ), - }; - controller.changeMarkers(updatedMarkers); - expect(controller.markers.length, 1); - - marker = controller.markers[const MarkerId('1')]?.marker; - expect(marker, isNotNull); - - position = marker!.position; - expect(position, isNotNull); expect(position!.lat, equals(0)); - expect(position.lng, moreOrLessEquals(0)); + expect(position.lng, equals(0)); }); testWidgets('removeMarkers', (WidgetTester tester) async { diff --git a/packages/google_maps_flutter/google_maps_flutter_web/lib/src/convert.dart b/packages/google_maps_flutter/google_maps_flutter_web/lib/src/convert.dart index 3afe23904a4..dcce8d35699 100644 --- a/packages/google_maps_flutter/google_maps_flutter_web/lib/src/convert.dart +++ b/packages/google_maps_flutter/google_maps_flutter_web/lib/src/convert.dart @@ -294,22 +294,11 @@ gmaps.MarkerOptions _markerOptionsFromMarker( Marker marker, gmaps.Marker? currentMarker, ) { - // The incoming Marker position always defaults to LatLng(0,0), so it's - // not possible to distinguish if users want to update the position of the - // marker or not. - // Since "Null Island" is an unlikely position for anything to happen, we - // assume that LatLng(0,0) means "do not change the current position", so - // Marker updates don't need to always specify a location. - // A workaround to position a Marker on "Null Island" is to pass an - // non-exactly-zero value (like Number.EPSILON) to either lat/lng of the - // update. - final gmaps.LatLng? newPosition = - (marker.position.latitude != 0 || marker.position.longitude != 0) - ? gmaps.LatLng(marker.position.latitude, marker.position.longitude) - : currentMarker?.position; - return gmaps.MarkerOptions() - ..position = newPosition + ..position = gmaps.LatLng( + marker.position.latitude, + marker.position.longitude, + ) ..title = sanitizeHtml(marker.infoWindow.title ?? '') ..zIndex = marker.zIndex ..visible = marker.visible From 4c3119c68f580ff2506b9aa89b20b9bcdad3ca1c Mon Sep 17 00:00:00 2001 From: David Iglesias Teixeira Date: Tue, 18 Apr 2023 17:00:43 -0700 Subject: [PATCH 6/6] Use dart run instead of the deprecated version. --- .../google_maps_flutter_web/example/regen_mocks.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/google_maps_flutter/google_maps_flutter_web/example/regen_mocks.sh b/packages/google_maps_flutter/google_maps_flutter_web/example/regen_mocks.sh index 78bcdc0f9e2..b3dc17c12c2 100755 --- a/packages/google_maps_flutter/google_maps_flutter_web/example/regen_mocks.sh +++ b/packages/google_maps_flutter/google_maps_flutter_web/example/regen_mocks.sh @@ -7,4 +7,4 @@ flutter pub get echo "(Re)generating mocks." -flutter pub run build_runner build --delete-conflicting-outputs +dart run build_runner build --delete-conflicting-outputs