Przeglądaj źródła

Creating separate command for each style.

Szymon Kupś 9 lat temu
rodzic
commit
a43d267aa3

+ 3 - 5
packages/ckeditor5-image/src/imagestyle/imagestyle.js

@@ -52,7 +52,7 @@ export default class ImageStyle extends Plugin {
 	 */
 	_createButton( style ) {
 		const editor = this.editor;
-		const command = editor.commands.get( 'imagestyle' );
+		const command = editor.commands.get( style.name );
 
 		editor.ui.componentFactory.add( style.name, ( locale ) => {
 			const view = new ButtonView( locale );
@@ -64,11 +64,9 @@ export default class ImageStyle extends Plugin {
 			} );
 
 			view.bind( 'isEnabled' ).to( command, 'isEnabled' );
-			view.bind( 'isOn' ).to( command, 'value', ( commandValue ) => {
-				return commandValue == style.value;
-			} );
+			view.bind( 'isOn' ).to( command, 'value' );
 
-			this.listenTo( view, 'execute', () => editor.execute( 'imagestyle', { value: style.value } ) );
+			this.listenTo( view, 'execute', () => editor.execute( style.name ) );
 
 			return view;
 		} );

+ 22 - 32
packages/ckeditor5-image/src/imagestyle/imagestylecommand.js

@@ -8,7 +8,7 @@
  */
 
 import Command from 'ckeditor5-core/src/command/command';
-import { isImage, getStyleByValue } from './utils';
+import { isImage } from './utils';
 
 /**
  * The image style command. It is used to apply different image styles.
@@ -17,30 +17,31 @@ import { isImage, getStyleByValue } from './utils';
  */
 export default class ImageStyleCommand extends Command {
 	/**
-	 * Creates instance of the command.
+	 * Creates instance of the image style command. Each command instance is handling one style.
 	 *
 	 * @param {module:core/editor/editor~Editor} editor Editor instance.
-	 * @param {Array.<module:image/imagestyle/imagestyleengine~ImageStyleFormat>} styles Allowed styles.
+	 * @param {module:image/imagestyle/imagestyleengine~ImageStyleFormat} styles Style to apply by this command.
 	 */
-	constructor( editor, styles ) {
+	constructor( editor, style ) {
 		super( editor );
 
 		/**
-		 * The current style value.
+		 * The current command value - `true` if style handled by the command is applied on currently selected image,
+		 * `false` otherwise.
 		 *
 		 * @readonly
 		 * @observable
-		 * @member {String} #value
+		 * @member {Boolean} #value
 		 */
 		this.set( 'value', false );
 
 		/**
-		 * Allowed image styles used by this command.
+		 * Style handled by this command.
 		 *
 		 * @readonly
-		 * @member {Array.<module:image/imagestyle/imagestyleengine~ImageStyleFormat>} #styles
+		 * @member {module:image/imagestyle/imagestyleengine~ImageStyleFormat} #style
 		 */
-		this.styles = styles;
+		this.style = style;
 
 		// Update current value and refresh state each time something change in model document.
 		this.listenTo( editor.document, 'changesDone', () => {
@@ -58,18 +59,16 @@ export default class ImageStyleCommand extends Command {
 		const doc = this.editor.document;
 		const element = doc.selection.getSelectedElement();
 
-		if ( isImage( element ) ) {
-			if ( element.hasAttribute( 'imageStyle' ) ) {
-				const value = element.getAttribute( 'imageStyle' );
+		if ( !element ) {
+			this.value = false;
 
-				// Check if value exists.
-				this.value = ( getStyleByValue( value, this.styles ) ? value : false );
-			} else {
-				// When there is no `style` attribute - set value to null.
-				this.value = null;
-			}
+			return;
+		}
+
+		if ( this.style.value === null ) {
+			this.value = !element.hasAttribute( 'imageStyle' );
 		} else {
-			this.value = false;
+			this.value = ( element.getAttribute( 'imageStyle' ) == this.style.value );
 		}
 	}
 
@@ -87,21 +86,12 @@ export default class ImageStyleCommand extends Command {
 	 *
 	 * @protected
 	 * @param {Object} options
-	 * @param {String} options.value Value to apply. It must be one of the values from styles passed to {@link #constructor}.
 	 * @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 ) {
-		const currentValue = this.value;
-		const newValue = options.value;
-
-		// Check if new value is valid.
-		if ( !getStyleByValue( newValue, this.styles ) ) {
-			return;
-		}
-
-		// Stop if same value is already applied.
-		if ( currentValue == newValue ) {
+	_doExecute( options = {} ) {
+		// Stop if style is already applied.
+		if ( this.value ) {
 			return;
 		}
 
@@ -113,7 +103,7 @@ export default class ImageStyleCommand extends Command {
 		doc.enqueueChanges( () => {
 			const batch = options.batch || doc.batch();
 
-			batch.setAttribute( imageElement, 'imageStyle', newValue );
+			batch.setAttribute( imageElement, 'imageStyle', this.style.value );
 		} );
 	}
 }

+ 7 - 6
packages/ckeditor5-image/src/imagestyle/imagestyleengine.js

@@ -64,16 +64,16 @@ export default class ImageStyleEngine extends Plugin {
 		editing.modelToView.on( 'removeAttribute:imageStyle:image', modelToViewConverter );
 		data.modelToView.on( 'removeAttribute:imageStyle:image', modelToViewConverter );
 
-		// Converter for figure element from view to model.
 		for ( let style of styles ) {
+			// Converter for figure element from view to model.
 			// Create converter only for non-null values.
 			if ( style.value !== null ) {
 				data.viewToModel.on( 'element:figure', viewToModelImageStyle( style ), { priority: 'low' } );
 			}
-		}
 
-		// Register image style command.
-		editor.commands.set( 'imagestyle', new ImageStyleCommand( editor, styles ) );
+			// Register separate command for each style.
+			editor.commands.set( style.name, new ImageStyleCommand( editor, style ) );
+		}
 	}
 }
 
@@ -91,8 +91,9 @@ export default class ImageStyleEngine extends Plugin {
  *	}
  *
  * @typedef {Object} module:image/imagestyle/imagestyleengine~ImageStyleFormat
- * @property {String} name Name of the style, it will be used to store style's button under that name in editor's
- * {@link module:ui/componentfactory~ComponentFactory ComponentFactory}.
+ * @property {String} name Name of the style. It will be used to:
+ * * register {@link module:core/command/command~Command command} which will apply this style,
+ * * store style's button in editor's {@link module:ui/componentfactory~ComponentFactory ComponentFactory}.
  * @property {String} value Value used to store this style in model attribute.
  * When value is `null` style will be used as default one. Default style does not apply any CSS class to the view element.
  * @property {String} icon SVG icon representation to use when creating style's button.

+ 2 - 2
packages/ckeditor5-image/tests/imagestyle/imagestyle.js

@@ -46,10 +46,10 @@ describe( 'ImageStyle', () => {
 	} );
 
 	it( 'should register buttons for each style', () => {
-		const command = editor.commands.get( 'imagestyle' );
 		const spy = sinon.spy( editor, 'execute' );
 
 		for ( let style of styles ) {
+			const command = editor.commands.get( style.name );
 			const buttonView =  editor.ui.componentFactory.create( style.name );
 
 			expect( buttonView ).to.be.instanceOf( ButtonView );
@@ -62,7 +62,7 @@ describe( 'ImageStyle', () => {
 			expect( buttonView.isEnabled ).to.be.false;
 
 			buttonView.fire( 'execute' );
-			sinon.assert.calledWithExactly( editor.execute, 'imagestyle', { value: style.value } );
+			sinon.assert.calledWithExactly( editor.execute, style.name );
 
 			spy.reset();
 		}

+ 27 - 29
packages/ckeditor5-image/tests/imagestyle/imagestylecommand.js

@@ -8,18 +8,17 @@ import ImageStyleCommand from 'ckeditor5-image/src/imagestyle/imagestylecommand'
 import { setData, getData } from 'ckeditor5-engine/src/dev-utils/model';
 
 describe( 'ImageStyleCommand', () => {
-	const styles = [
-		{ name: 'defaultStyle', title: 'foo bar', icon: 'icon-1', value: null },
-		{ name: 'otherStyle', title: 'baz', icon: 'icon-2', value: 'other', className: 'other-class-name' }
-	];
+	const defaultStyle = { name: 'defaultStyle', title: 'foo bar', icon: 'icon-1', value: null };
+	const otherStyle = { name: 'otherStyle', title: 'baz', icon: 'icon-2', value: 'other', className: 'other-class-name' };
 
-	let document, command;
+	let document, defaultStyleCommand, otherStyleCommand;
 
 	beforeEach( () => {
 		return ModelTestEditor.create()
 			.then( newEditor => {
 				document = newEditor.document;
-				command = new ImageStyleCommand( newEditor, styles );
+				defaultStyleCommand = new ImageStyleCommand( newEditor, defaultStyle );
+				otherStyleCommand = new ImageStyleCommand( newEditor, otherStyle );
 
 				document.schema.registerItem( 'p', '$block' );
 
@@ -30,50 +29,46 @@ describe( 'ImageStyleCommand', () => {
 			} );
 	} );
 
-	it( 'should have false if image is not selected', () => {
+	it( 'command value should be false if no image is selected', () => {
 		setData( document, '[]<image></image>' );
 
-		expect( command.value ).to.be.false;
+		expect( defaultStyleCommand.value ).to.be.false;
+		expect( otherStyleCommand.value ).to.be.false;
 	} );
 
-	it( 'should have null if image without style is selected', () => {
+	it( 'should match default style if no imageStyle attribute is present', () => {
 		setData( document, '[<image></image>]' );
 
-		expect( command.value ).to.be.null;
+		expect( defaultStyleCommand.value ).to.be.true;
+		expect( otherStyleCommand.value ).to.be.false;
 	} );
 
-	it( 'should have proper value if image with style is selected', () => {
+	it( 'proper command should have true value when imageStyle attribute is present', () => {
 		setData( document, '[<image imageStyle="other"></image>]' );
 
-		expect( command.value ).to.equal( 'other' );
+		expect( defaultStyleCommand.value ).to.be.false;
+		expect( otherStyleCommand.value ).to.be.true;
 	} );
 
-	it( 'should return false if value is not allowed', () => {
+	it( 'should have false value if style does not match', () => {
 		setData( document, '[<image imageStyle="foo"></image>]' );
 
-		expect( command.value ).to.be.false;
+		expect( defaultStyleCommand.value ).to.be.false;
+		expect( otherStyleCommand.value ).to.be.false;
 	} );
 
 	it( 'should set proper value when executed', () => {
 		setData( document, '[<image></image>]' );
 
-		command._doExecute( { value: 'other' } );
+		otherStyleCommand._doExecute();
 
 		expect( getData( document ) ).to.equal( '[<image imageStyle="other"></image>]' );
 	} );
 
-	it( 'should do nothing when executed with wrong value', () => {
-		setData( document, '[<image></image>]' );
-
-		command._doExecute( { value: 'foo' } );
-
-		expect( getData( document ) ).to.equal( '[<image></image>]' );
-	} );
-
-	it( 'should do nothing when executed with same value', () => {
+	it( 'should do nothing when attribute already present', () => {
 		setData( document, '[<image imageStyle="other"></image>]' );
 
-		command._doExecute( { value: 'other' } );
+		otherStyleCommand._doExecute();
 
 		expect( getData( document ) ).to.equal( '[<image imageStyle="other"></image>]' );
 	} );
@@ -84,7 +79,7 @@ describe( 'ImageStyleCommand', () => {
 
 		setData( document, '[<image></image>]' );
 
-		command._doExecute( { value: 'other', batch } );
+		otherStyleCommand._doExecute( { batch } );
 
 		expect( getData( document ) ).to.equal( '[<image imageStyle="other"></image>]' );
 		sinon.assert.calledOnce( spy );
@@ -93,18 +88,21 @@ describe( 'ImageStyleCommand', () => {
 	it( 'should be enabled on image element', () => {
 		setData( document, '[<image></image>]' );
 
-		expect( command.isEnabled ).to.be.true;
+		expect( defaultStyleCommand.isEnabled ).to.be.true;
+		expect( otherStyleCommand.isEnabled ).to.be.true;
 	} );
 
 	it( 'should be disabled when not placed on image', () => {
 		setData( document, '[<p></p>]' );
 
-		expect( command.isEnabled ).to.be.false;
+		expect( defaultStyleCommand.isEnabled ).to.be.false;
+		expect( otherStyleCommand.isEnabled ).to.be.false;
 	} );
 
 	it( 'should be disabled when not placed directly on image', () => {
 		setData( document, '[<p></p><image></image>]' );
 
-		expect( command.isEnabled ).to.be.false;
+		expect( defaultStyleCommand.isEnabled ).to.be.false;
+		expect( otherStyleCommand.isEnabled ).to.be.false;
 	} );
 } );

+ 8 - 5
packages/ckeditor5-image/tests/imagestyle/imagestyleengine.js

@@ -45,11 +45,14 @@ describe( 'ImageStyleEngine', () => {
 		expect( schema.check( { name: 'image', attributes: [ 'imageStyle', 'src' ], inside: '$root' } ) ).to.be.true;
 	} );
 
-	it( 'should register command', () => {
-		expect( editor.commands.has( 'imagestyle' ) ).to.be.true;
-		const command = editor.commands.get( 'imagestyle' );
-
-		expect( command ).to.be.instanceOf( ImageStyleCommand );
+	it( 'should register separate command for each style', () => {
+		expect( editor.commands.has( 'fullStyle' ) ).to.be.true;
+		expect( editor.commands.has( 'sideStyle' ) ).to.be.true;
+		expect( editor.commands.has( 'dummyStyle' ) ).to.be.true;
+
+		expect( editor.commands.get( 'fullStyle' ) ).to.be.instanceOf( ImageStyleCommand );
+		expect( editor.commands.get( 'sideStyle' ) ).to.be.instanceOf( ImageStyleCommand );
+		expect( editor.commands.get( 'dummyStyle' ) ).to.be.instanceOf( ImageStyleCommand );
 	} );
 
 	it( 'should convert from view to model', () => {