JasonSanford / leaflet-vector-layers

A little help to viewing ArcGIS Server, Geocommons, Arc2Earth, CartoDB, GIS Cloud, etc. vector data in a Leaflet map
http://jasonsanford.github.io/leaflet-vector-layers
BSD 2-Clause "Simplified" License
216 stars 61 forks source link

Added ILayer events to Layer.js #31

Open chriserickson opened 11 years ago

chriserickson commented 11 years ago

When I tried to use vector layers with the Layers Control or in a group layer, I got errors because the vector layers layer doesn't implement the ILayer interfaces onAdd/onRemove functions.

I was able to get it working with existing source by doing the following:

        lvector.AGS.prototype.onAdd = function() {
            this.setMap(window.map)
        }
        lvector.AGS.prototype.onRemove = function() {
            this.setMap(null)
        }

But thought it would be good to submit a pull request.

I've not built/tested this beyond my original code that added it to the prototype manually.

JasonSanford commented 11 years ago

Nice, Thanks!

I've got a couple of other pull requests I need to merge in as well. I'll try to work this in a bump a version.

chriserickson commented 11 years ago

Cool, let me know if I screwed something up. New to git/github as well as leaflet...

Thanks!


chris erickson 801.613.0450 "No man knows how bad he is till he has tried very hard to be good. "

On Fri, Jan 18, 2013 at 11:46 AM, Jason Sanford notifications@github.comwrote:

Nice, Thanks!

I've got a couple of other pull requests I need to merge in as well. I'll try to work this in a bump a version.

— Reply to this email directly or view it on GitHubhttps://github.com/JasonSanford/leaflet-vector-layers/pull/31#issuecomment-12435561.

JasonSanford commented 11 years ago

Hey Chris, any chance you can work in an example of using the iLayer interface in the debug directory. I can't seem to figure it out.

bmcbride commented 11 years ago

Hey Chris/Jason,

This would be a great addition to LVL. I played around a bit with your code snippet and it doesn't seem to work with layers that are initially added to the map. The layer is added to the control, but the check box is not checked.

BRYAN

chriserickson commented 11 years ago

Jason, I can try to take a look this week if I have some time. Sorry for the delay, I've been tied up.


chris erickson 801.613.0450 "No man knows how bad he is till he has tried very hard to be good. "

On Wed, Feb 27, 2013 at 10:50 PM, Jason Sanford notifications@github.comwrote:

Hey Chris, any chance you can work in an example of using the iLayer interface in the debug directory. I can't seem to figure it out.

— Reply to this email directly or view it on GitHubhttps://github.com/JasonSanford/leaflet-vector-layers/pull/31#issuecomment-14218100 .

chriserickson commented 11 years ago

I should be able to look at this sometime this week. I'll let you know


chris erickson 801.613.0450 "No man knows how bad he is till he has tried very hard to be good. "

On Tue, Mar 5, 2013 at 9:25 AM, Bryan McBride notifications@github.comwrote:

Hey Chris/Jason,

This would be a great addition to LVL. I played around a bit with your code snippet and it doesn't seem to work with layers that are initially added to the map. The layer is added to the control, but the check box is not checked.

BRYAN

— Reply to this email directly or view it on GitHubhttps://github.com/JasonSanford/leaflet-vector-layers/pull/31#issuecomment-14449076 .

draehal commented 10 years ago

I foolishly posted this same issue before seeing this one.

There is an added bug.

As a side effect, implementing ILayer introduces a bug between the layer control and the lvector.Layer. The layer control calls onRemove to remove the display of the llvector.Layer on the map. This causes lvector.Layer to set the map to null using the "this.setMap(null)" call. When the map is zoomed, the function "_checkLayerVisibility" is called. This pokes options.map for its zoom level. At this point, options.map is null and throws an exception. Fortunately, the code is written in such a way that this exception is equivalent to a null predicate of true and exits the function without disrupting anything else. It does however log an annoying exception in the debug console.

    //Layer.js line 222

    //
    // Get the current map zoom and check to see if the layer should still be visible
    //
    _checkLayerVisibility: function() {
        if (  !this.options.map )  { 
            return; // The layer still exists, but is not present on the map, so do nothing.
        }
        //
        // Store current visibility so we can see if it changed
        //
        var visibilityBefore = this.options.visibleAtScale;

        // ...
     }
draehal commented 10 years ago

Here is a gist with the arcgis demo code updated to use ILayer https://gist.github.com/draehal/6706599 The JS includes 2 bug fixes that needed to be done for the demo to work as additions to the prototype of lvector.AGS. Also, I had to update the version of Leaflet to 0.6.4 for event handling to work.