8
0
Quellcode durchsuchen

Update differ code and tests for coverage.

Maciej Gołaszewski vor 5 Jahren
Ursprung
Commit
11eb06def8

+ 22 - 21
packages/ckeditor5-engine/src/model/differ.js

@@ -600,18 +600,22 @@ export default class Differ {
 		this._removeAllNestedChanges( parent, offset, howMany );
 	}
 
+	/**
+	 * Saves and handles a remove change.
+	 *
+	 * @private
+	 * @param {module:engine/model/element~Element} parent
+	 * @param {Number} offset
+	 * @param {Number} howMany
+	 */
 	_markRefresh( parent, offset, howMany ) {
 		const changeItem = { type: 'refresh', offset, howMany, count: this._changeCount++ };
 
 		this._markChange( parent, changeItem );
-
-		// Needed to remove "attribute" change or other.
-		// @todo: might need to retain "slot" changes.
-		this._removeAllNestedAttributeChanges( parent, offset, howMany );
 	}
 
 	/**
-	 * Saves and handles an attribute change.
+	 * Saves and handles a refresh change.
 	 *
 	 * @private
 	 * @param {module:engine/model/item~Item} item
@@ -908,16 +912,17 @@ export default class Differ {
 				}
 			}
 
-			if ( inc.type == 'refresh' ) {
-				if ( old.type == 'insert' ) {
+			if ( inc.type === 'refresh' ) {
+				// TOOD: This might be handled on other level.
+				if ( old.type === 'insert' ) {
 					if ( inc.offset === old.offset && inc.howMany === old.howMany ) {
 						old.howMany = 0;
 					}
 				}
 
-				if ( old.type == 'remove' ) {
+				if ( old.type === 'attribute' ) {
 					if ( inc.offset === old.offset && inc.howMany === old.howMany ) {
-						inc.nodesToHandle = 0;
+						old.howMany = 0;
 					}
 				}
 			}
@@ -1061,7 +1066,13 @@ export default class Differ {
 		return this._isInInsertedElement( parent );
 	}
 
-	// TODO: copy-paste of above _isInInsertedElement.
+	/**
+	 * Checks whether given element or any of its parents is an element that is buffered as a refreshed element.
+	 *
+	 * @private
+	 * @param {module:engine/model/element~Element} element Element to check.
+	 * @returns {Boolean}
+	 */
 	_isInRefreshedElement( element ) {
 		const parent = element.parent;
 
@@ -1074,7 +1085,7 @@ export default class Differ {
 
 		if ( changes ) {
 			for ( const change of changes ) {
-				if ( change.type == 'refresh' && offset >= change.offset && offset < change.offset + change.howMany ) {
+				if ( change.type === 'refresh' && offset >= change.offset && offset < change.offset + change.howMany ) {
 					return true;
 				}
 			}
@@ -1104,16 +1115,6 @@ export default class Differ {
 			}
 		}
 	}
-
-	_removeAllNestedAttributeChanges( parent, offset, howMany ) {
-		const parentChanges = this._changesInElement.get( parent );
-
-		const notAttributeChange = change => change.type !== 'attribute' || change.offset !== offset || change.howMany !== howMany;
-
-		if ( parentChanges ) {
-			this._changesInElement.set( parent, parentChanges.filter( notAttributeChange ) );
-		}
-	}
 }
 
 // Returns an array that is a copy of passed child list with the exception that text nodes are split to one or more

+ 70 - 12
packages/ckeditor5-engine/tests/model/differ.js

@@ -1803,7 +1803,7 @@ describe( 'Differ', () => {
 			] );
 		} );
 
-		it( 'an element (block)', () => {
+		it( 'a refreshed element (block)', () => {
 			const p = root.getChild( 0 );
 
 			differ.refreshItem( p );
@@ -1813,7 +1813,7 @@ describe( 'Differ', () => {
 			], true );
 		} );
 
-		it( 'an element (complex)', () => {
+		it( 'a refreshed element (complex)', () => {
 			const complex = root.getChild( 2 );
 
 			differ.refreshItem( complex );
@@ -1823,7 +1823,7 @@ describe( 'Differ', () => {
 			], true );
 		} );
 
-		it( 'an element with child removed (refresh + remove)', () => {
+		it( 'a refreshed element with a child removed (refresh + remove)', () => {
 			const complex = root.getChild( 2 );
 
 			model.change( () => {
@@ -1836,7 +1836,7 @@ describe( 'Differ', () => {
 			} );
 		} );
 
-		it( 'an element with child removed (remove + refresh)', () => {
+		it( 'a refreshed element with a child removed (remove + refresh)', () => {
 			const complex = root.getChild( 2 );
 
 			model.change( () => {
@@ -1850,7 +1850,7 @@ describe( 'Differ', () => {
 			} );
 		} );
 
-		it( 'an element with child added (refresh + add)', () => {
+		it( 'a refreshed element with a child added (refresh + insert)', () => {
 			const complex = root.getChild( 2 );
 
 			model.change( () => {
@@ -1863,20 +1863,22 @@ describe( 'Differ', () => {
 			} );
 		} );
 
-		it( 'an element with child added (add + refresh)', () => {
+		it( 'a refreshed element with a child added (insert + refresh)', () => {
 			const complex = root.getChild( 2 );
 
 			model.change( () => {
+				const slot = new Element( 'slot' );
+				insert( slot, model.createPositionAt( complex, 2 ) );
 				differ.refreshItem( complex );
-				insert( new Element( 'slot' ), model.createPositionAt( complex, 2 ) );
 
 				expectChanges( [
-					{ type: 'refresh', name: 'complex', length: 1, position: model.createPositionBefore( complex ) }
+					{ type: 'refresh', name: 'complex', length: 1, position: model.createPositionBefore( complex ) },
+					{ type: 'insert', name: 'slot', length: 1, position: model.createPositionBefore( slot ) }
 				], true );
 			} );
 		} );
 
-		it( 'an element with attribute set (refresh + attribute)', () => {
+		it( 'a refreshed element with attribute set (refresh + attribute)', () => {
 			const complex = root.getChild( 2 );
 
 			model.change( () => {
@@ -1889,12 +1891,12 @@ describe( 'Differ', () => {
 			} );
 		} );
 
-		it( 'an element with attribute set (attribute + refresh)', () => {
+		it( 'a refreshed element with attribute set (attribute + refresh)', () => {
 			const complex = root.getChild( 2 );
 
 			model.change( () => {
-				differ.refreshItem( complex );
 				attribute( model.createRangeOn( complex ), 'foo', undefined, true );
+				differ.refreshItem( complex );
 
 				expectChanges( [
 					{ type: 'refresh', name: 'complex', length: 1, position: model.createPositionBefore( complex ) }
@@ -1902,6 +1904,62 @@ describe( 'Differ', () => {
 			} );
 		} );
 
+		it( 'an element added and refreshed', () => {
+			const complex = root.getChild( 2 );
+
+			model.change( () => {
+				const slot = new Element( 'slot' );
+				insert( slot, model.createPositionAt( complex, 2 ) );
+				differ.refreshItem( slot );
+
+				expectChanges( [
+					{ type: 'refresh', name: 'slot', length: 1, position: model.createPositionBefore( slot ) }
+				], true );
+			} );
+		} );
+
+		it( 'an element added and other refreshed', () => {
+			const complex = root.getChild( 2 );
+
+			model.change( () => {
+				const slot = new Element( 'slot' );
+				insert( slot, model.createPositionAt( complex, 2 ) );
+				differ.refreshItem( complex.getChild( 0 ) );
+
+				expectChanges( [
+					{ type: 'refresh', name: 'slot', length: 1, position: model.createPositionAt( complex, 0 ) },
+					{ type: 'insert', name: 'slot', length: 1, position: model.createPositionAt( complex, 2 ) }
+				], true );
+			} );
+		} );
+
+		it( 'an element refreshed and removed', () => {
+			const complex = root.getChild( 2 );
+
+			model.change( () => {
+				const slot = complex.getChild( 1 );
+				remove( model.createPositionAt( complex, 1 ), 1 );
+				differ.refreshItem( slot );
+
+				expectChanges( [
+					{ type: 'remove', name: 'slot', length: 1, position: model.createPositionAt( complex, 1 ) }
+				] );
+			} );
+		} );
+
+		it( 'inside a new element', () => {
+			// Since the refreshed element is inside a new element, it should not be listed on changes list.
+			model.change( () => {
+				insert( new Element( 'blockQuote', null, new Element( 'paragraph' ) ), new Position( root, [ 2 ] ) );
+
+				differ.refreshItem( root.getChild( 2 ).getChild( 0 ) );
+
+				expectChanges( [
+					{ type: 'insert', name: 'blockQuote', length: 1, position: new Position( root, [ 2 ] ) }
+				] );
+			} );
+		} );
+
 		it( 'markers refreshing', () => {
 			model.change( () => {
 				// Refreshed element contains marker.
@@ -2105,7 +2163,7 @@ describe( 'Differ', () => {
 	function expectChanges( expected, includeChangesInGraveyard = false ) {
 		const changes = differ.getChanges( { includeChangesInGraveyard } );
 
-		// expect( changes.length ).to.equal( expected.length );
+		expect( changes.length ).to.equal( expected.length );
 
 		for ( let i = 0; i < expected.length; i++ ) {
 			for ( const key in expected[ i ] ) {