浏览代码

Code refac. Deobfuscated some pieces of code. Allowed custom title for paragraph option. Enhanced tests.

Aleksander Nowodzinski 8 年之前
父节点
当前提交
fcd9fdb54f

+ 4 - 10
packages/ckeditor5-heading/src/heading.js

@@ -41,7 +41,7 @@ export default class Heading extends Plugin {
 		for ( let option of options ) {
 			// Add the option to the collection.
 			dropdownItems.add( new Model( {
-				modelElement: option.modelElement,
+				commandName: option.modelElement,
 				label: option.title
 			} ) );
 
@@ -82,8 +82,8 @@ export default class Heading extends Plugin {
 			const dropdown = createListDropdown( dropdownModel, locale );
 
 			// Execute command when an item from the dropdown is selected.
-			this.listenTo( dropdown, 'execute', ( { source: { modelElement } } ) => {
-				editor.execute( modelElement );
+			this.listenTo( dropdown, 'execute', ( evt ) => {
+				editor.execute( evt.source.commandName );
 				editor.editing.view.focus();
 			} );
 
@@ -113,13 +113,7 @@ export default class Heading extends Plugin {
 		};
 
 		return editor.config.get( 'heading.options' ).map( option => {
-			let title;
-
-			if ( option.modelElement == 'paragraph' ) {
-				title = localizedTitles.Paragraph;
-			} else {
-				title = localizedTitles[ option.title ];
-			}
+			const title = localizedTitles[ option.title ];
 
 			if ( title && title != option.title ) {
 				// Clone the option to avoid altering the original `config.heading.options`.

+ 7 - 14
packages/ckeditor5-heading/src/headingcommand.js

@@ -77,9 +77,6 @@ export default class HeadingCommand extends Command {
 	_doExecute( options = {} ) {
 		const editor = this.editor;
 		const document = editor.document;
-		const selection = document.selection;
-		const ranges = [ ...selection.getRanges() ];
-		const isSelectionBackward = selection.isBackward;
 
 		// If current option is same as new option - toggle already applied option back to default one.
 		const shouldRemove = this.value;
@@ -87,28 +84,24 @@ export default class HeadingCommand extends Command {
 		document.enqueueChanges( () => {
 			const batch = options.batch || document.batch();
 
-			for ( let element of document.selection.getSelectedBlocks() ) {
+			for ( let block of document.selection.getSelectedBlocks() ) {
 				// When removing applied option.
 				if ( shouldRemove ) {
-					if ( element.name === this.modelElement ) {
-						// Apply paragraph to the selection withing that particular element only instead
+					if ( block.is( this.modelElement ) ) {
+						// Apply paragraph to the selection withing that particular block only instead
 						// of working on the entire document selection.
 						const selection = new Selection();
-						selection.addRange( Range.createIn( element ) );
+						selection.addRange( Range.createIn( block ) );
 
 						// Share the batch with the paragraph command.
 						editor.execute( 'paragraph', { selection, batch } );
 					}
 				}
 				// When applying new option.
-				else {
-					batch.rename( element, this.modelElement );
+				else if ( !block.is( this.modelElement ) ) {
+					batch.rename( block, this.modelElement );
 				}
 			}
-
-			// If range's selection start/end is placed directly in renamed block - we need to restore it's position
-			// after renaming, because renaming puts new element there.
-			selection.setRanges( ranges, isSelectionBackward );
 		} );
 	}
 
@@ -121,7 +114,7 @@ export default class HeadingCommand extends Command {
 		const block = this.editor.document.selection.getSelectedBlocks().next().value;
 
 		if ( block ) {
-			this.value = this.modelElement == block.name;
+			this.value = block.is( this.modelElement );
 		}
 	}
 }

+ 4 - 6
packages/ckeditor5-heading/src/headingengine.js

@@ -32,7 +32,7 @@ export default class HeadingEngine extends Plugin {
 		// more properties (https://github.com/ckeditor/ckeditor5/issues/403).
 		editor.config.define( 'heading', {
 			options: [
-				{ modelElement: 'paragraph' },
+				{ modelElement: 'paragraph', title: 'Paragraph' },
 				{ modelElement: 'heading1', viewElement: 'h2', title: 'Heading 1' },
 				{ modelElement: 'heading2', viewElement: 'h3', title: 'Heading 2' },
 				{ modelElement: 'heading3', viewElement: 'h4', title: 'Heading 3' }
@@ -55,7 +55,6 @@ export default class HeadingEngine extends Plugin {
 		const data = editor.data;
 		const editing = editor.editing;
 		const options = editor.config.get( 'heading.options' );
-		let command;
 
 		for ( let option of options ) {
 			// Skip paragraph - it is defined in required Paragraph feature.
@@ -74,8 +73,7 @@ export default class HeadingEngine extends Plugin {
 					.toElement( option.modelElement );
 
 				// Register the heading command for this option.
-				command = new HeadingCommand( editor, option );
-				editor.commands.set( command.modelElement, command );
+				editor.commands.set( option.modelElement, new HeadingCommand( editor, option ) );
 			}
 		}
 	}
@@ -94,9 +92,9 @@ export default class HeadingEngine extends Plugin {
 			this.listenTo( enterCommand, 'afterExecute', ( evt, data ) => {
 				const positionParent = editor.document.selection.getFirstPosition().parent;
 				const batch = data.batch;
-				const isHeading = options.some( option => option.modelElement == positionParent.name );
+				const isHeading = options.some( option => positionParent.is( option.modelElement ) );
 
-				if ( isHeading && positionParent.name != defaultModelElement && positionParent.childCount === 0 ) {
+				if ( isHeading && !positionParent.is( defaultModelElement ) && positionParent.childCount === 0 ) {
 					batch.rename( positionParent, defaultModelElement );
 				}
 			} );

+ 57 - 27
packages/ckeditor5-heading/tests/heading.js

@@ -63,7 +63,7 @@ describe( 'Heading', () => {
 			const executeSpy = testUtils.sinon.spy( editor, 'execute' );
 			const dropdown = editor.ui.componentFactory.create( 'headings' );
 
-			dropdown.modelElement = 'paragraph';
+			dropdown.commandName = 'paragraph';
 			dropdown.fire( 'execute' );
 
 			sinon.assert.calledOnce( executeSpy );
@@ -74,7 +74,7 @@ describe( 'Heading', () => {
 			const focusSpy = testUtils.sinon.spy( editor.editing.view, 'focus' );
 			const dropdown = editor.ui.componentFactory.create( 'headings' );
 
-			dropdown.modelElement = 'paragraph';
+			dropdown.commandName = 'paragraph';
 			dropdown.fire( 'execute' );
 
 			sinon.assert.calledOnce( focusSpy );
@@ -118,35 +118,18 @@ describe( 'Heading', () => {
 			let commands;
 
 			beforeEach( () => {
-				const editorElement = document.createElement( 'div' );
-
-				return ClassicTestEditor.create( editorElement, {
-					plugins: [ Heading ],
-					toolbar: [ 'heading' ],
-					lang: 'pl',
-					heading: {
-						options: [
-							{ modelElement: 'paragraph', viewElement: 'p', title: 'Paragraph' },
-							{ modelElement: 'heading1', viewElement: 'h2', title: 'Heading 1' },
-							{ modelElement: 'heading2', viewElement: 'h3', title: 'Not automatically localized' }
-						]
-					}
-				} )
-				.then( newEditor => {
-					editor = newEditor;
-					dropdown = editor.ui.componentFactory.create( 'headings' );
-					commands = {};
-					editor.config.get( 'heading.options' ).forEach( ( { modelElement } ) => {
-						commands[ modelElement ] = editor.commands.get( modelElement );
-					} );
-				} );
+				return localizedEditor( [
+					{ modelElement: 'paragraph', title: 'Paragraph' },
+					{ modelElement: 'heading1', viewElement: 'h2', title: 'Heading 1' },
+					{ modelElement: 'heading2', viewElement: 'h3', title: 'Heading 2' }
+				] );
 			} );
 
 			it( 'does not alter the original config', () => {
 				expect( editor.config.get( 'heading.options' ) ).to.deep.equal( [
-					{ modelElement: 'paragraph', viewElement: 'p', title: 'Paragraph' },
+					{ modelElement: 'paragraph', title: 'Paragraph' },
 					{ modelElement: 'heading1', viewElement: 'h2', title: 'Heading 1' },
-					{ modelElement: 'heading2', viewElement: 'h3', title: 'Not automatically localized' }
+					{ modelElement: 'heading2', viewElement: 'h3', title: 'Heading 2' }
 				] );
 			} );
 
@@ -164,9 +147,56 @@ describe( 'Heading', () => {
 				expect( listView.items.map( item => item.label ) ).to.deep.equal( [
 					'Akapit',
 					'Nagłówek 1',
-					'Not automatically localized'
+					'Nagłówek 2'
 				] );
 			} );
+
+			it( 'allows custom titles', () => {
+				return localizedEditor( [
+					{ modelElement: 'paragraph', title: 'Custom paragraph title' },
+					{ modelElement: 'heading1', title: 'Custom heading1 title' }
+				] ).then( () => {
+					const listView = dropdown.listView;
+
+					expect( listView.items.map( item => item.label ) ).to.deep.equal( [
+						'Custom paragraph title',
+						'Custom heading1 title',
+					] );
+				} );
+			} );
+
+			it( 'translates default using the the locale', () => {
+				return localizedEditor( [
+					{ modelElement: 'paragraph', title: 'Paragraph' }
+				] ).then( () => {
+					const listView = dropdown.listView;
+
+					expect( listView.items.map( item => item.label ) ).to.deep.equal( [
+						'Akapit'
+					] );
+				} );
+			} );
+
+			function localizedEditor( options ) {
+				const editorElement = document.createElement( 'div' );
+
+				return ClassicTestEditor.create( editorElement, {
+					plugins: [ Heading ],
+					toolbar: [ 'heading' ],
+					lang: 'pl',
+					heading: {
+						options: options
+					}
+				} )
+				.then( newEditor => {
+					editor = newEditor;
+					dropdown = editor.ui.componentFactory.create( 'headings' );
+					commands = {};
+					editor.config.get( 'heading.options' ).forEach( ( { modelElement } ) => {
+						commands[ modelElement ] = editor.commands.get( modelElement );
+					} );
+				} );
+			}
 		} );
 	} );
 } );

+ 1 - 1
packages/ckeditor5-heading/tests/headingcommand.js

@@ -158,7 +158,7 @@ describe( 'HeadingCommand', () => {
 			}
 
 			it( 'converts all elements where selection is applied', () => {
-				setData( document, '<heading1>foo[</heading1><heading2>bar</heading2><heading2>]baz</heading2>' );
+				setData( document, '<heading1>foo[</heading1><heading2>bar</heading2><heading3>]baz</heading3>' );
 				commands.heading3._doExecute();
 
 				expect( getData( document ) ).to.equal(

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

@@ -9,16 +9,8 @@ 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 { add } from '@ckeditor/ckeditor5-utils/src/translation-service';
 import { getData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model';
 
-add( 'pl', {
-	'Paragraph': 'Akapit',
-	'Heading 1': 'Nagłówek 1',
-	'Heading 2': 'Nagłówek 2',
-	'Heading 3': 'Nagłówek 3',
-} );
-
 describe( 'HeadingEngine', () => {
 	let editor, document;
 
@@ -113,7 +105,7 @@ describe( 'HeadingEngine', () => {
 			describe( 'default value', () => {
 				it( 'should be set', () => {
 					expect( editor.config.get( 'heading.options' ) ).to.deep.equal( [
-						{ modelElement: 'paragraph' },
+						{ modelElement: 'paragraph', title: 'Paragraph' },
 						{ modelElement: 'heading1', viewElement: 'h2', title: 'Heading 1' },
 						{ modelElement: 'heading2', viewElement: 'h3', title: 'Heading 2' },
 						{ modelElement: 'heading3', viewElement: 'h4', title: 'Heading 3' }
@@ -123,7 +115,7 @@ describe( 'HeadingEngine', () => {
 
 			it( 'should customize options', () => {
 				const options = [
-					{ modelElement: 'paragraph', viewElement: 'p', title: 'Paragraph' },
+					{ modelElement: 'paragraph', title: 'Paragraph' },
 					{ modelElement: 'h4', viewElement: 'h4', title: 'H4' }
 				];