angular-ui / ui-leaflet

AngularJS directive to embed an interact with maps managed by Leaflet library
http://angular-ui.github.io/ui-leaflet
Other
315 stars 137 forks source link

Angular does not detect DOM change after popup is displayed on multiPolyline (created via paths directive) #166

Open nmccready opened 8 years ago

nmccready commented 8 years ago

From @vtytar on May 14, 2015 15:14

You can create a path with message field in its params, and as a result you will get it drawn with capability to click on it and raise a popup. If message field contains angular directives, they won't be recognized (parsed) as angular does not detect the DOM change after popup is displayed.

For instance,

HTML:

<leaflet [...] paths="map.paths"></leaflet>

Controller:

$scope.map.paths = [
    {
        type: 'multiPolyline',
        message: '<div ng-include="\'views/route.html\'"></div>',
        latlngs: [ 
            { lat: 49.784499, lng: 24.060047 },  
            { lat: 49.803566, lng: 23.996404 }
        ]
    }
];

Once you press on this polyline, empty popup is displayed without any attempt to download views/route.html. It happens because leaflet-directive paths link function binds the popup, but does not attach popupopen/popupclose events like marker directive does.

Here is how this is done for markers directive (which handles this situation very well):

listenMarkerEvents: function(marker, markerData, leafletScope) {
    marker.on("popupopen", function(/* event */) {
        safeApply(leafletScope, function() {
            markerData.focus = true;
        });
    });
    marker.on("popupclose", function(/* event */) {
        safeApply(leafletScope, function() {
            markerData.focus = false;
        });
    });
}

Probably something similar has to be done for paths as well. I would appreciate if you suggested fix for the issue described above.

Copied from original issue: tombatossals/angular-leaflet-directive#756

nmccready commented 8 years ago

My suggestion is to dig into the code base and try to copy the implementation from leafletMarkersHelpers to where it needs to be on path. I don't have time to implement this right now. Open source depends on more contributors.

nmccready commented 8 years ago

From @vtytar on May 16, 2015 1:34

Apparently, adding listenPathEvents is not enough (of course it is not), it also requires to copy managePopupOpen from the marker. I have also noticed that path vs. marker API is a bit different inside of leaflet library in what seems should be the same: for instance, basic methods like path.getPopup() or path.closePopup() are missing etc.

Anyway, I have managed to resolve described problem myself, with, of course, two small buts: 1) My project is based on angular-leaflet-directive-v0.7.10 (higher releases caused some regressions in critical path as compared to 0.7.10, so I decided to stick to this version until I am able to fix them and upgrade to ToB). 2) Obviously, I have added only the stuff required by my project, which is not a complete solution (5% of what marker does), but in general it seems no harm to overall functionality: it only adds, should not break.

@nmccready, let me know if you are interested in taking the fix, and I will find some time to upgrade my project to latest version and create a corresponding pull request. .

nmccready commented 8 years ago

From @vtytar on May 16, 2015 1:36

Patch based on angular-leaflet-directive-v0.7.10:

--- build/bower_components/angular-leaflet-directive/dist/angular-leaflet-directive.js  2014-12-12 20:22:39.000000000 +0200
+++ build/bower_components/angular-leaflet-directive/dist/angular-leaflet-directive.patched.js  2015-05-16 03:52:50.300140837 +0300
@@ -978,7 +978,10 @@
                 paths     = leafletScope.paths,
                 createPath = leafletPathsHelpers.createPath,
                 bindPathEvents = leafletEvents.bindPathEvents,
-                setPathOptions = leafletPathsHelpers.setPathOptions;
+                setPathOptions = leafletPathsHelpers.setPathOptions,
+                listenPathEvents = leafletPathsHelpers.listenPathEvents,
+                manageOpenPopup = leafletPathsHelpers.manageOpenPopup;
+

             mapController.getMap().then(function(map) {
                 var defaults = leafletMapDefaults.getDefaults(attrs.id),
@@ -1006,12 +1009,19 @@

                     // Function for listening every single path once created
                     var watchPathFn = function(leafletPath, name) {
-                        var clearWatch = leafletScope.$watch('paths.' + name, function(pathData) {
+                        var clearWatch = leafletScope.$watch('paths.' + name, function(pathData, oldPathData) {
                             if (!isDefined(pathData)) {
                                 map.removeLayer(leafletPath);
                                 clearWatch();
                                 return;
                             }
+
+                            // The markerData.focus property must be true so we update if there wasn't a previous value or it wasn't true
+                            if (pathData.focus === true && oldPathData.focus === false) {
+                                // Reopen the popup when focus is still true
+                                manageOpenPopup(leafletPath, pathData);
+                            }
+
                             setPathOptions(leafletPath, pathData.type, pathData);
                         }, true);
                     };
@@ -1078,6 +1088,7 @@
                                 }

                                 bindPathEvents(newPath, newName, pathData, leafletScope);
+                                listenPathEvents(newPath, pathData, leafletScope);
                             }
                         }

@@ -2744,11 +2755,12 @@
    };
 });

-angular.module("leaflet-directive").factory('leafletPathsHelpers', ["$rootScope", "$log", "leafletHelpers", function ($rootScope, $log, leafletHelpers) {
+angular.module("leaflet-directive").factory('leafletPathsHelpers', ["$rootScope", "$log", "$compile", "leafletHelpers", function ($rootScope, $log, $compile, leafletHelpers) {
     var isDefined = leafletHelpers.isDefined,
         isArray = leafletHelpers.isArray,
         isNumber = leafletHelpers.isNumber,
-        isValidPoint = leafletHelpers.isValidPoint;
+        isValidPoint = leafletHelpers.isValidPoint,
+        safeApply     = leafletHelpers.safeApply;
     var availableOptions = [
         // Path options
         'stroke', 'weight', 'color', 'opacity',
@@ -2992,6 +3004,41 @@
             }

             return pathTypes[path.type].createPath(options);
+        },
+        manageOpenPopup: function(path, pathData) {
+            //the path may have angular templates to compile
+            var popup = path._map._popup;
+            if (isDefined(popup)) {
+                var updatePopup = function(popup) {
+                    popup._updateLayout();
+                    popup._updatePosition();
+                };
+
+                $compile(popup._contentNode)($rootScope);
+                //in case of an ng-include, we need to update the content after template load
+                if (isDefined(popup._contentNode) && popup._contentNode.innerHTML.indexOf("ngInclude") > -1) {
+                    $rootScope.$on('$includeContentLoaded', function(event, src) {
+                        if (popup.getContent().indexOf(src) > -1) {
+                            updatePopup(popup);
+                        }
+                    });
+                }
+                else {
+                    updatePopup(popup);
+                }
+            }
+        },
+        listenPathEvents: function (path, pathData, leafletScope) {
+            path.on("popupopen", function(/* event */) {
+                safeApply(leafletScope, function() {
+                    pathData.focus = true;
+                });
+            });
+            path.on("popupclose", function(/* event */) {
+                safeApply(leafletScope, function() {
+                    pathData.focus = false;
+                });
+            });
         }
     };
 }]);