tombatossals / angular-leaflet-directive

AngularJS directive to embed an interact with maps managed by Leaflet library
http://tombatossals.github.io/angular-leaflet-directive
MIT License
1.5k stars 637 forks source link

Populate a map with a list of markers coming from the scope / backend #548

Closed gkatsanos closed 9 years ago

gkatsanos commented 9 years ago

I went through the examples and some past issues but was unable to find something similar to what I'm trying to do;

I have two controllers, a "Messages" controller and the "MapController" which is nested inside the messages. Messages generates a list of .. messages - which have a latitude and longitude coordinate.

I want my Map to present each message as a marker.

My messages are rendered with the following :

        // Find a list of Messages
        $scope.find = function() {
            $scope.messages = Messages.query();
        };

and the view:

        <a data-ng-repeat="message in messages" data-ng-href="#!/messages/{{message._id}}" class="list-group-item">
         <small class="list-group-item-text">
            Posted on
            <span data-ng-bind="message.created | date:'medium'"></span>
            by
            <span data-ng-bind="message.user.displayName"></span>
        </small>
        <div data-ng-bind="message.body"></div>
        <div data-ng-bind="message.location.lat"></div>
        <div data-ng-bind="message.location.lng"></div>
    </a>
porjo commented 9 years ago

Setup your leaflet tag as per the examples, and make sure to include the markers attribute. Then, inside your controller it should be as simple as iterating through your messages, something like this:

for (var i = 0; i < $scope.messages.length; i++) {
    $scope.markers['m' + i] = {
        lat: messages[i].location.lat,
        lng: messages[i].location.lng,
        message: messages[i].body
    };
}

$scope.markers['m' +i] is just a way to give each marker object a unique keyname.

cyrilchapon commented 9 years ago

That strategy works, but would just populate once the map, losing an entire point of angular which is two way data binding

How one could actually bind its model to the map, syncing markers collection with a model collection without reinventing the wheel ? Mapbox-angular does provide this through the view, with <marker ng-repeat>

mrpharderwijk commented 9 years ago

Does anyone have an update on this? Is it possible to update the map with new markers on the scope.markers?

cyrilchapon commented 9 years ago

Does anyone have an update on this?

I ended up keeping my real complex collection (eg. $scope.beans) in sync with $scope.markers. For this you have two choices :

Is it possible to update the map with new markers on the scope.markers?

Definately yes. You can of course simply add and remove markers, just pushing and slicing on $scope.markers. It's even possible to update the location of a marker, just making it move without erasing it (event keeping open and moving it's opened popup). For this, you need to update the lat and lng values after retreiving your marker. No "flush" or other action is required, the map is strongly bound to the collection you specify on markers attribute

nmccready commented 9 years ago

The watches are there I am not sure as to what your problem is. Please make a plnkr or jsfiddle.

cyrilchapon commented 9 years ago

Basically something like this

map.html

<leaflet>
  <marker ng-repeat="bean in beans" lat="bean.lat" lng="bean.lng"></marker>
</leaflet>

Would allow to keep just one, semantic, plain and maintanable angular collection without having to reinvent the wheel.

See here

nmccready commented 9 years ago

Honestly you don't want that ng-repeat is horrendous at speed. I know this from experience for angular-google-maps. The way this directive is designed is ideal.

nmccready commented 9 years ago

Different strokes for different folks. This directive is more of a config based directive and uses basically only one scope level directive leaflet. Changing the layers, markers, geojson to scope level directives would be a large change which I see as unnecessary. The main thing it gives you is flexibility to where your data and how your data is stored.

In the end though all you would here from me is to not use ng-repeat and then we have two duplicated directives for similar purposes. This causes a much more complex API .. see angular google maps. The way angular-leaflet is now is much easier to maintain.

nmccready commented 9 years ago

@tombatossals what do you think on this issue?

cyrilchapon commented 9 years ago

Hum ok. Performance purpose then. Understandable.

The "right-way" to bind my backend to those markers is, then, to $watch my true collection update, or even better intercept my backend event, and append them manually to the markers collection ?

I might be misunderstanding the markers attribute feature though, but I'm sorry I feel very weird $watching a two-ways-data-binded variable in order to, well, manualy one-way-data-bind it to another variable already two-way-data-binded to the view Oo

nmccready commented 9 years ago

How is your data formatted when it comes to the front end? Are you manually formatting the data to get it to match the angular-leaflet requirements? If you are; I suggest doing that on the backend via the service layer and or database (stored procedure).

cyrilchapon commented 9 years ago

My backend is returning geojsonish data embedded in objects, such as :

[
  {
    "id": 12,
    "name": "WalterWhite",
    "geoFeature": {
      "type": "Point",
      "coordinates": [2.2154,51.6545]
    }
  }
]

In this project, I'm indeed able to adapt the backend. But changing a backend layer to make it plugin-compliant is kinda brutal, isn't it ?

tombatossals commented 9 years ago

Hi @cyrilchapon, @nmccready, I understand perfectly this conversation. The two approaches have advantages and disadvantages. I coded the openlayers directive following the more semantic way of declaring the map components, and, as you say, the code is more clean but the performance is much slower.

Take a look at this example:

    <openlayers lat="39.92" lon="116.38" zoom="10" height="400" custom-layers="true">
        <ol-layer source-type="TileJSON" source-url="https://api.tiles.mapbox.com/v3/examples.map-i86nkdio.jsonp"></ol-layer>
        <ol-marker lat="39.92" lon="116.38" message="Here is Beijing. Dreamful place."></ol-marker>
    </openlayers>

http://tombatossals.github.io/angular-openlayers-directive/#/markers/marker-no-javascript

Cleaner to write and understand, but with a ng-repeat code, more complex, the latency time to show markers on the map was really high.

That said, I'm open to any suggestions that you think could improve the directive. :smile:

cyrilchapon commented 9 years ago

Hey @tombatossals. Therefore performance purpose, as well stated by @nmccready.

I've no problem (and I guess, neither the community) to accept a simple restriction, as well as I understand it, which I did, thanks to both of you.

For the community, basically, to the question :

The "right-way" to bind my backend to those markers is, then, to $watch my true collection update, or even better intercept my backend event, and append them manually to the markers collection ?

  • The answer is yes, performance matters disallow us to take full advantage of ng-repeat without any drastic performance issue, forcing us to manually handle the marker collection.
  • Another approach would be to insert a proxy (on the client, or on the server) between the directive and the backend, that would adapt the data to a leaflet-angular-directive compliant format.

Thanks again to both of you, and apologies for this poor English =)

j-r-t commented 9 years ago

I have a function which converts the endpoint data into the markers, which I'm sure most people are doing. In any sense, unless your data is returned from the endpoint in the correct format (which IMO shouldn't be - it should be in a generic format and be able to be consumed by a variety of applications), your always going to have to adapt it, ng-repeat or not. However I do like the ng-repeat idea - shame it's slow.

mrpharderwijk commented 9 years ago

My problem is fairly simple. I have a template with a leaflet directive on it. The markers variable gets filled by a $resource call (query) and fills the markers variable. This works. But for testing purposes, when I set a timeout and try to extend or push values to the markers value it doesn't add the markers to the leaflet. I've tried $scope.$apply at the end of the function. Nothing previously stated works...

I could make a watch for a $scope.foo that fills the $scope.markers from the backend, but I don't think thats what we want to achieve here. When the leaflet directive is compiled it has the initial backend call and all is right, but if I add the same $timeout and try to attach extra values to the array/object nothing gets updates/synced on the map (two way data binding, where are you?!)... I guess this has something to do with the fact that the core doesn't have any watchers for the markers??? Cause if I run the digest cycle through $scope.$apply nothing happens. Please advice!

cyrilchapon commented 9 years ago

push values to the markers value it doesn't add the markers

It should (it does).

the core doesn't have any watchers for the markers???

It has.

Could you provide some code then ?

mrpharderwijk commented 9 years ago

Here you go! I've created a $timeout that adds a new marker to the markers object after 5 seconds. http://plnkr.co/edit/QvXZE8j1LfxIL8Ekf8MK?p=preview

cyrilchapon commented 9 years ago

turn watch-markers="yes" to watch-markers="true"

Plunker

Edit: Or just remove it, true is the default

mrpharderwijk commented 9 years ago

Wow, how didn't I see that one!!! And why is turning it off stated as 'NO' when this is a boolean value for active state?

j-r-t commented 9 years ago

Something to do with this perhaps?

// backwards compat
if(isDefined(attrs.watchMarkers))
    watchOptions.doWatch = watchOptions.individual.doWatch =
    (!isDefined(attrs.watchMarkers) || Helpers.isTruthy(attrs.watchMarkers));
//mainly for checking attributes of directives lets keep this minimal (on what we accept)
isTruthy: function(val){
    return val === 'true' || val === true;
},
cyrilchapon commented 9 years ago

Maybe an isFalsy implementation would be more appropriate when default is true ?

Also !isDefined(attrs.watchMarkers) test inside isDefined(attrs.watchMarkers) is a nonsense, isn't it ?

nmccready commented 9 years ago

Yeah it was a copy paste mistake, leaving it as isTruthy is fine with me I don't want to complicate it. It is easy enough to set it to false as you said. Anything but true.