packages
packages copied to clipboard
[google_maps_flutter] Marker clustering support
Adds the initial support for the marker clustering for Android, iOS and Web platforms. Clustering is implemented using native google maps utils libraries for Android, iOS and Web.
The Google Maps Utils libraries provides the capability for future additions of features available under these libraries to the google_maps_flutter
plugin, such as heatmaps, although they are not included in this PR.
This PR is created from previous PR: https://github.com/flutter/plugins/pull/6752
Resolves https://github.com/flutter/flutter/issues/26863
Android:
iOS:
Web:
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.yaml
with an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes. - [x] I updated
CHANGELOG.md
to 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.
@jokerttu Is this still something you are interested in working on? How can we help you?
@jokerttu is this still something you're working on?
@jokerttu is this still something you're working on?
Yes, I am scheduled to work on this issue next week and will update the PR accordingly in the near future. The recent architectural changes in the google_maps_flutter_web
implementation have affected the integration tests, which I am addressing. Additionally, some new methods introduced in this PR will likely require updates to the native unit tests.
@stuartmorgan this is ready for review. We inspected the logs for the failing web platform tests and it seems to be reporting based on Android?
Note: that the original PR was reviewed at plugins repository at https://github.com/flutter/plugins/pull/6752 Implementation parts have been mainly same, but small fixes and changes has been done to fix conflicts. Web tests are rewritten because of changes on the web implementation (web plugin was endorsed). iOS Google Maps Utils library version is now pinned to 4.1.0 as newer versions do not support iOS platform versions 11 and 12
@stuartmorgan this is ready for review. We inspected the logs for the failing web platform tests and it seems to be reporting based on Android?
This is because there is a weird circular dependency in the tests where google_maps_flutter_web's example depends on google_maps, which then is depending on a non-overridden version of google_maps_flutter_android so things no longer match. Adding a dependency override for _android in google_maps_flutter_web/example/pubspec.yaml
should allow the tests to run.
@ditman Looks like the original never got your review for web; if you could take a look here when you get a chance that would be great.
Any progress updates on when we can expect this to come out?
@jokerttu is this ready for another review?
@hellohuanlin
@jokerttu is this ready for another review?
Not yet, most of the issues @stuartmorgan pointed out are fixed, but there are still few that are not yet done. Also, before this can be finalized, we need also some resolution to this question waiting review from @ditman: (https://github.com/flutter/packages/pull/4319#discussion_r1424267362)
@stuartmorgan @ditman I've addressed most of the review comments and made the corresponding updates to the PR (with help of @illuminati1911). However, a few discussions remain open, and I'm actively seeking further input on those.
Regarding the web implementation, I encountered challenges with using @staticInterop
annotations for methods that involve google-maps
interop objects as parameters. Since google-maps
objects do not utilize @staticInterop
, it appears we cannot proceed in this direction for the time being. I'm open to suggestions or alternative approaches to resolve this issue.
(Edit: this PR updates the google_maps
package, but it does not seem to use @staticInterop
so that might not help with this at all)
I've finished reviewing the platform interface, and since the overall structure was already reviewed in the previous PR you are good to split that out into the first prequel PR. I've left some comments that you can either address here before splitting, or just address in the new PR.
It looks like this still hasn't been split out?
@stuartmorgan
It looks like this still hasn't been split out?
The review comments were resolved in this PR before splitting so that the platform-specific implementations would not stretch too far in time after the platform interface update.
Here is the first prequel PR for google_maps_flutter_platform_interface
: https://github.com/flutter/packages/pull/6158
However, a few discussions remain open, and I'm actively seeking further input on those.
Was there anything other than web-specific questions? I looked back through the comments and didn't find any unresolved discussions with questions other than the web ones, but I may have missed something.
@stuartmorgan
However, a few discussions remain open, and I'm actively seeking further input on those.
Was there anything other than web-specific questions? I looked back through the comments and didn't find any unresolved discussions with questions other than the web ones, but I may have missed something.
Yes, checked and only web-specific discussions are still open.
@stuartmorgan added prequel PR:s for platforms as well as draft, as the platform_interface (https://github.com/flutter/packages/pull/6158) PR has not yet landed: Android: https://github.com/flutter/packages/pull/6185 iOS: https://github.com/flutter/packages/pull/6186 Web: https://github.com/flutter/packages/pull/6187 (@ditman)
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact "@test-exemption-reviewer" in the #hackers channel in Chat (don't just cc them here, they won't see it! Use Discord!).
If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?
Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.
Converted to draft to avoid unnecessary reviews before the platform implementations are landed.
@jokerttu any updates on this 🥺 Really looking forward to it. Really appreciate you for taking this on!
@jokerttu any updates on this 🥺 Really looking forward to it. Really appreciate you for taking this on!
The work is ongoing on platform implementation PR:s ~https://github.com/flutter/packages/pull/6185~ (merged) ~https://github.com/flutter/packages/pull/6186~ (merged) ~https://github.com/flutter/packages/pull/6187~ (merged)
Status update from triage: Blocked on landing the iOS platform implementation, which is turn blocked on Google Maps SDK-level issues.
Any update?
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!).
If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?
Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group.
https://medium.com/@rishyash8/clustering-support-in-google-map-flutter-2024-d8b1c68e515b
Basic implementation as example code is not updated in google_map_flutter on pub.dev
Pub.dev only shows a single file in the example tab, and the example app for this package is quite large; all the individual features are on sub-pages, and clustering is no different. The example app can be viewed or run locally to see specific features.
Pub.dev only shows a single file in the example tab, and the example app for this package is quite large; all the individual features are on sub-pages, and clustering is no different. The example app can be viewed or run locally to see specific features.
yep, but the example which was present in the package was quite complex to understand as a beginner so created a simple example for that.
Hey Everyone, just gone through whole code for clustering part for flutter, cant find Custom Cluster Renderer for customising the cluster icon. Any update on that, like are we picking it in near future.
@Mohit8G This is a PR that has landed, so it cannot be changed. If you have follow-up feature requests, please file them in the issue tracker.
yep, but the example which was present in the package was quite complex to understand as a beginner so created a simple example for that.
"I wanted to make a simpler example for beginners" and "the updated example code was not published to pub.dev" are very different statements; the latter would have indicated some kind of serious bug in the release process. Please don't post to a PR to promote your post but describe it as if it were a bug in the PR or release.
Temporarily locking, as this is attracting a lot of off-topic comments. Discussion on a PR should be about the code in the PR, not things like follow-up requests.