8
0
Просмотр исходного кода

Merge branch 'master' into t/ckeditor5/456

Piotrek Koszuliński 8 лет назад
Родитель
Сommit
c4842b3fbf

+ 1 - 0
packages/ckeditor5-heading/package.json

@@ -15,6 +15,7 @@
     "@ckeditor/ckeditor5-dev-lint": "^3.0.0",
     "@ckeditor/ckeditor5-enter": "^0.9.1",
     "@ckeditor/ckeditor5-editor-classic": "^0.7.3",
+    "@ckeditor/ckeditor5-image": "^0.6.0",
     "@ckeditor/ckeditor5-typing": "^0.9.1",
     "@ckeditor/ckeditor5-undo": "^0.8.1",
     "eslint-config-ckeditor5": "^1.0.0",

+ 12 - 3
packages/ckeditor5-heading/src/heading.js

@@ -116,14 +116,14 @@ export default class Heading extends Plugin {
 
 	/**
 	 * Returns heading options as defined in `config.heading.options` but processed to consider
-	 * editor localization, i.e. to display {@link module:heading/headingcommand~HeadingOption}
+	 * editor localization, i.e. to display {@link module:heading/heading~HeadingOption}
 	 * in the correct language.
 	 *
 	 * Note: The reason behind this method is that there's no way to use {@link module:utils/locale~Locale#t}
 	 * when the user config is defined because the editor does not exist yet.
 	 *
 	 * @private
-	 * @returns {Array.<module:heading/headingcommand~HeadingOption>}.
+	 * @returns {Array.<module:heading/heading~HeadingOption>}.
 	 */
 	_getLocalizedOptions() {
 		const editor = this.editor;
@@ -153,9 +153,18 @@ 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 ) {
 	return Array.prototype.concat( ...commands.map( c => [ c, attribute ] ) );
 }
+
+/**
+ * Heading option descriptor.
+ *
+ * @typedef {Object} module:heading/heading~HeadingOption
+ * @property {String} modelElement Element's name in the model.
+ * @property {String} viewElement The name of the view element that will be used to represent the model element in the view.
+ * @property {String} title The user-readable title of the option.
+ */

+ 41 - 66
packages/ckeditor5-heading/src/headingcommand.js

@@ -7,8 +7,9 @@
  * @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';
@@ -16,70 +17,56 @@ 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 {
 	/**
 	 * Creates an instance of the command.
 	 *
 	 * @param {module:core/editor/editor~Editor} editor Editor instance.
-	 * @param {module:heading/headingcommand~HeadingOption} option An option to be used by the command instance.
+	 * @param {String} modelElement Name of the element which this command will apply in the model.
 	 */
-	constructor( editor, option ) {
+	constructor( editor, modelElement ) {
 		super( editor );
 
-		Object.assign( this, option );
-
 		/**
-		 * Value of the command, indicating whether it is applied in the context
-		 * of current {@link module:engine/model/document~Document#selection selection}.
+		 * Whether the selection starts in a heading of {@link #modelElement this level}.
 		 *
-		 * @readonly
 		 * @observable
-		 * @member {Boolean}
+		 * @readonly
+		 * @member {Boolean} #value
 		 */
-		this.set( 'value', false );
-
-		// Update current value each time changes are done on document.
-		this.listenTo( editor.document, 'changesDone', () => {
-			this.refreshValue();
-			this.refreshState();
-		} );
 
 		/**
 		 * Unique identifier of the command, also element's name in the model.
-		 * See {@link module:heading/headingcommand~HeadingOption}.
+		 * See {@link module:heading/heading~HeadingOption}.
 		 *
 		 * @readonly
-		 * @member {String} #modelElement
+		 * @member {String}
 		 */
+		this.modelElement = modelElement;
+	}
 
-		/**
-		 * Element this command creates in the view.
-		 * See {@link module:heading/headingcommand~HeadingOption}.
-		 *
-		 * @readonly
-		 * @member {String} #viewElement
-		 */
+	/**
+	 * @inheritDoc
+	 */
+	refresh() {
+		const block = first( this.editor.document.selection.getSelectedBlocks() );
 
-		/**
-		 * User-readable title of the command.
-		 * See {@link module:heading/headingcommand~HeadingOption}.
-		 *
-		 * @readonly
-		 * @member {String} #title
-		 */
+		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;
 
@@ -88,8 +75,12 @@ export default class HeadingCommand extends Command {
 
 		document.enqueueChanges( () => {
 			const batch = options.batch || document.batch();
+			const blocks = Array.from( document.selection.getSelectedBlocks() )
+				.filter( block => {
+					return checkCanBecomeHeading( block, this.modelElement, document.schema );
+				} );
 
-			for ( const block of document.selection.getSelectedBlocks() ) {
+			for ( const block of blocks ) {
 				// When removing applied option.
 				if ( shouldRemove ) {
 					if ( block.is( this.modelElement ) ) {
@@ -109,34 +100,18 @@ 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 block = first( this.editor.document.selection.getSelectedBlocks() );
-
-		return !!block && this.editor.document.schema.check( {
-			name: this.modelElement,
-			inside: Position.createBefore( block )
-		} );
-	}
 }
 
-/**
- * Heading option descriptor.
- *
- * @typedef {Object} module:heading/headingcommand~HeadingOption
- * @property {String} modelElement Element's name in the model.
- * @property {String} viewElement The name of the view element that will be used to represent the model element in the view.
- * @property {String} title The user-readable title of the option.
- */
+// Checks whether the given block can be replaced by a specific heading.
+//
+// @private
+// @param {module:engine/model/element~Element} block A block to be tested.
+// @param {module:heading/headingcommand~HeadingCommand#modelElement} heading Command element name in the model.
+// @param {module:engine/model/schema~Schema} schema The schema of the document.
+// @returns {Boolean}
+function checkCanBecomeHeading( block, heading, schema ) {
+	return schema.check( {
+		name: heading,
+		inside: Position.createBefore( block )
+	} );
+}

+ 1 - 1
packages/ckeditor5-heading/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 ) );
+				editor.commands.add( option.modelElement, new HeadingCommand( editor, option.modelElement ) );
 			}
 		}
 	}

+ 43 - 26
packages/ckeditor5-heading/tests/headingcommand.js

@@ -25,11 +25,11 @@ 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 ) {
-				commands[ option.modelElement ] = new HeadingCommand( editor, option );
+				commands[ option.modelElement ] = new HeadingCommand( editor, option.modelElement );
 				schema.registerItem( option.modelElement, '$block' );
 			}
 
@@ -47,18 +47,10 @@ describe( 'HeadingCommand', () => {
 		}
 	} );
 
-	describe( 'basic properties', () => {
-		for ( const option of options ) {
-			test( option );
-		}
-
-		function test( { modelElement, viewElement, title } ) {
-			it( `are set for option.modelElement = ${ modelElement }`, () => {
-				expect( commands[ modelElement ].modelElement ).to.equal( modelElement );
-				expect( commands[ modelElement ].viewElement ).to.equal( viewElement );
-				expect( commands[ modelElement ].title ).to.equal( title );
-			} );
-		}
+	describe( 'modelElement', () => {
+		it( 'is set', () => {
+			expect( commands.heading1.modelElement ).to.equal( 'heading1' );
+		} );
 	} );
 
 	describe( 'value', () => {
@@ -92,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 );
@@ -101,23 +93,48 @@ 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;
 		} );
 
+		// https://github.com/ckeditor/ckeditor5-heading/issues/73
+		it( 'should not rename blocks which cannot become headings', () => {
+			document.schema.registerItem( 'restricted' );
+			document.schema.allow( { name: 'restricted', inside: '$root' } );
+			document.schema.disallow( { name: 'heading1', inside: 'restricted' } );
+
+			document.schema.registerItem( 'fooBlock', '$block' );
+			document.schema.allow( { name: 'fooBlock', inside: 'restricted' } );
+
+			setData(
+				document,
+				'<paragraph>a[bc</paragraph>' +
+				'<restricted><fooBlock></fooBlock></restricted>' +
+				'<paragraph>de]f</paragraph>'
+			);
+
+			commands.heading1.execute();
+
+			expect( getData( document ) ).to.equal(
+				'<heading1>a[bc</heading1>' +
+				'<restricted><fooBlock></fooBlock></restricted>' +
+				'<heading1>de]f</heading1>'
+			);
+		} );
+
 		describe( 'custom options', () => {
 			it( 'should use provided batch', () => {
 				const batch = editor.document.batch();
@@ -127,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 );
 			} );
@@ -140,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 );
 			} );
@@ -158,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>' );
 			} );
@@ -168,7 +185,7 @@ 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>' );
 			} );
@@ -176,7 +193,7 @@ describe( 'HeadingCommand', () => {
 			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 }>` );
 				} );
@@ -193,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>'
@@ -202,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>]'
@@ -216,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 }>`

+ 2 - 22
packages/ckeditor5-heading/tests/headingengine.js

@@ -8,7 +8,6 @@ import HeadingCommand from '../src/headingcommand';
 import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph';
 import ParagraphCommand from '@ckeditor/ckeditor5-paragraph/src/paragraphcommand';
 import VirtualTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/virtualtesteditor';
-import Enter from '@ckeditor/ckeditor5-enter/src/enter';
 import { getData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model';
 
 describe( 'HeadingEngine', () => {
@@ -16,7 +15,7 @@ describe( 'HeadingEngine', () => {
 
 	beforeEach( () => {
 		return VirtualTestEditor.create( {
-			plugins: [ Enter, HeadingEngine ]
+			plugins: [ HeadingEngine ]
 		} )
 		.then( newEditor => {
 			editor = newEditor;
@@ -75,25 +74,6 @@ describe( 'HeadingEngine', () => {
 		expect( editor.getData() ).to.equal( '<h4>foobar</h4>' );
 	} );
 
-	it( 'should make enter command insert a defaultOption block if selection ended at the end of heading block', () => {
-		editor.setData( '<h2>foobar</h2>' );
-		document.selection.collapse( document.getRoot().getChild( 0 ), 'end' );
-
-		editor.execute( 'enter' );
-
-		expect( getData( document ) ).to.equal( '<heading1>foobar</heading1><paragraph>[]</paragraph>' );
-	} );
-
-	it( 'should not alter enter command if selection not ended at the end of heading block', () => {
-		// This test is to fill code coverage.
-		editor.setData( '<h2>foobar</h2>' );
-		document.selection.collapse( document.getRoot().getChild( 0 ), 3 );
-
-		editor.execute( 'enter' );
-
-		expect( getData( document ) ).to.equal( '<heading1>foo</heading1><heading1>[]bar</heading1>' );
-	} );
-
 	it( 'should not blow up if there\'s no enter command in the editor', () => {
 		return VirtualTestEditor.create( {
 			plugins: [ HeadingEngine ]
@@ -120,7 +100,7 @@ describe( 'HeadingEngine', () => {
 				];
 
 				return VirtualTestEditor.create( {
-					plugins: [ Enter, HeadingEngine ],
+					plugins: [ HeadingEngine ],
 					heading: {
 						options
 					}

+ 83 - 0
packages/ckeditor5-heading/tests/integration.js

@@ -0,0 +1,83 @@
+/**
+ * @license Copyright (c) 2003-2017, CKSource - Frederico Knabben. All rights reserved.
+ * For licensing, see LICENSE.md.
+ */
+
+/* global document */
+
+import Heading from '../src/heading.js';
+import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph';
+
+import Enter from '@ckeditor/ckeditor5-enter/src/enter';
+import Image from '@ckeditor/ckeditor5-image/src/image';
+import ImageCaption from '@ckeditor/ckeditor5-image/src/imagecaption';
+
+import ClassicTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/classictesteditor';
+import { getData as getModelData, setData as setModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model';
+
+describe( 'Heading integration', () => {
+	let editor, doc, element;
+
+	beforeEach( () => {
+		element = document.createElement( 'div' );
+		document.body.appendChild( element );
+
+		return ClassicTestEditor.create( element, {
+			plugins: [ Paragraph, Heading, Enter, Image, ImageCaption ]
+		} )
+		.then( newEditor => {
+			editor = newEditor;
+			doc = editor.document;
+		} );
+	} );
+
+	afterEach( () => {
+		element.remove();
+
+		return editor.destroy();
+	} );
+
+	describe( 'with the enter key', () => {
+		it( 'should make the "enter" command insert a default heading block if the selection ended at the end of a heading block', () => {
+			editor.setData( '<h2>foobar</h2>' );
+			doc.selection.collapse( doc.getRoot().getChild( 0 ), 'end' );
+
+			editor.execute( 'enter' );
+
+			expect( getModelData( doc ) ).to.equal( '<heading1>foobar</heading1><paragraph>[]</paragraph>' );
+		} );
+
+		it( 'should not alter the "enter" command if selection not ended at the end of a heading block', () => {
+			// This test is to fill code coverage.
+			editor.setData( '<h2>foobar</h2>' );
+			doc.selection.collapse( doc.getRoot().getChild( 0 ), 3 );
+
+			editor.execute( 'enter' );
+
+			expect( getModelData( doc ) ).to.equal( '<heading1>foo</heading1><heading1>[]bar</heading1>' );
+		} );
+	} );
+
+	describe( 'with the image feature', () => {
+		// https://github.com/ckeditor/ckeditor5-heading/issues/73
+		it( 'should not destroy the image when a selection converted to a heading', () => {
+			setModelData( editor.document,
+				'<paragraph>fo[o</paragraph>' +
+				'<image src="foo.png">' +
+					'<caption>xxx</caption>' +
+				'</image>' +
+				'<paragraph>b]ar</paragraph>'
+			);
+
+			editor.execute( 'heading1' );
+
+			expect( getModelData( doc ) ).to.equal(
+				'<heading1>fo[o</heading1>' +
+				'<image src="foo.png">' +
+					'<caption>xxx</caption>' +
+				'</image>' +
+				'<heading1>b]ar</heading1>'
+			);
+		} );
+	} );
+} );