-
Notifications
You must be signed in to change notification settings - Fork 3k
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] Add marker clustering support - iOS implementation #6186
[google_maps_flutter] Add marker clustering support - iOS implementation #6186
Conversation
cc42eb1
to
0f29034
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code looks quite clean. But I feel I don't know enough about clusters to stamp it just yet. Could you also invite the reviewers from your previous PR?
packages/google_maps_flutter/google_maps_flutter_ios/ios/Classes/FLTClusterManagersController.h
Outdated
Show resolved
Hide resolved
packages/google_maps_flutter/google_maps_flutter_ios/ios/Classes/FLTClusterManagersController.h
Outdated
Show resolved
Hide resolved
packages/google_maps_flutter/google_maps_flutter_ios/ios/Classes/FLTClusterManagersController.m
Outdated
Show resolved
Hide resolved
packages/google_maps_flutter/google_maps_flutter_ios/ios/Classes/FLTClusterManagersController.m
Outdated
Show resolved
Hide resolved
packages/google_maps_flutter/google_maps_flutter_ios/ios/Classes/FLTClusterManagersController.m
Outdated
Show resolved
Hide resolved
packages/google_maps_flutter/google_maps_flutter_ios/ios/Classes/GoogleMapController.m
Show resolved
Hide resolved
packages/google_maps_flutter/google_maps_flutter_ios/ios/Classes/GoogleMapMarkerController.m
Outdated
Show resolved
Hide resolved
00b59e4
to
1669b76
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you add some unit tests on native side?
packages/google_maps_flutter/google_maps_flutter_ios/ios/google_maps_flutter_ios.podspec
Outdated
Show resolved
Hide resolved
b0e9ada
to
b4eaeba
Compare
Added some native unit tests |
b4eaeba
to
2c326e8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for adding those tests. The overall code looks clean.
@cbracken feel free to give it another pass (if you see fit) since you are the new owner. Otherwise I think it's good to land.
...er/google_maps_flutter_ios/example/ios12/ios/RunnerTests/FLTClusterManagersControllerTests.m
Outdated
Show resolved
Hide resolved
...er/google_maps_flutter_ios/example/ios14/ios/RunnerTests/FLTClusterManagersControllerTests.m
Outdated
Show resolved
Hide resolved
packages/google_maps_flutter/google_maps_flutter_ios/ios/Classes/FLTClusterManagersController.h
Outdated
Show resolved
Hide resolved
packages/google_maps_flutter/google_maps_flutter_ios/ios/Classes/FLTClusterManagersController.m
Outdated
Show resolved
Hide resolved
packages/google_maps_flutter/google_maps_flutter_ios/ios/Classes/FLTClusterManagersController.m
Outdated
Show resolved
Hide resolved
packages/google_maps_flutter/google_maps_flutter_ios/ios/Classes/GoogleMarkerUserData.h
Outdated
Show resolved
Hide resolved
packages/google_maps_flutter/google_maps_flutter_ios/ios/Classes/GoogleMarkerUtilities.h
Outdated
Show resolved
Hide resolved
packages/google_maps_flutter/google_maps_flutter_ios/ios/Classes/GoogleMarkerUtilities.h
Outdated
Show resolved
Hide resolved
packages/google_maps_flutter/google_maps_flutter_ios/ios/google_maps_flutter_ios.podspec
Outdated
Show resolved
Hide resolved
packages/google_maps_flutter/google_maps_flutter_ios/lib/src/google_maps_flutter_ios.dart
Outdated
Show resolved
Hide resolved
e382d51
to
9cc025d
Compare
@jokerttu is this ready for re-review? |
@jmagman not yet, this is waiting new release of Google-Maps-iOS-Utils library to have support latest Google Maps SDK versions. See discussion here: #6186 (comment) |
FYI I have raised the blocking issue internally with the Google Maps team. |
@stuartmorgan |
12face8
to
21dc4ef
Compare
…n Google-Maps-iOS-Utils
21dc4ef
to
b25170b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I missed a couple of things; once those are done this will be good to land.
packages/google_maps_flutter/google_maps_flutter_ios/ios/Classes/GoogleMapMarkerController.h
Outdated
Show resolved
Hide resolved
packages/google_maps_flutter/google_maps_flutter_ios/lib/src/google_maps_flutter_ios.dart
Outdated
Show resolved
Hide resolved
packages/google_maps_flutter/google_maps_flutter_ios/lib/src/google_maps_flutter_ios.dart
Outdated
Show resolved
Hide resolved
packages/google_maps_flutter/google_maps_flutter_ios/lib/src/google_maps_flutter_ios.dart
Outdated
Show resolved
Hide resolved
auto label is removed for flutter/packages/6186, due to This PR has not met approval requirements for merging. Changes were requested by {stuartmorgan}, please make the needed changes and resubmit this PR.
|
@stuartmorgan seems like I need separate approval from you before I can add autosubmit label. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Sorry, I should have pressed the other button last time to avoid that.
flutter/packages@551bde5...5cc0a01 2024-08-07 [email protected] [ci] Update repository for the release of Flutter 3.24 (flutter/packages#7331) 2024-08-07 [email protected] Roll Flutter (stable) from b0850be to 80c2e84 (1397 revisions) (flutter/packages#7322) 2024-08-07 49699333+dependabot[bot]@users.noreply.github.com Bump actions/upload-artifact from 4.3.5 to 4.3.6 (flutter/packages#7330) 2024-08-07 49699333+dependabot[bot]@users.noreply.github.com [pigeon]: Bump io.mockk:mockk from 1.13.7 to 1.13.12 in /packages/pigeon/platform_tests/test_plugin/android (flutter/packages#7189) 2024-08-07 [email protected] [google_maps_flutter] Marker clustering support (flutter/packages#4319) 2024-08-07 49699333+dependabot[bot]@users.noreply.github.com Bump github/codeql-action from 3.25.15 to 3.26.0 (flutter/packages#7329) 2024-08-07 [email protected] Manual roll Flutter from 1dd7141 to 0a7f8af (23 revisions) (flutter/packages#7328) 2024-08-06 [email protected] [image_picker_ios] Update image picker UI test query for iOS 18 (flutter/packages#7325) 2024-08-06 [email protected] [google_maps_flutter] Add marker clustering support - iOS implementation (flutter/packages#6186) 2024-08-06 [email protected] [url_launcher] launchUrl always returns true for valid schemes on the web. (flutter/packages#7229) 2024-08-06 [email protected] [google_maps_flutter] Add heatmap support (flutter/packages#3257) 2024-08-06 [email protected] [local_auth] Endorse macOS (flutter/packages#7274) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages-flutter-autoroll Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
flutter/packages@551bde5...5cc0a01 2024-08-07 [email protected] [ci] Update repository for the release of Flutter 3.24 (flutter/packages#7331) 2024-08-07 [email protected] Roll Flutter (stable) from b0850be to 80c2e84 (1397 revisions) (flutter/packages#7322) 2024-08-07 49699333+dependabot[bot]@users.noreply.github.com Bump actions/upload-artifact from 4.3.5 to 4.3.6 (flutter/packages#7330) 2024-08-07 49699333+dependabot[bot]@users.noreply.github.com [pigeon]: Bump io.mockk:mockk from 1.13.7 to 1.13.12 in /packages/pigeon/platform_tests/test_plugin/android (flutter/packages#7189) 2024-08-07 [email protected] [google_maps_flutter] Marker clustering support (flutter/packages#4319) 2024-08-07 49699333+dependabot[bot]@users.noreply.github.com Bump github/codeql-action from 3.25.15 to 3.26.0 (flutter/packages#7329) 2024-08-07 [email protected] Manual roll Flutter from 1dd7141 to 0a7f8af (23 revisions) (flutter/packages#7328) 2024-08-06 [email protected] [image_picker_ios] Update image picker UI test query for iOS 18 (flutter/packages#7325) 2024-08-06 [email protected] [google_maps_flutter] Add marker clustering support - iOS implementation (flutter/packages#6186) 2024-08-06 [email protected] [url_launcher] launchUrl always returns true for valid schemes on the web. (flutter/packages#7229) 2024-08-06 [email protected] [google_maps_flutter] Add heatmap support (flutter/packages#3257) 2024-08-06 [email protected] [local_auth] Endorse macOS (flutter/packages#7274) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages-flutter-autoroll Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
flutter/packages@551bde5...5cc0a01 2024-08-07 [email protected] [ci] Update repository for the release of Flutter 3.24 (flutter/packages#7331) 2024-08-07 [email protected] Roll Flutter (stable) from b0850be to 80c2e84 (1397 revisions) (flutter/packages#7322) 2024-08-07 49699333+dependabot[bot]@users.noreply.github.com Bump actions/upload-artifact from 4.3.5 to 4.3.6 (flutter/packages#7330) 2024-08-07 49699333+dependabot[bot]@users.noreply.github.com [pigeon]: Bump io.mockk:mockk from 1.13.7 to 1.13.12 in /packages/pigeon/platform_tests/test_plugin/android (flutter/packages#7189) 2024-08-07 [email protected] [google_maps_flutter] Marker clustering support (flutter/packages#4319) 2024-08-07 49699333+dependabot[bot]@users.noreply.github.com Bump github/codeql-action from 3.25.15 to 3.26.0 (flutter/packages#7329) 2024-08-07 [email protected] Manual roll Flutter from 1dd7141 to 0a7f8af (23 revisions) (flutter/packages#7328) 2024-08-06 [email protected] [image_picker_ios] Update image picker UI test query for iOS 18 (flutter/packages#7325) 2024-08-06 [email protected] [google_maps_flutter] Add marker clustering support - iOS implementation (flutter/packages#6186) 2024-08-06 [email protected] [url_launcher] launchUrl always returns true for valid schemes on the web. (flutter/packages#7229) 2024-08-06 [email protected] [google_maps_flutter] Add heatmap support (flutter/packages#3257) 2024-08-06 [email protected] [local_auth] Endorse macOS (flutter/packages#7274) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages-flutter-autoroll Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
flutter/packages@551bde5...5cc0a01 2024-08-07 [email protected] [ci] Update repository for the release of Flutter 3.24 (flutter/packages#7331) 2024-08-07 [email protected] Roll Flutter (stable) from b0850be to 80c2e84 (1397 revisions) (flutter/packages#7322) 2024-08-07 49699333+dependabot[bot]@users.noreply.github.com Bump actions/upload-artifact from 4.3.5 to 4.3.6 (flutter/packages#7330) 2024-08-07 49699333+dependabot[bot]@users.noreply.github.com [pigeon]: Bump io.mockk:mockk from 1.13.7 to 1.13.12 in /packages/pigeon/platform_tests/test_plugin/android (flutter/packages#7189) 2024-08-07 [email protected] [google_maps_flutter] Marker clustering support (flutter/packages#4319) 2024-08-07 49699333+dependabot[bot]@users.noreply.github.com Bump github/codeql-action from 3.25.15 to 3.26.0 (flutter/packages#7329) 2024-08-07 [email protected] Manual roll Flutter from 1dd7141 to 0a7f8af (23 revisions) (flutter/packages#7328) 2024-08-06 [email protected] [image_picker_ios] Update image picker UI test query for iOS 18 (flutter/packages#7325) 2024-08-06 [email protected] [google_maps_flutter] Add marker clustering support - iOS implementation (flutter/packages#6186) 2024-08-06 [email protected] [url_launcher] launchUrl always returns true for valid schemes on the web. (flutter/packages#7229) 2024-08-06 [email protected] [google_maps_flutter] Add heatmap support (flutter/packages#3257) 2024-08-06 [email protected] [local_auth] Endorse macOS (flutter/packages#7274) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages-flutter-autoroll Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
This PR introduces support for marker clustering for iOS platform
An example usabe is available in the example application at ./packages/google_maps_flutter/google_maps_flutter_ios/example/ios12 on the page
Manage clustering
This is prequel PR for: #4319
and sequel PR for: #6158
Containing only changes to
google_maps_flutter_ios
package.Follow up PR will hold the app-facing plugin implementation.
Linked issue: flutter/flutter#26863
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.