diff --git a/src/blockautoformatediting.js b/src/blockautoformatediting.js index fdee5e5..5e8d5e0 100644 --- a/src/blockautoformatediting.js +++ b/src/blockautoformatediting.js @@ -64,34 +64,35 @@ export default class BlockAutoformatEditing { } editor.model.document.on( 'change', () => { - const changes = editor.model.document.differ.getChanges(); + const changes = Array.from( editor.model.document.differ.getChanges() ); + const entry = changes[ 0 ]; - for ( const entry of changes ) { - if ( entry.type == 'insert' && entry.name == '$text' ) { - const item = entry.position.textNode || entry.position.nodeAfter; + // Typing is represented by only a single change. + if ( changes.length != 1 || entry.type !== 'insert' || entry.name != '$text' || entry.length != 1 ) { + return; + } + const item = entry.position.textNode || entry.position.nodeAfter; - if ( !item.parent.is( 'paragraph' ) ) { - continue; - } + if ( !item.parent.is( 'paragraph' ) ) { + return; + } - const match = pattern.exec( item.data ); + const match = pattern.exec( item.data ); - if ( !match ) { - continue; - } + if ( !match ) { + return; + } - // Use enqueueChange to create new batch to separate typing batch from the auto-format changes. - editor.model.enqueueChange( writer => { - // Matched range. - const range = Range.createFromParentsAndOffsets( item.parent, 0, item.parent, match[ 0 ].length ); + // Use enqueueChange to create new batch to separate typing batch from the auto-format changes. + editor.model.enqueueChange( writer => { + // Matched range. + const range = Range.createFromParentsAndOffsets( item.parent, 0, item.parent, match[ 0 ].length ); - // Remove matched text. - writer.remove( range ); + // Remove matched text. + writer.remove( range ); - callback( { match } ); - } ); - } - } + callback( { match } ); + } ); } ); } } diff --git a/src/inlineautoformatediting.js b/src/inlineautoformatediting.js index 14bf9f5..9dcb21c 100644 --- a/src/inlineautoformatediting.js +++ b/src/inlineautoformatediting.js @@ -7,7 +7,7 @@ * @module autoformat/inlineautoformatediting */ -import LiveRange from '@ckeditor/ckeditor5-engine/src/model/liverange'; +import ModelRange from '@ckeditor/ckeditor5-engine/src/model/range'; /** * The inline autoformatting engine. It allows to format various inline patterns. For example, @@ -141,74 +141,43 @@ export default class InlineAutoformatEditing { } ); editor.model.document.on( 'change', () => { - const changes = editor.model.document.differ.getChanges(); + const selection = editor.model.document.selection; - for ( const entry of changes ) { - if ( entry.type != 'insert' || entry.name != '$text' ) { - continue; - } - - const selection = editor.model.document.selection; + // Do nothing if selection is not collapsed. + if ( !selection.isCollapsed ) { + return; + } - if ( !selection.isCollapsed || !selection.focus || !selection.focus.parent ) { - continue; - } + const changes = Array.from( editor.model.document.differ.getChanges() ); + const entry = changes[ 0 ]; - const block = selection.focus.parent; - const text = getText( block ).slice( 0, selection.focus.offset ); - const ranges = testCallback( text ); - const rangesToFormat = []; - - // Apply format before deleting text. - ranges.format.forEach( range => { - if ( range[ 0 ] === undefined || range[ 1 ] === undefined ) { - return; - } - - rangesToFormat.push( LiveRange.createFromParentsAndOffsets( - block, range[ 0 ], - block, range[ 1 ] - ) ); - } ); - - const rangesToRemove = []; - - // Reverse order to not mix the offsets while removing. - ranges.remove.slice().reverse().forEach( range => { - if ( range[ 0 ] === undefined || range[ 1 ] === undefined ) { - return; - } - - rangesToRemove.push( LiveRange.createFromParentsAndOffsets( - block, range[ 0 ], - block, range[ 1 ] - ) ); - } ); - - if ( !( rangesToFormat.length && rangesToRemove.length ) ) { - continue; - } + // Typing is represented by only a single change. + if ( changes.length != 1 || entry.type !== 'insert' || entry.name != '$text' || entry.length != 1 ) { + return; + } - // Use enqueueChange to create new batch to separate typing batch from the auto-format changes. - editor.model.enqueueChange( writer => { - const validRanges = editor.model.schema.getValidRanges( rangesToFormat, attributeKey ); + const block = selection.focus.parent; + const text = getText( block ).slice( 0, selection.focus.offset ); + const testOutput = testCallback( text ); + const rangesToFormat = testOutputToRanges( block, testOutput.format ); + const rangesToRemove = testOutputToRanges( block, testOutput.remove ); - // Apply format. - formatCallback( writer, validRanges ); + if ( !( rangesToFormat.length && rangesToRemove.length ) ) { + return; + } - // Detach ranges used to apply Autoformat. Prevents memory leaks. #39 - rangesToFormat.forEach( range => range.detach() ); + // Use enqueueChange to create new batch to separate typing batch from the auto-format changes. + editor.model.enqueueChange( writer => { + const validRanges = editor.model.schema.getValidRanges( rangesToFormat, attributeKey ); - // Remove delimiters. - for ( const range of rangesToRemove ) { - writer.remove( range ); + // Apply format. + formatCallback( writer, validRanges ); - // Prevents memory leaks. - // https://github.com/ckeditor/ckeditor5-autoformat/issues/39 - range.detach(); - } - } ); - } + // Remove delimiters - use reversed order to not mix the offsets while removing. + for ( const range of rangesToRemove.reverse() ) { + writer.remove( range ); + } + } ); } ); } } @@ -221,3 +190,15 @@ export default class InlineAutoformatEditing { function getText( element ) { return Array.from( element.getChildren() ).reduce( ( a, b ) => a + b.data, '' ); } + +// Converts output of the test function provided to the InlineAutoformatEditing and converts it to the model ranges +// inside provided block. +// +// @private +// @param {module:engine/model/element~Element} block +// @param {Array.} arrays +function testOutputToRanges( block, arrays ) { + return arrays + .filter( array => ( array[ 0 ] !== undefined && array[ 1 ] !== undefined ) ) + .map( array => ModelRange.createFromParentsAndOffsets( block, array[ 0 ], block, array[ 1 ] ) ); +} diff --git a/tests/autoformat.js b/tests/autoformat.js index 82347a4..b4f08e0 100644 --- a/tests/autoformat.js +++ b/tests/autoformat.js @@ -190,7 +190,7 @@ describe( 'Autoformat', () => { } ); describe( 'Block quote', () => { - it( 'should replace greater-than character with heading', () => { + it( 'should replace greater-than character with block quote', () => { setData( model, '>[]' ); model.change( writer => { writer.insertText( ' ', doc.selection.getFirstPosition() ); diff --git a/tests/inlineautoformatediting.js b/tests/inlineautoformatediting.js index aabada9..b2b8ddf 100644 --- a/tests/inlineautoformatediting.js +++ b/tests/inlineautoformatediting.js @@ -115,32 +115,5 @@ describe( 'InlineAutoformatEditing', () => { sinon.assert.notCalled( formatSpy ); } ); - - it( 'should detach removed ranges', () => { - const detachSpies = []; - const callback = fixBatch => testUtils.sinon.stub( fixBatch, 'remove' ).callsFake( saveDetachSpy ); - testUtils.sinon.stub( editor.model.schema, 'getValidRanges' ) - .callThrough() - .callsFake( ranges => ranges.map( saveDetachSpy ) ); - - new InlineAutoformatEditing( editor, /(\*)(.+?)(\*)/g, callback ); // eslint-disable-line no-new - - setData( model, '*foobar[]' ); - - model.change( writer => { - writer.insertText( '*', doc.selection.getFirstPosition() ); - } ); - - // There should be two removed ranges and one range used to apply autoformat. - expect( detachSpies ).to.have.length( 3 ); - - for ( const spy of detachSpies ) { - testUtils.sinon.assert.calledOnce( spy ); - } - - function saveDetachSpy( range ) { - detachSpies.push( testUtils.sinon.spy( range, 'detach' ) ); - } - } ); } ); } ); diff --git a/tests/undointegration.js b/tests/undointegration.js new file mode 100644 index 0000000..6a08a50 --- /dev/null +++ b/tests/undointegration.js @@ -0,0 +1,193 @@ +/** + * @license Copyright (c) 2003-2018, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md. + */ + +import Autoformat from '../src/autoformat'; + +import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph'; +import ListEditing from '@ckeditor/ckeditor5-list/src/listediting'; +import HeadingEditing from '@ckeditor/ckeditor5-heading/src/headingediting'; +import BoldEditing from '@ckeditor/ckeditor5-basic-styles/src/bold/boldediting'; +import CodeEditing from '@ckeditor/ckeditor5-basic-styles/src/code/codeediting'; +import ItalicEditing from '@ckeditor/ckeditor5-basic-styles/src/italic/italicediting'; +import BlockQuoteEditing from '@ckeditor/ckeditor5-block-quote/src/blockquoteediting'; +import Enter from '@ckeditor/ckeditor5-enter/src/enter'; +import Undo from '@ckeditor/ckeditor5-undo/src/undoediting'; + +import VirtualTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/virtualtesteditor'; + +import { setData, getData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model'; +import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils'; + +testUtils.createSinonSandbox(); + +describe( 'Autoformat', () => { + let editor, model, doc; + + beforeEach( () => { + return VirtualTestEditor + .create( { + plugins: [ + Enter, + Undo, + Paragraph, + Autoformat, + ListEditing, + HeadingEditing, + BoldEditing, + ItalicEditing, + CodeEditing, + BlockQuoteEditing + ] + } ) + .then( newEditor => { + editor = newEditor; + model = editor.model; + doc = model.document; + } ); + } ); + + afterEach( () => { + return editor.destroy(); + } ); + + describe( 'inline', () => { + it( 'should undo replacing "**" with bold', () => { + setData( model, '**foobar*[]' ); + model.change( writer => { + writer.insertText( '*', doc.selection.getFirstPosition() ); + } ); + + expect( getData( model ) ).to.equal( '<$text bold="true">foobar[]' ); + editor.execute( 'undo' ); + expect( getData( model ) ).to.equal( '**foobar**[]' ); + } ); + + it( 'should undo replacing "__" with bold', () => { + setData( model, '__foobar_[]' ); + model.change( writer => { + writer.insertText( '_', doc.selection.getFirstPosition() ); + } ); + + expect( getData( model ) ).to.equal( '<$text bold="true">foobar[]' ); + editor.execute( 'undo' ); + expect( getData( model ) ).to.equal( '__foobar__[]' ); + } ); + + it( 'should undo replacing "*" with italic', () => { + setData( model, '*foobar[]' ); + model.change( writer => { + writer.insertText( '*', doc.selection.getFirstPosition() ); + } ); + + expect( getData( model ) ).to.equal( '<$text italic="true">foobar[]' ); + editor.execute( 'undo' ); + expect( getData( model ) ).to.equal( '*foobar*[]' ); + } ); + + it( 'should undo replacing "_" with italic', () => { + setData( model, '_foobar[]' ); + model.change( writer => { + writer.insertText( '_', doc.selection.getFirstPosition() ); + } ); + + expect( getData( model ) ).to.equal( '<$text italic="true">foobar[]' ); + editor.execute( 'undo' ); + expect( getData( model ) ).to.equal( '_foobar_[]' ); + } ); + + it( 'should undo replacing "`" with code', () => { + setData( model, '`foobar[]' ); + model.change( writer => { + writer.insertText( '`', doc.selection.getFirstPosition() ); + } ); + + expect( getData( model ) ).to.equal( '<$text code="true">foobar[]' ); + editor.execute( 'undo' ); + expect( getData( model ) ).to.equal( '`foobar`[]' ); + } ); + } ); + + describe( 'block', () => { + it( 'should work when replacing asterisk', () => { + setData( model, '*[]' ); + model.change( writer => { + writer.insertText( ' ', doc.selection.getFirstPosition() ); + } ); + + expect( getData( model ) ).to.equal( '[]' ); + + editor.execute( 'undo' ); + expect( getData( model ) ).to.equal( '* []' ); + } ); + + it( 'should work when replacing minus character', () => { + setData( model, '-[]' ); + model.change( writer => { + writer.insertText( ' ', doc.selection.getFirstPosition() ); + } ); + + expect( getData( model ) ).to.equal( '[]' ); + + editor.execute( 'undo' ); + expect( getData( model ) ).to.equal( '- []' ); + } ); + + it( 'should work when replacing digit with numbered list item using the dot format', () => { + setData( model, '1.[]' ); + model.change( writer => { + writer.insertText( ' ', doc.selection.getFirstPosition() ); + } ); + + expect( getData( model ) ).to.equal( '[]' ); + + editor.execute( 'undo' ); + expect( getData( model ) ).to.equal( '1. []' ); + } ); + + it( 'should work when replacing digit with numbered list item using the parenthesis format', () => { + setData( model, '1)[]' ); + model.change( writer => { + writer.insertText( ' ', doc.selection.getFirstPosition() ); + } ); + + expect( getData( model ) ).to.equal( '[]' ); + editor.execute( 'undo' ); + expect( getData( model ) ).to.equal( '1) []' ); + } ); + + it( 'should work when replacing hash character with heading', () => { + setData( model, '#[]' ); + model.change( writer => { + writer.insertText( ' ', doc.selection.getFirstPosition() ); + } ); + + expect( getData( model ) ).to.equal( '[]' ); + editor.execute( 'undo' ); + expect( getData( model ) ).to.equal( '# []' ); + } ); + + it( 'should work when replacing two hash characters with heading level 2', () => { + setData( model, '##[]' ); + model.change( writer => { + writer.insertText( ' ', doc.selection.getFirstPosition() ); + } ); + + expect( getData( model ) ).to.equal( '[]' ); + editor.execute( 'undo' ); + expect( getData( model ) ).to.equal( '## []' ); + } ); + + it( 'should work when replacing greater-than character with block quote', () => { + setData( model, '>[]' ); + model.change( writer => { + writer.insertText( ' ', doc.selection.getFirstPosition() ); + } ); + + expect( getData( model ) ).to.equal( '
[]
' ); + editor.execute( 'undo' ); + expect( getData( model ) ).to.equal( '> []' ); + } ); + } ); +} );