Flutter_map: Planning version 0.8

Created on 29 Aug 2019  路  12Comments  路  Source: fleaflet/flutter_map

This issue is to discuss priorities in the next version of the plugin.
The issues selected here should not preclude any PR merge, it's just a discussion to select which issues are necessary for getting at 0.8 release.

Most helpful comment

Hi,

I implemented my own map view recently because we needed a completely different coordinate system for this app but I looked a lot at yours to get inspirations.
One thing that I always didn't like was the way the Layers were created with flutter_map and I decided here to go a different way which I want to show you

This here is my mapview (I removed unimportant stuff):

class LocalMapView extends StatefulWidget {
  final LocalMapOptions options;
  final List<LayerDefinition> layers;

  const LocalMapView({Key key, this.options, this.layers}) : super(key: key);
  @override
  LocalMapViewState createState() => LocalMapViewState();
}

As you can see I add factories for my layers als parameter here. LayerDefinition id defined:

abstract class LayerDefinition {
  Widget createLayer({
    LocalMapOptions options,
    CustomPoint<double> centerInMeter,
    double scale,
    CustomPoint<double> canvasSize,
  });
}

abstract class Layer  {
  final LocalMapOptions mapOptions;
  final CustomPoint<double> centerInMeter;
  final double scale;
  final CustomPoint<double> canvasSize;

  Layer(
      {Key key,
      this.mapOptions,
      this.centerInMeter,
      this.scale,
      this.canvasSize});
}

and for the different layers you derive a new LayerDefinition which can get any additional parameters like:

class PolylineLayerDefinition implements LayerDefinition {
  Stream<List<Polyline>> polylineUpdates;

  PolylineLayerDefinition({this.polylineUpdates});
  @override
  Widget createLayer(
      {LocalMapOptions options,
      CustomPoint<double> centerInMeter,
      double scale,
      CustomPoint<double> canvasSize}) {
    return PolylineLayer(
      polylineUpdates: polylineUpdates,
      canvasSize: canvasSize,
      centerInMeter: centerInMeter,
      mapOptions: options,
      scale: scale,
    );
  }
}
````
This approach is much cleaner and more direct than the current one with a big switch to check which type the LayerOption is and then  call a factory function.
By this I can easy pass in a Stream to forward any data changes only the Layer it concerns instead to rebuild the whole map_view.

In the map_view build function it looks like this:

```Dart
      var layerStack = Stack(
        children: widget.layers
            .map((layerDef) => layerDef.createLayer(
                options: options,
                canvasSize: canvasSize,
                centerInMeter: centerMeter,
                scale: scale))
            .toList(),
      );

      Widget mapRoot;

      if (!options.interactive) {
        mapRoot = layerStack;
      } else {
        mapRoot = GestureDetector(
          onScaleStart: handleScaleStart,
          onScaleUpdate: handleScaleUpdate,
          onScaleEnd: handleScaleEnd,
          child: layerStack,
        );
   return mapRoot;   
}

When using this MapView it looks like this:

              return LocalMapView(
                options: LocalMapOptions(
                  initialCenter: LatLng(map.latitude, map.longitude),
                  mapCenter: LatLng(map.latitude, map.longitude),
                  initialScale: 0.5,
                  maxScale: 2,
                  minScale: 0.05,
                  mPerPixel: map.mpp,
                  initialRotation: 0,
                  mapWidthPixel: map.width,
                  mapHeightPixel: map.height,
                ),
                layers: [
                  TileLayerDefinition(
                    tileImageProvider: mapTileService.getTile,
                  ),
                  PolygonLayerDefinition(
                      polygonUpdates:
                          polyStreamController.stream.asBroadcastStream()),
                  MarkerLayerDefinition(
                    markerUpdates:
                        markerStreamController.stream.asBroadcastStream(),
                  ),
                  PolylineLayerDefinition(
                    polylineUpdates:
                        lineStreamController.stream.asBroadcastStream(),
                  ),
                ],
              );
````

I just wanted to show this as a starting point if the package could move in this direction instead of moving all to plugins. The direct passing of LayerFactories is so much easier to understand and more powerfull than this here:

```Dart
  Widget _createLayer(LayerOptions options, List<MapPlugin> plugins) {
    if (options is TileLayerOptions) {
      return TileLayer(
          options: options, mapState: mapState, stream: _merge(options));
    }
    if (options is MarkerLayerOptions) {
      return MarkerLayer(options, mapState, _merge(options));
    }
    if (options is PolylineLayerOptions) {
      return PolylineLayer(options, mapState, _merge(options));
    }
    if (options is PolygonLayerOptions) {
      return PolygonLayer(options, mapState, _merge(options));
    }

Which requires that all Layer types have to use the same parameters and to pass any other options over an Options object.

Please don't get me wrong, you created an awesome package. Perhaps we can improve the architecture a bit.

Cheers
Thomas

All 12 comments

IMO the discussion should start from https://github.com/johnpryan/flutter_map/issues/174 .
Getting this right would at least get the package in the right track to address some other issues, even close some.
For sure this (probably) breaking change should be done as soon as possible, the longer the wait the harder it will get.

The one I'm seeing as related and why:

The problem is that this change needs to be well thought out as getting this wrong would be nasty.

edit: writing is hard.

Should 0.8 remove isUserGesture as said here https://github.com/johnpryan/flutter_map/issues/241 ?

389 already removed it according to changelog

You are completely right.

Hi,

I implemented my own map view recently because we needed a completely different coordinate system for this app but I looked a lot at yours to get inspirations.
One thing that I always didn't like was the way the Layers were created with flutter_map and I decided here to go a different way which I want to show you

This here is my mapview (I removed unimportant stuff):

class LocalMapView extends StatefulWidget {
  final LocalMapOptions options;
  final List<LayerDefinition> layers;

  const LocalMapView({Key key, this.options, this.layers}) : super(key: key);
  @override
  LocalMapViewState createState() => LocalMapViewState();
}

As you can see I add factories for my layers als parameter here. LayerDefinition id defined:

abstract class LayerDefinition {
  Widget createLayer({
    LocalMapOptions options,
    CustomPoint<double> centerInMeter,
    double scale,
    CustomPoint<double> canvasSize,
  });
}

abstract class Layer  {
  final LocalMapOptions mapOptions;
  final CustomPoint<double> centerInMeter;
  final double scale;
  final CustomPoint<double> canvasSize;

  Layer(
      {Key key,
      this.mapOptions,
      this.centerInMeter,
      this.scale,
      this.canvasSize});
}

and for the different layers you derive a new LayerDefinition which can get any additional parameters like:

class PolylineLayerDefinition implements LayerDefinition {
  Stream<List<Polyline>> polylineUpdates;

  PolylineLayerDefinition({this.polylineUpdates});
  @override
  Widget createLayer(
      {LocalMapOptions options,
      CustomPoint<double> centerInMeter,
      double scale,
      CustomPoint<double> canvasSize}) {
    return PolylineLayer(
      polylineUpdates: polylineUpdates,
      canvasSize: canvasSize,
      centerInMeter: centerInMeter,
      mapOptions: options,
      scale: scale,
    );
  }
}
````
This approach is much cleaner and more direct than the current one with a big switch to check which type the LayerOption is and then  call a factory function.
By this I can easy pass in a Stream to forward any data changes only the Layer it concerns instead to rebuild the whole map_view.

In the map_view build function it looks like this:

```Dart
      var layerStack = Stack(
        children: widget.layers
            .map((layerDef) => layerDef.createLayer(
                options: options,
                canvasSize: canvasSize,
                centerInMeter: centerMeter,
                scale: scale))
            .toList(),
      );

      Widget mapRoot;

      if (!options.interactive) {
        mapRoot = layerStack;
      } else {
        mapRoot = GestureDetector(
          onScaleStart: handleScaleStart,
          onScaleUpdate: handleScaleUpdate,
          onScaleEnd: handleScaleEnd,
          child: layerStack,
        );
   return mapRoot;   
}

When using this MapView it looks like this:

              return LocalMapView(
                options: LocalMapOptions(
                  initialCenter: LatLng(map.latitude, map.longitude),
                  mapCenter: LatLng(map.latitude, map.longitude),
                  initialScale: 0.5,
                  maxScale: 2,
                  minScale: 0.05,
                  mPerPixel: map.mpp,
                  initialRotation: 0,
                  mapWidthPixel: map.width,
                  mapHeightPixel: map.height,
                ),
                layers: [
                  TileLayerDefinition(
                    tileImageProvider: mapTileService.getTile,
                  ),
                  PolygonLayerDefinition(
                      polygonUpdates:
                          polyStreamController.stream.asBroadcastStream()),
                  MarkerLayerDefinition(
                    markerUpdates:
                        markerStreamController.stream.asBroadcastStream(),
                  ),
                  PolylineLayerDefinition(
                    polylineUpdates:
                        lineStreamController.stream.asBroadcastStream(),
                  ),
                ],
              );
````

I just wanted to show this as a starting point if the package could move in this direction instead of moving all to plugins. The direct passing of LayerFactories is so much easier to understand and more powerfull than this here:

```Dart
  Widget _createLayer(LayerOptions options, List<MapPlugin> plugins) {
    if (options is TileLayerOptions) {
      return TileLayer(
          options: options, mapState: mapState, stream: _merge(options));
    }
    if (options is MarkerLayerOptions) {
      return MarkerLayer(options, mapState, _merge(options));
    }
    if (options is PolylineLayerOptions) {
      return PolylineLayer(options, mapState, _merge(options));
    }
    if (options is PolygonLayerOptions) {
      return PolygonLayer(options, mapState, _merge(options));
    }

Which requires that all Layer types have to use the same parameters and to pass any other options over an Options object.

Please don't get me wrong, you created an awesome package. Perhaps we can improve the architecture a bit.

Cheers
Thomas

177

Hopefully #458 would be taken under consideration when 0.8 is planned.

Thanks for the input everyone - 0.8.0 has been released (I bumped the minor version since Flutter 1.12 was just released).

I would like everyone to keep in mind that flutter_map is a community-maintained project. If you want a feature to be added, the best way is to make a pull request. The second-best way is to make a file a very clear and detailed issue, preferably with a link to the Leaflet equivalent, since this project is based on Leaflet.

If you are blocked by me, I'm happy to add you as a collaborator. Reach out to me and give me some examples of some pull requests or issues that you've made. the Kanban board is what I would like to use to track things. If you're interested in working on an issue, call it out in a comment and I will assign you.

happy flutter_mapping!

@johnpryan I'm actually a bit disappointed that I didn't get any feedback on my proposal to improve the overall architecture from you.

@escamoteur I think it got a little lost in the shuffle. Maybe you could file a new issue and link to it here so that people can give feedback

Was this page helpful?
0 / 5 - 0 ratings