Browse Source

Removed the ability to pass items to the constructor and wrapped the existing option in an options object.

Piotrek Koszuliński 10 năm trước cách đây
mục cha
commit
db80a1fe68

+ 11 - 16
packages/ckeditor5-engine/src/collection.js

@@ -26,9 +26,10 @@ CKEDITOR.define( [ 'emittermixin', 'ckeditorerror', 'utils' ], function( Emitter
 		 *
 		 * @constructor
 		 * @param {Iterale} [items] Items to be added to the collection.
-		 * @param {String} [idName='id'] The name of the property which is considered to identify an item.
+		 * @param {Object} options The options object.
+		 * @param {String} [options.idProperty='id'] The name of the property which is considered to identify an item.
 		 */
-		constructor( items, idName ) {
+		constructor( options ) {
 			/**
 			 * The internal list of items in the collection.
 			 *
@@ -59,13 +60,7 @@ CKEDITOR.define( [ 'emittermixin', 'ckeditorerror', 'utils' ], function( Emitter
 			 * @private
 			 * @property {String}
 			 */
-			this._idName = idName || 'id';
-
-			if ( items ) {
-				for ( let item of items ) {
-					this.add( item );
-				}
-			}
+			this._idProperty = options && options.idProperty || 'id';
 		}
 
 		/**
@@ -86,10 +81,10 @@ CKEDITOR.define( [ 'emittermixin', 'ckeditorerror', 'utils' ], function( Emitter
 		 */
 		add( item ) {
 			var itemId;
-			var idName = this._idName;
+			var idProperty = this._idProperty;
 
-			if ( ( idName in item ) ) {
-				itemId = item[ idName ];
+			if ( ( idProperty in item ) ) {
+				itemId = item[ idProperty ];
 
 				if ( typeof itemId != 'string' ) {
 					/**
@@ -110,7 +105,7 @@ CKEDITOR.define( [ 'emittermixin', 'ckeditorerror', 'utils' ], function( Emitter
 				}
 			} else {
 				itemId = this._getNextId();
-				item[ idName ] = itemId;
+				item[ idProperty ] = itemId;
 			}
 
 			this._items.push( item );
@@ -153,7 +148,7 @@ CKEDITOR.define( [ 'emittermixin', 'ckeditorerror', 'utils' ], function( Emitter
 		remove( subject ) {
 			var index, id, item;
 			var itemDoesNotExist = false;
-			var idName = this._idName;
+			var idProperty = this._idProperty;
 
 			if ( typeof subject == 'string' ) {
 				id = subject;
@@ -169,11 +164,11 @@ CKEDITOR.define( [ 'emittermixin', 'ckeditorerror', 'utils' ], function( Emitter
 				itemDoesNotExist = !item;
 
 				if ( item ) {
-					id = item[ idName ];
+					id = item[ idProperty ];
 				}
 			} else {
 				item = subject;
-				id = item[ idName ];
+				id = item[ idProperty ];
 				index = this._items.indexOf( item );
 				itemDoesNotExist = ( index == -1 || !this._itemMap.get( id ) );
 			}

+ 1 - 1
packages/ckeditor5-engine/src/plugincollection.js

@@ -25,7 +25,7 @@ CKEDITOR.define( [
 		 * @constructor
 		 */
 		constructor( editor ) {
-			super( null, 'name' );
+			super( { idProperty: 'name' } );
 
 			this._editor = editor;
 		}

+ 13 - 22
packages/ckeditor5-engine/tests/collection/collection.js

@@ -9,35 +9,26 @@ var modules = bender.amd.require( 'collection', 'ckeditorerror' );
 
 bender.tools.createSinonSandbox();
 
-function getCollection( items, idName ) {
+function getCollection( items, idProperty ) {
 	var Collection = modules.collection;
 
-	return new Collection( items, idName );
+	return new Collection( items, idProperty );
 }
 
-function getItem( id, idName ) {
+function getItem( id, idProperty ) {
 	return {
-		[ idName || 'id' ]: id
+		[ idProperty || 'id' ]: id
 	};
 }
 
 ///////////////////////////////////////
 
 describe( 'constructor', () => {
-	it( 'creates collection with items', () => {
-		var Collection = modules.collection;
-		var item1 = getItem( 'foo' );
-		var item2 = getItem( 'bar' );
-		var box = new Collection( [ item1, item2 ] );
-
-		expect( box ).to.have.length( 2 );
-	} );
-
-	it( 'allows to change the id name used by the collection', () => {
+	it( 'allows to change the id property used by the collection', () => {
 		var Collection = modules.collection;
 		var item1 = { id: 'foo', name: 'xx' };
 		var item2 = { id: 'foo', name: 'yy' };
-		var box = new Collection( null, 'name' );
+		var box = new Collection( { idProperty: 'name' } );
 
 		box.add( item1 );
 		box.add( item2 );
@@ -87,8 +78,8 @@ describe( 'add', () => {
 		expect( box.get( 'bar' ) ).to.equal( item2 );
 	} );
 
-	it( 'should enable get( id ) - custom id name', () => {
-		var box = getCollection( null, 'name' );
+	it( 'should enable get( id ) - custom id property', () => {
+		var box = getCollection( { idProperty: 'name' } );
 		var item1 = getItem( 'foo', 'name' );
 		var item2 = getItem( 'bar', 'name' );
 
@@ -109,8 +100,8 @@ describe( 'add', () => {
 		expect( box.get( item.id ) ).to.equal( item );
 	} );
 
-	it( 'should generate an id when not defined - custom id name', () => {
-		var box = getCollection( null, 'name' );
+	it( 'should generate an id when not defined - custom id property', () => {
+		var box = getCollection( { idProperty: 'name' } );
 		var item = {};
 
 		box.add( item );
@@ -228,8 +219,8 @@ describe( 'remove', () => {
 		expect( removedItem ).to.have.property( 'id', 'foo' );
 	} );
 
-	it( 'should remove the model by index - custom id name', () => {
-		var box = getCollection( null, 'name' );
+	it( 'should remove the model by index - custom id property', () => {
+		var box = getCollection( { idProperty: 'name' } );
 
 		box.add( getItem( 'foo', 'name' ) );
 
@@ -275,7 +266,7 @@ describe( 'remove', () => {
 		expect( removedItem ).to.equal( item );
 	} );
 
-	it( 'should remove the model by model - custom id name', () => {
+	it( 'should remove the model by model - custom id property', () => {
 		var box = getCollection( null, 'name' );
 		var item = getItem( 'foo', 'name' );