Skip to content
This repository has been archived by the owner on Jun 26, 2020. It is now read-only.

Commit

Permalink
Merge pull request #54 from ckeditor/t/53
Browse files Browse the repository at this point in the history 
Fix: Fixed integration with undo. Closes #53.
  • Loading branch information
@scofalik
scofalik authored Mar 12, 2018
2 parents 62e2627 + 9024f2b commit f5d68f4
Show file tree
Hide file tree
Showing 5 changed files with 258 additions and 110 deletions.
43 changes: 22 additions & 21 deletions src/blockautoformatediting.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 } );
} );
} );
}
}
103 changes: 42 additions & 61 deletions src/inlineautoformatediting.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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 );
}
} );
} );
}
}
Expand All @@ -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.<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 ] ) );
}
2 changes: 1 addition & 1 deletion tests/autoformat.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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, '<paragraph>>[]</paragraph>' );
model.change( writer => {
writer.insertText( ' ', doc.selection.getFirstPosition() );
Expand Down
27 changes: 0 additions & 27 deletions tests/inlineautoformatediting.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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, '<paragraph>*foobar[]</paragraph>' );

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' ) );
}
} );
} );
} );
Loading

0 comments on commit f5d68f4

Please sign in to comment.