-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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.
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
[google_maps_flutter_web] Implement my location & my location button #4486
Conversation
…into impl-my-location
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.) |
@stuartmorgan do you mean the The former are built-ins in the browser (see Geolocation API), and already "externalized" because they're provided by (Another alternative if we wanted to use an external geo-location client could be to rely on 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. |
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?)
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 🙂 |
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 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?) |
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.
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. |
There's an incoming migration to |
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? |
@stuartmorgan yes, I will update this pr soon with the above discussion. |
Marking as a draft for now, pending the updates to use |
could you refer which updates are pending? i'd like to follow |
This comment was marked as off-topic.
This comment was marked as off-topic.
(PR triage): Hey @stuartmorgan, is this still waiting on that work? |
Since this is marked as a draft and hasn't been updated in several months I'm going to close it to clean out our review queue. Please don't hesitate to submit a new PR if you decide to revisit this. Thanks! |
Currently, we don't "My Location" widget, so this PR i want support that feature, Thanks
Issue: flutter/flutter#64073
Recreating PR from flutter/plugins from flutter/plugins#6868
Demo:
Screen.Recording.2023-07-28.at.23.55.03.mov
Pre-launch Checklist
dart format
.)[shared_preferences]
pubspec.yaml
with an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.CHANGELOG.md
to add a description of the change, following repository CHANGELOG style.///
).If you need help, consider asking for advice on the #hackers-new channel on Discord.