-
Notifications
You must be signed in to change notification settings - Fork 193
Continue a polygon anywhere #228
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds very cool, thanks!
Would you mind adding a few tests ? I'd say four for rotateHere
(one which middle index, one with 0 and one with last index with simple polygon, and one at least with middle index for multipolygon) and one for calling continue
on a polygon ?
You should find examples in the current test to start from, but if you need more help on this please ask :)
Anyway, thanks again for this work!
MIN_VERTEX: 3, | ||
|
||
// 🍂method continueHere(latlngs: Array, index: int) | ||
// Set up drawing tools to continue the line forward, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe in the comment we should talk about "path" or "stroke" or even "polygon", given it does not apply for actually lines ?
for (let i = 0; i < len; i++) { | ||
// goal: latlngs[len-1] = old[index] | ||
latlngs[i] = old[(i + index + 1) % len] | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given this method is touching the latlngs, I think we whould call onEdited
here.
const index = this.getIndex() | ||
if (index === 0) this.editor.continueBackward(this.latlngs) | ||
else if (index === this.getLastIndex()) this.editor.continueForward(this.latlngs) | ||
if (index === 0 && this.editor.continueBackward) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add brackets now that the if
is not a one line anymore ?
Thank you for the feedback! Yes, I'll clean up the code/comments, emit the event, and write some tests to prevent future regressions once my schedule frees up a bit. |
Similar to continuing a line at its start or end, it is useful to tap into a polygon and draw more vertices.
Example:
polygon.on('editable:vertex:ctrlclick', (e) => e.vertex.continue())
The subshape's latlngs are rotated so that the clicked vertex is at the end of the latlngs, then it is continued with
startDrawingForward()
.It might not be clear whether the new vertices will be inserted counter-/clockwise after the vertex, but this might not matter to the user. Deleting the clicked vertex (and continuing inside its gap) would be more unambiguous behavior. Although, then
continue
would be the wrong name for that. Thoughts?Before: (1) is the start/end of the polygon, (2) is where the user ctrl-clicks.

After: new vertices have been drawn, here in counter-clockwise order.
