浏览代码

Changed: Make `FontCommand` work also on collapsed selection.

Maciej Gołaszewski 8 年之前
父节点
当前提交
27955ecff7

+ 20 - 17
packages/ckeditor5-font/src/fontcommand.js

@@ -19,13 +19,13 @@ export default class FontCommand extends Command {
 	 * Creates an instance of the command.
 	 *
 	 * @param {module:core/editor/editor~Editor} editor Editor instance.
-	 * @param {String} attribute Name of an model attribute on which this command operates.
+	 * @param {String} attributeKey Name of an model attribute on which this command operates.
 	 */
-	constructor( editor, attribute ) {
+	constructor( editor, attributeKey ) {
 		super( editor );
 
 		/**
-		 * If is set it means that selection has `attribute` set.
+		 * If is set it means that selection has `attributeKey` set to that value.
 		 *
 		 * @observable
 		 * @readonly
@@ -36,9 +36,9 @@ export default class FontCommand extends Command {
 		 * A model attribute on which this command operates.
 		 *
 		 * @readonly
-		 * @member {Boolean} module:font/fontcommand~FontCommand#attribute
+		 * @member {Boolean} module:font/fontcommand~FontCommand#attributeKey
 		 */
-		this.attribute = attribute;
+		this.attributeKey = attributeKey;
 	}
 
 	/**
@@ -48,8 +48,8 @@ export default class FontCommand extends Command {
 		const model = this.editor.model;
 		const doc = model.document;
 
-		this.value = doc.selection.getAttribute( this.attribute );
-		this.isEnabled = model.schema.checkAttributeInSelection( doc.selection, this.attribute );
+		this.value = doc.selection.getAttribute( this.attributeKey );
+		this.isEnabled = model.schema.checkAttributeInSelection( doc.selection, this.attributeKey );
 	}
 
 	/**
@@ -66,21 +66,24 @@ export default class FontCommand extends Command {
 		const document = model.document;
 		const selection = document.selection;
 
-		// Do not apply value on collapsed selection.
-		if ( selection.isCollapsed ) {
-			return;
-		}
-
 		const value = options.value;
 
 		model.change( writer => {
-			const ranges = model.schema.getValidRanges( selection.getRanges(), this.attribute );
-
-			for ( const range of ranges ) {
+			if ( selection.isCollapsed ) {
 				if ( value ) {
-					writer.setAttribute( this.attribute, value, range );
+					selection.setAttribute( this.attributeKey, value );
 				} else {
-					writer.removeAttribute( this.attribute, range );
+					selection.removeAttribute( this.attributeKey );
+				}
+			} else {
+				const ranges = model.schema.getValidRanges( selection.getRanges(), this.attributeKey );
+
+				for ( const range of ranges ) {
+					if ( value ) {
+						writer.setAttribute( this.attributeKey, value, range );
+					} else {
+						writer.removeAttribute( this.attributeKey, range );
+					}
 				}
 			}
 		} );

+ 150 - 14
packages/ckeditor5-font/tests/fontcommand.js

@@ -8,20 +8,29 @@ import FontCommand from '../src/fontcommand';
 import Command from '@ckeditor/ckeditor5-core/src/command';
 import ModelTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/modeltesteditor';
 import { getData, setData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model';
+import Range from '../../ckeditor5-engine/src/model/range';
+import Position from '../../ckeditor5-engine/src/model/position';
 
 describe( 'FontCommand', () => {
-	let editor, model, command;
+	let editor, model, doc, root, command;
 
 	beforeEach( () => {
 		return ModelTestEditor.create()
 			.then( newEditor => {
 				editor = newEditor;
 				model = editor.model;
+				doc = model.document;
+				root = doc.getRoot();
 
 				command = new FontCommand( editor, 'font' );
 				editor.commands.add( 'font', command );
 
 				model.schema.register( 'paragraph', { inheritAllFrom: '$block' } );
+				model.schema.register( 'img', {
+					allowWhere: [ '$block', '$text' ],
+					isObject: true
+				} );
+
 				model.schema.extend( '$text', { allowAttributes: 'font' } );
 			} );
 	} );
@@ -58,6 +67,16 @@ describe( 'FontCommand', () => {
 	} );
 
 	describe( 'execute()', () => {
+		it( 'should do nothing if the command is disabled', () => {
+			setData( model, '<paragraph>fo[ob]ar</paragraph>' );
+
+			command.isEnabled = false;
+
+			command.execute( { value: 'foo' } );
+
+			expect( getData( model ) ).to.equal( '<paragraph>fo[ob]ar</paragraph>' );
+		} );
+
 		it( 'should add font attribute on selected text', () => {
 			setData( model, '<paragraph>a[bc<$text font="foo">fo]obar</$text>xyz</paragraph>' );
 
@@ -108,19 +127,7 @@ describe( 'FontCommand', () => {
 			);
 		} );
 
-		it( 'should do nothing on collapsed range', () => {
-			setData( model, '<paragraph>abc<$text font="foo">foo[]bar</$text>xyz</paragraph>' );
-
-			expect( command.value ).to.equal( 'foo' );
-
-			command.execute( { value: 'foo' } );
-
-			expect( getData( model ) ).to.equal( '<paragraph>abc<$text font="foo">foo[]bar</$text>xyz</paragraph>' );
-
-			expect( command.value ).to.equal( 'foo' );
-		} );
-
-		it( 'should remove font attribute on selected nodes when passing undefined font param', () => {
+		it( 'should remove font attribute on selected nodes when passing undefined value', () => {
 			setData(
 				model,
 				'<paragraph>abcabc[<$text font="foo">abc</$text></paragraph>' +
@@ -139,5 +146,134 @@ describe( 'FontCommand', () => {
 				'<paragraph>barbar]bar</paragraph>'
 			);
 		} );
+
+		it( 'should change selection attribute if selection is collapsed in non-empty parent', () => {
+			setData( model, '<paragraph>a[]bc<$text font="foo">foobar</$text>xyz</paragraph><paragraph></paragraph>' );
+
+			expect( command.value ).to.be.undefined;
+
+			command.execute( { value: 'foo' } );
+
+			expect( command.value ).to.equal( 'foo' );
+			expect( doc.selection.hasAttribute( 'font' ) ).to.be.true;
+
+			command.execute();
+
+			expect( command.value ).to.be.undefined;
+			expect( doc.selection.hasAttribute( 'font' ) ).to.be.false;
+		} );
+
+		it( 'should not store attribute change on selection if selection is collapsed in non-empty parent', () => {
+			setData( model, '<paragraph>a[]bc<$text font="foo">foobar</$text>xyz</paragraph>' );
+
+			command.execute( { value: 'foo' } );
+
+			// It should not save that bold was executed at position ( root, [ 0, 1 ] ).
+
+			model.change( () => {
+				// Simulate clicking right arrow key by changing selection ranges.
+				doc.selection.setRanges( [ new Range( new Position( root, [ 0, 2 ] ), new Position( root, [ 0, 2 ] ) ) ] );
+
+				// Get back to previous selection.
+				doc.selection.setRanges( [ new Range( new Position( root, [ 0, 1 ] ), new Position( root, [ 0, 1 ] ) ) ] );
+			} );
+
+			expect( command.value ).to.be.undefined;
+		} );
+
+		it( 'should change selection attribute and store it if selection is collapsed in empty parent', () => {
+			setData( model, '<paragraph>abc<$text font="foo">foobar</$text>xyz</paragraph><paragraph>[]</paragraph>' );
+
+			expect( command.value ).to.be.undefined;
+
+			command.execute( { value: 'foo' } );
+
+			expect( command.value ).to.equal( 'foo' );
+			expect( doc.selection.hasAttribute( 'font' ) ).to.be.true;
+
+			// Attribute should be stored.
+			// Simulate clicking somewhere else in the editor.
+			model.change( () => {
+				doc.selection.setRanges( [ new Range( new Position( root, [ 0, 2 ] ), new Position( root, [ 0, 2 ] ) ) ] );
+			} );
+
+			expect( command.value ).to.be.undefined;
+
+			// Go back to where attribute was stored.
+			model.change( () => {
+				doc.selection.setRanges( [ new Range( new Position( root, [ 1, 0 ] ), new Position( root, [ 1, 0 ] ) ) ] );
+			} );
+
+			// Attribute should be restored.
+			expect( command.value ).to.equal( 'foo' );
+
+			command.execute();
+
+			expect( command.value ).to.be.undefined;
+			expect( doc.selection.hasAttribute( 'font' ) ).to.be.false;
+		} );
+
+		it( 'should not apply attribute change where it would invalid schema', () => {
+			model.schema.register( 'image', { inheritAllFrom: '$block' } );
+			setData( model, '<paragraph>ab[c<img></img><$text font="foo">foobar</$text>xy<img></img>]z</paragraph>' );
+
+			expect( command.isEnabled ).to.be.true;
+
+			command.execute( { value: 'foo' } );
+
+			expect( getData( model ) ).to.equal(
+				'<paragraph>ab[<$text font="foo">c</$text><img></img><$text font="foo">foobarxy</$text><img></img>]z</paragraph>'
+			);
+		} );
+
+		it( 'should use parent batch for storing undo steps', () => {
+			setData( model, '<paragraph>a[bc<$text font="foo">fo]obar</$text>xyz</paragraph>' );
+
+			model.change( writer => {
+				expect( writer.batch.deltas.length ).to.equal( 0 );
+				command.execute( { value: 'foo' } );
+				expect( writer.batch.deltas.length ).to.equal( 1 );
+			} );
+
+			expect( getData( model ) ).to.equal( '<paragraph>a[<$text font="foo">bcfo]obar</$text>xyz</paragraph>' );
+		} );
+
+		describe( 'should cause firing model change event', () => {
+			let spy;
+
+			beforeEach( () => {
+				spy = sinon.spy();
+			} );
+
+			it( 'collapsed selection in non-empty parent', () => {
+				setData( model, '<paragraph>x[]y</paragraph>' );
+
+				model.document.on( 'change', spy );
+
+				command.execute( { value: 'foo' } );
+
+				expect( spy.called ).to.be.true;
+			} );
+
+			it( 'non-collapsed selection', () => {
+				setData( model, '<paragraph>[xy]</paragraph>' );
+
+				model.document.on( 'change', spy );
+
+				command.execute( { value: 'foo' } );
+
+				expect( spy.called ).to.be.true;
+			} );
+
+			it( 'in empty parent', () => {
+				setData( model, '<paragraph>[]</paragraph>' );
+
+				model.document.on( 'change', spy );
+
+				command.execute( { value: 'foo' } );
+
+				expect( spy.called ).to.be.true;
+			} );
+		} );
 	} );
 } );

+ 2 - 2
packages/ckeditor5-font/tests/fontfamily/fontfamilycommand.js

@@ -29,7 +29,7 @@ describe( 'FontFamilyCommand', () => {
 		expect( command ).to.be.instanceOf( FontCommand );
 	} );
 
-	it( 'opeartes on fontFamily attribute', () => {
-		expect( command ).to.have.property( 'attribute', 'fontFamily' );
+	it( 'operates on fontFamily attribute', () => {
+		expect( command ).to.have.property( 'attributeKey', 'fontFamily' );
 	} );
 } );

+ 2 - 2
packages/ckeditor5-font/tests/fontsize/fontsizecommand.js

@@ -29,7 +29,7 @@ describe( 'FontSizeCommand', () => {
 		expect( command ).to.be.instanceOf( FontCommand );
 	} );
 
-	it( 'opeartes on fontSize attribute', () => {
-		expect( command ).to.have.property( 'attribute', 'fontSize' );
+	it( 'operates on fontSize attribute', () => {
+		expect( command ).to.have.property( 'attributeKey', 'fontSize' );
 	} );
 } );