Преглед на файлове

Aligned command API usage to the changes done in ckeditor5-core#89.

Piotrek Koszuliński преди 8 години
родител
ревизия
76889648e5

+ 15 - 40
packages/ckeditor5-link/src/linkcommand.js

@@ -7,7 +7,7 @@
  * @module link/linkcommand
  */
 
-import Command from '@ckeditor/ckeditor5-core/src/command/command';
+import Command from '@ckeditor/ckeditor5-core/src/command';
 import Text from '@ckeditor/ckeditor5-engine/src/model/text';
 import Range from '@ckeditor/ckeditor5-engine/src/model/range';
 import getSchemaValidRanges from '@ckeditor/ckeditor5-core/src/command/helpers/getschemavalidranges';
@@ -17,49 +17,25 @@ import findLinkRange from './findlinkrange';
 /**
  * The link command. It is used by the {@link module:link/link~Link link feature}.
  *
- * @extends module:core/command/command~Command
+ * @extends module:core/command~Command
  */
 export default class LinkCommand extends Command {
 	/**
-	 * @see module:core/command/command~Command
-	 * @param {module:core/editor/editor~Editor} editor
-	 */
-	constructor( editor ) {
-		super( editor );
-
-		/**
-		 * Currently selected `linkHref` attribute value.
-		 *
-		 * @observable
-		 * @member {Boolean} module:core/command/toggleattributecommand~ToggleAttributeCommand#value
-		 */
-		this.set( 'value', undefined );
-
-		// Checks whether the command should be enabled or disabled.
-		this.listenTo( editor.document, 'changesDone', () => {
-			this.refreshState();
-			this.refreshValue();
-		} );
-	}
-
-	/**
-	 * Updates command's {@link #value} based on the current selection.
+	 * The value of `'linkHref'` attribute if the start of a selection is located in a node with this attribute.
+	 *
+	 * @observable
+	 * @readonly
+	 * @member {Object|undefined} #value
 	 */
-	refreshValue() {
-		this.value = this.editor.document.selection.getAttribute( 'linkHref' );
-	}
 
 	/**
-	 * Checks if {@link module:engine/model/document~Document#schema} allows to create attribute in {@link
-	 * module:engine/model/document~Document#selection}
-	 *
-	 * @protected
-	 * @returns {Boolean}
+	 * @inheritDoc
 	 */
-	_checkEnabled() {
-		const document = this.editor.document;
+	refresh() {
+		const doc = this.editor.document;
 
-		return isAttributeAllowedInSelection( 'linkHref', document.selection, document.schema );
+		this.value = doc.selection.getAttribute( 'linkHref' );
+		this.isEnabled = isAttributeAllowedInSelection( 'linkHref', doc.selection, doc.schema );
 	}
 
 	/**
@@ -75,10 +51,10 @@ export default class LinkCommand extends Command {
 	 *
 	 * When selection is collapsed and inside text with `linkHref` attribute, the attribute value will be updated.
 	 *
-	 * @protected
+	 * @fires execute
 	 * @param {String} href Link destination.
 	 */
-	_doExecute( href ) {
+	execute( href ) {
 		const document = this.editor.document;
 		const selection = document.selection;
 
@@ -89,7 +65,6 @@ export default class LinkCommand extends Command {
 			// If selection is collapsed then update selected link or insert new one at the place of caret.
 			if ( selection.isCollapsed ) {
 				const position = selection.getFirstPosition();
-				const parent = position.parent;
 
 				// When selection is inside text with `linkHref` attribute.
 				if ( selection.hasAttribute( 'linkHref' ) ) {
@@ -102,7 +77,7 @@ export default class LinkCommand extends Command {
 					selection.setRanges( [ linkRange ] );
 				}
 				// If not then insert text node with `linkHref` attribute in place of caret.
-				else if ( document.schema.check( { name: '$text', attributes: 'linkHref', inside: parent.name } ) ) {
+				else {
 					const node = new Text( href, { linkHref: href } );
 
 					batch.insert( position, node );

+ 2 - 2
packages/ckeditor5-link/src/linkengine.js

@@ -47,7 +47,7 @@ export default class LinkEngine extends Plugin {
 			} ) );
 
 		// Create linking commands.
-		editor.commands.set( 'link', new LinkCommand( editor ) );
-		editor.commands.set( 'unlink', new UnlinkCommand( editor ) );
+		editor.commands.add( 'link', new LinkCommand( editor ) );
+		editor.commands.add( 'unlink', new UnlinkCommand( editor ) );
 	}
 }

+ 7 - 23
packages/ckeditor5-link/src/unlinkcommand.js

@@ -7,26 +7,20 @@
  * @module link/unlinkcommand
  */
 
-import Command from '@ckeditor/ckeditor5-core/src/command/command';
+import Command from '@ckeditor/ckeditor5-core/src/command';
 import findLinkRange from './findlinkrange';
 
 /**
  * The unlink command. It is used by the {@link module:link/link~Link link plugin}.
  *
- * @extends module:core/command/command~Command
+ * @extends module:core/command~Command
  */
 export default class UnlinkCommand extends Command {
 	/**
-	 * @see module:core/command/command~Command
-	 * @param {module:core/editor/editor~Editor} editor
+	 * @inheritDoc
 	 */
-	constructor( editor ) {
-		super( editor );
-
-		// Checks whether the command should be enabled or disabled.
-		this.listenTo( editor.document, 'changesDone', () => {
-			this.refreshState();
-		} );
+	refresh() {
+		this.isEnabled = this.editor.document.selection.hasAttribute( 'linkHref' );
 	}
 
 	/**
@@ -35,9 +29,9 @@ export default class UnlinkCommand extends Command {
 	 * When the selection is collapsed, removes `linkHref` attribute from each node with the same `linkHref` attribute value.
 	 * When the selection is non-collapsed, removes `linkHref` from each node in selected ranges.
 	 *
-	 * @protected
+	 * @fires execute
 	 */
-	_doExecute() {
+	execute() {
 		const document = this.editor.document;
 		const selection = document.selection;
 
@@ -55,14 +49,4 @@ export default class UnlinkCommand extends Command {
 			}
 		} );
 	}
-
-	/**
-	 * Checks if selection has `linkHref` attribute.
-	 *
-	 * @protected
-	 * @returns {Boolean}
-	 */
-	_checkEnabled() {
-		return this.editor.document.selection.hasAttribute( 'linkHref' );
-	}
 }

+ 49 - 67
packages/ckeditor5-link/tests/linkcommand.js

@@ -28,28 +28,44 @@ describe( 'LinkCommand', () => {
 	} );
 
 	afterEach( () => {
-		command.destroy();
+		return editor.destroy();
 	} );
 
-	describe( 'constructor()', () => {
-		// https://github.com/ckeditor/ckeditor5-link/issues/93
-		it( 'should listen on document#changesDone and refresh the command\'s state', () => {
-			const refreshStateSpy = sinon.spy( command, 'refreshState' );
+	describe( 'isEnabled', () => {
+		// This test doesn't tests every possible case.
+		// refresh() uses `isAttributeAllowedInSelection` helper which is fully tested in his own test.
 
-			document.fire( 'changesDone' );
+		beforeEach( () => {
+			document.schema.registerItem( 'x', '$block' );
+			document.schema.disallow( { name: '$text', inside: 'x', attributes: 'linkHref' } );
+		} );
 
-			expect( refreshStateSpy.calledOnce ).to.true;
+		describe( 'when selection is collapsed', () => {
+			it( 'should be true if characters with the attribute can be placed at caret position', () => {
+				setData( document, '<p>f[]oo</p>' );
+				expect( command.isEnabled ).to.be.true;
+			} );
+
+			it( 'should be false if characters with the attribute cannot be placed at caret position', () => {
+				setData( document, '<x>fo[]o</x>' );
+				expect( command.isEnabled ).to.be.false;
+			} );
 		} );
-	} );
 
-	describe( 'value', () => {
-		it( 'should be updated on document#changesDone', () => {
-			const spy = sinon.spy( command, 'refreshValue' );
+		describe( 'when selection is not collapsed', () => {
+			it( 'should be true if there is at least one node in selection that can have the attribute', () => {
+				setData( document, '<p>[foo]</p>' );
+				expect( command.isEnabled ).to.be.true;
+			} );
 
-			document.fire( 'changesDone' );
-			sinon.assert.calledOnce( spy );
+			it( 'should be false if there are no nodes in selection that can have the attribute', () => {
+				setData( document, '<x>[foo]</x>' );
+				expect( command.isEnabled ).to.be.false;
+			} );
 		} );
+	} );
 
+	describe( 'value', () => {
 		describe( 'collapsed selection', () => {
 			it( 'should be equal attribute value when selection is placed inside element with `linkHref` attribute', () => {
 				setData( document, '<$text linkHref="url">foo[]bar</$text>' );
@@ -60,7 +76,7 @@ describe( 'LinkCommand', () => {
 			it( 'should be undefined when selection is placed inside element without `linkHref` attribute', () => {
 				setData( document, '<$text bold="true">foo[]bar</$text>' );
 
-				expect( command.value ).to.undefined;
+				expect( command.value ).to.be.undefined;
 			} );
 		} );
 
@@ -74,19 +90,19 @@ describe( 'LinkCommand', () => {
 			it( 'should be undefined when selection contains not only elements with `linkHref` attribute', () => {
 				setData( document, 'f[o<$text linkHref="url">ob</$text>]ar' );
 
-				expect( command.value ).to.undefined;
+				expect( command.value ).to.be.undefined;
 			} );
 		} );
 	} );
 
-	describe( '_doExecute', () => {
+	describe( 'execute()', () => {
 		describe( 'non-collapsed selection', () => {
 			it( 'should set `linkHref` attribute to selected text', () => {
 				setData( document, 'f[ooba]r' );
 
-				expect( command.value ).to.undefined;
+				expect( command.value ).to.be.undefined;
 
-				command._doExecute( 'url' );
+				command.execute( 'url' );
 
 				expect( getData( document ) ).to.equal( 'f[<$text linkHref="url">ooba</$text>]r' );
 				expect( command.value ).to.equal( 'url' );
@@ -95,9 +111,9 @@ describe( 'LinkCommand', () => {
 			it( 'should set `linkHref` attribute to selected text when text already has attributes', () => {
 				setData( document, 'f[o<$text bold="true">oba]r</$text>' );
 
-				expect( command.value ).to.undefined;
+				expect( command.value ).to.be.undefined;
 
-				command._doExecute( 'url' );
+				command.execute( 'url' );
 
 				expect( command.value ).to.equal( 'url' );
 				expect( getData( document ) ).to.equal(
@@ -110,9 +126,9 @@ describe( 'LinkCommand', () => {
 			it( 'should overwrite existing `linkHref` attribute when selected text wraps text with `linkHref` attribute', () => {
 				setData( document, 'f[o<$text linkHref="other url">o</$text>ba]r' );
 
-				expect( command.value ).to.undefined;
+				expect( command.value ).to.be.undefined;
 
-				command._doExecute( 'url' );
+				command.execute( 'url' );
 
 				expect( getData( document ) ).to.equal( 'f[<$text linkHref="url">ooba</$text>]r' );
 				expect( command.value ).to.equal( 'url' );
@@ -123,7 +139,7 @@ describe( 'LinkCommand', () => {
 
 				expect( command.value ).to.equal( 'other url' );
 
-				command._doExecute( 'url' );
+				command.execute( 'url' );
 
 				expect( getData( document ) ).to.equal(
 					'f' +
@@ -142,7 +158,7 @@ describe( 'LinkCommand', () => {
 
 				expect( command.value ).to.equal( 'other url' );
 
-				command._doExecute( 'url' );
+				command.execute( 'url' );
 
 				expect( getData( document ) ).to.equal( 'f<$text linkHref="other url">o</$text>[<$text linkHref="url">oba</$text>]r' );
 				expect( command.value ).to.equal( 'url' );
@@ -152,9 +168,9 @@ describe( 'LinkCommand', () => {
 				'attribute', () => {
 				setData( document, 'f[o<$text linkHref="other url">ob]a</$text>r' );
 
-				expect( command.value ).to.undefined;
+				expect( command.value ).to.be.undefined;
 
-				command._doExecute( 'url' );
+				command.execute( 'url' );
 
 				expect( getData( document ) ).to.equal( 'f[<$text linkHref="url">oob</$text>]<$text linkHref="other url">a</$text>r' );
 				expect( command.value ).to.equal( 'url' );
@@ -163,9 +179,9 @@ describe( 'LinkCommand', () => {
 			it( 'should set `linkHref` attribute to selected text when text is split by $block element', () => {
 				setData( document, '<p>f[oo</p><p>ba]r</p>' );
 
-				expect( command.value ).to.undefined;
+				expect( command.value ).to.be.undefined;
 
-				command._doExecute( 'url' );
+				command.execute( 'url' );
 
 				expect( getData( document ) )
 					.to.equal( '<p>f[<$text linkHref="url">oo</$text></p><p><$text linkHref="url">ba</$text>]r</p>' );
@@ -178,9 +194,9 @@ describe( 'LinkCommand', () => {
 
 				setData( document, '<p>f[oo<img></img>ba]r</p>' );
 
-				expect( command.value ).to.undefined;
+				expect( command.value ).to.be.undefined;
 
-				command._doExecute( 'url' );
+				command.execute( 'url' );
 
 				expect( getData( document ) )
 					.to.equal( '<p>f[<$text linkHref="url">oo</$text><img></img><$text linkHref="url">ba</$text>]r</p>' );
@@ -192,7 +208,7 @@ describe( 'LinkCommand', () => {
 			it( 'should insert text with `linkHref` attribute, text data equal to href and select new link', () => {
 				setData( document, 'foo[]bar' );
 
-				command._doExecute( 'url' );
+				command.execute( 'url' );
 
 				expect( getData( document ) ).to.equal( 'foo[<$text linkHref="url">url</$text>]bar' );
 			} );
@@ -200,7 +216,7 @@ describe( 'LinkCommand', () => {
 			it( 'should update `linkHref` attribute and select whole link when selection is inside text with `linkHref` attribute', () => {
 				setData( document, '<$text linkHref="other url">foo[]bar</$text>' );
 
-				command._doExecute( 'url' );
+				command.execute( 'url' );
 
 				expect( getData( document ) ).to.equal( '[<$text linkHref="url">foobar</$text>]' );
 			} );
@@ -209,44 +225,10 @@ describe( 'LinkCommand', () => {
 				document.schema.disallow( { name: '$text', attributes: 'linkHref', inside: 'p' } );
 				setData( document, '<p>foo[]bar</p>' );
 
-				command._doExecute( 'url' );
+				command.execute( 'url' );
 
 				expect( getData( document ) ).to.equal( '<p>foo[]bar</p>' );
 			} );
 		} );
 	} );
-
-	describe( '_checkEnabled', () => {
-		// This test doesn't tests every possible case.
-		// Method `_checkEnabled` uses `isAttributeAllowedInSelection` helper which is fully tested in his own test.
-
-		beforeEach( () => {
-			document.schema.registerItem( 'x', '$block' );
-			document.schema.disallow( { name: '$text', inside: 'x', attributes: 'linkHref' } );
-		} );
-
-		describe( 'when selection is collapsed', () => {
-			it( 'should return true if characters with the attribute can be placed at caret position', () => {
-				setData( document, '<p>f[]oo</p>' );
-				expect( command._checkEnabled() ).to.be.true;
-			} );
-
-			it( 'should return false if characters with the attribute cannot be placed at caret position', () => {
-				setData( document, '<x>fo[]o</x>' );
-				expect( command._checkEnabled() ).to.be.false;
-			} );
-		} );
-
-		describe( 'when selection is not collapsed', () => {
-			it( 'should return true if there is at least one node in selection that can have the attribute', () => {
-				setData( document, '<p>[foo]</p>' );
-				expect( command._checkEnabled() ).to.be.true;
-			} );
-
-			it( 'should return false if there are no nodes in selection that can have the attribute', () => {
-				setData( document, '<x>[foo]</x>' );
-				expect( command._checkEnabled() ).to.be.false;
-			} );
-		} );
-	} );
 } );

+ 0 - 4
packages/ckeditor5-link/tests/linkengine.js

@@ -36,16 +36,12 @@ describe( 'LinkEngine', () => {
 
 	describe( 'command', () => {
 		it( 'should register link command', () => {
-			expect( editor.commands.has( 'link' ) ).to.be.true;
-
 			const command = editor.commands.get( 'link' );
 
 			expect( command ).to.be.instanceOf( LinkCommand );
 		} );
 
 		it( 'should register unlink command', () => {
-			expect( editor.commands.has( 'unlink' ) ).to.be.true;
-
 			const command = editor.commands.get( 'unlink' );
 
 			expect( command ).to.be.instanceOf( UnlinkCommand );

+ 28 - 34
packages/ckeditor5-link/tests/unlinkcommand.js

@@ -30,25 +30,33 @@ describe( 'UnlinkCommand', () => {
 	} );
 
 	afterEach( () => {
-		command.destroy();
+		return editor.destroy();
 	} );
 
-	describe( 'constructor()', () => {
-		it( 'should listen on document#changesDone and refresh the state', () => {
-			const refreshStateSpy = testUtils.sinon.spy( command, 'refreshState' );
+	describe( 'isEnabled', () => {
+		it( 'should be true when selection has `linkHref` attribute', () => {
+			document.enqueueChanges( () => {
+				document.selection.setAttribute( 'linkHref', 'value' );
+			} );
+
+			expect( command.isEnabled ).to.true;
+		} );
 
-			document.fire( 'changesDone' );
+		it( 'should be false when selection doesn\'t have `linkHref` attribute', () => {
+			document.enqueueChanges( () => {
+				document.selection.removeAttribute( 'linkHref' );
+			} );
 
-			expect( refreshStateSpy.calledOnce ).to.true;
+			expect( command.isEnabled ).to.false;
 		} );
 	} );
 
-	describe( '_doExecute', () => {
+	describe( 'execute()', () => {
 		describe( 'non-collapsed selection', () => {
 			it( 'should remove `linkHref` attribute from selected text', () => {
 				setData( document, '<$text linkHref="url">f[ooba]r</$text>' );
 
-				command._doExecute();
+				command.execute();
 
 				expect( getData( document ) ).to.equal( '<$text linkHref="url">f</$text>[ooba]<$text linkHref="url">r</$text>' );
 			} );
@@ -56,7 +64,7 @@ describe( 'UnlinkCommand', () => {
 			it( 'should remove `linkHref` attribute from selected text and do not modified other attributes', () => {
 				setData( document, '<$text bold="true" linkHref="url">f[ooba]r</$text>' );
 
-				command._doExecute();
+				command.execute();
 
 				expect( getData( document ) ).to.equal(
 					'<$text bold="true" linkHref="url">f</$text>' +
@@ -68,7 +76,7 @@ describe( 'UnlinkCommand', () => {
 			it( 'should remove `linkHref` attribute from selected text when attributes have different value', () => {
 				setData( document, '[<$text linkHref="url">foo</$text><$text linkHref="other url">bar</$text>]' );
 
-				command._doExecute();
+				command.execute();
 
 				expect( getData( document ) ).to.equal( '[foobar]' );
 			} );
@@ -76,7 +84,7 @@ describe( 'UnlinkCommand', () => {
 			it( 'should remove `linkHref` attribute from selection', () => {
 				setData( document, '<$text linkHref="url">f[ooba]r</$text>' );
 
-				command._doExecute();
+				command.execute();
 
 				expect( document.selection.hasAttribute( 'linkHref' ) ).to.false;
 			} );
@@ -86,7 +94,7 @@ describe( 'UnlinkCommand', () => {
 			it( 'should remove `linkHref` attribute from selection siblings with the same attribute value', () => {
 				setData( document, '<$text linkHref="url">foo[]bar</$text>' );
 
-				command._doExecute();
+				command.execute();
 
 				expect( getData( document ) ).to.equal( 'foo[]bar' );
 			} );
@@ -100,7 +108,7 @@ describe( 'UnlinkCommand', () => {
 					'<$text linkHref="other url">ar</$text>'
 				);
 
-				command._doExecute();
+				command.execute();
 
 				expect( getData( document ) ).to.equal(
 					'<$text linkHref="other url">fo</$text>' +
@@ -120,7 +128,7 @@ describe( 'UnlinkCommand', () => {
 					'<$text linkHref="same url">r</$text>'
 				);
 
-				command._doExecute();
+				command.execute();
 
 				expect( getData( document ) )
 					.to.equal(
@@ -145,7 +153,7 @@ describe( 'UnlinkCommand', () => {
 					'<$text linkHref="url">r</$text>'
 				);
 
-				command._doExecute();
+				command.execute();
 
 				expect( getData( document ) ).to.equal(
 					'f' +
@@ -164,7 +172,7 @@ describe( 'UnlinkCommand', () => {
 					'<p><$text linkHref="url">bar</$text></p>'
 				);
 
-				command._doExecute();
+				command.execute();
 
 				expect( getData( document ) ).to.equal(
 					'<p><$text linkHref="url">bar</$text></p>' +
@@ -176,7 +184,7 @@ describe( 'UnlinkCommand', () => {
 			it( 'should remove `linkHref` attribute from selection siblings when selection is at the end of link', () => {
 				setData( document, '<$text linkHref="url">foobar</$text>[]' );
 
-				command._doExecute();
+				command.execute();
 
 				expect( getData( document ) ).to.equal( 'foobar[]' );
 			} );
@@ -184,7 +192,7 @@ describe( 'UnlinkCommand', () => {
 			it( 'should remove `linkHref` attribute from selection siblings when selection is at the beginning of link', () => {
 				setData( document, '[]<$text linkHref="url">foobar</$text>' );
 
-				command._doExecute();
+				command.execute();
 
 				expect( getData( document ) ).to.equal( '[]foobar' );
 			} );
@@ -194,7 +202,7 @@ describe( 'UnlinkCommand', () => {
 			() => {
 				setData( document, '<$text linkHref="url">foo</$text>[]<$text linkHref="other url">bar</$text>' );
 
-				command._doExecute();
+				command.execute();
 
 				expect( getData( document ) ).to.equal( 'foo[]<$text linkHref="other url">bar</$text>' );
 			} );
@@ -202,24 +210,10 @@ describe( 'UnlinkCommand', () => {
 			it( 'should remove `linkHref` attribute from selection', () => {
 				setData( document, '<$text linkHref="url">foo[]bar</$text>' );
 
-				command._doExecute();
+				command.execute();
 
 				expect( document.selection.hasAttribute( 'linkHref' ) ).to.false;
 			} );
 		} );
 	} );
-
-	describe( '_checkEnabled', () => {
-		it( 'should return false when selection has `linkHref` attribute', () => {
-			document.selection.setAttribute( 'linkHref', 'value' );
-
-			expect( command._checkEnabled() ).to.true;
-		} );
-
-		it( 'should return false when selection doesn\'t have `linkHref` attribute', () => {
-			document.selection.removeAttribute( 'linkHref' );
-
-			expect( command._checkEnabled() ).to.false;
-		} );
-	} );
 } );