Taming the select feature control a bit. Previously, onUnselect was called twice for every unselection. r=elemoine (closes #1234)

git-svn-id: http://svn.openlayers.org/trunk/openlayers@5959 dc9f47b5-9b13-0410-9fdd-eb0c1a62fdaf

lib/OpenLayers/Control/SelectFeature.js

@@ -127,12 +127,20 @@ OpenLayers.Control.SelectFeature = OpenLayers.Class(OpenLayers.Control, {
 
     /**
      * Method: unselectAll
-     * Unselect all selected features.
+     * Unselect all selected features.  To unselect all except for a single
+     *     feature, set the options.except property to the feature.
+     *
+     * Parameters:
+     * options - {Object} Optional configuration object.
      */
-    unselectAll: function() {
+    unselectAll: function(options) {
         // we'll want an option to supress notification here
-        while (this.layer.selectedFeatures.length > 0) {
+        var feature;
-            this.unselect(this.layer.selectedFeatures[0]);
+        for(var i=this.layer.selectedFeatures.length-1; i>=0; --i) {
+            feature = this.layer.selectedFeatures[i];
+            if(!options || options.except != feature) {
+                this.unselect(feature);
+            }
         }
     },
 
@@ -145,26 +153,46 @@ OpenLayers.Control.SelectFeature = OpenLayers.Class(OpenLayers.Control, {
      * feature - {<OpenLayers.Feature.Vector>} 
      */
     clickFeature: function(feature) {
-        if(this.hover) {
+        if(!this.hover) {
-            return;
-        }
             var selected = (OpenLayers.Util.indexOf(this.layer.selectedFeatures,
                                                     feature) > -1);
-        if(!this.multiple && !this.handler.evt[this.multipleKey]) {
-            // perhaps an "except" argument
-            this.unselectAll();
-        }
             if(selected) {
-            if(this.toggle || this.handler.evt[this.toggleKey]) {
+                if(this.toggleSelect()) {
-                // notify here
                     this.unselect(feature);
-            } else {
+                } else if(!this.multipleSelect()) {
-                // don't notify here - could be removed if unselectAll is modified
+                    this.unselectAll({except: feature});
-                this.select(feature);
                 }
             } else {
+                if(!this.multipleSelect()) {
+                    this.unselectAll({except: feature});
+                }
                 this.select(feature);
             }
+        }
+    },
+
+    /**
+     * Method: multipleSelect
+     * Allow for multiple selected features based on <multiple> property and
+     *     <multipleKey> event modifier.
+     *
+     * Returns:
+     * {Boolean} Allow for multiple selected features.
+     */
+    multipleSelect: function() {
+        return this.multiple || this.handler.evt[this.multipleKey];
+    },
+    
+    /**
+     * Method: toggleSelect
+     * Event should toggle the selected state of a feature based on <toggle>
+     *     property and <toggleKey> event modifier.
+     *
+     * Returns:
+     * {Boolean} Toggle the selected state of a feature.
+     */
+    toggleSelect: function() {
+        return this.toggle || this.handler.evt[this.toggleKey];
     },
 
     /**

tests/Control/test_SelectFeature.html

@@ -73,6 +73,64 @@
         t.eq(feature.style.strokeColor, OpenLayers.Feature.Vector.style["default"].strokeColor, "style set back to original correctly");
     }
     
+    function test_Control_SelectFeature_clickFeature(t) {
+        t.plan(4);
+        // mock up layer
+        var layer = {
+            selectedFeatures: [],
+            drawFeature: function() {}
+        };
+        // mock up active control
+        var control = new OpenLayers.Control.SelectFeature(layer);
+        control.handler = {
+            evt: {}
+        };
+        // mock up features
+        var features = new Array(4);
+        for(var i=0; i<features.length; ++i) {
+            features[i] = {
+                id: Math.random(),
+                tested: 0,
+                style: ""
+            };
+        }
+        
+        // test that onSelect gets called properly
+        control.onSelect = function(feature) {
+            feature.tested += 1;
+            t.eq(feature, features[feature.index],
+                 "onSelect called with proper feature (" + feature.index + ")");
+            t.eq(feature.tested, feature.test,
+                 "onSelect called only once for feature (" + feature.index + ")");
+        }
+
+        // test that onUnselect gets called properly
+        control.onUnselect = function(feature) {
+            feature.tested += 1;
+            t.eq(feature, features[feature.index],
+                 "onUnselect called with proper feature (" + feature.index + ")");
+            t.eq(feature.tested, feature.test, 
+                 "onUnselect called only once for feature (" + feature.index + ")");
+        }
+        
+        // mock up first click on first feature (runs 2 tests from onSelect)
+        var feature = features[0];
+        feature.index = 0;
+        feature.test = 1;
+        control.clickFeature(feature);
+
+        // mock up second click on first feature (runs no tests - already selected)
+        control.toggle = false;
+        control.clickFeature(feature);
+
+        // mock up second click on first feature (runs 2 tests from onUnselect)
+        control.toggle = true;
+        feature.test = 2;
+        control.clickFeature(feature);
+
+        
+    }
+    
     function test_Control_SelectFeature_activate(t) {
         t.plan(2);
         var map = new OpenLayers.Map("map");