From 7b2c1c8f0f82a74557e8b38c3e64d34b22663c8b Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Wed, 8 Mar 2017 15:56:03 +0100 Subject: [PATCH] Removed HeadingEngine#commands. Simplified config.heading.options localization. --- src/heading.js | 80 ++++++++++++++++++++++++++---------------- src/headingengine.js | 65 ++-------------------------------- tests/heading.js | 25 ++++++++----- tests/headingengine.js | 7 ++-- 4 files changed, 70 insertions(+), 107 deletions(-) diff --git a/src/heading.js b/src/heading.js index 852a528..b7b9e42 100644 --- a/src/heading.js +++ b/src/heading.js @@ -8,7 +8,6 @@ */ import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph'; -import HeadingCommand from './headingcommand'; import HeadingEngine from './headingengine'; import Plugin from '@ckeditor/ckeditor5-core/src/plugin'; import Model from '@ckeditor/ckeditor5-ui/src/model'; @@ -34,21 +33,22 @@ export default class Heading extends Plugin { */ init() { const editor = this.editor; - const t = editor.t; - const headingEngine = editor.plugins.get( HeadingEngine ); - const commands = headingEngine.commands; const dropdownItems = new Collection(); - let defaultCommand; + const options = this._getLocalizedOptions(); + const commands = []; + let defaultOption; - for ( let command of commands ) { + for ( let option of options ) { // Add the option to the collection. dropdownItems.add( new Model( { - modelElement: getCommandModelElement( command ), - label: getCommandTitle( command, t ) + modelElement: option.modelElement, + label: option.title } ) ); - if ( !( command instanceof HeadingCommand ) ) { - defaultCommand = command; + commands.push( editor.commands.get( option.modelElement ) ); + + if ( !defaultOption && option.modelElement == 'paragraph' ) { + defaultOption = option; } } @@ -73,7 +73,7 @@ export default class Heading extends Plugin { const index = areActive.findIndex( value => value ); // If none of the commands is active, display the first one. - return getCommandTitle( index > -1 ? commands.get( index ) : defaultCommand, t ); + return ( options[ index ] || defaultOption ).title; } ); @@ -90,6 +90,45 @@ export default class Heading extends Plugin { return dropdown; } ); } + + /** + * Returns heading options as defined in `config.heading.options` but processed to consider + * editor localization, i.e. to display {@link module:heading/headingcommand~HeadingOption} + * in the correct language. + * + * Note: The reason behind this method is that there's no way to use {@link module:utils/locale~Locale#t} + * when the user config is defined because the editor does not exist yet. + * + * @private + * @returns {Array.}. + */ + _getLocalizedOptions() { + const editor = this.editor; + const t = editor.t; + const localizedTitles = { + Paragraph: t( 'Paragraph' ), + 'Heading 1': t( 'Heading 1' ), + 'Heading 2': t( 'Heading 2' ), + 'Heading 3': t( 'Heading 3' ) + }; + + return editor.config.get( 'heading.options' ).map( option => { + let title; + + if ( option.modelElement == 'paragraph' ) { + title = localizedTitles.Paragraph; + } else { + title = localizedTitles[ option.title ]; + } + + if ( title && title != option.title ) { + // Clone the option to avoid altering the original `config.heading.options`. + option = Object.assign( {}, option, { title } ); + } + + return option; + } ); + } } // Returns an array of binding components for @@ -103,22 +142,3 @@ export default class Heading extends Plugin { function getCommandsBindingTargets( commands, attribute ) { return Array.prototype.concat( ...commands.map( c => [ c, attribute ] ) ); } - -// Returns the `modelElement` string for given command. -// -// @private -// @param {module:core/command/command~Command} command -// @returns {String} -function getCommandModelElement( command ) { - return command instanceof HeadingCommand ? command.modelElement : 'paragraph'; -} - -// Returns the `title` string for given command. -// -// @private -// @param {module:core/command/command~Command} command -// @param {module:utils/locale~Locale#t} t -// @returns {String} -function getCommandTitle( command, t ) { - return command instanceof HeadingCommand ? command.title : t( 'Paragraph' ); -} diff --git a/src/headingengine.js b/src/headingengine.js index ae4f35e..708e4fa 100644 --- a/src/headingengine.js +++ b/src/headingengine.js @@ -8,7 +8,6 @@ */ import Plugin from '@ckeditor/ckeditor5-core/src/plugin'; -import Collection from '@ckeditor/ckeditor5-utils/src/collection'; import buildModelConverter from '@ckeditor/ckeditor5-engine/src/conversion/buildmodelconverter'; import buildViewConverter from '@ckeditor/ckeditor5-engine/src/conversion/buildviewconverter'; import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph'; @@ -29,14 +28,6 @@ export default class HeadingEngine extends Plugin { constructor( editor ) { super( editor ); - /** - * A collection of heading commands associated with heading engine. - * - * @readonly - * @member {module:utils/collection~Collection.} - */ - this.commands = new Collection( { idProperty: 'modelElement' } ); - // TODO: This needs proper documentation, i.e. why paragraph entry does not need // more properties (https://github.com/ckeditor/ckeditor5/issues/403). editor.config.define( 'heading', { @@ -63,7 +54,7 @@ export default class HeadingEngine extends Plugin { const editor = this.editor; const data = editor.data; const editing = editor.editing; - const options = this._getLocalizedOptions(); + const options = editor.config.get( 'heading.options' ); let command; for ( let option of options ) { @@ -85,11 +76,7 @@ export default class HeadingEngine extends Plugin { // Register the heading command for this option. command = new HeadingCommand( editor, option ); editor.commands.set( command.modelElement, command ); - } else { - command = editor.commands.get( defaultModelElement ); } - - this.commands.add( command ); } } @@ -101,7 +88,7 @@ export default class HeadingEngine extends Plugin { // Enter at the end of a heading element should create a paragraph. const editor = this.editor; const enterCommand = editor.commands.get( 'enter' ); - const options = this._getLocalizedOptions(); + const options = editor.config.get( 'heading.options' ); if ( enterCommand ) { this.listenTo( enterCommand, 'afterExecute', ( evt, data ) => { @@ -115,52 +102,4 @@ export default class HeadingEngine extends Plugin { } ); } } - - /** - * Returns heading options as defined in `config.heading.options` but processed to consider - * editor localization, i.e. to display {@link module:heading/headingcommand~HeadingOption} - * in the correct language. - * - * Note: The reason behind this method is that there's no way to use {@link module:utils/locale~Locale#t} - * when the user config is defined because the editor does not exist yet. - * - * @private - * @returns {Array.}. - */ - _getLocalizedOptions() { - if ( this._cachedLocalizedOptions ) { - return this._cachedLocalizedOptions; - } - - const editor = this.editor; - const t = editor.t; - const localizedLabels = { - Paragraph: t( 'Paragraph' ), - 'Heading 1': t( 'Heading 1' ), - 'Heading 2': t( 'Heading 2' ), - 'Heading 3': t( 'Heading 3' ) - }; - - /** - * Cached localized version of `config.heading.options` generated by - * {@link module:heading/headingengine~HeadingEngine#_getLocalizedOptions}. - * - * @private - * @readonly - * @member {Array.} #_cachedLocalizedOptions - */ - this._cachedLocalizedOptions = editor.config.get( 'heading.options' ) - .map( option => { - if ( localizedLabels[ option.title ] ) { - // Clone the option to avoid altering the original `config.heading.options`. - option = Object.assign( {}, option, { - title: localizedLabels[ option.title ] - } ); - } - - return option; - } ); - - return this._cachedLocalizedOptions; - } } diff --git a/tests/heading.js b/tests/heading.js index 724de28..802ff73 100644 --- a/tests/heading.js +++ b/tests/heading.js @@ -84,28 +84,32 @@ describe( 'Heading', () => { let commands; beforeEach( () => { - commands = editor.plugins.get( HeadingEngine ).commands; + commands = {}; + + editor.config.get( 'heading.options' ).forEach( ( { modelElement } ) => { + commands[ modelElement ] = editor.commands.get( modelElement ); + } ); } ); it( 'isEnabled', () => { - for ( let command of commands ) { - command.isEnabled = false; + for ( let name in commands ) { + commands[ name ].isEnabled = false; } expect( dropdown.buttonView.isEnabled ).to.be.false; - commands.get( 'heading2' ).isEnabled = true; + commands.heading2.isEnabled = true; expect( dropdown.buttonView.isEnabled ).to.be.true; } ); it( 'label', () => { - for ( let command of commands ) { - command.value = false; + for ( let name in commands ) { + commands[ name ].value = false; } expect( dropdown.buttonView.label ).to.equal( 'Paragraph' ); - commands.get( 'heading2' ).value = true; + commands.heading2.value = true; expect( dropdown.buttonView.label ).to.equal( 'Heading 2' ); } ); } ); @@ -131,7 +135,10 @@ describe( 'Heading', () => { .then( newEditor => { editor = newEditor; dropdown = editor.ui.componentFactory.create( 'headings' ); - commands = editor.plugins.get( HeadingEngine ).commands; + commands = {}; + editor.config.get( 'heading.options' ).forEach( ( { modelElement } ) => { + commands[ modelElement ] = editor.commands.get( modelElement ); + } ); } ); } ); @@ -147,7 +154,7 @@ describe( 'Heading', () => { const buttonView = dropdown.buttonView; expect( buttonView.label ).to.equal( 'Akapit' ); - commands.get( 'heading1' ).value = true; + commands.heading1.value = true; expect( buttonView.label ).to.equal( 'Nagłówek 1' ); } ); diff --git a/tests/headingengine.js b/tests/headingengine.js index 9f9523a..a307e37 100644 --- a/tests/headingengine.js +++ b/tests/headingengine.js @@ -136,11 +136,8 @@ describe( 'HeadingEngine', () => { .then( editor => { document = editor.document; - const commands = editor.plugins.get( HeadingEngine ).commands; - - expect( commands ).to.have.length( 2 ); - expect( commands.get( 0 ).id ).to.equal( options[ 0 ].id ); - expect( commands.get( 1 ).id ).to.equal( options[ 1 ].id ); + expect( editor.commands.get( 'h4' ) ).to.be.instanceOf( HeadingCommand ); + expect( editor.commands.get( 'paragraph' ) ).to.be.instanceOf( ParagraphCommand ); expect( document.schema.hasItem( 'paragraph' ) ).to.be.true; expect( document.schema.hasItem( 'h4' ) ).to.be.true;