sketch handler updates

update to sketch handlers

I'm sure "needs tests" is the next comment, but I thought I would start with the code changes.

Tim,

Handler/Point.js:

• destroyFeature should check if the layer isn't null before doing layer.destroyFeatures(), I have a case where a featureAdded callback (set in a draw feature control) deactivates the draw feature control, and deactivating the draw feature control destroys the sketch layer

Thanks

warning: this is getting moved to 2.8 unless it can gets reviewed and committed in the next 30 hours or so....

My final review comments.

Handler/Point.js:

• destroyFeature should check if the layer isn't null before doing layer.destroyFeatures(), I have a case where a featureAdded callback (set in a draw feature control) deactivates the draw feature control, and deactivating the draw feature control destroys the sketch layer
• why does createFeature now call layer.addFeatures (with a feature whose geometry has no coordinates)? I've seen issues with this and the new renderer code. The new renderer code does if (!feature.geometry.getBounds().intersectsBounds(this.extent)) { which is problematic if the geometry has no coordinates (getBounds() returns null)
• because of the above (I think), the unit tests fail with current trunk
• if we use addFeatures we should pass it {silent: true} to avoid featureadded and featuresadded events
• layerOptions should be a class property

Replying to elemoine:

Handler/Point.js: * destroyFeature should check if the layer isn't null before doing layer.destroyFeatures(), I have a case where a featureAdded callback (set in a draw feature control) deactivates the draw feature control, and deactivating the draw feature control destroys the sketch layer

Easy enough.

Replying to elemoine:

Handler/Point.js: * why does createFeature now call layer.addFeatures (with a feature whose geometry has no coordinates)? I've seen issues with this and the new renderer code. The new renderer code does if (!feature.geometry.getBounds().intersectsBounds(this.extent)) { which is problematic if the geometry has no coordinates (getBounds() returns null)

I was previously just doing layer.drawFeature(feature) instead of adding the feature to the (temporary) layer. This meant if you set persist to true and zoomed out after drawing a sketch, the sketch would not get redrawn. The proper way to draw features is to have them on the layer.

The renderer changes introduce a regression. Previously, we could add a feature to a layer with null geometry. I thought ahocevar changed things so this would still be allowed. I'll poke around a bit.

* because of the above (I think), the unit tests fail with current trunk

Will figure this out.

* if we use addFeatures we should pass it {silent: true} to avoid featureadded and featuresadded events

This is a temporary layer that only the handler has a reference to. Can't imagine who is registering any listeners for events there. But it is easy enough to call with silent true.

* layerOptions should be a class property

Will do.

Replying to elemoine:

Handler/Point.js: * because of the above (I think), the unit tests fail with current trunk

Ok, there were a couple places where the sketch handlers were still modifying geometries without clearing bounds. Previously, this wasn't a problem because the getBounds was not being called until all point geometries had valid coords.

updates to sketch handlers

New patch addresses all issues above. Examples work, (relevant) tests pass.

This is looking good. I tested the patch in my application (with the measure control and persis:true) and it works great. Please commit.

(In [7964]) Sketch handler updates. Patch by tschaub, review elemoine (Closes #1698)