Jelajahi Sumber

Implemented automatic command execution blocking and got rid of unnecesary CommandInterface.

Piotrek Koszuliński 8 tahun lalu
induk
melakukan
2fa0959369

+ 59 - 67
packages/ckeditor5-core/src/command.js

@@ -18,9 +18,8 @@ import mix from '@ckeditor/ckeditor5-utils/src/mix';
  * the {@link module:core/editor/editor~Editor editor} instance.
  *
  * Instances of registered commands can be retrieved from {@link module:core/editor/editor~Editor#commands}.
- * The shortest way to execute a command is through {@link module:core/editor/editor~Editor#execute}.
+ * The easiest way to execute a command is through {@link module:core/editor/editor~Editor#execute}.
  *
- * @implements module:core/command~CommandInterface
  * @mixes module:utils/observablemixin~ObservableMixin
  */
 export default class Command {
@@ -38,7 +37,28 @@ export default class Command {
 		 */
 		this.editor = editor;
 
+		/**
+		 * The value of a command. Concrete command class should define what it represents.
+		 *
+		 * For example, the `bold` command's value is whether the selection starts in a bolded text.
+		 * And the value of the `link` command may be an object with links details.
+		 *
+		 * It's possible for a command to have no value (e.g. for stateless actions such as `uploadImage`).
+		 *
+		 * @observable
+		 * @readonly
+		 * @member #value
+		 */
 		this.set( 'value', undefined );
+
+		/**
+		 * Flag indicating whether a command is enabled or disabled.
+		 * A disabled command should do nothing when executed.
+		 *
+		 * @observable
+		 * @readonly
+		 * @member {Boolean} #isEnabled
+		 */
 		this.set( 'isEnabled', true );
 
 		this.decorate( 'execute' );
@@ -47,84 +67,56 @@ export default class Command {
 		this.listenTo( this.editor.document, 'changesDone', () => {
 			this.refresh();
 		} );
+
+		this.on( 'execute', evt => {
+			if ( !this.isEnabled ) {
+				evt.stop();
+			}
+		}, { priority: 'high' } );
 	}
 
 	/**
-	 * @inheritDoc
+	 * Refreshes the command. The command should update its {@link #isEnabled} and {@link #value} property
+	 * in this method.
+	 *
+	 * This method is automatically called when
+	 * {@link module:engine/model/document~Document#event:changesDone any changes are applied to the model}.
 	 */
 	refresh() {
 		this.isEnabled = true;
 	}
 
 	/**
-	 * @inheritDoc
+	 * Destroys the command.
 	 */
 	destroy() {
 		this.stopListening();
 	}
-}
-
-mix( Command, ObservableMixin );
-
-/**
- * The command interface. Usually implemented by inheriting from the {@link module:core/command~Command base `Command` class}.
- *
- * @interface CommandInterface
- */
-
-/**
- * Flag indicating whether a command is enabled or disabled.
- * A disabled command should do nothing when executed.
- *
- * @observable
- * @readonly
- * @member {Boolean} #isEnabled
- */
-
-/**
- * The value of a command. Concrete command class should define what it represents.
- *
- * For example, the `bold` command's value is whether the selection starts in a bolded text.
- * And the value of the `link` command may be an object with links details.
- *
- * It's possible for a command to have no value (e.g. for stateless actions such as `uploadImage`).
- *
- * @observable
- * @readonly
- * @member #value
- */
 
-/**
- * Executes the command.
- *
- * A command may accept parameters. They will be passed from {@link module:core/editor/editor~Editor#execute}
- * to the command.
- *
- * The `execute()` method should abort when the command is disabled ({@link #isEnabled} is `false`).
- *
- * @method #execute
- */
-
-/**
- * Refreshes the command. The command should update its {@link #isEnabled} and {@link #value} property
- * in this method.
- *
- * @method #refresh
- */
+	/**
+	 * Executes the command.
+	 *
+	 * A command may accept parameters. They will be passed from {@link module:core/editor/editor~Editor#execute}
+	 * to the command.
+	 *
+	 * The `execute()` method will automatically abort when the command is disabled ({@link #isEnabled} is `false`).
+	 * This behavior is implemented by a high priority listener to the {@link #event:execute} event.
+	 *
+	 * @fires execute
+	 * @method #execute
+	 */
 
-/**
- * Destroys the command.
- *
- * @method #destroy
- */
+	/**
+	 * Event fired by the {@link #execute} method. The command action is a listener to this event so it's
+	 * possible to change/cancel the behavior of the command by listening to this event.
+	 *
+	 * See {@link module:utils/observablemixin~ObservableMixin.decorate} for more information and samples.
+	 *
+	 * **Note:** This event is fired even if command is disabled. However, it is automatically blocked
+	 * by a high priority listener in order to prevent command execution.
+	 *
+	 * @event execute
+	 */
+}
 
-/**
- * Event fired by the {@link #execute} method. The command action is a listener to this event so it's
- * possible to change/cancel the behavior of the command by listening to this event.
- *
- * See {@link module:utils/observablemixin~ObservableMixin.decorate} for more information and samples.
- *
- * **Note:** This event is fired even if command is disabled.
- *
- * @event execute
- */
+mix( Command, ObservableMixin );

+ 0 - 4
packages/ckeditor5-core/src/command/toggleattributecommand.js

@@ -78,10 +78,6 @@ export default class ToggleAttributeCommand extends Command {
 	 * @param {module:engine/model/batch~Batch} [options.batch] Batch to group undo steps.
 	 */
 	execute( options = {} ) {
-		if ( !this.isEnabled ) {
-			return;
-		}
-
 		const doc = this.editor.document;
 		const selection = doc.selection;
 		const value = ( options.forceValue === undefined ) ? !this.value : options.forceValue;

+ 3 - 3
packages/ckeditor5-core/src/commandcollection.js

@@ -30,7 +30,7 @@ export default class CommandCollection {
 	 * Registers a new command.
 	 *
 	 * @param {String} commandName The name of the command.
-	 * @param {module:core/command~CommandInterface} command
+	 * @param {module:core/command~Command} command
 	 */
 	add( commandName, command ) {
 		this._commands.set( commandName, command );
@@ -40,7 +40,7 @@ export default class CommandCollection {
 	 * Retrieves a command from the collection.
 	 *
 	 * @param {String} commandName The name of the command.
-	 * @returns {module:core/command~CommandInterface}
+	 * @returns {module:core/command~Command}
 	 */
 	get( commandName ) {
 		return this._commands.get( commandName );
@@ -79,7 +79,7 @@ export default class CommandCollection {
 	/**
 	 * Returns iterator of command instances.
 	 *
-	 * @returns {Iterator.<module:core/command~CommandInterface>}
+	 * @returns {Iterator.<module:core/command~Command>}
 	 */
 	* commands() {
 		yield* this._commands.values();

+ 1 - 1
packages/ckeditor5-core/src/editor/editor.js

@@ -162,7 +162,7 @@ export default class Editor {
 	 *
 	 * Shorthand for:
 	 *
-	 *		editor.commands.get( commandName ).execute( ... )
+	 *		editor.commands.get( commandName ).execute( ... );
 	 *
 	 * @param {String} commandName Name of command to execute.
 	 * @param {*} [...commandParams] Command parameters.

+ 25 - 0
packages/ckeditor5-core/tests/command.js

@@ -82,6 +82,31 @@ describe( 'Command', () => {
 			expect( spy.calledOnce ).to.be.true;
 			expect( spy.args[ 0 ][ 1 ] ).to.deep.equal( [ 1, 2 ] );
 		} );
+
+		it( 'is automatically blocked (with low priority listener) if command is disabled', () => {
+			const spyExecute = sinon.spy();
+			const spyHighest = sinon.spy();
+			const spyHigh = sinon.spy();
+
+			class SpyCommand extends Command {
+				execute() {
+					spyExecute();
+				}
+			}
+
+			const command = new SpyCommand( editor );
+
+			command.on( 'execute', spyHighest, { priority: 'highest' } );
+			command.on( 'execute', spyHigh, { priority: 'high' } );
+
+			command.isEnabled = false;
+
+			command.execute();
+
+			expect( spyExecute.called ).to.be.false;
+			expect( spyHighest.calledOnce ).to.be.true;
+			expect( spyHigh.called ).to.be.false;
+		} );
 	} );
 
 	describe( 'refresh()', () => {