Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.

Conversation

@iskakaushik
Copy link
Contributor

This doesn't yet expose the full projection functionality but
addresses the most common usecases to translate screen coordinates
to latlngs.

Issue: flutter/flutter#37959

This doesn't yet expose the full projection functionality but
addresses the most common usecases to translate screen coordinates
to latlngs.

Issue: flutter/flutter#37959
Copy link
Contributor

@mklim mklim left a comment

Choose a reason for hiding this comment

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

LGTM

}
case "map#getScreenCoordinate":
{
if (googleMap != null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

subjective, optional: Consider using early returns/guard clauses here instead to reduce nesting.

https://testing.googleblog.com/2017/06/code-health-reduce-nesting-reduce.html

You're doing this in multiple places, so it may even be worth extracting this out into a helper method that takes result and calls error for you and returns false if map isn't initialized. That way your checks could just be one liners guarding each method. This is just a totally optional style subjection though.

if (!assertGoogleMapInitialized(/*methodName=*/"getVisibleRegion", result)) {
  return;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Interesting approach. I like the idea. I will revisit this in a follow-up PR.

/// Return [ScreenCoordinate] of the [LatLng] in the current map view.
///
/// A projection is used to translate between on screen location and geographic coordinates.
/// Screen location is in screen pixels (not display pixels) with respect to the top left corner
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this would be useful information on ScreenInformation's DartDoc, too. I think it would be helpful to also say on the x and y members how the coordinate system works, eg is 0,0 top left or center?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated this.

@iskakaushik iskakaushik merged commit 6887acf into flutter:master Sep 25, 2019
mormih pushed a commit to mormih/plugins that referenced this pull request Nov 17, 2019
…2108)

[google_maps_flutter] Add Projection methods to google_maps

This doesn't yet expose the full projection functionality but
addresses the most common use-cases to translate screen coordinates
to latlngs.

Issue: flutter/flutter#37959
sungmin-park pushed a commit to sungmin-park/flutter-plugins that referenced this pull request Dec 17, 2019
…2108)

[google_maps_flutter] Add Projection methods to google_maps

This doesn't yet expose the full projection functionality but
addresses the most common use-cases to translate screen coordinates
to latlngs.

Issue: flutter/flutter#37959
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants