From f97ba0b78aead825f2f2069a0e47dee96231b744 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maksymilian=20Barna=C5=9B?= Date: Thu, 4 Feb 2021 15:26:27 +0100 Subject: [PATCH 01/26] Typo. --- packages/ckeditor5-engine/src/conversion/conversion.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/ckeditor5-engine/src/conversion/conversion.js b/packages/ckeditor5-engine/src/conversion/conversion.js index 3dd6393b6bc..7f93d4dc5d3 100644 --- a/packages/ckeditor5-engine/src/conversion/conversion.js +++ b/packages/ckeditor5-engine/src/conversion/conversion.js @@ -227,7 +227,7 @@ export default class Conversion { * editor.conversion.elementToElement( { * model: 'heading', * view: 'h2', - * // Convert "headling-like" paragraphs to headings. + * // Convert "heading-like" paragraphs to headings. * upcastAlso: viewElement => { * const fontSize = viewElement.getStyle( 'font-size' ); * From 159b3cf54b12f43cfcd7673e02d6771e412fc300 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maksymilian=20Barna=C5=9B?= Date: Fri, 5 Feb 2021 15:14:17 +0100 Subject: [PATCH 02/26] Handle alignment with classes. --- .../docs/features/text-alignment.md | 2 + .../src/alignmentediting.js | 100 +++++++++++++++++- .../tests/alignmentediting.js | 7 ++ .../tests/manual/alignment.js | 6 +- 4 files changed, 109 insertions(+), 6 deletions(-) diff --git a/packages/ckeditor5-alignment/docs/features/text-alignment.md b/packages/ckeditor5-alignment/docs/features/text-alignment.md index 625e94a7f75..a9c65b84487 100644 --- a/packages/ckeditor5-alignment/docs/features/text-alignment.md +++ b/packages/ckeditor5-alignment/docs/features/text-alignment.md @@ -22,6 +22,8 @@ There are more CKEditor 5 features that can help you organize your content: ## Configuring alignment options +TODO: Update docs + It is possible to configure which alignment options are available in the editor by setting the {@link module:alignment/alignment~AlignmentConfig#options `alignment.options`} configuration option. You can choose from `'left'`, `'right'`, `'center'` and `'justify'`. diff --git a/packages/ckeditor5-alignment/src/alignmentediting.js b/packages/ckeditor5-alignment/src/alignmentediting.js index b44320e5428..378f6a2543c 100644 --- a/packages/ckeditor5-alignment/src/alignmentediting.js +++ b/packages/ckeditor5-alignment/src/alignmentediting.js @@ -8,6 +8,7 @@ */ import { Plugin } from 'ckeditor5/src/core'; +import { CKEditorError } from '../../../src/utils'; import AlignmentCommand from './alignmentcommand'; import { isDefault, isSupported, supportedOptions } from './utils'; @@ -32,7 +33,8 @@ export default class AlignmentEditing extends Plugin { super( editor ); editor.config.define( 'alignment', { - options: [ ...supportedOptions ] + options: [ ...supportedOptions ], + classNames: [] } ); } @@ -46,22 +48,62 @@ export default class AlignmentEditing extends Plugin { // Filter out unsupported options. const enabledOptions = editor.config.get( 'alignment.options' ).filter( isSupported ); + const classNameConfig = editor.config.get( 'alignment.classNames' ); + + if ( !Array.isArray( classNameConfig ) || + ( classNameConfig.length && classNameConfig.length != enabledOptions.length ) + ) { + /** + * The number of items in `alignment.classNames` should match number of items in `alignment.options`. + * + * @error alignment-config-classnames-not-matching + * // TODO Fix params + * @param {Array.} classNameConfig Classes listed in the config. + */ + throw new CKEditorError( 'alignment-config-classnames-not-matching', null, { enabledOptions, classNameConfig } ); + } // Allow alignment attribute on all blocks. schema.extend( '$block', { allowAttributes: 'alignment' } ); editor.model.schema.setAttributeProperties( 'alignment', { isFormatting: true } ); - const definition = _buildDefinition( enabledOptions.filter( option => !isDefault( option, locale ) ) ); + const shouldUseClasses = classNameConfig.length; + const options = enabledOptions.filter( option => !isDefault( option, locale ) ); + + if ( shouldUseClasses ) { + // Upcast and Downcast classes. + + const alignmentClassNames = classNameConfig.reduce( ( classNameMap, className, index ) => { + classNameMap[ enabledOptions[ index ] ] = className; + + return classNameMap; + }, {} ); + + const definition = _buildClassDefinition( options, alignmentClassNames ); + + editor.conversion.attributeToAttribute( definition ); + } else { + // Downcast inline styles. - editor.conversion.attributeToAttribute( definition ); + const definition = _buildDowncastInlineDefinition( options ); + + editor.conversion.for( 'downcast' ).attributeToAttribute( definition ); + } + + const upcastInlineDefinitions = _buildUpcastInlineDefinitions( options ); + + // Always upcast from inline styles. + for ( const definition of upcastInlineDefinitions ) { + editor.conversion.for( 'upcast' ).attributeToAttribute( definition ); + } editor.commands.add( 'alignment', new AlignmentCommand( editor ) ); } } -// Utility function responsible for building converter definition. +// Prepare downcast conversion definition for inline alignment styling. // @private -function _buildDefinition( options ) { +function _buildDowncastInlineDefinition( options ) { const definition = { model: { key: 'alignment', @@ -81,3 +123,51 @@ function _buildDefinition( options ) { return definition; } + +// Prepare upcast definitions for inline alignment styles. +// @private +function _buildUpcastInlineDefinitions( options ) { + const definitions = []; + + for ( const option of options ) { + const view = { + key: 'style', + value: { + 'text-align': option + } + }; + const model = { + key: 'alignment', + value: option + }; + + definitions.push( { + view, + model + } ); + } + + return definitions; +} + +// Prepare conversion definitions for both upcast and downcast. +// @private +function _buildClassDefinition( options, alignmentClassNames ) { + const definition = { + model: { + key: 'alignment', + values: options.slice() + }, + view: {} + }; + + for ( const option of options ) { + definition.view[ option ] = { + key: 'class', + value: [ alignmentClassNames[ option ] ] + }; + } + + return definition; +} + diff --git a/packages/ckeditor5-alignment/tests/alignmentediting.js b/packages/ckeditor5-alignment/tests/alignmentediting.js index 59ed6bde282..2275f82f8b3 100644 --- a/packages/ckeditor5-alignment/tests/alignmentediting.js +++ b/packages/ckeditor5-alignment/tests/alignmentediting.js @@ -301,6 +301,13 @@ describe( 'AlignmentEditing', () => { } ); } ); } ); + + // TODO: + // * Can work without class names (backwards-compatible). + // * Incorrect length of params. + // * Not an array. + // * Limited options should map to limited set of classes. + describe( 'classNames', () => {} ); } ); } ); diff --git a/packages/ckeditor5-alignment/tests/manual/alignment.js b/packages/ckeditor5-alignment/tests/manual/alignment.js index 897b0c0feb6..7d5cdbb467c 100644 --- a/packages/ckeditor5-alignment/tests/manual/alignment.js +++ b/packages/ckeditor5-alignment/tests/manual/alignment.js @@ -14,7 +14,11 @@ ClassicEditor plugins: [ ArticlePluginSet, Alignment ], toolbar: [ 'heading', '|', 'alignment', '|', 'bold', 'italic', 'link', 'bulletedList', 'numberedList', 'blockQuote', 'undo', 'redo' - ] + ], + alignment: { + // classNames: [ 'ckx-align-left', 'ckx-align-right' ], // , 'ckx-align-center', 'ckx-align-justify' ], + options: [ 'left', 'center' ] + } } ) .then( editor => { window.editor = editor; From 0f93c3117e24695844d56a8c195c6351ee29ba7b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maksymilian=20Barna=C5=9B?= Date: Mon, 8 Feb 2021 10:43:30 +0100 Subject: [PATCH 03/26] Updated tests. --- .../tests/alignmentediting.js | 91 +++++++++++++++++-- 1 file changed, 85 insertions(+), 6 deletions(-) diff --git a/packages/ckeditor5-alignment/tests/alignmentediting.js b/packages/ckeditor5-alignment/tests/alignmentediting.js index 2275f82f8b3..769e92a257f 100644 --- a/packages/ckeditor5-alignment/tests/alignmentediting.js +++ b/packages/ckeditor5-alignment/tests/alignmentediting.js @@ -12,6 +12,7 @@ import VirtualTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/virtualtest import { getData as getModelData, setData as setModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model'; import AlignmentCommand from '../src/alignmentcommand'; +import { CKEditorError } from '../../../src/utils'; describe( 'AlignmentEditing', () => { let editor, model; @@ -302,12 +303,90 @@ describe( 'AlignmentEditing', () => { } ); } ); - // TODO: - // * Can work without class names (backwards-compatible). - // * Incorrect length of params. - // * Not an array. - // * Limited options should map to limited set of classes. - describe( 'classNames', () => {} ); + describe( 'classNames', () => { + it( 'default value', () => { + expect( editor.config.get( 'alignment.classNames' ) ).to.deep.equal( [ ] ); + } ); + + it( 'should throw when there are too many classes', async () => { + try { + await VirtualTestEditor + .create( { + language: { + content: 'ar' + }, + plugins: [ AlignmentEditing, Paragraph ], + alignment: { + classNames: [ 'foo-left', 'foo-right', 'foo-center', 'foo-justify', 'foo-extra' ] + } + } ); + } catch ( error ) { + expect( error.constructor ).to.equal( CKEditorError ); + expect( error ).to.match( /alignment-config-classnames-not-matching/ ); + } + } ); + + it( 'should throw when there are fewer classes than alignment options', async () => { + try { + await VirtualTestEditor + .create( { + language: { + content: 'ar' + }, + plugins: [ AlignmentEditing, Paragraph ], + alignment: { + classNames: [ 'foo-left', 'foo-right', 'foo-center' ] + } + } ); + } catch ( error ) { + expect( error.constructor ).to.equal( CKEditorError ); + expect( error ).to.match( /alignment-config-classnames-not-matching/ ); + } + } ); + + it( 'should throw when classNames are not an array', async () => { + try { + await VirtualTestEditor + .create( { + language: { + content: 'ar' + }, + plugins: [ AlignmentEditing, Paragraph ], + alignment: { + classNames: {} + } + } ); + } catch ( error ) { + expect( error.constructor ).to.equal( CKEditorError ); + expect( error ).to.match( /alignment-config-classnames-not-matching/ ); + } + } ); + + it( 'should map limited options to limited set of classes', () => { + return VirtualTestEditor + .create( { + language: { + content: 'ar' + }, + plugins: [ AlignmentEditing, Paragraph ], + alignment: { + options: [ 'left', 'center' ], + classNames: [ 'foo-left', 'foo-right' ] + } + } ) + .then( newEditor => { + const model = newEditor.model; + const data = '

x

'; + + newEditor.setData( data ); + + expect( getModelData( model ) ).to.equal( '[]x' ); + expect( newEditor.getData() ).to.equal( '

x

' ); + + return newEditor.destroy(); + } ); + } ); + } ); } ); } ); From d9ba476c130d54f1660d824f7fa0be23c767ba15 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maksymilian=20Barna=C5=9B?= Date: Mon, 8 Feb 2021 10:51:35 +0100 Subject: [PATCH 04/26] Update tests, update exception params. --- packages/ckeditor5-alignment/src/alignmentediting.js | 2 +- packages/ckeditor5-alignment/tests/alignmentediting.js | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/ckeditor5-alignment/src/alignmentediting.js b/packages/ckeditor5-alignment/src/alignmentediting.js index 378f6a2543c..4ba2ec702d6 100644 --- a/packages/ckeditor5-alignment/src/alignmentediting.js +++ b/packages/ckeditor5-alignment/src/alignmentediting.js @@ -57,7 +57,7 @@ export default class AlignmentEditing extends Plugin { * The number of items in `alignment.classNames` should match number of items in `alignment.options`. * * @error alignment-config-classnames-not-matching - * // TODO Fix params + * @param {Array.} enabledOptions Available alignment options set in config. * @param {Array.} classNameConfig Classes listed in the config. */ throw new CKEditorError( 'alignment-config-classnames-not-matching', null, { enabledOptions, classNameConfig } ); diff --git a/packages/ckeditor5-alignment/tests/alignmentediting.js b/packages/ckeditor5-alignment/tests/alignmentediting.js index 769e92a257f..765b1d25a45 100644 --- a/packages/ckeditor5-alignment/tests/alignmentediting.js +++ b/packages/ckeditor5-alignment/tests/alignmentediting.js @@ -304,8 +304,10 @@ describe( 'AlignmentEditing', () => { } ); describe( 'classNames', () => { - it( 'default value', () => { - expect( editor.config.get( 'alignment.classNames' ) ).to.deep.equal( [ ] ); + describe( 'default value', () => { + it( 'should be set', () => { + expect( editor.config.get( 'alignment.classNames' ) ).to.deep.equal( [ ] ); + } ); } ); it( 'should throw when there are too many classes', async () => { From 02c82ad5d3d71d6fa17a8a1d87abad9007530b48 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maksymilian=20Barna=C5=9B?= Date: Thu, 11 Feb 2021 09:49:31 +0100 Subject: [PATCH 05/26] Docs. --- .../docs/features/text-alignment.md | 27 ++++++++++++++++++- packages/ckeditor5-alignment/src/alignment.js | 18 +++++++++++++ 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/packages/ckeditor5-alignment/docs/features/text-alignment.md b/packages/ckeditor5-alignment/docs/features/text-alignment.md index a9c65b84487..73ed6ef32fd 100644 --- a/packages/ckeditor5-alignment/docs/features/text-alignment.md +++ b/packages/ckeditor5-alignment/docs/features/text-alignment.md @@ -22,7 +22,7 @@ There are more CKEditor 5 features that can help you organize your content: ## Configuring alignment options -TODO: Update docs +### Alignment options It is possible to configure which alignment options are available in the editor by setting the {@link module:alignment/alignment~AlignmentConfig#options `alignment.options`} configuration option. You can choose from `'left'`, `'right'`, `'center'` and `'justify'`. @@ -48,6 +48,31 @@ ClassicEditor {@snippet features/custom-text-alignment-options} +### Assigning class to alignment option + +By default alignment sets the style inline. If you wish to style the alignment by yourself, you can specify class names by using {@link module:alignment/alignment~AlignmentConfig#classNames `alignment.classNames`} and style it using CSS. + + + Make sure to specify the same number of classes as the number of values in {@link module:alignment/alignment~AlignmentConfig#options `alignment.options`}. If the {@link module:alignment/alignment~AlignmentConfig#options `alignment.options`} are not specified, they are equal to `[ 'left', 'right', 'center', 'justify' ]`, therefore you have to provide 4 class names. + + +The following configuration will set `.my-align-left` and `.my-align-right` to left and right alignment respectively. + +```js +ClassicEditor + .create( document.querySelector( '#editor' ), { + alignment: { + options: [ 'left', 'right' ], + classNames: [ 'my-align-left', 'my-align-right' ] + }, + toolbar: [ + 'heading', '|', 'bulletedList', 'numberedList', 'alignment', 'undo', 'redo' + ] + } ) + .then( ... ) + .catch( ... ); +``` + ## Configuring the toolbar You can choose to use the alignment dropdown (`'alignment'`) or configure the toolbar to use separate buttons for each of the options: diff --git a/packages/ckeditor5-alignment/src/alignment.js b/packages/ckeditor5-alignment/src/alignment.js index 6012d44ce7e..454fb1d4afb 100644 --- a/packages/ckeditor5-alignment/src/alignment.js +++ b/packages/ckeditor5-alignment/src/alignment.js @@ -64,6 +64,24 @@ export default class Alignment extends Plugin { * @interface AlignmentConfig */ +/** + * Class names to match {@link module:alignment/alignment~AlignmentConfig#options alignment options}. + * + * **Note:** The same number of classes should be provided as the number of `alignment.options`. + * + * ClassicEditor + * .create( editorElement, { + * alignment: { + * options: [ 'left', 'right' ], + * classNames: [ 'my-align-left', 'my-align-right' ] + * } + * } ) + * .then( ... ) + * .catch( ... ); + * + * @member {Array.} module:alignment/alignment~AlignmentConfig#classNames + */ + /** * Available alignment options. * From a018c19b7ed5f2eb9563ce94ecd90f6d8e8dc06b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maksymilian=20Barna=C5=9B?= Date: Thu, 11 Feb 2021 10:50:53 +0100 Subject: [PATCH 06/26] Tests. --- .../tests/alignmentediting.js | 167 ++++++++++++++++-- .../tests/manual/alignment.js | 1 - 2 files changed, 151 insertions(+), 17 deletions(-) diff --git a/packages/ckeditor5-alignment/tests/alignmentediting.js b/packages/ckeditor5-alignment/tests/alignmentediting.js index 765b1d25a45..432e8347e4c 100644 --- a/packages/ckeditor5-alignment/tests/alignmentediting.js +++ b/packages/ckeditor5-alignment/tests/alignmentediting.js @@ -138,6 +138,33 @@ describe( 'AlignmentEditing', () => { return newEditor.destroy(); } ); } ); + + it( 'uses class when alignment.classNames is set', async () => { + return VirtualTestEditor + .create( { + language: { + content: 'ar' + }, + plugins: [ AlignmentEditing, Paragraph ], + alignment: { + classNames: [ 'foo-left', 'foo-right', 'foo-center', 'foo-justify' ] + } + } ) + .then( newEditor => { + const model = newEditor.model; + + setModelData( model, '[]x' ); + + expect( newEditor.getData() ).to.equal( '

x

' ); + + newEditor.execute( 'alignment', { value: 'left' } ); + + expect( getModelData( model ) ).to.equal( '[]x' ); + expect( newEditor.getData() ).to.equal( '

x

' ); + + return newEditor.destroy(); + } ); + } ); } ); it( 'adds a converter to the view pipeline for removing attribute', () => { @@ -149,6 +176,30 @@ describe( 'AlignmentEditing', () => { expect( editor.getData() ).to.equal( '

x

' ); } ); + + it( 'uses class when alignment.classNames is set', async () => { + return VirtualTestEditor + .create( { + plugins: [ AlignmentEditing, Paragraph ], + alignment: { + classNames: [ 'foo-left', 'foo-right', 'foo-center', 'foo-justify' ] + } + } ) + .then( newEditor => { + const model = newEditor.model; + + setModelData( model, '[]x' ); + + expect( newEditor.getData() ).to.equal( '

x

' ); + + newEditor.execute( 'alignment', { value: 'left' } ); + + expect( getModelData( model ) ).to.equal( '[]x' ); + expect( newEditor.getData() ).to.equal( '

x

' ); + + return newEditor.destroy(); + } ); + } ); } ); describe( 'center alignment', () => { @@ -176,6 +227,33 @@ describe( 'AlignmentEditing', () => { expect( editor.getData() ).to.equal( '

x

' ); } ); + + it( 'uses class when alignment.classNames is set', async () => { + return VirtualTestEditor + .create( { + language: { + content: 'ar' + }, + plugins: [ AlignmentEditing, Paragraph ], + alignment: { + classNames: [ 'foo-left', 'foo-right', 'foo-center', 'foo-justify' ] + } + } ) + .then( newEditor => { + const model = newEditor.model; + + setModelData( model, '[]x' ); + + expect( newEditor.getData() ).to.equal( '

x

' ); + + newEditor.execute( 'alignment', { value: 'center' } ); + + expect( getModelData( model ) ).to.equal( '[]x' ); + expect( newEditor.getData() ).to.equal( '

x

' ); + + return newEditor.destroy(); + } ); + } ); } ); describe( 'right alignment', () => { @@ -236,6 +314,30 @@ describe( 'AlignmentEditing', () => { } ); } ); } ); + + it( 'uses class when alignment.classNames is set', async () => { + return VirtualTestEditor + .create( { + plugins: [ AlignmentEditing, Paragraph ], + alignment: { + classNames: [ 'foo-left', 'foo-right', 'foo-center', 'foo-justify' ] + } + } ) + .then( newEditor => { + const model = newEditor.model; + + setModelData( model, '[]x' ); + + expect( newEditor.getData() ).to.equal( '

x

' ); + + newEditor.execute( 'alignment', { value: 'right' } ); + + expect( getModelData( model ) ).to.equal( '[]x' ); + expect( newEditor.getData() ).to.equal( '

x

' ); + + return newEditor.destroy(); + } ); + } ); } ); describe( 'justify alignment', () => { @@ -253,6 +355,30 @@ describe( 'AlignmentEditing', () => { expect( editor.getData() ).to.equal( '

x

' ); } ); + + it( 'uses class when alignment.classNames is set', async () => { + return VirtualTestEditor + .create( { + plugins: [ AlignmentEditing, Paragraph ], + alignment: { + classNames: [ 'foo-left', 'foo-right', 'foo-center', 'foo-justify' ] + } + } ) + .then( newEditor => { + const model = newEditor.model; + + setModelData( model, '[]x' ); + + expect( newEditor.getData() ).to.equal( '

x

' ); + + newEditor.execute( 'alignment', { value: 'justify' } ); + + expect( getModelData( model ) ).to.equal( '[]x' ); + expect( newEditor.getData() ).to.equal( '

x

' ); + + return newEditor.destroy(); + } ); + } ); } ); describe( 'should be extensible', () => { @@ -314,9 +440,6 @@ describe( 'AlignmentEditing', () => { try { await VirtualTestEditor .create( { - language: { - content: 'ar' - }, plugins: [ AlignmentEditing, Paragraph ], alignment: { classNames: [ 'foo-left', 'foo-right', 'foo-center', 'foo-justify', 'foo-extra' ] @@ -332,9 +455,6 @@ describe( 'AlignmentEditing', () => { try { await VirtualTestEditor .create( { - language: { - content: 'ar' - }, plugins: [ AlignmentEditing, Paragraph ], alignment: { classNames: [ 'foo-left', 'foo-right', 'foo-center' ] @@ -350,9 +470,6 @@ describe( 'AlignmentEditing', () => { try { await VirtualTestEditor .create( { - language: { - content: 'ar' - }, plugins: [ AlignmentEditing, Paragraph ], alignment: { classNames: {} @@ -367,23 +484,41 @@ describe( 'AlignmentEditing', () => { it( 'should map limited options to limited set of classes', () => { return VirtualTestEditor .create( { - language: { - content: 'ar' - }, plugins: [ AlignmentEditing, Paragraph ], alignment: { options: [ 'left', 'center' ], - classNames: [ 'foo-left', 'foo-right' ] + classNames: [ 'foo-left', 'foo-center' ] } } ) .then( newEditor => { const model = newEditor.model; - const data = '

x

'; + const data = '

x

'; newEditor.setData( data ); - expect( getModelData( model ) ).to.equal( '[]x' ); - expect( newEditor.getData() ).to.equal( '

x

' ); + expect( getModelData( model ) ).to.equal( '[]x' ); + expect( newEditor.getData() ).to.equal( '

x

' ); + + return newEditor.destroy(); + } ); + } ); + + it( 'should map classes to default options', () => { + return VirtualTestEditor + .create( { + plugins: [ AlignmentEditing, Paragraph ], + alignment: { + classNames: [ 'foo-left', 'foo-right', 'foo-center', 'foo-justify' ] + } + } ) + .then( newEditor => { + const model = newEditor.model; + const data = '

x

'; + + newEditor.setData( data ); + + expect( getModelData( model ) ).to.equal( '[]x' ); + expect( newEditor.getData() ).to.equal( '

x

' ); return newEditor.destroy(); } ); diff --git a/packages/ckeditor5-alignment/tests/manual/alignment.js b/packages/ckeditor5-alignment/tests/manual/alignment.js index 7d5cdbb467c..9460952e778 100644 --- a/packages/ckeditor5-alignment/tests/manual/alignment.js +++ b/packages/ckeditor5-alignment/tests/manual/alignment.js @@ -16,7 +16,6 @@ ClassicEditor 'heading', '|', 'alignment', '|', 'bold', 'italic', 'link', 'bulletedList', 'numberedList', 'blockQuote', 'undo', 'redo' ], alignment: { - // classNames: [ 'ckx-align-left', 'ckx-align-right' ], // , 'ckx-align-center', 'ckx-align-justify' ], options: [ 'left', 'center' ] } } ) From 5a52dde4ee4f600da46aef2b8cdd507ce85c45e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maksymilian=20Barna=C5=9B?= Date: Fri, 12 Feb 2021 09:56:51 +0100 Subject: [PATCH 07/26] Remove unused code. --- packages/ckeditor5-alignment/src/alignmentediting.js | 2 +- packages/ckeditor5-alignment/tests/manual/alignment.js | 5 +---- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/packages/ckeditor5-alignment/src/alignmentediting.js b/packages/ckeditor5-alignment/src/alignmentediting.js index 4ba2ec702d6..d75dc74b0e3 100644 --- a/packages/ckeditor5-alignment/src/alignmentediting.js +++ b/packages/ckeditor5-alignment/src/alignmentediting.js @@ -150,7 +150,7 @@ function _buildUpcastInlineDefinitions( options ) { return definitions; } -// Prepare conversion definitions for both upcast and downcast. +// Prepare conversion definitions for upcast and downcast alignment with classes. // @private function _buildClassDefinition( options, alignmentClassNames ) { const definition = { diff --git a/packages/ckeditor5-alignment/tests/manual/alignment.js b/packages/ckeditor5-alignment/tests/manual/alignment.js index 9460952e778..897b0c0feb6 100644 --- a/packages/ckeditor5-alignment/tests/manual/alignment.js +++ b/packages/ckeditor5-alignment/tests/manual/alignment.js @@ -14,10 +14,7 @@ ClassicEditor plugins: [ ArticlePluginSet, Alignment ], toolbar: [ 'heading', '|', 'alignment', '|', 'bold', 'italic', 'link', 'bulletedList', 'numberedList', 'blockQuote', 'undo', 'redo' - ], - alignment: { - options: [ 'left', 'center' ] - } + ] } ) .then( editor => { window.editor = editor; From 950bd7585a13986c587f1e6560e379f119064b70 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maksymilian=20Barna=C5=9B?= Date: Tue, 16 Feb 2021 09:56:10 +0100 Subject: [PATCH 08/26] Docs cleanup. --- .../ckeditor5-alignment/docs/features/text-alignment.md | 8 ++++---- packages/ckeditor5-alignment/src/alignment.js | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/ckeditor5-alignment/docs/features/text-alignment.md b/packages/ckeditor5-alignment/docs/features/text-alignment.md index 73ed6ef32fd..f5dcae875c6 100644 --- a/packages/ckeditor5-alignment/docs/features/text-alignment.md +++ b/packages/ckeditor5-alignment/docs/features/text-alignment.md @@ -22,7 +22,7 @@ There are more CKEditor 5 features that can help you organize your content: ## Configuring alignment options -### Alignment options +### Defining available options It is possible to configure which alignment options are available in the editor by setting the {@link module:alignment/alignment~AlignmentConfig#options `alignment.options`} configuration option. You can choose from `'left'`, `'right'`, `'center'` and `'justify'`. @@ -48,12 +48,12 @@ ClassicEditor {@snippet features/custom-text-alignment-options} -### Assigning class to alignment option +### Using classes instead of inline style -By default alignment sets the style inline. If you wish to style the alignment by yourself, you can specify class names by using {@link module:alignment/alignment~AlignmentConfig#classNames `alignment.classNames`} and style it using CSS. +By default alignment is set inline using `text-align` CSS property. If you wish to style the alignment by yourself, you can specify class names by using {@link module:alignment/alignment~AlignmentConfig#classNames `config.alignment.classNames`} and style it using CSS. - Make sure to specify the same number of classes as the number of values in {@link module:alignment/alignment~AlignmentConfig#options `alignment.options`}. If the {@link module:alignment/alignment~AlignmentConfig#options `alignment.options`} are not specified, they are equal to `[ 'left', 'right', 'center', 'justify' ]`, therefore you have to provide 4 class names. + Make sure to specify the same number of classes as the number of values in {@link module:alignment/alignment~AlignmentConfig#options `config.alignment.options`}. If the {@link module:alignment/alignment~AlignmentConfig#options `config.alignment.options`} are not specified, they are equal to `[ 'left', 'right', 'center', 'justify' ]`, therefore you have to provide 4 class names. The following configuration will set `.my-align-left` and `.my-align-right` to left and right alignment respectively. diff --git a/packages/ckeditor5-alignment/src/alignment.js b/packages/ckeditor5-alignment/src/alignment.js index 454fb1d4afb..7f68a156814 100644 --- a/packages/ckeditor5-alignment/src/alignment.js +++ b/packages/ckeditor5-alignment/src/alignment.js @@ -67,7 +67,7 @@ export default class Alignment extends Plugin { /** * Class names to match {@link module:alignment/alignment~AlignmentConfig#options alignment options}. * - * **Note:** The same number of classes should be provided as the number of `alignment.options`. + * **Note:** The same number of classes should be provided as the number of `config.alignment.options`. * * ClassicEditor * .create( editorElement, { From 226b9372d3d80577e2fe22d545369b243be5ba5d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maksymilian=20Barna=C5=9B?= Date: Tue, 16 Feb 2021 12:48:24 +0100 Subject: [PATCH 09/26] Fix wording. --- packages/ckeditor5-alignment/docs/features/text-alignment.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/ckeditor5-alignment/docs/features/text-alignment.md b/packages/ckeditor5-alignment/docs/features/text-alignment.md index f5dcae875c6..b5f2eaa6559 100644 --- a/packages/ckeditor5-alignment/docs/features/text-alignment.md +++ b/packages/ckeditor5-alignment/docs/features/text-alignment.md @@ -50,7 +50,7 @@ ClassicEditor ### Using classes instead of inline style -By default alignment is set inline using `text-align` CSS property. If you wish to style the alignment by yourself, you can specify class names by using {@link module:alignment/alignment~AlignmentConfig#classNames `config.alignment.classNames`} and style it using CSS. +By default alignment is set inline using `text-align` CSS property. If you wish the feature to output more semantic content that uses classes instead of inline styles, you can specify class names by using the {@link module:alignment/alignment~AlignmentConfig#classNames `config.alignment.classNames`} option and style them by using a stylesheet. Make sure to specify the same number of classes as the number of values in {@link module:alignment/alignment~AlignmentConfig#options `config.alignment.options`}. If the {@link module:alignment/alignment~AlignmentConfig#options `config.alignment.options`} are not specified, they are equal to `[ 'left', 'right', 'center', 'justify' ]`, therefore you have to provide 4 class names. From 08a4365534bec24b1f7fe765b64251b8c7f0210a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maksymilian=20Barna=C5=9B?= Date: Wed, 17 Feb 2021 22:46:22 +0100 Subject: [PATCH 10/26] Normalize config. Clean up async tests. --- .../src/alignmentediting.js | 40 +-- packages/ckeditor5-alignment/src/utils.js | 45 +++ .../tests/alignmentediting.js | 339 +++++++++--------- 3 files changed, 237 insertions(+), 187 deletions(-) diff --git a/packages/ckeditor5-alignment/src/alignmentediting.js b/packages/ckeditor5-alignment/src/alignmentediting.js index d75dc74b0e3..88d7e50348e 100644 --- a/packages/ckeditor5-alignment/src/alignmentediting.js +++ b/packages/ckeditor5-alignment/src/alignmentediting.js @@ -8,10 +8,9 @@ */ import { Plugin } from 'ckeditor5/src/core'; -import { CKEditorError } from '../../../src/utils'; import AlignmentCommand from './alignmentcommand'; -import { isDefault, isSupported, supportedOptions } from './utils'; +import { defaultOptions, isDefault, isSupported, normalizeAlignmentOptions } from './utils'; /** * The alignment editing feature. It introduces the {@link module:alignment/alignmentcommand~AlignmentCommand command} and adds @@ -33,7 +32,7 @@ export default class AlignmentEditing extends Plugin { super( editor ); editor.config.define( 'alignment', { - options: [ ...supportedOptions ], + options: [ ...defaultOptions ], classNames: [] } ); } @@ -46,28 +45,19 @@ export default class AlignmentEditing extends Plugin { const locale = editor.locale; const schema = editor.model.schema; + const alignmentOptions = normalizeAlignmentOptions( editor.config.get( 'alignment.options' ) ); + // Filter out unsupported options. - const enabledOptions = editor.config.get( 'alignment.options' ).filter( isSupported ); - const classNameConfig = editor.config.get( 'alignment.classNames' ); - - if ( !Array.isArray( classNameConfig ) || - ( classNameConfig.length && classNameConfig.length != enabledOptions.length ) - ) { - /** - * The number of items in `alignment.classNames` should match number of items in `alignment.options`. - * - * @error alignment-config-classnames-not-matching - * @param {Array.} enabledOptions Available alignment options set in config. - * @param {Array.} classNameConfig Classes listed in the config. - */ - throw new CKEditorError( 'alignment-config-classnames-not-matching', null, { enabledOptions, classNameConfig } ); - } + const enabledOptions = alignmentOptions.map( option => option.name ).filter( isSupported ); + const classNameConfig = alignmentOptions.map( option => option.className ); // Allow alignment attribute on all blocks. schema.extend( '$block', { allowAttributes: 'alignment' } ); editor.model.schema.setAttributeProperties( 'alignment', { isFormatting: true } ); - const shouldUseClasses = classNameConfig.length; + const shouldUseClasses = classNameConfig.filter( className => !!className ).length; + + // There is no need for converting alignment that's the same as current text direction. const options = enabledOptions.filter( option => !isDefault( option, locale ) ); if ( shouldUseClasses ) { @@ -79,18 +69,18 @@ export default class AlignmentEditing extends Plugin { return classNameMap; }, {} ); - const definition = _buildClassDefinition( options, alignmentClassNames ); + const definition = buildClassDefinition( options, alignmentClassNames ); editor.conversion.attributeToAttribute( definition ); } else { // Downcast inline styles. - const definition = _buildDowncastInlineDefinition( options ); + const definition = buildDowncastInlineDefinition( options ); editor.conversion.for( 'downcast' ).attributeToAttribute( definition ); } - const upcastInlineDefinitions = _buildUpcastInlineDefinitions( options ); + const upcastInlineDefinitions = buildUpcastInlineDefinitions( options ); // Always upcast from inline styles. for ( const definition of upcastInlineDefinitions ) { @@ -103,7 +93,7 @@ export default class AlignmentEditing extends Plugin { // Prepare downcast conversion definition for inline alignment styling. // @private -function _buildDowncastInlineDefinition( options ) { +function buildDowncastInlineDefinition( options ) { const definition = { model: { key: 'alignment', @@ -126,7 +116,7 @@ function _buildDowncastInlineDefinition( options ) { // Prepare upcast definitions for inline alignment styles. // @private -function _buildUpcastInlineDefinitions( options ) { +function buildUpcastInlineDefinitions( options ) { const definitions = []; for ( const option of options ) { @@ -152,7 +142,7 @@ function _buildUpcastInlineDefinitions( options ) { // Prepare conversion definitions for upcast and downcast alignment with classes. // @private -function _buildClassDefinition( options, alignmentClassNames ) { +function buildClassDefinition( options, alignmentClassNames ) { const definition = { model: { key: 'alignment', diff --git a/packages/ckeditor5-alignment/src/utils.js b/packages/ckeditor5-alignment/src/utils.js index a7881b91581..0f81d9f3c01 100644 --- a/packages/ckeditor5-alignment/src/utils.js +++ b/packages/ckeditor5-alignment/src/utils.js @@ -3,6 +3,8 @@ * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license */ +import { CKEditorError } from '../../../src/utils'; + /** * @module alignment/utils */ @@ -16,6 +18,11 @@ * * `'justify'` */ export const supportedOptions = [ 'left', 'right', 'center', 'justify' ]; +export const defaultOptions = supportedOptions.map( option => { + return Object.assign( {}, { + name: option + } ); +} ); /** * Checks whether the passed option is supported by {@link module:alignment/alignmentediting~AlignmentEditing}. @@ -44,3 +51,41 @@ export function isDefault( alignment, locale ) { return alignment === 'left'; } } + +export function normalizeAlignmentOptions( configuredOptions = [] ) { + return configuredOptions.map( ( option, index, options ) => { + let optionObj; + + if ( typeof option == 'string' ) { + optionObj = Object.assign( {}, { name: option } ); + } else { + optionObj = Object.assign( {}, defaultOptions[ index ], option ); + } + + const succeedingOptions = options.slice( index + 1 ); + const nameAlreadyExists = succeedingOptions.some( item => { + const optionName = item.name || item; + + return optionName == optionObj.name; + } ); + + if ( nameAlreadyExists ) { + // TODO: Fill in api docs. + // eslint-disable-next-line ckeditor5-rules/ckeditor-error-message + throw new CKEditorError( 'alignment-config-name-already-defined' ); + } + + const classNameAlreadyExists = optionObj.className && succeedingOptions.some( + // The `item.className` can be undefined. We shouldn't count it as a duplicate. + item => item.className && item.className == optionObj.className + ); + + if ( classNameAlreadyExists ) { + // TODO: Fill in api docs. + // eslint-disable-next-line ckeditor5-rules/ckeditor-error-message + throw new CKEditorError( 'alignment-config-classname-already-defined' ); + } + + return optionObj; + } ); +} diff --git a/packages/ckeditor5-alignment/tests/alignmentediting.js b/packages/ckeditor5-alignment/tests/alignmentediting.js index 432e8347e4c..f5f99ae809e 100644 --- a/packages/ckeditor5-alignment/tests/alignmentediting.js +++ b/packages/ckeditor5-alignment/tests/alignmentediting.js @@ -17,15 +17,14 @@ import { CKEditorError } from '../../../src/utils'; describe( 'AlignmentEditing', () => { let editor, model; - beforeEach( () => { - return VirtualTestEditor + beforeEach( async () => { + const newEditor = await VirtualTestEditor .create( { plugins: [ AlignmentEditing, Paragraph ] - } ) - .then( newEditor => { - editor = newEditor; - model = editor.model; } ); + + editor = newEditor; + model = editor.model; } ); afterEach( () => { @@ -51,15 +50,14 @@ describe( 'AlignmentEditing', () => { } ); describe( 'integration', () => { - beforeEach( () => { - return VirtualTestEditor + beforeEach( async () => { + const newEditor = await VirtualTestEditor .create( { plugins: [ AlignmentEditing, ImageCaptionEditing, Paragraph, ListEditing, HeadingEditing ] - } ) - .then( newEditor => { - editor = newEditor; - model = editor.model; } ); + + editor = newEditor; + model = editor.model; } ); it( 'is allowed on paragraph', () => { @@ -100,70 +98,69 @@ describe( 'AlignmentEditing', () => { } ); describe( 'RTL content', () => { - it( 'adds converters to the data pipeline', () => { - return VirtualTestEditor + it( 'adds converters to the data pipeline', async () => { + const newEditor = await VirtualTestEditor .create( { language: { content: 'ar' }, plugins: [ AlignmentEditing, Paragraph ] - } ) - .then( newEditor => { - const model = newEditor.model; - const data = '

x

'; + } ); + const model = newEditor.model; + const data = '

x

'; - newEditor.setData( data ); + newEditor.setData( data ); - expect( getModelData( model ) ).to.equal( '[]x' ); - expect( newEditor.getData() ).to.equal( '

x

' ); + expect( getModelData( model ) ).to.equal( '[]x' ); + expect( newEditor.getData() ).to.equal( '

x

' ); - return newEditor.destroy(); - } ); + return newEditor.destroy(); } ); - it( 'adds a converter to the view pipeline', () => { - return VirtualTestEditor + it( 'adds a converter to the view pipeline', async () => { + const newEditor = await VirtualTestEditor .create( { language: { content: 'ar' }, plugins: [ AlignmentEditing, Paragraph ] - } ) - .then( newEditor => { - const model = newEditor.model; + } ); + const model = newEditor.model; - setModelData( model, '[]x' ); - expect( newEditor.getData() ).to.equal( '

x

' ); + setModelData( model, '[]x' ); + expect( newEditor.getData() ).to.equal( '

x

' ); - return newEditor.destroy(); - } ); + return newEditor.destroy(); } ); it( 'uses class when alignment.classNames is set', async () => { - return VirtualTestEditor + const newEditor = await VirtualTestEditor .create( { language: { content: 'ar' }, plugins: [ AlignmentEditing, Paragraph ], alignment: { - classNames: [ 'foo-left', 'foo-right', 'foo-center', 'foo-justify' ] + options: [ + { name: 'left', className: 'foo-left' }, + { name: 'right', className: 'foo-right' }, + { name: 'center', className: 'foo-center' }, + { name: 'justify', className: 'foo-justify' } + ] } - } ) - .then( newEditor => { - const model = newEditor.model; + } ); + const model = newEditor.model; - setModelData( model, '[]x' ); + setModelData( model, '[]x' ); - expect( newEditor.getData() ).to.equal( '

x

' ); + expect( newEditor.getData() ).to.equal( '

x

' ); - newEditor.execute( 'alignment', { value: 'left' } ); + newEditor.execute( 'alignment', { value: 'left' } ); - expect( getModelData( model ) ).to.equal( '[]x' ); - expect( newEditor.getData() ).to.equal( '

x

' ); + expect( getModelData( model ) ).to.equal( '[]x' ); + expect( newEditor.getData() ).to.equal( '

x

' ); - return newEditor.destroy(); - } ); + return newEditor.destroy(); } ); } ); @@ -178,27 +175,30 @@ describe( 'AlignmentEditing', () => { } ); it( 'uses class when alignment.classNames is set', async () => { - return VirtualTestEditor + const newEditor = await VirtualTestEditor .create( { plugins: [ AlignmentEditing, Paragraph ], alignment: { - classNames: [ 'foo-left', 'foo-right', 'foo-center', 'foo-justify' ] + options: [ + { name: 'left', className: 'foo-left' }, + { name: 'right', className: 'foo-right' }, + { name: 'center', className: 'foo-center' }, + { name: 'justify', className: 'foo-justify' } + ] } - } ) - .then( newEditor => { - const model = newEditor.model; + } ); + const model = newEditor.model; - setModelData( model, '[]x' ); + setModelData( model, '[]x' ); - expect( newEditor.getData() ).to.equal( '

x

' ); + expect( newEditor.getData() ).to.equal( '

x

' ); - newEditor.execute( 'alignment', { value: 'left' } ); + newEditor.execute( 'alignment', { value: 'left' } ); - expect( getModelData( model ) ).to.equal( '[]x' ); - expect( newEditor.getData() ).to.equal( '

x

' ); + expect( getModelData( model ) ).to.equal( '[]x' ); + expect( newEditor.getData() ).to.equal( '

x

' ); - return newEditor.destroy(); - } ); + return newEditor.destroy(); } ); } ); @@ -229,30 +229,33 @@ describe( 'AlignmentEditing', () => { } ); it( 'uses class when alignment.classNames is set', async () => { - return VirtualTestEditor + const newEditor = await VirtualTestEditor .create( { language: { content: 'ar' }, plugins: [ AlignmentEditing, Paragraph ], alignment: { - classNames: [ 'foo-left', 'foo-right', 'foo-center', 'foo-justify' ] + options: [ + { name: 'left', className: 'foo-left' }, + { name: 'right', className: 'foo-right' }, + { name: 'center', className: 'foo-center' }, + { name: 'justify', className: 'foo-justify' } + ] } - } ) - .then( newEditor => { - const model = newEditor.model; + } ); + const model = newEditor.model; - setModelData( model, '[]x' ); + setModelData( model, '[]x' ); - expect( newEditor.getData() ).to.equal( '

x

' ); + expect( newEditor.getData() ).to.equal( '

x

' ); - newEditor.execute( 'alignment', { value: 'center' } ); + newEditor.execute( 'alignment', { value: 'center' } ); - expect( getModelData( model ) ).to.equal( '[]x' ); - expect( newEditor.getData() ).to.equal( '

x

' ); + expect( getModelData( model ) ).to.equal( '[]x' ); + expect( newEditor.getData() ).to.equal( '

x

' ); - return newEditor.destroy(); - } ); + return newEditor.destroy(); } ); } ); @@ -275,68 +278,67 @@ describe( 'AlignmentEditing', () => { } ); describe( 'RTL content', () => { - it( 'adds converters to the data pipeline', () => { - return VirtualTestEditor + it( 'adds converters to the data pipeline', async () => { + const newEditor = await VirtualTestEditor .create( { language: { content: 'ar' }, plugins: [ AlignmentEditing, Paragraph ] - } ) - .then( newEditor => { - const model = newEditor.model; - const data = '

x

'; + } ); + const model = newEditor.model; + const data = '

x

'; - newEditor.setData( data ); + newEditor.setData( data ); - expect( getModelData( model ) ).to.equal( '[]x' ); - expect( newEditor.getData() ).to.equal( '

x

' ); + expect( getModelData( model ) ).to.equal( '[]x' ); + expect( newEditor.getData() ).to.equal( '

x

' ); - return newEditor.destroy(); - } ); + return newEditor.destroy(); } ); - it( 'adds a converter to the view pipeline', () => { - return VirtualTestEditor + it( 'adds a converter to the view pipeline', async () => { + const newEditor = await VirtualTestEditor .create( { language: { content: 'ar' }, plugins: [ AlignmentEditing, Paragraph ] - } ) - .then( newEditor => { - const model = newEditor.model; + } ); + const model = newEditor.model; - setModelData( model, '[]x' ); - expect( newEditor.getData() ).to.equal( '

x

' ); + setModelData( model, '[]x' ); + expect( newEditor.getData() ).to.equal( '

x

' ); - return newEditor.destroy(); - } ); + return newEditor.destroy(); } ); } ); it( 'uses class when alignment.classNames is set', async () => { - return VirtualTestEditor + const newEditor = await VirtualTestEditor .create( { plugins: [ AlignmentEditing, Paragraph ], alignment: { - classNames: [ 'foo-left', 'foo-right', 'foo-center', 'foo-justify' ] + options: [ + { name: 'left', className: 'foo-left' }, + { name: 'right', className: 'foo-right' }, + { name: 'center', className: 'foo-center' }, + { name: 'justify', className: 'foo-justify' } + ] } - } ) - .then( newEditor => { - const model = newEditor.model; + } ); + const model = newEditor.model; - setModelData( model, '[]x' ); + setModelData( model, '[]x' ); - expect( newEditor.getData() ).to.equal( '

x

' ); + expect( newEditor.getData() ).to.equal( '

x

' ); - newEditor.execute( 'alignment', { value: 'right' } ); + newEditor.execute( 'alignment', { value: 'right' } ); - expect( getModelData( model ) ).to.equal( '[]x' ); - expect( newEditor.getData() ).to.equal( '

x

' ); + expect( getModelData( model ) ).to.equal( '[]x' ); + expect( newEditor.getData() ).to.equal( '

x

' ); - return newEditor.destroy(); - } ); + return newEditor.destroy(); } ); } ); @@ -357,27 +359,30 @@ describe( 'AlignmentEditing', () => { } ); it( 'uses class when alignment.classNames is set', async () => { - return VirtualTestEditor + const newEditor = await VirtualTestEditor .create( { plugins: [ AlignmentEditing, Paragraph ], alignment: { - classNames: [ 'foo-left', 'foo-right', 'foo-center', 'foo-justify' ] + options: [ + { name: 'left', className: 'foo-left' }, + { name: 'right', className: 'foo-right' }, + { name: 'center', className: 'foo-center' }, + { name: 'justify', className: 'foo-justify' } + ] } - } ) - .then( newEditor => { - const model = newEditor.model; + } ); + const model = newEditor.model; - setModelData( model, '[]x' ); + setModelData( model, '[]x' ); - expect( newEditor.getData() ).to.equal( '

x

' ); + expect( newEditor.getData() ).to.equal( '

x

' ); - newEditor.execute( 'alignment', { value: 'justify' } ); + newEditor.execute( 'alignment', { value: 'justify' } ); - expect( getModelData( model ) ).to.equal( '[]x' ); - expect( newEditor.getData() ).to.equal( '

x

' ); + expect( getModelData( model ) ).to.equal( '[]x' ); + expect( newEditor.getData() ).to.equal( '

x

' ); - return newEditor.destroy(); - } ); + return newEditor.destroy(); } ); } ); @@ -424,7 +429,14 @@ describe( 'AlignmentEditing', () => { describe( 'options', () => { describe( 'default value', () => { it( 'should be set', () => { - expect( editor.config.get( 'alignment.options' ) ).to.deep.equal( [ 'left', 'right', 'center', 'justify' ] ); + expect( editor.config.get( 'alignment.options' ) ).to.deep.equal( + [ + { name: 'left' }, + { name: 'right' }, + { name: 'center' }, + { name: 'justify' } + ] + ); } ); } ); } ); @@ -436,92 +448,95 @@ describe( 'AlignmentEditing', () => { } ); } ); - it( 'should throw when there are too many classes', async () => { - try { - await VirtualTestEditor - .create( { - plugins: [ AlignmentEditing, Paragraph ], - alignment: { - classNames: [ 'foo-left', 'foo-right', 'foo-center', 'foo-justify', 'foo-extra' ] - } - } ); - } catch ( error ) { - expect( error.constructor ).to.equal( CKEditorError ); - expect( error ).to.match( /alignment-config-classnames-not-matching/ ); - } - } ); + it( 'should throw when options are repeated - repeated name', async () => { + let error; - it( 'should throw when there are fewer classes than alignment options', async () => { try { await VirtualTestEditor .create( { plugins: [ AlignmentEditing, Paragraph ], alignment: { - classNames: [ 'foo-left', 'foo-right', 'foo-center' ] + options: [ + { name: 'left', className: 'foo-left' }, + 'left' + ] } } ); - } catch ( error ) { - expect( error.constructor ).to.equal( CKEditorError ); - expect( error ).to.match( /alignment-config-classnames-not-matching/ ); + } catch ( err ) { + error = err; } + + expect( error.constructor ).to.equal( CKEditorError ); + expect( error ).to.match( /alignment-config-name-already-defined/ ); } ); - it( 'should throw when classNames are not an array', async () => { + it( 'should throw when options are repeated - repeated className', async () => { + let error; + try { await VirtualTestEditor .create( { plugins: [ AlignmentEditing, Paragraph ], alignment: { - classNames: {} + options: [ + 'left', + { name: 'right', className: 'foo-right' }, + { name: 'center', className: 'foo-right' } + ] } } ); - } catch ( error ) { - expect( error.constructor ).to.equal( CKEditorError ); - expect( error ).to.match( /alignment-config-classnames-not-matching/ ); + } catch ( err ) { + error = err; } + + expect( error.constructor ).to.equal( CKEditorError ); + expect( error ).to.match( /alignment-config-classname-already-defined/ ); } ); - it( 'should map limited options to limited set of classes', () => { - return VirtualTestEditor + it( 'should map limited options to limited set of classes', async () => { + const newEditor = await VirtualTestEditor .create( { plugins: [ AlignmentEditing, Paragraph ], alignment: { - options: [ 'left', 'center' ], - classNames: [ 'foo-left', 'foo-center' ] + options: [ + { name: 'left', className: 'foo-left' }, + { name: 'center', className: 'foo-center' } + ] } - } ) - .then( newEditor => { - const model = newEditor.model; - const data = '

x

'; + } ); + const model = newEditor.model; + const data = '

x

'; - newEditor.setData( data ); + newEditor.setData( data ); - expect( getModelData( model ) ).to.equal( '[]x' ); - expect( newEditor.getData() ).to.equal( '

x

' ); + expect( getModelData( model ) ).to.equal( '[]x' ); + expect( newEditor.getData() ).to.equal( '

x

' ); - return newEditor.destroy(); - } ); + return newEditor.destroy(); } ); - it( 'should map classes to default options', () => { - return VirtualTestEditor + it( 'should map classes to default options', async () => { + const newEditor = await VirtualTestEditor .create( { plugins: [ AlignmentEditing, Paragraph ], alignment: { - classNames: [ 'foo-left', 'foo-right', 'foo-center', 'foo-justify' ] + options: [ + { name: 'left', className: 'foo-left' }, + { name: 'right', className: 'foo-right' }, + { name: 'center', className: 'foo-center' }, + { name: 'justify', className: 'foo-justify' } + ] } - } ) - .then( newEditor => { - const model = newEditor.model; - const data = '

x

'; + } ); + const model = newEditor.model; + const data = '

x

'; - newEditor.setData( data ); + newEditor.setData( data ); - expect( getModelData( model ) ).to.equal( '[]x' ); - expect( newEditor.getData() ).to.equal( '

x

' ); + expect( getModelData( model ) ).to.equal( '[]x' ); + expect( newEditor.getData() ).to.equal( '

x

' ); - return newEditor.destroy(); - } ); + return newEditor.destroy(); } ); } ); } ); From 1ff1d301c3d4adfc2a2518617c5f40891a31624d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maksymilian=20Barna=C5=9B?= Date: Wed, 17 Feb 2021 22:53:17 +0100 Subject: [PATCH 11/26] Review fixes. --- .../src/alignmentediting.js | 23 ++++++++----------- .../tests/alignmentediting.js | 8 ------- 2 files changed, 10 insertions(+), 21 deletions(-) diff --git a/packages/ckeditor5-alignment/src/alignmentediting.js b/packages/ckeditor5-alignment/src/alignmentediting.js index 88d7e50348e..46e66d20ed7 100644 --- a/packages/ckeditor5-alignment/src/alignmentediting.js +++ b/packages/ckeditor5-alignment/src/alignmentediting.js @@ -120,20 +120,17 @@ function buildUpcastInlineDefinitions( options ) { const definitions = []; for ( const option of options ) { - const view = { - key: 'style', - value: { - 'text-align': option - } - }; - const model = { - key: 'alignment', - value: option - }; - definitions.push( { - view, - model + view: { + key: 'style', + value: { + 'text-align': option + } + }, + model: { + key: 'alignment', + value: option + } } ); } diff --git a/packages/ckeditor5-alignment/tests/alignmentediting.js b/packages/ckeditor5-alignment/tests/alignmentediting.js index f5f99ae809e..b082c477543 100644 --- a/packages/ckeditor5-alignment/tests/alignmentediting.js +++ b/packages/ckeditor5-alignment/tests/alignmentediting.js @@ -153,8 +153,6 @@ describe( 'AlignmentEditing', () => { setModelData( model, '[]x' ); - expect( newEditor.getData() ).to.equal( '

x

' ); - newEditor.execute( 'alignment', { value: 'left' } ); expect( getModelData( model ) ).to.equal( '[]x' ); @@ -248,8 +246,6 @@ describe( 'AlignmentEditing', () => { setModelData( model, '[]x' ); - expect( newEditor.getData() ).to.equal( '

x

' ); - newEditor.execute( 'alignment', { value: 'center' } ); expect( getModelData( model ) ).to.equal( '[]x' ); @@ -331,8 +327,6 @@ describe( 'AlignmentEditing', () => { setModelData( model, '[]x' ); - expect( newEditor.getData() ).to.equal( '

x

' ); - newEditor.execute( 'alignment', { value: 'right' } ); expect( getModelData( model ) ).to.equal( '[]x' ); @@ -375,8 +369,6 @@ describe( 'AlignmentEditing', () => { setModelData( model, '[]x' ); - expect( newEditor.getData() ).to.equal( '

x

' ); - newEditor.execute( 'alignment', { value: 'justify' } ); expect( getModelData( model ) ).to.equal( '[]x' ); From 38e0096870680b144d487b743152501e2ec2c745 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maksymilian=20Barna=C5=9B?= Date: Thu, 18 Feb 2021 08:56:31 +0100 Subject: [PATCH 12/26] Update failing tests. --- .../ckeditor5-alignment/src/alignmentui.js | 7 +- packages/ckeditor5-alignment/src/utils.js | 65 +++++++++++-------- 2 files changed, 41 insertions(+), 31 deletions(-) diff --git a/packages/ckeditor5-alignment/src/alignmentui.js b/packages/ckeditor5-alignment/src/alignmentui.js index 483345fd68e..0b43b4d2663 100644 --- a/packages/ckeditor5-alignment/src/alignmentui.js +++ b/packages/ckeditor5-alignment/src/alignmentui.js @@ -10,7 +10,7 @@ import { Plugin, icons } from 'ckeditor5/src/core'; import { ButtonView, createDropdown, addToolbarToDropdown } from 'ckeditor5/src/ui'; -import { isSupported } from './utils'; +import { isSupported, normalizeAlignmentOptions } from './utils'; const iconsMap = new Map( [ [ 'left', icons.alignLeft ], @@ -67,9 +67,10 @@ export default class AlignmentUI extends Plugin { const editor = this.editor; const componentFactory = editor.ui.componentFactory; const t = editor.t; - const options = editor.config.get( 'alignment.options' ); + const options = normalizeAlignmentOptions( editor.config.get( 'alignment.options' ) ); options + .map( option => option.name ) .filter( isSupported ) .forEach( option => this._addButton( option ) ); @@ -77,7 +78,7 @@ export default class AlignmentUI extends Plugin { const dropdownView = createDropdown( locale ); // Add existing alignment buttons to dropdown's toolbar. - const buttons = options.map( option => componentFactory.create( `alignment:${ option }` ) ); + const buttons = options.map( option => componentFactory.create( `alignment:${ option.name }` ) ); addToolbarToDropdown( dropdownView, buttons ); // Configure dropdown properties an behavior. diff --git a/packages/ckeditor5-alignment/src/utils.js b/packages/ckeditor5-alignment/src/utils.js index 0f81d9f3c01..ba8de5cf5b2 100644 --- a/packages/ckeditor5-alignment/src/utils.js +++ b/packages/ckeditor5-alignment/src/utils.js @@ -52,40 +52,49 @@ export function isDefault( alignment, locale ) { } } +/** + * Brings the configuration to the common form, an array of objects. + * + * @param {Array.} configuredOptions Alignment plugin configuration. + * @returns {Array.} Normalized object holding the configuration. + */ export function normalizeAlignmentOptions( configuredOptions = [] ) { - return configuredOptions.map( ( option, index, options ) => { - let optionObj; + return configuredOptions.map( normalizeOption ); +} - if ( typeof option == 'string' ) { - optionObj = Object.assign( {}, { name: option } ); - } else { - optionObj = Object.assign( {}, defaultOptions[ index ], option ); - } +// @private +function normalizeOption( option, index, allOptions ) { + let optionObj; - const succeedingOptions = options.slice( index + 1 ); - const nameAlreadyExists = succeedingOptions.some( item => { - const optionName = item.name || item; + if ( typeof option == 'string' ) { + optionObj = Object.assign( {}, { name: option } ); + } else { + optionObj = Object.assign( {}, defaultOptions[ index ], option ); + } - return optionName == optionObj.name; - } ); + const succeedingOptions = allOptions.slice( index + 1 ); + const nameAlreadyExists = succeedingOptions.some( item => { + const optionName = item.name || item; - if ( nameAlreadyExists ) { - // TODO: Fill in api docs. - // eslint-disable-next-line ckeditor5-rules/ckeditor-error-message - throw new CKEditorError( 'alignment-config-name-already-defined' ); - } + return optionName == optionObj.name; + } ); - const classNameAlreadyExists = optionObj.className && succeedingOptions.some( - // The `item.className` can be undefined. We shouldn't count it as a duplicate. - item => item.className && item.className == optionObj.className - ); + if ( nameAlreadyExists ) { + // TODO: Fill in api docs. + // eslint-disable-next-line ckeditor5-rules/ckeditor-error-message + throw new CKEditorError( 'alignment-config-name-already-defined' ); + } - if ( classNameAlreadyExists ) { - // TODO: Fill in api docs. - // eslint-disable-next-line ckeditor5-rules/ckeditor-error-message - throw new CKEditorError( 'alignment-config-classname-already-defined' ); - } + const classNameAlreadyExists = optionObj.className && succeedingOptions.some( + // The `item.className` can be undefined. We shouldn't count it as a duplicate. + item => item.className && item.className == optionObj.className + ); - return optionObj; - } ); + if ( classNameAlreadyExists ) { + // TODO: Fill in api docs. + // eslint-disable-next-line ckeditor5-rules/ckeditor-error-message + throw new CKEditorError( 'alignment-config-classname-already-defined' ); + } + + return optionObj; } From 77d95ae815fa17bfb62ef03bd775d5293293a550 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maksymilian=20Barna=C5=9B?= Date: Thu, 18 Feb 2021 09:52:05 +0100 Subject: [PATCH 13/26] Add tests for normalizeAlignmentOptions. --- packages/ckeditor5-alignment/tests/utils.js | 86 ++++++++++++++++++++- 1 file changed, 85 insertions(+), 1 deletion(-) diff --git a/packages/ckeditor5-alignment/tests/utils.js b/packages/ckeditor5-alignment/tests/utils.js index d02436d912d..8fe15b963d8 100644 --- a/packages/ckeditor5-alignment/tests/utils.js +++ b/packages/ckeditor5-alignment/tests/utils.js @@ -3,7 +3,8 @@ * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license */ -import { isDefault, isSupported, supportedOptions } from '../src/utils'; +import { CKEditorError } from '../../../src/utils'; +import { isDefault, isSupported, supportedOptions, normalizeAlignmentOptions } from '../src/utils'; describe( 'utils', () => { describe( 'isDefault()', () => { @@ -47,4 +48,87 @@ describe( 'utils', () => { expect( supportedOptions ).to.deep.equal( [ 'left', 'right', 'center', 'justify' ] ); } ); } ); + + describe( 'normalizeAlignmentOptions', () => { + it( 'does nothing when no parameters are provided', () => { + expect( () => normalizeAlignmentOptions() ).not.to.throw(); + } ); + + it( 'throws when the name already exists', () => { + const config = [ + 'center', + { + name: 'center' + } + ]; + let error; + + try { + normalizeAlignmentOptions( config ); + } catch ( err ) { + error = err; + } + + expect( error.constructor ).to.equal( CKEditorError ); + expect( error ).to.match( /alignment-config-name-already-defined/ ); + } ); + + it( 'throws when the className already exists', () => { + const config = [ + 'left', + { + name: 'center', + className: 'foo-center' + }, + { + name: 'justify', + className: 'foo-center' + } + ]; + let error; + + try { + normalizeAlignmentOptions( config ); + } catch ( err ) { + error = err; + } + + expect( error.constructor ).to.equal( CKEditorError ); + expect( error ).to.match( /alignment-config-classname-already-defined/ ); + } ); + + it( 'normalizes mixed input into an config array of objects', () => { + const config = [ + 'left', + { + name: 'right' + }, + 'center', + { + name: 'justify', + className: 'foo-center' + } + ]; + + const result = normalizeAlignmentOptions( config ); + + expect( result ).to.deep.equal( + [ + { + 'name': 'left' + }, + { + 'name': 'right' + }, + { + 'name': 'center' + }, + { + 'className': 'foo-center', + 'name': 'justify' + } + ] + ); + } ); + } ); } ); From 8fe8dad79dd8baa3f3f3fb2bc21ad304f2cff487 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maksymilian=20Barna=C5=9B?= Date: Fri, 19 Feb 2021 09:21:18 +0100 Subject: [PATCH 14/26] Update docs and tests. --- packages/ckeditor5-alignment/src/utils.js | 35 +++++++--- packages/ckeditor5-alignment/tests/utils.js | 76 +++++++++++---------- 2 files changed, 65 insertions(+), 46 deletions(-) diff --git a/packages/ckeditor5-alignment/src/utils.js b/packages/ckeditor5-alignment/src/utils.js index ba8de5cf5b2..3adeee7bf86 100644 --- a/packages/ckeditor5-alignment/src/utils.js +++ b/packages/ckeditor5-alignment/src/utils.js @@ -73,27 +73,40 @@ function normalizeOption( option, index, allOptions ) { } const succeedingOptions = allOptions.slice( index + 1 ); - const nameAlreadyExists = succeedingOptions.some( item => { - const optionName = item.name || item; + const nameAlreadyExists = succeedingOptions.some( + item => { + const optionName = item.name || item; - return optionName == optionObj.name; - } ); + return optionName == optionObj.name; + } ); if ( nameAlreadyExists ) { - // TODO: Fill in api docs. - // eslint-disable-next-line ckeditor5-rules/ckeditor-error-message - throw new CKEditorError( 'alignment-config-name-already-defined' ); + /** + * The same `name` in one of the `alignment.options` was already declared. + * Each `name` representing one alignment option can be set exactly once. + * + * @error alignment-config-name-already-defined + * @param {Object} option First option that declares given `name`. + * @param {Array.} allOptions Contents of `alignment.options`. + */ + throw new CKEditorError( 'alignment-config-name-already-defined', { option, allOptions } ); } const classNameAlreadyExists = optionObj.className && succeedingOptions.some( // The `item.className` can be undefined. We shouldn't count it as a duplicate. - item => item.className && item.className == optionObj.className + item => item.className && + item.className == optionObj.className ); if ( classNameAlreadyExists ) { - // TODO: Fill in api docs. - // eslint-disable-next-line ckeditor5-rules/ckeditor-error-message - throw new CKEditorError( 'alignment-config-classname-already-defined' ); + /** + * The same `className` in one of the `alignment.options` was already declared. + * + * @error alignment-config-classname-already-defined + * @param {Object} option First option that declares given `className`. + * @param {Array.} allOptions Contents of `alignment.options`. + */ + throw new CKEditorError( 'alignment-config-classname-already-defined', { option, allOptions } ); } return optionObj; diff --git a/packages/ckeditor5-alignment/tests/utils.js b/packages/ckeditor5-alignment/tests/utils.js index 8fe15b963d8..ecba0fb3ce8 100644 --- a/packages/ckeditor5-alignment/tests/utils.js +++ b/packages/ckeditor5-alignment/tests/utils.js @@ -51,7 +51,47 @@ describe( 'utils', () => { describe( 'normalizeAlignmentOptions', () => { it( 'does nothing when no parameters are provided', () => { - expect( () => normalizeAlignmentOptions() ).not.to.throw(); + let result; + + expect( () => { + result = normalizeAlignmentOptions(); + } ).not.to.throw(); + + expect( result ).to.deep.equal( [ ] ); + } ); + + it( 'normalizes mixed input into an config array of objects', () => { + const config = [ + 'left', + { + name: 'right' + }, + 'center', + { + name: 'justify', + className: 'foo-center' + } + ]; + + const result = normalizeAlignmentOptions( config ); + + expect( result ).to.deep.equal( + [ + { + 'name': 'left' + }, + { + 'name': 'right' + }, + { + 'name': 'center' + }, + { + 'className': 'foo-center', + 'name': 'justify' + } + ] + ); } ); it( 'throws when the name already exists', () => { @@ -96,39 +136,5 @@ describe( 'utils', () => { expect( error.constructor ).to.equal( CKEditorError ); expect( error ).to.match( /alignment-config-classname-already-defined/ ); } ); - - it( 'normalizes mixed input into an config array of objects', () => { - const config = [ - 'left', - { - name: 'right' - }, - 'center', - { - name: 'justify', - className: 'foo-center' - } - ]; - - const result = normalizeAlignmentOptions( config ); - - expect( result ).to.deep.equal( - [ - { - 'name': 'left' - }, - { - 'name': 'right' - }, - { - 'name': 'center' - }, - { - 'className': 'foo-center', - 'name': 'justify' - } - ] - ); - } ); } ); } ); From 5685f43bcaf9f9b876fef3869ea1aa91f4bef149 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maksymilian=20Barna=C5=9B?= Date: Fri, 19 Feb 2021 11:22:46 +0100 Subject: [PATCH 15/26] Update API docs. --- packages/ckeditor5-alignment/src/alignment.js | 29 +++++++++---------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/packages/ckeditor5-alignment/src/alignment.js b/packages/ckeditor5-alignment/src/alignment.js index 7f68a156814..6a8f0dc18ef 100644 --- a/packages/ckeditor5-alignment/src/alignment.js +++ b/packages/ckeditor5-alignment/src/alignment.js @@ -65,36 +65,35 @@ export default class Alignment extends Plugin { */ /** - * Class names to match {@link module:alignment/alignment~AlignmentConfig#options alignment options}. + * Available alignment options. * - * **Note:** The same number of classes should be provided as the number of `config.alignment.options`. + * The available options are: `'left'`, `'right'`, `'center'` and `'justify'`. Other values are ignored. + * + * **Note:** It is recommended to always use `'left'` or `'right'` as these are default values which the user should + * normally be able to choose depending on the + * {@glink features/ui-language#setting-the-language-of-the-content language of the editor content}. * * ClassicEditor * .create( editorElement, { * alignment: { - * options: [ 'left', 'right' ], - * classNames: [ 'my-align-left', 'my-align-right' ] + * options: [ 'left', 'right' ] * } * } ) * .then( ... ) * .catch( ... ); * - * @member {Array.} module:alignment/alignment~AlignmentConfig#classNames - */ - -/** - * Available alignment options. - * - * The available options are: `'left'`, `'right'`, `'center'` and `'justify'`. Other values are ignored. + * By default the alignment is set inline using `text-align` CSS property. To further customize the alignment you can + * provide names of classes for each alignment option using `className` property. * - * **Note:** It is recommended to always use `'left'` or `'right'` as these are default values which the user should - * normally be able to choose depending on the - * {@glink features/ui-language#setting-the-language-of-the-content language of the editor content}. + * **Note:** Once you define `className` property for one option, you need to specify it for all other options. * * ClassicEditor * .create( editorElement, { * alignment: { - * options: [ 'left', 'right' ] + * options: [ + * { name: 'left', className: 'foo-align-left' }, + * { name: 'right', className: 'foo-align-right' } + * ] * } * } ) * .then( ... ) From 442cca87a880cea3e473bb53a3209522044c5c4e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maksymilian=20Barna=C5=9B?= Date: Sun, 21 Feb 2021 22:01:54 +0100 Subject: [PATCH 16/26] Review fixes. --- .../docs/features/text-alignment.md | 2 +- .../src/alignmentediting.js | 67 ++++++-------- packages/ckeditor5-alignment/src/utils.js | 91 +++++++++---------- .../tests/alignmentediting.js | 6 -- 4 files changed, 76 insertions(+), 90 deletions(-) diff --git a/packages/ckeditor5-alignment/docs/features/text-alignment.md b/packages/ckeditor5-alignment/docs/features/text-alignment.md index b5f2eaa6559..f24b2a26fe1 100644 --- a/packages/ckeditor5-alignment/docs/features/text-alignment.md +++ b/packages/ckeditor5-alignment/docs/features/text-alignment.md @@ -50,7 +50,7 @@ ClassicEditor ### Using classes instead of inline style -By default alignment is set inline using `text-align` CSS property. If you wish the feature to output more semantic content that uses classes instead of inline styles, you can specify class names by using the {@link module:alignment/alignment~AlignmentConfig#classNames `config.alignment.classNames`} option and style them by using a stylesheet. +By default alignment is set inline using `text-align` CSS property. If you wish the feature to output more semantic content that uses classes instead of inline styles, you can specify class names by using the `config.alignment.classNames` option and style them by using a stylesheet. Make sure to specify the same number of classes as the number of values in {@link module:alignment/alignment~AlignmentConfig#options `config.alignment.options`}. If the {@link module:alignment/alignment~AlignmentConfig#options `config.alignment.options`} are not specified, they are equal to `[ 'left', 'right', 'center', 'justify' ]`, therefore you have to provide 4 class names. diff --git a/packages/ckeditor5-alignment/src/alignmentediting.js b/packages/ckeditor5-alignment/src/alignmentediting.js index 46e66d20ed7..5218cd179f4 100644 --- a/packages/ckeditor5-alignment/src/alignmentediting.js +++ b/packages/ckeditor5-alignment/src/alignmentediting.js @@ -10,7 +10,7 @@ import { Plugin } from 'ckeditor5/src/core'; import AlignmentCommand from './alignmentcommand'; -import { defaultOptions, isDefault, isSupported, normalizeAlignmentOptions } from './utils'; +import { defaultConfig, isDefault, isSupported, normalizeAlignmentOptions } from './utils'; /** * The alignment editing feature. It introduces the {@link module:alignment/alignmentcommand~AlignmentCommand command} and adds @@ -32,8 +32,7 @@ export default class AlignmentEditing extends Plugin { super( editor ); editor.config.define( 'alignment', { - options: [ ...defaultOptions ], - classNames: [] + options: [ ...defaultConfig ] } ); } @@ -45,42 +44,36 @@ export default class AlignmentEditing extends Plugin { const locale = editor.locale; const schema = editor.model.schema; - const alignmentOptions = normalizeAlignmentOptions( editor.config.get( 'alignment.options' ) ); + const options = normalizeAlignmentOptions( editor.config.get( 'alignment.options' ) ); - // Filter out unsupported options. - const enabledOptions = alignmentOptions.map( option => option.name ).filter( isSupported ); - const classNameConfig = alignmentOptions.map( option => option.className ); + // Filter out unsupported options and those that are redundant, e.g. `left` in LTR / `right` in RTL mode. + const optionsToConvert = options.filter( + option => isSupported( option.name ) && !isDefault( option.name, locale ) + ); + + // Converters for inline alignment need only alignment name. + const optionNamesToConvert = optionsToConvert.map( option => option.name ); + + // Once there is at least one `className` defined, we switch to alignment with classes. + const shouldUseClasses = optionsToConvert.some( option => !!option.className ); // Allow alignment attribute on all blocks. schema.extend( '$block', { allowAttributes: 'alignment' } ); editor.model.schema.setAttributeProperties( 'alignment', { isFormatting: true } ); - const shouldUseClasses = classNameConfig.filter( className => !!className ).length; - - // There is no need for converting alignment that's the same as current text direction. - const options = enabledOptions.filter( option => !isDefault( option, locale ) ); - if ( shouldUseClasses ) { - // Upcast and Downcast classes. - - const alignmentClassNames = classNameConfig.reduce( ( classNameMap, className, index ) => { - classNameMap[ enabledOptions[ index ] ] = className; - - return classNameMap; - }, {} ); - - const definition = buildClassDefinition( options, alignmentClassNames ); + // Downcast to only to classes. + const definition = buildClassDefinition( optionsToConvert ); editor.conversion.attributeToAttribute( definition ); } else { // Downcast inline styles. - - const definition = buildDowncastInlineDefinition( options ); + const definition = buildDowncastInlineDefinition( optionNamesToConvert ); editor.conversion.for( 'downcast' ).attributeToAttribute( definition ); } - const upcastInlineDefinitions = buildUpcastInlineDefinitions( options ); + const upcastInlineDefinitions = buildUpcastInlineDefinitions( optionNamesToConvert ); // Always upcast from inline styles. for ( const definition of upcastInlineDefinitions ) { @@ -93,20 +86,20 @@ export default class AlignmentEditing extends Plugin { // Prepare downcast conversion definition for inline alignment styling. // @private -function buildDowncastInlineDefinition( options ) { +function buildDowncastInlineDefinition( optionNames ) { const definition = { model: { key: 'alignment', - values: options.slice() + values: optionNames.slice() }, view: {} }; - for ( const option of options ) { - definition.view[ option ] = { + for ( const name of optionNames ) { + definition.view[ name ] = { key: 'style', value: { - 'text-align': option + 'text-align': name } }; } @@ -116,20 +109,20 @@ function buildDowncastInlineDefinition( options ) { // Prepare upcast definitions for inline alignment styles. // @private -function buildUpcastInlineDefinitions( options ) { +function buildUpcastInlineDefinitions( optionNames ) { const definitions = []; - for ( const option of options ) { + for ( const name of optionNames ) { definitions.push( { view: { key: 'style', value: { - 'text-align': option + 'text-align': name } }, model: { key: 'alignment', - value: option + value: name } } ); } @@ -139,19 +132,19 @@ function buildUpcastInlineDefinitions( options ) { // Prepare conversion definitions for upcast and downcast alignment with classes. // @private -function buildClassDefinition( options, alignmentClassNames ) { +function buildClassDefinition( options ) { const definition = { model: { key: 'alignment', - values: options.slice() + values: options.map( option => option.name ).slice() }, view: {} }; for ( const option of options ) { - definition.view[ option ] = { + definition.view[ option.name ] = { key: 'class', - value: [ alignmentClassNames[ option ] ] + value: [ option.className ] }; } diff --git a/packages/ckeditor5-alignment/src/utils.js b/packages/ckeditor5-alignment/src/utils.js index 3adeee7bf86..fc06b6eb180 100644 --- a/packages/ckeditor5-alignment/src/utils.js +++ b/packages/ckeditor5-alignment/src/utils.js @@ -18,10 +18,10 @@ import { CKEditorError } from '../../../src/utils'; * * `'justify'` */ export const supportedOptions = [ 'left', 'right', 'center', 'justify' ]; -export const defaultOptions = supportedOptions.map( option => { - return Object.assign( {}, { +export const defaultConfig = supportedOptions.map( option => { + return { name: option - } ); + }; } ); /** @@ -59,55 +59,54 @@ export function isDefault( alignment, locale ) { * @returns {Array.} Normalized object holding the configuration. */ export function normalizeAlignmentOptions( configuredOptions = [] ) { - return configuredOptions.map( normalizeOption ); -} - -// @private -function normalizeOption( option, index, allOptions ) { - let optionObj; + const optionNameToOptionMap = new Map( defaultConfig.map( option => [ option.name, option ] ) ); - if ( typeof option == 'string' ) { - optionObj = Object.assign( {}, { name: option } ); - } else { - optionObj = Object.assign( {}, defaultOptions[ index ], option ); - } + const normalizedOptions = configuredOptions + .map( option => { + let optionObj; - const succeedingOptions = allOptions.slice( index + 1 ); - const nameAlreadyExists = succeedingOptions.some( - item => { - const optionName = item.name || item; + if ( typeof option == 'string' ) { + optionObj = { name: option }; + } else { + optionObj = { ...optionNameToOptionMap[ option ], ...option }; + } - return optionName == optionObj.name; + return optionObj; } ); - if ( nameAlreadyExists ) { - /** - * The same `name` in one of the `alignment.options` was already declared. - * Each `name` representing one alignment option can be set exactly once. - * - * @error alignment-config-name-already-defined - * @param {Object} option First option that declares given `name`. - * @param {Array.} allOptions Contents of `alignment.options`. - */ - throw new CKEditorError( 'alignment-config-name-already-defined', { option, allOptions } ); - } + // Validate resulting config. + normalizedOptions.forEach( ( option, index, allOptions ) => { + const succeedingOptions = allOptions.slice( index + 1 ); + + const nameAlreadyExists = succeedingOptions.some( item => item.name == option.name ); + + if ( nameAlreadyExists ) { + /** + * The same `name` in one of the `alignment.options` was already declared. + * Each `name` representing one alignment option can be set exactly once. + * + * @error alignment-config-name-already-defined + * @param {Object} option First option that declares given `name`. + * @param {Array.} allOptions Contents of `alignment.options`. + */ + throw new CKEditorError( 'alignment-config-name-already-defined', { option, allOptions } ); + } - const classNameAlreadyExists = optionObj.className && succeedingOptions.some( // The `item.className` can be undefined. We shouldn't count it as a duplicate. - item => item.className && - item.className == optionObj.className - ); - - if ( classNameAlreadyExists ) { - /** - * The same `className` in one of the `alignment.options` was already declared. - * - * @error alignment-config-classname-already-defined - * @param {Object} option First option that declares given `className`. - * @param {Array.} allOptions Contents of `alignment.options`. - */ - throw new CKEditorError( 'alignment-config-classname-already-defined', { option, allOptions } ); - } + const classNameAlreadyExists = option.className && + succeedingOptions.some( item => item.className && item.className == option.className ); + + if ( classNameAlreadyExists ) { + /** + * The same `className` in one of the `alignment.options` was already declared. + * + * @error alignment-config-classname-already-defined + * @param {Object} option First option that declares given `className`. + * @param {Array.} allOptions Contents of `alignment.options`. + */ + throw new CKEditorError( 'alignment-config-classname-already-defined', { option, allOptions } ); + } + } ); - return optionObj; + return normalizedOptions; } diff --git a/packages/ckeditor5-alignment/tests/alignmentediting.js b/packages/ckeditor5-alignment/tests/alignmentediting.js index b082c477543..1d7e815cf04 100644 --- a/packages/ckeditor5-alignment/tests/alignmentediting.js +++ b/packages/ckeditor5-alignment/tests/alignmentediting.js @@ -434,12 +434,6 @@ describe( 'AlignmentEditing', () => { } ); describe( 'classNames', () => { - describe( 'default value', () => { - it( 'should be set', () => { - expect( editor.config.get( 'alignment.classNames' ) ).to.deep.equal( [ ] ); - } ); - } ); - it( 'should throw when options are repeated - repeated name', async () => { let error; From 30a3c67428c83d35efb1193c9813cc49fe54f02f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maksymilian=20Barna=C5=9B?= Date: Sun, 21 Feb 2021 22:10:35 +0100 Subject: [PATCH 17/26] Make the duplicate className detection more readable. --- packages/ckeditor5-alignment/src/utils.js | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/packages/ckeditor5-alignment/src/utils.js b/packages/ckeditor5-alignment/src/utils.js index fc06b6eb180..db800e3db90 100644 --- a/packages/ckeditor5-alignment/src/utils.js +++ b/packages/ckeditor5-alignment/src/utils.js @@ -92,11 +92,11 @@ export function normalizeAlignmentOptions( configuredOptions = [] ) { throw new CKEditorError( 'alignment-config-name-already-defined', { option, allOptions } ); } - // The `item.className` can be undefined. We shouldn't count it as a duplicate. - const classNameAlreadyExists = option.className && - succeedingOptions.some( item => item.className && item.className == option.className ); + // The `className` property is present. Check for duplicates then. + if ( option.className ) { + const classNameAlreadyExists = succeedingOptions.some( item => item.className == option.className ); - if ( classNameAlreadyExists ) { + if ( classNameAlreadyExists ) { /** * The same `className` in one of the `alignment.options` was already declared. * @@ -104,7 +104,8 @@ export function normalizeAlignmentOptions( configuredOptions = [] ) { * @param {Object} option First option that declares given `className`. * @param {Array.} allOptions Contents of `alignment.options`. */ - throw new CKEditorError( 'alignment-config-classname-already-defined', { option, allOptions } ); + throw new CKEditorError( 'alignment-config-classname-already-defined', { option, allOptions } ); + } } } ); From f5ccfa855b28c63535b226f3f45d4d10650a908a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maksymilian=20Barna=C5=9B?= Date: Sun, 21 Feb 2021 23:17:08 +0100 Subject: [PATCH 18/26] Remove redundant array. --- packages/ckeditor5-alignment/src/alignmentediting.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/ckeditor5-alignment/src/alignmentediting.js b/packages/ckeditor5-alignment/src/alignmentediting.js index 5218cd179f4..9d6ad39c3a4 100644 --- a/packages/ckeditor5-alignment/src/alignmentediting.js +++ b/packages/ckeditor5-alignment/src/alignmentediting.js @@ -144,7 +144,7 @@ function buildClassDefinition( options ) { for ( const option of options ) { definition.view[ option.name ] = { key: 'class', - value: [ option.className ] + value: option.className }; } From dce3a13aead2753cf56909f511e0cd64d9a3381f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maksymilian=20Barna=C5=9B?= Date: Mon, 22 Feb 2021 09:43:02 +0100 Subject: [PATCH 19/26] Clean up tests. --- .../tests/alignmentediting.js | 140 ++++++++---------- 1 file changed, 58 insertions(+), 82 deletions(-) diff --git a/packages/ckeditor5-alignment/tests/alignmentediting.js b/packages/ckeditor5-alignment/tests/alignmentediting.js index 1d7e815cf04..33b4861250d 100644 --- a/packages/ckeditor5-alignment/tests/alignmentediting.js +++ b/packages/ckeditor5-alignment/tests/alignmentediting.js @@ -133,7 +133,7 @@ describe( 'AlignmentEditing', () => { return newEditor.destroy(); } ); - it( 'uses class when alignment.classNames is set', async () => { + it( 'uses class when classNames is set', async () => { const newEditor = await VirtualTestEditor .create( { language: { @@ -172,7 +172,7 @@ describe( 'AlignmentEditing', () => { expect( editor.getData() ).to.equal( '

x

' ); } ); - it( 'uses class when alignment.classNames is set', async () => { + it( 'uses class when classNames is set', async () => { const newEditor = await VirtualTestEditor .create( { plugins: [ AlignmentEditing, Paragraph ], @@ -226,7 +226,7 @@ describe( 'AlignmentEditing', () => { expect( editor.getData() ).to.equal( '

x

' ); } ); - it( 'uses class when alignment.classNames is set', async () => { + it( 'uses class when classNames is set', async () => { const newEditor = await VirtualTestEditor .create( { language: { @@ -310,7 +310,7 @@ describe( 'AlignmentEditing', () => { } ); } ); - it( 'uses class when alignment.classNames is set', async () => { + it( 'uses class when classNames is set', async () => { const newEditor = await VirtualTestEditor .create( { plugins: [ AlignmentEditing, Paragraph ], @@ -352,7 +352,7 @@ describe( 'AlignmentEditing', () => { expect( editor.getData() ).to.equal( '

x

' ); } ); - it( 'uses class when alignment.classNames is set', async () => { + it( 'uses class when classNames is set', async () => { const newEditor = await VirtualTestEditor .create( { plugins: [ AlignmentEditing, Paragraph ], @@ -431,98 +431,74 @@ describe( 'AlignmentEditing', () => { ); } ); } ); - } ); - describe( 'classNames', () => { - it( 'should throw when options are repeated - repeated name', async () => { - let error; + describe( 'className property', () => { + it( 'should throw when options are repeated - repeated name', async () => { + let error; + + try { + await VirtualTestEditor + .create( { + plugins: [ AlignmentEditing, Paragraph ], + alignment: { + options: [ + { name: 'left', className: 'foo-left' }, + 'left' + ] + } + } ); + } catch ( err ) { + error = err; + } - try { - await VirtualTestEditor - .create( { - plugins: [ AlignmentEditing, Paragraph ], - alignment: { - options: [ - { name: 'left', className: 'foo-left' }, - 'left' - ] - } - } ); - } catch ( err ) { - error = err; - } + expect( error.constructor ).to.equal( CKEditorError ); + expect( error ).to.match( /alignment-config-name-already-defined/ ); + } ); - expect( error.constructor ).to.equal( CKEditorError ); - expect( error ).to.match( /alignment-config-name-already-defined/ ); - } ); + it( 'should throw when options are repeated - repeated className', async () => { + let error; + + try { + await VirtualTestEditor + .create( { + plugins: [ AlignmentEditing, Paragraph ], + alignment: { + options: [ + { name: 'right', className: 'foo-right' }, + 'left', + { name: 'center', className: 'foo-right' } + ] + } + } ); + } catch ( err ) { + error = err; + } - it( 'should throw when options are repeated - repeated className', async () => { - let error; + expect( error.constructor ).to.equal( CKEditorError ); + expect( error ).to.match( /alignment-config-classname-already-defined/ ); + } ); - try { - await VirtualTestEditor + it( 'should map limited options to limited set of classes', async () => { + const newEditor = await VirtualTestEditor .create( { plugins: [ AlignmentEditing, Paragraph ], alignment: { options: [ - 'left', - { name: 'right', className: 'foo-right' }, - { name: 'center', className: 'foo-right' } + { name: 'center', className: 'foo-center' }, + { name: 'left', className: 'foo-left' } ] } } ); - } catch ( err ) { - error = err; - } + const model = newEditor.model; + const data = '

x

'; - expect( error.constructor ).to.equal( CKEditorError ); - expect( error ).to.match( /alignment-config-classname-already-defined/ ); - } ); + newEditor.setData( data ); - it( 'should map limited options to limited set of classes', async () => { - const newEditor = await VirtualTestEditor - .create( { - plugins: [ AlignmentEditing, Paragraph ], - alignment: { - options: [ - { name: 'left', className: 'foo-left' }, - { name: 'center', className: 'foo-center' } - ] - } - } ); - const model = newEditor.model; - const data = '

x

'; + expect( getModelData( model ) ).to.equal( '[]x' ); + expect( newEditor.getData() ).to.equal( '

x

' ); - newEditor.setData( data ); - - expect( getModelData( model ) ).to.equal( '[]x' ); - expect( newEditor.getData() ).to.equal( '

x

' ); - - return newEditor.destroy(); - } ); - - it( 'should map classes to default options', async () => { - const newEditor = await VirtualTestEditor - .create( { - plugins: [ AlignmentEditing, Paragraph ], - alignment: { - options: [ - { name: 'left', className: 'foo-left' }, - { name: 'right', className: 'foo-right' }, - { name: 'center', className: 'foo-center' }, - { name: 'justify', className: 'foo-justify' } - ] - } - } ); - const model = newEditor.model; - const data = '

x

'; - - newEditor.setData( data ); - - expect( getModelData( model ) ).to.equal( '[]x' ); - expect( newEditor.getData() ).to.equal( '

x

' ); - - return newEditor.destroy(); + return newEditor.destroy(); + } ); } ); } ); } ); From a1677ccc9af8ffad3fae192b306eef3560b808d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maksymilian=20Barna=C5=9B?= Date: Mon, 22 Feb 2021 11:53:53 +0100 Subject: [PATCH 20/26] Cleanup of defaults. --- packages/ckeditor5-alignment/src/alignmentediting.js | 4 ++-- packages/ckeditor5-alignment/src/utils.js | 12 ++++-------- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/packages/ckeditor5-alignment/src/alignmentediting.js b/packages/ckeditor5-alignment/src/alignmentediting.js index 9d6ad39c3a4..02f7aca8b1c 100644 --- a/packages/ckeditor5-alignment/src/alignmentediting.js +++ b/packages/ckeditor5-alignment/src/alignmentediting.js @@ -10,7 +10,7 @@ import { Plugin } from 'ckeditor5/src/core'; import AlignmentCommand from './alignmentcommand'; -import { defaultConfig, isDefault, isSupported, normalizeAlignmentOptions } from './utils'; +import { isDefault, isSupported, normalizeAlignmentOptions, supportedOptions } from './utils'; /** * The alignment editing feature. It introduces the {@link module:alignment/alignmentcommand~AlignmentCommand command} and adds @@ -32,7 +32,7 @@ export default class AlignmentEditing extends Plugin { super( editor ); editor.config.define( 'alignment', { - options: [ ...defaultConfig ] + options: [ ...supportedOptions.map( option => ( { name: option } ) ) ] } ); } diff --git a/packages/ckeditor5-alignment/src/utils.js b/packages/ckeditor5-alignment/src/utils.js index db800e3db90..7d15611965e 100644 --- a/packages/ckeditor5-alignment/src/utils.js +++ b/packages/ckeditor5-alignment/src/utils.js @@ -18,11 +18,10 @@ import { CKEditorError } from '../../../src/utils'; * * `'justify'` */ export const supportedOptions = [ 'left', 'right', 'center', 'justify' ]; -export const defaultConfig = supportedOptions.map( option => { - return { - name: option - }; -} ); + +const optionNameToOptionMap = Object.fromEntries( + supportedOptions.map( option => [ option, { name: option } ] ) +); /** * Checks whether the passed option is supported by {@link module:alignment/alignmentediting~AlignmentEditing}. @@ -59,8 +58,6 @@ export function isDefault( alignment, locale ) { * @returns {Array.} Normalized object holding the configuration. */ export function normalizeAlignmentOptions( configuredOptions = [] ) { - const optionNameToOptionMap = new Map( defaultConfig.map( option => [ option.name, option ] ) ); - const normalizedOptions = configuredOptions .map( option => { let optionObj; @@ -77,7 +74,6 @@ export function normalizeAlignmentOptions( configuredOptions = [] ) { // Validate resulting config. normalizedOptions.forEach( ( option, index, allOptions ) => { const succeedingOptions = allOptions.slice( index + 1 ); - const nameAlreadyExists = succeedingOptions.some( item => item.name == option.name ); if ( nameAlreadyExists ) { From e16be307d017bbebf5ad60b1e88cbb336018c2a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maksymilian=20Barna=C5=9B?= Date: Mon, 22 Feb 2021 12:25:47 +0100 Subject: [PATCH 21/26] Update docs. --- .../docs/features/text-alignment.md | 10 ++++++---- packages/ckeditor5-alignment/src/alignment.js | 4 ++-- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/packages/ckeditor5-alignment/docs/features/text-alignment.md b/packages/ckeditor5-alignment/docs/features/text-alignment.md index f24b2a26fe1..b6d9c53bc20 100644 --- a/packages/ckeditor5-alignment/docs/features/text-alignment.md +++ b/packages/ckeditor5-alignment/docs/features/text-alignment.md @@ -50,10 +50,10 @@ ClassicEditor ### Using classes instead of inline style -By default alignment is set inline using `text-align` CSS property. If you wish the feature to output more semantic content that uses classes instead of inline styles, you can specify class names by using the `config.alignment.classNames` option and style them by using a stylesheet. +By default alignment is set inline using `text-align` CSS property. If you wish the feature to output more semantic content that uses classes instead of inline styles, you can specify class names by using the `className` property in `config.alignment.options` and style them by using a stylesheet. - Make sure to specify the same number of classes as the number of values in {@link module:alignment/alignment~AlignmentConfig#options `config.alignment.options`}. If the {@link module:alignment/alignment~AlignmentConfig#options `config.alignment.options`} are not specified, they are equal to `[ 'left', 'right', 'center', 'justify' ]`, therefore you have to provide 4 class names. + Once you decide to use classes for the alignment, you must define `className` for **all** alignment entries in {@link module:alignment/alignment~AlignmentConfig#options `config.alignment.options`}. The following configuration will set `.my-align-left` and `.my-align-right` to left and right alignment respectively. @@ -62,8 +62,10 @@ The following configuration will set `.my-align-left` and `.my-align-right` to l ClassicEditor .create( document.querySelector( '#editor' ), { alignment: { - options: [ 'left', 'right' ], - classNames: [ 'my-align-left', 'my-align-right' ] + options: [ + { name: 'left', className: 'my-align-left' }, + { name: 'right', className: 'my-align-right' } + ] }, toolbar: [ 'heading', '|', 'bulletedList', 'numberedList', 'alignment', 'undo', 'redo' diff --git a/packages/ckeditor5-alignment/src/alignment.js b/packages/ckeditor5-alignment/src/alignment.js index 6a8f0dc18ef..f7a2788f205 100644 --- a/packages/ckeditor5-alignment/src/alignment.js +++ b/packages/ckeditor5-alignment/src/alignment.js @@ -91,8 +91,8 @@ export default class Alignment extends Plugin { * .create( editorElement, { * alignment: { * options: [ - * { name: 'left', className: 'foo-align-left' }, - * { name: 'right', className: 'foo-align-right' } + * { name: 'left', className: 'my-align-left' }, + * { name: 'right', className: 'my-align-right' } * ] * } * } ) From 036cf4b7c77404a31cc3df20cd084d99e3e227c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maksymilian=20Barna=C5=9B?= Date: Mon, 22 Feb 2021 14:26:37 +0100 Subject: [PATCH 22/26] Missing comma. --- packages/ckeditor5-alignment/docs/features/text-alignment.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/ckeditor5-alignment/docs/features/text-alignment.md b/packages/ckeditor5-alignment/docs/features/text-alignment.md index b6d9c53bc20..7e67892de00 100644 --- a/packages/ckeditor5-alignment/docs/features/text-alignment.md +++ b/packages/ckeditor5-alignment/docs/features/text-alignment.md @@ -56,7 +56,7 @@ By default alignment is set inline using `text-align` CSS property. If you wish Once you decide to use classes for the alignment, you must define `className` for **all** alignment entries in {@link module:alignment/alignment~AlignmentConfig#options `config.alignment.options`}. -The following configuration will set `.my-align-left` and `.my-align-right` to left and right alignment respectively. +The following configuration will set `.my-align-left` and `.my-align-right` to left and right alignment, respectively. ```js ClassicEditor From 864f393ae1494e4be7b178f11f2b7f1f1f094bd8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maksymilian=20Barna=C5=9B?= Date: Tue, 23 Feb 2021 14:58:37 +0100 Subject: [PATCH 23/26] Review fixes. --- packages/ckeditor5-alignment/src/alignment.js | 2 +- .../src/alignmentediting.js | 38 ++++++++++++------- packages/ckeditor5-alignment/src/utils.js | 20 +++++----- .../tests/alignmentediting.js | 2 +- packages/ckeditor5-alignment/tests/utils.js | 10 ----- 5 files changed, 36 insertions(+), 36 deletions(-) diff --git a/packages/ckeditor5-alignment/src/alignment.js b/packages/ckeditor5-alignment/src/alignment.js index f7a2788f205..054c653fcb5 100644 --- a/packages/ckeditor5-alignment/src/alignment.js +++ b/packages/ckeditor5-alignment/src/alignment.js @@ -101,5 +101,5 @@ export default class Alignment extends Plugin { * * See the demo of {@glink features/text-alignment#configuring-alignment-options custom alignment options}. * - * @member {Array.} module:alignment/alignment~AlignmentConfig#options + * @member {Array.} module:alignment/alignment~AlignmentConfig#options */ diff --git a/packages/ckeditor5-alignment/src/alignmentediting.js b/packages/ckeditor5-alignment/src/alignmentediting.js index 02f7aca8b1c..28d198b6f11 100644 --- a/packages/ckeditor5-alignment/src/alignmentediting.js +++ b/packages/ckeditor5-alignment/src/alignmentediting.js @@ -51,9 +51,6 @@ export default class AlignmentEditing extends Plugin { option => isSupported( option.name ) && !isDefault( option.name, locale ) ); - // Converters for inline alignment need only alignment name. - const optionNamesToConvert = optionsToConvert.map( option => option.name ); - // Once there is at least one `className` defined, we switch to alignment with classes. const shouldUseClasses = optionsToConvert.some( option => !!option.className ); @@ -63,17 +60,13 @@ export default class AlignmentEditing extends Plugin { if ( shouldUseClasses ) { // Downcast to only to classes. - const definition = buildClassDefinition( optionsToConvert ); - - editor.conversion.attributeToAttribute( definition ); + editor.conversion.attributeToAttribute( buildClassDefinition( optionsToConvert ) ); } else { // Downcast inline styles. - const definition = buildDowncastInlineDefinition( optionNamesToConvert ); - - editor.conversion.for( 'downcast' ).attributeToAttribute( definition ); + editor.conversion.for( 'downcast' ).attributeToAttribute( buildDowncastInlineDefinition( optionsToConvert ) ); } - const upcastInlineDefinitions = buildUpcastInlineDefinitions( optionNamesToConvert ); + const upcastInlineDefinitions = buildUpcastInlineDefinitions( optionsToConvert ); // Always upcast from inline styles. for ( const definition of upcastInlineDefinitions ) { @@ -86,11 +79,12 @@ export default class AlignmentEditing extends Plugin { // Prepare downcast conversion definition for inline alignment styling. // @private -function buildDowncastInlineDefinition( optionNames ) { +function buildDowncastInlineDefinition( options ) { + const optionNames = options.map( option => option.name ); const definition = { model: { key: 'alignment', - values: optionNames.slice() + values: optionNames }, view: {} }; @@ -109,7 +103,8 @@ function buildDowncastInlineDefinition( optionNames ) { // Prepare upcast definitions for inline alignment styles. // @private -function buildUpcastInlineDefinitions( optionNames ) { +function buildUpcastInlineDefinitions( options ) { + const optionNames = options.map( option => option.name ); const definitions = []; for ( const name of optionNames ) { @@ -136,7 +131,7 @@ function buildClassDefinition( options ) { const definition = { model: { key: 'alignment', - values: options.map( option => option.name ).slice() + values: options.map( option => option.name ) }, view: {} }; @@ -151,3 +146,18 @@ function buildClassDefinition( options ) { return definition; } +/** + * The alignment configuration format descriptor. + * + * const alignmentFormat = { + * name: 'right', + * className: 'my-align-right-class' + * } + * + * @typedef {Object} module:alignment/alignmentediting~AlignmentFormat + * + * @property {'left'|'right'|'center'|'justify'} name One of the alignment names options. + * + * @property {String} className The CSS class used to represent the style in the view. + * Used to override default, inline styling for alignment. + */ diff --git a/packages/ckeditor5-alignment/src/utils.js b/packages/ckeditor5-alignment/src/utils.js index 7d15611965e..f16f8828c24 100644 --- a/packages/ckeditor5-alignment/src/utils.js +++ b/packages/ckeditor5-alignment/src/utils.js @@ -3,7 +3,7 @@ * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license */ -import { CKEditorError } from '../../../src/utils'; +import { CKEditorError } from 'ckeditor5/src/utils'; /** * @module alignment/utils @@ -57,7 +57,7 @@ export function isDefault( alignment, locale ) { * @param {Array.} configuredOptions Alignment plugin configuration. * @returns {Array.} Normalized object holding the configuration. */ -export function normalizeAlignmentOptions( configuredOptions = [] ) { +export function normalizeAlignmentOptions( configuredOptions ) { const normalizedOptions = configuredOptions .map( option => { let optionObj; @@ -78,13 +78,13 @@ export function normalizeAlignmentOptions( configuredOptions = [] ) { if ( nameAlreadyExists ) { /** - * The same `name` in one of the `alignment.options` was already declared. - * Each `name` representing one alignment option can be set exactly once. - * - * @error alignment-config-name-already-defined - * @param {Object} option First option that declares given `name`. - * @param {Array.} allOptions Contents of `alignment.options`. - */ + * The same `name` in one of the `alignment.options` was already declared. + * Each `name` representing one alignment option can be set exactly once. + * + * @error alignment-config-name-already-defined + * @param {Object} option First option that declares given `name`. + * @param {Array.} allOptions Contents of `alignment.options`. + */ throw new CKEditorError( 'alignment-config-name-already-defined', { option, allOptions } ); } @@ -93,7 +93,7 @@ export function normalizeAlignmentOptions( configuredOptions = [] ) { const classNameAlreadyExists = succeedingOptions.some( item => item.className == option.className ); if ( classNameAlreadyExists ) { - /** + /** * The same `className` in one of the `alignment.options` was already declared. * * @error alignment-config-classname-already-defined diff --git a/packages/ckeditor5-alignment/tests/alignmentediting.js b/packages/ckeditor5-alignment/tests/alignmentediting.js index 33b4861250d..39facaceae0 100644 --- a/packages/ckeditor5-alignment/tests/alignmentediting.js +++ b/packages/ckeditor5-alignment/tests/alignmentediting.js @@ -12,7 +12,7 @@ import VirtualTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/virtualtest import { getData as getModelData, setData as setModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model'; import AlignmentCommand from '../src/alignmentcommand'; -import { CKEditorError } from '../../../src/utils'; +import { CKEditorError } from 'ckeditor5/src/utils'; describe( 'AlignmentEditing', () => { let editor, model; diff --git a/packages/ckeditor5-alignment/tests/utils.js b/packages/ckeditor5-alignment/tests/utils.js index ecba0fb3ce8..ed5b5dbdd05 100644 --- a/packages/ckeditor5-alignment/tests/utils.js +++ b/packages/ckeditor5-alignment/tests/utils.js @@ -50,16 +50,6 @@ describe( 'utils', () => { } ); describe( 'normalizeAlignmentOptions', () => { - it( 'does nothing when no parameters are provided', () => { - let result; - - expect( () => { - result = normalizeAlignmentOptions(); - } ).not.to.throw(); - - expect( result ).to.deep.equal( [ ] ); - } ); - it( 'normalizes mixed input into an config array of objects', () => { const config = [ 'left', From 045e5f76c2b811d9aa4e04203b6ad9410d715b28 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maksymilian=20Barna=C5=9B?= Date: Tue, 23 Feb 2021 16:22:15 +0100 Subject: [PATCH 24/26] Review fixes part 2. --- .../src/alignmentediting.js | 17 +- packages/ckeditor5-alignment/src/utils.js | 25 +- .../tests/alignmentediting.js | 472 +++++++++++------- packages/ckeditor5-alignment/tests/utils.js | 52 +- 4 files changed, 355 insertions(+), 211 deletions(-) diff --git a/packages/ckeditor5-alignment/src/alignmentediting.js b/packages/ckeditor5-alignment/src/alignmentediting.js index 28d198b6f11..0820a80ceee 100644 --- a/packages/ckeditor5-alignment/src/alignmentediting.js +++ b/packages/ckeditor5-alignment/src/alignmentediting.js @@ -59,7 +59,6 @@ export default class AlignmentEditing extends Plugin { editor.model.schema.setAttributeProperties( 'alignment', { isFormatting: true } ); if ( shouldUseClasses ) { - // Downcast to only to classes. editor.conversion.attributeToAttribute( buildClassDefinition( optionsToConvert ) ); } else { // Downcast inline styles. @@ -80,20 +79,19 @@ export default class AlignmentEditing extends Plugin { // Prepare downcast conversion definition for inline alignment styling. // @private function buildDowncastInlineDefinition( options ) { - const optionNames = options.map( option => option.name ); const definition = { model: { key: 'alignment', - values: optionNames + values: options.map( option => option.name ) }, view: {} }; - for ( const name of optionNames ) { - definition.view[ name ] = { + for ( const option of options ) { + definition.view[ option.name ] = { key: 'style', value: { - 'text-align': name + 'text-align': option.name } }; } @@ -104,20 +102,19 @@ function buildDowncastInlineDefinition( options ) { // Prepare upcast definitions for inline alignment styles. // @private function buildUpcastInlineDefinitions( options ) { - const optionNames = options.map( option => option.name ); const definitions = []; - for ( const name of optionNames ) { + for ( const option of options ) { definitions.push( { view: { key: 'style', value: { - 'text-align': name + 'text-align': option.name } }, model: { key: 'alignment', - value: name + value: option.name } } ); } diff --git a/packages/ckeditor5-alignment/src/utils.js b/packages/ckeditor5-alignment/src/utils.js index f16f8828c24..8fe7f1983b6 100644 --- a/packages/ckeditor5-alignment/src/utils.js +++ b/packages/ckeditor5-alignment/src/utils.js @@ -3,7 +3,7 @@ * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license */ -import { CKEditorError } from 'ckeditor5/src/utils'; +import { CKEditorError, logWarning } from 'ckeditor5/src/utils'; /** * @module alignment/utils @@ -65,10 +65,27 @@ export function normalizeAlignmentOptions( configuredOptions ) { if ( typeof option == 'string' ) { optionObj = { name: option }; } else { - optionObj = { ...optionNameToOptionMap[ option ], ...option }; + optionObj = { ...optionNameToOptionMap[ option.name ], ...option }; } return optionObj; + } ) + // Remove all unknown options. + .filter( option => { + const isNameValid = !!supportedOptions.includes( option.name ); + if ( !isNameValid ) { + /** + * The `name` in one of the `alignment.options` is not recognized. + * The available options are: `'left'`, `'right'`, `'center'` and `'justify'`. + * + * @error alignment-config-name-not-recognized + * @param {Object} option Options with unknown value of the `name` property. + * @param {Array.} allOptions Contents of `alignment.options`. + */ + logWarning( 'alignment-config-name-not-recognized', { option, supportedOptions } ); + } + + return isNameValid; } ); // Validate resulting config. @@ -83,7 +100,7 @@ export function normalizeAlignmentOptions( configuredOptions ) { * * @error alignment-config-name-already-defined * @param {Object} option First option that declares given `name`. - * @param {Array.} allOptions Contents of `alignment.options`. + * @param {Array.} allOptions Contents of `alignment.options`. */ throw new CKEditorError( 'alignment-config-name-already-defined', { option, allOptions } ); } @@ -98,7 +115,7 @@ export function normalizeAlignmentOptions( configuredOptions ) { * * @error alignment-config-classname-already-defined * @param {Object} option First option that declares given `className`. - * @param {Array.} allOptions Contents of `alignment.options`. + * @param {Array.} allOptions Contents of `alignment.options`. */ throw new CKEditorError( 'alignment-config-classname-already-defined', { option, allOptions } ); } diff --git a/packages/ckeditor5-alignment/tests/alignmentediting.js b/packages/ckeditor5-alignment/tests/alignmentediting.js index 39facaceae0..44919ba026c 100644 --- a/packages/ckeditor5-alignment/tests/alignmentediting.js +++ b/packages/ckeditor5-alignment/tests/alignmentediting.js @@ -12,18 +12,16 @@ import VirtualTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/virtualtest import { getData as getModelData, setData as setModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model'; import AlignmentCommand from '../src/alignmentcommand'; -import { CKEditorError } from 'ckeditor5/src/utils'; describe( 'AlignmentEditing', () => { let editor, model; beforeEach( async () => { - const newEditor = await VirtualTestEditor + editor = await VirtualTestEditor .create( { plugins: [ AlignmentEditing, Paragraph ] } ); - editor = newEditor; model = editor.model; } ); @@ -95,6 +93,58 @@ describe( 'AlignmentEditing', () => { expect( editor.getData() ).to.equal( '

x

' ); } ); + + describe( 'className', () => { + it( 'adds converters to the data pipeline', async () => { + const newEditor = await VirtualTestEditor + .create( { + plugins: [ AlignmentEditing, Paragraph ], + alignment: { + options: [ + { name: 'left', className: 'foo-left' }, + { name: 'right', className: 'foo-right' }, + { name: 'center', className: 'foo-center' }, + { name: 'justify', className: 'foo-justify' } + ] + } + } ); + const model = newEditor.model; + const data = '

x

'; + + newEditor.setData( data ); + + expect( getModelData( model ) ).to.equal( '[]x' ); + + return newEditor.destroy(); + } ); + + it( 'adds a converter to the view pipeline', async () => { + const newEditor = await VirtualTestEditor + .create( { + plugins: [ AlignmentEditing, Paragraph ], + alignment: { + options: [ + { name: 'left', className: 'foo-left' }, + { name: 'right', className: 'foo-right' }, + { name: 'center', className: 'foo-center' }, + { name: 'justify', className: 'foo-justify' } + ] + } + } ); + const model = newEditor.model; + + setModelData( model, '[]x' ); + + expect( newEditor.getData() ).to.equal( '

x

' ); + + newEditor.execute( 'alignment', { value: 'left' } ); + + expect( getModelData( model ) ).to.equal( '[]x' ); + expect( newEditor.getData() ).to.equal( '

x

' ); + + return newEditor.destroy(); + } ); + } ); } ); describe( 'RTL content', () => { @@ -133,32 +183,60 @@ describe( 'AlignmentEditing', () => { return newEditor.destroy(); } ); - it( 'uses class when classNames is set', async () => { - const newEditor = await VirtualTestEditor - .create( { - language: { - content: 'ar' - }, - plugins: [ AlignmentEditing, Paragraph ], - alignment: { - options: [ - { name: 'left', className: 'foo-left' }, - { name: 'right', className: 'foo-right' }, - { name: 'center', className: 'foo-center' }, - { name: 'justify', className: 'foo-justify' } - ] - } - } ); - const model = newEditor.model; + describe( 'className', () => { + it( 'adds a converter to the view pipeline', async () => { + const newEditor = await VirtualTestEditor + .create( { + language: { + content: 'ar' + }, + plugins: [ AlignmentEditing, Paragraph ], + alignment: { + options: [ + { name: 'left', className: 'foo-left' }, + { name: 'right', className: 'foo-right' }, + { name: 'center', className: 'foo-center' }, + { name: 'justify', className: 'foo-justify' } + ] + } + } ); + const model = newEditor.model; - setModelData( model, '[]x' ); + setModelData( model, '[]x' ); - newEditor.execute( 'alignment', { value: 'left' } ); + newEditor.execute( 'alignment', { value: 'left' } ); - expect( getModelData( model ) ).to.equal( '[]x' ); - expect( newEditor.getData() ).to.equal( '

x

' ); + expect( getModelData( model ) ).to.equal( '[]x' ); + expect( newEditor.getData() ).to.equal( '

x

' ); - return newEditor.destroy(); + return newEditor.destroy(); + } ); + + it( 'adds converters to the data pipeline', async () => { + const newEditor = await VirtualTestEditor + .create( { + language: { + content: 'ar' + }, + plugins: [ AlignmentEditing, Paragraph ], + alignment: { + options: [ + { name: 'left', className: 'foo-left' }, + { name: 'right', className: 'foo-right' }, + { name: 'center', className: 'foo-center' }, + { name: 'justify', className: 'foo-justify' } + ] + } + } ); + const model = newEditor.model; + const data = '

x

'; + + newEditor.setData( data ); + + expect( getModelData( model ) ).to.equal( '[]x' ); + + return newEditor.destroy(); + } ); } ); } ); @@ -171,33 +249,6 @@ describe( 'AlignmentEditing', () => { expect( editor.getData() ).to.equal( '

x

' ); } ); - - it( 'uses class when classNames is set', async () => { - const newEditor = await VirtualTestEditor - .create( { - plugins: [ AlignmentEditing, Paragraph ], - alignment: { - options: [ - { name: 'left', className: 'foo-left' }, - { name: 'right', className: 'foo-right' }, - { name: 'center', className: 'foo-center' }, - { name: 'justify', className: 'foo-justify' } - ] - } - } ); - const model = newEditor.model; - - setModelData( model, '[]x' ); - - expect( newEditor.getData() ).to.equal( '

x

' ); - - newEditor.execute( 'alignment', { value: 'left' } ); - - expect( getModelData( model ) ).to.equal( '[]x' ); - expect( newEditor.getData() ).to.equal( '

x

' ); - - return newEditor.destroy(); - } ); } ); describe( 'center alignment', () => { @@ -226,32 +277,54 @@ describe( 'AlignmentEditing', () => { expect( editor.getData() ).to.equal( '

x

' ); } ); - it( 'uses class when classNames is set', async () => { - const newEditor = await VirtualTestEditor - .create( { - language: { - content: 'ar' - }, - plugins: [ AlignmentEditing, Paragraph ], - alignment: { - options: [ - { name: 'left', className: 'foo-left' }, - { name: 'right', className: 'foo-right' }, - { name: 'center', className: 'foo-center' }, - { name: 'justify', className: 'foo-justify' } - ] - } - } ); - const model = newEditor.model; + describe( 'className', () => { + it( 'adds a converter to the view pipeline', async () => { + const newEditor = await VirtualTestEditor + .create( { + plugins: [ AlignmentEditing, Paragraph ], + alignment: { + options: [ + { name: 'left', className: 'foo-left' }, + { name: 'right', className: 'foo-right' }, + { name: 'center', className: 'foo-center' }, + { name: 'justify', className: 'foo-justify' } + ] + } + } ); + const model = newEditor.model; + + setModelData( model, '[]x' ); - setModelData( model, '[]x' ); + newEditor.execute( 'alignment', { value: 'center' } ); - newEditor.execute( 'alignment', { value: 'center' } ); + expect( getModelData( model ) ).to.equal( '[]x' ); + expect( newEditor.getData() ).to.equal( '

x

' ); - expect( getModelData( model ) ).to.equal( '[]x' ); - expect( newEditor.getData() ).to.equal( '

x

' ); + return newEditor.destroy(); + } ); + + it( 'adds converters to the data pipeline', async () => { + const newEditor = await VirtualTestEditor + .create( { + plugins: [ AlignmentEditing, Paragraph ], + alignment: { + options: [ + { name: 'left', className: 'foo-left' }, + { name: 'right', className: 'foo-right' }, + { name: 'center', className: 'foo-center' }, + { name: 'justify', className: 'foo-justify' } + ] + } + } ); + const model = newEditor.model; + const data = '

x

'; + + newEditor.setData( data ); - return newEditor.destroy(); + expect( getModelData( model ) ).to.equal( '[]x' ); + + return newEditor.destroy(); + } ); } ); } ); @@ -271,6 +344,56 @@ describe( 'AlignmentEditing', () => { expect( editor.getData() ).to.equal( '

x

' ); } ); + + describe( 'className', () => { + it( 'adds a converter to the view pipeline', async () => { + const newEditor = await VirtualTestEditor + .create( { + plugins: [ AlignmentEditing, Paragraph ], + alignment: { + options: [ + { name: 'left', className: 'foo-left' }, + { name: 'right', className: 'foo-right' }, + { name: 'center', className: 'foo-center' }, + { name: 'justify', className: 'foo-justify' } + ] + } + } ); + const model = newEditor.model; + + setModelData( model, '[]x' ); + + newEditor.execute( 'alignment', { value: 'right' } ); + + expect( getModelData( model ) ).to.equal( '[]x' ); + expect( newEditor.getData() ).to.equal( '

x

' ); + + return newEditor.destroy(); + } ); + + it( 'adds converters to the data pipeline', async () => { + const newEditor = await VirtualTestEditor + .create( { + plugins: [ AlignmentEditing, Paragraph ], + alignment: { + options: [ + { name: 'left', className: 'foo-left' }, + { name: 'right', className: 'foo-right' }, + { name: 'center', className: 'foo-center' }, + { name: 'justify', className: 'foo-justify' } + ] + } + } ); + const model = newEditor.model; + const data = '

x

'; + + newEditor.setData( data ); + + expect( getModelData( model ) ).to.equal( '[]x' ); + + return newEditor.destroy(); + } ); + } ); } ); describe( 'RTL content', () => { @@ -308,31 +431,62 @@ describe( 'AlignmentEditing', () => { return newEditor.destroy(); } ); - } ); - it( 'uses class when classNames is set', async () => { - const newEditor = await VirtualTestEditor - .create( { - plugins: [ AlignmentEditing, Paragraph ], - alignment: { - options: [ - { name: 'left', className: 'foo-left' }, - { name: 'right', className: 'foo-right' }, - { name: 'center', className: 'foo-center' }, - { name: 'justify', className: 'foo-justify' } - ] - } + describe( 'className', () => { + it( 'adds a converter to the view pipeline', async () => { + const newEditor = await VirtualTestEditor + .create( { + language: { + content: 'ar' + }, + plugins: [ AlignmentEditing, Paragraph ], + alignment: { + options: [ + { name: 'left', className: 'foo-left' }, + { name: 'right', className: 'foo-right' }, + { name: 'center', className: 'foo-center' }, + { name: 'justify', className: 'foo-justify' } + ] + } + } ); + const model = newEditor.model; + + setModelData( model, '[]x' ); + + newEditor.execute( 'alignment', { value: 'right' } ); + + expect( getModelData( model ) ).to.equal( '[]x' ); + expect( newEditor.getData() ).to.equal( '

x

' ); + + return newEditor.destroy(); } ); - const model = newEditor.model; - setModelData( model, '[]x' ); + it( 'adds converters to the data pipeline', async () => { + const newEditor = await VirtualTestEditor + .create( { + language: { + content: 'ar' + }, + plugins: [ AlignmentEditing, Paragraph ], + alignment: { + options: [ + { name: 'left', className: 'foo-left' }, + { name: 'right', className: 'foo-right' }, + { name: 'center', className: 'foo-center' }, + { name: 'justify', className: 'foo-justify' } + ] + } + } ); + const model = newEditor.model; + const data = '

x

'; - newEditor.execute( 'alignment', { value: 'right' } ); + newEditor.setData( data ); - expect( getModelData( model ) ).to.equal( '[]x' ); - expect( newEditor.getData() ).to.equal( '

x

' ); + expect( getModelData( model ) ).to.equal( '[]x' ); - return newEditor.destroy(); + return newEditor.destroy(); + } ); + } ); } ); } ); @@ -352,29 +506,54 @@ describe( 'AlignmentEditing', () => { expect( editor.getData() ).to.equal( '

x

' ); } ); - it( 'uses class when classNames is set', async () => { - const newEditor = await VirtualTestEditor - .create( { - plugins: [ AlignmentEditing, Paragraph ], - alignment: { - options: [ - { name: 'left', className: 'foo-left' }, - { name: 'right', className: 'foo-right' }, - { name: 'center', className: 'foo-center' }, - { name: 'justify', className: 'foo-justify' } - ] - } - } ); - const model = newEditor.model; + describe( 'className', () => { + it( 'adds a converter to the view pipeline', async () => { + const newEditor = await VirtualTestEditor + .create( { + plugins: [ AlignmentEditing, Paragraph ], + alignment: { + options: [ + { name: 'left', className: 'foo-left' }, + { name: 'right', className: 'foo-right' }, + { name: 'center', className: 'foo-center' }, + { name: 'justify', className: 'foo-justify' } + ] + } + } ); + const model = newEditor.model; - setModelData( model, '[]x' ); + setModelData( model, '[]x' ); - newEditor.execute( 'alignment', { value: 'justify' } ); + newEditor.execute( 'alignment', { value: 'justify' } ); - expect( getModelData( model ) ).to.equal( '[]x' ); - expect( newEditor.getData() ).to.equal( '

x

' ); + expect( getModelData( model ) ).to.equal( '[]x' ); + expect( newEditor.getData() ).to.equal( '

x

' ); + + return newEditor.destroy(); + } ); + + it( 'adds converters to the data pipeline', async () => { + const newEditor = await VirtualTestEditor + .create( { + plugins: [ AlignmentEditing, Paragraph ], + alignment: { + options: [ + { name: 'left', className: 'foo-left' }, + { name: 'right', className: 'foo-right' }, + { name: 'center', className: 'foo-center' }, + { name: 'justify', className: 'foo-justify' } + ] + } + } ); + const model = newEditor.model; + const data = '

x

'; + + newEditor.setData( data ); + + expect( getModelData( model ) ).to.equal( '[]x' ); - return newEditor.destroy(); + return newEditor.destroy(); + } ); } ); } ); @@ -431,75 +610,6 @@ describe( 'AlignmentEditing', () => { ); } ); } ); - - describe( 'className property', () => { - it( 'should throw when options are repeated - repeated name', async () => { - let error; - - try { - await VirtualTestEditor - .create( { - plugins: [ AlignmentEditing, Paragraph ], - alignment: { - options: [ - { name: 'left', className: 'foo-left' }, - 'left' - ] - } - } ); - } catch ( err ) { - error = err; - } - - expect( error.constructor ).to.equal( CKEditorError ); - expect( error ).to.match( /alignment-config-name-already-defined/ ); - } ); - - it( 'should throw when options are repeated - repeated className', async () => { - let error; - - try { - await VirtualTestEditor - .create( { - plugins: [ AlignmentEditing, Paragraph ], - alignment: { - options: [ - { name: 'right', className: 'foo-right' }, - 'left', - { name: 'center', className: 'foo-right' } - ] - } - } ); - } catch ( err ) { - error = err; - } - - expect( error.constructor ).to.equal( CKEditorError ); - expect( error ).to.match( /alignment-config-classname-already-defined/ ); - } ); - - it( 'should map limited options to limited set of classes', async () => { - const newEditor = await VirtualTestEditor - .create( { - plugins: [ AlignmentEditing, Paragraph ], - alignment: { - options: [ - { name: 'center', className: 'foo-center' }, - { name: 'left', className: 'foo-left' } - ] - } - } ); - const model = newEditor.model; - const data = '

x

'; - - newEditor.setData( data ); - - expect( getModelData( model ) ).to.equal( '[]x' ); - expect( newEditor.getData() ).to.equal( '

x

' ); - - return newEditor.destroy(); - } ); - } ); } ); } ); } ); diff --git a/packages/ckeditor5-alignment/tests/utils.js b/packages/ckeditor5-alignment/tests/utils.js index ed5b5dbdd05..54c97d9b70d 100644 --- a/packages/ckeditor5-alignment/tests/utils.js +++ b/packages/ckeditor5-alignment/tests/utils.js @@ -3,10 +3,15 @@ * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license */ -import { CKEditorError } from '../../../src/utils'; +/* globals console */ + +import { CKEditorError } from 'ckeditor5/src/utils'; +import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils'; import { isDefault, isSupported, supportedOptions, normalizeAlignmentOptions } from '../src/utils'; describe( 'utils', () => { + testUtils.createSinonSandbox(); + describe( 'isDefault()', () => { it( 'should return true for "left" alignment only (LTR)', () => { const locale = { @@ -53,9 +58,7 @@ describe( 'utils', () => { it( 'normalizes mixed input into an config array of objects', () => { const config = [ 'left', - { - name: 'right' - }, + { name: 'right' }, 'center', { name: 'justify', @@ -67,15 +70,9 @@ describe( 'utils', () => { expect( result ).to.deep.equal( [ - { - 'name': 'left' - }, - { - 'name': 'right' - }, - { - 'name': 'center' - }, + { 'name': 'left' }, + { 'name': 'right' }, + { 'name': 'center' }, { 'className': 'foo-center', 'name': 'justify' @@ -84,12 +81,35 @@ describe( 'utils', () => { ); } ); + it( 'should warn if the name is not recognized', () => { + testUtils.sinon.stub( console, 'warn' ); + + const config = [ + 'left', + { name: 'center1' } + ]; + + expect( normalizeAlignmentOptions( config ) ).to.deep.equal( [ + { name: 'left' } + ] ); + + const params = { + option: { name: 'center1' }, + supportedOptions: [ 'left', 'right', 'center', 'justify' ] + }; + + sinon.assert.calledOnce( console.warn ); + sinon.assert.calledWithExactly( console.warn, + sinon.match( /^alignment-config-name-not-recognized/ ), + params, + sinon.match.string // Link to the documentation + ); + } ); + it( 'throws when the name already exists', () => { const config = [ 'center', - { - name: 'center' - } + { name: 'center' } ]; let error; From cbcaa1e03b2ae2e1a15466459a919c1abb0bb2ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maksymilian=20Barna=C5=9B?= Date: Tue, 23 Feb 2021 21:46:19 +0100 Subject: [PATCH 25/26] Throw exception when className is not provided for all the options. --- packages/ckeditor5-alignment/src/utils.js | 37 +++++++++++++------ .../tests/alignmentediting.js | 3 +- packages/ckeditor5-alignment/tests/utils.js | 33 +++++++++++------ 3 files changed, 47 insertions(+), 26 deletions(-) diff --git a/packages/ckeditor5-alignment/src/utils.js b/packages/ckeditor5-alignment/src/utils.js index 8fe7f1983b6..c82568e715e 100644 --- a/packages/ckeditor5-alignment/src/utils.js +++ b/packages/ckeditor5-alignment/src/utils.js @@ -54,21 +54,21 @@ export function isDefault( alignment, locale ) { /** * Brings the configuration to the common form, an array of objects. * - * @param {Array.} configuredOptions Alignment plugin configuration. - * @returns {Array.} Normalized object holding the configuration. + * @param {Array.} configuredOptions Alignment plugin configuration. + * @returns {Array.} Normalized object holding the configuration. */ export function normalizeAlignmentOptions( configuredOptions ) { const normalizedOptions = configuredOptions .map( option => { - let optionObj; + let result; if ( typeof option == 'string' ) { - optionObj = { name: option }; + result = { name: option }; } else { - optionObj = { ...optionNameToOptionMap[ option.name ], ...option }; + result = { ...optionNameToOptionMap[ option.name ], ...option }; } - return optionObj; + return result; } ) // Remove all unknown options. .filter( option => { @@ -80,14 +80,26 @@ export function normalizeAlignmentOptions( configuredOptions ) { * * @error alignment-config-name-not-recognized * @param {Object} option Options with unknown value of the `name` property. - * @param {Array.} allOptions Contents of `alignment.options`. */ - logWarning( 'alignment-config-name-not-recognized', { option, supportedOptions } ); + logWarning( 'alignment-config-name-not-recognized', { option } ); } return isNameValid; } ); + const classNameCount = normalizedOptions.filter( option => !!option.className ).length; + + // We either use classes for all styling options or for none. + if ( classNameCount && classNameCount < normalizedOptions.length ) { + /** + * The `className` property has to be defined for all options once at least one option declares `className`. + * + * @error alignment-config-classnames-are-missing + * @param {Array.} configuredOptions Contents of `alignment.options`. + */ + throw new CKEditorError( 'alignment-config-classnames-are-missing', { configuredOptions } ); + } + // Validate resulting config. normalizedOptions.forEach( ( option, index, allOptions ) => { const succeedingOptions = allOptions.slice( index + 1 ); @@ -100,9 +112,9 @@ export function normalizeAlignmentOptions( configuredOptions ) { * * @error alignment-config-name-already-defined * @param {Object} option First option that declares given `name`. - * @param {Array.} allOptions Contents of `alignment.options`. + * @param {Array.} configuredOptions Contents of `alignment.options`. */ - throw new CKEditorError( 'alignment-config-name-already-defined', { option, allOptions } ); + throw new CKEditorError( 'alignment-config-name-already-defined', { option, configuredOptions } ); } // The `className` property is present. Check for duplicates then. @@ -115,9 +127,10 @@ export function normalizeAlignmentOptions( configuredOptions ) { * * @error alignment-config-classname-already-defined * @param {Object} option First option that declares given `className`. - * @param {Array.} allOptions Contents of `alignment.options`. + * @param {Array.} configuredOptions + * Contents of `alignment.options`. */ - throw new CKEditorError( 'alignment-config-classname-already-defined', { option, allOptions } ); + throw new CKEditorError( 'alignment-config-classname-already-defined', { option, configuredOptions } ); } } } ); diff --git a/packages/ckeditor5-alignment/tests/alignmentediting.js b/packages/ckeditor5-alignment/tests/alignmentediting.js index 44919ba026c..3eacd9dbe65 100644 --- a/packages/ckeditor5-alignment/tests/alignmentediting.js +++ b/packages/ckeditor5-alignment/tests/alignmentediting.js @@ -49,12 +49,11 @@ describe( 'AlignmentEditing', () => { describe( 'integration', () => { beforeEach( async () => { - const newEditor = await VirtualTestEditor + const editor = await VirtualTestEditor .create( { plugins: [ AlignmentEditing, ImageCaptionEditing, Paragraph, ListEditing, HeadingEditing ] } ); - editor = newEditor; model = editor.model; } ); diff --git a/packages/ckeditor5-alignment/tests/utils.js b/packages/ckeditor5-alignment/tests/utils.js index 54c97d9b70d..2b6fb9ed974 100644 --- a/packages/ckeditor5-alignment/tests/utils.js +++ b/packages/ckeditor5-alignment/tests/utils.js @@ -60,10 +60,7 @@ describe( 'utils', () => { 'left', { name: 'right' }, 'center', - { - name: 'justify', - className: 'foo-center' - } + { name: 'justify' } ]; const result = normalizeAlignmentOptions( config ); @@ -73,15 +70,12 @@ describe( 'utils', () => { { 'name': 'left' }, { 'name': 'right' }, { 'name': 'center' }, - { - 'className': 'foo-center', - 'name': 'justify' - } + { 'name': 'justify' } ] ); } ); - it( 'should warn if the name is not recognized', () => { + it( 'warns if the name is not recognized', () => { testUtils.sinon.stub( console, 'warn' ); const config = [ @@ -94,8 +88,7 @@ describe( 'utils', () => { ] ); const params = { - option: { name: 'center1' }, - supportedOptions: [ 'left', 'right', 'center', 'justify' ] + option: { name: 'center1' } }; sinon.assert.calledOnce( console.warn ); @@ -106,6 +99,23 @@ describe( 'utils', () => { ); } ); + it( 'throws when the className is not defined for all options', () => { + const config = [ + 'left', + { name: 'center', className: 'foo-center' } + ]; + let error; + + try { + normalizeAlignmentOptions( config ); + } catch ( err ) { + error = err; + } + + expect( error.constructor ).to.equal( CKEditorError ); + expect( error ).to.match( /alignment-config-classnames-are-missing/ ); + } ); + it( 'throws when the name already exists', () => { const config = [ 'center', @@ -125,7 +135,6 @@ describe( 'utils', () => { it( 'throws when the className already exists', () => { const config = [ - 'left', { name: 'center', className: 'foo-center' From f9ee5f4b99a1c2cc1094845a2a474f92367fae90 Mon Sep 17 00:00:00 2001 From: Kuba Niegowski Date: Fri, 26 Feb 2021 14:40:51 +0100 Subject: [PATCH 26/26] Code refactoring. --- packages/ckeditor5-alignment/src/alignmentediting.js | 12 ++++++------ packages/ckeditor5-alignment/src/utils.js | 6 +----- 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/packages/ckeditor5-alignment/src/alignmentediting.js b/packages/ckeditor5-alignment/src/alignmentediting.js index 0820a80ceee..6da914b9de2 100644 --- a/packages/ckeditor5-alignment/src/alignmentediting.js +++ b/packages/ckeditor5-alignment/src/alignmentediting.js @@ -87,11 +87,11 @@ function buildDowncastInlineDefinition( options ) { view: {} }; - for ( const option of options ) { - definition.view[ option.name ] = { + for ( const { name } of options ) { + definition.view[ name ] = { key: 'style', value: { - 'text-align': option.name + 'text-align': name } }; } @@ -104,17 +104,17 @@ function buildDowncastInlineDefinition( options ) { function buildUpcastInlineDefinitions( options ) { const definitions = []; - for ( const option of options ) { + for ( const { name } of options ) { definitions.push( { view: { key: 'style', value: { - 'text-align': option.name + 'text-align': name } }, model: { key: 'alignment', - value: option.name + value: name } } ); } diff --git a/packages/ckeditor5-alignment/src/utils.js b/packages/ckeditor5-alignment/src/utils.js index c82568e715e..75883d864a9 100644 --- a/packages/ckeditor5-alignment/src/utils.js +++ b/packages/ckeditor5-alignment/src/utils.js @@ -19,10 +19,6 @@ import { CKEditorError, logWarning } from 'ckeditor5/src/utils'; */ export const supportedOptions = [ 'left', 'right', 'center', 'justify' ]; -const optionNameToOptionMap = Object.fromEntries( - supportedOptions.map( option => [ option, { name: option } ] ) -); - /** * Checks whether the passed option is supported by {@link module:alignment/alignmentediting~AlignmentEditing}. * @@ -65,7 +61,7 @@ export function normalizeAlignmentOptions( configuredOptions ) { if ( typeof option == 'string' ) { result = { name: option }; } else { - result = { ...optionNameToOptionMap[ option.name ], ...option }; + result = option; } return result;