Add ModifyFeature Control

Okay, I think I've got relatively complete unit tests for the ModifyFeature control. (I did find one bug -- virtual vertices being included for multipoints -- so I feel like the testing must have at least been somewhat complete ;))

Tests are included in Tim's sandbox code.

I also made a modification to support multiple deleteCodes -- 'delete' on the mac laptop is two keys on opposite sides of the keyboard, which makes it damn near unusable -- and changed the name to fit.

I was thinking of adding escape to the list of codes -- my first instinct has been to hit 'esc' every time i make a mistake.

Other than that, I'm happy with it: I'm relatively convinced that it works as expected, and that it does the right things in all the functions.

Note that the tests are one function short of 100% test coverage of the unit tests, but they are *unit* tests, and not integration tests: they're running on mock objects for the most part. Completing the onDrag test would bring us to 100% test coverage, and integration tests can be added as we find bugs in the code, as far as I'm concerned.

tests pass in ie/ff - will gladly commit w/ approval

Replying to tschaub:

tests pass in ie/ff - will gladly commit w/ approval

Tests did not fully pass on FF :

test_ModifyFeature_onModification planned 2 assertions but got 1; fail 0 ok 1

The keycode 146 is used for trigger deletion. 100 or 46 should be used instead. See patch-941-A0.diff for a patch that corrects that.

Replying to elemoine:

Replying to tschaub:

tests pass in ie/ff - will gladly commit w/ approval

Tests did not fully pass on FF : test_ModifyFeature_onModification planned 2 assertions but got 1; fail 0 ok 1 The keycode 146 is used for trigger deletion. 100 or 46 should be used instead. See patch-941-A0.diff for a patch that corrects that.

One additional comment on test_ModifyFeature_runDelete:

line 95, should'nt

control.vertices = [fakeFeature];

be

control.vertices = [fakePointFeature];

?

On a possibly related note: that particular test has 5 assertions planned (t.plan(5)) but actually includes 6 assertions that should pass. The assertion one line 113 (t.ok(true, "Called with 'd' keycode")) isn't evaluated, while it should be.

Thanks elemoine - this patch is suffering from two things: my being on a conf call while patching and my lack of understanding of crschmidt's tests. I'm taking time with it now to make a real test.

Modified handleKeypress tests. Think it is good to go now. Please confirm. Tests pass in IE/FF.

After discussion with crschmidt, we decided to disable point deletion (not vertex deletion). Since we're not doing full-feature in this control right now, we shouldn't do point deletion.

New patch on the way.

modify features patch

Ok, hit this one if you please (all good in IE/FF).

Looks good. Go for it

(In [4272]) Modify away! This was a long time coming. Thanks all for contributions. The ModifyFeature control lets you select a feature for modification, drag around vertices (or the whole feature in the case of a point), delete vertices, and add new vertices (closes #941).