瀏覽代碼

Merge pull request #216 from ckeditor/t/215

Feature: Introduced skipping items when binding collections. Closes #215. Closes https://github.com/ckeditor/ckeditor5-ui/issues/92.
Piotr Jasiun 8 年之前
父節點
當前提交
268479e311
共有 2 個文件被更改,包括 267 次插入9 次删除
  1. 108 6
      packages/ckeditor5-utils/src/collection.js
  2. 159 3
      packages/ckeditor5-utils/tests/collection.js

+ 108 - 6
packages/ckeditor5-utils/src/collection.js

@@ -81,6 +81,14 @@ export default class Collection {
 		this._bindToInternalToExternalMap = new WeakMap();
 
 		/**
+		 * Stores indexes of skipped items from bound external collection.
+		 *
+		 * @private
+		 * @member {Array}
+		 */
+		this._skippedIndexesFromExternal = [];
+
+		/**
 		 * A collection instance this collection is bound to as a result
 		 * of calling {@link #bindTo} method.
 		 *
@@ -271,7 +279,7 @@ export default class Collection {
 		this._bindToInternalToExternalMap.delete( item );
 		this._bindToExternalToInternalMap.delete( externalItem );
 
-		this.fire( 'remove', item );
+		this.fire( 'remove', item, index );
 
 		return item;
 	}
@@ -402,9 +410,28 @@ export default class Collection {
 	 *		console.log( target.get( 0 ).value ); // 'foo'
 	 *		console.log( target.get( 1 ).value ); // 'bar'
 	 *
+	 * It's possible to skip specified items by returning falsy value:
+	 *
+	 *		const source = new Collection();
+	 *		const target = new Collection();
+	 *
+	 *		target.bindTo( source ).using( item => {
+	 *			if ( item.hidden ) {
+	 *				return null;
+	 *			}
+	 *
+	 *			return item;
+	 *		} );
+	 *
+	 *		source.add( { hidden: true } );
+	 *		source.add( { hidden: false } );
+	 *
+	 *		console.log( source.length ); // 2
+	 *		console.log( target.length ); // 1
+	 *
 	 * **Note**: {@link #clear} can be used to break the binding.
 	 *
-	 * @param {module:utils/collection~Collection} collection A collection to be bound.
+	 * @param {module:utils/collection~Collection} externalCollection A collection to be bound.
 	 * @returns {Object}
 	 * @returns {module:utils/collection~Collection#bindTo#as} return.as
 	 * @returns {module:utils/collection~Collection#bindTo#using} return.using
@@ -476,28 +503,102 @@ export default class Collection {
 			} else {
 				const item = factory( externalItem );
 
+				// When there is no item we need to remember skipped index first and then we can skip this item.
+				if ( !item ) {
+					this._skippedIndexesFromExternal.push( index );
+
+					return;
+				}
+
+				// Lets try to put item at the same index as index in external collection
+				// but when there are a skipped items in one or both collections we need to recalculate this index.
+				let finalIndex = index;
+
+				// When we try to insert item after some skipped items from external collection we need
+				// to include this skipped items and decrease index.
+				//
+				// For the following example:
+				// external -> [ 'A', 'B - skipped for internal', 'C - skipped for internal' ]
+				// internal -> [ A ]
+				//
+				// Another item is been added at the end of external collection:
+				// external.add( 'D' )
+				// external -> [ 'A', 'B - skipped for internal', 'C - skipped for internal', 'D' ]
+				//
+				// We can't just add 'D' to internal at the same index as index in external because
+				// this will produce empty indexes what is invalid:
+				// internal -> [ 'A', empty, empty, 'D' ]
+				//
+				// So we need to include skipped items and decrease index
+				// internal -> [ 'A', 'D' ]
+				for ( const skipped of this._skippedIndexesFromExternal ) {
+					if ( index > skipped ) {
+						finalIndex--;
+					}
+				}
+
+				// We need to take into consideration that external collection could skip some items from
+				// internal collection.
+				//
+				// For the following example:
+				// internal -> [ 'A', 'B - skipped for external', 'C - skipped for external' ]
+				// external -> [ A ]
+				//
+				// Another item is been added at the end of external collection:
+				// external.add( 'D' )
+				// external -> [ 'A', 'D' ]
+				//
+				// We need to include skipped items and place new item after them:
+				// internal -> [ 'A', 'B - skipped for external', 'C - skipped for external', 'D' ]
+				for ( const skipped of externalCollection._skippedIndexesFromExternal ) {
+					if ( finalIndex >= skipped ) {
+						finalIndex++;
+					}
+				}
+
 				this._bindToExternalToInternalMap.set( externalItem, item );
 				this._bindToInternalToExternalMap.set( item, externalItem );
-
-				this.add( item, index );
+				this.add( item, finalIndex );
+
+				// After adding new element to internal collection we need update indexes
+				// of skipped items in external collection.
+				for ( let i = 0; i < externalCollection._skippedIndexesFromExternal.length; i++ ) {
+					if ( finalIndex <= externalCollection._skippedIndexesFromExternal[ i ] ) {
+						externalCollection._skippedIndexesFromExternal[ i ]++;
+					}
+				}
 			}
 		};
 
 		// Load the initial content of the collection.
 		for ( const externalItem of externalCollection ) {
-			addItem( null, externalItem );
+			addItem( null, externalItem, externalCollection.getIndex( externalItem ) );
 		}
 
 		// Synchronize the with collection as new items are added.
 		this.listenTo( externalCollection, 'add', addItem );
 
 		// Synchronize the with collection as new items are removed.
-		this.listenTo( externalCollection, 'remove', ( evt, externalItem ) => {
+		this.listenTo( externalCollection, 'remove', ( evt, externalItem, index ) => {
 			const item = this._bindToExternalToInternalMap.get( externalItem );
 
 			if ( item ) {
 				this.remove( item );
 			}
+
+			// After removing element from external collection we need update/remove indexes
+			// of skipped items in internal collection.
+			this._skippedIndexesFromExternal = this._skippedIndexesFromExternal.reduce( ( result, skipped ) => {
+				if ( index < skipped ) {
+					result.push( skipped - 1 );
+				}
+
+				if ( index > skipped ) {
+					result.push( skipped );
+				}
+
+				return result;
+			}, [] );
 		} );
 	}
 
@@ -520,6 +621,7 @@ export default class Collection {
 	 *
 	 * @event remove
 	 * @param {Object} item The removed item.
+	 * @param {Number} index Index from which item was removed.
 	 */
 }
 

+ 159 - 3
packages/ckeditor5-utils/tests/collection.js

@@ -448,9 +448,9 @@ describe( 'Collection', () => {
 			collection.remove( 'bom' );	// by id
 
 			sinon.assert.calledThrice( spy );
-			sinon.assert.calledWithExactly( spy, sinon.match.has( 'source', collection ), item1 );
-			sinon.assert.calledWithExactly( spy, sinon.match.has( 'source', collection ), item2 );
-			sinon.assert.calledWithExactly( spy, sinon.match.has( 'source', collection ), item3 );
+			sinon.assert.calledWithExactly( spy, sinon.match.has( 'source', collection ), item1, 0 );
+			sinon.assert.calledWithExactly( spy, sinon.match.has( 'source', collection ), item2, 1 );
+			sinon.assert.calledWithExactly( spy, sinon.match.has( 'source', collection ), item3, 0 );
 		} );
 
 		it( 'should throw an error on invalid index', () => {
@@ -767,6 +767,34 @@ describe( 'Collection', () => {
 					expect( collection.get( 0 ).value ).to.equal( 'foo' );
 					expect( collection.get( 1 ).value ).to.equal( 'bar' );
 				} );
+
+				it( 'skips when there is no item', () => {
+					// Add before collection is bound.
+					items.add( { value: 1, skip: true } );
+
+					expect( collection ).to.have.length( 0 );
+
+					collection.bindTo( items ).using( item => {
+						if ( item.skip ) {
+							return null;
+						}
+
+						return item;
+					} );
+
+					// Still 0 because initial item was skipped.
+					expect( collection ).to.have.length( 0 );
+
+					items.add( { value: 2, skip: false } );
+					items.add( { value: 3, skip: true } );
+					items.add( { value: 4, skip: false } );
+
+					expect( Array.from( collection, item => item.value ) ).to.deep.equal( [ 2, 4 ] );
+
+					items.add( { value: 5, skip: false }, 2 );
+
+					expect( Array.from( collection, item => item.value ) ).to.deep.equal( [ 2, 5, 4 ] );
+				} );
 			} );
 
 			describe( 'property name', () => {
@@ -802,6 +830,25 @@ describe( 'Collection', () => {
 					items.remove( 0 );
 					expect( collection ).to.have.length( 0 );
 				} );
+
+				it( 'skips when there is no item', () => {
+					items.add( { prop: null } );
+
+					collection.bindTo( items ).using( 'prop' );
+
+					// Still 0 because initial item was skipped.
+					expect( collection ).to.have.length( 0 );
+
+					items.add( { prop: { value: 2, skip: false } } );
+					items.add( { prop: null } );
+					items.add( { prop: { value: 4, skip: false } } );
+
+					expect( Array.from( collection, item => item.value ) ).to.deep.equal( [ 2, 4 ] );
+
+					items.add( { prop: { value: 5 } }, 2 );
+
+					expect( Array.from( collection, item => item.value ) ).to.deep.equal( [ 2, 5, 4 ] );
+				} );
 			} );
 		} );
 
@@ -1032,6 +1079,115 @@ describe( 'Collection', () => {
 				sinon.assert.callCount( spyRemoveA, 2 );
 				sinon.assert.callCount( spyRemoveB, 2 );
 			} );
+
+			describe( 'skipping items', () => {
+				let collectionA, collectionB;
+
+				beforeEach( () => {
+					collectionA = new Collection();
+					collectionB = new Collection();
+
+					// A<--->B
+					collectionA.bindTo( collectionB ).using( item => {
+						if ( item.skip ) {
+							return null;
+						}
+
+						return item;
+					} );
+
+					collectionB.bindTo( collectionA ).using( item => {
+						if ( item.skip ) {
+							return null;
+						}
+
+						return item;
+					} );
+				} );
+
+				it( 'should add items at the enf of collections when includes skipped items', () => {
+					collectionA.add( { v: 'A' } );
+					collectionA.add( { v: 'B', skip: true } );
+					collectionA.add( { v: 'C', skip: true } );
+					collectionA.add( { v: 'D' } );
+
+					expect( collectionA._skippedIndexesFromExternal ).to.have.members( [] );
+					expect( collectionB._skippedIndexesFromExternal ).to.have.members( [ 1, 2 ] );
+					assertItems( collectionA, [ 'A', 'B', 'C', 'D' ] );
+					assertItems( collectionB, [ 'A', 'D' ] );
+
+					collectionB.add( { v: 'E' } );
+					collectionB.add( { v: 'F', skip: true } );
+					collectionB.add( { v: 'G' } );
+
+					expect( collectionA._skippedIndexesFromExternal ).to.have.members( [ 3 ] );
+					expect( collectionB._skippedIndexesFromExternal ).to.have.members( [ 1, 2 ] );
+					assertItems( collectionA, [ 'A', 'B', 'C', 'D', 'E', 'G' ] );
+					assertItems( collectionB, [ 'A', 'D', 'E', 'F', 'G' ] );
+				} );
+
+				it( 'should add items between skipped items', () => {
+					collectionA.add( { v: 'A' } );
+					collectionA.add( { v: 'B', skip: true } );
+					collectionA.add( { v: 'C', skip: true } );
+					collectionA.add( { v: 'D' } );
+
+					collectionB.add( { v: 'E' } );
+					collectionB.add( { v: 'F', skip: true } );
+					collectionB.add( { v: 'G', skip: true } );
+					collectionB.add( { v: 'H' } );
+
+					expect( collectionA._skippedIndexesFromExternal ).to.have.members( [ 3, 4 ] );
+					expect( collectionB._skippedIndexesFromExternal ).to.have.members( [ 1, 2 ] );
+					assertItems( collectionA, [ 'A', 'B', 'C', 'D', 'E', 'H' ] );
+					assertItems( collectionB, [ 'A', 'D', 'E', 'F', 'G', 'H' ] );
+
+					collectionA.add( { v: 'I' }, 2 );
+
+					expect( collectionA._skippedIndexesFromExternal ).to.have.members( [ 4, 5 ] );
+					expect( collectionB._skippedIndexesFromExternal ).to.have.members( [ 1, 2 ] );
+					assertItems( collectionA, [ 'A', 'B', 'I', 'C', 'D', 'E', 'H' ] );
+					assertItems( collectionB, [ 'A', 'I', 'D', 'E', 'F', 'G', 'H' ] );
+
+					collectionB.add( { v: 'J' }, 5 );
+
+					expect( collectionA._skippedIndexesFromExternal ).to.have.members( [ 4, 5 ] );
+					expect( collectionB._skippedIndexesFromExternal ).to.have.members( [ 1, 2 ] );
+					assertItems( collectionA, [ 'A', 'B', 'I', 'C', 'D', 'E', 'J', 'H' ] );
+					assertItems( collectionB, [ 'A', 'I', 'D', 'E', 'F', 'J', 'G', 'H' ] );
+				} );
+
+				it( 'should properly remove skipped items and update skipped indexes', () => {
+					collectionA.add( { v: 'A' } );
+					collectionA.add( { v: 'B', skip: true } );
+					collectionA.add( { v: 'C', skip: true } );
+					collectionA.add( { v: 'D' } );
+
+					collectionB.add( { v: 'E' } );
+					collectionB.add( { v: 'F', skip: true } );
+					collectionB.add( { v: 'G', skip: true } );
+					collectionB.add( { v: 'H' } );
+
+					expect( collectionA._skippedIndexesFromExternal ).to.have.members( [ 3, 4 ] );
+					expect( collectionB._skippedIndexesFromExternal ).to.have.members( [ 1, 2 ] );
+					assertItems( collectionA, [ 'A', 'B', 'C', 'D', 'E', 'H' ] );
+					assertItems( collectionB, [ 'A', 'D', 'E', 'F', 'G', 'H' ] );
+
+					collectionA.remove( 2 );
+
+					expect( collectionA._skippedIndexesFromExternal ).to.have.members( [ 3, 4 ] );
+					expect( collectionB._skippedIndexesFromExternal ).to.have.members( [ 1 ] );
+					assertItems( collectionA, [ 'A', 'B', 'D', 'E', 'H' ] );
+					assertItems( collectionB, [ 'A', 'D', 'E', 'F', 'G', 'H' ] );
+
+					collectionB.remove( 3 );
+
+					expect( collectionA._skippedIndexesFromExternal ).to.have.members( [ 3 ] );
+					expect( collectionB._skippedIndexesFromExternal ).to.have.members( [ 1 ] );
+					assertItems( collectionA, [ 'A', 'B', 'D', 'E', 'H' ] );
+					assertItems( collectionB, [ 'A', 'D', 'E', 'G', 'H' ] );
+				} );
+			} );
 		} );
 	} );