From e24433bb1ffac1f7164a877a29671b45953c58d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Go=C5=82aszewski?= Date: Fri, 4 Oct 2019 09:56:10 +0200 Subject: [PATCH 1/3] Make IndentBlock work regardless of editor plugins order. Checking schema for registered elements should be done in afterInit call to prevent plugin ordering related bugs. --- src/indentblock.js | 11 ++--------- tests/indentblock-integration.js | 24 +++++++++++++++++++----- 2 files changed, 21 insertions(+), 14 deletions(-) diff --git a/src/indentblock.js b/src/indentblock.js index 4030248..ecb3e06 100644 --- a/src/indentblock.js +++ b/src/indentblock.js @@ -46,10 +46,9 @@ export default class IndentBlock extends Plugin { /** * @inheritDoc */ - init() { + afterInit() { const editor = this.editor; const schema = editor.model.schema; - const conversion = editor.conversion; const configuration = editor.config.get( 'indentBlock' ); // Enable block indentation by default in paragraph and default headings. @@ -67,7 +66,7 @@ export default class IndentBlock extends Plugin { const outdentConfig = Object.assign( { direction: 'backward' }, configuration ); if ( useOffsetConfig ) { - this._setupConversionUsingOffset( conversion ); + this._setupConversionUsingOffset( editor.conversion ); editor.commands.add( 'indentBlock', new IndentBlockCommand( editor, new IndentUsingOffset( indentConfig ) ) ); editor.commands.add( 'outdentBlock', new IndentBlockCommand( editor, new IndentUsingOffset( outdentConfig ) ) ); } else { @@ -75,13 +74,7 @@ export default class IndentBlock extends Plugin { editor.commands.add( 'indentBlock', new IndentBlockCommand( editor, new IndentUsingClasses( indentConfig ) ) ); editor.commands.add( 'outdentBlock', new IndentBlockCommand( editor, new IndentUsingClasses( outdentConfig ) ) ); } - } - /** - * @inheritDoc - */ - afterInit() { - const editor = this.editor; const indentCommand = editor.commands.get( 'indent' ); const outdentCommand = editor.commands.get( 'outdent' ); diff --git a/tests/indentblock-integration.js b/tests/indentblock-integration.js index 7b659c8..9d84653 100644 --- a/tests/indentblock-integration.js +++ b/tests/indentblock-integration.js @@ -13,7 +13,7 @@ import IndentEditing from '../src/indentediting'; import IndentBlock from '../src/indentblock'; describe( 'IndentBlock - integration', () => { - let editor, model, doc; + let editor, doc; testUtils.createSinonSandbox(); @@ -28,8 +28,7 @@ describe( 'IndentBlock - integration', () => { return createTestEditor( { indentBlock: { offset: 50, unit: 'px' } } ) .then( newEditor => { editor = newEditor; - model = editor.model; - doc = model.document; + doc = editor.model.document; } ); } ); @@ -54,8 +53,7 @@ describe( 'IndentBlock - integration', () => { indentBlock: { offset: 50, unit: 'px' } } ).then( newEditor => { editor = newEditor; - model = editor.model; - doc = model.document; + doc = editor.model.document; } ); } ); @@ -73,6 +71,22 @@ describe( 'IndentBlock - integration', () => { } ); } ); + it( 'should work with paragraph regardless of plugin order', () => { + return createTestEditor( { + plugins: [ IndentEditing, IndentBlock, Paragraph, HeadingEditing ], + indentBlock: { offset: 50, unit: 'px' } + } ).then( newEditor => { + editor = newEditor; + doc = editor.model.document; + + editor.setData( '

foo

' ); + + const paragraph = doc.getRoot().getChild( 0 ); + + expect( paragraph.hasAttribute( 'blockIndent' ) ).to.be.true; + } ); + } ); + function createTestEditor( extraConfig = {} ) { return VirtualTestEditor .create( Object.assign( { From 2e8db9db2d4cfebc145085a61be6b85222107d42 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Go=C5=82aszewski?= Date: Wed, 9 Oct 2019 11:40:46 +0200 Subject: [PATCH 2/3] Split indent block feature initialization to init and afterInit. The schema extending rules must be done in afterInit and all core indent block behavior should be done during the init phase. --- src/indentblock.js | 30 +++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/src/indentblock.js b/src/indentblock.js index ecb3e06..102c8ef 100644 --- a/src/indentblock.js +++ b/src/indentblock.js @@ -46,20 +46,10 @@ export default class IndentBlock extends Plugin { /** * @inheritDoc */ - afterInit() { + init() { const editor = this.editor; - const schema = editor.model.schema; const configuration = editor.config.get( 'indentBlock' ); - // Enable block indentation by default in paragraph and default headings. - const knownElements = [ 'paragraph', 'heading1', 'heading2', 'heading3', 'heading4', 'heading5', 'heading6' ]; - - knownElements.forEach( elementName => { - if ( schema.isRegistered( elementName ) ) { - schema.extend( elementName, { allowAttributes: 'blockIndent' } ); - } - } ); - const useOffsetConfig = !configuration.classes || !configuration.classes.length; const indentConfig = Object.assign( { direction: 'forward' }, configuration ); @@ -67,6 +57,7 @@ export default class IndentBlock extends Plugin { if ( useOffsetConfig ) { this._setupConversionUsingOffset( editor.conversion ); + editor.commands.add( 'indentBlock', new IndentBlockCommand( editor, new IndentUsingOffset( indentConfig ) ) ); editor.commands.add( 'outdentBlock', new IndentBlockCommand( editor, new IndentUsingOffset( outdentConfig ) ) ); } else { @@ -74,10 +65,27 @@ export default class IndentBlock extends Plugin { editor.commands.add( 'indentBlock', new IndentBlockCommand( editor, new IndentUsingClasses( indentConfig ) ) ); editor.commands.add( 'outdentBlock', new IndentBlockCommand( editor, new IndentUsingClasses( outdentConfig ) ) ); } + } + + /** + * @inheritDoc + */ + afterInit() { + const editor = this.editor; + const schema = editor.model.schema; const indentCommand = editor.commands.get( 'indent' ); const outdentCommand = editor.commands.get( 'outdent' ); + // Enable block indentation by default in paragraph and default headings. + const knownElements = [ 'paragraph', 'heading1', 'heading2', 'heading3', 'heading4', 'heading5', 'heading6' ]; + + knownElements.forEach( elementName => { + if ( schema.isRegistered( elementName ) ) { + schema.extend( elementName, { allowAttributes: 'blockIndent' } ); + } + } ); + indentCommand.registerChildCommand( editor.commands.get( 'indentBlock' ) ); outdentCommand.registerChildCommand( editor.commands.get( 'outdentBlock' ) ); } From 26e0ed351c8acb3b0f115c997eab379912a945d5 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Thu, 10 Oct 2019 12:35:12 +0200 Subject: [PATCH 3/3] Tests: Added a link to the issue. Improved language in the test description. --- tests/indentblock-integration.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/indentblock-integration.js b/tests/indentblock-integration.js index 9d84653..6f79093 100644 --- a/tests/indentblock-integration.js +++ b/tests/indentblock-integration.js @@ -71,7 +71,8 @@ describe( 'IndentBlock - integration', () => { } ); } ); - it( 'should work with paragraph regardless of plugin order', () => { + // https://github.com/ckeditor/ckeditor5/issues/2359 + it( 'should work with paragraphs regardless of plugin order', () => { return createTestEditor( { plugins: [ IndentEditing, IndentBlock, Paragraph, HeadingEditing ], indentBlock: { offset: 50, unit: 'px' }