浏览代码

Merge pull request #54 from ckeditor/t/53

Fix: Fixed integration with undo. Closes #53.
Szymon Cofalik 7 年之前
父节点
当前提交
79ddf0e52d

+ 22 - 21
packages/ckeditor5-autoformat/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 } );
+			} );
 		} );
 	}
 }

+ 42 - 61
packages/ckeditor5-autoformat/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.<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 ] ) );
+}

+ 1 - 1
packages/ckeditor5-autoformat/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, '<paragraph>>[]</paragraph>' );
 			model.change( writer => {
 				writer.insertText( ' ', doc.selection.getFirstPosition() );

+ 0 - 27
packages/ckeditor5-autoformat/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, '<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' ) );
-			}
-		} );
 	} );
 } );

+ 193 - 0
packages/ckeditor5-autoformat/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, '<paragraph>**foobar*[]</paragraph>' );
+			model.change( writer => {
+				writer.insertText( '*', doc.selection.getFirstPosition() );
+			} );
+
+			expect( getData( model ) ).to.equal( '<paragraph><$text bold="true">foobar</$text>[]</paragraph>' );
+			editor.execute( 'undo' );
+			expect( getData( model ) ).to.equal( '<paragraph>**foobar**[]</paragraph>' );
+		} );
+
+		it( 'should undo replacing "__" with bold', () => {
+			setData( model, '<paragraph>__foobar_[]</paragraph>' );
+			model.change( writer => {
+				writer.insertText( '_', doc.selection.getFirstPosition() );
+			} );
+
+			expect( getData( model ) ).to.equal( '<paragraph><$text bold="true">foobar</$text>[]</paragraph>' );
+			editor.execute( 'undo' );
+			expect( getData( model ) ).to.equal( '<paragraph>__foobar__[]</paragraph>' );
+		} );
+
+		it( 'should undo replacing "*" with italic', () => {
+			setData( model, '<paragraph>*foobar[]</paragraph>' );
+			model.change( writer => {
+				writer.insertText( '*', doc.selection.getFirstPosition() );
+			} );
+
+			expect( getData( model ) ).to.equal( '<paragraph><$text italic="true">foobar</$text>[]</paragraph>' );
+			editor.execute( 'undo' );
+			expect( getData( model ) ).to.equal( '<paragraph>*foobar*[]</paragraph>' );
+		} );
+
+		it( 'should undo replacing "_" with italic', () => {
+			setData( model, '<paragraph>_foobar[]</paragraph>' );
+			model.change( writer => {
+				writer.insertText( '_', doc.selection.getFirstPosition() );
+			} );
+
+			expect( getData( model ) ).to.equal( '<paragraph><$text italic="true">foobar</$text>[]</paragraph>' );
+			editor.execute( 'undo' );
+			expect( getData( model ) ).to.equal( '<paragraph>_foobar_[]</paragraph>' );
+		} );
+
+		it( 'should undo replacing "`" with code', () => {
+			setData( model, '<paragraph>`foobar[]</paragraph>' );
+			model.change( writer => {
+				writer.insertText( '`', doc.selection.getFirstPosition() );
+			} );
+
+			expect( getData( model ) ).to.equal( '<paragraph><$text code="true">foobar</$text>[]</paragraph>' );
+			editor.execute( 'undo' );
+			expect( getData( model ) ).to.equal( '<paragraph>`foobar`[]</paragraph>' );
+		} );
+	} );
+
+	describe( 'block', () => {
+		it( 'should work when replacing asterisk', () => {
+			setData( model, '<paragraph>*[]</paragraph>' );
+			model.change( writer => {
+				writer.insertText( ' ', doc.selection.getFirstPosition() );
+			} );
+
+			expect( getData( model ) ).to.equal( '<listItem indent="0" type="bulleted">[]</listItem>' );
+
+			editor.execute( 'undo' );
+			expect( getData( model ) ).to.equal( '<paragraph>* []</paragraph>' );
+		} );
+
+		it( 'should work when replacing minus character', () => {
+			setData( model, '<paragraph>-[]</paragraph>' );
+			model.change( writer => {
+				writer.insertText( ' ', doc.selection.getFirstPosition() );
+			} );
+
+			expect( getData( model ) ).to.equal( '<listItem indent="0" type="bulleted">[]</listItem>' );
+
+			editor.execute( 'undo' );
+			expect( getData( model ) ).to.equal( '<paragraph>- []</paragraph>' );
+		} );
+
+		it( 'should work when replacing digit with numbered list item using the dot format', () => {
+			setData( model, '<paragraph>1.[]</paragraph>' );
+			model.change( writer => {
+				writer.insertText( ' ', doc.selection.getFirstPosition() );
+			} );
+
+			expect( getData( model ) ).to.equal( '<listItem indent="0" type="numbered">[]</listItem>' );
+
+			editor.execute( 'undo' );
+			expect( getData( model ) ).to.equal( '<paragraph>1. []</paragraph>' );
+		} );
+
+		it( 'should work when replacing digit with numbered list item using the parenthesis format', () => {
+			setData( model, '<paragraph>1)[]</paragraph>' );
+			model.change( writer => {
+				writer.insertText( ' ', doc.selection.getFirstPosition() );
+			} );
+
+			expect( getData( model ) ).to.equal( '<listItem indent="0" type="numbered">[]</listItem>' );
+			editor.execute( 'undo' );
+			expect( getData( model ) ).to.equal( '<paragraph>1) []</paragraph>' );
+		} );
+
+		it( 'should work when replacing hash character with heading', () => {
+			setData( model, '<paragraph>#[]</paragraph>' );
+			model.change( writer => {
+				writer.insertText( ' ', doc.selection.getFirstPosition() );
+			} );
+
+			expect( getData( model ) ).to.equal( '<heading1>[]</heading1>' );
+			editor.execute( 'undo' );
+			expect( getData( model ) ).to.equal( '<paragraph># []</paragraph>' );
+		} );
+
+		it( 'should work when replacing two hash characters with heading level 2', () => {
+			setData( model, '<paragraph>##[]</paragraph>' );
+			model.change( writer => {
+				writer.insertText( ' ', doc.selection.getFirstPosition() );
+			} );
+
+			expect( getData( model ) ).to.equal( '<heading2>[]</heading2>' );
+			editor.execute( 'undo' );
+			expect( getData( model ) ).to.equal( '<paragraph>## []</paragraph>' );
+		} );
+
+		it( 'should work when replacing greater-than character with block quote', () => {
+			setData( model, '<paragraph>>[]</paragraph>' );
+			model.change( writer => {
+				writer.insertText( ' ', doc.selection.getFirstPosition() );
+			} );
+
+			expect( getData( model ) ).to.equal( '<blockQuote><paragraph>[]</paragraph></blockQuote>' );
+			editor.execute( 'undo' );
+			expect( getData( model ) ).to.equal( '<paragraph>> []</paragraph>' );
+		} );
+	} );
+} );