Other: Aligned the implementation to the new Command API (see https:/…

…/github.com/ckeditor/ckeditor5-core/issues/88). BREAKING CHANGES: The command API has been changed.
ckeditor · Jun 13, 2017 · 63b6c1c · 63b6c1c
2 parents c80e691 + c23a8b7
commit 63b6c1c
Show file tree

Hide file tree

Showing 4 changed files with 39 additions and 54 deletions.
diff --git a/src/heading.js b/src/heading.js
@@ -153,7 +153,7 @@ export default class Heading extends Plugin {
 // commands.
 //
 // @private
-// @param {Iterable.<module:core/command/command~Command>} commands
+// @param {Iterable.<module:core/command~Command>} commands
 // @param {String} attribute
 // @returns {Array.<String>}
 function getCommandsBindingTargets( commands, attribute ) {

diff --git a/src/headingcommand.js b/src/headingcommand.js
@@ -7,16 +7,17 @@
  * @module heading/headingcommand
  */
 
+import Command from '@ckeditor/ckeditor5-core/src/command';
+
 import Range from '@ckeditor/ckeditor5-engine/src/model/range';
-import Command from '@ckeditor/ckeditor5-core/src/command/command';
 import Selection from '@ckeditor/ckeditor5-engine/src/model/selection';
 import Position from '@ckeditor/ckeditor5-engine/src/model/position';
 import first from '@ckeditor/ckeditor5-utils/src/first';
 
 /**
  * The heading command. It is used by the {@link module:heading/heading~Heading heading feature} to apply headings.
  *
- * @extends module:core/command/command~Command
+ * @extends module:core/command~Command
  */
 export default class HeadingCommand extends Command {
 	/**
@@ -29,40 +30,43 @@ export default class HeadingCommand extends Command {
 		super( editor );
 
 		/**
-		 * Unique identifier of the command, also element's name in the model.
-		 * See {@link module:heading/heading~HeadingOption}.
+		 * Whether the selection starts in a heading of {@link #modelElement this level}.
 		 *
+		 * @observable
 		 * @readonly
-		 * @member {String}
+		 * @member {Boolean} #value
 		 */
-		this.modelElement = modelElement;
 
 		/**
-		 * Value of the command, indicating whether it is applied in the context
-		 * of current {@link module:engine/model/document~Document#selection selection}.
+		 * Unique identifier of the command, also element's name in the model.
+		 * See {@link module:heading/heading~HeadingOption}.
 		 *
 		 * @readonly
-		 * @observable
-		 * @member {Boolean}
+		 * @member {String}
 		 */
-		this.set( 'value', false );
+		this.modelElement = modelElement;
+	}
 
-		// Update current value each time changes are done on document.
-		this.listenTo( editor.document, 'changesDone', () => {
-			this.refreshValue();
-			this.refreshState();
-		} );
+	/**
+	 * @inheritDoc
+	 */
+	refresh() {
+		const block = first( this.editor.document.selection.getSelectedBlocks() );
+
+		this.value = !!block && block.is( this.modelElement );
+		this.isEnabled = !!block && checkCanBecomeHeading( block, this.modelElement, this.editor.document.schema );
 	}
 
 	/**
-	 * Executes command.
+	 * Executes the command. Applies the heading to the selected blocks or, if the first selected
+	 * block is a heading already, turns selected headings (of this level only) to paragraphs.
 	 *
-	 * @protected
+	 * @fires execute
 	 * @param {Object} [options] Options for executed command.
 	 * @param {module:engine/model/batch~Batch} [options.batch] Batch to collect all the change steps.
 	 * New batch will be created if this option is not set.
 	 */
-	_doExecute( options = {} ) {
+	execute( options = {} ) {
 		const editor = this.editor;
 		const document = editor.document;
 
@@ -96,25 +100,6 @@ export default class HeadingCommand extends Command {
 			}
 		} );
 	}
-
-	/**
-	 * Updates command's {@link #value value} based on current selection.
-	 */
-	refreshValue() {
-		const block = first( this.editor.document.selection.getSelectedBlocks() );
-
-		this.value = !!block && block.is( this.modelElement );
-	}
-
-	/**
-	 * @inheritDoc
-	 */
-	_checkEnabled() {
-		const document = this.editor.document;
-		const block = first( document.selection.getSelectedBlocks() );
-
-		return !!block && checkCanBecomeHeading( block, this.modelElement, document.schema );
-	}
 }
 
 // Checks whether the given block can be replaced by a specific heading.

diff --git a/src/headingengine.js b/src/headingengine.js
@@ -74,7 +74,7 @@ export default class HeadingEngine extends Plugin {
 					.toElement( option.modelElement );
 
 				// Register the heading command for this option.
-				editor.commands.set( option.modelElement, new HeadingCommand( editor, option.modelElement ) );
+				editor.commands.add( option.modelElement, new HeadingCommand( editor, option.modelElement ) );
 			}
 		}
 	}

diff --git a/tests/headingcommand.js b/tests/headingcommand.js
@@ -25,7 +25,7 @@ describe( 'HeadingCommand', () => {
 			commands = {};
 			schema = document.schema;
 
-			editor.commands.set( 'paragraph', new ParagraphCommand( editor ) );
+			editor.commands.add( 'paragraph', new ParagraphCommand( editor ) );
 			schema.registerItem( 'paragraph', '$block' );
 
 			for ( const option of options ) {
@@ -84,7 +84,7 @@ describe( 'HeadingCommand', () => {
 				expect( commands[ modelElement ].value ).to.be.false;
 			} );
 
-			it( 'should be refreshed after calling refreshValue()', () => {
+			it( 'should be refreshed after calling refresh()', () => {
 				const command = commands[ modelElement ];
 				setData( document, `<${ modelElement }>[foo]</${ modelElement }><notBlock>foo</notBlock>` );
 				const element = document.getRoot().getChild( 1 );
@@ -93,18 +93,18 @@ describe( 'HeadingCommand', () => {
 				document.selection.setRanges( [ Range.createIn( element ) ] );
 
 				expect( command.value ).to.be.true;
-				command.refreshValue();
+				command.refresh();
 				expect( command.value ).to.be.false;
 			} );
 		}
 	} );
 
-	describe( '_doExecute', () => {
+	describe( 'execute()', () => {
 		it( 'should update value after execution', () => {
 			const command = commands.heading1;
 
 			setData( document, '<paragraph>[]</paragraph>' );
-			command._doExecute();
+			command.execute();
 
 			expect( getData( document ) ).to.equal( '<heading1>[]</heading1>' );
 			expect( command.value ).to.be.true;
@@ -126,7 +126,7 @@ describe( 'HeadingCommand', () => {
 				'<paragraph>de]f</paragraph>'
 			);
 
-			commands.heading1._doExecute();
+			commands.heading1.execute();
 
 			expect( getData( document ) ).to.equal(
 				'<heading1>a[bc</heading1>' +
@@ -144,7 +144,7 @@ describe( 'HeadingCommand', () => {
 
 				expect( batch.deltas.length ).to.equal( 0 );
 
-				command._doExecute( { batch } );
+				command.execute( { batch } );
 
 				expect( batch.deltas.length ).to.be.above( 0 );
 			} );
@@ -157,7 +157,7 @@ describe( 'HeadingCommand', () => {
 
 				expect( batch.deltas.length ).to.equal( 0 );
 
-				command._doExecute( { batch } );
+				command.execute( { batch } );
 
 				expect( batch.deltas.length ).to.be.above( 0 );
 			} );
@@ -175,7 +175,7 @@ describe( 'HeadingCommand', () => {
 				const command = commands.heading1;
 
 				setData( document, '<heading1>foo[]bar</heading1>' );
-				command._doExecute();
+				command.execute();
 
 				expect( getData( document ) ).to.equal( '<paragraph>foo[]bar</paragraph>' );
 			} );
@@ -185,15 +185,15 @@ describe( 'HeadingCommand', () => {
 				schema.allow( { name: '$text', inside: 'inlineImage' } );
 
 				setData( document, '<paragraph><inlineImage>foo[]</inlineImage>bar</paragraph>' );
-				commands.heading1._doExecute();
+				commands.heading1.execute();
 
 				expect( getData( document ) ).to.equal( '<heading1><inlineImage>foo[]</inlineImage>bar</heading1>' );
 			} );
 
 			function test( from, to ) {
 				it( `converts ${ from.modelElement } to ${ to.modelElement } on collapsed selection`, () => {
 					setData( document, `<${ from.modelElement }>foo[]bar</${ from.modelElement }>` );
-					commands[ to.modelElement ]._doExecute();
+					commands[ to.modelElement ].execute();
 
 					expect( getData( document ) ).to.equal( `<${ to.modelElement }>foo[]bar</${ to.modelElement }>` );
 				} );
@@ -210,7 +210,7 @@ describe( 'HeadingCommand', () => {
 
 			it( 'converts all elements where selection is applied', () => {
 				setData( document, '<heading1>foo[</heading1><heading2>bar</heading2><heading3>]baz</heading3>' );
-				commands.heading3._doExecute();
+				commands.heading3.execute();
 
 				expect( getData( document ) ).to.equal(
 					'<heading3>foo[</heading3><heading3>bar</heading3><heading3>]baz</heading3>'
@@ -219,7 +219,7 @@ describe( 'HeadingCommand', () => {
 
 			it( 'resets to default value all elements with same option', () => {
 				setData( document, '<heading1>foo[</heading1><heading1>bar</heading1><heading2>baz</heading2>]' );
-				commands.heading1._doExecute();
+				commands.heading1.execute();
 
 				expect( getData( document ) ).to.equal(
 					'<paragraph>foo[</paragraph><paragraph>bar</paragraph><heading2>baz</heading2>]'
@@ -233,7 +233,7 @@ describe( 'HeadingCommand', () => {
 						`<${ fromElement }>foo[bar</${ fromElement }><${ fromElement }>baz]qux</${ fromElement }>`
 					);
 
-					commands[ toElement ]._doExecute();
+					commands[ toElement ].execute();
 
 					expect( getData( document ) ).to.equal(
 						`<${ toElement }>foo[bar</${ toElement }><${ toElement }>baz]qux</${ toElement }>`