Refactor postEditor#insertPost to handle more situations #267
Conversation
| @@ -754,8 +754,7 @@ class Editor { | |||
| let pastedPost = parsePostFromPaste(event, this.builder, this._parserPlugins); | |||
|
|
|||
| this.run(postEditor => { | |||
| let nextPosition = postEditor.insertPost(position, pastedPost); | |||
| postEditor.setRange(new Range(nextPosition)); | |||
There was a problem hiding this comment.
Can this be left outside insertPost?
We discussed the range stuff in detail but I don't remember if we talked about moving cursor changes into postEditor methods. Am I forgetting a reason this must be done?
There was a problem hiding this comment.
I'm not sure what you mean by "outside", but this can still use the position-returning semantics...I switched it back to the way it was. postEditor.insertPost now returns a Position
There was a problem hiding this comment.
Cool. I'm not sure if we had the discussion or not. I know having mutation methods with the side effect of changing cursor position is something we've avoided until now. However the arguments against it are fewer now that we have setRange, a safe way to set the position and read it back without the DOM.
|
Some pretty great stuff!!! |
bantic
changed the title
|
@mixonic I've added some additional test coverage for new methods and addressed the first round of feedback. This is ready for a final review now. If all looks good, I'll squash and merge. |
* fix bug in post#cloneRange when range starts in list item * add assert.isPostSimilar, assert.isRenderTreeEqual, assert.isPositionEqual * fix ListSection#clone to use the same tagName for the clonee as the cloner * add PostEditor#insertMarkers method * add private/intimate methods for splitting list items and lists in postEditor Fix #249 #259
bantic
force-pushed
the
copy-paste-refactor-259
branch
from
be66fd7 to
cda1e7e
Compare
Refactor postEditor#insertPost to handle more situations
Ready to start getting feedback on this. There are some todos and fixmes I intend to address before this should be merged, though.
Fix #249 #259