Hello Tim and all,
I've started working on changing things in Handler.Feature. Currently
Handler.Feature has the notion of feature selection, which I think it
shouldn't. The feature selection logic must be in
Control.SelectFeature and not elsewhere. For example, I don't think
Handler.Feature should keep a reference to a feature, this should be
done in the upper-level object - the SelectFeature control. In
addition, keeping references to selected objects is done in the vector
layer, so that's another reason for Handler.Feature not to keep any
reference on selected feature.
I'm attaching a patch to get a sense on what I'm doing. This isn't
complete, as it most probably breaks Control.DragFeature. But you'll
note that the patch simplifies the code, most notably it removes the
complex select() method in Handler.Feature.
What do you think? Does some work on that direction make sense?
Thanks,
--
Eric
Index: lib/OpenLayers/Control/SelectFeature.js
===================================================================
--- lib/OpenLayers/Control/SelectFeature.js (revision 5115)
+++ lib/OpenLayers/Control/SelectFeature.js (working copy)
@@ -27,7 +27,7 @@
* ignores clicks and only listens to mouse moves.
*/
hover: false,
-
+
/**
* APIProperty: onSelect
* {Function} Optional function to be called when a feature is selected.
@@ -84,9 +84,8 @@
OpenLayers.Control.prototype.initialize.apply(this, [options]);
this.layer = layer;
this.callbacks = OpenLayers.Util.extend({
- click: this.clickFeature,
- over: this.overFeature,
- out: this.outFeature
+ click: this.onClick,
+ move: this.onMove,
}, this.callbacks);
var handlerOptions = {geometryTypes: this.geometryTypes};
this.handler = new OpenLayers.Handler.Feature(this, layer,
@@ -96,67 +95,65 @@
/**
* Method: clickFeature
- * Called when the feature handler detects a click on a feature
*
* Parameters:
* feature - {<OpenLayers.Vector.Feature>}
+ *
+ * Returns:
+ * {Boolean}
*/
- clickFeature: function(feature) {
+ onClick: function(feature) {
if(this.hover) {
- return;
+ return false;
}
- if (this.multiple) {
- if(OpenLayers.Util.indexOf(this.layer.selectedFeatures, feature) > -1) {
- this.unselect(feature);
- } else {
- this.select(feature);
- }
- } else {
- if(OpenLayers.Util.indexOf(this.layer.selectedFeatures, feature) > -1) {
- this.unselect(feature);
- } else {
- if (this.layer.selectedFeatures) {
- for (var i = 0; i < this.layer.selectedFeatures.length; i++) {
- this.unselect(this.layer.selectedFeatures[i]);
- }
- }
- this.select(feature);
- }
- }
+ return this._onEvent(feature);
},
/**
- * Method: overFeature
- * Called when the feature handler detects a mouse-over on a feature.
- * Only responds if this.hover is true.
- *
+ * Method: onMove
+ *
* Parameters:
- * feature - {<OpenLayers.Feature.Vector>}
+ * feature - {<OpenLayers.Feature.Vector>}
+ *
+ * Returns:
+ * {Boolean}
*/
- overFeature: function(feature) {
- if(!this.hover) {
- return;
+ onMove: function(feature) {
+ if (!this.hover) {
+ return false;
}
- if(!(OpenLayers.Util.indexOf(this.layer.selectedFeatures, feature) > -1)) {
- this.select(feature);
- }
+ return this._onEvent(feature);
},
/**
- * Method: outFeature
- * Called when the feature handler detects a mouse-out on a feature.
- * Only responds if this.hover is true.
- *
+ * Method: _onEvent
+ * Called when the feature handler detects an event.
+ *
* Parameters:
- * feature - {<OpenLayers.Feature.Vector>}
+ * feature - {<OpenLayers.Feature.Vector>}
+ *
+ * Returns:
+ * {Boolean}
*/
- outFeature: function(feature) {
- if(!this.hover) {
- return;
+ _onEvent: function(feature) {
+ var selected = false;
+ if (!this.multiple && this.layer.selectedFeatures) {
+ for (var i = 0; i < this.layer.selectedFeatures.length; i++) {
+ this.unselect(this.layer.selectedFeatures[i]);
+ }
}
- this.unselect(feature);
+ if (feature && (this.geometryTypes == null ||
+ OpenLayers.Util.indexOf(this.geometryTypes,
+ feature.geometry.CLASS_NAME) > -1)) {
+
+ if (OpenLayers.Util.indexOf(this.layer.selectedFeatures, feature) < 0) {
+ this.select(feature);
+ selected = true;
+ }
+ }
+ return selected;
},
-
+
/**
* Method: select
* Add feature to the layer's selectedFeature array, render the feature as
Index: lib/OpenLayers/Handler.js
===================================================================
--- lib/OpenLayers/Handler.js (revision 5115)
+++ lib/OpenLayers/Handler.js (working copy)
@@ -195,10 +195,13 @@
* of the handler's callbacks object.
* args - {Array(*)} An array of arguments (any type) with which to call
* the callback (defined by the control).
+ *
+ * Returns:
+ * {Boolean}
*/
callback: function (name, args) {
if (this.callbacks[name]) {
- this.callbacks[name].apply(this.control, args);
+ return this.callbacks[name].apply(this.control, args);
}
},
Index: lib/OpenLayers/Handler/Feature.js
===================================================================
--- lib/OpenLayers/Handler/Feature.js (revision 5115)
+++ lib/OpenLayers/Handler/Feature.js (working copy)
@@ -28,12 +28,6 @@
layerIndex: null,
/**
- * Property: feature
- * {<OpenLayers.Feature.Vector>}
- */
- feature: null,
-
- /**
* Constructor: OpenLayers.Handler.Feature
*
* Parameters:
@@ -57,7 +51,7 @@
* evt - {Event}
*/
click: function(evt) {
- var selected = this.select('click', evt);
+ var selected = this.handle('click', evt);
return !selected; // stop event propagation if selected
},
@@ -69,7 +63,7 @@
* evt - {Event}
*/
mousedown: function(evt) {
- var selected = this.select('down', evt);
+ var selected = this.handle('down', evt);
return !selected; // stop event propagation if selected
},
@@ -83,7 +77,7 @@
* evt - {Event}
*/
mousemove: function(evt) {
- this.select('move', evt);
+ this.handle('move', evt);
return true;
},
@@ -95,7 +89,7 @@
* evt - {Event}
*/
mouseup: function(evt) {
- var selected = this.select('up', evt);
+ var selected = this.handle('up', evt);
return !selected; // stop event propagation if selected
},
@@ -109,57 +103,23 @@
* evt - {Event}
*/
dblclick: function(evt) {
- var selected = this.select('dblclick', evt);
+ var selected = this.handle('dblclick', evt);
return !selected; // stop event propagation if selected
},
/**
- * Method: select
- * Trigger the appropriate callback if a feature is under the mouse.
+ * Method: handle
+ * Trigger the appropriate callback.
*
* Parameters:
* type - {String} Callback key
+ * evt - {OpenLayers.Event} Event
*
* Returns:
- * {Boolean} A feature was selected
+ * {Boolean} A feature was handled
*/
- select: function(type, evt) {
- var feature = this.layer.getFeatureFromEvent(evt);
- var selected = false;
- if(feature) {
- if(this.geometryTypes == null ||
- (OpenLayers.Util.indexOf(this.geometryTypes,
- feature.geometry.CLASS_NAME) > -1)) {
- // three cases:
- // over a new, out of the last and over a new, or still on the last
- if(!this.feature) {
- // over a new feature
- this.callback('over', [feature]);
- } else if(this.feature != feature) {
- // out of the last and over a new
- this.callback('out', [this.feature]);
- this.callback('over', [feature]);
- }
- this.feature = feature;
- this.callback(type, [feature]);
- selected = true;
- } else {
- if(this.feature && (this.feature != feature)) {
- // out of the last and over a new
- this.callback('out', [this.feature]);
- this.feature = null;
- }
- selected = false;
- }
- } else {
- if(this.feature) {
- // out of the last
- this.callback('out', [this.feature]);
- this.feature = null;
- }
- selected = false;
- }
- return selected;
+ handle: function(type, evt) {
+ return this.callback(type, [this.layer.getFeatureFromEvent(evt)]);
},
/**
_______________________________________________
Dev mailing list
Dev@openlayers.org
http://openlayers.org/mailman/listinfo/dev
