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 #292 from ckeditor/t/291
Browse files Browse the repository at this point in the history
Other: `ToolbarView#fillFromConfig()` will warn when the factory does not provide a component. Closes #291. Closes ckeditor/ckeditor5#526.
  • Loading branch information
Reinmar authored Sep 12, 2017
2 parents 56d0df1 + 6b8be86 commit 2e63e70
Show file tree
Hide file tree
Showing 4 changed files with 69 additions and 8 deletions.
18 changes: 13 additions & 5 deletions src/componentfactory.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ export default class ComponentFactory {
* i.e. to set attribute values, create attribute bindings, etc.
*/
add( name, callback ) {
if ( this._components.get( name ) ) {
if ( this.has( name ) ) {
/**
* The item already exists in the component factory.
*
Expand All @@ -83,9 +83,7 @@ export default class ComponentFactory {
* @returns {module:ui/view~View} The instantiated component view.
*/
create( name ) {
const component = this._components.get( name );

if ( !component ) {
if ( !this.has( name ) ) {
/**
* There is no such UI component in the factory.
*
Expand All @@ -97,6 +95,16 @@ export default class ComponentFactory {
);
}

return component( this.editor.locale );
return this._components.get( name )( this.editor.locale );
}

/**
* Checks if a component of a given name is registered in the factory.
*
* @param {String} name The name of the component.
* @returns {Boolean}
*/
has( name ) {
return this._components.has( name );
}
}
27 changes: 24 additions & 3 deletions src/toolbar/toolbarview.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import FocusCycler from '../focuscycler';
import KeystrokeHandler from '@ckeditor/ckeditor5-utils/src/keystrokehandler';
import ToolbarSeparatorView from './toolbarseparatorview';
import preventDefault from '../bindings/preventdefault.js';
import log from '@ckeditor/ckeditor5-utils/src/log';

/**
* The toolbar view class.
Expand Down Expand Up @@ -126,9 +127,29 @@ export default class ToolbarView extends View {
}

config.map( name => {
const component = name == '|' ? new ToolbarSeparatorView() : factory.create( name );

this.items.add( component );
if ( name == '|' ) {
this.items.add( new ToolbarSeparatorView() );
} else if ( factory.has( name ) ) {
this.items.add( factory.create( name ) );
} else {
/**
* There was a problem processing the configuration of the toolbar. The item with the given
* name does not exist so it was omitted when rendering the toolbar.
*
* This warning usually shows up when the {@link module:core/plugin~Plugin} which is supposed
* to provide a toolbar item has not been loaded or there is a typo in the configuration.
*
* Make sure the plugin responsible for this toolbar item is loaded and the toolbar configuration
* is correct, e.g. {@link module:basic-styles/bold~Bold} is loaded for the `'bold'` toolbar item.
*
* @error toolbarview-item-unavailable
* @param {String} name The name of the component.
*/
log.warn(
'toolbarview-item-unavailable: The requested toolbar item is unavailable.',
{ name }
);
}
} );
}
}
Expand Down
11 changes: 11 additions & 0 deletions tests/componentfactory.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,4 +70,15 @@ describe( 'ComponentFactory', () => {
expect( instance.locale ).to.equal( locale );
} );
} );

describe( 'has()', () => {
it( 'checks if the factory contains a component of a given name', () => {
factory.add( 'foo', () => {} );
factory.add( 'bar', () => {} );

expect( factory.has( 'foo' ) ).to.be.true;
expect( factory.has( 'bar' ) ).to.be.true;
expect( factory.has( 'baz' ) ).to.be.false;
} );
} );
} );
21 changes: 21 additions & 0 deletions tests/toolbar/toolbarview.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,15 @@ import FocusTracker from '@ckeditor/ckeditor5-utils/src/focustracker';
import FocusCycler from '../../src/focuscycler';
import { keyCodes } from '@ckeditor/ckeditor5-utils/src/keyboard';
import ViewCollection from '../../src/viewcollection';
import log from '@ckeditor/ckeditor5-utils/src/log';
import View from '../../src/view';
import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils';

describe( 'ToolbarView', () => {
let locale, view;

testUtils.createSinonSandbox();

beforeEach( () => {
locale = {};
view = new ToolbarView( locale );
Expand Down Expand Up @@ -247,6 +251,23 @@ describe( 'ToolbarView', () => {
expect( items.get( 2 ) ).to.be.instanceOf( ToolbarSeparatorView );
expect( items.get( 3 ).name ).to.equal( 'foo' );
} );

it( 'warns if there is no such component in the factory', () => {
const items = view.items;
testUtils.sinon.stub( log, 'warn' );

view.fillFromConfig( [ 'foo', 'bar', 'baz' ], factory );

expect( items ).to.have.length( 2 );
expect( items.get( 0 ).name ).to.equal( 'foo' );
expect( items.get( 1 ).name ).to.equal( 'bar' );

sinon.assert.calledOnce( log.warn );
sinon.assert.calledWithExactly( log.warn,
sinon.match( /^toolbarview-item-unavailable/ ),
{ name: 'baz' }
);
} );
} );
} );

Expand Down

0 comments on commit 2e63e70

Please sign in to comment.