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

Commit 94417e9

Browse files
authored
Merge pull request #325 from ckeditor/t/324
Other: The `ComponentFactory` should be case-insensitive. Closes #324.
2 parents 7b33f15 + 67522ba commit 94417e9

File tree

2 files changed

+47
-5
lines changed

2 files changed

+47
-5
lines changed

src/componentfactory.js

+18-4
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';
1414
*
1515
* It allows functions producing specific UI components to be registered under their unique names
1616
* in the factory. A registered component can be then instantiated by providing its name.
17+
* Note that names are case-insensitive.
1718
*
1819
* // Editor provides localization tools for the factory.
1920
* const factory = new ComponentFactory( editor );
@@ -24,6 +25,9 @@ import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';
2425
* // An instance of FooView.
2526
* const fooInstance = factory.create( 'foo' );
2627
*
28+
* // Names are case-insensitive os this is also allowed:
29+
* const barInstance = factory.create( 'Bar' );
30+
*
2731
* The {@link module:core/editor/editor~Editor#locale editor locale} is passed to the factory
2832
* function when {@link module:ui/componentfactory~ComponentFactory#create} is called.
2933
*/
@@ -53,7 +57,7 @@ export default class ComponentFactory {
5357
}
5458

5559
/**
56-
* Returns an iterator of registered component names.
60+
* Returns an iterator of registered component names. Names are returned in lower case.
5761
*
5862
* @returns {Iterator.<String>}
5963
*/
@@ -83,7 +87,7 @@ export default class ComponentFactory {
8387
);
8488
}
8589

86-
this._components.set( name, callback );
90+
this._components.set( getNormalized( name ), callback );
8791
}
8892

8993
/**
@@ -111,7 +115,7 @@ export default class ComponentFactory {
111115
);
112116
}
113117

114-
return this._components.get( name )( this.editor.locale );
118+
return this._components.get( getNormalized( name ) )( this.editor.locale );
115119
}
116120

117121
/**
@@ -121,6 +125,16 @@ export default class ComponentFactory {
121125
* @returns {Boolean}
122126
*/
123127
has( name ) {
124-
return this._components.has( name );
128+
return this._components.has( getNormalized( name ) );
125129
}
126130
}
131+
132+
//
133+
// Ensures that component name used as key in internal map is in lower case.
134+
//
135+
// @private
136+
// @param {String} name
137+
// @returns {String}
138+
function getNormalized( name ) {
139+
return String( name ).toLowerCase();
140+
}

tests/componentfactory.js

+29-1
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,9 @@ describe( 'ComponentFactory', () => {
3131
it( 'returns iterator of command names', () => {
3232
factory.add( 'foo', () => {} );
3333
factory.add( 'bar', () => {} );
34+
factory.add( 'Baz', () => {} );
3435

35-
expect( Array.from( factory.names() ) ).to.have.members( [ 'foo', 'bar' ] );
36+
expect( Array.from( factory.names() ) ).to.have.members( [ 'foo', 'bar', 'baz' ] );
3637
} );
3738
} );
3839

@@ -44,6 +45,14 @@ describe( 'ComponentFactory', () => {
4445
factory.add( 'foo', () => {} );
4546
} ).to.throw( CKEditorError, /^componentfactory-item-exists/ );
4647
} );
48+
49+
it( 'throws when trying to override already registered component added with different case', () => {
50+
factory.add( 'Foo', () => {} );
51+
52+
expect( () => {
53+
factory.add( 'foo', () => {} );
54+
} ).to.throw( CKEditorError, /^componentfactory-item-exists/ );
55+
} );
4756
} );
4857

4958
describe( 'create()', () => {
@@ -69,6 +78,23 @@ describe( 'ComponentFactory', () => {
6978
expect( instance ).to.be.instanceof( View );
7079
expect( instance.locale ).to.equal( locale );
7180
} );
81+
82+
it( 'creates an instance even with different case', () => {
83+
class View {
84+
constructor( locale ) {
85+
this.locale = locale;
86+
}
87+
}
88+
89+
const locale = editor.locale = {};
90+
91+
factory.add( 'Foo', locale => new View( locale ) );
92+
93+
const instance = factory.create( 'foo' );
94+
95+
expect( instance ).to.be.instanceof( View );
96+
expect( instance.locale ).to.equal( locale );
97+
} );
7298
} );
7399

74100
describe( 'has()', () => {
@@ -79,6 +105,8 @@ describe( 'ComponentFactory', () => {
79105
expect( factory.has( 'foo' ) ).to.be.true;
80106
expect( factory.has( 'bar' ) ).to.be.true;
81107
expect( factory.has( 'baz' ) ).to.be.false;
108+
expect( factory.has( 'Foo' ) ).to.be.true;
109+
expect( factory.has( 'fOO' ) ).to.be.true;
82110
} );
83111
} );
84112
} );

0 commit comments

Comments
 (0)