-
Notifications
You must be signed in to change notification settings - Fork 3.6k
[google_maps_flutter][iOS 17] takeSnapshot FIX #5823
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][iOS 17] takeSnapshot FIX #5823
Conversation
packages/google_maps_flutter/google_maps_flutter_ios/CHANGELOG.md
Outdated
Show resolved
Hide resolved
packages/google_maps_flutter/google_maps_flutter_ios/ios/Classes/GoogleMapController.m
Show resolved
Hide resolved
|
Now i just changed 2.3.5 to 2.3.6 for the package version. @hellohuanlin |
hellohuanlin
left a comment
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 contributing. Can you add the comment as we discussed in my previous review?
packages/google_maps_flutter/google_maps_flutter_ios/CHANGELOG.md
Outdated
Show resolved
Hide resolved
|
Sorry for being inactive here for weeks. I am solving all these points today :) |
packages/google_maps_flutter/google_maps_flutter_ios/ios/Classes/GoogleMapController.m
Outdated
Show resolved
Hide resolved
…hub.com/GavrielRosendorn74/packages into google_maps_flutter_ios_17_snapshot_fix
|
Hi @stuartmorgan ! How are you ? Is there some changes to add to this PR ? |
I won't know that until I have reviewed it, which I have not done in the one working day I've had since I marked it for review. Please see https://github.com/flutter/flutter/wiki/Tree-hygiene for expectations around review turnaround time. |
Got it ! Thanks ! :) |
packages/google_maps_flutter/google_maps_flutter_ios/CHANGELOG.md
Outdated
Show resolved
Hide resolved
packages/google_maps_flutter/google_maps_flutter_ios/CHANGELOG.md
Outdated
Show resolved
Hide resolved
packages/google_maps_flutter/google_maps_flutter_ios/ios/Classes/GoogleMapController.m
Outdated
Show resolved
Hide resolved
|
Hi @stuartmorgan. All the fixes you have asked for are done :) |
stuartmorgan-g
left a comment
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 LGTM, but the CI failures here need to be fixed before this can land.
|
@stuartmorgan that's it i think on this time ! 🚀 |
stuartmorgan-g
left a comment
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
Skips `testTakeSnapshot` test re-enabeld by #5823, but causing failures and blocking the `flutter/packages` tree, e.g. https://ci.chromium.org/ui/p/flutter/builders/prod/Mac_arm64%20ios_platform_tests_shard_2%20master/1697/overview.
flutter/packages@9385bbb...a864254 2024-02-14 [email protected] Allow deprecated members from the Dart SDK/Flutter Framework to roll in (flutter/packages#6111) 2024-02-14 [email protected] [google_maps_flutter][iOS 12] Skip `testTakeSnapshot` (flutter/packages#6120) 2024-02-13 [email protected] [ci] Allow dependencies on local_auth_ios (flutter/packages#6116) 2024-02-13 [email protected] [url_launcher] Add `InAppBrowserConfiguration` parameter in implementations (flutter/packages#5759) 2024-02-13 [email protected] [flutter_markdown] Use Text.rich to replace RichText in Flutter Markdown (flutter/packages#6062) 2024-02-13 [email protected] [google_maps_flutter][iOS 17] takeSnapshot FIX (flutter/packages#5823) 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
Skips `testTakeSnapshot` test re-enabeld by flutter/packages#5823, but causing failures and blocking the `flutter/packages` tree, e.g. https://ci.chromium.org/ui/p/flutter/builders/prod/Mac_arm64%20ios_platform_tests_shard_2%20master/1697/overview.
I have fixed the function takeSnapShot for IOS17.
Here you got some screenshots of before now :
Before :
Now :
This issue was fixed by this PR : #139733
Pre-launch Checklist
dart format.)[shared_preferences]pubspec.yamlwith an appropriate new version according to the [pub versioning philosophy], or this PR is [exempt from version changes].CHANGELOG.mdto add a description of the change, [following repository CHANGELOG style].///).