[google_maps_flutter_web] Issue 64073 implement my location#12186
[google_maps_flutter_web] Issue 64073 implement my location#12186Zubii12 wants to merge 12 commits into
Conversation
01d001a to
771d770
Compare
There was a problem hiding this comment.
Code Review
This pull request adds support for the 'My Location' button and current location tracking in the Google Maps Web plugin by introducing MyLocationController, MyLocationButton, and a mockable GeolocationApi. The review feedback highlights several critical issues, including potential resource and memory leaks from uncleared geolocation watches and map event listeners, runtime crashes in non-secure contexts due to unchecked geolocation support, and unsafe force-unwrapping of _markersController. Additionally, the feedback suggests using relative asset paths instead of web.window.location.href, triggering the location button's loading animation, preventing duplicate CSS injections, adding accessibility attributes, and improving double literal readability.
| class WebGeolocationApi implements GeolocationApi { | ||
| final web.Geolocation _geolocation = web.window.navigator.geolocation; | ||
|
|
||
| @override | ||
| int watchPosition( | ||
| void Function(double latitude, double longitude) onSuccess, | ||
| void Function(dynamic error) onError, | ||
| ) { | ||
| return _geolocation.watchPosition( | ||
| (web.GeolocationPosition location) { | ||
| onSuccess(location.coords.latitude, location.coords.longitude); | ||
| }.toJS, | ||
| (web.GeolocationPositionError error) { | ||
| onError(error); | ||
| }.toJS, | ||
| web.PositionOptions(), | ||
| ); | ||
| } | ||
|
|
||
| @override | ||
| void getCurrentPosition( | ||
| void Function(double latitude, double longitude) onSuccess, | ||
| void Function(dynamic error) onError, { | ||
| int timeoutMs = 30000, | ||
| }) { | ||
| _geolocation.getCurrentPosition( | ||
| (web.GeolocationPosition location) { | ||
| onSuccess(location.coords.latitude, location.coords.longitude); | ||
| }.toJS, | ||
| (web.GeolocationPositionError error) { | ||
| onError(error); | ||
| }.toJS, | ||
| web.PositionOptions(timeout: timeoutMs), | ||
| ); | ||
| } | ||
|
|
||
| @override | ||
| void clearWatch(int watchId) => _geolocation.clearWatch(watchId); | ||
| } |
There was a problem hiding this comment.
In non-secure contexts (HTTP) or browsers where Geolocation is not supported, web.window.navigator.geolocation is undefined / null. Accessing it directly and calling methods on it will throw a runtime TypeError and crash the application. Check if geolocation is supported before attempting to use it.
class WebGeolocationApi implements GeolocationApi {
web.Geolocation? get _geolocation {
try {
return web.window.navigator.geolocation;
} catch (_) {
return null;
}
}
@override
int watchPosition(
void Function(double latitude, double longitude) onSuccess,
void Function(dynamic error) onError,
) {
final geolocation = _geolocation;
if (geolocation == null) {
onError('Geolocation is not supported or not available in this context.');
return 0;
}
return geolocation.watchPosition(
(web.GeolocationPosition location) {
onSuccess(location.coords.latitude, location.coords.longitude);
}.toJS,
(web.GeolocationPositionError error) {
onError(error);
}.toJS,
web.PositionOptions(),
);
}
@override
void getCurrentPosition(
void Function(double latitude, double longitude) onSuccess,
void Function(dynamic error) onError, {
int timeoutMs = 30000,
}) {
final geolocation = _geolocation;
if (geolocation == null) {
onError('Geolocation is not supported or not available in this context.');
return;
}
geolocation.getCurrentPosition(
(web.GeolocationPosition location) {
onSuccess(location.coords.latitude, location.coords.longitude);
}.toJS,
(web.GeolocationPositionError error) {
onError(error);
}.toJS,
web.PositionOptions(timeout: timeoutMs),
);
}
@override
void clearWatch(int watchId) {
_geolocation?.clearWatch(watchId);
}
}There was a problem hiding this comment.
web.Geolocation is not null
…derated plugin testing
771d770 to
17725bd
Compare
…removeMyLocationButton
…n rendering my location button
Implement my location button, add blue dot location
Fixes flutter/flutter#64073
Pre-Review Checklist
[shared_preferences]///).If you need help, consider asking for advice on the #hackers-new channel on Discord.
Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the
gemini-code-assistbot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.Footnotes
Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling. ↩ ↩2