From 5e549f9bf696a1971fea9d9e806401f2be8ae819 Mon Sep 17 00:00:00 2001 From: Andrzej Stanek Date: Wed, 1 Jun 2022 13:52:47 +0200 Subject: [PATCH 1/5] Export canBeCodeBlock(). Change indent/outdent code block command to use insertContent()/deleteContent(). --- packages/ckeditor5-code-block/src/codeblockcommand.js | 2 +- packages/ckeditor5-code-block/src/indentcodeblockcommand.js | 3 ++- .../ckeditor5-code-block/src/outdentcodeblockcommand.js | 6 +++--- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/packages/ckeditor5-code-block/src/codeblockcommand.js b/packages/ckeditor5-code-block/src/codeblockcommand.js index 19dd0da9297..17bf374f815 100644 --- a/packages/ckeditor5-code-block/src/codeblockcommand.js +++ b/packages/ckeditor5-code-block/src/codeblockcommand.js @@ -179,7 +179,7 @@ export default class CodeBlockCommand extends Command { } } -function canBeCodeBlock( schema, element ) { +export function canBeCodeBlock( schema, element ) { if ( element.is( 'rootElement' ) || schema.isLimit( element ) ) { return false; } diff --git a/packages/ckeditor5-code-block/src/indentcodeblockcommand.js b/packages/ckeditor5-code-block/src/indentcodeblockcommand.js index 1d8afb234c7..e5975e4938d 100644 --- a/packages/ckeditor5-code-block/src/indentcodeblockcommand.js +++ b/packages/ckeditor5-code-block/src/indentcodeblockcommand.js @@ -76,7 +76,8 @@ export default class IndentCodeBlockCommand extends Command { // // for ( const position of positions ) { - writer.insertText( this._indentSequence, position ); + // writer.insertText( this._indentSequence, position ); + editor.model.insertContent( writer.createText( this._indentSequence ), position ); } } ); } diff --git a/packages/ckeditor5-code-block/src/outdentcodeblockcommand.js b/packages/ckeditor5-code-block/src/outdentcodeblockcommand.js index 3d1211c7a17..99f3ee8c019 100644 --- a/packages/ckeditor5-code-block/src/outdentcodeblockcommand.js +++ b/packages/ckeditor5-code-block/src/outdentcodeblockcommand.js @@ -51,7 +51,7 @@ export default class OutdentCodeBlockCommand extends Command { const editor = this.editor; const model = editor.model; - model.change( writer => { + model.change( () => { const positions = getIndentOutdentPositions( model ); // Outdent all positions, for instance assuming the indent sequence is 4x space (" "): @@ -76,10 +76,10 @@ export default class OutdentCodeBlockCommand extends Command { // bazqux // for ( const position of positions ) { - const range = getLastOutdentableSequenceRange( this.editor.model, position, this._indentSequence ); + const range = getLastOutdentableSequenceRange( model, position, this._indentSequence ); if ( range ) { - writer.remove( range ); + model.deleteContent( model.createSelection( range ) ); } } } ); From 1d491f27ff35a1427d067c501d7cb00f17f126b4 Mon Sep 17 00:00:00 2001 From: Andrzej Stanek Date: Wed, 1 Jun 2022 14:45:45 +0200 Subject: [PATCH 2/5] Remove commented code. --- packages/ckeditor5-code-block/src/indentcodeblockcommand.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/ckeditor5-code-block/src/indentcodeblockcommand.js b/packages/ckeditor5-code-block/src/indentcodeblockcommand.js index e5975e4938d..73956a75b42 100644 --- a/packages/ckeditor5-code-block/src/indentcodeblockcommand.js +++ b/packages/ckeditor5-code-block/src/indentcodeblockcommand.js @@ -76,8 +76,9 @@ export default class IndentCodeBlockCommand extends Command { // // for ( const position of positions ) { - // writer.insertText( this._indentSequence, position ); - editor.model.insertContent( writer.createText( this._indentSequence ), position ); + const indentSequenceTextElement = writer.createText( this._indentSequence ); + + editor.model.insertContent( indentSequenceTextElement, position ); } } ); } From 6cff4ed96c086a2b11a491aea2c00fb57e6041df Mon Sep 17 00:00:00 2001 From: Andrzej Stanek Date: Thu, 2 Jun 2022 11:45:13 +0200 Subject: [PATCH 3/5] Add tests to canBeCodeBlock(), comment reason for changing writer.insert() to insertContent(). --- .../src/codeblockcommand.js | 10 +-- .../src/indentcodeblockcommand.js | 4 + .../src/outdentcodeblockcommand.js | 4 + packages/ckeditor5-code-block/src/utils.js | 15 ++++ packages/ckeditor5-code-block/tests/utils.js | 90 +++++++++++++++++++ 5 files changed, 114 insertions(+), 9 deletions(-) create mode 100644 packages/ckeditor5-code-block/tests/utils.js diff --git a/packages/ckeditor5-code-block/src/codeblockcommand.js b/packages/ckeditor5-code-block/src/codeblockcommand.js index 17bf374f815..8f77867e723 100644 --- a/packages/ckeditor5-code-block/src/codeblockcommand.js +++ b/packages/ckeditor5-code-block/src/codeblockcommand.js @@ -10,7 +10,7 @@ import { Command } from 'ckeditor5/src/core'; import { first } from 'ckeditor5/src/utils'; -import { getNormalizedAndLocalizedLanguageDefinitions } from './utils'; +import { getNormalizedAndLocalizedLanguageDefinitions, canBeCodeBlock } from './utils'; /** * The code block command plugin. @@ -179,14 +179,6 @@ export default class CodeBlockCommand extends Command { } } -export function canBeCodeBlock( schema, element ) { - if ( element.is( 'rootElement' ) || schema.isLimit( element ) ) { - return false; - } - - return schema.checkChild( element.parent, 'codeBlock' ); -} - // Picks the language for the new code block. If any language is passed as an option, // it will be returned. Else, if option usePreviousLanguageChoice is true and some // code block was already created (lastLanguage is not null) then previously used diff --git a/packages/ckeditor5-code-block/src/indentcodeblockcommand.js b/packages/ckeditor5-code-block/src/indentcodeblockcommand.js index 73956a75b42..3ee034c6982 100644 --- a/packages/ckeditor5-code-block/src/indentcodeblockcommand.js +++ b/packages/ckeditor5-code-block/src/indentcodeblockcommand.js @@ -78,6 +78,10 @@ export default class IndentCodeBlockCommand extends Command { for ( const position of positions ) { const indentSequenceTextElement = writer.createText( this._indentSequence ); + // Previously insertion was done by writer.insertText(). It was changed to insertContent() to enable + // integration of code block with track changes. It's the easiest way of integration because insertContent() + // is already integrated with track changes, but if it ever cause any troubles it can be reverted, however + // some additional work will be required in track changes integration of code block. editor.model.insertContent( indentSequenceTextElement, position ); } } ); diff --git a/packages/ckeditor5-code-block/src/outdentcodeblockcommand.js b/packages/ckeditor5-code-block/src/outdentcodeblockcommand.js index 99f3ee8c019..83401f4ebb3 100644 --- a/packages/ckeditor5-code-block/src/outdentcodeblockcommand.js +++ b/packages/ckeditor5-code-block/src/outdentcodeblockcommand.js @@ -79,6 +79,10 @@ export default class OutdentCodeBlockCommand extends Command { const range = getLastOutdentableSequenceRange( model, position, this._indentSequence ); if ( range ) { + // Previously deletion was done by writer.remove(). It was changed to deleteContent() to enable + // integration of code block with track changes. It's the easiest way of integration because deleteContent() + // is already integrated with track changes, but if it ever cause any troubles it can be reverted, however + // some additional work will be required in track changes integration of code block. model.deleteContent( model.createSelection( range ) ); } } diff --git a/packages/ckeditor5-code-block/src/utils.js b/packages/ckeditor5-code-block/src/utils.js index 3b0c794573a..586cd4f17b8 100644 --- a/packages/ckeditor5-code-block/src/utils.js +++ b/packages/ckeditor5-code-block/src/utils.js @@ -219,3 +219,18 @@ export function isModelSelectionInCodeBlock( selection ) { return firstBlock && firstBlock.is( 'element', 'codeBlock' ); } + +/** + * Checks if an {@link module:engine/model/element~Element Element} can become a code block. + * + * @param {module:engine/model/schema~Schema} schema Model's schema. + * @param {module:engine/model/element~Element} element The element to be checked. + * @returns {Boolean} Check result. + */ +export function canBeCodeBlock( schema, element ) { + if ( element.is( 'rootElement' ) || schema.isLimit( element ) ) { + return false; + } + + return schema.checkChild( element.parent, 'codeBlock' ); +} diff --git a/packages/ckeditor5-code-block/tests/utils.js b/packages/ckeditor5-code-block/tests/utils.js new file mode 100644 index 00000000000..4bf84fed43c --- /dev/null +++ b/packages/ckeditor5-code-block/tests/utils.js @@ -0,0 +1,90 @@ +/** + * @license Copyright (c) 2003-2022, CKSource Holding sp. z o.o. All rights reserved. + * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license + */ + +import Model from '@ckeditor/ckeditor5-engine/src/model/model'; + +import { canBeCodeBlock } from '../src/utils'; + +describe( 'utils', () => { + describe( 'canBecomeCodeBlock()', () => { + it( 'should not allow a root element to become a code block', () => { + const model = new Model(); + const root = model.document.createRoot(); + + const testResult = canBeCodeBlock( model.schema, root ); + + expect( root.is( 'rootElement' ) ).to.be.true; + expect( testResult ).to.be.false; + } ); + + it( 'should not allow a limit element to become a code block', () => { + const model = new Model(); + const root = model.document.createRoot(); + + model.schema.register( 'limitElement', { isObject: true } ); + + model.change( writer => { + const limitElement = writer.createElement( 'limitElement' ); + + writer.insert( limitElement, root, 0 ); + } ); + + const limitElement = root.getChild( 0 ); + + const testResult = canBeCodeBlock( model.schema, limitElement ); + + expect( model.schema.isLimit( limitElement ) ).to.be.true; + expect( testResult ).to.be.false; + } ); + + it( 'should not allow an element to become a code block if it isnt allowed in parent', () => { + const model = new Model(); + const root = model.document.createRoot(); + + model.schema.register( 'container', { allowIn: '$root' } ); + model.schema.register( 'fooElement', { allowIn: 'container' } ); + model.schema.register( 'codeBlock', { } ); + + let fooElement; + let containerElement; + + model.change( writer => { + containerElement = writer.createElement( 'container' ); + fooElement = writer.createElement( 'codeBlock' ); + + writer.insert( containerElement, root, 0 ); + writer.insert( fooElement, containerElement, 0 ); + } ); + + const testResult = canBeCodeBlock( model.schema, fooElement ); + + expect( testResult ).to.be.false; + } ); + + it( 'should allow an element to become a code block if it is allowed in parent', () => { + const model = new Model(); + const root = model.document.createRoot(); + + model.schema.register( 'container', { allowIn: '$root' } ); + model.schema.register( 'fooElement', { allowIn: 'container' } ); + model.schema.register( 'codeBlock', { allowIn: 'container' } ); + + let fooElement; + let containerElement; + + model.change( writer => { + containerElement = writer.createElement( 'container' ); + fooElement = writer.createElement( 'codeBlock' ); + + writer.insert( containerElement, root, 0 ); + writer.insert( fooElement, containerElement, 0 ); + } ); + + const testResult = canBeCodeBlock( model.schema, fooElement ); + + expect( testResult ).to.be.true; + } ); + } ); +} ); From 069686130f7c6c6ede07a64adfa2ccd2911c35a3 Mon Sep 17 00:00:00 2001 From: Andrzej Stanek Date: Fri, 10 Jun 2022 14:08:47 +0200 Subject: [PATCH 4/5] Add tests ensuring insertContent() and deleteContent() in code block indent/outdent command won't be reverted. --- .../tests/indentcodeblockcommand.js | 17 +++++++++++++++ .../tests/outdentcodeblockcommand.js | 21 +++++++++++++++++++ packages/ckeditor5-code-block/tests/utils.js | 2 +- 3 files changed, 39 insertions(+), 1 deletion(-) diff --git a/packages/ckeditor5-code-block/tests/indentcodeblockcommand.js b/packages/ckeditor5-code-block/tests/indentcodeblockcommand.js index 84ca726ea2d..6f8cbbbea92 100644 --- a/packages/ckeditor5-code-block/tests/indentcodeblockcommand.js +++ b/packages/ckeditor5-code-block/tests/indentcodeblockcommand.js @@ -13,10 +13,13 @@ import BlockQuoteEditing from '@ckeditor/ckeditor5-block-quote/src/blockquoteedi import ModelTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/modeltesteditor'; import { getData as getModelData, setData as setModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model'; +import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils'; describe( 'IndentCodeBlockCommand', () => { let editor, model, indentCommand; + testUtils.createSinonSandbox(); + beforeEach( () => { return ModelTestEditor .create( { @@ -143,6 +146,20 @@ describe( 'IndentCodeBlockCommand', () => { ' f[oo b]ar' ); } ); + // Need to ensure that insertContent() will not be reverted to model.change() to not break integration + // with Track Changes. + it( 'should insert indent with insertContent()', () => { + const insertContentSpy = sinon.spy( model, 'insertContent' ); + const modelChangeSpy = sinon.spy( model, 'change' ); + + setModelData( model, '[]Foo' ); + + indentCommand.execute(); + + expect( insertContentSpy.calledOnce ).to.be.true; + expect( modelChangeSpy.calledAfter( insertContentSpy ) ).to.be.true; + } ); + describe( 'config.codeBlock.indentSequence', () => { it( 'should be used when indenting', () => { return ModelTestEditor diff --git a/packages/ckeditor5-code-block/tests/outdentcodeblockcommand.js b/packages/ckeditor5-code-block/tests/outdentcodeblockcommand.js index 099dabe003f..84528bfeba1 100644 --- a/packages/ckeditor5-code-block/tests/outdentcodeblockcommand.js +++ b/packages/ckeditor5-code-block/tests/outdentcodeblockcommand.js @@ -13,10 +13,13 @@ import BlockQuoteEditing from '@ckeditor/ckeditor5-block-quote/src/blockquoteedi import ModelTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/modeltesteditor'; import { getData as getModelData, setData as setModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model'; +import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils'; describe( 'OutdentCodeBlockCommand', () => { let editor, model, outdentCommand; + testUtils.createSinonSandbox(); + beforeEach( () => { return ModelTestEditor .create( { @@ -278,6 +281,24 @@ describe( 'OutdentCodeBlockCommand', () => { expect( getModelData( model ) ).to.equal( 'f[ooba]r' ); } ); + // Need to ensure that deleteContent() will not be reverted to model.change() to not break integration + // with Track Changes. + it( 'should outdent with deleteContent()', () => { + const deleteContentSpy = sinon.spy( model, 'deleteContent' ); + const modelChangeSpy = sinon.spy( model, 'change' ); + + setModelData( model, '[]Foo' ); + + model.change( writer => { + writer.insertText( ' ', model.document.getRoot().getChild( 0 ), 0 ); + } ); + + outdentCommand.execute(); + + expect( deleteContentSpy.calledOnce ).to.be.true; + expect( modelChangeSpy.calledAfter( deleteContentSpy ) ).to.be.true; + } ); + describe( 'config.codeBlock.indentSequence', () => { it( 'should be respected', () => { return ModelTestEditor diff --git a/packages/ckeditor5-code-block/tests/utils.js b/packages/ckeditor5-code-block/tests/utils.js index 4bf84fed43c..4ea9e22636b 100644 --- a/packages/ckeditor5-code-block/tests/utils.js +++ b/packages/ckeditor5-code-block/tests/utils.js @@ -7,7 +7,7 @@ import Model from '@ckeditor/ckeditor5-engine/src/model/model'; import { canBeCodeBlock } from '../src/utils'; -describe( 'utils', () => { +describe( 'CodeBlock - utils', () => { describe( 'canBecomeCodeBlock()', () => { it( 'should not allow a root element to become a code block', () => { const model = new Model(); From e56ec8a2ab41701bd8f81fd792083e66b1ae15b2 Mon Sep 17 00:00:00 2001 From: Kuba Niegowski <1232187+niegowski@users.noreply.github.com> Date: Fri, 10 Jun 2022 16:31:58 +0200 Subject: [PATCH 5/5] Apply review comment. --- packages/ckeditor5-code-block/src/indentcodeblockcommand.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/ckeditor5-code-block/src/indentcodeblockcommand.js b/packages/ckeditor5-code-block/src/indentcodeblockcommand.js index 3ee034c6982..b264d8f4a81 100644 --- a/packages/ckeditor5-code-block/src/indentcodeblockcommand.js +++ b/packages/ckeditor5-code-block/src/indentcodeblockcommand.js @@ -82,7 +82,7 @@ export default class IndentCodeBlockCommand extends Command { // integration of code block with track changes. It's the easiest way of integration because insertContent() // is already integrated with track changes, but if it ever cause any troubles it can be reverted, however // some additional work will be required in track changes integration of code block. - editor.model.insertContent( indentSequenceTextElement, position ); + model.insertContent( indentSequenceTextElement, position ); } } ); }