Jelajahi Sumber

Removed HeadingEngine#commands. Simplified config.heading.options localization.

Aleksander Nowodzinski 9 tahun lalu
induk
melakukan
1f77b8de1e

+ 50 - 30
packages/ckeditor5-heading/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.<module:heading/headingcommand~HeadingOption>}.
+	 */
+	_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' );
-}

+ 2 - 63
packages/ckeditor5-heading/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.<module:heading/headingcommand~HeadingCommand>}
-		 */
-		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.<module:heading/headingcommand~HeadingOption>}.
-	 */
-	_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.<module:heading/headingcommand~HeadingOption>} #_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;
-	}
 }

+ 16 - 9
packages/ckeditor5-heading/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' );
 			} );
 

+ 2 - 5
packages/ckeditor5-heading/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;