Fix: ContextualToolbar will accept the object format of config.contextualToolbar. Closes #316.

BREAKING CHANGE: `Toolbar#fillFromConfig()` cannot be now called with an `undefined`. Make sure to use `normalizeToolbarConfig()` to get a reliable object.

Author: Reinmar

src/toolbar/contextual/contextualtoolbar.js

@@ -14,6 +14,7 @@ import ToolbarView from '../toolbarview';
 import BalloonPanelView from '../../panel/balloon/balloonpanelview.js';
 import debounce from '@ckeditor/ckeditor5-utils/src/lib/lodash/debounce';
 import Rect from '@ckeditor/ckeditor5-utils/src/dom/rect';
+import normalizeToolbarConfig from '../normalizetoolbarconfig';
 
 /**
  * The contextual toolbar.
@@ -89,15 +90,15 @@ export default class ContextualToolbar extends Plugin {
 
 	/**
 	 * Creates toolbar components based on given configuration.
-	 * This needs to be done when all plugins will be ready.
+	 * This needs to be done when all plugins are ready.
 	 *
 	 * @inheritDoc
 	 */
 	afterInit() {
-		const config = this.editor.config.get( 'contextualToolbar' );
+		const config = normalizeToolbarConfig( this.editor.config.get( 'contextualToolbar' ) );
 		const factory = this.editor.ui.componentFactory;
 
-		this.toolbarView.fillFromConfig( config, factory );
+		this.toolbarView.fillFromConfig( config.items, factory );
 	}
 
 	/**
@@ -296,5 +297,5 @@ function getBalloonPositions( isBackward ) {
  *
  * Read also about configuring the main editor toolbar in {@link module:core/editor/editorconfig~EditorConfig#toolbar}.
  *
- * @member {Array.<String>} module:core/editor/editorconfig~EditorConfig#contextualToolbar
+ * @member {Array.<String>|Object} module:core/editor/editorconfig~EditorConfig#contextualToolbar
  */

src/toolbar/normalizetoolbarconfig.js

@@ -8,28 +8,40 @@
  */
 
 /**
- * Normalizes the toolbar configuration (`config.toolbar`), which may be defined as an `Array`:
+ * Normalizes the toolbar configuration (`config.toolbar`), which:
+ *
+ * * may be defined as an `Array`:
  *
  * 		toolbar: [ 'headings', 'bold', 'italic', 'link', ... ]
  *
- * or an `Object`:
+ * * or an `Object`:
  *
  *		toolbar: {
  *			items: [ 'headings', 'bold', 'italic', 'link', ... ],
  *			...
  *		}
  *
+ * * or may not be defined at all (`undefined`)
+ *
  * and returns it in the object form.
  *
- * @param {Array|Object} config The value of `config.toolbar`.
+ * @param {Array|Object|undefined} config The value of `config.toolbar`.
  * @returns {Object} A normalized toolbar config object.
  */
 export default function normalizeToolbarConfig( config ) {
 	if ( Array.isArray( config ) ) {
-		config = {
+		return {
 			items: config
 		};
 	}
 
-	return config;
+	if ( !config ) {
+		return {
+			items: []
+		};
+	}
+
+	return Object.assign( {
+		items: []
+	}, config );
 }

src/toolbar/toolbarview.js

@@ -122,10 +122,6 @@ export default class ToolbarView extends View {
 	 * @param {module:ui/componentfactory~ComponentFactory} factory A factory producing toolbar items.
 	 */
 	fillFromConfig( config, factory ) {
-		if ( !config ) {
-			return;
-		}
-
 		config.map( name => {
 			if ( name == '|' ) {
 				this.items.add( new ToolbarSeparatorView() );

tests/toolbar/contextual/contextualtoolbar.js

@@ -10,6 +10,7 @@ import ToolbarView from '../../../src/toolbar/toolbarview';
 import Plugin from '@ckeditor/ckeditor5-core/src/plugin';
 import Bold from '@ckeditor/ckeditor5-basic-styles/src/bold';
 import Italic from '@ckeditor/ckeditor5-basic-styles/src/italic';
+import Underline from '@ckeditor/ckeditor5-basic-styles/src/underline';
 import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph';
 
 import { setData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model.js';
@@ -71,6 +72,28 @@ describe( 'ContextualToolbar', () => {
 		expect( contextualToolbar.toolbarView.items ).to.length( 2 );
 	} );
 
+	it( 'should accept the extended format of the toolbar config', () => {
+		const editorElement = document.createElement( 'div' );
+		document.body.appendChild( editorElement );
+
+		return ClassicTestEditor
+			.create( editorElement, {
+				plugins: [ Paragraph, Bold, Italic, Underline, ContextualToolbar ],
+				contextualToolbar: {
+					items: [ 'bold', 'italic', 'underline' ]
+				}
+			} )
+			.then( editor => {
+				const contextualToolbar = editor.plugins.get( ContextualToolbar );
+
+				expect( contextualToolbar.toolbarView.items ).to.length( 3 );
+
+				editorElement.remove();
+
+				return editor.destroy();
+			} );
+	} );
+
 	it( 'should fire internal `_selectionChangeDebounced` event 200 ms after last selection change', done => {
 		// This test uses setTimeout to test lodash#debounce because sinon fake timers
 		// doesn't work with lodash. Lodash keeps time related stuff in a closure

tests/toolbar/normalizetoolbarconfig.js

@@ -11,7 +11,7 @@ describe( 'normalizeToolbarConfig()', () => {
 		const normalized = normalizeToolbarConfig( cfg );
 
 		expect( normalized ).to.be.an( 'object' );
-		expect( normalized.items ).to.equal( cfg );
+		expect( normalized.items ).to.deep.equal( cfg );
 	} );
 
 	it( 'passes through an already normalized config', () => {
@@ -21,8 +21,27 @@ describe( 'normalizeToolbarConfig()', () => {
 		};
 		const normalized = normalizeToolbarConfig( cfg );
 
-		expect( normalized ).to.equal( cfg );
-		expect( normalized.items ).to.equal( cfg.items );
-		expect( normalized.foo ).to.equal( cfg.foo );
+		expect( normalized ).to.deep.equal( cfg );
+	} );
+
+	it( 'adds missing items property', () => {
+		const cfg = {
+			foo: 'bar'
+		};
+
+		const normalized = normalizeToolbarConfig( cfg );
+
+		expect( normalized ).to.deep.equal( {
+			items: [],
+			foo: 'bar'
+		} );
+		expect( normalized ).to.not.equal( cfg ); // Make sure we don't modify an existing obj.
+	} );
+
+	it( 'returns an empty config if config is not defined', () => {
+		const normalized = normalizeToolbarConfig();
+
+		expect( normalized ).to.be.an( 'object' );
+		expect( normalized.items ).to.be.an( 'array' ).of.length( 0 );
 	} );
 } );

tests/toolbar/toolbarview.js

@@ -234,12 +234,6 @@ describe( 'ToolbarView', () => {
 			factory.add( 'bar', namedFactory( 'bar' ) );
 		} );
 
-		it( 'does not throw when no config is provided', () => {
-			expect( () => {
-				view.fillFromConfig();
-			} ).to.not.throw();
-		} );
-
 		it( 'expands the config into collection', () => {
 			view.fillFromConfig( [ 'foo', 'bar', '|', 'foo' ], factory );