Skip to content
This repository has been archived by the owner on Jun 26, 2020. It is now read-only.

Commit

Permalink
Merge pull request #325 from ckeditor/t/324
Browse files Browse the repository at this point in the history
Other: The `ComponentFactory` should be case-insensitive. Closes #324.
  • Loading branch information
oleq authored Oct 16, 2017
2 parents 7b33f15 + 67522ba commit 94417e9
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 5 deletions.
22 changes: 18 additions & 4 deletions src/componentfactory.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 );
Expand All @@ -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.
*/
Expand Down Expand Up @@ -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>}
*/
Expand Down Expand Up @@ -83,7 +87,7 @@ export default class ComponentFactory {
);
}

this._components.set( name, callback );
this._components.set( getNormalized( name ), callback );
}

/**
Expand Down Expand Up @@ -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 );
}

/**
Expand All @@ -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();
}
30 changes: 29 additions & 1 deletion tests/componentfactory.js
Original file line number Diff line number Diff line change
Expand Up @@ -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' ] );
} );
} );

Expand All @@ -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()', () => {
Expand All @@ -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()', () => {
Expand All @@ -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;
} );
} );
} );

0 comments on commit 94417e9

Please sign in to comment.