ui-leaflet icon indicating copy to clipboard operation
ui-leaflet copied to clipboard

To avoid bug with markers update is it required to use '{}'

Open ghost opened this issue 8 years ago • 16 comments

First of all I saw this example here: http://angular-ui.github.io/ui-leaflet/examples/0000-viewer.html#/mixed/overlays-markers-nested-no-watch-example I am using this directive and one of my software features require that sometimes after initial load of markers I update all markers with new ones. What happens is that data between markers is somehow mixed and not same as model that I provided - updating the markers doesn't work. I resolved this issue with this:

leafletData.getDirectiveControls().then(function (controls) {
                    var markers = addressPointsToMarkers(data)
                    controls.markers.create(markers ,$scope.markers);
                    $scope.markers = markers;
                });

But issue is completely resolved when I changed my array ('[]') of objects with objects ('key: {data: data}',key: {data: data}',key: {data: data}'...). This makes another issue and that is some angular features like $filter are not possible anymore (at least not in the angular way). So, my question is: Is it necessary to have object ('{}') for markers or it is possible to use this workaround with standard array ('[]')? I need confirmation for this, please, it is very important to know this.

ghost avatar Mar 19 '16 20:03 ghost

It should support array and objects.

nmccready avatar Mar 19 '16 22:03 nmccready

Hi @nmccready thank you but can you please try to help me than am I doing something wrong here: This is directive on view:

<leaflet lf-center="vm.center" 
             event-broadcast="events" 
             markers="vm.markers | filter: vm.filterTerm" 
             defaults="defaults"
             watch-options="watchOptions"
             height="480px" 
             width="100%">
    </leaflet>

This is my init function:

angular.extend(vm, {
            center: {
                lat: ...,
                lng: ...,
                zoom: 10
            },
            watchOptions: {
                markers: {
                    type: null,
                    individual: {
                        type: null
                    }
                }
            },
            defaults: {
                scrollWheelZoom: true
            },
            position: {
                lat: 51,
                lng: 0
            },
            events: {
                markers: {
                    enable: {},
                    logic: 'emit'
                }
            }
        });

This is code that create list of markers and set it to markers scope variable:

var markers = [];
for loop....
var newMarker = { lat:...lng:...message:...}
...
markers.push(newMarker);
loop end
leafletData.getDirectiveControls().then(function (controls) {
            var _markers = markers;
            controls.markers.create(_markers, vm.markers);
            vm.markers = _markers;            
        });

The result of this code is all markers are displayed as expected. But when I change data source and call again function from above that create marker objects array marker data are not as expected. It seams that some data are from previous markers. (in popup for example) Am I doing something wrong here? I solved this issue before by using:

$scope.markers = {
"1" : {data:....},
"2" : {data:....},
"3" : {data:....}
}

But source like this is {} and what I am trying to do is []. Actually it looks like mouse events are stucked.

ghost avatar Mar 19 '16 22:03 ghost

Anybody?

ghost avatar Mar 21 '16 11:03 ghost

Can you fire something up on jsbin, plnkr, jsfiddle?

nmccready avatar Mar 21 '16 14:03 nmccready

here is an plnkr example of markers still not getting updated even when using an object instead of array to instantiate. Note that the different marker objects use the same key and this is what breaks the events. If the keys are changed to unique, it works. So it seems the only fix (especially if looping over a large array of sites) is to assign randomly generated key values to the objects as they are added to the markers.

Also referenced in #221

http://plnkr.co/edit/PSDeVjHiZLrwPHRf54Yl?p=preview

marsmith avatar Mar 23 '16 15:03 marsmith

@nmccready according to that plnkr seems like ui-leaflet still not updating the markers watchers in the background. Seems like the only workaround right now is to assign each marker a random string key value so that they get updated when the markers are swapped out.

Like @seadour hints at in #199 and #95

BlakeDraper avatar Mar 23 '16 16:03 BlakeDraper

@nmccready we hijacked a couple related issues, let me know if this qualifies as a new issue: Switching to a markers object ('{}') does NOT fix this issue if there are duplicated keys.

marsmith avatar Mar 23 '16 17:03 marsmith

The cleanest workaround is to not rely on watchers at all and use directiveControls.

https://github.com/angular-ui/ui-leaflet/blob/master/examples/0512-markers-clustering-10000-markers-example-no-watch.html#L72-L75

https://github.com/angular-ui/ui-leaflet/blob/master/examples/0605-mixed-no-watch-example.html#L21-L29

I currently am involved in backend issues for work. I will not have time to dive into this until this weekend or later.

nmccready avatar Mar 23 '16 17:03 nmccready

@nmccready I implemented the directiveControls even though full disclosure I'm not sure what they are doing and can't find any docs, and still no joy:

http://plnkr.co/edit/5DQpnGnBKsdLrMZ9cbtt

marsmith avatar Mar 23 '16 18:03 marsmith

FYI, I did figure out how to make this work. First @nmccready the two examples you posted using directiveControls have two very different watch-options properties and directive names so one of them is wrong I'm assuming. This is what worked for me:

markersWatchOptions: {
     doWatch: false,
     isDeep: false,
     individual: {
          doWatch: false,
          isDeep: false
          }
}

in the directive:

<leaflet lf-center="london" markers="markers" event-broadcast="events" markers-watch-options="markersWatchOptions" height="480px" width="100%"></leaflet>

Second, you need to force an empty object into controls.markers.create before you can send it new data:

leafletData.getDirectiveControls().then(function (controls) {
     controls.markers.create({} ,$scope.markers);
     var markers = $scope.redMarkers;
     controls.markers.create(markers ,$scope.markers);
     $scope.markers = markers;
});

http://plnkr.co/edit/NjEqU0KogXrkU8bjOo5K?p=preview

Hopefully this helps some people -- it is pretty clunky workaround. Alternatively as @BlakeDraper says you can use some random key generator as you append markers to $scope.markers but this is not efficient and would eventually cause a memory leak since somewhere behind the scenes the watched markers are not getting cleared out.

marsmith avatar Mar 23 '16 19:03 marsmith

Is the proposed change from @vincent-ollivier-everysens in issue #240 relevant to this?

MattSidor avatar Mar 23 '16 20:03 MattSidor

I need to dig deeper into it , but yes it is probably relevant.

nmccready avatar Mar 23 '16 20:03 nmccready

Hi @marsmith I used the same workaround like you:

leafletData.getDirectiveControls().then(function (controls) {
     controls.markers.create({} ,$scope.markers);
     var markers = $scope.redMarkers;
     controls.markers.create(markers ,$scope.markers);
     $scope.markers = markers;
});

This same workaround doesn't work when I use markers in the following way:

var markers = [];
markers.push({prop1:...prop2:...});

But defined like this:

var markers = {};
markers["SOME-UNIQUE-STRING"] = {prop1....prop2...};

It work. The issue why I asked for this is that I can't use angular filters on '{}'.

ghost avatar Mar 23 '16 20:03 ghost

@zinotinder Can you just write your own filter that will handle a nested object?

http://jsfiddle.net/NqA8d/5/

marsmith avatar Mar 23 '16 20:03 marsmith

@seadour , @nmccready For what its worth I tried the fix from @vincent-ollivier-everysens no luck there. The "_deleteMarker" function isn't called when I update $scope.markers

marsmith avatar Mar 23 '16 20:03 marsmith

@marsmith yes something like this. https://plnkr.co/edit/Gi4gWHne57owB44MTsAB?p=preview And it's ok to use it that way. Can't explain really why I want [] instead of {} I just noticed it doesn't work too late, which leads me to some code rewrite later.

ghost avatar Mar 23 '16 20:03 ghost