From f17a3c70e458b988271edda54b5fdfa4bd14a0a9 Mon Sep 17 00:00:00 2001 From: Matt Priour Date: Tue, 9 Oct 2012 11:10:55 -0500 Subject: [PATCH 1/2] Fire the 'changelayer:visibility' event from layer's display method Move the changelayer event firing logic for in / out of resolution range from the Map class to the Layer class. Tests have been also been created to specifically test that the display method works correctly and fires events only when needed. --- lib/OpenLayers/Layer.js | 5 +++++ lib/OpenLayers/Map.js | 4 +--- tests/Layer.html | 35 +++++++++++++++++++++++++++++++++++ tests/Map.html | 20 +++++++++++++++++++- 4 files changed, 60 insertions(+), 4 deletions(-) diff --git a/lib/OpenLayers/Layer.js b/lib/OpenLayers/Layer.js index e381a353db..5b234dfa5b 100644 --- a/lib/OpenLayers/Layer.js +++ b/lib/OpenLayers/Layer.js @@ -758,6 +758,11 @@ OpenLayers.Layer = OpenLayers.Class({ display: function(display) { if (display != (this.div.style.display != "none")) { this.div.style.display = (display && this.calculateInRange()) ? "block" : "none"; + if(this.map){ + this.map.events.triggerEvent("changelayer", { + layer: this, property: "visibility" + }); + } } }, diff --git a/lib/OpenLayers/Map.js b/lib/OpenLayers/Map.js index 53de7c390d..5f4cf565a7 100644 --- a/lib/OpenLayers/Map.js +++ b/lib/OpenLayers/Map.js @@ -1976,9 +1976,7 @@ OpenLayers.Map = OpenLayers.Class({ if (!inRange) { layer.display(false); } - this.events.triggerEvent("changelayer", { - layer: layer, property: "visibility" - }); + } if (inRange && layer.visibility) { layer.moveTo(bounds, zoomChanged, options.dragging); diff --git a/tests/Layer.html b/tests/Layer.html index abe4e1a2c2..f658e478fb 100644 --- a/tests/Layer.html +++ b/tests/Layer.html @@ -858,6 +858,41 @@ "setOpacity() does not trigger changelayer if the opacity value is the same"); } + function test_display(t) { + t.plan(8); + + var map, layer, log; + + map = new OpenLayers.Map("map"); + layer = new OpenLayers.Layer("", { + alwaysInRange: true, + visibility: true + }); + map.addLayer(layer); + + log = []; + map.events.register('changelayer', t, function(event) { + log.push({ + layer: event.layer, + property: event.property + }); + }); + layer.display(false); + t.eq(layer.div.style.display, "none", "display() set layer's display style to correct value"); + t.eq(layer.getVisibility(), true, "display() does not affect layer's visibility state"); + t.eq(log.length, 1, "display() triggers changelayer once"); + t.ok(log[0].layer == layer, "changelayer listener called with expected layer"); + t.eq(log[0].property, "visibility", "changelayer listener called with expected property"); + layer.visibility = false; + layer.display(true); + t.eq(layer.div.style.display, "block", "display() set layer's display style to correct value"); + t.eq(layer.getVisibility(), false, "display() does not affect layer's visibility state"); + + // This call must not trig the event because the opacity value is the same. + log = []; + layer.display(true); + t.eq(log.length, 0, "display() does not trigger changelayer if the display value is the same"); + } /****** * diff --git a/tests/Map.html b/tests/Map.html index 50b7cdf770..ffa5234b61 100644 --- a/tests/Map.html +++ b/tests/Map.html @@ -977,13 +977,21 @@ } function test_Map_moveTo(t) { - t.plan(2); + t.plan(8); map = new OpenLayers.Map('map'); var baseLayer = new OpenLayers.Layer.WMS("Test Layer", "http://octo.metacarta.com/cgi-bin/mapserv?", {map: "/mapdata/vmap_wms.map", layers: "basic"}, {maxResolution: 'auto', maxExtent: new OpenLayers.Bounds(-10,-10,10,10)}); + var testLayer = new OpenLayers.Layer("",{maxResolution: 0.1, minResolution: 0.03, isBaseLayer: false, visibility: true}); + var log = []; + map.events.register('changelayer', t, function(event) { + log.push({ + layer: event.layer, + property: event.property + }); + }); baseLayer.events.on({ move: function() { t.ok(true, "move listener called"); @@ -993,10 +1001,20 @@ } }); map.addLayer(baseLayer); + map.addLayer(testLayer); + log = []; var ll = new OpenLayers.LonLat(-100,-150); map.moveTo(ll, 2); + t.ok(map.getCenter().equals(new OpenLayers.LonLat(0,0)), "safely sets out-of-bounds lonlat"); + t.eq(testLayer.div.style.display, "none", "moveTo out of resolution range set layer's display style to correct value"); + t.eq(log.length, 1, "Map.moveTo out of resolution range triggers changelayer once"); + t.ok(log[0].layer == testLayer, "changelayer listener called with expected layer"); + t.eq(log[0].property, "visibility", "changelayer listener called with expected property"); + map.moveTo(new OpenLayers.LonLat(0,0), 0); + t.eq(testLayer.div.style.display, "block", "moveTo in to resolution range set layer's display style to correct value"); + map.destroy(); } From e8cc0deeb5ebfd8dc4524b717d532e062024a44d Mon Sep 17 00:00:00 2001 From: Matt Priour Date: Tue, 9 Oct 2012 12:21:25 -0500 Subject: [PATCH 2/2] Remove extra event firing logic from Layer::setVisibility --- lib/OpenLayers/Layer.js | 6 ------ tests/Layer.html | 5 ++++- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/lib/OpenLayers/Layer.js b/lib/OpenLayers/Layer.js index 5b234dfa5b..f8ddfbe9fd 100644 --- a/lib/OpenLayers/Layer.js +++ b/lib/OpenLayers/Layer.js @@ -736,12 +736,6 @@ OpenLayers.Layer = OpenLayers.Class({ this.visibility = visibility; this.display(visibility); this.redraw(); - if (this.map != null) { - this.map.events.triggerEvent("changelayer", { - layer: this, - property: "visibility" - }); - } this.events.triggerEvent("visibilitychanged"); } }, diff --git a/tests/Layer.html b/tests/Layer.html index f658e478fb..22935f7e3a 100644 --- a/tests/Layer.html +++ b/tests/Layer.html @@ -859,7 +859,7 @@ } function test_display(t) { - t.plan(8); + t.plan(9); var map, layer, log; @@ -887,11 +887,14 @@ layer.display(true); t.eq(layer.div.style.display, "block", "display() set layer's display style to correct value"); t.eq(layer.getVisibility(), false, "display() does not affect layer's visibility state"); + layer.setVisibility(true); // This call must not trig the event because the opacity value is the same. log = []; layer.display(true); t.eq(log.length, 0, "display() does not trigger changelayer if the display value is the same"); + layer.setVisibility(false); + t.eq(log.length, 1, "changelayer event called only once. setVisibility doesn't fire any extra changelayer events"); } /******