packages
packages copied to clipboard
[google_maps_flutter_web] Implement my location & my location button
Currently, we don't "My Location" widget, so this PR i want support that feature, Thanks
Issue: https://github.com/flutter/flutter/issues/64073
Recreating PR from flutter/plugins from https://github.com/flutter/plugins/pull/6868
Demo:
https://github.com/flutter/packages/assets/34020090/e966dbde-04a7-4778-b6fe-e03140f3ed39
Pre-launch Checklist
- [x] I read the Contributor Guide and followed the process outlined there for submitting PRs.
- [x] I read the Tree Hygiene wiki page, which explains my responsibilities.
- [x] I read and followed the relevant style guides and ran the auto-formatter. (Unlike the flutter/flutter repo, the flutter/packages repo does use
dart format.) - [x] I signed the CLA.
- [x] The title of the PR starts with the name of the package surrounded by square brackets, e.g.
[shared_preferences] - [x] I listed at least one issue that this PR fixes in the description above.
- [x] I updated
pubspec.yamlwith an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes. - [x] I updated
CHANGELOG.mdto add a description of the change, following repository CHANGELOG style. - [x] I updated/added relevant documentation (doc comments with
///). - [x] I added new tests to check the change I am making, or this PR is test-exempt.
- [x] All existing and new tests are passing.
If you need help, consider asking for advice on the #hackers-new channel on Discord.
Update from triage: @ditman is currently OOO
(@ditman, when you get back to this review, one high-level question: do we want to have the polyfill in the plugin, vs exposing the location API to allow this code to live as a separate third-party package to do a polyfill? We usually just wrap the functionality of the native SDKs.)
do we want to have the polyfill in the plugin, vs exposing the location API to allow this code to live as a separate third-party package to do a polyfill? We usually just wrap the functionality of the native SDKs.
@stuartmorgan do you mean the Geolocation/Geoposition classes, or the whole shebang of the MyLocation widget, the button on the map and friends?
The former are built-ins in the browser (see Geolocation API), and already "externalized" because they're provided by dart:html (or package:web in the near future).
(Another alternative if we wanted to use an external geo-location client could be to rely on package:geolocator? but that might be too much extra code for what this plugin needs).
The latter might be externalizable, but then that package would end up being something that just renders a bunch of HTML elements that can only be used from our Maps plugin.
The former are built-ins in the browser (see Geolocation API), and already "externalized" because they're provided by
dart:html(orpackage:webin the near future).
Ah, I thought this was a maps API for some reason.
So then my question would be: is there anything that would prevent this entire PR from instead being a separate package that people who want functionality that isn't in the SDK could use? (And if so, could we reasonably add a small set of APIs to fix that?)
The latter might be externalizable, but then that package would end up being something that just renders a bunch of HTML elements that can only be used from our Maps plugin.
Wouldn't it be a package that gets your location and adds HTML elements based on it, which could be used with any map plugin, not just ours?
But even if it's plugin specific, is that a problem? If people using our Maps plugin and who want specific extras functionality can install a third-party package to get it, that seems like the ecosystem working as intended 🙂
But even if it's plugin specific, is that a problem? If people using our Maps plugin and who want specific extras functionality can install a third-party package to get it, that seems like the ecosystem working as intended 🙂
No, in fact, it'd be nice if the google maps plugin allowed for this. The main missing feature I see now is "creating new UI elements in the map Widget to add new behavior" (in this case, what the "_addMyLocationButton" method is doing).
Rendering the "position" itself seems to be just rendering an extra Marker and keep its position updated, so that'd be easy to inject (just give users a method to inject the position marker into the list of markers that they want to render, for example).
One of the issues here is that this feature is built-in in the mobile version of the maps (probably the last "big" feature remaining on the web from the OG feature set of the plugin?)
One of the issues here is that this feature is built-in in the mobile version of the maps
It's your call if you want to own the polyfill. I tend to view the role of a plugin like this to be more "expose the SDK features to Flutter", which means functionality that's missing on one platform is missing, but it's definitely not a bright line.
One of the issues here is that this feature is built-in in the mobile version of the maps
Sure, but presumably non-Flutter web devs also view that as an issue and make stand-alone polyfills? 🤷 Anyway, up to you.
@ditman Ping on this review.
hey @nploi congrats for this PR, this would be such a good addition for Google Maps Web.
Do you know if this will be merged? Did @ditman had the chance to review it?
Thanks again!
There's an incoming migration to package:web that would make this PR more complicated, let's have that PR land first, and then I'll update this one with the latest code, and some changes that I've seen (for example, the way that we compute the asset URL for the button is not great!)
It looks like the linked PR has landed; is this ready to be updated then? @ditman
This PR works for me (on Chrome and Archlinux, I'll test on chrome/Firefox on Android soon). Just had to pin a couple of dependencies to get it to run, but that's all. @nploi , thank you, I appreciate your hard work. Any chance you could merge master in your branch to make this code more up to date? It moved a bit since you opened this PR.
Again, thank you so much!
@nploi Are you still planning on updating this PR per the discussion above?
@nploi Are you still planning on updating this PR per the discussion above?
@stuartmorgan yes, I will update this pr soon with the above discussion.
Marking as a draft for now, pending the updates to use package:web being integrated.
Marking as a draft for now, pending the updates to use
package:webbeing integrated.
could you refer which updates are pending? i'd like to follow
@ditman This pull request #4486 contains important library updates that I need to use in my project. Please respond promptly to this request. Thank you in advance for your understanding and cooperation.