Selaa lähdekoodia

Changed: MoveOperation#movedRangeStart changed from property to method #getMovedRangeStart to have it always up-to-date with #targetPosition.

Szymon Cofalik 9 vuotta sitten
vanhempi
commit
18ec54fe27

+ 19 - 17
packages/ckeditor5-engine/src/model/operation/moveoperation.js

@@ -50,22 +50,6 @@ export default class MoveOperation extends Operation {
 		this.targetPosition = Position.createFromPosition( targetPosition );
 
 		/**
-		 * Position of the start of the moved range after it got moved. This may be different than
-		 * {@link engine.model.operation.MoveOperation#targetPosition} in some cases, i.e. when a range is moved
-		 * inside the same parent but {@link engine.model.operation.MoveOperation#targetPosition targetPosition}
-		 * is after {@link engine.model.operation.MoveOperation#sourcePosition sourcePosition}.
-		 *
-		 *		 vv              vv
-		 *		abcdefg ===> adefbcg
-		 *		     ^          ^
-		 *		     targetPos	movedRangeStart
-		 *		     offset 6	offset 4
-		 *
-		 * @member {engine.model.Position} engine.model.operation.MoveOperation#movedRangeStart
-		 */
-		this.movedRangeStart = this.targetPosition._getTransformedByDeletion( this.sourcePosition, this.howMany );
-
-		/**
 		 * Defines whether `MoveOperation` is sticky. If `MoveOperation` is sticky, during
 		 * {@link engine.model.operation.transform operational transformation} if there will be an operation that
 		 * inserts some nodes at the position equal to the boundary of this `MoveOperation`, that operation will
@@ -95,13 +79,31 @@ export default class MoveOperation extends Operation {
 	}
 
 	/**
+	 * Position of the start of the moved range after it got moved. This may be different than
+	 * {@link engine.model.operation.MoveOperation#targetPosition} in some cases, i.e. when a range is moved
+	 * inside the same parent but {@link engine.model.operation.MoveOperation#targetPosition targetPosition}
+	 * is after {@link engine.model.operation.MoveOperation#sourcePosition sourcePosition}.
+	 *
+	 *		 vv              vv
+	 *		abcdefg ===> adefbcg
+	 *		     ^          ^
+	 *		     targetPos	movedRangeStart
+	 *		     offset 6	offset 4
+	 *
+	 * @type {engine.model.Position}
+	 */
+	getMovedRangeStart() {
+		return this.targetPosition._getTransformedByDeletion( this.sourcePosition, this.howMany );
+	}
+
+	/**
 	 * @inheritDoc
 	 * @returns {engine.model.operation.MoveOperation}
 	 */
 	getReversed() {
 		let newTargetPosition = this.sourcePosition._getTransformedByInsertion( this.targetPosition, this.howMany );
 
-		const op = new this.constructor( this.movedRangeStart, this.howMany, newTargetPosition, this.baseVersion + 1 );
+		const op = new this.constructor( this.getMovedRangeStart(), this.howMany, newTargetPosition, this.baseVersion + 1 );
 		op.isSticky = this.isSticky;
 
 		return op;

+ 0 - 1
packages/ckeditor5-engine/src/model/operation/removeoperation.js

@@ -104,7 +104,6 @@ export default class RemoveOperation extends MoveOperation {
 	clone() {
 		let removeOperation = new RemoveOperation( this.sourcePosition, this.howMany, this.baseVersion );
 		removeOperation.targetPosition = Position.createFromPosition( this.targetPosition );
-		removeOperation.movedRangeStart = Position.createFromPosition( this.movedRangeStart );
 
 		return removeOperation;
 	}

+ 5 - 5
packages/ckeditor5-engine/src/model/operation/transform.js

@@ -180,15 +180,15 @@ const ot = {
 				// previously transformed target position.
 				// Note that we do not use Position._getTransformedByMove on range boundaries because we need to
 				// transform by insertion a range as a whole, since newTargetPosition might be inside that range.
-				ranges = difference._getTransformedByInsertion( b.movedRangeStart, b.howMany, true, false ).reverse();
+				ranges = difference._getTransformedByInsertion( b.getMovedRangeStart(), b.howMany, true, false ).reverse();
 			}
 
 			if ( common !== null ) {
 				// Here we do not need to worry that newTargetPosition is inside moved range, because that
 				// would mean that the MoveOperation targets into itself, and that is incorrect operation.
 				// Instead, we calculate the new position of that part of original range.
-				common.start = common.start._getCombined( b.sourcePosition, b.movedRangeStart );
-				common.end = common.end._getCombined( b.sourcePosition, b.movedRangeStart );
+				common.start = common.start._getCombined( b.sourcePosition, b.getMovedRangeStart() );
+				common.end = common.end._getCombined( b.sourcePosition, b.getMovedRangeStart() );
 
 				ranges.push( common );
 			}
@@ -373,8 +373,8 @@ const ot = {
 				// Here we do not need to worry that newTargetPosition is inside moved range, because that
 				// would mean that the MoveOperation targets into itself, and that is incorrect operation.
 				// Instead, we calculate the new position of that part of original range.
-				common.start = common.start._getCombined( b.sourcePosition, b.movedRangeStart );
-				common.end = common.end._getCombined( b.sourcePosition, b.movedRangeStart );
+				common.start = common.start._getCombined( b.sourcePosition, b.getMovedRangeStart() );
+				common.end = common.end._getCombined( b.sourcePosition, b.getMovedRangeStart() );
 
 				// We have to take care of proper range order.
 				if ( difference && difference.start.isBefore( common.start ) ) {

+ 13 - 1
packages/ckeditor5-engine/tests/model/operation/moveoperation.js

@@ -269,6 +269,19 @@ describe( 'MoveOperation', () => {
 		expect( clone.baseVersion ).to.equal( baseVersion );
 	} );
 
+	describe( 'getMovedRangeStart', () => {
+		it( 'should return move operation target position transformed by removing move operation source range', () => {
+			let sourcePosition = new Position( root, [ 0, 2 ] );
+			let targetPosition = new Position( root, [ 0, 6 ] );
+			let howMany = 3;
+			let baseVersion = doc.version;
+
+			let op = new MoveOperation( sourcePosition, howMany, targetPosition, baseVersion );
+
+			expect( op.getMovedRangeStart().path ).to.deep.equal( [ 0, 3 ] );
+		} );
+	} );
+
 	describe( 'toJSON', () => {
 		it( 'should create proper json object', () => {
 			const sourcePosition = new Position( root, [ 0, 0 ] );
@@ -282,7 +295,6 @@ describe( 'MoveOperation', () => {
 				baseVersion: 0,
 				howMany: 1,
 				isSticky: false,
-				movedRangeStart: jsonParseStringify( op.movedRangeStart ),
 				sourcePosition: jsonParseStringify( sourcePosition ),
 				targetPosition: jsonParseStringify( targetPosition )
 			} );

+ 0 - 1
packages/ckeditor5-engine/tests/model/operation/reinsertoperation.js

@@ -102,7 +102,6 @@ describe( 'ReinsertOperation', () => {
 				baseVersion: 0,
 				howMany: 2,
 				isSticky: false,
-				movedRangeStart: jsonParseStringify( operation.movedRangeStart ),
 				sourcePosition: jsonParseStringify( operation.sourcePosition ),
 				targetPosition: jsonParseStringify( operation.targetPosition )
 			} );

+ 0 - 1
packages/ckeditor5-engine/tests/model/operation/removeoperation.js

@@ -190,7 +190,6 @@ describe( 'RemoveOperation', () => {
 				baseVersion: 0,
 				howMany: 2,
 				isSticky: false,
-				movedRangeStart: jsonParseStringify( op.movedRangeStart ),
 				sourcePosition: jsonParseStringify( op.sourcePosition ),
 				targetPosition: jsonParseStringify( op.targetPosition )
 			} );

+ 0 - 2
packages/ckeditor5-engine/tests/model/operation/transform.js

@@ -1377,7 +1377,6 @@ describe( 'transform', () => {
 				);
 
 				transformBy.targetPosition.path = [ 0, 0 ];
-				transformBy.movedRangeStart.path = [ 0, 0 ];
 
 				let transOp = transform( op, transformBy );
 
@@ -1397,7 +1396,6 @@ describe( 'transform', () => {
 				);
 
 				transformBy.targetPosition.path = [ 4, 0 ];
-				transformBy.movedRangeStart.path = [ 4, 0 ];
 
 				let transOp = transform( op, transformBy );