Przeglądaj źródła

Merge pull request #106 from ckeditor/t/89

T/89 Changes emitter for tree model document
Szymon Cofalik 10 lat temu
rodzic
commit
dd2c56eba1

+ 31 - 2
packages/ckeditor5-utils/src/document/document.js

@@ -88,9 +88,11 @@ CKEDITOR.define( [
 					{ operation: operation } );
 			}
 
-			operation._execute();
+			let changes = operation._execute();
+
 			this.version++;
-			this.fire( 'operationApplied', operation );
+
+			this.fire( 'change', operation.type, changes );
 		}
 
 		/**
@@ -151,6 +153,33 @@ CKEDITOR.define( [
 
 			return this.roots.get( name );
 		}
+
+		/**
+		 * Fired when document changes by applying an operation.
+		 *
+		 * There are 5 types of change:
+		 *
+		 * * 'insert' when nodes are inserted,
+		 * * 'remove' when nodes are removed,
+		 * * 'reinsert' when remove is undone,
+		 * * 'move' when nodes are moved,
+		 * * 'attr' when attributes change.
+		 *
+		 * Change event is fired after the change is done. This means that any ranges or positions passed in
+		 * `changeInfo` are referencing nodes and paths in updated tree model.
+		 *
+		 * @event change
+		 * @param {String} type Change type, possible option: 'insert', 'remove', 'reinsert', 'move', 'attr'.
+		 * @param {Object} changeInfo Additional information about the change.
+		 * @param {document.Range} changeInfo.range Range containing changed nodes. Note that for 'remove' the range will be in the
+		 * {@link #_graveyard graveyard root}.
+		 * @param {document.Position} [changeInfo.sourcePosition] Change source position. Exists for 'remove', 'reinsert' and 'move'.
+		 * Note that for 'reinsert' the source position will be in the {@link #_graveyard graveyard root}.
+		 * @param {document.Attribute} [changeInfo.oldAttr] Only for 'attr' type. If the type is 'attr' and `oldAttr`
+		 * is `undefined` it means that new attribute was inserted. Otherwise it contains changed or removed attribute.
+		 * @param {document.Attribute} [changeInfo.newAttr] Only for 'attr' type. If the type is 'attr' and `newAttr`
+		 * is `undefined` it means that attribute was removed. Otherwise it contains changed or inserted attribute.
+		 */
 	}
 
 	utils.extend( Document.prototype, EmitterMixin );

+ 6 - 0
packages/ckeditor5-utils/src/document/operation/changeoperation.js

@@ -61,6 +61,10 @@ CKEDITOR.define( [
 			this.newAttr = newAttr;
 		}
 
+		get type() {
+			return 'attr';
+		}
+
 		clone() {
 			return new ChangeOperation( this.range.clone(), this.oldAttr, this.newAttr, this.baseVersion );
 		}
@@ -130,6 +134,8 @@ CKEDITOR.define( [
 					value.node.setAttr( newAttr );
 				}
 			}
+
+			return { range: this.range, oldAttr: oldAttr, newAttr: newAttr };
 		}
 	}
 

+ 10 - 1
packages/ckeditor5-utils/src/document/operation/insertoperation.js

@@ -8,8 +8,9 @@
 CKEDITOR.define( [
 	'document/operation/operation',
 	'document/nodelist',
+	'document/range',
 	'document/operation/removeoperation'
-], ( Operation, NodeList ) => {
+], ( Operation, NodeList, Range ) => {
 	/**
 	 * Operation to insert list of nodes on the given position in the tree data model.
 	 *
@@ -45,6 +46,10 @@ CKEDITOR.define( [
 			this.nodeList = new NodeList( nodes );
 		}
 
+		get type() {
+			return 'insert';
+		}
+
 		clone() {
 			return new InsertOperation( this.position, this.nodeList, this.baseVersion );
 		}
@@ -58,6 +63,10 @@ CKEDITOR.define( [
 
 		_execute() {
 			this.position.parent.insertChildren( this.position.offset, this.nodeList );
+
+			return {
+				range: Range.createFromPositionAndOffset( this.position, this.nodeList.length )
+			};
 		}
 	}
 

+ 11 - 1
packages/ckeditor5-utils/src/document/operation/moveoperation.js

@@ -7,9 +7,10 @@
 
 CKEDITOR.define( [
 	'document/operation/operation',
+	'document/range',
 	'ckeditorerror',
 	'utils'
-], ( Operation, CKEditorError, utils ) => {
+], ( Operation, Range, CKEditorError, utils ) => {
 	/**
 	 * Operation to move list of subsequent nodes from one position in the document to another.
 	 *
@@ -50,6 +51,10 @@ CKEDITOR.define( [
 			this.howMany = howMany;
 		}
 
+		get type() {
+			return 'move';
+		}
+
 		clone() {
 			return new MoveOperation( this.sourcePosition.clone(), this.targetPosition.clone(), this.howMany, this.baseVersion );
 		}
@@ -121,6 +126,11 @@ CKEDITOR.define( [
 			const removedNodes = sourceElement.removeChildren( sourceOffset, this.howMany );
 
 			targetElement.insertChildren( targetOffset, removedNodes );
+
+			return {
+				sourcePosition: this.sourcePosition,
+				range: Range.createFromPositionAndOffset( this.targetPosition, this.howMany )
+			};
 		}
 	}
 

+ 9 - 1
packages/ckeditor5-utils/src/document/operation/operation.js

@@ -30,6 +30,12 @@ CKEDITOR.define( [], () => {
 			 */
 			this.baseVersion = baseVersion;
 
+			/**
+			 * Operation type.
+			 *
+			 * @property {String} type
+			 */
+
 			/**
 			 * {@link Document.Delta Delta} which the operation is a part of. This property is set by the
 			 * {@link Document.Delta delta} when the operations is added to it by the
@@ -62,8 +68,10 @@ CKEDITOR.define( [], () => {
 			 * Executes the operation - modifications described by the operation attributes
 			 * will be applied to the tree model.
 			 *
-			 * @method _execute
 			 * @protected
+			 * @method _execute
+			 * @returns {Object} Object with additional information about the applied changes. Always has `range`
+			 * property containing changed nodes. May have additional properties depending on the operation type.
 			 */
 		}
 	}

+ 4 - 0
packages/ckeditor5-utils/src/document/operation/reinsertoperation.js

@@ -27,6 +27,10 @@ CKEDITOR.define( [
 
 			return new RemoveOperation( this.targetPosition, this.howMany, this.baseVersion + 1 );
 		}
+
+		get type() {
+			return 'reinsert';
+		}
 	}
 
 	return ReinsertOperation;

+ 4 - 0
packages/ckeditor5-utils/src/document/operation/removeoperation.js

@@ -33,6 +33,10 @@ CKEDITOR.define( [
 			super( position, graveyardPosition, howMany, baseVersion );
 		}
 
+		get type() {
+			return 'remove';
+		}
+
 		getReversed() {
 			// Because of circular dependencies we need to re-require reinsert operation here.
 			const ReinsertOperation = CKEDITOR.require( 'document/operation/reinsertoperation' );

+ 186 - 0
packages/ckeditor5-utils/tests/document/document/change-event.js

@@ -0,0 +1,186 @@
+/**
+ * @license Copyright (c) 2003-2015, CKSource - Frederico Knabben. All rights reserved.
+ * For licensing, see LICENSE.md.
+ */
+
+/* bender-tags: document */
+
+'use strict';
+
+const modules = bender.amd.require(
+	'document/document',
+	'document/element',
+	'document/rootelement',
+	'document/attribute',
+	'document/position',
+	'document/range',
+	'document/operation/changeoperation',
+	'document/operation/insertoperation',
+	'document/operation/moveoperation',
+	'document/operation/reinsertoperation',
+	'document/operation/removeoperation'
+);
+
+describe( 'Document change event', () => {
+	let Document, RootElement, Element, Range, Position;
+	let ChangeOperation, InsertOperation, MoveOperation, ReinsertOperation, RemoveOperation, Attribute;
+
+	before( () => {
+		Document = modules[ 'document/document' ];
+		Element = modules[ 'document/element' ];
+		RootElement = modules[ 'document/rootelement' ];
+		Attribute = modules[ 'document/attribute' ];
+		Position = modules[ 'document/position' ];
+		Range = modules[ 'document/range' ];
+
+		InsertOperation = modules[ 'document/operation/insertoperation' ];
+		ChangeOperation = modules[ 'document/operation/changeoperation' ];
+		MoveOperation = modules[ 'document/operation/moveoperation' ];
+		ReinsertOperation = modules[ 'document/operation/reinsertoperation' ];
+		RemoveOperation = modules[ 'document/operation/removeoperation' ];
+	} );
+
+	let doc, root, graveyard, types, changes;
+
+	beforeEach( () => {
+		doc = new Document();
+		root = doc.createRoot( 'root' );
+		graveyard = doc._graveyard;
+
+		types = [];
+		changes = [];
+
+		doc.on( 'change', ( evt, type, change ) => {
+			types.push( type );
+			changes.push( change );
+		} );
+	} );
+
+	it( 'should be fired when text is inserted', () => {
+		doc.applyOperation( new InsertOperation( new Position( [ 0 ], root ), 'foo', doc.version ) );
+
+		expect( changes ).to.have.length( 1 );
+		expect( types[ 0 ] ).to.equal( 'insert' );
+		expect( changes[ 0 ].range ).to.deep.equal( Range.createFromParentsAndOffsets( root, 0, root, 3 ) );
+	} );
+
+	it( 'should be fired when element is inserted', () => {
+		const element = new Element( 'p' );
+		doc.applyOperation( new InsertOperation( new Position( [ 0 ], root ), element, doc.version ) );
+
+		expect( changes ).to.have.length( 1 );
+		expect( types[ 0 ] ).to.equal( 'insert' );
+		expect( changes[ 0 ].range ).to.deep.equal( Range.createFromParentsAndOffsets( root, 0, root, 1 ) );
+	} );
+
+	it( 'should be fired when nodes are inserted', () => {
+		const element = new Element( 'p' );
+		doc.applyOperation( new InsertOperation( new Position( [ 0 ], root ), [ element, 'foo' ], doc.version ) );
+
+		expect( changes ).to.have.length( 1 );
+		expect( types[ 0 ] ).to.equal( 'insert' );
+		expect( changes[ 0 ].range ).to.deep.equal( Range.createFromParentsAndOffsets( root, 0, root, 4 ) );
+	} );
+
+	it( 'should be fired when nodes are moved', () => {
+		const p1 = new Element( 'p' );
+		p1.insertChildren( 0, [ new Element( 'p' ), 'foo' ] );
+
+		const p2 = new Element( 'p' );
+
+		root.insertChildren( 0, [ p1, p2 ] );
+
+		doc.applyOperation(
+			new MoveOperation(
+				new Position( [ 0, 0 ], root ),
+				new Position( [ 1, 0 ], root ),
+				3,
+				doc.version
+			)
+		);
+
+		expect( changes ).to.have.length( 1 );
+		expect( types[ 0 ] ).to.equal( 'move' );
+		expect( changes[ 0 ].range ).to.deep.equal( Range.createFromParentsAndOffsets( p2, 0, p2, 3 ) );
+		expect( changes[ 0 ].sourcePosition ).to.deep.equal( Position.createFromParentAndOffset( p1, 0 ) );
+	} );
+
+	it( 'should be fired when multiple nodes are removed and reinserted', () => {
+		root.insertChildren( 0, 'foo' );
+
+		const removeOperation = new RemoveOperation( new Position( [ 0 ], root ), 3, doc.version );
+		doc.applyOperation( removeOperation );
+
+		const reinsertOperation = removeOperation.getReversed();
+		doc.applyOperation( reinsertOperation );
+
+		expect( changes ).to.have.length( 2 );
+
+		expect( types[ 0 ] ).to.equal( 'remove' );
+		expect( changes[ 0 ].range ).to.deep.equal( Range.createFromParentsAndOffsets( graveyard, 0, graveyard, 3 ) );
+		expect( changes[ 0 ].sourcePosition ).to.deep.equal( Position.createFromParentAndOffset( root, 0 ) );
+
+		expect( types[ 1 ] ).to.equal( 'reinsert' );
+		expect( changes[ 1 ].range ).to.deep.equal( Range.createFromParentsAndOffsets( root, 0, root, 3 ) );
+		expect( changes[ 1 ].sourcePosition ).to.deep.equal( Position.createFromParentAndOffset( graveyard, 0 ) );
+	} );
+
+	it( 'should be fired when attribute is inserted', () => {
+		root.insertChildren( 0, 'foo' );
+
+		doc.applyOperation(
+			new ChangeOperation(
+				Range.createFromParentsAndOffsets( root, 0, root, 3 ),
+				null,
+				new Attribute( 'key', 'new' ),
+				doc.version
+			)
+		);
+
+		expect( changes ).to.have.length( 1 );
+		expect( types[ 0 ] ).to.equal( 'attr' );
+		expect( changes[ 0 ].range ).to.deep.equal( Range.createFromParentsAndOffsets( root, 0, root, 3 ) );
+		expect( changes[ 0 ].oldAttr ).to.be.null;
+		expect( changes[ 0 ].newAttr ).to.deep.equal( new Attribute( 'key', 'new' ) );
+	} );
+
+	it( 'should be fired when attribute is removed', () => {
+		const elem = new Element( 'p', [ new Attribute( 'key', 'old' ) ] );
+		root.insertChildren( 0, elem );
+
+		doc.applyOperation(
+			new ChangeOperation(
+				Range.createFromParentsAndOffsets( root, 0, elem, 0 ),
+				new Attribute( 'key', 'old' ),
+				null,
+				doc.version
+			)
+		);
+
+		expect( changes ).to.have.length( 1 );
+		expect( types[ 0 ] ).to.equal( 'attr' );
+		expect( changes[ 0 ].range ).to.deep.equal( Range.createFromParentsAndOffsets( root, 0, elem, 0 ) );
+		expect( changes[ 0 ].oldAttr ).to.deep.equal( new Attribute( 'key', 'old' ) );
+		expect( changes[ 0 ].newAttr ).to.be.null;
+	}  );
+
+	it( 'should be fired when attribute changes', () => {
+		const elem = new Element( 'p', [ new Attribute( 'key', 'old' ) ] );
+		root.insertChildren( 0, elem );
+
+		doc.applyOperation(
+			new ChangeOperation(
+				Range.createFromParentsAndOffsets( root, 0, elem, 0 ),
+				new Attribute( 'key', 'old' ),
+				new Attribute( 'key', 'new' ),
+				doc.version
+			)
+		);
+
+		expect( changes ).to.have.length( 1 );
+		expect( types[ 0 ] ).to.equal( 'attr' );
+		expect( changes[ 0 ].range ).to.deep.equal( Range.createFromParentsAndOffsets( root, 0, elem, 0 ) );
+		expect( changes[ 0 ].oldAttr ).to.deep.equal( new Attribute( 'key', 'old' ) );
+		expect( changes[ 0 ].newAttr ).to.deep.equal( new Attribute( 'key', 'new' ) );
+	}  );
+} );

+ 12 - 9
packages/ckeditor5-utils/tests/document/document.js → packages/ckeditor5-utils/tests/document/document/document.js

@@ -77,30 +77,33 @@ describe( 'Document', () => {
 	} );
 
 	describe( 'applyOperation', () => {
-		it( 'should increase document version, execute operation and fire operationApplied', () => {
-			let operationApplied = sinon.spy();
+		it( 'should increase document version, execute operation and fire event with proper data', () => {
+			const changeCallback = sinon.spy();
+			const type = 't';
+			const data = { data: 'x' };
+
 			let operation = {
+				type: type,
 				baseVersion: 0,
-				_execute: sinon.spy()
+				_execute: sinon.stub().returns( data )
 			};
 
-			document.on( 'operationApplied', operationApplied );
-
+			document.on( 'change', changeCallback );
 			document.applyOperation( operation );
 
 			expect( document.version ).to.equal( 1 );
-			sinon.assert.calledOnce( operationApplied );
 			sinon.assert.calledOnce( operation._execute );
+
+			sinon.assert.calledOnce( changeCallback );
+			expect( changeCallback.args[ 0 ][ 1 ] ).to.equal( type );
+			expect( changeCallback.args[ 0 ][ 2 ] ).to.equal( data );
 		} );
 
 		it( 'should throw an error on the operation base version and the document version is different', () => {
-			let operationApplied = sinon.spy();
 			let operation = {
 				baseVersion: 1
 			};
 
-			document.on( 'operationApplied', operationApplied );
-
 			expect(
 				() => {
 					document.applyOperation( operation );

+ 11 - 0
packages/ckeditor5-utils/tests/document/operation/changeoperation.js

@@ -44,6 +44,17 @@ describe( 'ChangeOperation', () => {
 		root = doc.createRoot( 'root' );
 	} );
 
+	it( 'should have proper type', () => {
+		const opp = new ChangeOperation(
+			new Range( new Position( [ 0 ], root ), new Position( [ 2 ], root ) ),
+			null,
+			new Attribute( 'isNew', true ),
+			doc.version
+		);
+
+		expect( opp.type ).to.equal( 'attr' );
+	} );
+
 	it( 'should insert attribute to the set of nodes', () => {
 		let newAttr = new Attribute( 'isNew', true );
 

+ 10 - 0
packages/ckeditor5-utils/tests/document/operation/insertoperation.js

@@ -39,6 +39,16 @@ describe( 'InsertOperation', () => {
 		root = doc.createRoot( 'root' );
 	} );
 
+	it( 'should have proper type', () => {
+		const opp = new InsertOperation(
+			new Position( [ 0 ], root ),
+			new Character( 'x' ),
+			doc.version
+		);
+
+		expect( opp.type ).to.equal( 'insert' );
+	} );
+
 	it( 'should insert node', () => {
 		doc.applyOperation(
 			new InsertOperation(

+ 11 - 0
packages/ckeditor5-utils/tests/document/operation/moveoperation.js

@@ -36,6 +36,17 @@ describe( 'MoveOperation', () => {
 		root = doc.createRoot( 'root' );
 	} );
 
+	it( 'should have proper type', () => {
+		const opp = new MoveOperation(
+			new Position( [ 0, 0 ], root ),
+			new Position( [ 1, 0 ], root ),
+			1,
+			doc.version
+		);
+
+		expect( opp.type ).to.equal( 'move' );
+	} );
+
 	it( 'should move from one node to another', () => {
 		let p1 = new Element( 'p1', [], new Element( 'x' ) );
 		let p2 = new Element( 'p2' );

+ 4 - 0
packages/ckeditor5-utils/tests/document/operation/reinsertoperation.js

@@ -44,6 +44,10 @@ describe( 'ReinsertOperation', () => {
 		);
 	} );
 
+	it( 'should have proper type', () => {
+		expect( operation.type ).to.equal( 'reinsert' );
+	} );
+
 	it( 'should extend MoveOperation class', () => {
 		expect( operation ).to.be.instanceof( MoveOperation );
 	} );

+ 10 - 0
packages/ckeditor5-utils/tests/document/operation/removeoperation.js

@@ -34,6 +34,16 @@ describe( 'RemoveOperation', () => {
 		graveyard = doc._graveyard;
 	} );
 
+	it( 'should have proper type', () => {
+		const opp = new RemoveOperation(
+			new Position( [ 2 ], root ),
+			2,
+			doc.version
+		);
+
+		expect( opp.type ).to.equal( 'remove' );
+	} );
+
 	it( 'should extend MoveOperation class', () => {
 		let operation = new RemoveOperation(
 			new Position( [ 2 ], root ),

+ 4 - 4
packages/ckeditor5-utils/tests/document/position.js

@@ -81,7 +81,7 @@ describe( 'position', () => {
 		} ).to.throw( CKEditorError, /position-root-not-rootelement/ );
 	} );
 
-	it( 'should make positions form node and offset', () => {
+	it( 'should create positions form node and offset', () => {
 		expect( Position.createFromParentAndOffset( root, 0 ) ).to.have.property( 'path' ).that.deep.equals( [ 0 ] );
 		expect( Position.createFromParentAndOffset( root, 1 ) ).to.have.property( 'path' ).that.deep.equals( [ 1 ] );
 		expect( Position.createFromParentAndOffset( root, 2 ) ).to.have.property( 'path' ).that.deep.equals( [ 2 ] );
@@ -98,7 +98,7 @@ describe( 'position', () => {
 		expect( Position.createFromParentAndOffset( li1, 3 ) ).to.have.property( 'path' ).that.deep.equals( [ 1, 0, 3 ] );
 	} );
 
-	it( 'should make positions before elements', () => {
+	it( 'should create positions before elements', () => {
 		expect( Position.createBefore( p ) ).to.have.property( 'path' ).that.deep.equals( [ 0 ] );
 
 		expect( Position.createBefore( ul ) ).to.have.property( 'path' ).that.deep.equals( [ 1 ] );
@@ -116,13 +116,13 @@ describe( 'position', () => {
 		expect( Position.createBefore( r ) ).to.have.property( 'path' ).that.deep.equals( [ 1, 1, 2 ] );
 	} );
 
-	it( 'should throw error if one try to make positions before root', () => {
+	it( 'should throw error if one try to create positions before root', () => {
 		expect( () => {
 			Position.createBefore( root );
 		} ).to.throw( CKEditorError, /position-before-root/ );
 	} );
 
-	it( 'should make positions after elements', () => {
+	it( 'should create positions after elements', () => {
 		expect( Position.createAfter( p ) ).to.have.property( 'path' ).that.deep.equals( [ 1 ] );
 
 		expect( Position.createAfter( ul ) ).to.have.property( 'path' ).that.deep.equals( [ 2 ] );