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

Conversation

mravn-google
Copy link
Contributor

  • Add marker API
  • Simplified example pages
  • Consolidated some Java code

@mravn-google mravn-google requested a review from sigurdm April 9, 2018 22:12
@mravn-google mravn-google self-assigned this Apr 9, 2018
Copy link
Contributor

@sigurdm sigurdm left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -182,6 +198,10 @@ private static LatLngBounds toLatLngBounds(Object o) {
return new LatLngBounds(toLatLng(data.get(0)), toLatLng(data.get(1)));
}

private static boolean toBoolean(Object o) {
Copy link
Contributor

Choose a reason for hiding this comment

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

neccesary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Made use of toXxx(...) consistent across all types.


void removeMarker(String markerId) {
final Marker marker = marker(markerId);
marker.remove();
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you also want to remove from markers?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, good catch. Done.

return new BitmapDescriptor._(<dynamic>['defaultMarker', hue]);
}

static BitmapDescriptor fromAsset(String assetName, [String package]) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Make package a named argument to align with AssetImage.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.


part of google_mobile_maps;

class BitmapDescriptor {
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps worthy of a short dart-doc as the name is very generic.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

Copy link
Contributor Author

@mravn-google mravn-google left a comment

Choose a reason for hiding this comment

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

Thanks for the review!

@@ -182,6 +198,10 @@ private static LatLngBounds toLatLngBounds(Object o) {
return new LatLngBounds(toLatLng(data.get(0)), toLatLng(data.get(1)));
}

private static boolean toBoolean(Object o) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Made use of toXxx(...) consistent across all types.


void removeMarker(String markerId) {
final Marker marker = marker(markerId);
marker.remove();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, good catch. Done.


part of google_mobile_maps;

class BitmapDescriptor {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

return new BitmapDescriptor._(<dynamic>['defaultMarker', hue]);
}

static BitmapDescriptor fromAsset(String assetName, [String package]) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@mravn-google mravn-google merged commit 5a95351 into flutter:master Apr 10, 2018
@mravn-google mravn-google deleted the marker_api branch April 10, 2018 09:46
slightfoot pushed a commit to slightfoot/plugins that referenced this pull request Jun 5, 2018
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