Closed miguelcobain closed 9 years ago
@AW-UC and people who are using this branch of ember-leaflet:
What is your general strategy for passing data to the {{leaflet-map}}
component?
I first extend the component with options I need for that specific map view. Then I pass data into the component by binding a property like {{leaflet-map place=model variables=leafletService}}
(you could e.g. also bind to properties of the route). Then I can acces those properties in the extended component like this: this.get('place').get('latitude')
etc. Sometimes it can also be useful to not use models but have a service where you get and set values. You can inject it into the component and access it the same way you access models.
Here is a more detailed description on how to deal with it:
Let's say wh have a model place
, with attributes like lat
and lng
(floating point). I now add a computed property to it like this:
//app/models/place.js
/* global L */
import DS from 'ember-data';
export default DS.Model.extend({
//...other attributes...
location: function() {
var x = this.get('lat');
var y = this.get('lng');
return L.latLng(x, y);
}.property('lat', 'lng')
});
I want to render the map on places/{id}
. On the corresponding js for the route I include something like:
export default Ember.Route.extend({
model: function() {
return this.store.find('place');
}
});
Now in the template for this route I call the component:
{{leaflet-map place=model tileUrl=leafletService.tileUrl}}
If you don't want to do much with place
in the component, then it would be even better to just set all input on the render call:
{{leaflet-map location=model.location tileUrl=leafletService.tileUrl}}
Then the get
calls in the component would be even simpler.
A correspondent component would look like this:
/* global L */
import EmberLeafletComponent from 'ember-leaflet/components/leaflet-map';
import MarkerCollectionLayer from 'ember-leaflet/layers/marker-collection';
import TileLayer from 'ember-leaflet/layers/tile';
export default EmberLeafletComponent.extend({
center: function(){
return this.get('place').get('location');
}.property('location'),
childLayers: [
TileLayer.extend({
tileUrl: function(){
return this.controller.get('tileUrl');
}.property('tileUrl'),
}),
MarkerCollectionLayer.extend({
content: function(){
return [
{location: this.controller.get('place').get('location')}
];
}.property('location')
})
]
});
I think this is the cleanest approach (fat models, skinny controllers). If for some reason I want all logic in my component (so no computed properties in the model or elsewhere) I can include a data function in my component:
data: function(){
var x = this.get('place').get('lat');
var y = this.get('place').get('lng');
return {
location: L.latLng(x, y)
};
}.property('place')
Then I can call in the component:
this.get('data').location
Generally, note that it is this.get('place')
in the component extension and this.controller.get('place')
in the embedded extensions.
I can also set some model indepedant and/or more app specific stuff in a service that I inject in the application or in some routes:
//app/services/leafletService.js
import Ember from 'ember';
export default Ember.Service.extend({
tileUrl: "url"
});
How can you do this.controller.get('tileUrl');
in a component?
I thought a component was isolated from its controllers.
Quoting ember docs:
An Ember.Component is a view that is completely isolated. Properties accessed in its templates go to the view object and actions are targeted at the view object. There is no access to the surrounding context or outer controller; all contextual information must be passed in.
@miguelcobain I'm not sure about this. I think it is mainly a naming issue: The controller here is not the "real" controller, but refers to the scope of the component itself. Thus, if you extend TileLayer
in the component, controller
would be the components scope. Maybe there is also another way to make this work, but I haven't found it, yet. Do you know of any?
In other words: controller
is not the outer controller, but the inner controller.
Ah, probably some legacy code from when this was a view.
You probably can replace:
TileLayer.extend({
tileUrl: function(){
return this.controller.get('tileUrl');
}.property('tileUrl'),
}),
with
TileLayer.extend({
tileUrl: this.get('tileUrl')
})
No, that would give you:
`this` at the top level is undefined
You have to use it in a function. And then I could only make it work with controller
, because the context has changed.
How about something like this?
/* global L */
import Ember from 'ember';
import EmberLeafletComponent from 'ember-leaflet/components/leaflet-map';
import MarkerCollectionLayer from 'ember-leaflet/layers/marker-collection';
import TileLayer from 'ember-leaflet/layers/tile';
var MyTileLayer = TileLayer.extend({
tileUrl: Ember.computed.reads('leafletService.tileUrl')
});
var MyMarkerCollectionLayer = MarkerCollectionLayer.extend({
content: Ember.computed('place.location', function () {
return [{ location: this.get('place.location') }];
})
});
export default EmberLeafletComponent.extend({
leafletService: Ember.inject.service('leaflet-service'),
center: Ember.computed.reads('place.location'),
tileLayer: Ember.computed('leafletService', function () {
return MyTileLayer.create({
leafletService: this.get('leafletService')
});
}),
markerCollectionLayer: Ember.computed('place', function () {
return MyMarkerCollectionLayer.create({
place: this.get('place')
});
}),
childLayers: Ember.computed.collect('tileLayer', 'markerCollectionLayer')
});
{{leaflet-map place=model}}
export default Ember.Route.extend({
model: function() {
return this.store.find('place');
}
});
Updated my comment to fix the computed property in MyMarkerCollectionLayer
.
@csantero that is also nice. I will try your way and see how I like it.
Very smart things there. But I really hate all this computed property madness. :cry:
Edit: Don't like the service either. Seems like we're fighting ember/ember-leaflet.
@miguelcobain Could you elaborate?
Well, after looking into this I am also in favor of the Ember.computed
syntax, because it doesn't depent on prototype extensions. Note that the syntax will change, because of https://github.com/emberjs/rfcs/pull/11. You can try the new syntax via: https://github.com/rwjblue/ember-new-computed.
Regarding injecting the service into the component: in most cases I am not in favor of it, because it reduces reusability of the component. Think of:
{{leaflet-map tileUrl=leafletService.urlA}}
{{leaflet-map tileUrl=leafletService.urlB}}
No need to change anything in the component or use conditionals in it in order to render two maps with different tile sources. Generally, I'm more and more leaning toward passing every single value via the template. I would only pass the whole model, if I want to do a lot with it in the component. So it would be like:
{{leaflet-map location=model.location name=model.name}}
This way you get leaner and more reusable components.
@csantero What is you reason for using create
instead of extend
?
@miguelcobain Yet another approach in your examples. Personally, I wouldn't do it like that, because you make heavy use of controllers. Ember is moving towards dropping controllers and using routable components instead (see https://github.com/emberjs/rfcs/pull/15).
Regarding injecting the service into the component: in most cases I am not in favor of it, because it reduces reusability of the component.
Exactly.
Ember is moving towards dropping controllers and using routable components instead
When routable components land, the controllers will become components, and the templates will become the components template. I see routable components as, mainly, non reusable components. But maybe I'm wrong. I believe migration will not be complicated. model
is on controller just for illustration purposes.
Alternatively I could render a component like {{example1-block}}
instead of {{render "example1"}}
.
I'm open to alternatives.
@AW-UC I messed up: since I'm defining a computed property on TileLayer, I have to do that at extend-time. I'll edit my comment to fix that up.
Here is an example without the service:
/* global L */
import Ember from 'ember';
import EmberLeafletComponent from 'ember-leaflet/components/leaflet-map';
import MarkerCollectionLayer from 'ember-leaflet/layers/marker-collection';
import TileLayer from 'ember-leaflet/layers/tile';
var MyMarkerCollectionLayer = MarkerCollectionLayer.extend({
content: Ember.computed('place.location', function () {
return [{ location: this.get('place.location') }];
})
});
export default EmberLeafletComponent.extend({
center: Ember.computed.reads('place.location'),
tileLayer: Ember.computed('tileUrl', function () {
return TileLayer.create({
tileUrl: this.get('tileUrl')
});
}),
markerCollectionLayer: Ember.computed('place', function () {
return MyMarkerCollectionLayer.create({
place: this.get('place')
});
}),
childLayers: Ember.computed.collect('tileLayer', 'markerCollectionLayer')
});
{{leaflet-map place=model tileUrl=tileUrl}}
export default Ember.Route.extend({
model: function() {
return this.store.find('place');
},
setupController: function (controller, model) {
this._super(controller, model);
controller.set('tileUrl', 'http://example.com/some-tile-url');
}
});
I'm not totally in love with re-creating the layer objects whenever 'tileUrl' or 'place' changes. It would be better to use the existing layer objects and then bind them to data. The alternative could be:
/* global L */
import Ember from 'ember';
import EmberLeafletComponent from 'ember-leaflet/components/leaflet-map';
import MarkerCollectionLayer from 'ember-leaflet/layers/marker-collection';
import TileLayer from 'ember-leaflet/layers/tile';
var MyTileLayer = TileLayer.extend({
tileUrl: Ember.computed.reads('component.tileUrl')
});
var MyMarkerCollectionLayer = MarkerCollectionLayer.extend({
content: Ember.computed('component.place.location', function () {
return [{ location: this.get('component.place.location') }];
})
});
export default EmberLeafletComponent.extend({
center: Ember.computed.reads('place.location'),
tileLayer: Ember.computed(function () {
return TileLayer.create({
component: this
});
}),
markerCollectionLayer: Ember.computed(function () {
return MyMarkerCollectionLayer.create({
component: this
});
}),
childLayers: Ember.computed.collect('tileLayer', 'markerCollectionLayer')
});
Now the layers are bound directly to the data and are only instantiated once. Although passing this
seems kind of ugly.
@csantero your approach of binding the layers directly to the data is interesting. I will have to try this to see how much of a difference it makes. @miguelcobain you are right: refactoring will be easy. I think your examples work fine and do a good job of illustrating the most important things :+1:
Let me ask for your opinions.
Back in the days of Views, every view had a property controller
set to the controller in which it was being rendered in.
Now, in Components, controller
is set to the component instance itself, during initialization. See here: https://github.com/emberjs/ember.js/blob/v1.11.1/packages/ember-views/lib/views/component.js#L125
I was surprised that @AW-UC could access controller
from child layers. I didn't make sense to me because Components don't have controllers per se, only Views do. However, he could do it just fine because when we create child layers, we also set its controller
property to the parent's controller
property. This is where that happens: https://github.com/gabesmed/ember-leaflet/blob/ember-cli-es6/addon%2Fmixins%2Fcontainer-layer.js#L120
That is what allows us to do things like:
export default LeafletMapComponent.extend({
childLayers: [
MarkerCollectionLayer.extend({
//child layer holds `controller` property here
content: Ember.computed.alias('controller.markers')
})
]
});
And this component was rendered from a template like:
{{leaflet-map markers=model}}
This is really cool. However, I don't think it makes sense to use the name controller
anymore. In a previous comment, @csantero used a computed property "trick" to create child layers with a component
property set to the parent component.
My opinion is that we should change the name of this property. Ember.Component docs don't even mention any controller
property, and I believe Ember devs did that just for backwards compatibility reasons. At least it wasn't clear for me which controller we were referring to.
Any suggestions for the name of this property? After all, it is a very important API that people will certainly use, and it should be documented. Since @gabesmed was the original dev of this feature, he may have some suggestions to make. :)
map
?component
?container
?context
? (Ember.Component also sets context
to its own instance, just like it does with controller
)parent
?@miguelcobain dataSource
?
I think map
or mapData
is nice. It is short and it makes clear, that it is something added by the addon.
BTW, while you can access data in the component itself through this.get('value')
or Ember.computed.alias('value')
you can alternatively also do this.get('controller.value')
and Ember.computed.alias('controller.value')
. If we rename controller
we can suggest to use it everywhere for the sake of consistency. Than we would always use e.g.:
this.get('map.value')
and
Ember.computed.alias('map.value')
@csantero dataSource
looks like a connection to a database or something related with models. This is not necessarily something related with "data". This is a reference to the entire map component.
At the risk of derailing this into a pedantic discussion, I will argue that the layer objects shouldn't care whether the source of their data is the map component or something else. Passing the component instance to the layer is hardly elegant, but it is a convenient way to provide a bound data provider to the layer so that the layer can update its content. One could easily pass in something else to the layer that drives its content
CP, and there's no reason that needs to be the map component. Abstracting that provider as "dataSource" seemed to make sense to me.
Of course if the compositional workflow you suggested in #98 (which I really love) ever becomes a reality, then this discussion becomes irrelevant.
I completely agree with you.
I'm starting to think that https://github.com/gabesmed/ember-leaflet/pull/98#issuecomment-78551161 is what's left for a complete migration to the new ember standards... Having these references and component subclassing around is a solution, but far inferior.
Using it somehow like https://github.com/gabesmed/ember-leaflet/pull/98#issuecomment-78551161 would definately be the most idiomatic way IMHO.
To open another topic: We are also in need for a strategy regarding Leaflet plugins. If that plugin adds a layer, then you can easily extend addon/layers/layer.js
and acces controller
from there. But if that plugin adds controls: how would you use it in a module? If you extend it, you can't access controller
, because we miss the mixins.
For example, if I have http://www.liedman.net/leaflet-routing-machine/ installed I can make it work like this:
/* global L */
import EmberLeafletComponent from 'ember-leaflet/components/leaflet-map';
export default EmberLeafletComponent.extend({
routingOptions: Ember.computed('origin','destination', function() {
return {
waypoints: [
this.get('origin'),
this.get('destination')
],
fitSelectedRoutes: true
};
}),
routingMachine: Ember.computed('routingOptions', function() {
return L.Routing.control(this.get('routingOptions'));
}),
didCreateLayer: function() {
this._super();
this.get('routingMachine').addTo(this._layer);
}
});
with {{show-route origin=model.origin destination=model.destination}}
while origin
and destination
are of L.latLng();
.
Now it would of course be nice, to import the routing part as a module. For this, we need to extend the API. Maybe create something like controlContainer
. Maybe it is also possible to just use containerLayer
for this. I haven't tried that, yet.
Please provide an example of how you imagine it would be a better way to include that plugin/some control. What you posted looks like a decent way to me.
@miguelcobain yes sure, it is totally sufficient. Would just be nice to import a module instead that can be reused. But I agree, that it is not the most pressing issue. I will try to present something better, once I got the time.
@miguelcobain Regarding your suggestion at https://github.com/gabesmed/ember-leaflet/pull/111#issuecomment-96763910 to use a component instead of {{render}}
: I think that might be a little better, because not using render
is more in accordance with the new Glimmer engine of Ember 1.13. However, this is certainly not that important right now.
For a third example, I would suggest something like http://leafletjs.com/examples/geojson.html. This is not too complicated and it would illustrate how to extend the component.
@miguelcobain I was wondering if the branch you created with the ember-leaflet
component (ember-cli-es6
) was in a PR somewhere? I'd love to use it but it is 134 commits behind and that does not seem very encouraging.
What would be awesome is to have it as a 1.0.0-beta or something so we would have a guarantee that there is no come back at this point.
I'm using ember-leaflet in production but I'm about to replace it with Leaflet and integrate it myself because I really want to use it in components. Unless your branch makes it to a beta release... ?
Ember 2.0 is supposed to be released in a month (June 12th?) and it'd be awesome to use ember-leaflet inside components!
Let me know if I am mistaken :)
ember-cli-es6 is, quoting github:
This branch is 59 commits ahead, 134 commits behind master
This is expected because it was a complete rewrite from scratch. There are also 59 commits ahead.
You can start using the ember-cli branch using:
$ npm install --save-dev install gabesmed/ember-leaflet.git#ember-cli-es6
$ bower install --save leaflet leaflet.markercluster
When the branch lands on master/npm it should be really easy to migrate (just edit your package.json or run normal install).
Got it, that makes sense now. Do you have an idea of when this branch will make it to master? I'm gonna try to integrate it this weekend in my app and let you know if I have feedback. Thanks!
Thank you! For me it will be merged on the example page is done.
I'm gonna try to integrate it this weekend in my app and let you know if I have feedback.
@gzurbach How did it go?
I have not been able to do it yet unfortunately. I really need to anyways because the current implementation with view is causing a lot of issues. I will get back you you guys as soon as I have something.
I'm working now on moving my map to use the ES6 branch. The main issue I'm having right now is that I can't figure out how to add a layer to a cluster. I'm using the marker-cluster layer (https://github.com/gabesmed/ember-leaflet/blob/ember-cli-es6/addon/layers/marker-cluster.js).
The code i'm trying to migrate does the following:
var markers = new L.MarkerClusterGroup();
var geoJsonLayer = new L.geoJson();
geoJsonLayer.addData(data);
markers.addLayer(geoJsonLayer);
Which I translated to something like:
import EmberLeafletComponent from 'ember-leaflet/components/leaflet-map';
import MarkerClusterLayer from 'ember-leaflet/layers/marker-cluster';
import TileLayer from 'ember-leaflet/layers/tile';
var tileLayer = [...]
var markerClusterLayer = MarkerClusterLayer.extend();
export default EmberLeafletComponent.extend({
childLayers: [
tileLayer,
markerClusterLayer
],
markersDidChange: function () {
var markers = this.get('controller.markers');
var geoJsonLayer = new L.geoJson();
geoJsonLayer.addData(markers);
markerClusterLayer.addLayer(geoJsonLayer);
}.observes('controller.markers')
});
I am getting: markerClusterLayer.addLayer is not a function
Sorry if my question is silly. I'm still new to Leaflet. Any suggestions?
Tests are often a good source of examples. Check marker cluster tests: https://github.com/gabesmed/ember-leaflet/blob/ember-cli-es6/tests/unit/layers/marker-cluster-test.js
Thanks for pointing out the tests. If I understand correctly, it seems like I cannot use MarkerClusterLayer without MarkerCollectionLayer? I'm not sure I understand where the GeoJson layer should be then? As a child of the MarkerCollectionLayer which itself is a child of MarkerClusterLayer?
I've started the project page.
But I've encountered some problems converting the old examples to the new ember-cli. I created this PR to get everyone's suggestions.
Here is how the page looks right now: preview