Skip to content
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

Leaflet 'autoPan' does not work when popup is compiled by angular #741

Closed
fbrandel opened this issue May 3, 2015 · 7 comments
Closed

Leaflet 'autoPan' does not work when popup is compiled by angular #741

fbrandel opened this issue May 3, 2015 · 7 comments

Comments

@fbrandel
Copy link
Contributor

fbrandel commented May 3, 2015

Version: 0.7.15

Normally when a popup is opened, the map is scrolled so the popup is fully visible (see autoPan)
This functionally does not work when the popup content is compiled by an ngInclude directive.

@MarkAPhillips
Copy link
Contributor

I have also noticed this issue when using the nginclude directive - autoPan was working correctly until I moved the html into a template.

@nmccready
Copy link
Contributor

Your best bet is to dig into the directive and modify and play with it.
https://github.com/tombatossals/angular-leaflet-directive/blob/master/src/services/leafletMarkersHelpers.js#L122-L155

@MarkAPhillips
Copy link
Contributor

After having some time to get back to this I have pinpointed the issue. It relates to the following

_adjustPan: function () {
        if (!this.options.autoPan) { return; }

        var map = this._map,
            containerHeight = this._container.offsetHeight,
            containerWidth = this._containerWidth,

            layerPos = new L.Point(this._containerLeft, -containerHeight - this._containerBottom);

        if (this._animated) {
            layerPos._add(L.DomUtil.getPosition(this._container));
        }

        var containerPos = map.layerPointToContainerPoint(layerPos),
            padding = L.point(this.options.autoPanPadding),
            paddingTL = L.point(this.options.autoPanPaddingTopLeft || padding),
            paddingBR = L.point(this.options.autoPanPaddingBottomRight || padding),
            size = map.getSize(),
            dx = 0,
            dy = 0;

        if (containerPos.x + containerWidth + paddingBR.x > size.x) { // right
            dx = containerPos.x + containerWidth - size.x + paddingBR.x;
        }
        if (containerPos.x - dx - paddingTL.x < 0) { // left
            dx = containerPos.x - paddingTL.x;
        }
        if (containerPos.y + containerHeight + paddingBR.y > size.y) { // bottom
            dy = containerPos.y + containerHeight - size.y + paddingBR.y;
        }
        if (containerPos.y - dy - paddingTL.y < 0) { // top
            dy = containerPos.y - paddingTL.y;
        }
        if (dx || dy) {
            map
                .fire('autopanstart')
                .panBy([dx, dy]);
        }

in leaflet and the calculation of the offsetHeight of the container, which seems to be incorrect and therefore dx and dy are still 0 so the autopan does not work. I assume this is an issue with compiling the HTML i.e. leaflet obtains the container before it has compiled to HTML. I will take a look into it further but thought I would give an update in case anyone is interested.

@MarkAPhillips
Copy link
Contributor

Ok seems that the _autoPan function is called first therefore does not have the correct settings

changing

 var updatePopup = function(popup) {
                popup._updateLayout();
                popup._updatePosition();
            };

            $compile(popup._contentNode)(markerScope);

            // In case of an ng-include, we need to update the content after template load
            if (popup._contentNode.innerHTML.indexOf("ngInclude") > -1) {
                var unregister = markerScope.$on('$includeContentLoaded', function() {
                    updatePopup(popup);
                    unregister();
                });
            }
            else {
                // We need to wait until after the next draw in order to get the correct width
                $timeout(function() {
                    updatePopup(popup);
                });
            }

to

 var updatePopup = function(popup,adjustPan) {
                popup._updateLayout();
                popup._updatePosition();
                if (adjustPan) {
                    popup._adjustPan();
                }
            };

            $compile(popup._contentNode)(markerScope);

            // In case of an ng-include, we need to update the content after template load
            if (popup._contentNode.innerHTML.indexOf("ngInclude") > -1) {
                var unregister = markerScope.$on('$includeContentLoaded', function () {
                    $timeout(function() {
                        updatePopup(popup, true);
                        unregister();
                    });
                });
            }
            else {
                // We need to wait until after the next draw in order to get the correct width
                $timeout(function() {
                    updatePopup(popup);
                });
            }
        };

seemed to fix it but would suggest someone review it. It seems if you call adjustPan explicitly it has the correct values. Please note I updated to 0.8.1 and notice you wrapped the update pop in the $timeout service - I think this is also required for the method calls in the $includeContentLoaded event.

@nmccready or @fbrandel can you review as whilst the above works I am not sure if the above has any other implications on other areas - thanks

@MarkAPhillips
Copy link
Contributor

I will create a pull request for this as its important for my application - feel free to review. The only issue I have is that the _autopan method is called twice. When you explicitly call it as above this works but sometimes its not as smooth as I would like.

@j-r-t
Copy link
Contributor

j-r-t commented Jun 3, 2015

I'd like to just add for future reference that support needs to be added for https://github.com/erictheise/rrose or get this pushed out Leaflet/Leaflet#2324 for this autoPan feature to work with maxBounds

@j-r-t
Copy link
Contributor

j-r-t commented Jun 15, 2015

#800 changed this yet again

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants