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 #44 from ckeditor/t/ckeditor5-engine/1207
Browse files Browse the repository at this point in the history 
Other: Callbacks should listen to new `model.Document#event:change` and use `model.Document#differ`.
  • Loading branch information
Piotr Jasiun committed Jan 11, 2018
2 parents ea3d7af + 28c8b3d commit 2d83221
Show file tree
Hide file tree
Showing 4 changed files with 79 additions and 140 deletions.
55 changes: 21 additions & 34 deletions src/blockautoformatengine.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
*/

import Range from '@ckeditor/ckeditor5-engine/src/model/range';
import TextProxy from '@ckeditor/ckeditor5-engine/src/model/textproxy';

/**
* The block autoformatting engine. It allows to format various block patterns. For example,
Expand Down Expand Up @@ -64,47 +63,35 @@ export default class BlockAutoformatEngine {
};
}

editor.model.document.on( 'change', ( event, type, changes, batch ) => {
if ( batch.type == 'transparent' ) {
return;
}
editor.model.document.on( 'change', () => {
const changes = editor.model.document.differ.getChanges();

if ( type != 'insert' ) {
return;
}
for ( const entry of changes ) {
if ( entry.type == 'insert' && entry.name == '$text' ) {
const item = entry.position.textNode || entry.position.nodeAfter;

// Take the first element. Typing shouldn't add more than one element at once.
// And if it is not typing (e.g. paste), Autoformat should not be fired.
const value = changes.range.getItems().next().value;
if ( !item.parent.is( 'paragraph' ) ) {
continue;
}

if ( !( value instanceof TextProxy ) ) {
return;
}
const match = pattern.exec( item.data );

const textNode = value.textNode;
const text = textNode.data;
if ( !match ) {
continue;
}

// Run matching only on non-empty paragraphs.
if ( textNode.parent.name !== 'paragraph' || !text ) {
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 );

const match = pattern.exec( text );
// Remove matched text.
writer.remove( range );

if ( !match ) {
return;
callback( { match } );
} );
}
}

// Use enqueueChange to create new batch to separate typing batch from the autoformat changes.
editor.model.enqueueChange( writer => {
// Matched range.
const range = Range.createFromParentsAndOffsets( textNode.parent, 0, textNode.parent, match[ 0 ].length );

// Remove matched text.
writer.remove( range );

callback( { match } );
} );
} );
}
}
116 changes: 58 additions & 58 deletions src/inlineautoformatengine.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -136,75 +136,75 @@ export default class InlineAutoformatEngine {
}
} );

editor.model.document.on( 'change', ( evt, type, changes, batch ) => {
if ( batch.type == 'transparent' ) {
return;
}

if ( type !== 'insert' ) {
return;
}

const selection = editor.model.document.selection;

if ( !selection.isCollapsed || !selection.focus || !selection.focus.parent ) {
return;
}

const block = selection.focus.parent;
const text = getText( block ).slice( 0, selection.focus.offset );
const ranges = testCallback( text );
const rangesToFormat = [];
editor.model.document.on( 'change', () => {
const changes = editor.model.document.differ.getChanges();

// Apply format before deleting text.
ranges.format.forEach( range => {
if ( range[ 0 ] === undefined || range[ 1 ] === undefined ) {
return;
for ( const entry of changes ) {
if ( entry.type != 'insert' || entry.name != '$text' ) {
continue;
}

rangesToFormat.push( LiveRange.createFromParentsAndOffsets(
block, range[ 0 ],
block, range[ 1 ]
) );
} );
const selection = editor.model.document.selection;

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;
if ( !selection.isCollapsed || !selection.focus || !selection.focus.parent ) {
continue;
}

rangesToRemove.push( LiveRange.createFromParentsAndOffsets(
block, range[ 0 ],
block, range[ 1 ]
) );
} );

if ( !( rangesToFormat.length && rangesToRemove.length ) ) {
return;
}
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;
}

// Use enqueueChange to create new batch to separate typing batch from the autoformat changes.
editor.model.enqueueChange( writer => {
const validRanges = editor.model.schema.getValidRanges( rangesToFormat, command );
// 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, command );

// Apply format.
formatCallback( writer, validRanges );
// Apply format.
formatCallback( writer, validRanges );

// Detach ranges used to apply Autoformat. Prevents memory leaks. #39
rangesToFormat.forEach( range => range.detach() );
// Detach ranges used to apply Autoformat. Prevents memory leaks. #39
rangesToFormat.forEach( range => range.detach() );

// Remove delimiters.
for ( const range of rangesToRemove ) {
writer.remove( range );
// Remove delimiters.
for ( const range of rangesToRemove ) {
writer.remove( range );

// Prevents memory leaks.
// https://github.com/ckeditor/ckeditor5-autoformat/issues/39
range.detach();
}
} );
// Prevents memory leaks.
// https://github.com/ckeditor/ckeditor5-autoformat/issues/39
range.detach();
}
} );
}
} );
}
}
Expand Down
25 changes: 0 additions & 25 deletions tests/blockautoformatengine.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,19 +42,6 @@ describe( 'BlockAutoformatEngine', () => {
sinon.assert.calledOnce( spy );
} );

it( 'should not run a command when changes are in transparent batch', () => {
const spy = testUtils.sinon.spy();
editor.commands.add( 'testCommand', new TestCommand( editor, spy ) );
new BlockAutoformatEngine( editor, /^[*]\s$/, 'testCommand' ); // eslint-disable-line no-new

setData( model, '<paragraph>*[]</paragraph>' );
model.enqueueChange( 'transparent', writer => {
writer.insertText( ' ', doc.selection.getFirstPosition() );
} );

sinon.assert.notCalled( spy );
} );

it( 'should remove found pattern', () => {
const spy = testUtils.sinon.spy();
editor.commands.add( 'testCommand', new TestCommand( editor, spy ) );
Expand Down Expand Up @@ -83,18 +70,6 @@ describe( 'BlockAutoformatEngine', () => {
sinon.assert.calledOnce( spy );
} );

it( 'should not run a callback when changes are in transparent batch', () => {
const spy = testUtils.sinon.spy();
new BlockAutoformatEngine( editor, /^[*]\s$/, spy ); // eslint-disable-line no-new

setData( model, '<paragraph>*[]</paragraph>' );
model.enqueueChange( 'transparent', writer => {
writer.insertText( ' ', doc.selection.getFirstPosition() );
} );

sinon.assert.notCalled( spy );
} );

it( 'should ignore other delta operations', () => {
const spy = testUtils.sinon.spy();
new BlockAutoformatEngine( editor, /^[*]\s/, spy ); // eslint-disable-line no-new
Expand Down
23 changes: 0 additions & 23 deletions tests/inlineautoformatengine.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,17 +52,6 @@ describe( 'InlineAutoformatEngine', () => {
expect( getData( model ) ).to.equal( '<paragraph><$text testAttribute="true">foobar</$text>[]</paragraph>' );
} );

it( 'should not apply an attribute when changes are in transparent batch', () => {
new InlineAutoformatEngine( editor, /(\*)(.+?)(\*)/g, 'testAttribute' ); // eslint-disable-line no-new

setData( model, '<paragraph>*foobar[]</paragraph>' );
model.enqueueChange( 'transparent', writer => {
writer.insertText( '*', doc.selection.getFirstPosition() );
} );

expect( getData( model ) ).to.equal( '<paragraph>*foobar*[]</paragraph>' );
} );

it( 'should stop early if selection is not collapsed', () => {
new InlineAutoformatEngine( editor, /(\*)(.+?)\*/g, 'testAttribute' ); // eslint-disable-line no-new

Expand All @@ -76,18 +65,6 @@ describe( 'InlineAutoformatEngine', () => {
} );

describe( 'Callback', () => {
it( 'should not run a callback when changes are in transparent batch', () => {
const spy = testUtils.sinon.spy();
new InlineAutoformatEngine( editor, /(\*)(.+?)(\*)/g, spy ); // eslint-disable-line no-new

setData( model, '<paragraph>*foobar[]</paragraph>' );
model.enqueueChange( 'transparent', writer => {
writer.insertText( '*', doc.selection.getFirstPosition() );
} );

sinon.assert.notCalled( spy );
} );

it( 'should stop when there are no format ranges returned from testCallback', () => {
const formatSpy = testUtils.sinon.spy();
const testStub = testUtils.sinon.stub().returns( {
Expand Down

0 comments on commit 2d83221

Please sign in to comment.