-
Notifications
You must be signed in to change notification settings - Fork 35
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
project page + examples #111
Conversation
@AW-UC and people who are using this branch of ember-leaflet: What is your general strategy for passing data to the |
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 |
Here is a more detailed description on how to deal with it:
I want to render the map on
Now in the template for this route I call the component:
If you don't want to do much with
Then the A correspondent component would look like this:
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:
Then I can call in the component:
Generally, note that it is 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:
|
How can you do
|
@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 |
In other words: |
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:
You have to use it in a function. And then I could only make it work with |
How about something like this?
|
Updated my comment to fix the computed property in |
@csantero that is also nice. I will try your way and see how I like it. |
Very smart things there. 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
No need to change anything in the component or use conditionals in it in order to render two maps with different tile sources.
This way you get leaner and more reusable components. |
Exactly.
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. |
@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:
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:
Now the layers are bound directly to the data and are only instantiated once. Although passing |
@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. |
Let me ask for your opinions. Back in the days of Views, every view had a property I was surprised that @AW-UC could access 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 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. :)
|
@miguelcobain |
I think
and
|
@csantero |
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 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 #98 (comment) is what's left for a complete migration to the new ember standards... |
Using it somehow like #98 (comment) would definately be the most idiomatic way IMHO.
with |
Please provide an example of how you imagine it would be a better way to include that plugin/some control. |
@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 #111 (comment) to use a component instead of |
@miguelcobain I was wondering if the branch you created with the |
ember-cli-es6 is, quoting github:
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! |
@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. 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: Sorry if my question is silly. I'm still new to Leaflet. Any suggestions? |
Tests are often a good source of examples. |
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