From cb1c3a834aedea34e98fe20df112304316287004 Mon Sep 17 00:00:00 2001 From: Slawomir Messner Date: Tue, 18 Sep 2012 08:14:54 +0200 Subject: [PATCH 1/7] [New] raise feature function to move features in layer. --- lib/OpenLayers/Layer/Vector.js | 23 +++++++++++++- tests/Layer/Vector.html | 56 +++++++++++++++++++++++++++++++++- 2 files changed, 77 insertions(+), 2 deletions(-) diff --git a/lib/OpenLayers/Layer/Vector.js b/lib/OpenLayers/Layer/Vector.js index 4300e59b16..ddd2f6f4a0 100644 --- a/lib/OpenLayers/Layer/Vector.js +++ b/lib/OpenLayers/Layer/Vector.js @@ -699,7 +699,28 @@ OpenLayers.Layer.Vector = OpenLayers.Class(OpenLayers.Layer, { this.events.triggerEvent("featuresremoved", {features: features}); } }, - + + /** + * APIMethod: raiseFeature + * Changes the index of the given feature by delta and redraws the layer. If delta is positive, the feature is moved up the layer's feature stack; if delta is negative, the feature is moved down. + * If delta + current is negative it is set to 0 and feature will be at the bottom; if it is bigger then the features count than it is set to the last index and the feature will be at the top. + */ + raiseFeature:function(feature, delta) { + var base = this.features.indexOf(feature); + var idx = base + delta; + if (idx < 0) { + idx = 0; + } else if (idx > this.features.length) { + idx = this.features.length; + } + if (base != idx) { + this.features.splice(base, 1); + this.features.splice(idx, 0, feature); + this.eraseFeatures(this.features); + this.redraw(); + } + }, + /** * APIMethod: removeAllFeatures * Remove all features from the layer. diff --git a/tests/Layer/Vector.html b/tests/Layer/Vector.html index aa3e2f8a7e..2cfc9c07bd 100644 --- a/tests/Layer/Vector.html +++ b/tests/Layer/Vector.html @@ -461,7 +461,44 @@ layer.features = []; } - + function test_drawfeature_ng(t) { + t.plan(3); + var layer = new OpenLayers.Layer.Vector(); + layer.drawn = true; + var log = []; + + // Bogus layer renderer needs some methods + // for functional tests. + var renderer = { + initialize: function() {}, + drawFeature: function(feature, style) { + log.push(style); + }, + root: document.createElement("div"), + destroy: function() { }, + eraseFeatures: function() {}, + setExtent: function() {}, + setSize: function() {} + }; + var OrigRendererNG = OpenLayers.Renderer.NG; + OpenLayers.Renderer.NG = OpenLayers.Class(renderer); + + layer.renderer = new OpenLayers.Renderer.NG(); + layer.drawFeature(new OpenLayers.Feature.Vector()); + t.eq(log[0]._createSymbolizer, undefined, "no _createSymbolizer function for static styles"); + + var styleMap = new OpenLayers.StyleMap(new OpenLayers.Style({foo: "${bar}"}, + {context: {"bar": function(feature){ return "baz" }}} + )); + layer.styleMap = styleMap; + layer.drawFeature(new OpenLayers.Feature.Vector()); + t.eq(log[1]._createSymbolizer().foo, "baz", "_createSymbolizer function added to style and returns correct result"); + OpenLayers.Renderer.NG = OrigRendererNG; + + layer.renderer = renderer; + layer.drawFeature(new OpenLayers.Feature.Vector()); + t.eq(log[2]._createSymbolizer, undefined, "no _createSymbolizer function for non-NG renderer"); + } function test_deleted_state(t) { t.plan(9); @@ -869,6 +906,23 @@ "featuresadded event received expected number of features"); } + function test_raiseFeature(t) { + t.plan(4); + var map = new OpenLayers.Map('map'); + layer = new OpenLayers.Layer.Vector(""); + map.addLayer(layer); + var feature1 = new OpenLayers.Feature.Vector(new OpenLayers.Geometry(1.0, 1.0)); + var feature2 = new OpenLayers.Feature.Vector(new OpenLayers.Geometry(2.0, 2.0)); + layer.addFeatures([feature1,feature2]); + layer.raiseFeature(feature1, 1); + t.eq(layer.features.indexOf(feature1), 1, "first feature raised one up"); + layer.raiseFeature(feature1, -1); + t.eq(layer.features.indexOf(feature1), 0, "first feature raised one down"); + layer.raiseFeature(feature1, -1); + t.eq(layer.features.indexOf(feature1), 0, "feature stays at index 0 because there is no lower position than index 0."); + layer.raiseFeature(feature1, 2); + t.eq(layer.features.indexOf(feature1), 1, "first feature raised 2 up, but has index 1 because there are only two features in layer."); + } From 5bffb3dea3ac23cc50a8ee88c8e2ff58c11fc2f5 Mon Sep 17 00:00:00 2001 From: mosesonline Date: Fri, 7 Dec 2012 12:46:28 +0100 Subject: [PATCH 2/7] Update lib/OpenLayers/Handler/Feature.js simplify code a little --- lib/OpenLayers/Handler/Feature.js | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/lib/OpenLayers/Handler/Feature.js b/lib/OpenLayers/Handler/Feature.js index 63d64b1f63..c71d141151 100644 --- a/lib/OpenLayers/Handler/Feature.js +++ b/lib/OpenLayers/Handler/Feature.js @@ -325,10 +325,8 @@ OpenLayers.Handler.Feature = OpenLayers.Class(OpenLayers.Handler, { // we enter handle. Yes, a bit hackish... this.feature = null; } - } else { - if(this.lastFeature && (previouslyIn || click)) { - this.triggerCallback(type, 'out', [this.lastFeature]); - } + } else if(this.lastFeature && (previouslyIn || click)) { + this.triggerCallback(type, 'out', [this.lastFeature]); } return handled; }, From 9ea9b859880ea900e6c1e3211f1f4f3d8ae0b6d6 Mon Sep 17 00:00:00 2001 From: mosesonline Date: Tue, 11 Dec 2012 07:27:09 +0100 Subject: [PATCH 3/7] Revert "[New] raise feature function to move features in layer." This reverts commit cb1c3a834aedea34e98fe20df112304316287004. the drawing order is controlled by Renderer not the Layer classes --- lib/OpenLayers/Layer/Vector.js | 23 +------------- tests/Layer/Vector.html | 56 +--------------------------------- 2 files changed, 2 insertions(+), 77 deletions(-) diff --git a/lib/OpenLayers/Layer/Vector.js b/lib/OpenLayers/Layer/Vector.js index ddd2f6f4a0..4300e59b16 100644 --- a/lib/OpenLayers/Layer/Vector.js +++ b/lib/OpenLayers/Layer/Vector.js @@ -699,28 +699,7 @@ OpenLayers.Layer.Vector = OpenLayers.Class(OpenLayers.Layer, { this.events.triggerEvent("featuresremoved", {features: features}); } }, - - /** - * APIMethod: raiseFeature - * Changes the index of the given feature by delta and redraws the layer. If delta is positive, the feature is moved up the layer's feature stack; if delta is negative, the feature is moved down. - * If delta + current is negative it is set to 0 and feature will be at the bottom; if it is bigger then the features count than it is set to the last index and the feature will be at the top. - */ - raiseFeature:function(feature, delta) { - var base = this.features.indexOf(feature); - var idx = base + delta; - if (idx < 0) { - idx = 0; - } else if (idx > this.features.length) { - idx = this.features.length; - } - if (base != idx) { - this.features.splice(base, 1); - this.features.splice(idx, 0, feature); - this.eraseFeatures(this.features); - this.redraw(); - } - }, - + /** * APIMethod: removeAllFeatures * Remove all features from the layer. diff --git a/tests/Layer/Vector.html b/tests/Layer/Vector.html index 2cfc9c07bd..aa3e2f8a7e 100644 --- a/tests/Layer/Vector.html +++ b/tests/Layer/Vector.html @@ -461,44 +461,7 @@ layer.features = []; } - function test_drawfeature_ng(t) { - t.plan(3); - var layer = new OpenLayers.Layer.Vector(); - layer.drawn = true; - var log = []; - - // Bogus layer renderer needs some methods - // for functional tests. - var renderer = { - initialize: function() {}, - drawFeature: function(feature, style) { - log.push(style); - }, - root: document.createElement("div"), - destroy: function() { }, - eraseFeatures: function() {}, - setExtent: function() {}, - setSize: function() {} - }; - var OrigRendererNG = OpenLayers.Renderer.NG; - OpenLayers.Renderer.NG = OpenLayers.Class(renderer); - - layer.renderer = new OpenLayers.Renderer.NG(); - layer.drawFeature(new OpenLayers.Feature.Vector()); - t.eq(log[0]._createSymbolizer, undefined, "no _createSymbolizer function for static styles"); - - var styleMap = new OpenLayers.StyleMap(new OpenLayers.Style({foo: "${bar}"}, - {context: {"bar": function(feature){ return "baz" }}} - )); - layer.styleMap = styleMap; - layer.drawFeature(new OpenLayers.Feature.Vector()); - t.eq(log[1]._createSymbolizer().foo, "baz", "_createSymbolizer function added to style and returns correct result"); - OpenLayers.Renderer.NG = OrigRendererNG; - - layer.renderer = renderer; - layer.drawFeature(new OpenLayers.Feature.Vector()); - t.eq(log[2]._createSymbolizer, undefined, "no _createSymbolizer function for non-NG renderer"); - } + function test_deleted_state(t) { t.plan(9); @@ -906,23 +869,6 @@ "featuresadded event received expected number of features"); } - function test_raiseFeature(t) { - t.plan(4); - var map = new OpenLayers.Map('map'); - layer = new OpenLayers.Layer.Vector(""); - map.addLayer(layer); - var feature1 = new OpenLayers.Feature.Vector(new OpenLayers.Geometry(1.0, 1.0)); - var feature2 = new OpenLayers.Feature.Vector(new OpenLayers.Geometry(2.0, 2.0)); - layer.addFeatures([feature1,feature2]); - layer.raiseFeature(feature1, 1); - t.eq(layer.features.indexOf(feature1), 1, "first feature raised one up"); - layer.raiseFeature(feature1, -1); - t.eq(layer.features.indexOf(feature1), 0, "first feature raised one down"); - layer.raiseFeature(feature1, -1); - t.eq(layer.features.indexOf(feature1), 0, "feature stays at index 0 because there is no lower position than index 0."); - layer.raiseFeature(feature1, 2); - t.eq(layer.features.indexOf(feature1), 1, "first feature raised 2 up, but has index 1 because there are only two features in layer."); - } From 0cdb3aeb5267f77bbc6571b3bd4be1da40c4b5e7 Mon Sep 17 00:00:00 2001 From: mosesonline Date: Tue, 18 Dec 2012 09:34:48 +0100 Subject: [PATCH 4/7] [BugFix] Fix selectFeatures is null exception when layer is destroyed. Since you can listen only to preremovelayer to handle removing layer with SelectFeature. But preremovelayer is triggered after selectFeatures is set to null. --- lib/OpenLayers/Control/SelectFeature.js | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/lib/OpenLayers/Control/SelectFeature.js b/lib/OpenLayers/Control/SelectFeature.js index 198cef9b1b..654a1f77a7 100644 --- a/lib/OpenLayers/Control/SelectFeature.js +++ b/lib/OpenLayers/Control/SelectFeature.js @@ -305,14 +305,16 @@ OpenLayers.Control.SelectFeature = OpenLayers.Class(OpenLayers.Control, { for(l=0; l numExcept) { - feature = layer.selectedFeatures[numExcept]; - if(!options || options.except != feature) { - this.unselect(feature); - } else { - ++numExcept; + if(layer.selectedFeatures != null) { + while(layer.selectedFeatures.length > numExcept) { + feature = layer.selectedFeatures[numExcept]; + if(!options || options.except != feature) { + this.unselect(feature); + } else { + ++numExcept; + } } - } + } } }, From 7aed43185b77b7945fe65b5879039232101283ad Mon Sep 17 00:00:00 2001 From: mosesonline Date: Tue, 18 Dec 2012 09:43:22 +0100 Subject: [PATCH 5/7] [BugFix] Added tests for handle destroyed layer in SelectFeature. --- tests/Control/SelectFeature.html | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/tests/Control/SelectFeature.html b/tests/Control/SelectFeature.html index 150ab07c5c..b0d7d885a1 100644 --- a/tests/Control/SelectFeature.html +++ b/tests/Control/SelectFeature.html @@ -545,6 +545,31 @@ t.eq((control.layers === null), true, "When using setLayer with a single layer, the layers property is removed if present before"); map.destroy(); } + + function test_setLayerWithRemoved(t) { + t.plan(2); + var map = new OpenLayers.Map("map"); + var layer1 = new OpenLayers.Layer.Vector(); + var layer2 = new OpenLayers.Layer.Vector(); + map.addLayer(layer1, layer2); + // initialize it with a single layer + var control = new OpenLayers.Control.SelectFeature(layer1); + map.addControl(control); + control.activate(); + var noError = true; + map.events.register("preremovelayer", this, function(e) { + try { + control.setLayer(layer2); + } catch (e) { + noError = false; + } + }); + layer1.destroy(); + t.eq(layer2.id, control.layer.id, "Layer is set correctly without error"); + t.eq(noError, true,"No error occured during setLayer"); + control.destroy(); + map.destroy(); + } function test_destroy(t) { t.plan(1); From 0263b2b5e1406a77c824664764996a53a6735d9e Mon Sep 17 00:00:00 2001 From: mosesonline Date: Tue, 18 Dec 2012 09:47:33 +0100 Subject: [PATCH 6/7] [Change] renamed test and added print of error --- tests/Control/SelectFeature.html | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/Control/SelectFeature.html b/tests/Control/SelectFeature.html index b0d7d885a1..6e522e77f9 100644 --- a/tests/Control/SelectFeature.html +++ b/tests/Control/SelectFeature.html @@ -546,7 +546,7 @@ map.destroy(); } - function test_setLayerWithRemoved(t) { + function test_setLayerWithRemoving(t) { t.plan(2); var map = new OpenLayers.Map("map"); var layer1 = new OpenLayers.Layer.Vector(); @@ -556,17 +556,17 @@ var control = new OpenLayers.Control.SelectFeature(layer1); map.addControl(control); control.activate(); - var noError = true; + var noError = null; map.events.register("preremovelayer", this, function(e) { try { control.setLayer(layer2); } catch (e) { - noError = false; + noError = e; } }); layer1.destroy(); t.eq(layer2.id, control.layer.id, "Layer is set correctly without error"); - t.eq(noError, true,"No error occured during setLayer"); + t.eq(noError, null,"No error occured during setLayer. Error is: '"+noError+"'"); control.destroy(); map.destroy(); } From ef028b1e913a0028dc94048f4e2d9bb81583167d Mon Sep 17 00:00:00 2001 From: mosesonline Date: Thu, 20 Dec 2012 08:55:21 +0100 Subject: [PATCH 7/7] Update lib/OpenLayers/Control/SelectFeature.js indentation fixed and added comment to explain null case --- lib/OpenLayers/Control/SelectFeature.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/OpenLayers/Control/SelectFeature.js b/lib/OpenLayers/Control/SelectFeature.js index 654a1f77a7..837487cdc5 100644 --- a/lib/OpenLayers/Control/SelectFeature.js +++ b/lib/OpenLayers/Control/SelectFeature.js @@ -305,7 +305,10 @@ OpenLayers.Control.SelectFeature = OpenLayers.Class(OpenLayers.Control, { for(l=0; l numExcept) { feature = layer.selectedFeatures[numExcept]; if(!options || options.except != feature) { @@ -314,7 +317,7 @@ OpenLayers.Control.SelectFeature = OpenLayers.Class(OpenLayers.Control, { ++numExcept; } } - } + } } },