Create vertex on boundary single click #3635
Conversation
bjornharrtell
changed the title
|
Ready for review. Ping @tschaub. |
|
Rebased on 3.5.0 master to keep this ready for review. |
There was a problem hiding this comment.
This could be initialized to [NaN, NaN].
There was a problem hiding this comment.
Or maybe this.lastNewVertexPixel_ = null?
There was a problem hiding this comment.
Though I think that would break the checks below, right?
There was a problem hiding this comment.
I only noticed the checks below after I send my comment. You are right, [NaN, NaN] is cleaner because you don't have to do an additional check (goog.isNull).
|
This looks like a nice solution @bjornharrtell. What do you think about clearing the |
|
Thanks for the review. Changed to initialize with NaN. Clearing |
ghost
mentioned this pull request
|
With #3461 now merged I think this change is even more important so I've rebased this on master. |
|
LGTM |
Create vertex on boundary single click
|
I about to suggest this change for linestrings, but it looks like this PR also handles lines. I was misled by the title referring to 'boundaries'. |
|
@bjornharrtell could you maybe have a look into #3935? it seems to be related to this change. |
|
I haven't been able to understand the connection between this PR and #3935, admittedly without putting any real time into it, but I'll look at rebasing this when it's in. |
|
Ehm, seems I forgot that this PR is actually merged for quite some time :) |
|
@bjornharrtell my proposed fix for #3935 is in #3946. feel free to review. |
6 participants
This PR changes the current behaviour of Modify interaction in that it will create a vertex when single clicking a boundary and raise revision by 1.
Previously, single clicking a boundary would create a vertex but immediately delete it resulting in unmodified geometry but with revision raised by 2.