Skip to content
This repository has been archived by the owner on Dec 27, 2022. It is now read-only.

First feedback #1

Closed
ben-xD opened this issue Dec 19, 2022 · 3 comments
Closed

First feedback #1

ben-xD opened this issue Dec 19, 2022 · 3 comments

Comments

@ben-xD
Copy link

ben-xD commented Dec 19, 2022

Hi @motrieux-thomas, since this is the official mapbox package, it might get a lot of popularity in the future, I think it's quite important to mention some of the issues I noticed which might make it a lot nicer to use.

  • It's quite hard to notice or find this repo, which is probably the source of https://pub.dev/packages/mapbox_maps_flutter. Please make the real repository available, and please link to the repo from pubspec.yaml, so that we can find this repo on pub.dev more easily to contact you.
  • 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.
    CleanShot 2022-12-19 at 18 51 52@2x
  • 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.
@motrieux-thomas
Copy link
Owner

motrieux-thomas commented Dec 19, 2022

Hi @ben-xD

I'm not a developer at mapbox, I uploaded the pub package on my Github so I could make changes while waiting for the official package to be open-source and be able to fork it. (The team say that will happen soon)

I'll change the README to avoid confusion in the future.

However, You could send your feedback to the team on this discord channel : https://discord.gg/paz23RrV

And BTW, I'm 100% agree with your feedback!

@ben-xD
Copy link
Author

ben-xD commented Dec 19, 2022

Thanks for the quick reply @motrieux-thomas. Also thanks for the link.

What's interesting is mapbox used to have a package for flutter, and then abandoned it? mapbox/flutter-mapbox-gl#75 🤔

@motrieux-thomas
Copy link
Owner

motrieux-thomas commented Dec 19, 2022

You're welcome ;) @ben-xD
The package you mention was moved here : https://github.com/flutter-mapbox-gl/maps and was maintained by the community.
The SDK behind are out to date (that cause many performance issue an android for instance), this is why a migration was needed : flutter-mapbox-gl/maps#691

Mapbox released an official package two weeks ago.
The lib' can of course be improve but, in my experience (I migrated an app from one to another), it's way much better than the previous community one especially because the API looks like the officials Android and iOS SDK :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants