From 74ec5d3c15a72a5e09c63419847b65d1c3975207 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szymon=20Kup=C5=9B?= Date: Thu, 15 Dec 2016 09:30:50 +0100 Subject: [PATCH 01/35] Added ImageStyleCommand class. --- src/imagestyle/imagestylecommand.js | 56 +++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) create mode 100644 src/imagestyle/imagestylecommand.js diff --git a/src/imagestyle/imagestylecommand.js b/src/imagestyle/imagestylecommand.js new file mode 100644 index 00000000..64d91f07 --- /dev/null +++ b/src/imagestyle/imagestylecommand.js @@ -0,0 +1,56 @@ +/** + * @license Copyright (c) 2003-2016, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md. + */ + +/** + * @module image/imagestyle/imagestylecommand + */ + +import Command from '../../core/command/command.js'; +import ModelElement from '../../engine/model/element.js'; + +export default class ImageStyleCommand extends Command { + constructor( editor ) { + super( editor ); + + this.set( 'value', false ); + + const document = this.editor.document; + + this.listenTo( document.selection, 'change', () => { + const element = getSelectedElement( document.selection ); + + if ( element && element.name === 'image' && element.hasAttribute( 'style' ) ) { + this.value = element.getAttribute( 'style' ); + } else { + this.value = false; + } + } ); + + this.listenTo( document, 'changesDone', () => { + this.refreshState(); + } ); + } + + _checkEnabled() { + const document = this.editor.document; + const element = getSelectedElement( document.selection ); + + // Todo: add schema checking. + return element && element.name === 'image'; + } +} + +// TODO: duplicated code - move to model selection. +function getSelectedElement( modelSelection ) { + if ( modelSelection.rangeCount !== 1 ) { + return null; + } + + const range = modelSelection.getFirstRange(); + const nodeAfterStart = range.start.nodeAfter; + const nodeBeforeEnd = range.end.nodeBefore; + + return ( nodeAfterStart instanceof ModelElement && nodeAfterStart == nodeBeforeEnd ) ? nodeAfterStart : null; +} From 777827a66418216b7a883b75a9fce9eba951328b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szymon=20Kup=C5=9B?= Date: Fri, 16 Dec 2016 15:37:38 +0100 Subject: [PATCH 02/35] Added first implementation of converters for image styles. --- src/imagestyle/imagestyle.js | 47 ++++++++ src/imagestyle/imagestylecommand.js | 20 +--- src/imagestyle/imagestyleengine.js | 165 ++++++++++++++++++++++++++++ tests/manual/image.html | 2 +- tests/manual/image.js | 5 +- 5 files changed, 221 insertions(+), 18 deletions(-) create mode 100644 src/imagestyle/imagestyle.js create mode 100644 src/imagestyle/imagestyleengine.js diff --git a/src/imagestyle/imagestyle.js b/src/imagestyle/imagestyle.js new file mode 100644 index 00000000..1296e66b --- /dev/null +++ b/src/imagestyle/imagestyle.js @@ -0,0 +1,47 @@ +/** + * @license Copyright (c) 2003-2016, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md. + */ + +/** + * @module image/imagestyle/imagestyle + */ + +import Plugin from '../../core/plugin.js'; +import ImageStyleEngine from './imagestyleengine.js'; +import ButtonView from '../../ui/button/buttonview.js'; + +export default class ImageStyle extends Plugin { + /** + * @inheritDoc + */ + static get requires() { + return [ ImageStyleEngine ]; + } + + /** + * @inheritDoc + */ + init() { + const editor = this.editor; + const t = editor.t; + const command = editor.commands.get( 'imagestyle' ); + + // Add bold button to feature components. + editor.ui.componentFactory.add( 'imagestyle', ( locale ) => { + const view = new ButtonView( locale ); + + view.set( { + label: t( 'Image style' ), + icon: 'bold' + } ); + + view.bind( 'isOn', 'isEnabled' ).to( command, 'value', 'isEnabled' ); + + // // Execute command. + this.listenTo( view, 'execute', () => editor.execute( 'imagestyle' ) ); + + return view; + } ); + } +} diff --git a/src/imagestyle/imagestylecommand.js b/src/imagestyle/imagestylecommand.js index 64d91f07..7a7865d1 100644 --- a/src/imagestyle/imagestylecommand.js +++ b/src/imagestyle/imagestylecommand.js @@ -17,9 +17,8 @@ export default class ImageStyleCommand extends Command { this.set( 'value', false ); const document = this.editor.document; - this.listenTo( document.selection, 'change', () => { - const element = getSelectedElement( document.selection ); + const element = document.selection.getSelectedElement(); if ( element && element.name === 'image' && element.hasAttribute( 'style' ) ) { this.value = element.getAttribute( 'style' ); @@ -35,22 +34,13 @@ export default class ImageStyleCommand extends Command { _checkEnabled() { const document = this.editor.document; - const element = getSelectedElement( document.selection ); + const element = document.selection.getSelectedElement(); - // Todo: add schema checking. return element && element.name === 'image'; } -} -// TODO: duplicated code - move to model selection. -function getSelectedElement( modelSelection ) { - if ( modelSelection.rangeCount !== 1 ) { - return null; + _doExecute() { + console.log( 'execute image style command' ); } - - const range = modelSelection.getFirstRange(); - const nodeAfterStart = range.start.nodeAfter; - const nodeBeforeEnd = range.end.nodeBefore; - - return ( nodeAfterStart instanceof ModelElement && nodeAfterStart == nodeBeforeEnd ) ? nodeAfterStart : null; } + diff --git a/src/imagestyle/imagestyleengine.js b/src/imagestyle/imagestyleengine.js new file mode 100644 index 00000000..28688bdb --- /dev/null +++ b/src/imagestyle/imagestyleengine.js @@ -0,0 +1,165 @@ +/** + * @license Copyright (c) 2003-2016, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md. + */ + +/** + * @module image/imagestyle/imagestyleengine + */ + +import Plugin from '../../core/plugin.js'; +import ImageStyleCommand from './imagestylecommand.js'; +import ImageEngine from '../imageengine.js'; +import ModelElement from '../../engine/model/element.js'; + +export default class ImageStyleEngine extends Plugin { + /** + * @inheritDoc + */ + static get requires() { + return [ ImageEngine ]; + } + + init() { + const editor = this.editor; + const doc = editor.document; + const schema = doc.schema; + const data = editor.data; + const editing = editor.editing; + + // Define default configuration. + editor.config.define( 'image.styles', { + options: { + // This option is equal to situation when no style is applied at all. + imageStyleFull: { title: 'Full size image', image: 'bold', value: null }, + + // This represents side image. + imageStyleSide: { title: 'Side image', image: 'italic', value: 'side', className: 'image-style-side' } + } + } ); + + // Get configuration. + const styles = editor.config.get( 'image.styles.options' ); + + // Allow style attribute in image. + schema.allow( { name: 'image', attributes: 'style' } ); + + // Converters for models element style attribute. + editing.modelToView.on( 'addAttribute:style', addStyle( styles ) ); + editing.modelToView.on( 'changeAttribute:style', changeStyle( styles ) ); + editing.modelToView.on( 'removeAttribute:style', removeStyle( styles ) ); + + for ( let key in styles ) { + const style = styles[ key ]; + + // 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 ) ); + } +} + +function addStyle( styles ) { + return ( event, data, consumable, conversionApi ) => { + // Check if we can consume, and we are adding in image. + if ( !consumable.consume( data.item, 'addAttribute:style' ) || !isImage( data.item ) ) { + return; + } + + // Check if there is class name associated with given value. + const newStyle = getStyleByValue( data.attributeNewValue, styles ); + + // Check if new style is allowed in configuration. + if ( !newStyle ) { + return; + } + + conversionApi.mapper.toViewElement( data.item ).addClass( newStyle.className ); + }; +} + +function changeStyle( styles ) { + return ( event, data, consumable, conversionApi ) => { + if ( !consumable.consume( data.item, 'changeAttribute:style' ) || !isImage( data.item ) ) { + return; + } + + // Check if there is class name associated with given value. + const newStyle = getStyleByValue( data.attributeNewValue, styles ); + + // Check if new style is allowed in configuration. + if ( !newStyle ) { + return; + } + + const viewElement = conversionApi.mapper.toViewElement( data.item ); + viewElement.removeClass( data.attributeOldValue ); + viewElement.addClass( newStyle.className ); + }; +} + +function removeStyle( styles ) { + return ( event, data, consumable, conversionApi ) => { + if ( !consumable.consume( data.item, 'removeAttribute:style' ) || !isImage( data.item ) ) { + return; + } + + // Check if there is class name associated with given value. + const newStyle = getStyleByValue( data.attributeNewValue, styles ); + + // Check if new style is allowed in configuration. + if ( !newStyle ) { + return; + } + + conversionApi.mapper.toViewElement( data.item ).viewElement.removeClass( data.attributeOldValue ); + }; +} + +function viewToModelImageStyle( style ) { + return ( evt, data, consumable, conversionApi ) => { + const viewFigureElement = data.input; + const modelImageElement = data.output; + + // *** Step 1: Validate conversion. + // Check if view element has proper class to consume. + if ( !consumable.test( viewFigureElement, { class: style.className } ) ) { + return; + } + + // Check if figure is converted to image. + if ( !isImage( modelImageElement ) ) { + return; + } + + // Check if image element can be placed in current context wit additional attribute. + const attributes = [ ...modelImageElement.getAttributeKeys(), 'style' ]; + + if ( !conversionApi.schema.check( { name: 'image', inside: data.context, attributes } ) ) { + return; + } + + // *** Step2: Convert to model. + consumable.consume( viewFigureElement, { class: style.className } ); + modelImageElement.setAttribute( 'style', style.value ); + }; +} + +function isImage( modelElement ) { + return modelElement instanceof ModelElement && modelElement.name == 'image'; +} + +function getStyleByValue( value, styles ) { + for ( let key in styles ) { + const style = styles[ key ]; + + if ( style.value == value ) { + return style; + } + } +} diff --git a/tests/manual/image.html b/tests/manual/image.html index ae7e45e9..77a55b15 100644 --- a/tests/manual/image.html +++ b/tests/manual/image.html @@ -1,6 +1,6 @@

Lorem ipsum dolor sit amet, consectetur adipiscing elit. Nulla finibus consequat placerat. Vestibulum id tellus et mauris sagittis tincidunt quis id mauris. Curabitur consectetur lectus sit amet tellus mattis, non lobortis leo interdum.

-
+
diff --git a/tests/manual/image.js b/tests/manual/image.js index c54b28c2..88af0253 100644 --- a/tests/manual/image.js +++ b/tests/manual/image.js @@ -13,10 +13,11 @@ import HeadingPlugin from 'ckeditor5-heading/src/heading'; import ImagePlugin from 'ckeditor5-image/src/image'; import UndoPlugin from 'ckeditor5-undo/src/undo'; import ClipboardPlugin from 'ckeditor5-clipboard/src/clipboard'; +import ImageStyle from 'ckeditor5-image/src/imagestyle/imagestyle'; ClassicEditor.create( document.querySelector( '#editor' ), { - plugins: [ EnterPlugin, TypingPlugin, ParagraphPlugin, HeadingPlugin, ImagePlugin, UndoPlugin, ClipboardPlugin ], - toolbar: [ 'headings', 'undo', 'redo' ] + plugins: [ EnterPlugin, TypingPlugin, ParagraphPlugin, HeadingPlugin, ImagePlugin, UndoPlugin, ClipboardPlugin, ImageStyle ], + toolbar: [ 'headings', 'undo', 'redo', 'imagestyle' ] } ) .then( editor => { window.editor = editor; From af6541db900890e81d6fd0405c79f6a5a554c186 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szymon=20Kup=C5=9B?= Date: Mon, 19 Dec 2016 13:26:32 +0100 Subject: [PATCH 03/35] Update image style command, added manual test. --- src/imagestyle/imagestyle.js | 26 ++++++++--- src/imagestyle/imagestylecommand.js | 69 +++++++++++++++++++++-------- src/imagestyle/imagestyleengine.js | 30 ++++--------- src/imagestyle/utils.js | 24 ++++++++++ tests/manual/imagestyle.html | 14 ++++++ tests/manual/imagestyle.js | 27 +++++++++++ tests/manual/imagestyle.md | 1 + theme/theme.scss | 8 ++++ 8 files changed, 154 insertions(+), 45 deletions(-) create mode 100644 src/imagestyle/utils.js create mode 100644 tests/manual/imagestyle.html create mode 100644 tests/manual/imagestyle.js create mode 100644 tests/manual/imagestyle.md diff --git a/src/imagestyle/imagestyle.js b/src/imagestyle/imagestyle.js index 1296e66b..6dd8e1a8 100644 --- a/src/imagestyle/imagestyle.js +++ b/src/imagestyle/imagestyle.js @@ -24,22 +24,36 @@ export default class ImageStyle extends Plugin { */ init() { const editor = this.editor; - const t = editor.t; + + // Get configuration. + const styles = editor.config.get( 'image.styles.options' ); + + for ( let name in styles ) { + this._createButton( name, styles[ name ] ); + } + } + + _createButton( name, style ) { + const editor = this.editor; const command = editor.commands.get( 'imagestyle' ); + const t = editor.t; // Add bold button to feature components. - editor.ui.componentFactory.add( 'imagestyle', ( locale ) => { + editor.ui.componentFactory.add( name, ( locale ) => { const view = new ButtonView( locale ); view.set( { - label: t( 'Image style' ), - icon: 'bold' + label: t( style.title ), + icon: style.icon } ); - view.bind( 'isOn', 'isEnabled' ).to( command, 'value', 'isEnabled' ); + view.bind( 'isEnabled' ).to( command, 'isEnabled' ); + view.bind( 'isOn' ).to( command, 'value', ( commandValue ) => { + return commandValue == style.value; + } ); // // Execute command. - this.listenTo( view, 'execute', () => editor.execute( 'imagestyle' ) ); + this.listenTo( view, 'execute', () => editor.execute( 'imagestyle', { value: style.value } ) ); return view; } ); diff --git a/src/imagestyle/imagestylecommand.js b/src/imagestyle/imagestylecommand.js index 7a7865d1..916df04b 100644 --- a/src/imagestyle/imagestylecommand.js +++ b/src/imagestyle/imagestylecommand.js @@ -8,39 +8,72 @@ */ import Command from '../../core/command/command.js'; -import ModelElement from '../../engine/model/element.js'; +import { isImage, getStyleByValue } from './utils.js'; export default class ImageStyleCommand extends Command { - constructor( editor ) { + constructor( editor, styles ) { super( editor ); this.set( 'value', false ); - const document = this.editor.document; - this.listenTo( document.selection, 'change', () => { - const element = document.selection.getSelectedElement(); + this.styles = styles; - if ( element && element.name === 'image' && element.hasAttribute( 'style' ) ) { - this.value = element.getAttribute( 'style' ); - } else { - this.value = false; - } - } ); - - this.listenTo( document, 'changesDone', () => { + this.listenTo( editor.document, 'changesDone', () => { + this._updateValue(); this.refreshState(); } ); } + _updateValue() { + const doc = this.editor.document; + const element = doc.selection.getSelectedElement(); + + if ( isImage( element ) ) { + if ( element.hasAttribute( 'style' ) ) { + const value = element.getAttribute( 'style' ); + + // 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; + } + } else { + this.value = false; + } + } + _checkEnabled() { - const document = this.editor.document; - const element = document.selection.getSelectedElement(); + const element = this.editor.document.selection.getSelectedElement(); - return element && element.name === 'image'; + return isImage( element ); } - _doExecute() { - console.log( 'execute image style command' ); + _doExecute( options = {} ) { + // TODO: add batch to options. + const currentValue = this.value; + const newValue = options.value; + + // Check if new value is valid. + if ( getStyleByValue( newValue, this.styles ) === undefined ) { + return; + } + + // Stop if same value is already applied. + if ( currentValue == newValue ) { + return; + } + + const editor = this.editor; + const doc = editor.document; + const selection = doc.selection; + const imageElement = selection.getSelectedElement(); + + doc.enqueueChanges( () => { + const batch = options.batch || doc.batch(); + + batch.setAttribute( imageElement, 'style', newValue ); + } ); } } diff --git a/src/imagestyle/imagestyleengine.js b/src/imagestyle/imagestyleengine.js index 28688bdb..44b9bb3b 100644 --- a/src/imagestyle/imagestyleengine.js +++ b/src/imagestyle/imagestyleengine.js @@ -10,7 +10,7 @@ import Plugin from '../../core/plugin.js'; import ImageStyleCommand from './imagestylecommand.js'; import ImageEngine from '../imageengine.js'; -import ModelElement from '../../engine/model/element.js'; +import { isImage, getStyleByValue } from './utils.js'; export default class ImageStyleEngine extends Plugin { /** @@ -31,10 +31,10 @@ export default class ImageStyleEngine extends Plugin { editor.config.define( 'image.styles', { options: { // This option is equal to situation when no style is applied at all. - imageStyleFull: { title: 'Full size image', image: 'bold', value: null }, + imageStyleFull: { title: 'Full size image', icon: 'bold', value: null }, // This represents side image. - imageStyleSide: { title: 'Side image', image: 'italic', value: 'side', className: 'image-style-side' } + imageStyleSide: { title: 'Side image', icon: 'italic', value: 'side', className: 'image-style-side' } } } ); @@ -60,7 +60,7 @@ export default class ImageStyleEngine extends Plugin { } // Register image style command. - editor.commands.set( 'imagestyle', new ImageStyleCommand( editor ) ); + editor.commands.set( 'imagestyle', new ImageStyleCommand( editor, styles ) ); } } @@ -111,13 +111,15 @@ function removeStyle( styles ) { // Check if there is class name associated with given value. const newStyle = getStyleByValue( data.attributeNewValue, styles ); + const oldStyle = getStyleByValue( data.attributeOldValue, styles ); - // Check if new style is allowed in configuration. - if ( !newStyle ) { + // Check if styles are allowed in configuration. + if ( !newStyle || !oldStyle ) { return; } - conversionApi.mapper.toViewElement( data.item ).viewElement.removeClass( data.attributeOldValue ); + const viewElement = conversionApi.mapper.toViewElement( data.item ); + viewElement.removeClass( oldStyle.className ); }; } @@ -149,17 +151,3 @@ function viewToModelImageStyle( style ) { modelImageElement.setAttribute( 'style', style.value ); }; } - -function isImage( modelElement ) { - return modelElement instanceof ModelElement && modelElement.name == 'image'; -} - -function getStyleByValue( value, styles ) { - for ( let key in styles ) { - const style = styles[ key ]; - - if ( style.value == value ) { - return style; - } - } -} diff --git a/src/imagestyle/utils.js b/src/imagestyle/utils.js new file mode 100644 index 00000000..63a52c00 --- /dev/null +++ b/src/imagestyle/utils.js @@ -0,0 +1,24 @@ +/** + * @license Copyright (c) 2003-2016, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md. + */ + +/** + * @module image/imagestyle/utils + */ + +import ModelElement from '../../engine/model/element.js'; + +export function isImage( modelElement ) { + return modelElement instanceof ModelElement && modelElement.name == 'image'; +} + +export function getStyleByValue( value, styles ) { + for ( let key in styles ) { + const style = styles[ key ]; + + if ( style.value === value ) { + return style; + } + } +} diff --git a/tests/manual/imagestyle.html b/tests/manual/imagestyle.html new file mode 100644 index 00000000..6eb02b68 --- /dev/null +++ b/tests/manual/imagestyle.html @@ -0,0 +1,14 @@ + + + + +
+

Lorem ipsum dolor sit amet, consectetur adipiscing elit. Nulla finibus consequat placerat. Vestibulum id tellus et mauris sagittis tincidunt quis id mauris. Curabitur consectetur lectus sit amet tellus mattis, non lobortis leo interdum.

+
+ +
+

Lorem ipsum dolor sit amet, consectetur adipiscing elit. Nulla finibus consequat placerat. Vestibulum id tellus et mauris sagittis tincidunt quis id mauris. Curabitur consectetur lectus sit amet tellus mattis, non lobortis leo interdum. Lorem ipsum dolor sit amet, consectetur adipiscing elit. Nulla finibus consequat placerat. Vestibulum id tellus et mauris sagittis tincidunt quis id mauris. Curabitur consectetur lectus sit amet tellus mattis, non lobortis leo interdum. Lorem ipsum dolor sit amet, consectetur adipiscing elit. Nulla finibus consequat placerat. Vestibulum id tellus et mauris sagittis tincidunt quis id mauris. Curabitur consectetur lectus sit amet tellus mattis, non lobortis leo interdum. Lorem ipsum dolor sit amet, consectetur adipiscing elit. Nulla finibus consequat placerat. Vestibulum id tellus et mauris sagittis tincidunt quis id mauris. Curabitur consectetur lectus sit amet tellus mattis, non lobortis leo interdum.

+

Lorem ipsum dolor sit amet, consectetur adipiscing elit. Nulla finibus consequat placerat. Vestibulum id tellus et mauris sagittis tincidunt quis id mauris. Curabitur consectetur lectus sit amet tellus mattis, non lobortis leo interdum.

+

Lorem ipsum dolor sit amet, consectetur adipiscing elit. Nulla finibus consequat placerat. Vestibulum id tellus et mauris sagittis tincidunt quis id mauris. Curabitur consectetur lectus sit amet tellus mattis, non lobortis leo interdum.

+

Lorem ipsum dolor sit amet, consectetur adipiscing elit. Nulla finibus consequat placerat. Vestibulum id tellus et mauris sagittis tincidunt quis id mauris. Curabitur consectetur lectus sit amet tellus mattis, non lobortis leo interdum.

+
diff --git a/tests/manual/imagestyle.js b/tests/manual/imagestyle.js new file mode 100644 index 00000000..96d3c8a5 --- /dev/null +++ b/tests/manual/imagestyle.js @@ -0,0 +1,27 @@ +/** + * @license Copyright (c) 2003-2016, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md. + */ + +/* global document, console, window */ + +import ClassicEditor from 'ckeditor5/editor-classic/classic.js'; +import EnterPlugin from 'ckeditor5/enter/enter.js'; +import TypingPlugin from 'ckeditor5/typing/typing.js'; +import ParagraphPlugin from 'ckeditor5/paragraph/paragraph.js'; +import HeadingPlugin from 'ckeditor5/heading/heading.js'; +import ImagePlugin from 'ckeditor5/image/image.js'; +import UndoPlugin from 'ckeditor5/undo/undo.js'; +import ClipboardPlugin from 'ckeditor5/clipboard/clipboard.js'; +import ImageStyle from 'ckeditor5/image/imagestyle/imagestyle.js'; + +ClassicEditor.create( document.querySelector( '#editor' ), { + plugins: [ EnterPlugin, TypingPlugin, ParagraphPlugin, HeadingPlugin, ImagePlugin, UndoPlugin, ClipboardPlugin, ImageStyle ], + toolbar: [ 'headings', 'undo', 'redo', 'imageStyleFull', 'imageStyleSide' ] +} ) + .then( editor => { + window.editor = editor; + } ) + .catch( err => { + console.error( err.stack ); + } ); diff --git a/tests/manual/imagestyle.md b/tests/manual/imagestyle.md new file mode 100644 index 00000000..d2159a2d --- /dev/null +++ b/tests/manual/imagestyle.md @@ -0,0 +1 @@ +## ImageStyle feature diff --git a/theme/theme.scss b/theme/theme.scss index 05b9b568..7e57f36b 100644 --- a/theme/theme.scss +++ b/theme/theme.scss @@ -22,4 +22,12 @@ // Image widget's styles. .ck-widget.image { text-align: center; + clear: both; + + &.image-style-side { + display: inline-block; + float: right; + margin-left: 0.8em; + } } + From 9b7a5568c2834db882e71329e58723c5856ac292 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szymon=20Kup=C5=9B?= Date: Mon, 19 Dec 2016 15:30:38 +0100 Subject: [PATCH 04/35] Added image toolbar. --- src/imagetoolbar.js | 114 +++++++++++++++++++++++++++++++++++++ tests/manual/imagestyle.js | 8 ++- 2 files changed, 120 insertions(+), 2 deletions(-) create mode 100644 src/imagetoolbar.js diff --git a/src/imagetoolbar.js b/src/imagetoolbar.js new file mode 100644 index 00000000..d9f94eef --- /dev/null +++ b/src/imagetoolbar.js @@ -0,0 +1,114 @@ +/** + * @license Copyright (c) 2003-2016, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md. + */ + +/** + * @module image/imagetoolbar + */ + +/* globals window */ + +import Plugin from '../core/plugin.js'; +import ToolbarView from '../ui/toolbar/toolbarview.js'; +import BalloonPanelView from '../ui/balloonpanel/balloonpanelview.js'; +import Template from '../ui/template.js'; +import ClickObserver from 'ckeditor5/engine/view/observer/clickobserver.js'; + +const arrowVOffset = BalloonPanelView.arrowVerticalOffset; +const positions = { + // [text range] + // ^ + // +-----------------+ + // | Balloon | + // +-----------------+ + south: ( targetRect, balloonRect ) => ( { + top: targetRect.bottom + arrowVOffset, + left: targetRect.left + targetRect.width / 2 - balloonRect.width / 2, + name: 's' + } ), + + // +-----------------+ + // | Balloon | + // +-----------------+ + // V + // [text range] + north: ( targetRect, balloonRect ) => ( { + top: targetRect.top - balloonRect.height - arrowVOffset, + left: targetRect.left + targetRect.width / 2 - balloonRect.width / 2, + name: 'n' + } ) +}; + +export default class ImageToolbar extends Plugin { + init() { + const editor = this.editor; + + // Create a plain toolbar instance. + const toolbar = new ToolbarView(); + + // Create a BalloonPanelView instance. + const panel = new BalloonPanelView( editor.locale ); + + Template.extend( panel.template, { + attributes: { + class: [ + 'ck-toolbar__container', + ] + } + } ); + + // Putting the toolbar inside of the balloon panel. + panel.content.add( toolbar ); + + return editor.ui.view.body.add( panel ).then( () => { + const editingView = editor.editing.view; + const toolbarConfig = editor.config.get( 'image.toolbar' ); + const promises = []; + + if ( toolbarConfig ) { + for ( let name of toolbarConfig ) { + promises.push( toolbar.items.add( editor.ui.componentFactory.create( name ) ) ); + } + } + + // Let the focusTracker know about new focusable UI element. + editor.ui.focusTracker.add( panel.element ); + + // Hide the panel when editor loses focus but no the other way around. + panel.listenTo( editor.ui.focusTracker, 'change:isFocused', ( evt, name, is, was ) => { + if ( was && !is ) { + panel.hide(); + } + } ); + + editingView.addObserver( ClickObserver ); + + // Check if the toolbar should be displayed each time the user clicked in editable. + editor.listenTo( editingView, 'click', () => { + if ( editingView.selection.isFake ) { + attachToolbar(); + + // TODO: These 2 need interval–based event debouncing for performance + // reasons. I guess even lodash offers such a helper. + editor.ui.view.listenTo( window, 'scroll', attachToolbar ); + editor.ui.view.listenTo( window, 'resize', attachToolbar ); + } else { + panel.hide(); + + editor.ui.view.stopListening( window, 'scroll', attachToolbar ); + editor.ui.view.stopListening( window, 'resize', attachToolbar ); + } + } ); + + function attachToolbar() { + panel.attachTo( { + target: editingView.domConverter.viewRangeToDom( editingView.selection.getFirstRange() ), + positions: [ positions.north, positions.south ] + } ); + } + + return Promise.all( promises ); + } ); + } +} diff --git a/tests/manual/imagestyle.js b/tests/manual/imagestyle.js index 96d3c8a5..aeb1e53b 100644 --- a/tests/manual/imagestyle.js +++ b/tests/manual/imagestyle.js @@ -14,10 +14,14 @@ import ImagePlugin from 'ckeditor5/image/image.js'; import UndoPlugin from 'ckeditor5/undo/undo.js'; import ClipboardPlugin from 'ckeditor5/clipboard/clipboard.js'; import ImageStyle from 'ckeditor5/image/imagestyle/imagestyle.js'; +import ImageToolbar from 'ckeditor5/image/imagetoolbar.js'; ClassicEditor.create( document.querySelector( '#editor' ), { - plugins: [ EnterPlugin, TypingPlugin, ParagraphPlugin, HeadingPlugin, ImagePlugin, UndoPlugin, ClipboardPlugin, ImageStyle ], - toolbar: [ 'headings', 'undo', 'redo', 'imageStyleFull', 'imageStyleSide' ] + plugins: [ EnterPlugin, TypingPlugin, ParagraphPlugin, HeadingPlugin, ImagePlugin, UndoPlugin, ClipboardPlugin, ImageStyle, ImageToolbar ], + toolbar: [ 'headings', 'undo', 'redo' ], + image: { + toolbar: [ 'imageStyleFull', 'imageStyleSide' ] + } } ) .then( editor => { window.editor = editor; From 26578db02f64b592b42df054cac1d7b5e2c2144f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szymon=20Kup=C5=9B?= Date: Wed, 21 Dec 2016 10:57:44 +0100 Subject: [PATCH 05/35] Added tests to ImageStyleCommand. --- tests/imagestyle/imagestylecommand.js | 110 ++++++++++++++++++++++++++ theme/icons/object-center.svg | 1 + theme/icons/object-left.svg | 1 + theme/icons/object-right.svg | 1 + 4 files changed, 113 insertions(+) create mode 100644 tests/imagestyle/imagestylecommand.js create mode 100644 theme/icons/object-center.svg create mode 100644 theme/icons/object-left.svg create mode 100644 theme/icons/object-right.svg diff --git a/tests/imagestyle/imagestylecommand.js b/tests/imagestyle/imagestylecommand.js new file mode 100644 index 00000000..c4a55074 --- /dev/null +++ b/tests/imagestyle/imagestylecommand.js @@ -0,0 +1,110 @@ +/** + * @license Copyright (c) 2003-2016, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md. + */ + +import ModelTestEditor from 'tests/core/_utils/modeltesteditor.js'; +import ImageStyleCommand from 'ckeditor5/image/imagestyle/imagestylecommand.js'; +import { setData, getData } from 'ckeditor5/engine/dev-utils/model.js'; + +describe( 'ImageStyleCommand', () => { + const styles = { + defaultStyle: { title: 'foo bar', icon: 'icon-1', value: null }, + otherStyle: { title: 'baz', icon: 'icon-2', value: 'other', className: 'other-class-name' } + }; + + let document, command; + + beforeEach( () => { + return ModelTestEditor.create() + .then( newEditor => { + document = newEditor.document; + command = new ImageStyleCommand( newEditor, styles ); + + document.schema.registerItem( 'p', '$block' ); + + document.schema.registerItem( 'image' ); + document.schema.objects.add( 'image' ); + document.schema.allow( { name: 'image', inside: '$root' } ); + document.schema.allow( { name: 'image', inside: '$root', attributes: [ 'imageStyle' ] } ); + } ); + } ); + + it( 'should have false if image is not selected', () => { + setData( document, '[]' ); + + expect( command.value ).to.be.false; + } ); + + it( 'should have null if image without style is selected', () => { + setData( document, '[]' ); + + expect( command.value ).to.be.null; + } ); + + it( 'should have proper value if image with style is selected', () => { + setData( document, '[]' ); + + expect( command.value ).to.equal( 'other' ); + } ); + + it( 'should return false if value is not allowed', () => { + setData( document, '[]' ); + + expect( command.value ).to.be.false; + } ); + + it( 'should set proper value when executed', () => { + setData( document, '[]' ); + + command._doExecute( { value: 'other' } ); + + expect( getData( document ) ).to.equal( '[]' ); + } ); + + it( 'should do nothing when executed with wrong value', () => { + setData( document, '[]' ); + + command._doExecute( { value: 'foo' } ); + + expect( getData( document ) ).to.equal( '[]' ); + } ); + + it( 'should do nothing when executed with same value', () => { + setData( document, '[]' ); + + command._doExecute( { value: 'other' } ); + + expect( getData( document ) ).to.equal( '[]' ); + } ); + + it( 'should allow to provide batch instance', () => { + const batch = document.batch(); + const spy = sinon.spy( batch, 'setAttribute' ); + + setData( document, '[]' ); + + command._doExecute( { value: 'other', batch } ); + + expect( getData( document ) ).to.equal( '[]' ); + sinon.assert.calledOnce( spy ); + } ); + + it( 'should be enabled on image element', () => { + setData( document, '[]' ); + + expect( command.isEnabled ).to.be.true; + } ); + + it( 'should be disabled when not placed on image', () => { + setData( document, '[

]' ); + + expect( command.isEnabled ).to.be.false; + } ); + + it( 'should be disabled when not placed directly on image', () => { + setData( document, '[

]' ); + + expect( command.isEnabled ).to.be.false; + } ); +} ); diff --git a/theme/icons/object-center.svg b/theme/icons/object-center.svg new file mode 100644 index 00000000..20728135 --- /dev/null +++ b/theme/icons/object-center.svg @@ -0,0 +1 @@ + diff --git a/theme/icons/object-left.svg b/theme/icons/object-left.svg new file mode 100644 index 00000000..fc10c9b8 --- /dev/null +++ b/theme/icons/object-left.svg @@ -0,0 +1 @@ + diff --git a/theme/icons/object-right.svg b/theme/icons/object-right.svg new file mode 100644 index 00000000..ff700a92 --- /dev/null +++ b/theme/icons/object-right.svg @@ -0,0 +1 @@ + From 0f96c7b20ce1c58a9406468894e48172582312fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szymon=20Kup=C5=9B?= Date: Wed, 21 Dec 2016 13:07:22 +0100 Subject: [PATCH 06/35] Updated docs in ImageStyleCommand. --- src/imagestyle/imagestylecommand.js | 55 +++++++++++++++++++++++++---- 1 file changed, 49 insertions(+), 6 deletions(-) diff --git a/src/imagestyle/imagestylecommand.js b/src/imagestyle/imagestylecommand.js index 916df04b..9f524974 100644 --- a/src/imagestyle/imagestylecommand.js +++ b/src/imagestyle/imagestylecommand.js @@ -10,27 +10,59 @@ import Command from '../../core/command/command.js'; import { isImage, getStyleByValue } from './utils.js'; +/** + * The image style command. It is used to apply different image styles. + * + * @extends module:core/command/command~Command + */ export default class ImageStyleCommand extends Command { + + /** + * Creates instance of the command. + * + * @param {module:core/editor/editor~Editor} editor Editor instance. + * @param {Object} styles Allowed styles. See {@link module:image/imagestyle/imagestyleengine~ImageStyleFormat + * ImageStyleFormat} for more information. + */ constructor( editor, styles ) { super( editor ); + /** + * The current style value. + * + * @readonly + * @observable + * @member {String} #value + */ this.set( 'value', false ); + /** + * Image styles uses by this command. + * + * @readonly + * @member {Object} Allowed image styles. See {@link module:image/imagestyle/imagestyleengine~ImageStyleFormat}. + */ this.styles = styles; + // Update current value and refresh state each time something change in model document. this.listenTo( editor.document, 'changesDone', () => { this._updateValue(); this.refreshState(); } ); } + /** + * Updates command's value. + * + * @private + */ _updateValue() { const doc = this.editor.document; const element = doc.selection.getSelectedElement(); if ( isImage( element ) ) { - if ( element.hasAttribute( 'style' ) ) { - const value = element.getAttribute( 'style' ); + if ( element.hasAttribute( 'imageStyle' ) ) { + const value = element.getAttribute( 'imageStyle' ); // Check if value exists. this.value = ( getStyleByValue( value, this.styles ) ? value : false ); @@ -43,19 +75,30 @@ export default class ImageStyleCommand extends Command { } } + /** + * @inheritDoc + */ _checkEnabled() { const element = this.editor.document.selection.getSelectedElement(); return isImage( element ); } - _doExecute( options = {} ) { - // TODO: add batch to options. + /** + * Executes 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 ) === undefined ) { + if ( !getStyleByValue( newValue, this.styles ) ) { return; } @@ -72,7 +115,7 @@ export default class ImageStyleCommand extends Command { doc.enqueueChanges( () => { const batch = options.batch || doc.batch(); - batch.setAttribute( imageElement, 'style', newValue ); + batch.setAttribute( imageElement, 'imageStyle', newValue ); } ); } } From 8069e197634d4b9137148ff4d9f8eaaceca4e4b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szymon=20Kup=C5=9B?= Date: Wed, 21 Dec 2016 17:02:01 +0100 Subject: [PATCH 07/35] Updated docs and tests for ImageStyleEngine. --- src/imagestyle/converters.js | 101 +++++++++++++++++++++ src/imagestyle/imagestylecommand.js | 1 - src/imagestyle/imagestyleengine.js | 127 +++++++-------------------- src/imagetoolbar.js | 11 ++- tests/imagestyle/imagestyleengine.js | 72 +++++++++++++++ 5 files changed, 211 insertions(+), 101 deletions(-) create mode 100644 src/imagestyle/converters.js create mode 100644 tests/imagestyle/imagestyleengine.js diff --git a/src/imagestyle/converters.js b/src/imagestyle/converters.js new file mode 100644 index 00000000..e35a5be2 --- /dev/null +++ b/src/imagestyle/converters.js @@ -0,0 +1,101 @@ +/** + * @license Copyright (c) 2003-2016, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md. + */ + +/** + * @module image/imagestyle/converters + */ + +import { isImage, getStyleByValue } from './utils.js'; + +export function addStyle( styles ) { + return ( event, data, consumable, conversionApi ) => { + // Check if we can consume, and we are adding in image. + if ( !consumable.consume( data.item, 'addAttribute:imageStyle' ) || !isImage( data.item ) ) { + return; + } + + // Check if there is class name associated with given value. + const newStyle = getStyleByValue( data.attributeNewValue, styles ); + + // Check if new style is allowed in configuration. + if ( !newStyle ) { + return; + } + + conversionApi.mapper.toViewElement( data.item ).addClass( newStyle.className ); + }; +} + +export function changeStyle( styles ) { + return ( event, data, consumable, conversionApi ) => { + if ( !consumable.consume( data.item, 'changeAttribute:imageStyle' ) || !isImage( data.item ) ) { + return; + } + + // Check if there is class name associated with given value. + const newStyle = getStyleByValue( data.attributeNewValue, styles ); + const oldStyle = getStyleByValue( data.attributeOldValue, styles ); + + // Check if new style is allowed in configuration. + if ( !newStyle || !oldStyle ) { + return; + } + + const viewElement = conversionApi.mapper.toViewElement( data.item ); + viewElement.removeClass( data.attributeOldValue ); + viewElement.addClass( newStyle.className ); + }; +} + +export function removeStyle( styles ) { + return ( event, data, consumable, conversionApi ) => { + if ( !consumable.test( data.item, 'removeAttribute:imageStyle' ) || !isImage( data.item ) ) { + return; + } + + // Check if there is class name associated with given value. + const newStyle = getStyleByValue( data.attributeNewValue, styles ); + const oldStyle = getStyleByValue( data.attributeOldValue, styles ); + + // Check if styles are allowed in configuration. + if ( !newStyle || !oldStyle ) { + return; + } + + consumable.consume( data.item, 'removeAttribute:imageStyle' ); + + const viewElement = conversionApi.mapper.toViewElement( data.item ); + viewElement.removeClass( oldStyle.className ); + }; +} + +export function viewToModelImageStyle( style ) { + return ( evt, data, consumable, conversionApi ) => { + const viewFigureElement = data.input; + const modelImageElement = data.output; + + // *** Step 1: Validate conversion. + // Check if view element has proper class to consume. + if ( !consumable.test( viewFigureElement, { class: style.className } ) ) { + return; + } + + // Check if figure is converted to image. + if ( !isImage( modelImageElement ) ) { + return; + } + + // Check if image element can be placed in current context wit additional attribute. + const attributes = [ ...modelImageElement.getAttributeKeys(), 'imageStyle' ]; + + if ( !conversionApi.schema.check( { name: 'image', inside: data.context, attributes } ) ) { + return; + } + + // *** Step2: Convert to model. + consumable.consume( viewFigureElement, { class: style.className } ); + modelImageElement.setAttribute( 'imageStyle', style.value ); + }; +} diff --git a/src/imagestyle/imagestylecommand.js b/src/imagestyle/imagestylecommand.js index 9f524974..1c0e02f6 100644 --- a/src/imagestyle/imagestylecommand.js +++ b/src/imagestyle/imagestylecommand.js @@ -16,7 +16,6 @@ import { isImage, getStyleByValue } from './utils.js'; * @extends module:core/command/command~Command */ export default class ImageStyleCommand extends Command { - /** * Creates instance of the command. * diff --git a/src/imagestyle/imagestyleengine.js b/src/imagestyle/imagestyleengine.js index 44b9bb3b..dca8399e 100644 --- a/src/imagestyle/imagestyleengine.js +++ b/src/imagestyle/imagestyleengine.js @@ -10,8 +10,14 @@ import Plugin from '../../core/plugin.js'; import ImageStyleCommand from './imagestylecommand.js'; import ImageEngine from '../imageengine.js'; -import { isImage, getStyleByValue } from './utils.js'; +import { addStyle, changeStyle, removeStyle, viewToModelImageStyle } from './converters.js'; +/** + * The image style engine plugin. Sets default configuration, creates converters and registers + * {@link module:image/imagestyle/imagestylecommand~ImageStyleCommand ImageStyleCommand}. + * + * @extends {module:core/plugin~Plugin} + */ export default class ImageStyleEngine extends Plugin { /** * @inheritDoc @@ -20,6 +26,9 @@ export default class ImageStyleEngine extends Plugin { return [ ImageEngine ]; } + /** + * @inheritDoc + */ init() { const editor = this.editor; const doc = editor.document; @@ -31,23 +40,26 @@ export default class ImageStyleEngine extends Plugin { editor.config.define( 'image.styles', { options: { // This option is equal to situation when no style is applied at all. - imageStyleFull: { title: 'Full size image', icon: 'bold', value: null }, + imageStyleFull: { title: 'Full size image', icon: 'object-center', value: null }, // This represents side image. - imageStyleSide: { title: 'Side image', icon: 'italic', value: 'side', className: 'image-style-side' } + imageStyleSide: { title: 'Side image', icon: 'object-right', value: 'side', className: 'image-style-side' } } } ); // Get configuration. const styles = editor.config.get( 'image.styles.options' ); - // Allow style attribute in image. - schema.allow( { name: 'image', attributes: 'style' } ); + // Allow imageStyle attribute in image. + // We could call it 'style' but https://github.com/ckeditor/ckeditor5-engine/issues/559. + schema.allow( { name: 'image', attributes: 'imageStyle', inside: '$root' } ); - // Converters for models element style attribute. - editing.modelToView.on( 'addAttribute:style', addStyle( styles ) ); - editing.modelToView.on( 'changeAttribute:style', changeStyle( styles ) ); - editing.modelToView.on( 'removeAttribute:style', removeStyle( styles ) ); + // Converters for models element imageStyle attribute. + editing.modelToView.on( 'addAttribute:imageStyle', addStyle( styles ) ); + data.modelToView.on( 'addAttribute:imageStyle', addStyle( styles ) ); + editing.modelToView.on( 'changeAttribute:imageStyle', changeStyle( styles ) ); + data.modelToView.on( 'changeAttribute:imageStyle', changeStyle( styles ) ); + editing.modelToView.on( 'removeAttribute:imageStyle', removeStyle( styles ) ); for ( let key in styles ) { const style = styles[ key ]; @@ -64,90 +76,13 @@ export default class ImageStyleEngine extends Plugin { } } -function addStyle( styles ) { - return ( event, data, consumable, conversionApi ) => { - // Check if we can consume, and we are adding in image. - if ( !consumable.consume( data.item, 'addAttribute:style' ) || !isImage( data.item ) ) { - return; - } - - // Check if there is class name associated with given value. - const newStyle = getStyleByValue( data.attributeNewValue, styles ); - - // Check if new style is allowed in configuration. - if ( !newStyle ) { - return; - } - - conversionApi.mapper.toViewElement( data.item ).addClass( newStyle.className ); - }; -} - -function changeStyle( styles ) { - return ( event, data, consumable, conversionApi ) => { - if ( !consumable.consume( data.item, 'changeAttribute:style' ) || !isImage( data.item ) ) { - return; - } - - // Check if there is class name associated with given value. - const newStyle = getStyleByValue( data.attributeNewValue, styles ); - - // Check if new style is allowed in configuration. - if ( !newStyle ) { - return; - } - - const viewElement = conversionApi.mapper.toViewElement( data.item ); - viewElement.removeClass( data.attributeOldValue ); - viewElement.addClass( newStyle.className ); - }; -} - -function removeStyle( styles ) { - return ( event, data, consumable, conversionApi ) => { - if ( !consumable.consume( data.item, 'removeAttribute:style' ) || !isImage( data.item ) ) { - return; - } - - // Check if there is class name associated with given value. - const newStyle = getStyleByValue( data.attributeNewValue, styles ); - const oldStyle = getStyleByValue( data.attributeOldValue, styles ); - - // Check if styles are allowed in configuration. - if ( !newStyle || !oldStyle ) { - return; - } - - const viewElement = conversionApi.mapper.toViewElement( data.item ); - viewElement.removeClass( oldStyle.className ); - }; -} - -function viewToModelImageStyle( style ) { - return ( evt, data, consumable, conversionApi ) => { - const viewFigureElement = data.input; - const modelImageElement = data.output; - - // *** Step 1: Validate conversion. - // Check if view element has proper class to consume. - if ( !consumable.test( viewFigureElement, { class: style.className } ) ) { - return; - } - - // Check if figure is converted to image. - if ( !isImage( modelImageElement ) ) { - return; - } - - // Check if image element can be placed in current context wit additional attribute. - const attributes = [ ...modelImageElement.getAttributeKeys(), 'style' ]; - - if ( !conversionApi.schema.check( { name: 'image', inside: data.context, attributes } ) ) { - return; - } - - // *** Step2: Convert to model. - consumable.consume( viewFigureElement, { class: style.className } ); - modelImageElement.setAttribute( 'style', style.value ); - }; -} +/** + * Image style format descriptor. + * + * @typedef {Object} module:image/imagestyle/imagestyleengine~ImageStyleFormat + * @property {String} value Value used to store this style in model. + * 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 Icon name to use when creating style's toolbar button. + * @property {String} title Style's title. + * @property {String} className CSS class used to represent style in view. + */ diff --git a/src/imagetoolbar.js b/src/imagetoolbar.js index d9f94eef..5b0547e9 100644 --- a/src/imagetoolbar.js +++ b/src/imagetoolbar.js @@ -14,6 +14,7 @@ import ToolbarView from '../ui/toolbar/toolbarview.js'; import BalloonPanelView from '../ui/balloonpanel/balloonpanelview.js'; import Template from '../ui/template.js'; import ClickObserver from 'ckeditor5/engine/view/observer/clickobserver.js'; +import { isImageWidget } from './utils.js'; const arrowVOffset = BalloonPanelView.arrowVerticalOffset; const positions = { @@ -84,9 +85,11 @@ export default class ImageToolbar extends Plugin { editingView.addObserver( ClickObserver ); - // Check if the toolbar should be displayed each time the user clicked in editable. - editor.listenTo( editingView, 'click', () => { - if ( editingView.selection.isFake ) { + // Check if the toolbar should be displayed each time view is rendered. + editor.listenTo( editingView, 'render', () => { + const selectedElement = editingView.selection.getSelectedElement(); + + if ( selectedElement && isImageWidget( selectedElement ) ) { attachToolbar(); // TODO: These 2 need interval–based event debouncing for performance @@ -99,7 +102,7 @@ export default class ImageToolbar extends Plugin { editor.ui.view.stopListening( window, 'scroll', attachToolbar ); editor.ui.view.stopListening( window, 'resize', attachToolbar ); } - } ); + }, { priority: 'low' } ); function attachToolbar() { panel.attachTo( { diff --git a/tests/imagestyle/imagestyleengine.js b/tests/imagestyle/imagestyleengine.js new file mode 100644 index 00000000..cf07c3ad --- /dev/null +++ b/tests/imagestyle/imagestyleengine.js @@ -0,0 +1,72 @@ +/** + * @license Copyright (c) 2003-2016, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md. + */ + +import VirtualTestEditor from 'tests/core/_utils/virtualtesteditor.js'; +import ImageStyleEngine from 'ckeditor5/image/imagestyle/imagestyleengine.js'; +import ImageEngine from 'ckeditor5/image/imageengine.js'; +import ImageStyleCommand from 'ckeditor5/image/imagestyle/imagestylecommand.js'; +import { getData } from 'ckeditor5/engine/dev-utils/model.js'; + +describe( 'ImageStyleEngine', () => { + let editor, document; + + beforeEach( () => { + return VirtualTestEditor.create( { + plugins: [ ImageStyleEngine ], + } ) + .then( newEditor => { + editor = newEditor; + document = editor.document; + } ); + } ); + + it( 'should be loaded', () => { + expect( editor.plugins.get( ImageStyleEngine ) ).to.be.instanceOf( ImageStyleEngine ); + } ); + + it( 'should load image engine', () => { + expect( editor.plugins.get( ImageEngine ) ).to.be.instanceOf( ImageEngine ); + } ); + + it( 'should set schema rules for image style', () => { + const schema = document.schema; + + 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 ); + } ); + + // TODO: check default configuration. + + it( 'should convert from view to model', () => { + editor.setData( '
' ); + + expect( getData( document, { withoutSelection: true } ) ).to.equal( '' ); + } ); + + it( 'should not convert from view to model if class is not defined', () => { + editor.setData( '
' ); + + expect( getData( document, { withoutSelection: true } ) ).to.equal( '' ); + } ); + + it( 'should not convert from view to model when not in image figure', () => { + editor.setData( '
' ); + + expect( getData( document, { withoutSelection: true } ) ).to.equal( '' ); + } ); + + it( 'should not convert from view to model if schema prevents it', () => { + document.schema.disallow( { name: 'image', attributes: 'imageStyle' } ); + editor.setData( '
' ); + + expect( getData( document, { withoutSelection: true } ) ).to.equal( '' ); + } ); +} ); From b8a6178e81efd7db0965c9734655a4b9300008a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szymon=20Kup=C5=9B?= Date: Thu, 22 Dec 2016 01:20:39 +0100 Subject: [PATCH 08/35] Added styles to ImageStyleEngine. --- src/imagestyle/converters.js | 6 +- src/imagestyle/imagestyle.js | 2 +- src/imagestyle/imagestyleengine.js | 13 ++- tests/imagestyle/imagestyleengine.js | 147 +++++++++++++++++++++++++-- 4 files changed, 150 insertions(+), 18 deletions(-) diff --git a/src/imagestyle/converters.js b/src/imagestyle/converters.js index e35a5be2..5f88a203 100644 --- a/src/imagestyle/converters.js +++ b/src/imagestyle/converters.js @@ -12,7 +12,7 @@ import { isImage, getStyleByValue } from './utils.js'; export function addStyle( styles ) { return ( event, data, consumable, conversionApi ) => { // Check if we can consume, and we are adding in image. - if ( !consumable.consume( data.item, 'addAttribute:imageStyle' ) || !isImage( data.item ) ) { + if ( !consumable.test( data.item, 'addAttribute:imageStyle' ) || !isImage( data.item ) ) { return; } @@ -24,13 +24,14 @@ export function addStyle( styles ) { return; } + consumable.consume( data.item, 'addAttribute:imageStyle' ); conversionApi.mapper.toViewElement( data.item ).addClass( newStyle.className ); }; } export function changeStyle( styles ) { return ( event, data, consumable, conversionApi ) => { - if ( !consumable.consume( data.item, 'changeAttribute:imageStyle' ) || !isImage( data.item ) ) { + if ( !consumable.test( data.item, 'changeAttribute:imageStyle' ) || !isImage( data.item ) ) { return; } @@ -43,6 +44,7 @@ export function changeStyle( styles ) { return; } + consumable.consume( data.item, 'changeAttribute:imageStyle' ); const viewElement = conversionApi.mapper.toViewElement( data.item ); viewElement.removeClass( data.attributeOldValue ); viewElement.addClass( newStyle.className ); diff --git a/src/imagestyle/imagestyle.js b/src/imagestyle/imagestyle.js index 6dd8e1a8..4bc62f5d 100644 --- a/src/imagestyle/imagestyle.js +++ b/src/imagestyle/imagestyle.js @@ -26,7 +26,7 @@ export default class ImageStyle extends Plugin { const editor = this.editor; // Get configuration. - const styles = editor.config.get( 'image.styles.options' ); + const styles = editor.config.get( 'image.styles' ); for ( let name in styles ) { this._createButton( name, styles[ name ] ); diff --git a/src/imagestyle/imagestyleengine.js b/src/imagestyle/imagestyleengine.js index dca8399e..9d3bf74a 100644 --- a/src/imagestyle/imagestyleengine.js +++ b/src/imagestyle/imagestyleengine.js @@ -38,17 +38,15 @@ export default class ImageStyleEngine extends Plugin { // Define default configuration. editor.config.define( 'image.styles', { - options: { - // This option is equal to situation when no style is applied at all. - imageStyleFull: { title: 'Full size image', icon: 'object-center', value: null }, + // This option is equal to situation when no style is applied at all. + imageStyleFull: { title: 'Full size image', icon: 'object-center', value: null }, - // This represents side image. - imageStyleSide: { title: 'Side image', icon: 'object-right', value: 'side', className: 'image-style-side' } - } + // This represents side image. + imageStyleSide: { title: 'Side image', icon: 'object-right', value: 'side', className: 'image-style-side' } } ); // Get configuration. - const styles = editor.config.get( 'image.styles.options' ); + const styles = editor.config.get( 'image.styles' ); // Allow imageStyle attribute in image. // We could call it 'style' but https://github.com/ckeditor/ckeditor5-engine/issues/559. @@ -60,6 +58,7 @@ export default class ImageStyleEngine extends Plugin { editing.modelToView.on( 'changeAttribute:imageStyle', changeStyle( styles ) ); data.modelToView.on( 'changeAttribute:imageStyle', changeStyle( styles ) ); editing.modelToView.on( 'removeAttribute:imageStyle', removeStyle( styles ) ); + data.modelToView.on( 'removeAttribute:imageStyle', removeStyle( styles ) ); for ( let key in styles ) { const style = styles[ key ]; diff --git a/tests/imagestyle/imagestyleengine.js b/tests/imagestyle/imagestyleengine.js index cf07c3ad..fc27e595 100644 --- a/tests/imagestyle/imagestyleengine.js +++ b/tests/imagestyle/imagestyleengine.js @@ -7,18 +7,25 @@ import VirtualTestEditor from 'tests/core/_utils/virtualtesteditor.js'; import ImageStyleEngine from 'ckeditor5/image/imagestyle/imagestyleengine.js'; import ImageEngine from 'ckeditor5/image/imageengine.js'; import ImageStyleCommand from 'ckeditor5/image/imagestyle/imagestylecommand.js'; -import { getData } from 'ckeditor5/engine/dev-utils/model.js'; +import { getData as getModelData, setData as setModelData } from 'ckeditor5/engine/dev-utils/model.js'; +import { getData as getViewData } from 'ckeditor5/engine/dev-utils/view.js'; describe( 'ImageStyleEngine', () => { - let editor, document; + let editor, document, viewDocument; beforeEach( () => { return VirtualTestEditor.create( { plugins: [ ImageStyleEngine ], + image: { + styles: { + imageStyleDummy: { title: 'Dummy style', icon: 'dummy-icon', value: 'dummy', className: 'image-style-dummy' } + } + } } ) .then( newEditor => { editor = newEditor; document = editor.document; + viewDocument = editor.editing.view; } ); } ); @@ -43,30 +50,154 @@ describe( 'ImageStyleEngine', () => { expect( command ).to.be.instanceOf( ImageStyleCommand ); } ); - // TODO: check default configuration. - it( 'should convert from view to model', () => { editor.setData( '
' ); - expect( getData( document, { withoutSelection: true } ) ).to.equal( '' ); + expect( getModelData( document, { withoutSelection: true } ) ).to.equal( '' ); } ); it( 'should not convert from view to model if class is not defined', () => { editor.setData( '
' ); - expect( getData( document, { withoutSelection: true } ) ).to.equal( '' ); + expect( getModelData( document, { withoutSelection: true } ) ).to.equal( '' ); } ); it( 'should not convert from view to model when not in image figure', () => { editor.setData( '
' ); - expect( getData( document, { withoutSelection: true } ) ).to.equal( '' ); + expect( getModelData( document, { withoutSelection: true } ) ).to.equal( '' ); } ); it( 'should not convert from view to model if schema prevents it', () => { document.schema.disallow( { name: 'image', attributes: 'imageStyle' } ); editor.setData( '
' ); - expect( getData( document, { withoutSelection: true } ) ).to.equal( '' ); + expect( getModelData( document, { withoutSelection: true } ) ).to.equal( '' ); + } ); + + it( 'should convert model to view: adding attribute', () => { + setModelData( document, '' ); + const image = document.getRoot().getChild( 0 ); + const batch = document.batch(); + + document.enqueueChanges( () => { + batch.setAttribute( image, 'imageStyle', 'side' ); + } ); + + expect( editor.getData() ).to.equal( '
' ); + } ); + + it( 'should convert model to view: removing attribute', () => { + setModelData( document, '' ); + const image = document.getRoot().getChild( 0 ); + const batch = document.batch(); + + document.enqueueChanges( () => { + batch.setAttribute( image, 'imageStyle', null ); + } ); + + expect( editor.getData() ).to.equal( '
' ); + } ); + + it( 'should convert model to view: change attribute', () => { + setModelData( document, '' ); + const image = document.getRoot().getChild( 0 ); + const batch = document.batch(); + + document.enqueueChanges( () => { + batch.setAttribute( image, 'imageStyle', 'side' ); + } ); + + expect( editor.getData() ).to.equal( '
' ); + } ); + + it( 'should not convert from model to view if already consumed: adding attribute', () => { + editor.editing.modelToView.on( 'addAttribute:imageStyle', ( evt, data, consumable ) => { + consumable.consume( data.item, 'addAttribute:imageStyle' ); + }, { priority: 'high' } ); + + setModelData( document, '' ); + const image = document.getRoot().getChild( 0 ); + const batch = document.batch(); + + document.enqueueChanges( () => { + batch.setAttribute( image, 'imageStyle', 'side' ); + } ); + + expect( getViewData( viewDocument, { withoutSelection: true } ) ).to.equal( + '
' + ); + } ); + + it( 'should not convert from model to view if already consumed: removing attribute', () => { + editor.editing.modelToView.on( 'removeAttribute:imageStyle', ( evt, data, consumable ) => { + consumable.consume( data.item, 'removeAttribute:imageStyle' ); + }, { priority: 'high' } ); + + setModelData( document, '' ); + const image = document.getRoot().getChild( 0 ); + const batch = document.batch(); + + document.enqueueChanges( () => { + batch.setAttribute( image, 'imageStyle', null ); + } ); + + expect( getViewData( viewDocument, { withoutSelection: true } ) ).to.equal( + '
' + ); + } ); + + it( 'should not convert from model to view if already consumed: change attribute', () => { + editor.editing.modelToView.on( 'changeAttribute:imageStyle', ( evt, data, consumable ) => { + consumable.consume( data.item, 'changeAttribute:imageStyle' ); + }, { priority: 'high' } ); + + setModelData( document, '' ); + const image = document.getRoot().getChild( 0 ); + const batch = document.batch(); + + document.enqueueChanges( () => { + batch.setAttribute( image, 'imageStyle', 'side' ); + } ); + + expect( getViewData( viewDocument, { withoutSelection: true } ) ).to.equal( + '
' + ); + } ); + + it( 'should not convert from model to view if style is not present: adding attribute', () => { + setModelData( document, '' ); + const image = document.getRoot().getChild( 0 ); + const batch = document.batch(); + + document.enqueueChanges( () => { + batch.setAttribute( image, 'imageStyle', 'foo' ); + } ); + + expect( editor.getData() ).to.equal( '
' ); + } ); + + it( 'should not convert from model to view if style is not present: change attribute', () => { + setModelData( document, '' ); + const image = document.getRoot().getChild( 0 ); + const batch = document.batch(); + + document.enqueueChanges( () => { + batch.setAttribute( image, 'imageStyle', 'foo' ); + } ); + + expect( editor.getData() ).to.equal( '
' ); + } ); + + it( 'should not convert from model to view if style is not present: remove attribute', () => { + setModelData( document, '' ); + const image = document.getRoot().getChild( 0 ); + const batch = document.batch(); + + document.enqueueChanges( () => { + batch.setAttribute( image, 'imageStyle', null ); + } ); + + expect( editor.getData() ).to.equal( '
' ); } ); } ); From 98e4051829fb030d100b26107c449b452e610844 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szymon=20Kup=C5=9B?= Date: Thu, 22 Dec 2016 09:07:21 +0100 Subject: [PATCH 09/35] Simplified converters. --- src/imagestyle/converters.js | 77 ++++++++++++------------------ src/imagestyle/imagestyleengine.js | 20 ++++---- 2 files changed, 40 insertions(+), 57 deletions(-) diff --git a/src/imagestyle/converters.js b/src/imagestyle/converters.js index 5f88a203..43ddbc2a 100644 --- a/src/imagestyle/converters.js +++ b/src/imagestyle/converters.js @@ -9,70 +9,53 @@ import { isImage, getStyleByValue } from './utils.js'; -export function addStyle( styles ) { - return ( event, data, consumable, conversionApi ) => { - // Check if we can consume, and we are adding in image. - if ( !consumable.test( data.item, 'addAttribute:imageStyle' ) || !isImage( data.item ) ) { - return; - } - - // Check if there is class name associated with given value. - const newStyle = getStyleByValue( data.attributeNewValue, styles ); - - // Check if new style is allowed in configuration. - if ( !newStyle ) { - return; - } - - consumable.consume( data.item, 'addAttribute:imageStyle' ); - conversionApi.mapper.toViewElement( data.item ).addClass( newStyle.className ); - }; -} +/** + * Returns converter for `imageStyle` attribute. It can be used for adding, changing and removing the attribute. + * + * @param {Object} styles Object containing available styles. See {@link module:image/imagestyle/imagestyleengine~ImageStyleFormat} + * for more details. + * @return {Function} Model to view attribute converter. + */ +export function modelToViewSetStyle( styles ) { + return ( evt, data, consumable, conversionApi ) => { + const eventType = evt.name.split( ':' )[ 0 ]; + const consumableType = eventType + ':imageStyle'; -export function changeStyle( styles ) { - return ( event, data, consumable, conversionApi ) => { - if ( !consumable.test( data.item, 'changeAttribute:imageStyle' ) || !isImage( data.item ) ) { + if ( !consumable.test( data.item, consumableType ) ) { return; } // Check if there is class name associated with given value. const newStyle = getStyleByValue( data.attributeNewValue, styles ); const oldStyle = getStyleByValue( data.attributeOldValue, styles ); - - // Check if new style is allowed in configuration. - if ( !newStyle || !oldStyle ) { - return; - } - - consumable.consume( data.item, 'changeAttribute:imageStyle' ); const viewElement = conversionApi.mapper.toViewElement( data.item ); - viewElement.removeClass( data.attributeOldValue ); - viewElement.addClass( newStyle.className ); - }; -} -export function removeStyle( styles ) { - return ( event, data, consumable, conversionApi ) => { - if ( !consumable.test( data.item, 'removeAttribute:imageStyle' ) || !isImage( data.item ) ) { - return; + if ( eventType == 'addAttribute' || eventType == 'changeAttribute' ) { + if ( !newStyle ) { + return; + } + + viewElement.addClass( newStyle.className ); } - // Check if there is class name associated with given value. - const newStyle = getStyleByValue( data.attributeNewValue, styles ); - const oldStyle = getStyleByValue( data.attributeOldValue, styles ); + if ( eventType == 'changeAttribute' || eventType == 'removeAttribute' ) { + if ( !oldStyle ) { + return; + } - // Check if styles are allowed in configuration. - if ( !newStyle || !oldStyle ) { - return; + viewElement.removeClass( data.attributeOldValue ); } - consumable.consume( data.item, 'removeAttribute:imageStyle' ); - - const viewElement = conversionApi.mapper.toViewElement( data.item ); - viewElement.removeClass( oldStyle.className ); + consumable.consume( data.item, consumableType ); }; } +/** + * Returns view to model converter converting image style CSS class to proper value in the model. + * + * @param {module:image/imagestyle/imagestyleengine~ImageStyleFormat} style Style for which converter is created. + * @return {Function} View to model converter. + */ export function viewToModelImageStyle( style ) { return ( evt, data, consumable, conversionApi ) => { const viewFigureElement = data.input; diff --git a/src/imagestyle/imagestyleengine.js b/src/imagestyle/imagestyleengine.js index 9d3bf74a..4ea7f083 100644 --- a/src/imagestyle/imagestyleengine.js +++ b/src/imagestyle/imagestyleengine.js @@ -10,7 +10,7 @@ import Plugin from '../../core/plugin.js'; import ImageStyleCommand from './imagestylecommand.js'; import ImageEngine from '../imageengine.js'; -import { addStyle, changeStyle, removeStyle, viewToModelImageStyle } from './converters.js'; +import { viewToModelImageStyle, modelToViewSetStyle } from './converters.js'; /** * The image style engine plugin. Sets default configuration, creates converters and registers @@ -52,18 +52,18 @@ export default class ImageStyleEngine extends Plugin { // We could call it 'style' but https://github.com/ckeditor/ckeditor5-engine/issues/559. schema.allow( { name: 'image', attributes: 'imageStyle', inside: '$root' } ); - // Converters for models element imageStyle attribute. - editing.modelToView.on( 'addAttribute:imageStyle', addStyle( styles ) ); - data.modelToView.on( 'addAttribute:imageStyle', addStyle( styles ) ); - editing.modelToView.on( 'changeAttribute:imageStyle', changeStyle( styles ) ); - data.modelToView.on( 'changeAttribute:imageStyle', changeStyle( styles ) ); - editing.modelToView.on( 'removeAttribute:imageStyle', removeStyle( styles ) ); - data.modelToView.on( 'removeAttribute:imageStyle', removeStyle( styles ) ); + // Converters for imageStyle attribute from model to view. + editing.modelToView.on( 'addAttribute:imageStyle:image', modelToViewSetStyle( styles ) ); + data.modelToView.on( 'addAttribute:imageStyle:image', modelToViewSetStyle( styles ) ); + editing.modelToView.on( 'changeAttribute:imageStyle:image', modelToViewSetStyle( styles ) ); + data.modelToView.on( 'changeAttribute:imageStyle:image', modelToViewSetStyle( styles ) ); + editing.modelToView.on( 'removeAttribute:imageStyle:image', modelToViewSetStyle( styles ) ); + data.modelToView.on( 'removeAttribute:imageStyle:image', modelToViewSetStyle( styles ) ); + // Converter for figure element from view to model. for ( let key in styles ) { const style = styles[ key ]; - // 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' } ); @@ -79,7 +79,7 @@ export default class ImageStyleEngine extends Plugin { * Image style format descriptor. * * @typedef {Object} module:image/imagestyle/imagestyleengine~ImageStyleFormat - * @property {String} value Value used to store this style in model. + * @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 Icon name to use when creating style's toolbar button. * @property {String} title Style's title. From a005ff78de419c772f5f4e250be30bf9ad186d12 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szymon=20Kup=C5=9B?= Date: Thu, 22 Dec 2016 11:12:45 +0100 Subject: [PATCH 10/35] Changed ImageStyle configuration object to array. --- src/imagestyle/imagestyle.js | 8 ++++---- src/imagestyle/imagestylecommand.js | 6 ++---- src/imagestyle/imagestyleengine.js | 14 +++++++------- src/imagestyle/utils.js | 4 +--- tests/imagestyle/imagestylecommand.js | 8 ++++---- tests/imagestyle/imagestyleengine.js | 22 ++++++++++++---------- 6 files changed, 30 insertions(+), 32 deletions(-) diff --git a/src/imagestyle/imagestyle.js b/src/imagestyle/imagestyle.js index 4bc62f5d..7c0797e7 100644 --- a/src/imagestyle/imagestyle.js +++ b/src/imagestyle/imagestyle.js @@ -28,18 +28,18 @@ export default class ImageStyle extends Plugin { // Get configuration. const styles = editor.config.get( 'image.styles' ); - for ( let name in styles ) { - this._createButton( name, styles[ name ] ); + for ( let style of styles ) { + this._createButton( style ); } } - _createButton( name, style ) { + _createButton( style ) { const editor = this.editor; const command = editor.commands.get( 'imagestyle' ); const t = editor.t; // Add bold button to feature components. - editor.ui.componentFactory.add( name, ( locale ) => { + editor.ui.componentFactory.add( style.name, ( locale ) => { const view = new ButtonView( locale ); view.set( { diff --git a/src/imagestyle/imagestylecommand.js b/src/imagestyle/imagestylecommand.js index 1c0e02f6..44ffeacf 100644 --- a/src/imagestyle/imagestylecommand.js +++ b/src/imagestyle/imagestylecommand.js @@ -20,12 +20,10 @@ export default class ImageStyleCommand extends Command { * Creates instance of the command. * * @param {module:core/editor/editor~Editor} editor Editor instance. - * @param {Object} styles Allowed styles. See {@link module:image/imagestyle/imagestyleengine~ImageStyleFormat - * ImageStyleFormat} for more information. + * @param {Array.} styles Allowed styles. */ constructor( editor, styles ) { super( editor ); - /** * The current style value. * @@ -39,7 +37,7 @@ export default class ImageStyleCommand extends Command { * Image styles uses by this command. * * @readonly - * @member {Object} Allowed image styles. See {@link module:image/imagestyle/imagestyleengine~ImageStyleFormat}. + * @member {Array.} Allowed image styles. */ this.styles = styles; diff --git a/src/imagestyle/imagestyleengine.js b/src/imagestyle/imagestyleengine.js index 4ea7f083..be998226 100644 --- a/src/imagestyle/imagestyleengine.js +++ b/src/imagestyle/imagestyleengine.js @@ -37,13 +37,13 @@ export default class ImageStyleEngine extends Plugin { const editing = editor.editing; // Define default configuration. - editor.config.define( 'image.styles', { + editor.config.define( 'image.styles', [ // This option is equal to situation when no style is applied at all. - imageStyleFull: { title: 'Full size image', icon: 'object-center', value: null }, + { name: 'imageStyleFull', title: 'Full size image', icon: 'object-center', value: null }, // This represents side image. - imageStyleSide: { title: 'Side image', icon: 'object-right', value: 'side', className: 'image-style-side' } - } ); + { name: 'imageStyleSide', title: 'Side image', icon: 'object-right', value: 'side', className: 'image-style-side' } + ] ); // Get configuration. const styles = editor.config.get( 'image.styles' ); @@ -61,9 +61,7 @@ export default class ImageStyleEngine extends Plugin { data.modelToView.on( 'removeAttribute:imageStyle:image', modelToViewSetStyle( styles ) ); // Converter for figure element from view to model. - for ( let key in styles ) { - const style = styles[ key ]; - + for ( let style of styles ) { // Create converter only for non-null values. if ( style.value !== null ) { data.viewToModel.on( 'element:figure', viewToModelImageStyle( style ), { priority: 'low' } ); @@ -79,6 +77,8 @@ export default class ImageStyleEngine extends Plugin { * Image style format descriptor. * * @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} 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 Icon name to use when creating style's toolbar button. diff --git a/src/imagestyle/utils.js b/src/imagestyle/utils.js index 63a52c00..b0caa386 100644 --- a/src/imagestyle/utils.js +++ b/src/imagestyle/utils.js @@ -14,9 +14,7 @@ export function isImage( modelElement ) { } export function getStyleByValue( value, styles ) { - for ( let key in styles ) { - const style = styles[ key ]; - + for ( let style of styles ) { if ( style.value === value ) { return style; } diff --git a/tests/imagestyle/imagestylecommand.js b/tests/imagestyle/imagestylecommand.js index c4a55074..b04ead68 100644 --- a/tests/imagestyle/imagestylecommand.js +++ b/tests/imagestyle/imagestylecommand.js @@ -8,10 +8,10 @@ import ImageStyleCommand from 'ckeditor5/image/imagestyle/imagestylecommand.js'; import { setData, getData } from 'ckeditor5/engine/dev-utils/model.js'; describe( 'ImageStyleCommand', () => { - const styles = { - defaultStyle: { title: 'foo bar', icon: 'icon-1', value: null }, - otherStyle: { title: 'baz', icon: 'icon-2', value: 'other', className: 'other-class-name' } - }; + 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' } + ]; let document, command; diff --git a/tests/imagestyle/imagestyleengine.js b/tests/imagestyle/imagestyleengine.js index fc27e595..a975ff47 100644 --- a/tests/imagestyle/imagestyleengine.js +++ b/tests/imagestyle/imagestyleengine.js @@ -17,9 +17,11 @@ describe( 'ImageStyleEngine', () => { return VirtualTestEditor.create( { plugins: [ ImageStyleEngine ], image: { - styles: { - imageStyleDummy: { title: 'Dummy style', icon: 'dummy-icon', value: 'dummy', className: 'image-style-dummy' } - } + styles: [ + { name: 'fullStyle', title: 'foo', icon: 'object-center', value: null }, + { name: 'sideStyle', title: 'bar', icon: 'object-right', value: 'side', className: 'side-class' }, + { name: 'dummyStyle', title: 'baz', icon: 'object-dummy', value: 'dummy', className: 'dummy-class' } + ] } } ) .then( newEditor => { @@ -51,7 +53,7 @@ describe( 'ImageStyleEngine', () => { } ); it( 'should convert from view to model', () => { - editor.setData( '
' ); + editor.setData( '
' ); expect( getModelData( document, { withoutSelection: true } ) ).to.equal( '' ); } ); @@ -63,14 +65,14 @@ describe( 'ImageStyleEngine', () => { } ); it( 'should not convert from view to model when not in image figure', () => { - editor.setData( '
' ); + editor.setData( '
' ); expect( getModelData( document, { withoutSelection: true } ) ).to.equal( '' ); } ); it( 'should not convert from view to model if schema prevents it', () => { document.schema.disallow( { name: 'image', attributes: 'imageStyle' } ); - editor.setData( '
' ); + editor.setData( '
' ); expect( getModelData( document, { withoutSelection: true } ) ).to.equal( '' ); } ); @@ -84,7 +86,7 @@ describe( 'ImageStyleEngine', () => { batch.setAttribute( image, 'imageStyle', 'side' ); } ); - expect( editor.getData() ).to.equal( '
' ); + expect( editor.getData() ).to.equal( '
' ); } ); it( 'should convert model to view: removing attribute', () => { @@ -108,7 +110,7 @@ describe( 'ImageStyleEngine', () => { batch.setAttribute( image, 'imageStyle', 'side' ); } ); - expect( editor.getData() ).to.equal( '
' ); + expect( editor.getData() ).to.equal( '
' ); } ); it( 'should not convert from model to view if already consumed: adding attribute', () => { @@ -143,7 +145,7 @@ describe( 'ImageStyleEngine', () => { } ); expect( getViewData( viewDocument, { withoutSelection: true } ) ).to.equal( - '
' + '
' ); } ); @@ -161,7 +163,7 @@ describe( 'ImageStyleEngine', () => { } ); expect( getViewData( viewDocument, { withoutSelection: true } ) ).to.equal( - '
' + '
' ); } ); From 6e15b810ddcf8a4d7a6b21fb4a282df2e5399434 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szymon=20Kup=C5=9B?= Date: Thu, 22 Dec 2016 15:14:15 +0100 Subject: [PATCH 11/35] Added docs and tests to ImageStyle plugin. --- src/imagestyle/imagestyle.js | 19 +++++--- src/imagestyle/imagestylecommand.js | 1 - tests/imagestyle/imagestyle.js | 70 +++++++++++++++++++++++++++++ 3 files changed, 84 insertions(+), 6 deletions(-) create mode 100644 tests/imagestyle/imagestyle.js diff --git a/src/imagestyle/imagestyle.js b/src/imagestyle/imagestyle.js index 7c0797e7..b07a438f 100644 --- a/src/imagestyle/imagestyle.js +++ b/src/imagestyle/imagestyle.js @@ -11,6 +11,13 @@ import Plugin from '../../core/plugin.js'; import ImageStyleEngine from './imagestyleengine.js'; import ButtonView from '../../ui/button/buttonview.js'; +/** + * The image style plugin. + * + * Uses {@link module:image/imagestyle/imagestyleengine~ImageStyleEngine}. + * + * @extends module:core/plugin~Plugin + */ export default class ImageStyle extends Plugin { /** * @inheritDoc @@ -23,16 +30,19 @@ export default class ImageStyle extends Plugin { * @inheritDoc */ init() { - const editor = this.editor; - - // Get configuration. - const styles = editor.config.get( 'image.styles' ); + const styles = this.editor.config.get( 'image.styles' ); for ( let style of styles ) { this._createButton( style ); } } + /** + * Creates button for each style and stores it in editor's {@link module:ui/componentfactory~ComponentFactory ComponentFactory}. + * + * @private + * @param {module:image/imagestyle/imagestyleengine~ImageStyleFormat} style + */ _createButton( style ) { const editor = this.editor; const command = editor.commands.get( 'imagestyle' ); @@ -52,7 +62,6 @@ export default class ImageStyle extends Plugin { return commandValue == style.value; } ); - // // Execute command. this.listenTo( view, 'execute', () => editor.execute( 'imagestyle', { value: style.value } ) ); return view; diff --git a/src/imagestyle/imagestylecommand.js b/src/imagestyle/imagestylecommand.js index 44ffeacf..ff92f889 100644 --- a/src/imagestyle/imagestylecommand.js +++ b/src/imagestyle/imagestylecommand.js @@ -116,4 +116,3 @@ export default class ImageStyleCommand extends Command { } ); } } - diff --git a/tests/imagestyle/imagestyle.js b/tests/imagestyle/imagestyle.js new file mode 100644 index 00000000..a642c912 --- /dev/null +++ b/tests/imagestyle/imagestyle.js @@ -0,0 +1,70 @@ +/** + * @license Copyright (c) 2003-2016, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md. + */ + +/* global document */ + +import ClassicTestEditor from 'tests/core/_utils/classictesteditor.js'; +import ImageStyle from 'ckeditor5/image/imagestyle/imagestyle.js'; +import ImageStyleEngine from 'ckeditor5/image/imagestyle/imagestyleengine.js'; +import ButtonView from 'ckeditor5/ui/button/buttonview.js'; + +describe( 'ImageStyle', () => { + let editor; + const styles = [ + { name: 'style 1', title: 'Style 1 title', icon: 'style1-icon', value: null }, + { name: 'style 2', title: 'Style 2 title', icon: 'style2-icon', value: 'style2', cssClass: 'style2-class' }, + { name: 'style 3', title: 'Style 3 title', icon: 'style3-icon', value: 'style3', cssClass: 'style3-class' } + ]; + + beforeEach( () => { + const editorElement = document.createElement( 'div' ); + document.body.appendChild( editorElement ); + + return ClassicTestEditor.create( editorElement, { + plugins: [ ImageStyle ], + image: { + styles + } + } ) + .then( newEditor => { + editor = newEditor; + } ); + } ); + + afterEach( () => { + return editor.destroy(); + } ); + + it( 'should be loaded', () => { + expect( editor.plugins.get( ImageStyle ) ).to.be.instanceOf( ImageStyle ); + } ); + + it( 'should load ImageStyleEngine plugin', () => { + expect( editor.plugins.get( ImageStyleEngine ) ).to.be.instanceOf( ImageStyleEngine ); + } ); + + 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 buttonView = editor.ui.componentFactory.create( style.name ); + + expect( buttonView ).to.be.instanceOf( ButtonView ); + expect( buttonView.label ).to.equal( style.title ); + expect( buttonView.icon ).to.equal( style.icon ); + + command.isEnabled = true; + expect( buttonView.isEnabled ).to.be.true; + command.isEnabled = false; + expect( buttonView.isEnabled ).to.be.false; + + buttonView.fire( 'execute' ); + sinon.assert.calledWithExactly( editor.execute, 'imagestyle', { value: style.value } ); + + spy.reset(); + } + } ); +} ); From 70045818aacad357c074722ff5f5002340a60c29 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szymon=20Kup=C5=9B?= Date: Thu, 22 Dec 2016 15:31:53 +0100 Subject: [PATCH 12/35] Docs fixes. --- src/imagestyle/converters.js | 4 ++-- src/imagestyle/imagestylecommand.js | 4 ++-- src/imagestyle/utils.js | 14 ++++++++++++++ 3 files changed, 18 insertions(+), 4 deletions(-) diff --git a/src/imagestyle/converters.js b/src/imagestyle/converters.js index 43ddbc2a..795a25ab 100644 --- a/src/imagestyle/converters.js +++ b/src/imagestyle/converters.js @@ -14,7 +14,7 @@ import { isImage, getStyleByValue } from './utils.js'; * * @param {Object} styles Object containing available styles. See {@link module:image/imagestyle/imagestyleengine~ImageStyleFormat} * for more details. - * @return {Function} Model to view attribute converter. + * @returns {Function} Model to view attribute converter. */ export function modelToViewSetStyle( styles ) { return ( evt, data, consumable, conversionApi ) => { @@ -54,7 +54,7 @@ export function modelToViewSetStyle( styles ) { * Returns view to model converter converting image style CSS class to proper value in the model. * * @param {module:image/imagestyle/imagestyleengine~ImageStyleFormat} style Style for which converter is created. - * @return {Function} View to model converter. + * @returns {Function} View to model converter. */ export function viewToModelImageStyle( style ) { return ( evt, data, consumable, conversionApi ) => { diff --git a/src/imagestyle/imagestylecommand.js b/src/imagestyle/imagestylecommand.js index ff92f889..f65ce146 100644 --- a/src/imagestyle/imagestylecommand.js +++ b/src/imagestyle/imagestylecommand.js @@ -34,10 +34,10 @@ export default class ImageStyleCommand extends Command { this.set( 'value', false ); /** - * Image styles uses by this command. + * Allowed image styles used by this command. * * @readonly - * @member {Array.} Allowed image styles. + * @member {Array.} #styles */ this.styles = styles; diff --git a/src/imagestyle/utils.js b/src/imagestyle/utils.js index b0caa386..f504546e 100644 --- a/src/imagestyle/utils.js +++ b/src/imagestyle/utils.js @@ -9,10 +9,24 @@ import ModelElement from '../../engine/model/element.js'; +/** + * Checks if provided modelElement is an instance of {@link module:engine/model/element~Element Element} and its name + * equals to `image`. + * + * @param {module:engine/model/element~Element} modelElement + * @returns {Boolean} + */ export function isImage( modelElement ) { return modelElement instanceof ModelElement && modelElement.name == 'image'; } +/** + * Returns style with given `value` from array of styles. + * + * @param {String} value + * @param {Array. } styles + * @return {module:image/imagestyle/imagestyleengine~ImageStyleFormat|undefined} + */ export function getStyleByValue( value, styles ) { for ( let style of styles ) { if ( style.value === value ) { From 18b2819b86cf92505f2a6797a5542538bfa6f5da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szymon=20Kup=C5=9B?= Date: Thu, 22 Dec 2016 15:48:50 +0100 Subject: [PATCH 13/35] Fixed model to view converter for image styles. --- src/imagestyle/converters.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/imagestyle/converters.js b/src/imagestyle/converters.js index 795a25ab..a439a3b4 100644 --- a/src/imagestyle/converters.js +++ b/src/imagestyle/converters.js @@ -30,20 +30,20 @@ export function modelToViewSetStyle( styles ) { const oldStyle = getStyleByValue( data.attributeOldValue, styles ); const viewElement = conversionApi.mapper.toViewElement( data.item ); - if ( eventType == 'addAttribute' || eventType == 'changeAttribute' ) { - if ( !newStyle ) { + if ( eventType == 'changeAttribute' || eventType == 'removeAttribute' ) { + if ( !oldStyle ) { return; } - viewElement.addClass( newStyle.className ); + viewElement.removeClass( oldStyle.className ); } - if ( eventType == 'changeAttribute' || eventType == 'removeAttribute' ) { - if ( !oldStyle ) { + if ( eventType == 'addAttribute' || eventType == 'changeAttribute' ) { + if ( !newStyle ) { return; } - viewElement.removeClass( data.attributeOldValue ); + viewElement.addClass( newStyle.className ); } consumable.consume( data.item, consumableType ); From 8267f6fa1144229aff6680849f2dc874fc1e24b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szymon=20Kup=C5=9B?= Date: Thu, 22 Dec 2016 15:55:19 +0100 Subject: [PATCH 14/35] Updated description of image style manual test. --- tests/manual/imagestyle.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/manual/imagestyle.md b/tests/manual/imagestyle.md index d2159a2d..22ff3082 100644 --- a/tests/manual/imagestyle.md +++ b/tests/manual/imagestyle.md @@ -1 +1,5 @@ ## ImageStyle feature + +* Click on image - toolbar with icons should appear. "Full size image" icon should be selected. +* Click on "Side image" icon. Image should be aligned to right. +* Click on "Full size image" icon. Image should be back to its original state. From b90621f7eb3923ef576b29d08dfc5bd723325baa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szymon=20Kup=C5=9B?= Date: Thu, 22 Dec 2016 16:20:17 +0100 Subject: [PATCH 15/35] Added tests for image styles utilities. --- src/imagestyle/imagestyleengine.js | 2 +- tests/imagestyle/utils.js | 48 ++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 1 deletion(-) create mode 100644 tests/imagestyle/utils.js diff --git a/src/imagestyle/imagestyleengine.js b/src/imagestyle/imagestyleengine.js index be998226..d2cf34d4 100644 --- a/src/imagestyle/imagestyleengine.js +++ b/src/imagestyle/imagestyleengine.js @@ -38,7 +38,7 @@ export default class ImageStyleEngine extends Plugin { // Define default configuration. editor.config.define( 'image.styles', [ - // This option is equal to situation when no style is applied at all. + // This option is equal to situation when no style is applied. { name: 'imageStyleFull', title: 'Full size image', icon: 'object-center', value: null }, // This represents side image. diff --git a/tests/imagestyle/utils.js b/tests/imagestyle/utils.js new file mode 100644 index 00000000..6f577824 --- /dev/null +++ b/tests/imagestyle/utils.js @@ -0,0 +1,48 @@ +/** + * @license Copyright (c) 2003-2016, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md. + */ + +import { isImage, getStyleByValue } from 'ckeditor5/image/imagestyle/utils.js'; +import ModelElement from 'ckeditor5/engine/model/element.js'; + +describe( 'ImageStyle utils', () => { + describe( 'isImage', () => { + it( 'should return true for image element', () => { + const image = new ModelElement( 'image' ); + + expect( isImage( image ) ).to.be.true; + } ); + + it( 'should return true false for different elements', () => { + const image = new ModelElement( 'foo' ); + + expect( isImage( image ) ).to.be.false; + } ); + + it( 'should return true false for null and undefined', () => { + expect( isImage( null ) ).to.be.false; + expect( isImage( undefined ) ).to.be.false; + } ); + } ); + + describe( 'getStyleByValue', () => { + const styles = [ + { name: 'style 1', title: 'title 1', icon: 'style1-icon', value: 'style1', className: 'style1-class' }, + { name: 'style 2', title: 'title 2', icon: 'style2-icon', value: 'style2', className: 'style2-class' } + ]; + + it( 'should return proper styles for values', () => { + expect( getStyleByValue( 'style1', styles ) ).to.equal( styles[ 0 ] ); + expect( getStyleByValue( 'style2', styles ) ).to.equal( styles[ 1 ] ); + } ); + + it( 'should return undefined when style is not found', () => { + expect( getStyleByValue( 'foo', styles ) ).to.be.undefined; + } ); + + it( 'should return undefined when empty styles array is provided', () => { + expect( getStyleByValue( 'style1', [] ) ).to.be.undefined; + } ); + } ); +} ); From bb60e258c9537a62b7f9eb66634c8b4d9fc7dee9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szymon=20Kup=C5=9B?= Date: Thu, 22 Dec 2016 16:54:03 +0100 Subject: [PATCH 16/35] One converter for model to view conversion in ImageStyleEngine. --- src/imagestyle/imagestyleengine.js | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/imagestyle/imagestyleengine.js b/src/imagestyle/imagestyleengine.js index d2cf34d4..4c7c4a82 100644 --- a/src/imagestyle/imagestyleengine.js +++ b/src/imagestyle/imagestyleengine.js @@ -53,12 +53,13 @@ export default class ImageStyleEngine extends Plugin { schema.allow( { name: 'image', attributes: 'imageStyle', inside: '$root' } ); // Converters for imageStyle attribute from model to view. - editing.modelToView.on( 'addAttribute:imageStyle:image', modelToViewSetStyle( styles ) ); - data.modelToView.on( 'addAttribute:imageStyle:image', modelToViewSetStyle( styles ) ); - editing.modelToView.on( 'changeAttribute:imageStyle:image', modelToViewSetStyle( styles ) ); - data.modelToView.on( 'changeAttribute:imageStyle:image', modelToViewSetStyle( styles ) ); - editing.modelToView.on( 'removeAttribute:imageStyle:image', modelToViewSetStyle( styles ) ); - data.modelToView.on( 'removeAttribute:imageStyle:image', modelToViewSetStyle( styles ) ); + const modelToViewConverter = modelToViewSetStyle( styles ); + editing.modelToView.on( 'addAttribute:imageStyle:image', modelToViewConverter ); + data.modelToView.on( 'addAttribute:imageStyle:image', modelToViewConverter ); + editing.modelToView.on( 'changeAttribute:imageStyle:image', modelToViewConverter ); + data.modelToView.on( 'changeAttribute:imageStyle:image', modelToViewConverter ); + 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 ) { From 7ec5d65752cca509d536691b4b72cf1aa4f33665 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szymon=20Kup=C5=9B?= Date: Wed, 28 Dec 2016 09:04:03 +0100 Subject: [PATCH 17/35] Added lodash throttle method to scroll and resize events in ImageToolbar, docs. --- src/imagetoolbar.js | 35 ++++++++++++++++++++++------------- tests/manual/imagestyle.md | 1 + 2 files changed, 23 insertions(+), 13 deletions(-) diff --git a/src/imagetoolbar.js b/src/imagetoolbar.js index 5b0547e9..356b150b 100644 --- a/src/imagetoolbar.js +++ b/src/imagetoolbar.js @@ -7,22 +7,22 @@ * @module image/imagetoolbar */ -/* globals window */ - import Plugin from '../core/plugin.js'; import ToolbarView from '../ui/toolbar/toolbarview.js'; import BalloonPanelView from '../ui/balloonpanel/balloonpanelview.js'; import Template from '../ui/template.js'; import ClickObserver from 'ckeditor5/engine/view/observer/clickobserver.js'; import { isImageWidget } from './utils.js'; +import throttle from '../utils/lib/lodash/throttle.js'; +import global from '../utils/dom/global.js'; const arrowVOffset = BalloonPanelView.arrowVerticalOffset; const positions = { - // [text range] - // ^ - // +-----------------+ - // | Balloon | - // +-----------------+ + // [text range] + // ^ + // +-----------------+ + // | Balloon | + // +-----------------+ south: ( targetRect, balloonRect ) => ( { top: targetRect.bottom + arrowVOffset, left: targetRect.left + targetRect.width / 2 - balloonRect.width / 2, @@ -41,7 +41,17 @@ const positions = { } ) }; +/** + * Image toolbar class. Creates image toolbar placed inside balloon panel that is showed when image widget is selected. + * Toolbar components are created using editor's {@link module:ui/componentfactory~ComponentFactory ComponentFactory} + * based on {@link module:core/editor/editor~Editor#config configuration} stored under `image.toolbar`. + * + * @extends module:core/plugin~Plugin. + */ export default class ImageToolbar extends Plugin { + /** + * @inheritDoc + */ init() { const editor = this.editor; @@ -88,19 +98,18 @@ export default class ImageToolbar extends Plugin { // Check if the toolbar should be displayed each time view is rendered. editor.listenTo( editingView, 'render', () => { const selectedElement = editingView.selection.getSelectedElement(); + const attachToolbarCallback = throttle( attachToolbar, 100 ); if ( selectedElement && isImageWidget( selectedElement ) ) { attachToolbar(); - // TODO: These 2 need interval–based event debouncing for performance - // reasons. I guess even lodash offers such a helper. - editor.ui.view.listenTo( window, 'scroll', attachToolbar ); - editor.ui.view.listenTo( window, 'resize', attachToolbar ); + editor.ui.view.listenTo( global.window, 'scroll', attachToolbarCallback ); + editor.ui.view.listenTo( global.window, 'resize', attachToolbarCallback ); } else { panel.hide(); - editor.ui.view.stopListening( window, 'scroll', attachToolbar ); - editor.ui.view.stopListening( window, 'resize', attachToolbar ); + editor.ui.view.stopListening( global.window, 'scroll', attachToolbarCallback ); + editor.ui.view.stopListening( global.window, 'resize', attachToolbarCallback ); } }, { priority: 'low' } ); diff --git a/tests/manual/imagestyle.md b/tests/manual/imagestyle.md index 22ff3082..548d3d25 100644 --- a/tests/manual/imagestyle.md +++ b/tests/manual/imagestyle.md @@ -3,3 +3,4 @@ * Click on image - toolbar with icons should appear. "Full size image" icon should be selected. * Click on "Side image" icon. Image should be aligned to right. * Click on "Full size image" icon. Image should be back to its original state. +* When image toolbar is visible, resize the browser window and scroll - check if toolbar is placed in proper position. From 21c86c70cb3bfe24500ae928a8aa689dc41f323a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szymon=20Kup=C5=9B?= Date: Wed, 28 Dec 2016 11:46:09 +0100 Subject: [PATCH 18/35] ImageStyle adds all styles buttons to ImageToolbar if no other configuration is provided. --- src/imagestyle/imagestyle.js | 7 +++++++ src/imagetoolbar.js | 9 +++++++-- tests/imagestyle/imagestyle.js | 22 ++++++++++++++++++++++ tests/manual/imagestyle.js | 7 ++----- 4 files changed, 38 insertions(+), 7 deletions(-) diff --git a/src/imagestyle/imagestyle.js b/src/imagestyle/imagestyle.js index b07a438f..831594af 100644 --- a/src/imagestyle/imagestyle.js +++ b/src/imagestyle/imagestyle.js @@ -35,6 +35,13 @@ export default class ImageStyle extends Plugin { for ( let style of styles ) { this._createButton( style ); } + + // If there is no default image toolbar configuration, add all style buttons. + const imageToolbarConfig = this.editor.config.get( 'image.toolbar' ); + + if ( !imageToolbarConfig ) { + this.editor.config.set( 'image.toolbar', styles.map( style => style.name ) ); + } } /** diff --git a/src/imagetoolbar.js b/src/imagetoolbar.js index 356b150b..b882edb3 100644 --- a/src/imagetoolbar.js +++ b/src/imagetoolbar.js @@ -52,8 +52,14 @@ export default class ImageToolbar extends Plugin { /** * @inheritDoc */ - init() { + afterInit() { const editor = this.editor; + const toolbarConfig = editor.config.get( 'image.toolbar' ); + + // Don't add the toolbar if there is no configuration. + if ( !toolbarConfig ) { + return; + } // Create a plain toolbar instance. const toolbar = new ToolbarView(); @@ -74,7 +80,6 @@ export default class ImageToolbar extends Plugin { return editor.ui.view.body.add( panel ).then( () => { const editingView = editor.editing.view; - const toolbarConfig = editor.config.get( 'image.toolbar' ); const promises = []; if ( toolbarConfig ) { diff --git a/tests/imagestyle/imagestyle.js b/tests/imagestyle/imagestyle.js index a642c912..970c84e0 100644 --- a/tests/imagestyle/imagestyle.js +++ b/tests/imagestyle/imagestyle.js @@ -67,4 +67,26 @@ describe( 'ImageStyle', () => { spy.reset(); } } ); + + it( 'should add buttons to image toolbar if there is no default configuration', () => { + const toolbarConfig = editor.config.get( 'image.toolbar' ); + + expect( toolbarConfig ).to.eql( styles.map( style => style.name ) ); + } ); + + it( 'should not add buttons to image toolbar if configuration is present', () => { + const editorElement = document.createElement( 'div' ); + document.body.appendChild( editorElement ); + + return ClassicTestEditor.create( editorElement, { + plugins: [ ImageStyle ], + image: { + styles, + toolbar: [ 'foo', 'bar' ] + } + } ) + .then( newEditor => { + expect( newEditor.config.get( 'image.toolbar' ) ).to.eql( [ 'foo', 'bar' ] ); + } ); + } ); } ); diff --git a/tests/manual/imagestyle.js b/tests/manual/imagestyle.js index aeb1e53b..a2ee0136 100644 --- a/tests/manual/imagestyle.js +++ b/tests/manual/imagestyle.js @@ -17,11 +17,8 @@ import ImageStyle from 'ckeditor5/image/imagestyle/imagestyle.js'; import ImageToolbar from 'ckeditor5/image/imagetoolbar.js'; ClassicEditor.create( document.querySelector( '#editor' ), { - plugins: [ EnterPlugin, TypingPlugin, ParagraphPlugin, HeadingPlugin, ImagePlugin, UndoPlugin, ClipboardPlugin, ImageStyle, ImageToolbar ], - toolbar: [ 'headings', 'undo', 'redo' ], - image: { - toolbar: [ 'imageStyleFull', 'imageStyleSide' ] - } + plugins: [ ImageToolbar, EnterPlugin, TypingPlugin, ParagraphPlugin, HeadingPlugin, ImagePlugin, UndoPlugin, ClipboardPlugin, ImageStyle ], + toolbar: [ 'headings', 'undo', 'redo' ] } ) .then( editor => { window.editor = editor; From 12fbc449e152973a47c4f0d042414e2b2a1cd74c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szymon=20Kup=C5=9B?= Date: Thu, 29 Dec 2016 09:58:45 +0100 Subject: [PATCH 19/35] Added tests to ImageToolbar. --- src/imagestyle/imagestyle.js | 1 - src/imagetoolbar.js | 6 +- tests/imagestyle/imagestyle.js | 1 + tests/imagetoolbar.js | 167 +++++++++++++++++++++++++++++++++ 4 files changed, 170 insertions(+), 5 deletions(-) create mode 100644 tests/imagetoolbar.js diff --git a/src/imagestyle/imagestyle.js b/src/imagestyle/imagestyle.js index 831594af..53643f7f 100644 --- a/src/imagestyle/imagestyle.js +++ b/src/imagestyle/imagestyle.js @@ -55,7 +55,6 @@ export default class ImageStyle extends Plugin { const command = editor.commands.get( 'imagestyle' ); const t = editor.t; - // Add bold button to feature components. editor.ui.componentFactory.add( style.name, ( locale ) => { const view = new ButtonView( locale ); diff --git a/src/imagetoolbar.js b/src/imagetoolbar.js index b882edb3..b7d659b1 100644 --- a/src/imagetoolbar.js +++ b/src/imagetoolbar.js @@ -82,10 +82,8 @@ export default class ImageToolbar extends Plugin { const editingView = editor.editing.view; const promises = []; - if ( toolbarConfig ) { - for ( let name of toolbarConfig ) { - promises.push( toolbar.items.add( editor.ui.componentFactory.create( name ) ) ); - } + for ( let name of toolbarConfig ) { + promises.push( toolbar.items.add( editor.ui.componentFactory.create( name ) ) ); } // Let the focusTracker know about new focusable UI element. diff --git a/tests/imagestyle/imagestyle.js b/tests/imagestyle/imagestyle.js index 970c84e0..54dc2809 100644 --- a/tests/imagestyle/imagestyle.js +++ b/tests/imagestyle/imagestyle.js @@ -87,6 +87,7 @@ describe( 'ImageStyle', () => { } ) .then( newEditor => { expect( newEditor.config.get( 'image.toolbar' ) ).to.eql( [ 'foo', 'bar' ] ); + newEditor.destroy(); } ); } ); } ); diff --git a/tests/imagetoolbar.js b/tests/imagetoolbar.js new file mode 100644 index 00000000..1ec79f33 --- /dev/null +++ b/tests/imagetoolbar.js @@ -0,0 +1,167 @@ +/** + * @license Copyright (c) 2003-2016, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md. + */ + +/* global Event */ + +import ClassicEditor from 'ckeditor5/editor-classic/classic.js'; +import ImageToolbar from 'ckeditor5/image/imagetoolbar.js'; +import Image from 'ckeditor5/image/image.js'; +import global from 'ckeditor5/utils/dom/global.js'; +import BalloonPanelView from 'ckeditor5/ui/balloonpanel/balloonpanelview.js'; +import Plugin from 'ckeditor5/core/plugin.js'; +import ButtonView from 'ckeditor5/ui/button/buttonview.js'; +import { setData } from 'ckeditor5/engine/dev-utils/model.js'; + +describe( 'ImageToolbar', () => { + let editor, button, editingView, doc, panel; + + beforeEach( () => { + const editorElement = global.document.createElement( 'div' ); + global.document.body.appendChild( editorElement ); + + return ClassicEditor.create( editorElement, { + plugins: [ Image, ImageToolbar, FakeButton ], + image: { + toolbar: [ 'fake_button' ] + } + } ) + .then( newEditor => { + editor = newEditor; + editingView = editor.editing.view; + doc = editor.document; + panel = getBalloonPanelView( editor.ui.view.body ); + } ); + } ); + + afterEach( () => { + return editor.destroy(); + } ); + + it( 'should be loaded', () => { + expect( editor.plugins.get( ImageToolbar ) ).to.be.instanceOf( ImageToolbar ); + } ); + + it( 'should not initialize if there is no configuration', () => { + const editorElement = global.document.createElement( 'div' ); + global.document.body.appendChild( editorElement ); + + return ClassicEditor.create( editorElement, { + plugins: [ ImageToolbar ], + } ) + .then( newEditor => { + const viewBody = newEditor.ui.view.body; + expect( getBalloonPanelView( viewBody ) ).to.be.undefined; + + newEditor.destroy(); + } ); + } ); + + it( 'should add BalloonPanelView to view body', () => { + expect( panel ).to.be.instanceOf( BalloonPanelView ); + } ); + + it( 'should attach toolbar when image is selected', () => { + const spy = sinon.spy( panel, 'attachTo' ); + setData( doc, '[]' ); + + testPanelAttach( spy ); + } ); + + it( 'should calculate panel position on scroll event', () => { + setData( doc, '[]' ); + const spy = sinon.spy( panel, 'attachTo' ); + + global.window.dispatchEvent( new Event( 'scroll' ) ); + + testPanelAttach( spy ); + } ); + + it( 'should calculate panel position on resize event', () => { + setData( doc, '[]' ); + const spy = sinon.spy( panel, 'attachTo' ); + + global.window.dispatchEvent( new Event( 'resize' ) ); + + testPanelAttach( spy ); + } ); + + it( 'should not calculate panel position on scroll if no image is selected', () => { + setData( doc, '' ); + const spy = sinon.spy( panel, 'attachTo' ); + + global.window.dispatchEvent( new Event( 'scroll' ) ); + + sinon.assert.notCalled( spy ); + } ); + + it( 'should not calculate panel position on resize if no image is selected', () => { + setData( doc, '' ); + const spy = sinon.spy( panel, 'attachTo' ); + + global.window.dispatchEvent( new Event( 'resize' ) ); + + sinon.assert.notCalled( spy ); + } ); + + it( 'should hide the panel when editor looses focus', () => { + setData( doc, '[]' ); + editor.ui.focusTracker.isFocused = true; + const spy = sinon.spy( panel, 'hide' ); + editor.ui.focusTracker.isFocused = false; + + sinon.assert.calledOnce( spy ); + } ); + + // Returns BalloonPanelView from provided collection. + function getBalloonPanelView( viewCollection ) { + return viewCollection.find( item => item instanceof BalloonPanelView ); + } + + // Tests if panel.attachTo() was called correctly. + function testPanelAttach( spy ) { + const domRange = editor.editing.view.domConverter.viewRangeToDom( editingView.selection.getFirstRange() ); + + sinon.assert.calledOnce( spy ); + const options = spy.firstCall.args[ 0 ]; + + // Check if proper range was used. + expect( options.target.startContainer ).to.equal( domRange.startContainer ); + expect( options.target.startOffset ).to.equal( domRange.startOffset ); + expect( options.target.endContainer ).to.equal( domRange.endContainer ); + expect( options.target.endOffset ).to.equal( domRange.endOffset ); + + // Check if north/south calculation is correct. + const [ north, south ] = options.positions; + const targetRect = { top: 10, left: 20, width: 200, height: 100, bottom: 110, right: 220 }; + const balloonRect = { width: 50, height: 20 }; + + const northPosition = north( targetRect, balloonRect ); + expect( northPosition.name ).to.equal( 'n' ); + expect( northPosition.top ).to.equal( targetRect.top - balloonRect.height - BalloonPanelView.arrowVerticalOffset ); + expect( northPosition.left ).to.equal( targetRect.left + targetRect.width / 2 - balloonRect.width / 2 ); + + const southPosition = south( targetRect, balloonRect ); + expect( southPosition.name ).to.equal( 's' ); + expect( southPosition.top ).to.equal( targetRect.bottom + BalloonPanelView.arrowVerticalOffset ); + expect( southPosition.left ).to.equal( targetRect.left + targetRect.width / 2 - balloonRect.width / 2 ); + } + + // Plugin that adds fake_button to editor's component factory. + class FakeButton extends Plugin { + init() { + this.editor.ui.componentFactory.add( 'fake_button', ( locale ) => { + const view = new ButtonView( locale ); + + view.set( { + label: 'fake button' + } ); + + button = view; + + return view; + } ); + } + } +} ); From 3f09d99a1c2cf627a94dc3e0cb971c5d0e979049 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szymon=20Kup=C5=9B?= Date: Thu, 29 Dec 2016 10:01:26 +0100 Subject: [PATCH 20/35] Use utils globals in tests. --- tests/imagestyle/imagestyle.js | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/tests/imagestyle/imagestyle.js b/tests/imagestyle/imagestyle.js index 54dc2809..b03c6b33 100644 --- a/tests/imagestyle/imagestyle.js +++ b/tests/imagestyle/imagestyle.js @@ -3,12 +3,11 @@ * For licensing, see LICENSE.md. */ -/* global document */ - import ClassicTestEditor from 'tests/core/_utils/classictesteditor.js'; import ImageStyle from 'ckeditor5/image/imagestyle/imagestyle.js'; import ImageStyleEngine from 'ckeditor5/image/imagestyle/imagestyleengine.js'; import ButtonView from 'ckeditor5/ui/button/buttonview.js'; +import global from 'ckeditor5/utils/dom/global.js'; describe( 'ImageStyle', () => { let editor; @@ -19,8 +18,8 @@ describe( 'ImageStyle', () => { ]; beforeEach( () => { - const editorElement = document.createElement( 'div' ); - document.body.appendChild( editorElement ); + const editorElement = global.document.createElement( 'div' ); + global.document.body.appendChild( editorElement ); return ClassicTestEditor.create( editorElement, { plugins: [ ImageStyle ], @@ -75,8 +74,8 @@ describe( 'ImageStyle', () => { } ); it( 'should not add buttons to image toolbar if configuration is present', () => { - const editorElement = document.createElement( 'div' ); - document.body.appendChild( editorElement ); + const editorElement = global.document.createElement( 'div' ); + global.document.body.appendChild( editorElement ); return ClassicTestEditor.create( editorElement, { plugins: [ ImageStyle ], From cd305e34b95acc5a0ea10ce878271301e790a006 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szymon=20Kup=C5=9B?= Date: Thu, 29 Dec 2016 10:04:56 +0100 Subject: [PATCH 21/35] Docs typo. --- src/imagestyle/imagestyle.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/imagestyle/imagestyle.js b/src/imagestyle/imagestyle.js index 53643f7f..bfd20fc6 100644 --- a/src/imagestyle/imagestyle.js +++ b/src/imagestyle/imagestyle.js @@ -36,7 +36,7 @@ export default class ImageStyle extends Plugin { this._createButton( style ); } - // If there is no default image toolbar configuration, add all style buttons. + // If there is no default image toolbar configuration, add all image styles buttons. const imageToolbarConfig = this.editor.config.get( 'image.toolbar' ); if ( !imageToolbarConfig ) { From 7a2b3a03e95fbd91c12175bfba866d68ddfed789 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szymon=20Kup=C5=9B?= Date: Fri, 30 Dec 2016 11:03:30 +0100 Subject: [PATCH 22/35] Removed unused ClickObserver from ImageToolbar. --- src/imagetoolbar.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/imagetoolbar.js b/src/imagetoolbar.js index b7d659b1..8461cc90 100644 --- a/src/imagetoolbar.js +++ b/src/imagetoolbar.js @@ -11,7 +11,6 @@ import Plugin from '../core/plugin.js'; import ToolbarView from '../ui/toolbar/toolbarview.js'; import BalloonPanelView from '../ui/balloonpanel/balloonpanelview.js'; import Template from '../ui/template.js'; -import ClickObserver from 'ckeditor5/engine/view/observer/clickobserver.js'; import { isImageWidget } from './utils.js'; import throttle from '../utils/lib/lodash/throttle.js'; import global from '../utils/dom/global.js'; @@ -96,8 +95,6 @@ export default class ImageToolbar extends Plugin { } } ); - editingView.addObserver( ClickObserver ); - // Check if the toolbar should be displayed each time view is rendered. editor.listenTo( editingView, 'render', () => { const selectedElement = editingView.selection.getSelectedElement(); From c6948e5cf80bf7f808f2e889b53aac02d01334e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szymon=20Kup=C5=9B?= Date: Fri, 30 Dec 2016 11:54:05 +0100 Subject: [PATCH 23/35] Attaching same scroll and resize callback in ImageToolbar. --- src/imagetoolbar.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/imagetoolbar.js b/src/imagetoolbar.js index 8461cc90..dd8d2eca 100644 --- a/src/imagetoolbar.js +++ b/src/imagetoolbar.js @@ -95,10 +95,11 @@ export default class ImageToolbar extends Plugin { } } ); + const attachToolbarCallback = throttle( attachToolbar, 100 ); + // Check if the toolbar should be displayed each time view is rendered. editor.listenTo( editingView, 'render', () => { const selectedElement = editingView.selection.getSelectedElement(); - const attachToolbarCallback = throttle( attachToolbar, 100 ); if ( selectedElement && isImageWidget( selectedElement ) ) { attachToolbar(); From 8493080ef2d95be6180705cb7cf9d53a951682a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szymon=20Kup=C5=9B?= Date: Mon, 2 Jan 2017 09:57:11 +0100 Subject: [PATCH 24/35] Updated paths. --- src/imagestyle/converters.js | 2 +- src/imagestyle/imagestyle.js | 6 +++--- src/imagestyle/imagestylecommand.js | 4 ++-- src/imagestyle/imagestyleengine.js | 16 +++++++++------- src/imagestyle/utils.js | 2 +- src/imagetoolbar.js | 14 +++++++------- tests/imagestyle/imagestyle.js | 10 +++++----- tests/imagestyle/imagestylecommand.js | 6 +++--- tests/imagestyle/imagestyleengine.js | 12 ++++++------ tests/imagestyle/utils.js | 4 ++-- tests/imagetoolbar.js | 16 ++++++++-------- tests/manual/image.js | 5 ++--- tests/manual/imagestyle.html | 4 ---- tests/manual/imagestyle.js | 20 ++++++++++---------- theme/icons/object-center.svg | 1 - theme/icons/object-left.svg | 1 - theme/icons/object-right.svg | 1 - 17 files changed, 59 insertions(+), 65 deletions(-) delete mode 100644 theme/icons/object-center.svg delete mode 100644 theme/icons/object-left.svg delete mode 100644 theme/icons/object-right.svg diff --git a/src/imagestyle/converters.js b/src/imagestyle/converters.js index a439a3b4..f09b9515 100644 --- a/src/imagestyle/converters.js +++ b/src/imagestyle/converters.js @@ -7,7 +7,7 @@ * @module image/imagestyle/converters */ -import { isImage, getStyleByValue } from './utils.js'; +import { isImage, getStyleByValue } from './utils'; /** * Returns converter for `imageStyle` attribute. It can be used for adding, changing and removing the attribute. diff --git a/src/imagestyle/imagestyle.js b/src/imagestyle/imagestyle.js index bfd20fc6..d1790c6c 100644 --- a/src/imagestyle/imagestyle.js +++ b/src/imagestyle/imagestyle.js @@ -7,9 +7,9 @@ * @module image/imagestyle/imagestyle */ -import Plugin from '../../core/plugin.js'; -import ImageStyleEngine from './imagestyleengine.js'; -import ButtonView from '../../ui/button/buttonview.js'; +import Plugin from 'ckeditor5-core/src/plugin'; +import ImageStyleEngine from './imagestyleengine'; +import ButtonView from 'ckeditor5-ui/src/button/buttonview'; /** * The image style plugin. diff --git a/src/imagestyle/imagestylecommand.js b/src/imagestyle/imagestylecommand.js index f65ce146..69668fc1 100644 --- a/src/imagestyle/imagestylecommand.js +++ b/src/imagestyle/imagestylecommand.js @@ -7,8 +7,8 @@ * @module image/imagestyle/imagestylecommand */ -import Command from '../../core/command/command.js'; -import { isImage, getStyleByValue } from './utils.js'; +import Command from 'ckeditor5-core/src/command/command'; +import { isImage, getStyleByValue } from './utils'; /** * The image style command. It is used to apply different image styles. diff --git a/src/imagestyle/imagestyleengine.js b/src/imagestyle/imagestyleengine.js index 4c7c4a82..3d8e9929 100644 --- a/src/imagestyle/imagestyleengine.js +++ b/src/imagestyle/imagestyleengine.js @@ -7,10 +7,12 @@ * @module image/imagestyle/imagestyleengine */ -import Plugin from '../../core/plugin.js'; -import ImageStyleCommand from './imagestylecommand.js'; -import ImageEngine from '../imageengine.js'; -import { viewToModelImageStyle, modelToViewSetStyle } from './converters.js'; +import Plugin from 'ckeditor5-core/src/plugin'; +import ImageStyleCommand from './imagestylecommand'; +import ImageEngine from '../imageengine'; +import { viewToModelImageStyle, modelToViewSetStyle } from './converters'; +import fullSizeIcon from 'ckeditor5-core/theme/icons/align-center.svg'; +import sideIcon from 'ckeditor5-core/theme/icons/align-right.svg'; /** * The image style engine plugin. Sets default configuration, creates converters and registers @@ -39,10 +41,10 @@ export default class ImageStyleEngine extends Plugin { // Define default configuration. editor.config.define( 'image.styles', [ // This option is equal to situation when no style is applied. - { name: 'imageStyleFull', title: 'Full size image', icon: 'object-center', value: null }, + { name: 'imageStyleFull', title: 'Full size image', icon: fullSizeIcon, value: null }, // This represents side image. - { name: 'imageStyleSide', title: 'Side image', icon: 'object-right', value: 'side', className: 'image-style-side' } + { name: 'imageStyleSide', title: 'Side image', icon: sideIcon, value: 'side', className: 'image-style-side' } ] ); // Get configuration. @@ -82,7 +84,7 @@ export default class ImageStyleEngine extends Plugin { * {@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 Icon name to use when creating style's toolbar button. + * @property {String} icon SVG icon representation to use when creating style's button. * @property {String} title Style's title. * @property {String} className CSS class used to represent style in view. */ diff --git a/src/imagestyle/utils.js b/src/imagestyle/utils.js index f504546e..4b70e27f 100644 --- a/src/imagestyle/utils.js +++ b/src/imagestyle/utils.js @@ -7,7 +7,7 @@ * @module image/imagestyle/utils */ -import ModelElement from '../../engine/model/element.js'; +import ModelElement from 'ckeditor5-engine/src/model/element'; /** * Checks if provided modelElement is an instance of {@link module:engine/model/element~Element Element} and its name diff --git a/src/imagetoolbar.js b/src/imagetoolbar.js index dd8d2eca..41d950c5 100644 --- a/src/imagetoolbar.js +++ b/src/imagetoolbar.js @@ -7,13 +7,13 @@ * @module image/imagetoolbar */ -import Plugin from '../core/plugin.js'; -import ToolbarView from '../ui/toolbar/toolbarview.js'; -import BalloonPanelView from '../ui/balloonpanel/balloonpanelview.js'; -import Template from '../ui/template.js'; -import { isImageWidget } from './utils.js'; -import throttle from '../utils/lib/lodash/throttle.js'; -import global from '../utils/dom/global.js'; +import Plugin from 'ckeditor5-core/src/plugin'; +import ToolbarView from 'ckeditor5-ui/src/toolbar/toolbarview'; +import BalloonPanelView from 'ckeditor5-ui/src/balloonpanel/balloonpanelview'; +import Template from 'ckeditor5-ui/src/template'; +import { isImageWidget } from './utils'; +import throttle from 'ckeditor5-utils/src/lib/lodash/throttle'; +import global from 'ckeditor5-utils/src/dom/global'; const arrowVOffset = BalloonPanelView.arrowVerticalOffset; const positions = { diff --git a/tests/imagestyle/imagestyle.js b/tests/imagestyle/imagestyle.js index b03c6b33..89b12eea 100644 --- a/tests/imagestyle/imagestyle.js +++ b/tests/imagestyle/imagestyle.js @@ -3,11 +3,11 @@ * For licensing, see LICENSE.md. */ -import ClassicTestEditor from 'tests/core/_utils/classictesteditor.js'; -import ImageStyle from 'ckeditor5/image/imagestyle/imagestyle.js'; -import ImageStyleEngine from 'ckeditor5/image/imagestyle/imagestyleengine.js'; -import ButtonView from 'ckeditor5/ui/button/buttonview.js'; -import global from 'ckeditor5/utils/dom/global.js'; +import ClassicTestEditor from 'ckeditor5-core/tests/_utils/classictesteditor'; +import ImageStyle from 'ckeditor5-image/src/imagestyle/imagestyle'; +import ImageStyleEngine from 'ckeditor5-image/src/imagestyle/imagestyleengine'; +import ButtonView from 'ckeditor5-ui/src/button/buttonview'; +import global from 'ckeditor5-utils/src/dom/global'; describe( 'ImageStyle', () => { let editor; diff --git a/tests/imagestyle/imagestylecommand.js b/tests/imagestyle/imagestylecommand.js index b04ead68..16279bcc 100644 --- a/tests/imagestyle/imagestylecommand.js +++ b/tests/imagestyle/imagestylecommand.js @@ -3,9 +3,9 @@ * For licensing, see LICENSE.md. */ -import ModelTestEditor from 'tests/core/_utils/modeltesteditor.js'; -import ImageStyleCommand from 'ckeditor5/image/imagestyle/imagestylecommand.js'; -import { setData, getData } from 'ckeditor5/engine/dev-utils/model.js'; +import ModelTestEditor from 'ckeditor5-core/tests/_utils/modeltesteditor'; +import ImageStyleCommand from 'ckeditor5-image/src/imagestyle/imagestylecommand'; +import { setData, getData } from 'ckeditor5-engine/src/dev-utils/model'; describe( 'ImageStyleCommand', () => { const styles = [ diff --git a/tests/imagestyle/imagestyleengine.js b/tests/imagestyle/imagestyleengine.js index a975ff47..2fcea360 100644 --- a/tests/imagestyle/imagestyleengine.js +++ b/tests/imagestyle/imagestyleengine.js @@ -3,12 +3,12 @@ * For licensing, see LICENSE.md. */ -import VirtualTestEditor from 'tests/core/_utils/virtualtesteditor.js'; -import ImageStyleEngine from 'ckeditor5/image/imagestyle/imagestyleengine.js'; -import ImageEngine from 'ckeditor5/image/imageengine.js'; -import ImageStyleCommand from 'ckeditor5/image/imagestyle/imagestylecommand.js'; -import { getData as getModelData, setData as setModelData } from 'ckeditor5/engine/dev-utils/model.js'; -import { getData as getViewData } from 'ckeditor5/engine/dev-utils/view.js'; +import VirtualTestEditor from 'ckeditor5-core/tests/_utils/virtualtesteditor'; +import ImageStyleEngine from 'ckeditor5-image/src/imagestyle/imagestyleengine'; +import ImageEngine from 'ckeditor5-image/src/imageengine'; +import ImageStyleCommand from 'ckeditor5-image/src/imagestyle/imagestylecommand'; +import { getData as getModelData, setData as setModelData } from 'ckeditor5-engine/src/dev-utils/model'; +import { getData as getViewData } from 'ckeditor5-engine/src/dev-utils/view'; describe( 'ImageStyleEngine', () => { let editor, document, viewDocument; diff --git a/tests/imagestyle/utils.js b/tests/imagestyle/utils.js index 6f577824..85ca06a5 100644 --- a/tests/imagestyle/utils.js +++ b/tests/imagestyle/utils.js @@ -3,8 +3,8 @@ * For licensing, see LICENSE.md. */ -import { isImage, getStyleByValue } from 'ckeditor5/image/imagestyle/utils.js'; -import ModelElement from 'ckeditor5/engine/model/element.js'; +import { isImage, getStyleByValue } from 'ckeditor5-image/src/imagestyle/utils'; +import ModelElement from 'ckeditor5-engine/src/model/element'; describe( 'ImageStyle utils', () => { describe( 'isImage', () => { diff --git a/tests/imagetoolbar.js b/tests/imagetoolbar.js index 1ec79f33..098025ad 100644 --- a/tests/imagetoolbar.js +++ b/tests/imagetoolbar.js @@ -5,14 +5,14 @@ /* global Event */ -import ClassicEditor from 'ckeditor5/editor-classic/classic.js'; -import ImageToolbar from 'ckeditor5/image/imagetoolbar.js'; -import Image from 'ckeditor5/image/image.js'; -import global from 'ckeditor5/utils/dom/global.js'; -import BalloonPanelView from 'ckeditor5/ui/balloonpanel/balloonpanelview.js'; -import Plugin from 'ckeditor5/core/plugin.js'; -import ButtonView from 'ckeditor5/ui/button/buttonview.js'; -import { setData } from 'ckeditor5/engine/dev-utils/model.js'; +import ClassicEditor from 'ckeditor5-editor-classic/src/classic'; +import ImageToolbar from 'ckeditor5-image/src/imagetoolbar'; +import Image from 'ckeditor5-image/src/image'; +import global from 'ckeditor5-utils/src/dom/global'; +import BalloonPanelView from 'ckeditor5-ui/src/balloonpanel/balloonpanelview'; +import Plugin from 'ckeditor5-core/src/plugin'; +import ButtonView from 'ckeditor5-ui/src/button/buttonview'; +import { setData } from 'ckeditor5-engine/src/dev-utils/model'; describe( 'ImageToolbar', () => { let editor, button, editingView, doc, panel; diff --git a/tests/manual/image.js b/tests/manual/image.js index 88af0253..c54b28c2 100644 --- a/tests/manual/image.js +++ b/tests/manual/image.js @@ -13,11 +13,10 @@ import HeadingPlugin from 'ckeditor5-heading/src/heading'; import ImagePlugin from 'ckeditor5-image/src/image'; import UndoPlugin from 'ckeditor5-undo/src/undo'; import ClipboardPlugin from 'ckeditor5-clipboard/src/clipboard'; -import ImageStyle from 'ckeditor5-image/src/imagestyle/imagestyle'; ClassicEditor.create( document.querySelector( '#editor' ), { - plugins: [ EnterPlugin, TypingPlugin, ParagraphPlugin, HeadingPlugin, ImagePlugin, UndoPlugin, ClipboardPlugin, ImageStyle ], - toolbar: [ 'headings', 'undo', 'redo', 'imagestyle' ] + plugins: [ EnterPlugin, TypingPlugin, ParagraphPlugin, HeadingPlugin, ImagePlugin, UndoPlugin, ClipboardPlugin ], + toolbar: [ 'headings', 'undo', 'redo' ] } ) .then( editor => { window.editor = editor; diff --git a/tests/manual/imagestyle.html b/tests/manual/imagestyle.html index 6eb02b68..32168677 100644 --- a/tests/manual/imagestyle.html +++ b/tests/manual/imagestyle.html @@ -1,7 +1,3 @@ - - - -

Lorem ipsum dolor sit amet, consectetur adipiscing elit. Nulla finibus consequat placerat. Vestibulum id tellus et mauris sagittis tincidunt quis id mauris. Curabitur consectetur lectus sit amet tellus mattis, non lobortis leo interdum.

diff --git a/tests/manual/imagestyle.js b/tests/manual/imagestyle.js index a2ee0136..5f95236b 100644 --- a/tests/manual/imagestyle.js +++ b/tests/manual/imagestyle.js @@ -5,16 +5,16 @@ /* global document, console, window */ -import ClassicEditor from 'ckeditor5/editor-classic/classic.js'; -import EnterPlugin from 'ckeditor5/enter/enter.js'; -import TypingPlugin from 'ckeditor5/typing/typing.js'; -import ParagraphPlugin from 'ckeditor5/paragraph/paragraph.js'; -import HeadingPlugin from 'ckeditor5/heading/heading.js'; -import ImagePlugin from 'ckeditor5/image/image.js'; -import UndoPlugin from 'ckeditor5/undo/undo.js'; -import ClipboardPlugin from 'ckeditor5/clipboard/clipboard.js'; -import ImageStyle from 'ckeditor5/image/imagestyle/imagestyle.js'; -import ImageToolbar from 'ckeditor5/image/imagetoolbar.js'; +import ClassicEditor from 'ckeditor5-editor-classic/src/classic'; +import EnterPlugin from 'ckeditor5-enter/src/enter'; +import TypingPlugin from 'ckeditor5-typing/src/typing'; +import ParagraphPlugin from 'ckeditor5-paragraph/src/paragraph'; +import HeadingPlugin from 'ckeditor5-heading/src/heading'; +import ImagePlugin from 'ckeditor5-image/src/image'; +import UndoPlugin from 'ckeditor5-undo/src/undo'; +import ClipboardPlugin from 'ckeditor5-clipboard/src/clipboard'; +import ImageStyle from 'ckeditor5-image/src/imagestyle/imagestyle'; +import ImageToolbar from 'ckeditor5-image/src/imagetoolbar'; ClassicEditor.create( document.querySelector( '#editor' ), { plugins: [ ImageToolbar, EnterPlugin, TypingPlugin, ParagraphPlugin, HeadingPlugin, ImagePlugin, UndoPlugin, ClipboardPlugin, ImageStyle ], diff --git a/theme/icons/object-center.svg b/theme/icons/object-center.svg deleted file mode 100644 index 20728135..00000000 --- a/theme/icons/object-center.svg +++ /dev/null @@ -1 +0,0 @@ - diff --git a/theme/icons/object-left.svg b/theme/icons/object-left.svg deleted file mode 100644 index fc10c9b8..00000000 --- a/theme/icons/object-left.svg +++ /dev/null @@ -1 +0,0 @@ - diff --git a/theme/icons/object-right.svg b/theme/icons/object-right.svg deleted file mode 100644 index ff700a92..00000000 --- a/theme/icons/object-right.svg +++ /dev/null @@ -1 +0,0 @@ - From 091a1289557b77466c9dfd3862f0f0f32b021f85 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szymon=20Kup=C5=9B?= Date: Tue, 3 Jan 2017 16:18:10 +0100 Subject: [PATCH 25/35] Added proper icons to image styles. --- src/imagestyle/imagestyleengine.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/imagestyle/imagestyleengine.js b/src/imagestyle/imagestyleengine.js index 3d8e9929..cb27405e 100644 --- a/src/imagestyle/imagestyleengine.js +++ b/src/imagestyle/imagestyleengine.js @@ -11,8 +11,8 @@ import Plugin from 'ckeditor5-core/src/plugin'; import ImageStyleCommand from './imagestylecommand'; import ImageEngine from '../imageengine'; import { viewToModelImageStyle, modelToViewSetStyle } from './converters'; -import fullSizeIcon from 'ckeditor5-core/theme/icons/align-center.svg'; -import sideIcon from 'ckeditor5-core/theme/icons/align-right.svg'; +import fullSizeIcon from 'ckeditor5-core/theme/icons/object-center.svg'; +import sideIcon from 'ckeditor5-core/theme/icons/object-right.svg'; /** * The image style engine plugin. Sets default configuration, creates converters and registers From 1f511d52a8f175d2063e34e864fd8aff52da43bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szymon=20Kup=C5=9B?= Date: Wed, 4 Jan 2017 11:59:09 +0100 Subject: [PATCH 26/35] Minor fixes. --- src/imagestyle/imagestyle.js | 6 +++--- src/imagestyle/imagestylecommand.js | 1 + src/imagestyle/imagestyleengine.js | 15 +++++++++++++-- src/imagetoolbar.js | 2 +- 4 files changed, 18 insertions(+), 6 deletions(-) diff --git a/src/imagestyle/imagestyle.js b/src/imagestyle/imagestyle.js index d1790c6c..ee91f1ce 100644 --- a/src/imagestyle/imagestyle.js +++ b/src/imagestyle/imagestyle.js @@ -53,14 +53,14 @@ export default class ImageStyle extends Plugin { _createButton( style ) { const editor = this.editor; const command = editor.commands.get( 'imagestyle' ); - const t = editor.t; editor.ui.componentFactory.add( style.name, ( locale ) => { const view = new ButtonView( locale ); view.set( { - label: t( style.title ), - icon: style.icon + label: style.title, + icon: style.icon, + tooltip: true } ); view.bind( 'isEnabled' ).to( command, 'isEnabled' ); diff --git a/src/imagestyle/imagestylecommand.js b/src/imagestyle/imagestylecommand.js index 69668fc1..bf0a7ae8 100644 --- a/src/imagestyle/imagestylecommand.js +++ b/src/imagestyle/imagestylecommand.js @@ -24,6 +24,7 @@ export default class ImageStyleCommand extends Command { */ constructor( editor, styles ) { super( editor ); + /** * The current style value. * diff --git a/src/imagestyle/imagestyleengine.js b/src/imagestyle/imagestyleengine.js index cb27405e..be7b98d5 100644 --- a/src/imagestyle/imagestyleengine.js +++ b/src/imagestyle/imagestyleengine.js @@ -33,6 +33,7 @@ export default class ImageStyleEngine extends Plugin { */ init() { const editor = this.editor; + const t = editor.t; const doc = editor.document; const schema = doc.schema; const data = editor.data; @@ -41,10 +42,10 @@ export default class ImageStyleEngine extends Plugin { // Define default configuration. editor.config.define( 'image.styles', [ // This option is equal to situation when no style is applied. - { name: 'imageStyleFull', title: 'Full size image', icon: fullSizeIcon, value: null }, + { name: 'imageStyleFull', title: t( 'Full size image' ), icon: fullSizeIcon, value: null }, // This represents side image. - { name: 'imageStyleSide', title: 'Side image', icon: sideIcon, value: 'side', className: 'image-style-side' } + { name: 'imageStyleSide', title: t( 'Side image' ), icon: sideIcon, value: 'side', className: 'image-style-side' } ] ); // Get configuration. @@ -79,6 +80,16 @@ export default class ImageStyleEngine extends Plugin { /** * Image style format descriptor. * + * import fullIcon from 'path/to/icon.svg`; + * + * const imageStyleFormat = { + * name: 'fullSizeImage', + * value: 'full', + * icon: fullIcon, + * title: `Full size image`, + * class: `image-full-size` + * } + * * @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}. diff --git a/src/imagetoolbar.js b/src/imagetoolbar.js index 41d950c5..b2e10c78 100644 --- a/src/imagetoolbar.js +++ b/src/imagetoolbar.js @@ -69,7 +69,7 @@ export default class ImageToolbar extends Plugin { Template.extend( panel.template, { attributes: { class: [ - 'ck-toolbar__container', + 'ck-toolbar__container' ] } } ); From d953a7bf795869a86056b951b727b344805ec05d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szymon=20Kup=C5=9B?= Date: Wed, 4 Jan 2017 13:22:17 +0100 Subject: [PATCH 27/35] Changed ImageToolbar default configuration processing. --- src/imagestyle/imagestyle.js | 8 ++++---- src/imagetoolbar.js | 15 +++++++++++++-- tests/imagestyle/imagestyle.js | 19 ++++++++++++++++--- tests/imagetoolbar.js | 33 +++++++++++++++++++++++++++++++++ 4 files changed, 66 insertions(+), 9 deletions(-) diff --git a/src/imagestyle/imagestyle.js b/src/imagestyle/imagestyle.js index ee91f1ce..853aca04 100644 --- a/src/imagestyle/imagestyle.js +++ b/src/imagestyle/imagestyle.js @@ -36,11 +36,11 @@ export default class ImageStyle extends Plugin { this._createButton( style ); } - // If there is no default image toolbar configuration, add all image styles buttons. - const imageToolbarConfig = this.editor.config.get( 'image.toolbar' ); + // Push buttons to default image toolbar if one exists. + const defaultImageToolbarConfig = this.editor.config.get( 'image.defaultToolbar' ); - if ( !imageToolbarConfig ) { - this.editor.config.set( 'image.toolbar', styles.map( style => style.name ) ); + if ( defaultImageToolbarConfig ) { + styles.forEach( style => defaultImageToolbarConfig.push( style.name ) ); } } diff --git a/src/imagetoolbar.js b/src/imagetoolbar.js index b2e10c78..cbd740f3 100644 --- a/src/imagetoolbar.js +++ b/src/imagetoolbar.js @@ -44,19 +44,30 @@ const positions = { * Image toolbar class. Creates image toolbar placed inside balloon panel that is showed when image widget is selected. * Toolbar components are created using editor's {@link module:ui/componentfactory~ComponentFactory ComponentFactory} * based on {@link module:core/editor/editor~Editor#config configuration} stored under `image.toolbar`. + * Other plugins can add new components to the default toolbar configuration by pushing them to `image.defaultToolbar` + * configuration. Default configuration is used when `image.toolbar` config is not present. * * @extends module:core/plugin~Plugin. */ export default class ImageToolbar extends Plugin { + /** + * @inheritDoc + */ + constructor( editor ) { + super( editor ); + + editor.config.set( 'image.defaultToolbar', [] ); + } + /** * @inheritDoc */ afterInit() { const editor = this.editor; - const toolbarConfig = editor.config.get( 'image.toolbar' ); + const toolbarConfig = editor.config.get( 'image.toolbar' ) || editor.config.get( 'image.defaultToolbar' ); // Don't add the toolbar if there is no configuration. - if ( !toolbarConfig ) { + if ( !toolbarConfig.length ) { return; } diff --git a/tests/imagestyle/imagestyle.js b/tests/imagestyle/imagestyle.js index 89b12eea..dc1ec33c 100644 --- a/tests/imagestyle/imagestyle.js +++ b/tests/imagestyle/imagestyle.js @@ -4,6 +4,7 @@ */ import ClassicTestEditor from 'ckeditor5-core/tests/_utils/classictesteditor'; +import ImageToolbar from 'ckeditor5-image/src/imagetoolbar'; import ImageStyle from 'ckeditor5-image/src/imagestyle/imagestyle'; import ImageStyleEngine from 'ckeditor5-image/src/imagestyle/imagestyleengine'; import ButtonView from 'ckeditor5-ui/src/button/buttonview'; @@ -67,10 +68,22 @@ describe( 'ImageStyle', () => { } } ); - it( 'should add buttons to image toolbar if there is no default configuration', () => { - const toolbarConfig = editor.config.get( 'image.toolbar' ); + it( 'should not add buttons to default image toolbar if image toolbar is not present', () => { + expect( editor.config.get( 'image.defaultToolbar' ) ).to.be.undefined; + } ); + + it( 'should add buttons to default image toolbar if toolbar is present', () => { + const editorElement = global.document.createElement( 'div' ); + global.document.body.appendChild( editorElement ); + + return ClassicTestEditor.create( editorElement, { + plugins: [ ImageStyle, ImageToolbar ] + } ) + .then( newEditor => { + expect( newEditor.config.get( 'image.defaultToolbar' ) ).to.eql( [ 'imageStyleFull', 'imageStyleSide' ] ); - expect( toolbarConfig ).to.eql( styles.map( style => style.name ) ); + newEditor.destroy(); + } ); } ); it( 'should not add buttons to image toolbar if configuration is present', () => { diff --git a/tests/imagetoolbar.js b/tests/imagetoolbar.js index 098025ad..6ec156ff 100644 --- a/tests/imagetoolbar.js +++ b/tests/imagetoolbar.js @@ -43,6 +43,10 @@ describe( 'ImageToolbar', () => { expect( editor.plugins.get( ImageToolbar ) ).to.be.instanceOf( ImageToolbar ); } ); + it( 'should initialize image.defaultToolbar to an empty array', () => { + expect( editor.config.get( 'image.defaultToolbar' ) ).to.eql( [] ); + } ); + it( 'should not initialize if there is no configuration', () => { const editorElement = global.document.createElement( 'div' ); global.document.body.appendChild( editorElement ); @@ -58,6 +62,25 @@ describe( 'ImageToolbar', () => { } ); } ); + it( 'should allow other plugins to alter default config', () => { + const editorElement = global.document.createElement( 'div' ); + global.document.body.appendChild( editorElement ); + + return ClassicEditor.create( editorElement, { + plugins: [ ImageToolbar, FakeButton, AlterDefaultConfig ] + } ) + .then( newEditor => { + const panel = getBalloonPanelView( newEditor.ui.view.body ); + const toolbar = panel.content.get( 0 ); + const button = toolbar.items.get( 0 ); + + expect( newEditor.config.get( 'image.defaultToolbar' ) ).to.eql( [ 'fake_button' ] ); + expect( button.label ).to.equal( 'fake button' ); + + newEditor.destroy(); + } ); + } ); + it( 'should add BalloonPanelView to view body', () => { expect( panel ).to.be.instanceOf( BalloonPanelView ); } ); @@ -164,4 +187,14 @@ describe( 'ImageToolbar', () => { } ); } } + + class AlterDefaultConfig extends Plugin { + init() { + const defaultImageToolbarConfig = this.editor.config.get( 'image.defaultToolbar' ); + + if ( defaultImageToolbarConfig ) { + defaultImageToolbarConfig.push( 'fake_button' ); + } + } + } } ); From 40f1907951e24b08a49a03212c965c1076d9b71b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szymon=20Kup=C5=9B?= Date: Wed, 4 Jan 2017 17:16:23 +0100 Subject: [PATCH 28/35] Creating separate command for each style. --- src/imagestyle/imagestyle.js | 8 ++-- src/imagestyle/imagestylecommand.js | 54 +++++++++++--------------- src/imagestyle/imagestyleengine.js | 13 ++++--- tests/imagestyle/imagestyle.js | 4 +- tests/imagestyle/imagestylecommand.js | 56 +++++++++++++-------------- tests/imagestyle/imagestyleengine.js | 13 ++++--- 6 files changed, 69 insertions(+), 79 deletions(-) diff --git a/src/imagestyle/imagestyle.js b/src/imagestyle/imagestyle.js index 853aca04..19949758 100644 --- a/src/imagestyle/imagestyle.js +++ b/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; } ); diff --git a/src/imagestyle/imagestylecommand.js b/src/imagestyle/imagestylecommand.js index bf0a7ae8..44d46391 100644 --- a/src/imagestyle/imagestylecommand.js +++ b/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.} 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.} #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 ); } ); } } diff --git a/src/imagestyle/imagestyleengine.js b/src/imagestyle/imagestyleengine.js index be7b98d5..f08e97e8 100644 --- a/src/imagestyle/imagestyleengine.js +++ b/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. diff --git a/tests/imagestyle/imagestyle.js b/tests/imagestyle/imagestyle.js index dc1ec33c..fdab177c 100644 --- a/tests/imagestyle/imagestyle.js +++ b/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(); } diff --git a/tests/imagestyle/imagestylecommand.js b/tests/imagestyle/imagestylecommand.js index 16279bcc..77fbdc8f 100644 --- a/tests/imagestyle/imagestylecommand.js +++ b/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, '[]' ); - 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, '[]' ); - 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, '[]' ); - 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, '[]' ); - 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, '[]' ); - command._doExecute( { value: 'other' } ); + otherStyleCommand._doExecute(); expect( getData( document ) ).to.equal( '[]' ); } ); - it( 'should do nothing when executed with wrong value', () => { - setData( document, '[]' ); - - command._doExecute( { value: 'foo' } ); - - expect( getData( document ) ).to.equal( '[]' ); - } ); - - it( 'should do nothing when executed with same value', () => { + it( 'should do nothing when attribute already present', () => { setData( document, '[]' ); - command._doExecute( { value: 'other' } ); + otherStyleCommand._doExecute(); expect( getData( document ) ).to.equal( '[]' ); } ); @@ -84,7 +79,7 @@ describe( 'ImageStyleCommand', () => { setData( document, '[]' ); - command._doExecute( { value: 'other', batch } ); + otherStyleCommand._doExecute( { batch } ); expect( getData( document ) ).to.equal( '[]' ); sinon.assert.calledOnce( spy ); @@ -93,18 +88,21 @@ describe( 'ImageStyleCommand', () => { it( 'should be enabled on image element', () => { setData( document, '[]' ); - 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, '[

]' ); - 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, '[

]' ); - expect( command.isEnabled ).to.be.false; + expect( defaultStyleCommand.isEnabled ).to.be.false; + expect( otherStyleCommand.isEnabled ).to.be.false; } ); } ); diff --git a/tests/imagestyle/imagestyleengine.js b/tests/imagestyle/imagestyleengine.js index 2fcea360..6c7e71ab 100644 --- a/tests/imagestyle/imagestyleengine.js +++ b/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', () => { From 7327f578a1d4964fdc5c535363c268115374f854 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szymon=20Kup=C5=9B?= Date: Wed, 4 Jan 2017 17:43:08 +0100 Subject: [PATCH 29/35] Refactoring util methods. --- src/imagestyle/converters.js | 14 ++++++++- src/imagestyle/imagestylecommand.js | 2 +- src/imagestyle/utils.js | 36 ---------------------- src/utils.js | 12 ++++++++ tests/imagestyle/utils.js | 48 ----------------------------- tests/utils.js | 22 ++++++++++++- 6 files changed, 47 insertions(+), 87 deletions(-) delete mode 100644 src/imagestyle/utils.js delete mode 100644 tests/imagestyle/utils.js diff --git a/src/imagestyle/converters.js b/src/imagestyle/converters.js index f09b9515..fface3ee 100644 --- a/src/imagestyle/converters.js +++ b/src/imagestyle/converters.js @@ -7,7 +7,7 @@ * @module image/imagestyle/converters */ -import { isImage, getStyleByValue } from './utils'; +import { isImage } from '../utils'; /** * Returns converter for `imageStyle` attribute. It can be used for adding, changing and removing the attribute. @@ -84,3 +84,15 @@ export function viewToModelImageStyle( style ) { modelImageElement.setAttribute( 'imageStyle', style.value ); }; } + +// Returns style with given `value` from array of styles. +// @param {String} value +// @param {Array. } styles +// @return {module:image/imagestyle/imagestyleengine~ImageStyleFormat|undefined} +function getStyleByValue( value, styles ) { + for ( let style of styles ) { + if ( style.value === value ) { + return style; + } + } +} diff --git a/src/imagestyle/imagestylecommand.js b/src/imagestyle/imagestylecommand.js index 44d46391..f382aa74 100644 --- a/src/imagestyle/imagestylecommand.js +++ b/src/imagestyle/imagestylecommand.js @@ -8,7 +8,7 @@ */ import Command from 'ckeditor5-core/src/command/command'; -import { isImage } from './utils'; +import { isImage } from '../utils'; /** * The image style command. It is used to apply different image styles. diff --git a/src/imagestyle/utils.js b/src/imagestyle/utils.js deleted file mode 100644 index 4b70e27f..00000000 --- a/src/imagestyle/utils.js +++ /dev/null @@ -1,36 +0,0 @@ -/** - * @license Copyright (c) 2003-2016, CKSource - Frederico Knabben. All rights reserved. - * For licensing, see LICENSE.md. - */ - -/** - * @module image/imagestyle/utils - */ - -import ModelElement from 'ckeditor5-engine/src/model/element'; - -/** - * Checks if provided modelElement is an instance of {@link module:engine/model/element~Element Element} and its name - * equals to `image`. - * - * @param {module:engine/model/element~Element} modelElement - * @returns {Boolean} - */ -export function isImage( modelElement ) { - return modelElement instanceof ModelElement && modelElement.name == 'image'; -} - -/** - * Returns style with given `value` from array of styles. - * - * @param {String} value - * @param {Array. } styles - * @return {module:image/imagestyle/imagestyleengine~ImageStyleFormat|undefined} - */ -export function getStyleByValue( value, styles ) { - for ( let style of styles ) { - if ( style.value === value ) { - return style; - } - } -} diff --git a/src/utils.js b/src/utils.js index 8965f261..6e5c8ed6 100644 --- a/src/utils.js +++ b/src/utils.js @@ -8,6 +8,7 @@ */ import { widgetize, isWidget } from './widget/utils'; +import ModelElement from 'ckeditor5-engine/src/model/element'; const imageSymbol = Symbol( 'isImage' ); @@ -34,3 +35,14 @@ export function toImageWidget( viewElement ) { export function isImageWidget( viewElement ) { return !!viewElement.getCustomProperty( imageSymbol ) && isWidget( viewElement ); } + +/** + * Checks if provided modelElement is an instance of {@link module:engine/model/element~Element Element} and its name + * is `image`. + * + * @param {module:engine/model/element~Element} modelElement + * @returns {Boolean} + */ +export function isImage( modelElement ) { + return modelElement instanceof ModelElement && modelElement.name == 'image'; +} diff --git a/tests/imagestyle/utils.js b/tests/imagestyle/utils.js deleted file mode 100644 index 85ca06a5..00000000 --- a/tests/imagestyle/utils.js +++ /dev/null @@ -1,48 +0,0 @@ -/** - * @license Copyright (c) 2003-2016, CKSource - Frederico Knabben. All rights reserved. - * For licensing, see LICENSE.md. - */ - -import { isImage, getStyleByValue } from 'ckeditor5-image/src/imagestyle/utils'; -import ModelElement from 'ckeditor5-engine/src/model/element'; - -describe( 'ImageStyle utils', () => { - describe( 'isImage', () => { - it( 'should return true for image element', () => { - const image = new ModelElement( 'image' ); - - expect( isImage( image ) ).to.be.true; - } ); - - it( 'should return true false for different elements', () => { - const image = new ModelElement( 'foo' ); - - expect( isImage( image ) ).to.be.false; - } ); - - it( 'should return true false for null and undefined', () => { - expect( isImage( null ) ).to.be.false; - expect( isImage( undefined ) ).to.be.false; - } ); - } ); - - describe( 'getStyleByValue', () => { - const styles = [ - { name: 'style 1', title: 'title 1', icon: 'style1-icon', value: 'style1', className: 'style1-class' }, - { name: 'style 2', title: 'title 2', icon: 'style2-icon', value: 'style2', className: 'style2-class' } - ]; - - it( 'should return proper styles for values', () => { - expect( getStyleByValue( 'style1', styles ) ).to.equal( styles[ 0 ] ); - expect( getStyleByValue( 'style2', styles ) ).to.equal( styles[ 1 ] ); - } ); - - it( 'should return undefined when style is not found', () => { - expect( getStyleByValue( 'foo', styles ) ).to.be.undefined; - } ); - - it( 'should return undefined when empty styles array is provided', () => { - expect( getStyleByValue( 'style1', [] ) ).to.be.undefined; - } ); - } ); -} ); diff --git a/tests/utils.js b/tests/utils.js index 5a40bcd2..686d98c9 100644 --- a/tests/utils.js +++ b/tests/utils.js @@ -4,7 +4,8 @@ */ import ViewElement from 'ckeditor5-engine/src/view/element'; -import { toImageWidget, isImageWidget } from 'ckeditor5-image/src/utils'; +import ModelElement from 'ckeditor5-engine/src/model/element'; +import { toImageWidget, isImageWidget, isImage } from 'ckeditor5-image/src/utils'; import { isWidget } from 'ckeditor5-image/src/widget/utils'; describe( 'image widget utils', () => { @@ -30,4 +31,23 @@ describe( 'image widget utils', () => { expect( isImageWidget( new ViewElement( 'p' ) ) ).to.be.false; } ); } ); + + describe( 'isImage', () => { + it( 'should return true for image element', () => { + const image = new ModelElement( 'image' ); + + expect( isImage( image ) ).to.be.true; + } ); + + it( 'should return true false for different elements', () => { + const image = new ModelElement( 'foo' ); + + expect( isImage( image ) ).to.be.false; + } ); + + it( 'should return true false for null and undefined', () => { + expect( isImage( null ) ).to.be.false; + expect( isImage( undefined ) ).to.be.false; + } ); + } ); } ); From 12d75d950f8ed68da44b2de7184e18c732754c68 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szymon=20Kup=C5=9B?= Date: Mon, 9 Jan 2017 17:46:29 +0100 Subject: [PATCH 30/35] ImageStyle - converting from view to model in one callback for all styles. --- src/imagestyle/converters.js | 63 +++++++++++++++++++----------- src/imagestyle/imagestyleengine.js | 13 +++--- 2 files changed, 45 insertions(+), 31 deletions(-) diff --git a/src/imagestyle/converters.js b/src/imagestyle/converters.js index fface3ee..84ffaaff 100644 --- a/src/imagestyle/converters.js +++ b/src/imagestyle/converters.js @@ -51,38 +51,55 @@ export function modelToViewSetStyle( styles ) { } /** - * Returns view to model converter converting image style CSS class to proper value in the model. + * Returns view to model converter converting image CSS classes to proper value in the model. * - * @param {module:image/imagestyle/imagestyleengine~ImageStyleFormat} style Style for which converter is created. + * @param {Array.} styles Styles for which converter is created. * @returns {Function} View to model converter. */ -export function viewToModelImageStyle( style ) { - return ( evt, data, consumable, conversionApi ) => { - const viewFigureElement = data.input; - const modelImageElement = data.output; +export function viewToModelImageStyles( styles ) { + // Convert only styles without `null` value. + const filteredStyles = styles.filter( style => style.value !== null ); - // *** Step 1: Validate conversion. - // Check if view element has proper class to consume. - if ( !consumable.test( viewFigureElement, { class: style.className } ) ) { - return; + return ( evt, data, consumable, conversionApi ) => { + for ( let style of filteredStyles ) { + viewToModelImageStyle( style, data, consumable, conversionApi ); } + }; +} - // Check if figure is converted to image. - if ( !isImage( modelImageElement ) ) { - return; - } +// Converter from view to model converting single style. +// For more information see {@link module:engine/conversion/viewconversiondispatcher~ViewConversionDispatcher}; +// +// @private +// @param {module:image/imagestyle/imagestyleengine~ImageStyleFormat} style +// @param {Object} data +// @param {module:engine/conversion/viewconsumable~ViewConsumable} consumable +// @param {Object} conversionApi +function viewToModelImageStyle( style, data, consumable, conversionApi ) { + const viewFigureElement = data.input; + const modelImageElement = data.output; + + // *** Step 1: Validate conversion. + // Check if view element has proper class to consume. + if ( !consumable.test( viewFigureElement, { class: style.className } ) ) { + return; + } - // Check if image element can be placed in current context wit additional attribute. - const attributes = [ ...modelImageElement.getAttributeKeys(), 'imageStyle' ]; + // Check if figure is converted to image. + if ( !isImage( modelImageElement ) ) { + return; + } - if ( !conversionApi.schema.check( { name: 'image', inside: data.context, attributes } ) ) { - return; - } + // Check if image element can be placed in current context wit additional attribute. + const attributes = [ ...modelImageElement.getAttributeKeys(), 'imageStyle' ]; - // *** Step2: Convert to model. - consumable.consume( viewFigureElement, { class: style.className } ); - modelImageElement.setAttribute( 'imageStyle', style.value ); - }; + if ( !conversionApi.schema.check( { name: 'image', inside: data.context, attributes } ) ) { + return; + } + + // *** Step2: Convert to model. + consumable.consume( viewFigureElement, { class: style.className } ); + modelImageElement.setAttribute( 'imageStyle', style.value ); } // Returns style with given `value` from array of styles. diff --git a/src/imagestyle/imagestyleengine.js b/src/imagestyle/imagestyleengine.js index f08e97e8..26c9e012 100644 --- a/src/imagestyle/imagestyleengine.js +++ b/src/imagestyle/imagestyleengine.js @@ -10,7 +10,7 @@ import Plugin from 'ckeditor5-core/src/plugin'; import ImageStyleCommand from './imagestylecommand'; import ImageEngine from '../imageengine'; -import { viewToModelImageStyle, modelToViewSetStyle } from './converters'; +import { viewToModelImageStyles, modelToViewSetStyle } from './converters'; import fullSizeIcon from 'ckeditor5-core/theme/icons/object-center.svg'; import sideIcon from 'ckeditor5-core/theme/icons/object-right.svg'; @@ -64,14 +64,11 @@ export default class ImageStyleEngine extends Plugin { editing.modelToView.on( 'removeAttribute:imageStyle:image', modelToViewConverter ); data.modelToView.on( 'removeAttribute:imageStyle:image', modelToViewConverter ); - 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' } ); - } + // Converter for figure element from view to model. + data.viewToModel.on( 'element:figure', viewToModelImageStyles( styles ), { priority: 'low' } ); - // Register separate command for each style. + // Register separate command for each style. + for ( let style of styles ) { editor.commands.set( style.name, new ImageStyleCommand( editor, style ) ); } } From c350b10a79157f8814ebdfde8251754300af810a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szymon=20Kup=C5=9B?= Date: Tue, 10 Jan 2017 00:48:56 +0100 Subject: [PATCH 31/35] Split model to view converter in image style. --- src/imagestyle/converters.js | 59 +++++++++++++++++++++++++----------- 1 file changed, 42 insertions(+), 17 deletions(-) diff --git a/src/imagestyle/converters.js b/src/imagestyle/converters.js index 84ffaaff..1e04d9c6 100644 --- a/src/imagestyle/converters.js +++ b/src/imagestyle/converters.js @@ -30,23 +30,10 @@ export function modelToViewSetStyle( styles ) { const oldStyle = getStyleByValue( data.attributeOldValue, styles ); const viewElement = conversionApi.mapper.toViewElement( data.item ); - if ( eventType == 'changeAttribute' || eventType == 'removeAttribute' ) { - if ( !oldStyle ) { - return; - } - - viewElement.removeClass( oldStyle.className ); - } - - if ( eventType == 'addAttribute' || eventType == 'changeAttribute' ) { - if ( !newStyle ) { - return; - } - - viewElement.addClass( newStyle.className ); + if ( handleRemoval( eventType, oldStyle, viewElement ) && + handleAddition( eventType, newStyle, viewElement ) ) { + consumable.consume( data.item, consumableType ); } - - consumable.consume( data.item, consumableType ); }; } @@ -70,7 +57,6 @@ export function viewToModelImageStyles( styles ) { // Converter from view to model converting single style. // For more information see {@link module:engine/conversion/viewconversiondispatcher~ViewConversionDispatcher}; // -// @private // @param {module:image/imagestyle/imagestyleengine~ImageStyleFormat} style // @param {Object} data // @param {module:engine/conversion/viewconsumable~ViewConsumable} consumable @@ -103,6 +89,7 @@ function viewToModelImageStyle( style, data, consumable, conversionApi ) { } // Returns style with given `value` from array of styles. +// // @param {String} value // @param {Array. } styles // @return {module:image/imagestyle/imagestyleengine~ImageStyleFormat|undefined} @@ -113,3 +100,41 @@ function getStyleByValue( value, styles ) { } } } + +// Handles converting removal of the attribute. +// Returns `true` when handling was processed correctly and further conversion can be performed. +// +// @param {String} eventType Type of the event. +// @param {module:image/imagestyle/imagestyleengine~ImageStyleFormat} style +// @param {module:engine/view/element~Element} viewElement +// @returns {Boolean} +function handleRemoval( eventType, style, viewElement ) { + if ( eventType == 'changeAttribute' || eventType == 'removeAttribute' ) { + if ( !style ) { + return false; + } + + viewElement.removeClass( style.className ); + } + + return true; +} + +// Handles converting addition of the attribute. +// Returns `true` when handling was processed correctly and further conversion can be performed. +// +// @param {String} eventType Type of the event. +// @param {module:image/imagestyle/imagestyleengine~ImageStyleFormat} style +// @param {module:engine/view/element~Element} viewElement +// @returns {Boolean} +function handleAddition( evenType, style, viewElement ) { + if ( evenType == 'addAttribute' || evenType == 'changeAttribute' ) { + if ( !style ) { + return false; + } + + viewElement.addClass( style.className ); + } + + return true; +} From 9ba4a3d780bc70bfa29f1917976288927ac5684c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szymon=20Kup=C5=9B?= Date: Tue, 10 Jan 2017 09:39:12 +0100 Subject: [PATCH 32/35] Using buildViewConverter for converting image styles. --- src/imagestyle/converters.js | 53 ---------------------------- src/imagestyle/imagestyleengine.js | 17 +++++++-- tests/imagestyle/imagestyleengine.js | 14 +++++++- 3 files changed, 27 insertions(+), 57 deletions(-) diff --git a/src/imagestyle/converters.js b/src/imagestyle/converters.js index 1e04d9c6..05f2a658 100644 --- a/src/imagestyle/converters.js +++ b/src/imagestyle/converters.js @@ -7,8 +7,6 @@ * @module image/imagestyle/converters */ -import { isImage } from '../utils'; - /** * Returns converter for `imageStyle` attribute. It can be used for adding, changing and removing the attribute. * @@ -37,57 +35,6 @@ export function modelToViewSetStyle( styles ) { }; } -/** - * Returns view to model converter converting image CSS classes to proper value in the model. - * - * @param {Array.} styles Styles for which converter is created. - * @returns {Function} View to model converter. - */ -export function viewToModelImageStyles( styles ) { - // Convert only styles without `null` value. - const filteredStyles = styles.filter( style => style.value !== null ); - - return ( evt, data, consumable, conversionApi ) => { - for ( let style of filteredStyles ) { - viewToModelImageStyle( style, data, consumable, conversionApi ); - } - }; -} - -// Converter from view to model converting single style. -// For more information see {@link module:engine/conversion/viewconversiondispatcher~ViewConversionDispatcher}; -// -// @param {module:image/imagestyle/imagestyleengine~ImageStyleFormat} style -// @param {Object} data -// @param {module:engine/conversion/viewconsumable~ViewConsumable} consumable -// @param {Object} conversionApi -function viewToModelImageStyle( style, data, consumable, conversionApi ) { - const viewFigureElement = data.input; - const modelImageElement = data.output; - - // *** Step 1: Validate conversion. - // Check if view element has proper class to consume. - if ( !consumable.test( viewFigureElement, { class: style.className } ) ) { - return; - } - - // Check if figure is converted to image. - if ( !isImage( modelImageElement ) ) { - return; - } - - // Check if image element can be placed in current context wit additional attribute. - const attributes = [ ...modelImageElement.getAttributeKeys(), 'imageStyle' ]; - - if ( !conversionApi.schema.check( { name: 'image', inside: data.context, attributes } ) ) { - return; - } - - // *** Step2: Convert to model. - consumable.consume( viewFigureElement, { class: style.className } ); - modelImageElement.setAttribute( 'imageStyle', style.value ); -} - // Returns style with given `value` from array of styles. // // @param {String} value diff --git a/src/imagestyle/imagestyleengine.js b/src/imagestyle/imagestyleengine.js index 26c9e012..348c82d2 100644 --- a/src/imagestyle/imagestyleengine.js +++ b/src/imagestyle/imagestyleengine.js @@ -10,9 +10,10 @@ import Plugin from 'ckeditor5-core/src/plugin'; import ImageStyleCommand from './imagestylecommand'; import ImageEngine from '../imageengine'; -import { viewToModelImageStyles, modelToViewSetStyle } from './converters'; +import { modelToViewSetStyle } from './converters'; import fullSizeIcon from 'ckeditor5-core/theme/icons/object-center.svg'; import sideIcon from 'ckeditor5-core/theme/icons/object-right.svg'; +import buildViewConverter from 'ckeditor5-engine/src/conversion/buildviewconverter'; /** * The image style engine plugin. Sets default configuration, creates converters and registers @@ -64,8 +65,18 @@ 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. - data.viewToModel.on( 'element:figure', viewToModelImageStyles( styles ), { priority: 'low' } ); + const viewConverter = buildViewConverter().for( data.viewToModel ); + + for ( let style of styles ) { + if ( style.value === null ) { + continue; + } + + viewConverter + .from( { name: 'figure', class: [ 'image', style.className ] } ) + .consuming( { class: style.className } ) + .toAttribute( 'imageStyle', style.value ); + } // Register separate command for each style. for ( let style of styles ) { diff --git a/tests/imagestyle/imagestyleengine.js b/tests/imagestyle/imagestyleengine.js index 6c7e71ab..06d0bbfb 100644 --- a/tests/imagestyle/imagestyleengine.js +++ b/tests/imagestyle/imagestyleengine.js @@ -55,12 +55,24 @@ describe( 'ImageStyleEngine', () => { expect( editor.commands.get( 'dummyStyle' ) ).to.be.instanceOf( ImageStyleCommand ); } ); - it( 'should convert from view to model', () => { + it( 'should convert from view to model #1', () => { + editor.setData( '
' ); + + expect( getModelData( document, { withoutSelection: true } ) ).to.equal( '' ); + } ); + + it( 'should convert from view to model #2', () => { editor.setData( '
' ); expect( getModelData( document, { withoutSelection: true } ) ).to.equal( '' ); } ); + it( 'should convert from view to model #3', () => { + editor.setData( '
' ); + + expect( getModelData( document, { withoutSelection: true } ) ).to.equal( '' ); + } ); + it( 'should not convert from view to model if class is not defined', () => { editor.setData( '
' ); From cc7370fc75c927a4031390ebabb1e36a7c227343 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szymon=20Kup=C5=9B?= Date: Tue, 10 Jan 2017 09:58:54 +0100 Subject: [PATCH 33/35] Revert "Using buildViewConverter for converting image styles." This reverts commit 9ba4a3d780bc70bfa29f1917976288927ac5684c. --- src/imagestyle/converters.js | 53 ++++++++++++++++++++++++++++ src/imagestyle/imagestyleengine.js | 17 ++------- tests/imagestyle/imagestyleengine.js | 14 +------- 3 files changed, 57 insertions(+), 27 deletions(-) diff --git a/src/imagestyle/converters.js b/src/imagestyle/converters.js index 05f2a658..1e04d9c6 100644 --- a/src/imagestyle/converters.js +++ b/src/imagestyle/converters.js @@ -7,6 +7,8 @@ * @module image/imagestyle/converters */ +import { isImage } from '../utils'; + /** * Returns converter for `imageStyle` attribute. It can be used for adding, changing and removing the attribute. * @@ -35,6 +37,57 @@ export function modelToViewSetStyle( styles ) { }; } +/** + * Returns view to model converter converting image CSS classes to proper value in the model. + * + * @param {Array.} styles Styles for which converter is created. + * @returns {Function} View to model converter. + */ +export function viewToModelImageStyles( styles ) { + // Convert only styles without `null` value. + const filteredStyles = styles.filter( style => style.value !== null ); + + return ( evt, data, consumable, conversionApi ) => { + for ( let style of filteredStyles ) { + viewToModelImageStyle( style, data, consumable, conversionApi ); + } + }; +} + +// Converter from view to model converting single style. +// For more information see {@link module:engine/conversion/viewconversiondispatcher~ViewConversionDispatcher}; +// +// @param {module:image/imagestyle/imagestyleengine~ImageStyleFormat} style +// @param {Object} data +// @param {module:engine/conversion/viewconsumable~ViewConsumable} consumable +// @param {Object} conversionApi +function viewToModelImageStyle( style, data, consumable, conversionApi ) { + const viewFigureElement = data.input; + const modelImageElement = data.output; + + // *** Step 1: Validate conversion. + // Check if view element has proper class to consume. + if ( !consumable.test( viewFigureElement, { class: style.className } ) ) { + return; + } + + // Check if figure is converted to image. + if ( !isImage( modelImageElement ) ) { + return; + } + + // Check if image element can be placed in current context wit additional attribute. + const attributes = [ ...modelImageElement.getAttributeKeys(), 'imageStyle' ]; + + if ( !conversionApi.schema.check( { name: 'image', inside: data.context, attributes } ) ) { + return; + } + + // *** Step2: Convert to model. + consumable.consume( viewFigureElement, { class: style.className } ); + modelImageElement.setAttribute( 'imageStyle', style.value ); +} + // Returns style with given `value` from array of styles. // // @param {String} value diff --git a/src/imagestyle/imagestyleengine.js b/src/imagestyle/imagestyleengine.js index 348c82d2..26c9e012 100644 --- a/src/imagestyle/imagestyleengine.js +++ b/src/imagestyle/imagestyleengine.js @@ -10,10 +10,9 @@ import Plugin from 'ckeditor5-core/src/plugin'; import ImageStyleCommand from './imagestylecommand'; import ImageEngine from '../imageengine'; -import { modelToViewSetStyle } from './converters'; +import { viewToModelImageStyles, modelToViewSetStyle } from './converters'; import fullSizeIcon from 'ckeditor5-core/theme/icons/object-center.svg'; import sideIcon from 'ckeditor5-core/theme/icons/object-right.svg'; -import buildViewConverter from 'ckeditor5-engine/src/conversion/buildviewconverter'; /** * The image style engine plugin. Sets default configuration, creates converters and registers @@ -65,18 +64,8 @@ export default class ImageStyleEngine extends Plugin { editing.modelToView.on( 'removeAttribute:imageStyle:image', modelToViewConverter ); data.modelToView.on( 'removeAttribute:imageStyle:image', modelToViewConverter ); - const viewConverter = buildViewConverter().for( data.viewToModel ); - - for ( let style of styles ) { - if ( style.value === null ) { - continue; - } - - viewConverter - .from( { name: 'figure', class: [ 'image', style.className ] } ) - .consuming( { class: style.className } ) - .toAttribute( 'imageStyle', style.value ); - } + // Converter for figure element from view to model. + data.viewToModel.on( 'element:figure', viewToModelImageStyles( styles ), { priority: 'low' } ); // Register separate command for each style. for ( let style of styles ) { diff --git a/tests/imagestyle/imagestyleengine.js b/tests/imagestyle/imagestyleengine.js index 06d0bbfb..6c7e71ab 100644 --- a/tests/imagestyle/imagestyleengine.js +++ b/tests/imagestyle/imagestyleengine.js @@ -55,24 +55,12 @@ describe( 'ImageStyleEngine', () => { expect( editor.commands.get( 'dummyStyle' ) ).to.be.instanceOf( ImageStyleCommand ); } ); - it( 'should convert from view to model #1', () => { - editor.setData( '
' ); - - expect( getModelData( document, { withoutSelection: true } ) ).to.equal( '' ); - } ); - - it( 'should convert from view to model #2', () => { + it( 'should convert from view to model', () => { editor.setData( '
' ); expect( getModelData( document, { withoutSelection: true } ) ).to.equal( '' ); } ); - it( 'should convert from view to model #3', () => { - editor.setData( '
' ); - - expect( getModelData( document, { withoutSelection: true } ) ).to.equal( '' ); - } ); - it( 'should not convert from view to model if class is not defined', () => { editor.setData( '
' ); From a174bdc3bdc1fc6a8970b5e0c45e74bde9c91c4d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotrek=20Koszuli=C5=84ski?= Date: Tue, 10 Jan 2017 11:53:46 +0100 Subject: [PATCH 34/35] Code refactoring: simplified logic. --- src/imagestyle/converters.js | 27 +++++++++++---------------- 1 file changed, 11 insertions(+), 16 deletions(-) diff --git a/src/imagestyle/converters.js b/src/imagestyle/converters.js index 1e04d9c6..35eb60fe 100644 --- a/src/imagestyle/converters.js +++ b/src/imagestyle/converters.js @@ -30,8 +30,7 @@ export function modelToViewSetStyle( styles ) { const oldStyle = getStyleByValue( data.attributeOldValue, styles ); const viewElement = conversionApi.mapper.toViewElement( data.item ); - if ( handleRemoval( eventType, oldStyle, viewElement ) && - handleAddition( eventType, newStyle, viewElement ) ) { + if ( handleRemoval( eventType, oldStyle, viewElement ) || handleAddition( eventType, newStyle, viewElement ) ) { consumable.consume( data.item, consumableType ); } }; @@ -107,17 +106,15 @@ function getStyleByValue( value, styles ) { // @param {String} eventType Type of the event. // @param {module:image/imagestyle/imagestyleengine~ImageStyleFormat} style // @param {module:engine/view/element~Element} viewElement -// @returns {Boolean} +// @returns {Boolean} Whether the change was handled. function handleRemoval( eventType, style, viewElement ) { - if ( eventType == 'changeAttribute' || eventType == 'removeAttribute' ) { - if ( !style ) { - return false; - } - + if ( style && ( eventType == 'changeAttribute' || eventType == 'removeAttribute' ) ) { viewElement.removeClass( style.className ); + + return true; } - return true; + return false; } // Handles converting addition of the attribute. @@ -126,15 +123,13 @@ function handleRemoval( eventType, style, viewElement ) { // @param {String} eventType Type of the event. // @param {module:image/imagestyle/imagestyleengine~ImageStyleFormat} style // @param {module:engine/view/element~Element} viewElement -// @returns {Boolean} +// @returns {Boolean} Whether the change was handled. function handleAddition( evenType, style, viewElement ) { - if ( evenType == 'addAttribute' || evenType == 'changeAttribute' ) { - if ( !style ) { - return false; - } - + if ( style && ( evenType == 'addAttribute' || evenType == 'changeAttribute' ) ) { viewElement.addClass( style.className ); + + return true; } - return true; + return false; } From 0b39383645724d3fcdc6ff02215d29d2fdf0cd0b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotrek=20Koszuli=C5=84ski?= Date: Tue, 10 Jan 2017 11:59:13 +0100 Subject: [PATCH 35/35] Code refactoring: renamed functions. --- src/imagestyle/converters.js | 6 +++--- src/imagestyle/imagestyleengine.js | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/imagestyle/converters.js b/src/imagestyle/converters.js index 35eb60fe..9dbedaf1 100644 --- a/src/imagestyle/converters.js +++ b/src/imagestyle/converters.js @@ -10,13 +10,13 @@ import { isImage } from '../utils'; /** - * Returns converter for `imageStyle` attribute. It can be used for adding, changing and removing the attribute. + * Returns converter for the `imageStyle` attribute. It can be used for adding, changing and removing the attribute. * * @param {Object} styles Object containing available styles. See {@link module:image/imagestyle/imagestyleengine~ImageStyleFormat} * for more details. * @returns {Function} Model to view attribute converter. */ -export function modelToViewSetStyle( styles ) { +export function modelToViewStyleAttribute( styles ) { return ( evt, data, consumable, conversionApi ) => { const eventType = evt.name.split( ':' )[ 0 ]; const consumableType = eventType + ':imageStyle'; @@ -42,7 +42,7 @@ export function modelToViewSetStyle( styles ) { * @param {Array.} styles Styles for which converter is created. * @returns {Function} View to model converter. */ -export function viewToModelImageStyles( styles ) { +export function viewToModelStyleAttribute( styles ) { // Convert only styles without `null` value. const filteredStyles = styles.filter( style => style.value !== null ); diff --git a/src/imagestyle/imagestyleengine.js b/src/imagestyle/imagestyleengine.js index 26c9e012..bdf67664 100644 --- a/src/imagestyle/imagestyleengine.js +++ b/src/imagestyle/imagestyleengine.js @@ -10,7 +10,7 @@ import Plugin from 'ckeditor5-core/src/plugin'; import ImageStyleCommand from './imagestylecommand'; import ImageEngine from '../imageengine'; -import { viewToModelImageStyles, modelToViewSetStyle } from './converters'; +import { viewToModelStyleAttribute, modelToViewStyleAttribute } from './converters'; import fullSizeIcon from 'ckeditor5-core/theme/icons/object-center.svg'; import sideIcon from 'ckeditor5-core/theme/icons/object-right.svg'; @@ -56,7 +56,7 @@ export default class ImageStyleEngine extends Plugin { schema.allow( { name: 'image', attributes: 'imageStyle', inside: '$root' } ); // Converters for imageStyle attribute from model to view. - const modelToViewConverter = modelToViewSetStyle( styles ); + const modelToViewConverter = modelToViewStyleAttribute( styles ); editing.modelToView.on( 'addAttribute:imageStyle:image', modelToViewConverter ); data.modelToView.on( 'addAttribute:imageStyle:image', modelToViewConverter ); editing.modelToView.on( 'changeAttribute:imageStyle:image', modelToViewConverter ); @@ -65,7 +65,7 @@ export default class ImageStyleEngine extends Plugin { data.modelToView.on( 'removeAttribute:imageStyle:image', modelToViewConverter ); // Converter for figure element from view to model. - data.viewToModel.on( 'element:figure', viewToModelImageStyles( styles ), { priority: 'low' } ); + data.viewToModel.on( 'element:figure', viewToModelStyleAttribute( styles ), { priority: 'low' } ); // Register separate command for each style. for ( let style of styles ) {