Przeglądaj źródła

Merge pull request #7644 from ckeditor/i/7627

Feature: Introduced the `Collection.addMany()` method for adding multiple items in a single call. Closes #7627.

Feature: Introduced the `Collection.change` event. See #7627.
Maciej 5 lat temu
rodzic
commit
a1f0efd3c0

+ 5 - 15
packages/ckeditor5-font/src/documentcolorcollection.js

@@ -29,6 +29,10 @@ export default class DocumentColorCollection extends Collection {
 		 * @member {Boolean} #isEmpty
 		 */
 		this.set( 'isEmpty', true );
+
+		this.on( 'change', () => {
+			this.set( 'isEmpty', this.length === 0 );
+		} );
 	}
 
 	/**
@@ -44,6 +48,7 @@ export default class DocumentColorCollection extends Collection {
 	 * @param {Number} [index] The position of the item in the collection. The item
 	 * is pushed to the collection when `index` is not specified.
 	 * @fires add
+	 * @fires change
 	 */
 	add( item, index ) {
 		if ( this.find( element => element.color === item.color ) ) {
@@ -52,21 +57,6 @@ export default class DocumentColorCollection extends Collection {
 		}
 
 		super.add( item, index );
-
-		this.set( 'isEmpty', false );
-	}
-
-	/**
-	 * @inheritdoc
-	 */
-	remove( subject ) {
-		const ret = super.remove( subject );
-
-		if ( this.length === 0 ) {
-			this.set( 'isEmpty', true );
-		}
-
-		return ret;
 	}
 
 	/**

+ 116 - 49
packages/ckeditor5-utils/src/collection.js

@@ -179,11 +179,24 @@ export default class Collection {
 	 * @param {Number} [index] The position of the item in the collection. The item
 	 * is pushed to the collection when `index` not specified.
 	 * @fires add
+	 * @fires change
 	 */
 	add( item, index ) {
-		const itemId = this._getItemIdBeforeAdding( item );
+		return this.addMany( [ item ], index );
+	}
 
-		// TODO: Use ES6 default function argument.
+	/**
+	 * Adds multiple items into the collection.
+	 *
+	 * Any item not containing an id will get an automatically generated one.
+	 *
+	 * @chainable
+	 * @param {Iterable.<Object>} item
+	 * @param {Number} [index] The position of the insertion. Items will be appended if no `index` is specified.
+	 * @fires add
+	 * @fires change
+	 */
+	addMany( items, index ) {
 		if ( index === undefined ) {
 			index = this._items.length;
 		} else if ( index > this._items.length || index < 0 ) {
@@ -195,11 +208,22 @@ export default class Collection {
 			throw new CKEditorError( 'collection-add-item-invalid-index', this );
 		}
 
-		this._items.splice( index, 0, item );
+		for ( let offset = 0; offset < items.length; offset++ ) {
+			const item = items[ offset ];
+			const itemId = this._getItemIdBeforeAdding( item );
+			const currentItemIndex = index + offset;
 
-		this._itemMap.set( itemId, item );
+			this._items.splice( currentItemIndex, 0, item );
+			this._itemMap.set( itemId, item );
 
-		this.fire( 'add', item, index );
+			this.fire( 'add', item, currentItemIndex );
+		}
+
+		this.fire( 'change', {
+			added: items,
+			removed: [],
+			index
+		} );
 
 		return this;
 	}
@@ -271,52 +295,16 @@ export default class Collection {
 	 * @param {Object|Number|String} subject The item to remove, its id or index in the collection.
 	 * @returns {Object} The removed item.
 	 * @fires remove
+	 * @fires change
 	 */
 	remove( subject ) {
-		let index, id, item;
-		let itemDoesNotExist = false;
-		const idProperty = this._idProperty;
-
-		if ( typeof subject == 'string' ) {
-			id = subject;
-			item = this._itemMap.get( id );
-			itemDoesNotExist = !item;
-
-			if ( item ) {
-				index = this._items.indexOf( item );
-			}
-		} else if ( typeof subject == 'number' ) {
-			index = subject;
-			item = this._items[ index ];
-			itemDoesNotExist = !item;
-
-			if ( item ) {
-				id = item[ idProperty ];
-			}
-		} else {
-			item = subject;
-			id = item[ idProperty ];
-			index = this._items.indexOf( item );
-			itemDoesNotExist = ( index == -1 || !this._itemMap.get( id ) );
-		}
+		const [ item, index ] = this._remove( subject );
 
-		if ( itemDoesNotExist ) {
-			/**
-			 * Item not found.
-			 *
-			 * @error collection-remove-404
-			 */
-			throw new CKEditorError( 'collection-remove-404: Item not found.', this );
-		}
-
-		this._items.splice( index, 1 );
-		this._itemMap.delete( id );
-
-		const externalItem = this._bindToInternalToExternalMap.get( item );
-		this._bindToInternalToExternalMap.delete( item );
-		this._bindToExternalToInternalMap.delete( externalItem );
-
-		this.fire( 'remove', item, index );
+		this.fire( 'change', {
+			added: [],
+			removed: [ item ],
+			index
+		} );
 
 		return item;
 	}
@@ -363,6 +351,9 @@ export default class Collection {
 	/**
 	 * Removes all items from the collection and destroys the binding created using
 	 * {@link #bindTo}.
+	 *
+	 * @fires remove
+	 * @fires change
 	 */
 	clear() {
 		if ( this._bindToCollection ) {
@@ -370,9 +361,17 @@ export default class Collection {
 			this._bindToCollection = null;
 		}
 
+		const removedItems = Array.from( this._items );
+
 		while ( this.length ) {
-			this.remove( 0 );
+			this._remove( 0 );
 		}
+
+		this.fire( 'change', {
+			added: [],
+			removed: removedItems,
+			index: 0
+		} );
 	}
 
 	/**
@@ -664,6 +663,65 @@ export default class Collection {
 		return itemId;
 	}
 
+	/**
+	 * Core {@link #remove} method implementation shared in other functions.
+	 *
+	 * In contrast this method **does not** fire the {@link #event:change} event.
+	 *
+	 * @private
+	 * @param {Object} subject The item to remove, its id or index in the collection.
+	 * @returns {Array} Returns an array with the removed item and its index.
+	 * @fires remove
+	 */
+	_remove( subject ) {
+		let index, id, item;
+		let itemDoesNotExist = false;
+		const idProperty = this._idProperty;
+
+		if ( typeof subject == 'string' ) {
+			id = subject;
+			item = this._itemMap.get( id );
+			itemDoesNotExist = !item;
+
+			if ( item ) {
+				index = this._items.indexOf( item );
+			}
+		} else if ( typeof subject == 'number' ) {
+			index = subject;
+			item = this._items[ index ];
+			itemDoesNotExist = !item;
+
+			if ( item ) {
+				id = item[ idProperty ];
+			}
+		} else {
+			item = subject;
+			id = item[ idProperty ];
+			index = this._items.indexOf( item );
+			itemDoesNotExist = ( index == -1 || !this._itemMap.get( id ) );
+		}
+
+		if ( itemDoesNotExist ) {
+			/**
+			 * Item not found.
+			 *
+			 * @error collection-remove-404
+			 */
+			throw new CKEditorError( 'collection-remove-404: Item not found.', this );
+		}
+
+		this._items.splice( index, 1 );
+		this._itemMap.delete( id );
+
+		const externalItem = this._bindToInternalToExternalMap.get( item );
+		this._bindToInternalToExternalMap.delete( item );
+		this._bindToExternalToInternalMap.delete( externalItem );
+
+		this.fire( 'remove', item, index );
+
+		return [ item, index ];
+	}
+
 	/**
 	 * Iterable interface.
 	 *
@@ -680,6 +738,15 @@ export default class Collection {
 	 * @param {Object} item The added item.
 	 */
 
+	/**
+	 * Fired when the collection was changed due to adding or removing items.
+	 *
+	 * @event change
+	 * @param {Iterable.<Object>} added A list of added items.
+	 * @param {Iterable.<Object>} removed A list of removed items.
+	 * @param {Number} index An index where the addition or removal occurred.
+	 */
+
 	/**
 	 * Fired when an item is removed from the collection.
 	 *

+ 306 - 0
packages/ckeditor5-utils/tests/collection.js

@@ -365,6 +365,277 @@ describe( 'Collection', () => {
 		} );
 	} );
 
+	describe( 'addMany()', () => {
+		it( 'should be chainable', () => {
+			expect( collection.addMany( [ {} ] ) ).to.equal( collection );
+		} );
+
+		it( 'should change the length', () => {
+			expect( collection ).to.have.length( 0 );
+
+			collection.addMany( [ {}, {} ] );
+			expect( collection ).to.have.length( 2 );
+
+			collection.addMany( [ {} ] );
+			expect( collection ).to.have.length( 3 );
+		} );
+
+		it( 'should enable get( index )', () => {
+			const item1 = {};
+			const item2 = {};
+
+			collection.addMany( [ item1, item2 ] );
+			expect( collection.get( 0 ) ).to.equal( item1 );
+			expect( collection.get( 1 ) ).to.equal( item2 );
+		} );
+
+		it( 'should enable get( id )', () => {
+			const item1 = getItem( 'foo' );
+			const item2 = getItem( 'bar' );
+
+			collection.addMany( [ item1, item2 ] );
+
+			expect( collection.get( 'foo' ) ).to.equal( item1 );
+			expect( collection.get( 'bar' ) ).to.equal( item2 );
+		} );
+
+		it( 'should enable get( id ) - custom id property', () => {
+			const collection = new Collection( { idProperty: 'name' } );
+			const item1 = getItem( 'foo', 'name' );
+			const item2 = getItem( 'bar', 'name' );
+
+			collection.add( item1 );
+			collection.add( item2 );
+
+			expect( collection.get( 'foo' ) ).to.equal( item1 );
+			expect( collection.get( 'bar' ) ).to.equal( item2 );
+		} );
+
+		it( 'should generate an id when not defined', () => {
+			const item = {};
+
+			collection.addMany( [ item ] );
+
+			expect( item.id ).to.be.a( 'string' );
+			expect( collection.get( item.id ) ).to.equal( item );
+		} );
+
+		it( 'should generate an id when not defined - custom id property', () => {
+			const collection = new Collection( { idProperty: 'name' } );
+			const item = {};
+
+			collection.addMany( [ item ] );
+
+			expect( item.name ).to.be.a( 'string' );
+			expect( collection.get( item.name ) ).to.equal( item );
+		} );
+
+		it( 'should not change an existing id of an item', () => {
+			const item = getItem( 'foo' );
+
+			collection.addMany( [ item ] );
+
+			expect( item.id ).to.equal( 'foo' );
+		} );
+
+		it( 'should throw when item with this id already exists - single call', () => {
+			const item1 = getItem( 'foo' );
+
+			expectToThrowCKEditorError( () => {
+				collection.addMany( [ item1, item1 ] );
+			}, /^collection-add-item-already-exists/ );
+		} );
+
+		it( 'should throw when item with this id already exists - multiple calls', () => {
+			const item1 = getItem( 'foo' );
+			const item2 = getItem( 'foo' );
+
+			collection.addMany( [ item1 ] );
+
+			expectToThrowCKEditorError( () => {
+				collection.addMany( [ item2 ] );
+			}, /^collection-add-item-already-exists/ );
+		} );
+
+		it( 'should throw when item\'s id is not a string', () => {
+			const item = { id: 1 };
+
+			expectToThrowCKEditorError( () => {
+				collection.addMany( [ item ] );
+			}, /^collection-add-invalid-id/ );
+		} );
+
+		it(
+			'should generate an id when not defined, which is globally unique ' +
+			'so it is possible to move items between collections and avoid id collisions',
+			() => {
+				const collectionA = new Collection();
+				const collectionB = new Collection();
+				const itemA = {};
+				const itemB = {};
+
+				collectionA.addMany( [ itemA ] );
+				collectionB.addMany( [ itemB ] );
+				collectionB.addMany( [ collectionA.remove( itemA ) ] );
+
+				expect( collectionA.length ).to.equal( 0 );
+				expect( collectionB.length ).to.equal( 2 );
+				expect( collectionB.get( 0 ) ).to.equal( itemB );
+				expect( collectionB.get( 1 ) ).to.equal( itemA );
+
+				expect( itemA.id ).to.not.equal( itemB.id );
+			}
+		);
+
+		it(
+			'should generate an id when not defined, which is globally unique ' +
+			'so it is possible to move items between collections and avoid id collisions ' +
+			'– custom id property',
+			() => {
+				const collectionA = new Collection( { idProperty: 'foo' } );
+				const collectionB = new Collection( { idProperty: 'foo' } );
+				const itemA = {};
+				const itemB = {};
+
+				collectionA.addMany( [ itemA ] );
+				collectionB.addMany( [ itemB ] );
+				collectionB.addMany( [ collectionA.remove( itemA ) ] );
+
+				expect( collectionA.length ).to.equal( 0 );
+				expect( collectionB.length ).to.equal( 2 );
+				expect( collectionB.get( 0 ) ).to.equal( itemB );
+				expect( collectionB.get( 1 ) ).to.equal( itemA );
+
+				expect( itemA.foo ).to.not.equal( itemB.foo );
+			}
+		);
+
+		it( 'should allow an item which is already in some other collection', () => {
+			const collectionA = new Collection();
+			const collectionB = new Collection();
+			const item = {};
+
+			collectionA.addMany( [ item ] );
+			collectionB.addMany( [ item ] );
+
+			expect( collectionA.length ).to.equal( 1 );
+			expect( collectionB.length ).to.equal( 1 );
+			expect( collectionA.get( item.id ) ).to.equal( collectionB.get( 0 ) );
+		} );
+
+		it( 'should fire the "add" event', () => {
+			const spy = sinon.spy();
+			const item = {};
+
+			collection.on( 'add', spy );
+
+			collection.addMany( [ item ] );
+
+			sinon.assert.calledWithExactly( spy, sinon.match.has( 'source', collection ), item, 0 );
+		} );
+
+		it( 'should fire the "add" event for each item', () => {
+			const spy = sinon.spy();
+			const items = [ {}, {} ];
+
+			collection.on( 'add', spy );
+
+			collection.addMany( items );
+
+			sinon.assert.calledWithExactly( spy, sinon.match.has( 'source', collection ), items[ 0 ], 0 );
+			sinon.assert.calledWithExactly( spy, sinon.match.has( 'source', collection ), items[ 1 ], 1 );
+
+			expect( spy.callCount ).to.equal( 2 );
+		} );
+
+		it( 'should fire the "add" event with the index argument', () => {
+			const spy = sinon.spy();
+
+			collection.addMany( [ {} ] );
+			collection.addMany( [ {} ] );
+
+			collection.on( 'add', spy );
+
+			const item = {};
+			collection.addMany( [ item ], 1 );
+
+			sinon.assert.calledWithExactly( spy, sinon.match.has( 'source', collection ), item, 1 );
+		} );
+
+		it( 'should fire the "change" event', () => {
+			const spy = sinon.spy();
+			const items = [ {}, {} ];
+
+			collection.on( 'change', spy );
+
+			collection.addMany( items );
+
+			sinon.assert.calledOnce( spy );
+			expect( spy.args[ 0 ][ 1 ] ).to.deep.eql( {
+				added: items,
+				removed: [],
+				index: 0
+			} );
+		} );
+
+		it( 'should fire the "change" event with the index argument', () => {
+			const spy = sinon.spy();
+			const firstBatch = [ {}, {} ];
+			const secondBatch = [ {}, {} ];
+
+			collection.addMany( firstBatch );
+
+			collection.on( 'change', spy );
+
+			collection.addMany( secondBatch, 1 );
+
+			expect( spy.callCount, 'call count' ).to.equal( 1 );
+			expect( spy.args[ 0 ][ 1 ] ).to.deep.eql( {
+				added: secondBatch,
+				removed: [],
+				index: 1
+			} );
+		} );
+
+		it( 'should support an optional index argument', () => {
+			const item1 = getItem( 'foo' );
+			const item2 = getItem( 'bar' );
+			const item3 = getItem( 'baz' );
+			const item4 = getItem( 'abc' );
+
+			collection.addMany( [ item1 ] );
+			collection.addMany( [ item2 ], 0 );
+			collection.addMany( [ item3 ], 1 );
+			collection.addMany( [ item4 ], 3 );
+
+			expect( collection.get( 0 ) ).to.equal( item2 );
+			expect( collection.get( 1 ) ).to.equal( item3 );
+			expect( collection.get( 2 ) ).to.equal( item1 );
+			expect( collection.get( 3 ) ).to.equal( item4 );
+		} );
+
+		it( 'should throw when index argument is invalid', () => {
+			const item1 = getItem( 'foo' );
+			const item2 = getItem( 'bar' );
+			const item3 = getItem( 'baz' );
+
+			collection.addMany( [ item1 ] );
+
+			expectToThrowCKEditorError( () => {
+				collection.addMany( [ item2 ], -1 );
+			}, /^collection-add-item-invalid-index/ );
+
+			expectToThrowCKEditorError( () => {
+				collection.addMany( [ item2 ], 2 );
+			}, /^collection-add-item-invalid-index/ );
+
+			collection.addMany( [ item2 ], 1 );
+			collection.addMany( [ item3 ], 0 );
+
+			expect( collection.length ).to.equal( 3 );
+		} );
+	} );
+
 	describe( 'get()', () => {
 		it( 'should return an item', () => {
 			const item = getItem( 'foo' );
@@ -546,6 +817,23 @@ describe( 'Collection', () => {
 			sinon.assert.calledWithExactly( spy, sinon.match.has( 'source', collection ), item3, 0 );
 		} );
 
+		it( 'should fire the "change" event', () => {
+			const item = getItem( 'foo' );
+			const spy = sinon.spy();
+
+			collection.add( item );
+			collection.on( 'change', spy );
+
+			collection.remove( item );
+
+			sinon.assert.calledOnce( spy );
+			expect( spy.args[ 0 ][ 1 ] ).to.deep.eql( {
+				added: [],
+				removed: [ item ],
+				index: 0
+			} );
+		} );
+
 		it( 'should throw an error on invalid index', () => {
 			collection.add( getItem( 'foo' ) );
 
@@ -656,6 +944,24 @@ describe( 'Collection', () => {
 
 			expect( collection._bindToCollection ).to.be.null;
 		} );
+
+		it( 'should fire the "change" event', () => {
+			const items = [ {}, {}, {} ];
+			const spy = sinon.spy();
+
+			collection.addMany( items );
+			collection.on( 'change', spy );
+
+			collection.clear();
+
+			sinon.assert.calledOnce( spy );
+
+			expect( spy.args[ 0 ][ 1 ] ).to.deep.eql( {
+				added: [],
+				removed: items,
+				index: 0
+			} );
+		} );
 	} );
 
 	describe( 'bindTo()', () => {