Feedback

[ben-xD]

I added some feedback in a Github repo before this one was made public: motrieux-thomas/mapbox_maps_flutter-0.3.0#1 (comment).

I think it would be good to address them:

• The API for setting location is quite weird. We don't actually have a toJson function on Point. It's not exactly clear how the Map should look. Please consider using a LatLng (or find a better name) type which you provide the serialization for (rather than asking us to provide a JSON). Instead of:

  /// Coordinate at the center of the camera.
  Map<String?, Object?>? center;

do

LatLng center;

• Package documentation assumes people have turf (https://pub.dev/packages/turf) installed, since that is how they can do Position(-80.1263, 25.7845)).toJson(). This is kind of confusing, since docs don't mention turf. It would be good to avoid asking developers to instead turf though.
  

• Please consider renaming the controller type to MapController or MapboxController. It's confusing that the map widget is MapWidget, and the controller is MapboxMap. The convention is to suffix names Controller if they are controllers.

[cedvdb]

MapboxView & MapboxController would be more idiomatic

[lukas-h]

Hi @ben-xD,

you actually don't have to use turf here. The underlying data standard is GeoJSON. GeoJSON is itself based on JSON to represent geo data.
And if you want GeoJSON in a serialized Dart-representation, it consists of Maps, Lists, and numbers.

So the GeoJSON that you want to inject into the CameraOptions.center attribute can look like this (without turf library):

CameraOptions(
  center: {
    "type": "Point",
    "coordinates": [-80.1263, 25.7845]
  }
  // ...
),
// ...

Or isolated:

{
    "type": "Point",
    "coordinates": [-80.1263, 25.7845]
}

What turf simply provides, is an optional to use class-object representation of GeoJSON. Check it out in the README of the turf library https://github.com/dartclub/turf_dart.

The output of the Point.toJson method is the same Dart Map/List/number representation as shown in my example.

The benefits of using turf with Mapbox can be static analysis and runtime checks, to make sure if the GeoJSON data structures are valid.

[lukas-h]

LatLng center;

A new data type LatLng could add a lot of confusion, because:

• It does not really correspond to the Point or Position/coordinate data type from the GeoJSON RFC standard, that Mapbox is using for the representation of the geo data.
• The actual order of the Position's/coordinate's dimensions are: [LONGITUDE, LATITUDE, ALTITUDE (optional)] See here RFC. So to stay compliant with the GeoJSON standard regarding the order of the dimensions is important.

Though I totally see, that it would make sense to have more type information than just Map<String?, Object?> ("a Map with some String? keys and some Object? values")...

[ben-xD]

If we stick to using GeoJSON, won't we always have this problem? The 2 points mentioned in #20 (comment) can be ignored if we just drop the GeoJSON requirement.

I think users would have a nicer time if they use types rather than need to create untyped JSONs. Why don't we use types provided by library?