Skip to content
This repository has been archived by the owner on Feb 22, 2023. It is now read-only.

[google_maps_flutter] Migrate to NNBD #3548

Merged
merged 23 commits into from
Feb 19, 2021
Merged
Show file tree
Hide file tree
Changes from 18 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
4 changes: 4 additions & 0 deletions packages/google_maps_flutter/google_maps_flutter/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 2.0.0-nullsafety

* Migrate to null-safety

## 1.2.0

* Support custom tiles.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright 2019, the Chromium project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
// @dart=2.9

import 'dart:typed_data';
import 'package:flutter/services.dart';
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright 2019, the Chromium project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
// @dart=2.9

import 'dart:async';
import 'dart:io';
Expand Down Expand Up @@ -954,10 +955,8 @@ void main() {
final BitmapDescriptor scaled = await BitmapDescriptor.fromAssetImage(
imageConfiguration, 'red_square.png',
mipmaps: false);
// ignore: invalid_use_of_visible_for_testing_member
expect(mip.toJson()[2], 1);
// ignore: invalid_use_of_visible_for_testing_member
expect(scaled.toJson()[2], 2);
expect((mip.toJson() as List<dynamic>)[2], 1);
Copy link

Choose a reason for hiding this comment

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

nit: assert size before reading the 2nd element

Copy link
Contributor Author

Choose a reason for hiding this comment

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

While updating these I decided it wasn't worth adding complexity to the test code to assert incremental steps; if toJson returns a one-element array this test will fail with an out-of-bounds index exception, which gives us the same information than a length check would give us.

(The bigger problem is that this is testing code from the interface package; this test doesn't belong here at all. But moving it was out of scope for this change.)

expect((scaled.toJson() as List<dynamic>)[2], 2);
});

testWidgets('testTakeSnapshot', (WidgetTester tester) async {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ class AnimateCamera extends StatefulWidget {
}

class AnimateCameraState extends State<AnimateCamera> {
GoogleMapController mapController;
late GoogleMapController mapController;

void _onMapCreated(GoogleMapController controller) {
mapController = controller;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,9 @@ class _MapClickBody extends StatefulWidget {
class _MapClickBodyState extends State<_MapClickBody> {
_MapClickBodyState();

GoogleMapController mapController;
LatLng _lastTap;
LatLng _lastLongPress;
GoogleMapController? mapController;
Copy link

Choose a reason for hiding this comment

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

should this be late similar to animate_camera.dart?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

build kicks off async maps creation, and also uses mapController if present, so it does need to be nullable.

I went back and took a closer look at the late cases, and all but one were actually wrong, and had a (latent) bug where there was a race condition where someone could potentially press a control before the map controller was actually created. (I was originally fooled because one of them has a boolean for whether creation has completed and gates showing the controls on that boolean, so late is correct, and I missed that others that used the controller unconditionally in callbacks didn't have the same structure.)

I think it's not ideal that there are at least three different patterns for handling the async construction in these example files, but don't want to make changing that part of nullability migration.

Copy link

Choose a reason for hiding this comment

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

ah I see. Yeah, I didn't have that context. Thanks

LatLng? _lastTap;
LatLng? _lastLongPress;

@override
Widget build(BuildContext context) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ class _MapCoordinatesBody extends StatefulWidget {
class _MapCoordinatesBodyState extends State<_MapCoordinatesBody> {
_MapCoordinatesBodyState();

GoogleMapController mapController;
GoogleMapController? mapController;
Copy link

Choose a reason for hiding this comment

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

ditto: in some cases, late is used

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See other comment; this is correct.

LatLngBounds _visibleRegion = LatLngBounds(
southwest: const LatLng(0, 0),
northeast: const LatLng(0, 0),
Expand Down Expand Up @@ -87,7 +87,7 @@ class _MapCoordinatesBodyState extends State<_MapCoordinatesBody> {
child: const Text('Get Visible Region Bounds'),
onPressed: () async {
final LatLngBounds visibleRegion =
await mapController.getVisibleRegion();
await mapController!.getVisibleRegion();
setState(() {
_visibleRegion = visibleRegion;
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ class MapUiBodyState extends State<MapUiBody> {
bool _myLocationEnabled = true;
bool _myTrafficEnabled = false;
bool _myLocationButtonEnabled = true;
GoogleMapController _controller;
late GoogleMapController _controller;
bool _nightMode = false;

@override
Expand Down Expand Up @@ -249,10 +249,9 @@ class MapUiBodyState extends State<MapUiBody> {
});
}

// Should only be called if _isMapCreated is true.
Widget _nightModeToggler() {
if (!_isMapCreated) {
return null;
}
assert(_isMapCreated);
return TextButton(
child: Text('${_nightMode ? 'disable' : 'enable'} night mode'),
onPressed: () {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ class MarkerIconsBody extends StatefulWidget {
const LatLng _kMapCenter = LatLng(52.4478, -3.5402);

class MarkerIconsBodyState extends State<MarkerIconsBody> {
GoogleMapController controller;
BitmapDescriptor _markerIcon;
GoogleMapController? controller;
BitmapDescriptor? _markerIcon;
Comment on lines +32 to +33
Copy link

Choose a reason for hiding this comment

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

ditto

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are flows in the existing logic that assume _markerIcon can be null.


@override
Widget build(BuildContext context) {
Expand All @@ -48,7 +48,7 @@ class MarkerIconsBodyState extends State<MarkerIconsBody> {
target: _kMapCenter,
zoom: 7.0,
),
markers: _createMarker(),
markers: <Marker>{_createMarker()},
onMapCreated: _onMapCreated,
),
),
Expand All @@ -57,17 +57,19 @@ class MarkerIconsBodyState extends State<MarkerIconsBody> {
);
}

Set<Marker> _createMarker() {
// TODO(iskakaushik): Remove this when collection literals makes it to stable.
// https://github.com/flutter/flutter/issues/28312
// ignore: prefer_collection_literals
return <Marker>[
Marker(
Marker _createMarker() {
if (_markerIcon != null) {
return Marker(
markerId: MarkerId("marker_1"),
position: _kMapCenter,
icon: _markerIcon,
),
].toSet();
icon: _markerIcon!,
);
} else {
return Marker(
markerId: MarkerId("marker_1"),
position: _kMapCenter,
);
}
}

Future<void> _createMarkerImageFromAsset(BuildContext context) async {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ class MoveCamera extends StatefulWidget {
}

class MoveCameraState extends State<MoveCamera> {
GoogleMapController mapController;
late GoogleMapController mapController;

void _onMapCreated(GoogleMapController controller) {
mapController = controller;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ class MarkerIconsBody extends StatefulWidget {
const LatLng _kMapCenter = LatLng(52.4478, -3.5402);

class MarkerIconsBodyState extends State<MarkerIconsBody> {
GoogleMapController controller;
GoogleMapController? controller;

EdgeInsets _padding = const EdgeInsets.all(0);

Expand Down Expand Up @@ -152,10 +152,10 @@ class MarkerIconsBodyState extends State<MarkerIconsBody> {
onPressed: () {
setState(() {
_padding = EdgeInsets.fromLTRB(
double.tryParse(_leftController.value?.text) ?? 0,
double.tryParse(_topController.value?.text) ?? 0,
double.tryParse(_rightController.value?.text) ?? 0,
double.tryParse(_bottomController.value?.text) ?? 0);
double.tryParse(_leftController.value.text) ?? 0,
double.tryParse(_topController.value.text) ?? 0,
double.tryParse(_rightController.value.text) ?? 0,
double.tryParse(_bottomController.value.text) ?? 0);
});
},
),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,10 @@ class PlaceCircleBody extends StatefulWidget {
class PlaceCircleBodyState extends State<PlaceCircleBody> {
PlaceCircleBodyState();

GoogleMapController controller;
GoogleMapController? controller;
Map<CircleId, Circle> circles = <CircleId, Circle>{};
int _circleIdCounter = 1;
CircleId selectedCircle;
CircleId? selectedCircle;

// Values when toggling circle color
int fillColorsIndex = 0;
Expand Down Expand Up @@ -62,12 +62,14 @@ class PlaceCircleBodyState extends State<PlaceCircleBody> {
});
}

void _remove() {
void _remove(CircleId circleId) {
setState(() {
if (circles.containsKey(selectedCircle)) {
circles.remove(selectedCircle);
if (circles.containsKey(circleId)) {
circles.remove(circleId);
}
if (circleId == selectedCircle) {
selectedCircle = null;
}
selectedCircle = null;
});
}

Expand Down Expand Up @@ -100,44 +102,45 @@ class PlaceCircleBodyState extends State<PlaceCircleBody> {
});
}

void _toggleVisible() {
final Circle circle = circles[selectedCircle];
void _toggleVisible(CircleId circleId) {
final Circle circle = circles[circleId]!;
setState(() {
circles[selectedCircle] = circle.copyWith(
circles[circleId] = circle.copyWith(
visibleParam: !circle.visible,
);
});
}

void _changeFillColor() {
final Circle circle = circles[selectedCircle];
void _changeFillColor(CircleId circleId) {
final Circle circle = circles[circleId]!;
setState(() {
circles[selectedCircle] = circle.copyWith(
circles[circleId] = circle.copyWith(
fillColorParam: colors[++fillColorsIndex % colors.length],
);
});
}

void _changeStrokeColor() {
final Circle circle = circles[selectedCircle];
void _changeStrokeColor(CircleId circleId) {
final Circle circle = circles[circleId]!;
setState(() {
circles[selectedCircle] = circle.copyWith(
circles[circleId] = circle.copyWith(
strokeColorParam: colors[++strokeColorsIndex % colors.length],
);
});
}

void _changeStrokeWidth() {
final Circle circle = circles[selectedCircle];
void _changeStrokeWidth(CircleId circleId) {
final Circle circle = circles[circleId]!;
setState(() {
circles[selectedCircle] = circle.copyWith(
circles[circleId] = circle.copyWith(
strokeWidthParam: widths[++widthsIndex % widths.length],
);
});
}

@override
Widget build(BuildContext context) {
final CircleId? selectedId = selectedCircle;
return Column(
mainAxisAlignment: MainAxisAlignment.spaceEvenly,
crossAxisAlignment: CrossAxisAlignment.stretch,
Expand Down Expand Up @@ -171,34 +174,37 @@ class PlaceCircleBodyState extends State<PlaceCircleBody> {
),
TextButton(
child: const Text('remove'),
onPressed: (selectedCircle == null) ? null : _remove,
onPressed: (selectedId == null)
? null
: () => _remove(selectedId),
),
TextButton(
child: const Text('toggle visible'),
onPressed:
(selectedCircle == null) ? null : _toggleVisible,
onPressed: (selectedId == null)
? null
: () => _toggleVisible(selectedId),
),
],
),
Column(
children: <Widget>[
TextButton(
child: const Text('change stroke width'),
onPressed: (selectedCircle == null)
onPressed: (selectedId == null)
? null
: _changeStrokeWidth,
: () => _changeStrokeWidth(selectedId),
),
TextButton(
child: const Text('change stroke color'),
onPressed: (selectedCircle == null)
onPressed: (selectedId == null)
? null
: _changeStrokeColor,
: () => _changeStrokeColor(selectedId),
),
TextButton(
child: const Text('change fill color'),
onPressed: (selectedCircle == null)
onPressed: (selectedId == null)
? null
: _changeFillColor,
: () => _changeFillColor(selectedId),
),
],
)
Expand Down
Loading