Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[google_maps_flutter] Custom marker size improvements - platform impls #6826

Merged
merged 5 commits into from
Jun 4, 2024
Merged

[google_maps_flutter] Custom marker size improvements - platform impls #6826

merged 5 commits into from
Jun 4, 2024

Conversation

jokerttu
Copy link
Contributor

Platform implementations portion of : #4055

Adds platform handling for new BitmapDescriptor classes AssetMapBitmap and BytesMapBitmap introduced in #6687

Containing only changes to packages

  • google_maps_flutter_android
  • google_maps_flutter_ios
  • google_maps_flutter_web

Follow up PR will hold the app-facing plugin implementations.

Linked issue: flutter/flutter#34657

Pre-launch Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@jokerttu jokerttu changed the title [google_maps_flutter] Platform implementations for custom marker size [google_maps_flutter] Custom marker size improvements - platform impls May 29, 2024
@jokerttu jokerttu mentioned this pull request May 29, 2024
Merged
11 tasks
@jokerttu jokerttu force-pushed the feature/google_maps_flutter_platform_implementations_custom_marker_size_improvements branch from 1ae5234 to 9c0f1d0 Compare May 29, 2024 10:07
@stuartmorgan stuartmorgan mentioned this pull request May 29, 2024
Closed
@jokerttu jokerttu mentioned this pull request May 29, 2024
Merged
11 tasks
@jokerttu jokerttu force-pushed the feature/google_maps_flutter_platform_implementations_custom_marker_size_improvements branch from c63d41c to 1aa3f10 Compare May 29, 2024 20:45
auto-submit bot pushed a commit that referenced this pull request May 29, 2024
@jokerttu
[google_maps_flutter] Undeprecate BitmapDescriptor methods (#6832)
28e8afd 
Undeprecate BitmapDescriptor.fromAssetImage and BitmapDescriptor.fromBytes

* Undeprecates `BitmapDescriptor.fromAssetImage`.
* Undeprecates `BitmapDescriptor.fromBytes`.
* Fixes issues with deprecation in version 2.7.0.

The new formats won't be supported until #6826 lands.
Deprecation notices can be re-added after the implementation PRs have landed.

Fixes: flutter/flutter#149183
stuartmorgan
stuartmorgan requested changes May 30, 2024
View reviewed changes
Copy link
Contributor

@stuartmorgan stuartmorgan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly this needs a review from @reidbaker since Android is the only platform that didn't have sign-off in the main PR yet, but I did a quick pass over everything as well.

...google_maps_flutter_android/android/src/main/java/io/flutter/plugins/googlemaps/Convert.java Outdated Show resolved Hide resolved
...google_maps_flutter_android/android/src/main/java/io/flutter/plugins/googlemaps/Convert.java Outdated Show resolved Hide resolved
packages/google_maps_flutter/google_maps_flutter_android/example/pubspec.yaml Outdated Show resolved Hide resolved
...aps_flutter/google_maps_flutter_ios/example/ios14/ios/RunnerTests/ExtractIconFromDataTests.m Outdated Show resolved Hide resolved
...aps_flutter/google_maps_flutter_ios/example/ios14/ios/RunnerTests/ExtractIconFromDataTests.m Outdated Show resolved Hide resolved
packages/google_maps_flutter/google_maps_flutter_ios/ios/Classes/GoogleMapMarkerController.m
reason:@"Unable to interpret asset, expected a dictionary as the "
@"second parameter."
userInfo:nil];
@throw exception;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Long-term we'll need to rework this, as throwing exceptions violates the style guide. But for now since this is the established pattern in this method, restructuring is out of scope.)

packages/google_maps_flutter/google_maps_flutter_ios/ios/Classes/GoogleMapMarkerController.m Outdated Show resolved Hide resolved
packages/google_maps_flutter/google_maps_flutter_ios/ios/Classes/GoogleMapMarkerController.m Outdated Show resolved Hide resolved
packages/google_maps_flutter/google_maps_flutter_ios/ios/Classes/GoogleMapMarkerController.m Outdated Show resolved Hide resolved
packages/google_maps_flutter/google_maps_flutter_ios/ios/Classes/GoogleMapMarkerController.m Outdated Show resolved Hide resolved
reidbaker
reidbaker requested changes May 30, 2024
View reviewed changes
Copy link
Contributor

@reidbaker reidbaker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Android section reviewed.

...google_maps_flutter_android/android/src/main/java/io/flutter/plugins/googlemaps/Convert.java Outdated Show resolved Hide resolved
...google_maps_flutter_android/android/src/main/java/io/flutter/plugins/googlemaps/Convert.java Outdated Show resolved Hide resolved
...google_maps_flutter_android/android/src/main/java/io/flutter/plugins/googlemaps/Convert.java Outdated Show resolved Hide resolved
...google_maps_flutter_android/android/src/main/java/io/flutter/plugins/googlemaps/Convert.java Outdated Show resolved Hide resolved
...le_maps_flutter_android/android/src/test/java/io/flutter/plugins/googlemaps/ConvertTest.java Outdated Show resolved Hide resolved
...lutter/google_maps_flutter_android/example/integration_test/resources/icon_image_base64.dart Show resolved Hide resolved
...tter/google_maps_flutter_ios/example/ios14/integration_test/resources/icon_image_base64.dart Show resolved Hide resolved
@jokerttu jokerttu force-pushed the feature/google_maps_flutter_platform_implementations_custom_marker_size_improvements branch 3 times, most recently from b5b5126 to f53caaa Compare June 3, 2024 20:24
@stuartmorgan stuartmorgan added triage-android Should be looked at in Android triage triage-ios Should be looked at in iOS triage labels Jun 3, 2024
@jokerttu jokerttu force-pushed the feature/google_maps_flutter_platform_implementations_custom_marker_size_improvements branch 2 times, most recently from 64941a9 to a8542ba Compare June 3, 2024 20:45
@jokerttu
Copy link
Contributor Author

jokerttu commented Jun 3, 2024 •
edited
Loading

@ditman @reidbaker @stuartmorgan @cbracken
I have addressed the review comments. It should now be ready for recheck.

@jokerttu jokerttu force-pushed the feature/google_maps_flutter_platform_implementations_custom_marker_size_improvements branch from ddc5ddd to 6226b9e Compare June 4, 2024 07:17
stuartmorgan
stuartmorgan approved these changes Jun 4, 2024
View reviewed changes
Copy link
Contributor

@stuartmorgan stuartmorgan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

...google_maps_flutter_android/android/src/main/java/io/flutter/plugins/googlemaps/Convert.java Outdated Show resolved Hide resolved
...aps_flutter/google_maps_flutter_ios/example/ios14/ios/RunnerTests/ExtractIconFromDataTests.m Outdated Show resolved Hide resolved
packages/google_maps_flutter/google_maps_flutter_ios/ios/Classes/GoogleMapMarkerController.m Outdated Show resolved Hide resolved
packages/google_maps_flutter/google_maps_flutter_ios/ios/Classes/GoogleMapMarkerController.m Outdated Show resolved Hide resolved
packages/google_maps_flutter/google_maps_flutter_ios/ios/Classes/GoogleMapMarkerController.m Outdated Show resolved Hide resolved
@stuartmorgan
Copy link
Contributor

This had initial iOS and Web approval in the main PR, and I've briefly reviewed everything, so this is good to land once the final comments are addressed. Thanks!

@jokerttu
Copy link
Contributor Author

jokerttu commented Jun 4, 2024

This had initial iOS and Web approval in the main PR, and I've briefly reviewed everything, so this is good to land once the final comments are addressed. Thanks!

Thanks, final comments are now addressed.

@stuartmorgan stuartmorgan added the autosubmit Merge PR when tree becomes green via auto submit App label Jun 4, 2024
@jokerttu jokerttu force-pushed the feature/google_maps_flutter_platform_implementations_custom_marker_size_improvements branch from ece090f to defaebd Compare June 4, 2024 18:15
@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jun 4, 2024
@auto-submit Auto-submit
Copy link
Contributor

auto-submit bot commented Jun 4, 2024

auto label is removed for flutter/packages/6826, due to - The status or check suite Mac_arm64 ios_platform_tests_shard_1 master has failed. Please fix the issues identified (or deflake) before re-applying this label.

@jokerttu jokerttu added the autosubmit Merge PR when tree becomes green via auto submit App label Jun 4, 2024
@auto-submit auto-submit bot merged commit dcd8586 into flutter:main Jun 4, 2024
74 checks passed
@jokerttu jokerttu deleted the feature/google_maps_flutter_platform_implementations_custom_marker_size_improvements branch June 5, 2024 12:40
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jun 5, 2024
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 5, 2024
@engine-flutter-autoroll
dcd8586 [google_maps_flutter] Custom marker size improvements - platf…
59c88cf 
…orm impls (flutter/packages#6826)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jun 5, 2024
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 5, 2024
@engine-flutter-autoroll
dcd8586 [google_maps_flutter] Custom marker size improvements - platf…
7f694a4 
…orm impls (flutter/packages#6826)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jun 5, 2024
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 5, 2024
@engine-flutter-autoroll
dcd8586 [google_maps_flutter] Custom marker size improvements - platf…
613c3a0 
…orm impls (flutter/packages#6826)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jun 6, 2024
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 6, 2024
@engine-flutter-autoroll
dcd8586 [google_maps_flutter] Custom marker size improvements - platf…
dff155f 
…orm impls (flutter/packages#6826)
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Jun 6, 2024
@engine-flutter-autoroll
Roll Packages from 11e192a to 586faa6 (13 revisions) (#149820)
b31781a 
flutter/packages@11e192a...586faa6

2024-06-05 ditman@gmail.com [google_sign_in_web] Update button_tester to use web_only library. (flutter/packages#6868)
2024-06-05 engine-flutter-autoroll@skia.org Roll Flutter from c246ecd to 27e0656 (17 revisions) (flutter/packages#6875)
2024-06-05 15619084+vashworth@users.noreply.github.com [path_provider] Skip verifying sample file on macOS (flutter/packages#6874)
2024-06-05 joonas.kerttula@codemate.com [google_maps_flutter] Custom marker size improvements (flutter/packages#4055)
2024-06-05 uberchilly@gmail.com [rfw] Material slider widget (flutter/packages#6610)
2024-06-04 ditman@gmail.com [ci] Manual roll Flutter to c246ecd (84 revisions) + fixes (flutter/packages#6863)
2024-06-04 molchanovia.dev@gmail.com Correcting the typo of Flutter in projects (flutter/packages#6850)
2024-06-04 joonas.kerttula@codemate.com [google_maps_flutter] Custom marker size improvements - platform impls (flutter/packages#6826)
2024-06-04 lrn@google.com Avoid cumbersome formatter workaround (flutter/packages#6573)
2024-06-04 15619084+vashworth@users.noreply.github.com Clean Xcode project before analyzing and testing (flutter/packages#6842)
2024-06-03 37848204+RobinCombrink@users.noreply.github.com [pigeon] Kotlin/Java method overloading for the `setUp` method (flutter/packages#6843)
2024-06-03 40719830+Alex-Usmanov@users.noreply.github.com [url_launcher] Add support for setting show title on Chrome Custom Tabs (flutter/packages#6097)
2024-06-03 ditman@gmail.com Revert "Roll Flutter from c85fa6a to 7eebe29 (#6836)" (flutter/packages#6860)

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 flutter-ecosystem@google.com,rmistry@google.com 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
arc-yong pushed a commit to Arctuition/packages-arc that referenced this pull request Jun 14, 2024
@jokerttu @arc-yong
[google_maps_flutter] Undeprecate BitmapDescriptor methods (flutter#6832
aa614f8 
)

Undeprecate BitmapDescriptor.fromAssetImage and BitmapDescriptor.fromBytes

* Undeprecates `BitmapDescriptor.fromAssetImage`.
* Undeprecates `BitmapDescriptor.fromBytes`.
* Fixes issues with deprecation in version 2.7.0.

The new formats won't be supported until flutter#6826 lands.
Deprecation notices can be re-added after the implementation PRs have landed.

Fixes: flutter/flutter#149183
arc-yong pushed a commit to Arctuition/packages-arc that referenced this pull request Jun 14, 2024
@jokerttu @arc-yong
[google_maps_flutter] Custom marker size improvements - platform impls (
9882d36 
flutter#6826)

Platform implementations portion of : flutter#4055

Adds platform handling for new BitmapDescriptor classes `AssetMapBitmap` and `BytesMapBitmap` introduced in flutter#6687

Containing only changes to packages
 * `google_maps_flutter_android`
 * `google_maps_flutter_ios`
 * `google_maps_flutter_web`

Follow up PR will hold the app-facing plugin implementations.

Linked issue: flutter/flutter#34657
@reidbaker reidbaker mentioned this pull request Jul 2, 2024
Closed
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@reidbaker reidbaker reidbaker approved these changes

@stuartmorgan stuartmorgan stuartmorgan approved these changes

@cbracken cbracken Awaiting requested review from cbracken cbracken is a code owner

@ditman ditman Awaiting requested review from ditman ditman is a code owner

Assignees
No one assigned
Labels
autosubmit Merge PR when tree becomes green via auto submit App p: google_maps_flutter platform-android platform-ios platform-web triage-android Should be looked at in Android triage triage-ios Should be looked at in iOS triage
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jokerttu @stuartmorgan @reidbaker