Browse Source

Merge pull request #325 from ckeditor/t/324

Other: The `ComponentFactory` should be case-insensitive. Closes #324.
Aleksander Nowodzinski 8 năm trước cách đây
mục cha
commit
efc68c744c

+ 18 - 4
packages/ckeditor5-ui/src/componentfactory.js

@@ -14,6 +14,7 @@ import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';
  *
  * It allows functions producing specific UI components to be registered under their unique names
  * in the factory. A registered component can be then instantiated by providing its name.
+ * Note that names are case-insensitive.
  *
  *		// Editor provides localization tools for the factory.
  *		const factory = new ComponentFactory( editor );
@@ -24,6 +25,9 @@ import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';
  *		// An instance of FooView.
  *		const fooInstance = factory.create( 'foo' );
  *
+ *		// Names are case-insensitive os this is also allowed:
+ *		const barInstance = factory.create( 'Bar' );
+ *
  * The {@link module:core/editor/editor~Editor#locale editor locale} is passed to the factory
  * function when {@link module:ui/componentfactory~ComponentFactory#create} is called.
  */
@@ -53,7 +57,7 @@ export default class ComponentFactory {
 	}
 
 	/**
-	 * Returns an iterator of registered component names.
+	 * Returns an iterator of registered component names. Names are returned in lower case.
 	 *
 	 * @returns {Iterator.<String>}
 	 */
@@ -83,7 +87,7 @@ export default class ComponentFactory {
 			);
 		}
 
-		this._components.set( name, callback );
+		this._components.set( getNormalized( name ), callback );
 	}
 
 	/**
@@ -111,7 +115,7 @@ export default class ComponentFactory {
 			);
 		}
 
-		return this._components.get( name )( this.editor.locale );
+		return this._components.get( getNormalized( name ) )( this.editor.locale );
 	}
 
 	/**
@@ -121,6 +125,16 @@ export default class ComponentFactory {
 	 * @returns {Boolean}
 	 */
 	has( name ) {
-		return this._components.has( name );
+		return this._components.has( getNormalized( name ) );
 	}
 }
+
+//
+// Ensures that component name used as key in internal map is in lower case.
+//
+// @private
+// @param {String} name
+// @returns {String}
+function getNormalized( name ) {
+	return String( name ).toLowerCase();
+}

+ 29 - 1
packages/ckeditor5-ui/tests/componentfactory.js

@@ -31,8 +31,9 @@ describe( 'ComponentFactory', () => {
 		it( 'returns iterator of command names', () => {
 			factory.add( 'foo', () => {} );
 			factory.add( 'bar', () => {} );
+			factory.add( 'Baz', () => {} );
 
-			expect( Array.from( factory.names() ) ).to.have.members( [ 'foo', 'bar' ] );
+			expect( Array.from( factory.names() ) ).to.have.members( [ 'foo', 'bar', 'baz' ] );
 		} );
 	} );
 
@@ -44,6 +45,14 @@ describe( 'ComponentFactory', () => {
 				factory.add( 'foo', () => {} );
 			} ).to.throw( CKEditorError, /^componentfactory-item-exists/ );
 		} );
+
+		it( 'throws when trying to override already registered component added with different case', () => {
+			factory.add( 'Foo', () => {} );
+
+			expect( () => {
+				factory.add( 'foo', () => {} );
+			} ).to.throw( CKEditorError, /^componentfactory-item-exists/ );
+		} );
 	} );
 
 	describe( 'create()', () => {
@@ -69,6 +78,23 @@ describe( 'ComponentFactory', () => {
 			expect( instance ).to.be.instanceof( View );
 			expect( instance.locale ).to.equal( locale );
 		} );
+
+		it( 'creates an instance even with different case', () => {
+			class View {
+				constructor( locale ) {
+					this.locale = locale;
+				}
+			}
+
+			const locale = editor.locale = {};
+
+			factory.add( 'Foo', locale => new View( locale ) );
+
+			const instance = factory.create( 'foo' );
+
+			expect( instance ).to.be.instanceof( View );
+			expect( instance.locale ).to.equal( locale );
+		} );
 	} );
 
 	describe( 'has()', () => {
@@ -79,6 +105,8 @@ describe( 'ComponentFactory', () => {
 			expect( factory.has( 'foo' ) ).to.be.true;
 			expect( factory.has( 'bar' ) ).to.be.true;
 			expect( factory.has( 'baz' ) ).to.be.false;
+			expect( factory.has( 'Foo' ) ).to.be.true;
+			expect( factory.has( 'fOO' ) ).to.be.true;
 		} );
 	} );
 } );