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.

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

[DO NOT MERGE] Feat/google maps flutter ground overlay support #9

Closed

Conversation

illuminati1911
Copy link
Collaborator

Replace this paragraph with a description of what this PR is changing or adding, and why. Consider including before/after screenshots.

List which issues are fixed by this PR. You must list at least one issue.

Pre-launch Checklist

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

@jokerttu jokerttu force-pushed the feat/google_maps_flutter_ground_overlay_support branch from db7e48b to b04e31f Compare December 20, 2024 12:53
@jokerttu jokerttu force-pushed the feat/google_maps_flutter_ground_overlay_support branch 2 times, most recently from 316f241 to 0d259ee Compare January 9, 2025 09:48
Copy link
Collaborator

@aednlaxer aednlaxer left a comment

Choose a reason for hiding this comment

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

Looks good! Several code and comments related comments

Comment on lines 101 to 102
/// Returns the [GroundOverlay] with the given [GroundOverlayId] for testing
/// purposes.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wonder if "for testing purposes" could be used along with the @visibleForTesting annotation?

Choose a reason for hiding this comment

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

Yes, it would be nice, but this is used in GoogleMapsInspectorWeb which is not test file... even when it is meant to be used as testing support class only.
causing following linting error:

The member 'getGroundOverlay' can only be used within 'package:google_maps_flutter_web/src/ground_overlays.dart' or a test.dart[invalid_use_of_visible_for_testing_member](https://dart.dev/diagnostics/invalid_use_of_visible_for_testing_member)

@jokerttu
Copy link

Thanks, @aednlaxer!
I've addressed the review comments.

jokerttu and others added 15 commits January 14, 2025 09:15
* test: add initial ios integration tests

* fix integration tests for iOS

* fix: add zoom level assert to ios groundoverlays

* test: ios ground overlay unit tests

* test: refactor integration tests

* refactor: pr fixes and refactoring

---------

Co-authored-by: Joonas Kerttula <[email protected]>
add assert if bitmapScaling is not MapBitmapScaling.none
…id/src/main/java/io/flutter/plugins/googlemaps/Convert.java

Co-authored-by: Alexander Troshkov <[email protected]>
…rc/google_maps_flutter_android.dart

Co-authored-by: Alexander Troshkov <[email protected]>
…rc/google_maps_flutter_android.dart

Co-authored-by: Alexander Troshkov <[email protected]>
@jokerttu jokerttu force-pushed the feat/google_maps_flutter_ground_overlay_support branch from 2edb03a to 6c72f3f Compare January 14, 2025 12:38
@jokerttu jokerttu closed this Jan 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants