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

args.model of leafletDirectiveMarker doesn't return correct marker if one of marker is deleted

Open hkcity1111 opened this issue 9 years ago • 10 comments

Hi all

If I define 3 marker in the map and use $scope.$on(leafletDirectiveMarker.mapid.click, function(event, args)){..}) to extract args.model which contain "marker 2" if I click "marker 2" Marker.

But if I delete "marker 2" in $scope.markers and click "marker 3" in the map, arg.model in leafletDirectiveMarker.click event listener return "marker 2" data to which is deleted already in $scope.markers.

Would anyone know to fix this issue? Thanks a lot

Regards, Kenneth

$scope.markers = [{
        lat : -700, lng : -200, focus : true, title : "Marker 1",
        label : {
            message : "Mark 1",
            options : {
                noHide : true
            }}}, {
        lat : -300, lng : -300, focus : true, title : "Marker 2",
        label : {
            message : "Mark 2",
            options : {
                noHide : true,
            }}}, {
        lat : -600, lng : -100, focus : true, title : "Marker 3",
        label : {
            message : "Mark 3",
            options : {
                noHide : true,
            }}}];

hkcity1111 avatar Feb 12 '16 07:02 hkcity1111

Hi @hkcity1111, please write code into markdown tags for code. https://help.github.com/articles/creating-and-highlighting-code-blocks/

elesdoar avatar Feb 12 '16 14:02 elesdoar

Hah, I just fixed it

nmccready avatar Feb 12 '16 14:02 nmccready

Thanks nmcredy. I use '' before which make the code concatenate into one line. I know use triple ' now. Thanks all for reminder.

hkcity1111 avatar Feb 12 '16 15:02 hkcity1111

@nmccready Does that mean that ui-leaflet is now updating the markers watchers in the background? I am still struggling with this - not sure it is fixed. Random string key value assignment for the individual markers seems like the only way to ensure the args.model gets updated when swapping out the markers with a new set.

BlakeDraper avatar Mar 23 '16 16:03 BlakeDraper

also referenced in #239 -- plnkr that demonstrates this issue:

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

marsmith avatar Mar 23 '16 16:03 marsmith

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

MattSidor avatar Mar 23 '16 20:03 MattSidor

Hi,

to fix this issue, I think that you just have to modify _seeWhatWeAlreadyHave function in markers directive. This line doesn't detect changes to markers who were after deleted marker : equals = angular.equals(newMarker, oldMarker) && isEqual You can replace it with : equals = angular.equals(newMarker, oldMarker) === isEqual

It seems working well, because isEqual==false when _seeWhatWeAlreadyHave is called to test if marker's data is modified. After, it will call cb function, which is going to refresh current marker's data. if (!isDefined(markerModels) || !Object.keys(markerModels).length || !isDefined(markerModels[name]) || !Object.keys(markerModels[name]).length || equals) { if (cb && Helpers.isFunction(cb)) cb(newMarker, oldMarker, name); }

Drj5 avatar Jul 01 '16 07:07 Drj5

It looks to me as if the proposed changes from issue #240 don't work. In the same issue there is a reference to a merged solution #273. The suggest fix of @Drj5 however seems to do the trick for me. Any chance this could get fixed ? Or is this isEquals check there for a wholly different purpose ?

oenie avatar Jun 07 '18 14:06 oenie

OK, I think I have found the problem that the original poster was experiencing, and it's related to the way he's (and we too) using ui-leaflet:

You have to provide the marker array (in this case $scope.markers) as a associative array instead of an indexed array.

By just using an indexed array, the _seeWhatWeAlreadyHave function will match the wrong indexes and remove the wrong data from the markers list.

@Drj5, although your 'fix' seemed to work, it did cause other tests in the ui-leaflet test-suite to fail.

oenie avatar Jun 08 '18 13:06 oenie

Hi,

I'm working with this "fix" from 2 years now without any problem. But you're right, if test-suite is failing that's not a good solution. I prefer using indexed arrays because it's a little bit faster than associative arrays (objects in JS)...

Drj5 avatar Aug 01 '18 08:08 Drj5