Skip to content

Commit

Permalink
Merge pull request #11862 from ckeditor/cf/3194-integrate-code-blocks
Browse files Browse the repository at this point in the history
Other (code-block): The `IndentCodeBlockCommand` and the `OutdentCodeBlockCommand` are now using `model.insertContent()` and `model.deleteContent()` for easier extendability.
  • Loading branch information
niegowski authored Jun 10, 2022
2 parents d0ca32c + e56ec8a commit dd6cf1c
Show file tree
Hide file tree
Showing 7 changed files with 158 additions and 13 deletions.
10 changes: 1 addition & 9 deletions packages/ckeditor5-code-block/src/codeblockcommand.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -179,14 +179,6 @@ export default class CodeBlockCommand extends Command {
}
}

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
Expand Down
8 changes: 7 additions & 1 deletion packages/ckeditor5-code-block/src/indentcodeblockcommand.js
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,13 @@ export default class IndentCodeBlockCommand extends Command {
// </codeBlock>
//
for ( const position of positions ) {
writer.insertText( this._indentSequence, position );
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.
model.insertContent( indentSequenceTextElement, position );
}
} );
}
Expand Down
10 changes: 7 additions & 3 deletions packages/ckeditor5-code-block/src/outdentcodeblockcommand.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 (" "):
Expand All @@ -76,10 +76,14 @@ export default class OutdentCodeBlockCommand extends Command {
// bazqux
// </codeBlock>
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 );
// 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 ) );
}
}
} );
Expand Down
15 changes: 15 additions & 0 deletions packages/ckeditor5-code-block/src/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -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' );
}
17 changes: 17 additions & 0 deletions packages/ckeditor5-code-block/tests/indentcodeblockcommand.js
Original file line number Diff line number Diff line change
Expand Up @@ -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( {
Expand Down Expand Up @@ -143,6 +146,20 @@ describe( 'IndentCodeBlockCommand', () => {
'<codeBlock language="foo"> f[oo<softBreak></softBreak> b]ar</codeBlock>' );
} );

// 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, '<codeBlock language="foo">[]Foo</codeBlock>' );

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
Expand Down
21 changes: 21 additions & 0 deletions packages/ckeditor5-code-block/tests/outdentcodeblockcommand.js
Original file line number Diff line number Diff line change
Expand Up @@ -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( {
Expand Down Expand Up @@ -278,6 +281,24 @@ describe( 'OutdentCodeBlockCommand', () => {
expect( getModelData( model ) ).to.equal( '<codeBlock language="foo">f[oo<softBreak></softBreak>ba]r</codeBlock>' );
} );

// 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, '<codeBlock language="foo">[]Foo</codeBlock>' );

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
Expand Down
90 changes: 90 additions & 0 deletions packages/ckeditor5-code-block/tests/utils.js
Original file line number Diff line number Diff line change
@@ -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( 'CodeBlock - 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;
} );
} );
} );

0 comments on commit dd6cf1c

Please sign in to comment.