-
Notifications
You must be signed in to change notification settings - Fork 40
Temporarily substitute wrap and unwrap operations with inserts and moves #1565
Conversation
…operty. Renamed `SplitOperation#position` to `#splitPosition`.
These errors are because of block-quote's post fixer is not available in that build. |
src/model/differ.js
Outdated
@@ -238,9 +232,7 @@ export default class Differ { | |||
|
|||
// If the wrap took the element from the graveyard, mark that the element from the graveyard was removed. |
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.
Comment need to be updated.
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.
Mmmm... I don't understand why?
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.
As I mentioned belove, we should remove all the code related to wrap and unwrap operations which is not used anymore.
@@ -119,7 +119,12 @@ export default class MergeOperation extends Operation { | |||
* @returns {module:engine/model/operation/splitoperation~SplitOperation} | |||
*/ | |||
getReversed() { | |||
return new SplitOperation( this.targetPosition, this.howMany, this.graveyardPosition, this.baseVersion + 1 ); | |||
const targetPosition = this.targetPosition._getTransformedByMergeOperation( this ); |
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.
Some comment is needed.
* @param {Number} howMany Total offset size of elements that are in the split element after `position`. | ||
* @param {module:engine/model/position~Position} insertionPosition Position at which the clone of split element | ||
* (or element from graveyard) will be inserted. |
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 some example?
src/model/writer.js
Outdated
@@ -653,7 +651,8 @@ export default class Writer { | |||
do { | |||
const version = splitElement.root.document ? splitElement.root.document.version : null; | |||
const howMany = splitElement.maxOffset - position.offset; | |||
const split = new SplitOperation( position, howMany, null, version ); | |||
const insertionPosition = SplitOperation.getInsertionPosition( position ); | |||
const split = new SplitOperation( position, howMany, insertionPosition, null, version ); |
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.
The way you create this operation is too complicated. :/
We should remove the code and tests related to wrap operations the model (position, differ, etc.). It can be done in a follow-up. |
Suggested merge commit message (convention)
Internal: Removed usage of
WrapOperation
andUnwrapOperation
, usingMoveOperation
andInsertOperation
instead. ChangedSplitOperation#insertionPosition
to a settable. RenamedSplitOperation#position
toSplitOperation#splitPosition
. Closes ckeditor/ckeditor5#4432.