gabesmed / ember-leaflet

Ember + Leaflet = Fun with maps
gabesmed.github.io/ember-leaflet
MIT License
164 stars 35 forks source link

Showing a collection in a single ember-data object #72

Closed muziejus closed 10 years ago

muziejus commented 10 years ago

Hey, I've had a lot of fun trying to get ember-leaflet to work in this project I'm working on, but now I'm getting profoundly stumped and actively frustrated.

So it seems to be the case that a MarkerCollectionLayer requires an ArrayController, whereas a MarkerLayer works with an ObjectController. So I can have one map with a marker collection layer, attached to addresses.index that shows each marker associated with each instance of the Address model (via a computed location property that returns a latLng), and another map, with a marker layer, that shows the specific marker associated with a specific instance of Address in addresses.show. So far, so good.

However, I have another model, Individual, which hasMany('address', {async:true}). In individuals.show, I can see a text rendering of the content of the Address objects via an {{each}}, but I can't, for the life of me, figure out how to get those three (say) Address markers to show up in the IndividualMapView for an Individual. Conceptually, I know I need to be giving the layer an array of locations, but if I can't even get a MarkerCollectionLayer with just one item in it—defined in IndividualsShowController—to show a solitary marker, then I think that there's no chance of my showing a bunch.

So is my assumption correct, that collection layers "only" work with array controllers? And if it's wrong, how I do I get around it? I'm not very good at ember, etc., so apologies if the answer is very simple.

Thanks!

Here's what's not working:

App.MarkerCollectionLayer = EmberLeaflet.MarkerCollectionLayer.extend({
  contentBinding: "controller"
});
App.MarkerLayer = EmberLeaflet.MarkerLayer.extend({
  contentBinding: "controller"
});
App.MapView = EmberLeaflet.MapView.extend({ //"parent" view for the app
  classNames: ['ember-leaflet-map'],
  center: L.latLng(40.713282, -74.006978),
  content: Ember.computed.alias('controller'),
  didCreateLayer: function() {
    this._super();
    this._layer.panTo(this.get("content.location"));
  },
  childLayers: [
    App.TileLayer,
    App.MarkerCollectionLayer
  ],
  zoom: 13
});
App.IndividualMapView = App.MapView.extend({
  childLayers: [
    App.TileLayer,
    App.MarkerCollectionLayer
    // App.MarkerLayer
  ]
});
App.IndividualsShowController = Ember.ObjectController.extend({
  location: L.latLng(40.714, -74.050)
});

Yet when I replace App.MarkerCollectionLayer with App.MarkerLayer in App.InvididualMapView, the marker does appear.

muziejus commented 10 years ago

This was complex, but I solved it.

First, I have the Individual create a locations computed property (is there a cleaner way to do this? I couldn't figure out how to do with .mapBy):

App.Individual = DS.Model.extend({
...
addresses: DS.hasMany('address', {async: true}),
locations: function(){
    array = [];
    this.get('addresses').forEach(function(item, index){
      array.push({location: item.get('location')});
    });
    return array;
  }.property('addresses.@each.location')
});

Next, I tell the map view to use a different controller:

App.IndividualMapView = App.MapView.extend({
  controller: App.IndividualsAddressesController.create({
    container: App.__container__}),
  childLayers: [
    App.TileLayer,
    App.MarkerCollectionLayer
  ]
});

Next, I define that new controller that is an ArrayController, and, hence, will work with a MarkerCollectionLayer:

App.IndividualsAddressesController = Ember.ArrayController.extend({
  needs: ['individualsShow'],
  content: Ember.computed.alias("controllers.individualsShow.locations")
});

And it works. Single Individual, with three markers corresponding to three different Addresses on the map!

miguelcobain commented 10 years ago

A slightly cleaner way to write that computed property would be:

locations: function(){
   return this.get('addresses').map(function(item){
      return {location: item.get('location')};
   });
}.property('addresses.@each.location')

I didn't have the time to fully analyse your problem, sorry, but I don't like that reference to App.__container__.

muziejus commented 10 years ago

Thanks for the tip with map(). I'm not really familiar with enumerable syntax in JS.

As for App.__container__, I don't like it either, but without it, I get a complaint about the needs. The idea came from here:

http://stackoverflow.com/questions/24169955/please-ensure-this-controller-was-instantiated-with-a-container

because that was the error I was getting.

gabesmed commented 10 years ago

Thanks for helping @miguelcobain!

An even simpler way, I'm pretty sure (new syntax), is: locations: Ember.computed.mapBy('addresses', 'location')

Agreed that App.__container__ is not idiomatic. Couldn't you just have:

App.IndividualMapView = App.MapView.extend({...});
App.IndividualMapController = Ember.ArrayController.extend({
  needs: ['individualsShow'],
  content: Ember.computed.alias("controllers.individualsShow.locations")
});

? then mapView.controller would automatically map to the controller you want?

miguelcobain commented 10 years ago

I think he wanted an array with objects with a location property. That's why I didn't use mapBy.

That way you get an array with all the location properties, which may also be appropriate for this case. However it's different from the original code.

muziejus commented 10 years ago

Thanks for this feedback! @miguelcobain is right that I was trying to match the examples given by @gabesmed as closely as possible, hence, [{location: LatLng object}...]. But the property is also sending more than just the location, so it "really" looks like this:

locations: function(){
  name = this.get('firstName') + " " + this.get('lastName');
  return this.get('addresses').map(function(item){
    return {location: item.get('location'), popupContent: name + "<br>" + item.get('street')};
  });
}.property('addresses.@each.location', 'addresses.@each.street', 'firstName', 'lastName')

With @gabesmed's suggestion, I get an Uncaught TypeError pointing to line 791 of ember-leaflet (inside _contentDidChange). This is the code:

App.IndividualMapController = Ember.ArrayController.extend({
  needs: ['individualsShow'],
  content: Ember.computed.alias("controllers.individualsShow.locations")
});
App.IndividualMapView = App.MapView.extend({
  // controller: App.IndividualMapController.create(),
  // controller: App.IndividualsAddressesController.create({
  //   container: App.__container__}),
  childLayers: [
    App.TileLayer,
    App.IndividualMarkerCollectionLayer
  ]
});

If I uncomment either of the controller: properties, I get the "specifies 'needs', but does not have a container" error again.

gabesmed commented 10 years ago

@miguelcobain ha totally right, i didn't see that @muziejus got it. Yeah you don't want to manually create controllers..but i think you're on the right track! If we can help any more, maybe you could make a minimal jsfiddle for us to look at? Otherwise you've figured it out and that we could be helpful!

muziejus commented 10 years ago

I tried to make a jsfiddle, but I couldn't get it to work, ha ha. I'll try again.

I think these are important things to look at, because the power of ember-leaflet will, imo, grow once its is more closely hooked into ember-data. My use case described here, I imagine, is not so bizarre as to require non-idiomatic controller creation. I wish, for example, that there were examples that took this into account in the documentation (though I know that's a WIP that I could theoretically be a part of, though I don't know if I have a good enough understanding to start documenting). But this is getting off topic. What I've got works for what I need (and I can even extrapolate it).