Skip to content

[google_maps_flutter_platform_(web/android/ios)] Add a new zIndexInt param to marker and deprecate zIndex #9408

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.

Sign up for GitHub

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

Merged
auto-submit merged 11 commits into from
Jun 16, 2025
Merged

[google_maps_flutter_platform_(web/android/ios)] Add a new zIndexInt param to marker and deprecate zIndex #9408

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
9ec71a6
All platform implementation changes from the main PR
Hari-07 Jun 9, 2025
2b3db64
Fix quotes on flutter version string
Hari-07 Jun 9, 2025
fd18f22
Another quotes fix
Hari-07 Jun 9, 2025
5fdb91b
Use build number for web package
Hari-07 Jun 9, 2025
26b6037
Remove extension in web marker conversion
Hari-07 Jun 9, 2025
f7726bf
Change zIndex on ios to integer field
Hari-07 Jun 10, 2025
d619269
Revert android version change
Hari-07 Jun 10, 2025
378b108
Add an explicit cast
Hari-07 Jun 10, 2025
9ab5e86
Remove android changelog
Hari-07 Jun 13, 2025
1824936
Merge branch 'main' of github.com:flutter/packages into maps-zindex-t…
Hari-07 Jun 13, 2025
4ad3e6e
Update ios version
Hari-07 Jun 13, 2025
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
4 changes: 4 additions & 0 deletions packages/google_maps_flutter/google_maps_flutter_android/CHANGELOG.md
View file
Open in desktop

@stuartmorgan-g stuartmorgan-g Jun 12, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The changes to this file can be reverted as well; we don't need to CHANGELOG an integration test comment.

Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 2.16.2

* Deprecates `zIndex` parameter in Marker in favor of `zIndexInt`.

## 2.16.1

* Removes obsolete code related to supporting SDK <21.
Expand Down
1 change: 1 addition & 0 deletions ..._maps_flutter/google_maps_flutter_android/example/integration_test/google_maps_tests.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2009,6 +2009,7 @@ Marker _copyMarkerWithClusterManagerId(
position: marker.position,
rotation: marker.rotation,
visible: marker.visible,
// The deprecated parameter is used here to avoid losing precision.
zIndex: marker.zIndex,
onTap: marker.onTap,
onDragStart: marker.onDragStart,
Expand Down
4 changes: 2 additions & 2 deletions packages/google_maps_flutter/google_maps_flutter_android/example/pubspec.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ publish_to: none

environment:
sdk: ^3.6.0
flutter: ">=3.27.0"
flutter: '>=3.27.0'

@stuartmorgan-g stuartmorgan-g Jun 9, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why does this PR change the style of all the string values in pubspec.yaml files to deviate from our usual style?

@Hari-07 Hari-07 Jun 9, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I assume there might be something wrong with my linte, will fix that. I had to do it manually for the interface package


dependencies:
cupertino_icons: ^1.0.5
Expand All @@ -18,7 +18,7 @@ dependencies:
# The example app is bundled with the plugin so we use a path dependency on
# the parent directory to use the current plugin's version.
path: ../
google_maps_flutter_platform_interface: ^2.11.0
google_maps_flutter_platform_interface: ^2.12.1

dev_dependencies:
build_runner: ^2.1.10
Expand Down
8 changes: 4 additions & 4 deletions packages/google_maps_flutter/google_maps_flutter_android/pubspec.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@ name: google_maps_flutter_android
description: Android implementation of the google_maps_flutter plugin.
repository: https://github.com/flutter/packages/tree/main/packages/google_maps_flutter/google_maps_flutter_android
issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+maps%22
version: 2.16.1
version: 2.16.2

environment:
sdk: ^3.6.0
flutter: ">=3.27.0"
flutter: '>=3.27.0'

flutter:
plugin:
Expand All @@ -21,7 +21,7 @@ dependencies:
flutter:
sdk: flutter
flutter_plugin_android_lifecycle: ^2.0.1
google_maps_flutter_platform_interface: ^2.11.0
google_maps_flutter_platform_interface: ^2.12.1

@stuartmorgan-g stuartmorgan-g Jun 10, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looking at the PR again... do we actually need any of the changes to this file? It looks like the only actual change is to the integration tests and the example pubspec, and those don't require publishing the package.

@Hari-07 Hari-07 Jun 10, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

In that case Im gonna close this PR. And since it doesn't need a new version, the main PR's overrides can all be removed.

@Hari-07 Hari-07 Jun 10, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Doesn't it need to target the updated platform interface package though? Not sure if Im misunderstanding this line from the federated packages section

If new functionality is being added to the API surface of the app-facing package, be sure to update the version constraints of the implementation packages in its pubspec.yaml

@stuartmorgan-g stuartmorgan-g Jun 10, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Doesn't it need to target the updated platform interface package though?

What is "it" here? If it's google_maps_flutter_android, what part of the google_maps_flutter_android requires the new APIs added to the platform interface?

If it's the app-facing package (in the other PR), yes, because it's exporting that surface to clients.

Not sure if Im misunderstanding this line from the federated packages section

If new functionality is being added to the API surface of the app-facing package, be sure to update the version constraints of the implementation packages in its pubspec.yaml

See the explanation in the following sentence for the reason this would need to be done.

For example, if the platform interface adds a new Future<void> foo() { throw UnimplementedError(); }, and all the platforms are updated to implement foo(), and the app-facing package is updated to call foo(), we don't want a valid set of resolved packages where clients can get the updated version of the app-facing and platform interface packages, but not the implementation packages, such that everything compiles but all calls to foo() throw.

@Hari-07 Hari-07 Jun 10, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ok got it. For android we dont't need a new version. I guess even web doesn't need a new version by the same logic since that's just a comment change?

@stuartmorgan-g stuartmorgan-g Jun 10, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I guess even web doesn't need a new version by the same logic since that's just a comment change?

Correct; IIRC the tooling will do the right thing with a comment-only change, but if not we can override the version check.

stream_transform: ^2.0.0

dev_dependencies:
Expand All @@ -30,7 +30,7 @@ dev_dependencies:
flutter_test:
sdk: flutter
mockito: ^5.4.4
pigeon: ^22.6.0
pigeon: ^22.7.4
plugin_platform_interface: ^2.1.7

topics:
Expand Down
3 changes: 2 additions & 1 deletion packages/google_maps_flutter/google_maps_flutter_ios/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
## NEXT
## 2.15.3

* Deprecates `zIndex` parameter in Marker in favor of `zIndexInt`.
* Updates minimum supported SDK version to Flutter 3.27/Dart 3.6.

## 2.15.2
Expand Down
4 changes: 2 additions & 2 deletions packages/google_maps_flutter/google_maps_flutter_ios/example/ios14/pubspec.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ publish_to: none

environment:
sdk: ^3.6.0
flutter: ">=3.27.0"
flutter: '>=3.27.0'

dependencies:
cupertino_icons: ^1.0.5
Expand All @@ -18,7 +18,7 @@ dependencies:
# The example app is bundled with the plugin so we use a path dependency on
# the parent directory to use the current plugin's version.
path: ../../
google_maps_flutter_platform_interface: ^2.11.0
google_maps_flutter_platform_interface: ^2.12.1
maps_example_dart:
path: ../shared/maps_example_dart/

Expand Down
4 changes: 2 additions & 2 deletions packages/google_maps_flutter/google_maps_flutter_ios/example/ios15/pubspec.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ publish_to: none

environment:
sdk: ^3.6.0
flutter: ">=3.27.0"
flutter: '>=3.27.0'

dependencies:
cupertino_icons: ^1.0.5
Expand All @@ -18,7 +18,7 @@ dependencies:
# The example app is bundled with the plugin so we use a path dependency on
# the parent directory to use the current plugin's version.
path: ../../
google_maps_flutter_platform_interface: ^2.11.0
google_maps_flutter_platform_interface: ^2.12.1
maps_example_dart:
path: ../shared/maps_example_dart/

Expand Down
4 changes: 2 additions & 2 deletions ...ps_flutter/google_maps_flutter_ios/example/shared/maps_example_dart/lib/place_marker.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -269,10 +269,10 @@ class PlaceMarkerBodyState extends State<PlaceMarkerBody> {

Future<void> _changeZIndex(MarkerId markerId) async {
final Marker marker = markers[markerId]!;
final double current = marker.zIndex;
final int current = marker.zIndexInt;
setState(() {
markers[markerId] = marker.copyWith(
zIndexParam: current == 12.0 ? 0.0 : current + 1.0,
zIndexIntParam: current == 12 ? 0 : current + 1,
);
});
}
Expand Down
4 changes: 2 additions & 2 deletions ...google_maps_flutter/google_maps_flutter_ios/example/shared/maps_example_dart/pubspec.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ publish_to: none

environment:
sdk: ^3.6.0
flutter: ">=3.27.0"
flutter: '>=3.27.0'

dependencies:
cupertino_icons: ^1.0.5
Expand All @@ -18,7 +18,7 @@ dependencies:
# The example app is bundled with the plugin so we use a path dependency on
# the parent directory to use the current plugin's version.
path: ../../../
google_maps_flutter_platform_interface: ^2.11.0
google_maps_flutter_platform_interface: ^2.12.1

dev_dependencies:
flutter_test:
Expand Down
6 changes: 3 additions & 3 deletions packages/google_maps_flutter/google_maps_flutter_ios/pubspec.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@ name: google_maps_flutter_ios
description: iOS implementation of the google_maps_flutter plugin.
repository: https://github.com/flutter/packages/tree/main/packages/google_maps_flutter/google_maps_flutter_ios
issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+maps%22
version: 2.15.2
version: 2.15.3

@stuartmorgan-g stuartmorgan-g Jun 10, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Similarly, the only change to this package is to a file in the example app that's not the one visible on pub.dev, so this should be exempt from versioning.

@stuartmorgan-g stuartmorgan-g Jun 10, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Although, why is only the example changing for iOS? Shouldn't it be using zIndexInt instead of the deprecated zIndex now, since it can't use any fractional values anyway?

@Hari-07 Hari-07 Jun 10, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

You're right, not sure how that ended up getting removed or at what point. I think the reasonable thing to do would be to make ios's PlatformMarker expect an integral zIndex, so that this truncation would now be a type error to which we can pass zIndexInt

@stuartmorgan-g stuartmorgan-g Jun 10, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Agreed, the iOS changes look good here.

Based on the CI failure though, it appears that the Obj-C compiler flags 64-bit int assignment to 32-bit int assignment as lossy, but did not flag it when it was assigning a double to a 32-bit int. Odd, but you'll need to add an explicit cast here to make the analysis check happy.

@Hari-07 Hari-07 Jun 10, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That's seems reasonable from flags.
-Wno-float-conversion looks like it allows implicit float conversion which is what was happening
-Wshorten-64-to-32 would disallow the long to int cast that's happening at the line you pointed out


environment:
sdk: ^3.6.0
flutter: ">=3.27.0"
flutter: '>=3.27.0'

flutter:
plugin:
Expand All @@ -19,7 +19,7 @@ flutter:
dependencies:
flutter:
sdk: flutter
google_maps_flutter_platform_interface: ^2.11.0
google_maps_flutter_platform_interface: ^2.12.1
stream_transform: ^2.0.0

dev_dependencies:
Expand Down
3 changes: 2 additions & 1 deletion packages/google_maps_flutter/google_maps_flutter_web/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
## NEXT
## 0.5.13

* Deprecates `zIndex` parameter in Marker in favor of `zIndexInt`.
* Updates minimum supported SDK version to Flutter 3.27/Dart 3.6.

## 0.5.12
Expand Down
15 changes: 15 additions & 0 deletions ...es/google_maps_flutter/google_maps_flutter_web/example/integration_test/markers_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -536,5 +536,20 @@ void main() {
expect(icon2.anchor!.x, width * anchorOffset.dx);
expect(icon2.anchor!.y, height * anchorOffset.dy);
});

testWidgets('interpret correct zIndex in convertsion',
(WidgetTester tester) async {
const MarkerId markerId = MarkerId('1');

final Set<Marker> markers = <Marker>{
const Marker(markerId: markerId, zIndexInt: 4),
};

await controller.addMarkers(markers);

final gmaps.Marker? gmMarker = controller.markers[markerId]?.marker;
expect(gmMarker, isNotNull);
expect(gmMarker!.zIndex, 4);
});
});
}
4 changes: 2 additions & 2 deletions packages/google_maps_flutter/google_maps_flutter_web/example/pubspec.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@ publish_to: none

environment:
sdk: ^3.6.0
flutter: ">=3.27.0"
flutter: '>=3.27.0'

dependencies:
flutter:
sdk: flutter
google_maps_flutter_platform_interface: ^2.10.0
google_maps_flutter_platform_interface: ^2.12.1
google_maps_flutter_web:
path: ../
web: ^1.0.0
Expand Down
18 changes: 14 additions & 4 deletions packages/google_maps_flutter/google_maps_flutter_web/lib/src/convert.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ gmaps.InfoWindowOptions? _infoWindowOptionsFromMarker(Marker marker) {

return gmaps.InfoWindowOptions()
..content = container
..zIndex = marker.zIndex;
..zIndex = marker.effectiveZIndex;
// TODO(ditman): Compute the pixelOffset of the infoWindow, from the size of the Marker,
// and the marker.infoWindow.anchor property.
}
Expand Down Expand Up @@ -453,8 +453,8 @@ Future<gmaps.Icon?> _gmIconFromBitmapDescriptor(
return icon;
}

// Computes the options for a new [gmaps.Marker] from an incoming set of options
// [marker], and the existing marker registered with the map: [currentMarker].
/// Computes the options for a new [gmaps.Marker] from an incoming set of options
/// [marker], and the existing marker registered with the map: [currentMarker].
Future<gmaps.MarkerOptions> _markerOptionsFromMarker(
Marker marker,
gmaps.Marker? currentMarker,
Expand All @@ -465,7 +465,7 @@ Future<gmaps.MarkerOptions> _markerOptionsFromMarker(
marker.position.longitude,
)
..title = sanitizeHtml(marker.infoWindow.title ?? '')
..zIndex = marker.zIndex
..zIndex = marker.effectiveZIndex
..visible = marker.visible
..opacity = marker.alpha
..draggable = marker.draggable
Expand Down Expand Up @@ -730,3 +730,13 @@ gmaps.LatLng _pixelToLatLng(gmaps.Map map, int x, int y) {

return projection.fromPointToLatLng(point)!;
}

extension on Marker {

@stuartmorgan-g stuartmorgan-g Jun 9, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Extensions violate Flutter's style guide. There are cases where we have to use them for JS interop, but that doesn't apply here.

Why do we need this at all, vs just using zIndex with a comment like in the Android example?

@Hari-07 Hari-07 Jun 9, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Right. IIRC this was written when 2 different parameters were there initially, now that we don't allow the constructor to pass in both at the same time, this isn't required

// We compare with zero because zIndexInt if non zero should get higher priority than zIndex
num? get effectiveZIndex {
if (zIndexInt != 0) {
return zIndexInt;
}
return zIndex;
}
}
8 changes: 4 additions & 4 deletions packages/google_maps_flutter/google_maps_flutter_web/pubspec.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@ 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.12
version: 0.5.13

@stuartmorgan-g stuartmorgan-g Jun 9, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This should be 0.5.12+1 as it's a very small change.


environment:
sdk: ^3.6.0
flutter: ">=3.27.0"
flutter: '>=3.27.0'

flutter:
plugin:
Expand All @@ -23,10 +23,10 @@ dependencies:
flutter_web_plugins:
sdk: flutter
google_maps: ^8.0.0
google_maps_flutter_platform_interface: ^2.10.0
google_maps_flutter_platform_interface: ^2.12.1
sanitize_html: ^2.0.0
stream_transform: ^2.0.0
web: ">=0.5.1 <2.0.0"
web: '>=0.5.1 <2.0.0'

dev_dependencies:
flutter_test:
Expand Down