Ver Fonte

Provided Command with default implementation of refresh() and changed default values of command properties to better match how commands usually look.

Piotrek Koszuliński há 8 anos atrás
pai
commit
3c2423261d

+ 16 - 3
packages/ckeditor5-core/src/command.js

@@ -38,8 +38,8 @@ export default class Command {
 		 */
 		this.editor = editor;
 
-		this.set( 'value', null );
-		this.set( 'isEnabled', false );
+		this.set( 'value', undefined );
+		this.set( 'isEnabled', true );
 
 		this.decorate( 'execute' );
 
@@ -50,7 +50,14 @@ export default class Command {
 	}
 
 	/**
-	 * Destroys the command.
+	 * @inheritDoc
+	 */
+	refresh() {
+		this.isEnabled = true;
+	}
+
+	/**
+	 * @inheritDoc
 	 */
 	destroy() {
 		this.stopListening();
@@ -105,6 +112,12 @@ mix( Command, ObservableMixin );
  * @method #refresh
  */
 
+/**
+ * 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.

+ 35 - 5
packages/ckeditor5-core/tests/command.js

@@ -8,8 +8,6 @@ import ModelTestEditor from './_utils/modeltesteditor';
 
 class SomeCommand extends Command {
 	execute() {}
-
-	refresh() {}
 }
 
 describe( 'Command', () => {
@@ -36,8 +34,8 @@ describe( 'Command', () => {
 		} );
 
 		it( 'sets the state properties', () => {
-			expect( command.value ).to.be.null;
-			expect( command.isEnabled ).to.be.false;
+			expect( command.value ).to.be.undefined;
+			expect( command.isEnabled ).to.be.true;
 		} );
 
 		it( 'adds a listener which refreshed the command on editor.document#changesDone', () => {
@@ -67,7 +65,7 @@ describe( 'Command', () => {
 
 			command.on( 'change:isEnabled', spy );
 
-			command.isEnabled = true;
+			command.isEnabled = false;
 
 			expect( spy.calledOnce ).to.be.true;
 		} );
@@ -85,4 +83,36 @@ describe( 'Command', () => {
 			expect( spy.args[ 0 ][ 1 ] ).to.deep.equal( [ 1, 2 ] );
 		} );
 	} );
+
+	describe( 'refresh()', () => {
+		it( 'sets isEnabled to true', () => {
+			command.isEnabled = false;
+
+			command.refresh();
+
+			expect( command.isEnabled ).to.be.true;
+		} );
+
+		// This is an acceptance test for the ability to override a command's state from outside
+		// in a way that at any moment the action can be reverted by just offing the listener and
+		// refreshing the command once again.
+		it( 'is safely overridable using change:isEnabled', () => {
+			command.on( 'change:isEnabled', callback, { priority: 'high' } );
+			command.isEnabled = false;
+			command.refresh();
+
+			expect( command.isEnabled ).to.be.false;
+
+			command.off( 'change:isEnabled', callback );
+			command.refresh();
+
+			expect( command.isEnabled ).to.be.true;
+
+			function callback( evt ) {
+				command.isEnabled = false;
+
+				evt.stop();
+			}
+		} );
+	} );
 } );