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

Commit

Permalink
Fix: ContextualToolbar will accept the object format of `config.con…
Browse files Browse the repository at this point in the history
…textualToolbar`. Closes #316.

BREAKING CHANGE: `Toolbar#fillFromConfig()` cannot be now called with an `undefined`. Make sure to use `normalizeToolbarConfig()` to get a reliable object.
  • Loading branch information
Reinmar committed Sep 30, 2017
1 parent 93feb9c commit d71cad8
Show file tree
Hide file tree
Showing 6 changed files with 68 additions and 23 deletions.
9 changes: 5 additions & 4 deletions src/toolbar/contextual/contextualtoolbar.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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 );
}

/**
Expand Down Expand Up @@ -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
*/
22 changes: 17 additions & 5 deletions src/toolbar/normalizetoolbarconfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 );
}
4 changes: 0 additions & 4 deletions src/toolbar/toolbarview.js
Original file line number Diff line number Diff line change
Expand Up @@ -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() );
Expand Down
23 changes: 23 additions & 0 deletions tests/toolbar/contextual/contextualtoolbar.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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
Expand Down
27 changes: 23 additions & 4 deletions tests/toolbar/normalizetoolbarconfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand All @@ -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 );
} );
} );
6 changes: 0 additions & 6 deletions tests/toolbar/toolbarview.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 );

Expand Down

0 comments on commit d71cad8

Please sign in to comment.