From af4e0fcd5fd48d2e989e84e2464a65b9cdfccc16 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Krzto=C5=84?= Date: Wed, 15 Feb 2017 09:02:00 +0100 Subject: [PATCH 1/9] ChangeBuffer locking. --- src/changebuffer.js | 54 +++++++++++++++++++++--- tests/changebuffer.js | 95 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 143 insertions(+), 6 deletions(-) diff --git a/src/changebuffer.js b/src/changebuffer.js index f7ad9d5..f640eef 100644 --- a/src/changebuffer.js +++ b/src/changebuffer.js @@ -60,12 +60,28 @@ export default class ChangeBuffer { */ this.limit = limit; + /** + * Whether the buffer is locked. The locked buffer cannot be reset unless it gets unlocked. + * + * @readonly + * @member {Boolean} #isLocked + */ + this.isLocked = false; + this._changeCallback = ( evt, type, changes, batch ) => { this._onBatch( batch ); }; + this._resetOnChangeCallback = () => { + this._reset(); + }; + doc.on( 'change', this._changeCallback ); + doc.selection.on( 'change:range', this._resetOnChangeCallback ); + + doc.selection.on( 'change:attribute', this._resetOnChangeCallback ); + /** * The current batch instance. * @@ -79,13 +95,20 @@ export default class ChangeBuffer { * @private * @member #_changeCallback */ + + /** + * The callback to document selection change:attribute and change:range events which resets the buffer. + * + * @private + * @member #_resetOnChangeCallback + */ } /** * The current batch to which a feature should add its deltas. Once the {@link #size} * is reached or exceeds the {@link #limit}, the batch is set to a new instance and the size is reset. * - * @type {engine.treeModel.batch.Batch} + * @type {module:engine/model/batch~Batch} */ get batch() { if ( !this._batch ) { @@ -105,15 +128,31 @@ export default class ChangeBuffer { this.size += changeCount; if ( this.size >= this.limit ) { - this._reset(); + this._reset( true ); } } + /** + * Locks the buffer. + */ + lock() { + this.isLocked = true; + } + + /** + * Unlocks the buffer. + */ + unlock() { + this.isLocked = false; + } + /** * Destroys the buffer. */ destroy() { this.document.off( 'change', this._changeCallback ); + this.document.selection.off( 'change:range', this._resetOnChangeCallback ); + this.document.selection.off( 'change:attribute', this._resetOnChangeCallback ); } /** @@ -130,7 +169,7 @@ export default class ChangeBuffer { _onBatch( batch ) { // One operation means a newly created batch. if ( batch.type != 'transparent' && batch !== this._batch && count( batch.getOperations() ) <= 1 ) { - this._reset(); + this._reset( true ); } } @@ -138,9 +177,12 @@ export default class ChangeBuffer { * Resets the change buffer. * * @private + * @param {Boolean} [ignoreLock] Whether internal lock {@link #isLocked} should be ignored. */ - _reset() { - this._batch = null; - this.size = 0; + _reset( ignoreLock ) { + if ( !this.isLocked || ignoreLock ) { + this._batch = null; + this.size = 0; + } } } diff --git a/tests/changebuffer.js b/tests/changebuffer.js index d529851..72b12b5 100644 --- a/tests/changebuffer.js +++ b/tests/changebuffer.js @@ -23,6 +23,7 @@ describe( 'ChangeBuffer', () => { expect( buffer ).to.have.property( 'document', doc ); expect( buffer ).to.have.property( 'limit', CHANGE_LIMIT ); expect( buffer ).to.have.property( 'size', 0 ); + expect( buffer ).to.have.property( 'isLocked', false ); } ); it( 'sets limit property according to default value', () => { @@ -32,6 +33,26 @@ describe( 'ChangeBuffer', () => { } ); } ); + describe( 'locking', () => { + it( 'is unlocked by default', () => { + expect( buffer.isLocked ).to.be.false; + } ); + + it( 'is locked by lock method', () => { + buffer.lock(); + + expect( buffer.isLocked ).to.be.true; + } ); + + it( 'is unlocked by unlock method', () => { + buffer.isLocked = true; + + buffer.unlock(); + + expect( buffer.isLocked ).to.be.false; + } ); + } ); + describe( 'batch', () => { it( 'it is set initially', () => { expect( buffer ).to.have.property( 'batch' ); @@ -107,6 +128,60 @@ describe( 'ChangeBuffer', () => { expect( buffer.batch ).to.equal( bufferBatch ); } ); + + it( 'is not reset while locked', () => { + const initialBatch = buffer.batch; + + buffer.lock(); + + buffer.input( 1 ); + buffer._reset(); + + buffer.unlock(); + + expect( buffer.batch ).to.be.equal( initialBatch ); + expect( buffer.size ).to.equal( 1 ); + } ); + + it( 'is reset while locked with ignoreLock used', () => { + const initialBatch = buffer.batch; + + buffer.lock(); + + buffer.input( 1 ); + buffer._reset( true ); + + buffer.unlock(); + + expect( buffer.batch ).to.not.equal( initialBatch ); + expect( buffer.size ).to.equal( 0 ); + } ); + + it( 'is reset while locked and limit exceeded', () => { + const initialBatch = buffer.batch; + + buffer.lock(); + + buffer.input( CHANGE_LIMIT + 1 ); + + buffer.unlock(); + + expect( buffer.batch ).to.not.equal( initialBatch ); + expect( buffer.size ).to.equal( 0 ); + } ); + + it( 'is reset while locked and new batch is applied', () => { + const initialBatch = buffer.batch; + + buffer.lock(); + + doc.batch().insert( Position.createAt( root, 0 ), 'a' ); + + buffer.unlock(); + + expect( buffer.batch ).to.not.equal( initialBatch ); + expect( buffer.size ).to.equal( 0 ); + } ); } ); describe( 'destroy', () => { @@ -119,5 +194,25 @@ describe( 'ChangeBuffer', () => { expect( buffer.batch ).to.equal( batch1 ); } ); + + it( 'offs the buffer from the selection change:range', () => { + const batch1 = buffer.batch; + + buffer.destroy(); + + doc.selection.fire( 'change:attribute' ); + + expect( buffer.batch ).to.equal( batch1 ); + } ); + + it( 'offs the buffer from the selection change:attribute', () => { + const batch1 = buffer.batch; + + buffer.destroy(); + + doc.selection.fire( 'change:range' ); + + expect( buffer.batch ).to.equal( batch1 ); + } ); } ); } ); From 48aa9287b205daa1226e3f2f7e1f241f791f824e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Krzto=C5=84?= Date: Wed, 15 Feb 2017 10:31:33 +0100 Subject: [PATCH 2/9] Tests: manual tests - new undo step on selection/attribute change. --- tests/manual/20/1.html | 4 ++++ tests/manual/20/1.js | 31 +++++++++++++++++++++++++++++++ tests/manual/20/1.md | 15 +++++++++++++++ tests/manual/21/1.html | 4 ++++ tests/manual/21/1.js | 31 +++++++++++++++++++++++++++++++ tests/manual/21/1.md | 18 ++++++++++++++++++ 6 files changed, 103 insertions(+) create mode 100644 tests/manual/20/1.html create mode 100644 tests/manual/20/1.js create mode 100644 tests/manual/20/1.md create mode 100644 tests/manual/21/1.html create mode 100644 tests/manual/21/1.js create mode 100644 tests/manual/21/1.md diff --git a/tests/manual/20/1.html b/tests/manual/20/1.html new file mode 100644 index 0000000..b2364bb --- /dev/null +++ b/tests/manual/20/1.html @@ -0,0 +1,4 @@ +
+

Heading 1

+

This is an editor instance.

+
diff --git a/tests/manual/20/1.js b/tests/manual/20/1.js new file mode 100644 index 0000000..962663f --- /dev/null +++ b/tests/manual/20/1.js @@ -0,0 +1,31 @@ +/** + * @license Copyright (c) 2003-2017, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md. + */ + +/* globals console, window, document */ + +import ClassicEditor from '@ckeditor/ckeditor5-editor-classic/src/classic'; +import Enter from '@ckeditor/ckeditor5-enter/src/enter'; +import Typing from '../../../src/typing'; +import Heading from '@ckeditor/ckeditor5-heading/src/heading'; +import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph'; +import Undo from '@ckeditor/ckeditor5-undo/src/undo'; +import Bold from '@ckeditor/ckeditor5-basic-styles/src/bold'; +import Italic from '@ckeditor/ckeditor5-basic-styles/src/italic'; +import { getData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model'; + +window.setInterval( function() { + console.log( getData( window.editor.document ) ); +}, 3000 ); + +ClassicEditor.create( document.querySelector( '#editor' ), { + plugins: [ Enter, Typing, Paragraph, Undo, Bold, Italic, Heading ], + toolbar: [ 'headings', 'bold', 'italic', 'undo', 'redo' ] +} ) + .then( editor => { + window.editor = editor; + } ) + .catch( err => { + console.error( err.stack ); + } ); diff --git a/tests/manual/20/1.md b/tests/manual/20/1.md new file mode 100644 index 0000000..6a979a0 --- /dev/null +++ b/tests/manual/20/1.md @@ -0,0 +1,15 @@ +## New undo step on changing selection ([#20](https://github.com/ckeditor/ckeditor5-typing/issues/20)) + +*Every selection change should create a new undo step.* + +**Check**: + +1. Type "aaa" in one place. +1. Move selection to another. +1. Type "bbb". +1. Move selection to another place. +1. Type "ccc". +1. Undo 3 times. + +**Expected**: +3 undo steps were created. It is possible to undo 3 times, each time 3 letters from steps 5, 3, 1 are undone. diff --git a/tests/manual/21/1.html b/tests/manual/21/1.html new file mode 100644 index 0000000..b2364bb --- /dev/null +++ b/tests/manual/21/1.html @@ -0,0 +1,4 @@ +
+

Heading 1

+

This is an editor instance.

+
diff --git a/tests/manual/21/1.js b/tests/manual/21/1.js new file mode 100644 index 0000000..962663f --- /dev/null +++ b/tests/manual/21/1.js @@ -0,0 +1,31 @@ +/** + * @license Copyright (c) 2003-2017, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md. + */ + +/* globals console, window, document */ + +import ClassicEditor from '@ckeditor/ckeditor5-editor-classic/src/classic'; +import Enter from '@ckeditor/ckeditor5-enter/src/enter'; +import Typing from '../../../src/typing'; +import Heading from '@ckeditor/ckeditor5-heading/src/heading'; +import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph'; +import Undo from '@ckeditor/ckeditor5-undo/src/undo'; +import Bold from '@ckeditor/ckeditor5-basic-styles/src/bold'; +import Italic from '@ckeditor/ckeditor5-basic-styles/src/italic'; +import { getData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model'; + +window.setInterval( function() { + console.log( getData( window.editor.document ) ); +}, 3000 ); + +ClassicEditor.create( document.querySelector( '#editor' ), { + plugins: [ Enter, Typing, Paragraph, Undo, Bold, Italic, Heading ], + toolbar: [ 'headings', 'bold', 'italic', 'undo', 'redo' ] +} ) + .then( editor => { + window.editor = editor; + } ) + .catch( err => { + console.error( err.stack ); + } ); diff --git a/tests/manual/21/1.md b/tests/manual/21/1.md new file mode 100644 index 0000000..5becbef --- /dev/null +++ b/tests/manual/21/1.md @@ -0,0 +1,18 @@ +## New undo step on applying attribute ([#21](https://github.com/ckeditor/ckeditor5-typing/issues/21)) + +*Every attribute change should create a new undo step.* + +**Check**: + +1. Type few letters. +1. Press styling button (italic, bold, etc). +1. Type few more letters. +1. Press undo button. + +**Expected**: +Only the letters typed in step 3 should be undone. + +5. Press undo button. + +**Expected**: +Letters typed in step 1 should be undone. From 6cccce565d6bf0f44556fa53549f557609bd2035 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Krzto=C5=84?= Date: Wed, 15 Feb 2017 11:17:57 +0100 Subject: [PATCH 3/9] Tests: delete command and input buffer un/lock test. --- tests/deletecommand.js | 18 ++++++++++++++++- tests/input.js | 44 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 1 deletion(-) diff --git a/tests/deletecommand.js b/tests/deletecommand.js index 8b2f879..4ca5c21 100644 --- a/tests/deletecommand.js +++ b/tests/deletecommand.js @@ -6,10 +6,13 @@ import ModelTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/modeltesteditor'; import DeleteCommand from '../src/deletecommand'; import { getData, setData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model'; +import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils'; describe( 'DeleteCommand', () => { let editor, doc; + testUtils.createSinonSandbox(); + beforeEach( () => { return ModelTestEditor.create( ) .then( newEditor => { @@ -33,13 +36,26 @@ describe( 'DeleteCommand', () => { it( 'uses enqueueChanges', () => { setData( doc, '

foo[]bar

' ); - const spy = sinon.spy( doc, 'enqueueChanges' ); + const spy = testUtils.sinon.spy( doc, 'enqueueChanges' ); editor.execute( 'delete' ); expect( spy.calledOnce ).to.be.true; } ); + it( 'locks buffer when executing', () => { + setData( doc, '

foo[]bar

' ); + + const buffer = editor.commands.get( 'delete' )._buffer; + const lockSpy = testUtils.sinon.spy( buffer, 'lock' ); + const unlockSpy = testUtils.sinon.spy( buffer, 'unlock' ); + + editor.execute( 'delete' ); + + expect( lockSpy.calledOnce ).to.be.true; + expect( unlockSpy.calledOnce ).to.be.true; + } ); + it( 'deletes previous character when selection is collapsed', () => { setData( doc, '

foo[]bar

' ); diff --git a/tests/input.js b/tests/input.js index 0f294a5..feb801c 100644 --- a/tests/input.js +++ b/tests/input.js @@ -23,6 +23,8 @@ import { getCode } from '@ckeditor/ckeditor5-utils/src/keyboard'; import { getData as getModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model'; import { getData as getViewData } from '@ckeditor/ckeditor5-engine/src/dev-utils/view'; +import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils'; + describe( 'Input feature', () => { let editor, model, modelRoot, view, viewRoot, listenter; @@ -434,6 +436,48 @@ describe( 'Input feature', () => { expect( getModelData( model ) ).to.equal( 'foo[]bar' ); } ); + + it( 'should lock buffer if selection is not collapsed', () => { + const buffer = editor.plugins.get( Input )._buffer; + const lockSpy = testUtils.sinon.spy( buffer, 'lock' ); + const unlockSpy = testUtils.sinon.spy( buffer, 'unlock' ); + + model.enqueueChanges( () => { + model.selection.setRanges( [ + ModelRange.createFromParentsAndOffsets( modelRoot.getChild( 0 ), 2, modelRoot.getChild( 0 ), 4 ) ] ); + } ); + + view.fire( 'keydown', { keyCode: getCode( 'y' ) } ); + + expect( lockSpy.calledOnce ).to.be.true; + expect( unlockSpy.calledOnce ).to.be.true; + } ); + + it( 'should not lock buffer on non printable keys', () => { + const buffer = editor.plugins.get( Input )._buffer; + const lockSpy = testUtils.sinon.spy( buffer, 'lock' ); + const unlockSpy = testUtils.sinon.spy( buffer, 'unlock' ); + + view.fire( 'keydown', { keyCode: 16 } ); // Shift + view.fire( 'keydown', { keyCode: 35 } ); // Home + view.fire( 'keydown', { keyCode: 112 } ); // F1 + + expect( lockSpy.callCount ).to.be.equal( 0 ); + expect( unlockSpy.callCount ).to.be.equal( 0 ); + } ); + + it( 'should not lock buffer on collapsed selection', () => { + const buffer = editor.plugins.get( Input )._buffer; + const lockSpy = testUtils.sinon.spy( buffer, 'lock' ); + const unlockSpy = testUtils.sinon.spy( buffer, 'unlock' ); + + view.fire( 'keydown', { keyCode: getCode( 'b' ) } ); + view.fire( 'keydown', { keyCode: getCode( 'a' ) } ); + view.fire( 'keydown', { keyCode: getCode( 'z' ) } ); + + expect( lockSpy.callCount ).to.be.equal( 0 ); + expect( unlockSpy.callCount ).to.be.equal( 0 ); + } ); } ); } ); From 1ed928d7748bb2fe91d7f6ec1ee6a39d1dbc32d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Krzto=C5=84?= Date: Wed, 15 Feb 2017 13:39:35 +0100 Subject: [PATCH 4/9] Lock buffer on input and delete. --- src/deletecommand.js | 4 ++++ src/input.js | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/src/deletecommand.js b/src/deletecommand.js index b494a7a..fe480fb 100644 --- a/src/deletecommand.js +++ b/src/deletecommand.js @@ -61,6 +61,8 @@ export default class DeleteCommand extends Command { const dataController = this.editor.data; doc.enqueueChanges( () => { + this._buffer.lock(); + const selection = Selection.createFromSelection( doc.selection ); // Try to extend the selection in the specified direction. @@ -85,6 +87,8 @@ export default class DeleteCommand extends Command { this._buffer.input( changeCount ); doc.selection.setRanges( selection.getRanges(), selection.isBackward ); + + this._buffer.unlock(); } ); } } diff --git a/src/input.js b/src/input.js index 2899b4f..afa9fc6 100644 --- a/src/input.js +++ b/src/input.js @@ -66,9 +66,13 @@ export default class Input extends Plugin { return; } + this._buffer.lock(); + doc.enqueueChanges( () => { this.editor.data.deleteContent( doc.selection, buffer.batch ); } ); + + this._buffer.unlock(); } /** From 3f187498dcdcf6af05a79ec0335d9b93154543bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Krzto=C5=84?= Date: Mon, 6 Mar 2017 12:41:15 +0100 Subject: [PATCH 5/9] Refactoring. --- src/changebuffer.js | 12 +++++------ src/input.js | 4 ++-- src/inputcommand.js | 4 ++++ tests/changebuffer.js | 46 +++++++++++++++++++++++++++++++++++++++++++ tests/input.js | 8 +++----- tests/inputcommand.js | 14 +++++++++++++ 6 files changed, 75 insertions(+), 13 deletions(-) diff --git a/src/changebuffer.js b/src/changebuffer.js index f640eef..ffff490 100644 --- a/src/changebuffer.js +++ b/src/changebuffer.js @@ -72,15 +72,15 @@ export default class ChangeBuffer { this._onBatch( batch ); }; - this._resetOnChangeCallback = () => { + this._selectionChangeCallback = () => { this._reset(); }; doc.on( 'change', this._changeCallback ); - doc.selection.on( 'change:range', this._resetOnChangeCallback ); + doc.selection.on( 'change:range', this._selectionChangeCallback ); - doc.selection.on( 'change:attribute', this._resetOnChangeCallback ); + doc.selection.on( 'change:attribute', this._selectionChangeCallback ); /** * The current batch instance. @@ -100,7 +100,7 @@ export default class ChangeBuffer { * The callback to document selection change:attribute and change:range events which resets the buffer. * * @private - * @member #_resetOnChangeCallback + * @member #_selectionChangeCallback */ } @@ -151,8 +151,8 @@ export default class ChangeBuffer { */ destroy() { this.document.off( 'change', this._changeCallback ); - this.document.selection.off( 'change:range', this._resetOnChangeCallback ); - this.document.selection.off( 'change:attribute', this._resetOnChangeCallback ); + this.document.selection.off( 'change:range', this._selectionChangeCallback ); + this.document.selection.off( 'change:attribute', this._selectionChangeCallback ); } /** diff --git a/src/input.js b/src/input.js index afa9fc6..c1977c4 100644 --- a/src/input.js +++ b/src/input.js @@ -66,13 +66,13 @@ export default class Input extends Plugin { return; } - this._buffer.lock(); + buffer.lock(); doc.enqueueChanges( () => { this.editor.data.deleteContent( doc.selection, buffer.batch ); } ); - this._buffer.unlock(); + buffer.unlock(); } /** diff --git a/src/inputcommand.js b/src/inputcommand.js index a4b6815..5ad4c7d 100644 --- a/src/inputcommand.js +++ b/src/inputcommand.js @@ -78,6 +78,8 @@ export default class InputCommand extends Command { doc.enqueueChanges( () => { const isCollapsedRange = range.isCollapsed; + this._buffer.lock(); + if ( !isCollapsedRange ) { this._buffer.batch.remove( range ); } @@ -91,6 +93,8 @@ export default class InputCommand extends Command { this.editor.data.model.selection.collapse( range.start.getShiftedBy( textInsertions ) ); } + this._buffer.unlock(); + this._buffer.input( textInsertions ); } ); } diff --git a/tests/changebuffer.js b/tests/changebuffer.js index 72b12b5..2ee5a7a 100644 --- a/tests/changebuffer.js +++ b/tests/changebuffer.js @@ -182,6 +182,52 @@ describe( 'ChangeBuffer', () => { expect( buffer.batch ).to.not.equal( initialBatch ); expect( buffer.size ).to.equal( 0 ); } ); + + it( 'is reset on selection change:range', () => { + const initialBatch = buffer.batch; + + doc.selection.fire( 'change:range' ); + + expect( buffer.batch ).to.not.equal( initialBatch ); + expect( buffer.size ).to.equal( 0 ); + } ); + + it( 'is reset on selection change:attribute', () => { + const initialBatch = buffer.batch; + + doc.selection.fire( 'change:attribute' ); + + expect( buffer.batch ).to.not.equal( initialBatch ); + expect( buffer.size ).to.equal( 0 ); + } ); + + it( 'is not reset on selection change:range while locked', () => { + const initialBatch = buffer.batch; + buffer.size = 1; + + buffer.lock(); + + doc.selection.fire( 'change:range' ); + + buffer.unlock(); + + expect( buffer.batch ).to.be.equal( initialBatch ); + expect( buffer.size ).to.equal( 1 ); + } ); + + it( 'is not reset on selection change:attribute while locked', () => { + const initialBatch = buffer.batch; + buffer.size = 1; + + buffer.lock(); + + doc.selection.fire( 'change:attribute' ); + + buffer.unlock(); + + expect( buffer.batch ).to.be.equal( initialBatch ); + expect( buffer.size ).to.equal( 1 ); + } ); } ); describe( 'destroy', () => { diff --git a/tests/input.js b/tests/input.js index feb801c..6b732a0 100644 --- a/tests/input.js +++ b/tests/input.js @@ -23,8 +23,6 @@ import { getCode } from '@ckeditor/ckeditor5-utils/src/keyboard'; import { getData as getModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model'; import { getData as getViewData } from '@ckeditor/ckeditor5-engine/src/dev-utils/view'; -import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils'; - describe( 'Input feature', () => { let editor, model, modelRoot, view, viewRoot, listenter; @@ -438,7 +436,7 @@ describe( 'Input feature', () => { } ); it( 'should lock buffer if selection is not collapsed', () => { - const buffer = editor.plugins.get( Input )._buffer; + const buffer = editor.commands.get( 'input' )._buffer; const lockSpy = testUtils.sinon.spy( buffer, 'lock' ); const unlockSpy = testUtils.sinon.spy( buffer, 'unlock' ); @@ -454,7 +452,7 @@ describe( 'Input feature', () => { } ); it( 'should not lock buffer on non printable keys', () => { - const buffer = editor.plugins.get( Input )._buffer; + const buffer = editor.commands.get( 'input' )._buffer; const lockSpy = testUtils.sinon.spy( buffer, 'lock' ); const unlockSpy = testUtils.sinon.spy( buffer, 'unlock' ); @@ -467,7 +465,7 @@ describe( 'Input feature', () => { } ); it( 'should not lock buffer on collapsed selection', () => { - const buffer = editor.plugins.get( Input )._buffer; + const buffer = editor.commands.get( 'input' )._buffer; const lockSpy = testUtils.sinon.spy( buffer, 'lock' ); const unlockSpy = testUtils.sinon.spy( buffer, 'unlock' ); diff --git a/tests/inputcommand.js b/tests/inputcommand.js index 86ba6d6..cc1ec11 100644 --- a/tests/inputcommand.js +++ b/tests/inputcommand.js @@ -71,6 +71,20 @@ describe( 'InputCommand', () => { expect( spy.calledOnce ).to.be.true; } ); + it( 'should lock and unlock buffer', () => { + setData( doc, '

foo[]bar

' ); + + const spyLock = testUtils.sinon.spy( buffer, 'lock' ); + const spyUnlock = testUtils.sinon.spy( buffer, 'unlock' ); + + editor.execute( 'input', { + text: '' + } ); + + expect( spyLock.calledOnce ).to.be.true; + expect( spyUnlock.calledOnce ).to.be.true; + } ); + it( 'inserts text for collapsed range', () => { setData( doc, '

foo[]

' ); From 297e5cba71007b4c95b0a74973e151b96d3d6ea2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Krzto=C5=84?= Date: Mon, 6 Mar 2017 12:58:47 +0100 Subject: [PATCH 6/9] Tests: input command integration tests. --- tests/inputcommand-integration.js | 206 ++++++++++++++++++++++++++++++ 1 file changed, 206 insertions(+) create mode 100644 tests/inputcommand-integration.js diff --git a/tests/inputcommand-integration.js b/tests/inputcommand-integration.js new file mode 100644 index 0000000..1dfbbe1 --- /dev/null +++ b/tests/inputcommand-integration.js @@ -0,0 +1,206 @@ +/** + * @license Copyright (c) 2003-2017, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md. + */ + +/* globals document */ + +import ClassicTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/classictesteditor'; + +import Typing from '@ckeditor/ckeditor5-typing/src/typing'; +import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph'; +import Undo from '@ckeditor/ckeditor5-undo/src/undo'; +import Bold from '@ckeditor/ckeditor5-basic-styles/src/bold'; +import Italic from '@ckeditor/ckeditor5-basic-styles/src/italic'; +import Enter from '@ckeditor/ckeditor5-enter/src/enter'; + +import Range from '@ckeditor/ckeditor5-engine/src/model/range'; +import Position from '@ckeditor/ckeditor5-engine/src/model/position'; + +import { setData as setModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model'; +import { getData as getModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model'; +import { getData as getViewData } from '@ckeditor/ckeditor5-engine/src/dev-utils/view'; + +describe( 'InputCommand integration', () => { + let editor, doc, viewDocument, boldView, italicView; + + beforeEach( () => { + const editorElement = document.createElement( 'div' ); + document.body.appendChild( editorElement ); + + return ClassicTestEditor.create( editorElement, { + plugins: [ Typing, Paragraph, Undo, Bold, Italic, Enter ], + typing: { undoStep: 3 } + } ) + .then( newEditor => { + editor = newEditor; + doc = editor.document; + viewDocument = editor.editing.view; + + boldView = editor.ui.componentFactory.create( 'bold' ); + italicView = editor.ui.componentFactory.create( 'italic' ); + } ); + } ); + + afterEach( () => { + return editor.destroy(); + } ); + + function expectOutput( modelOutput, viewOutput ) { + expect( getModelData( editor.document ) ).to.equal( modelOutput ); + expect( getViewData( viewDocument ) ).to.equal( viewOutput ); + } + + function simulateTyping( text ) { + // While typing, every character is an atomic change. + text.split( '' ).forEach( ( character ) => { + editor.execute( 'input', { + text: character + } ); + } ); + } + + function simulateBatches( batches ) { + // Use longer text at once in input command. + batches.forEach( ( batch ) => { + editor.execute( 'input', { + text: batch + } ); + } ); + } + + function setSelection( pathA, pathB ) { + doc.selection.setRanges( [ new Range( new Position( doc.getRoot(), pathA ), new Position( doc.getRoot(), pathB ) ) ] ); + } + + describe( 'InputCommand integration', () => { + it( 'resets the buffer on typing respecting typing.undoStep', () => { + setModelData( doc, '0[]' ); + + simulateTyping( '123456789' ); + + expectOutput( '0123456789[]', '

0123456789{}

' ); + + editor.execute( 'undo' ); + + expectOutput( '0123456[]', '

0123456{}

' ); + + editor.execute( 'undo' ); + + expectOutput( '0123[]', '

0123{}

' ); + + editor.execute( 'redo' ); + + expectOutput( '0123456[]', '

0123456{}

' ); + } ); + + it( 'resets the buffer on text insertion respecting typing.undoStep', () => { + setModelData( doc, '0[]' ); + + simulateBatches( [ '1234', '5', '678', '9' ] ); + + expectOutput( '0123456789[]', '

0123456789{}

' ); + + editor.execute( 'undo' ); + + expectOutput( '012345678[]', '

012345678{}

' ); + + editor.execute( 'undo' ); + + expectOutput( '01234[]', '

01234{}

' ); + + editor.execute( 'redo' ); + + expectOutput( '012345678[]', '

012345678{}

' ); + } ); + + it( 'resets the buffer when selection changes', () => { + setModelData( doc, 'Foo[] Bar' ); + + setSelection( [ 0, 5 ], [ 0, 5 ] ); + simulateTyping( '1' ); + + setSelection( [ 0, 7 ], [ 0, 8 ] ); + simulateTyping( '2' ); + + expectOutput( 'Foo B1a2[]', '

Foo B1a2{}

' ); + + editor.execute( 'undo' ); + + expectOutput( 'Foo B1a[r]', '

Foo B1a{r}

' ); + + editor.execute( 'undo' ); + + expectOutput( 'Foo B[]ar', '

Foo B{}ar

' ); + + editor.execute( 'redo' ); + + expectOutput( 'Foo B1[]ar', '

Foo B1{}ar

' ); + } ); + + it( 'resets the buffer when selection changes (with enter)', () => { + setModelData( doc, 'Foo[]Bar' ); + + simulateTyping( '1' ); + editor.execute( 'enter' ); + + setSelection( [ 1, 3 ], [ 1, 3 ] ); + simulateTyping( '2' ); + editor.execute( 'enter' ); + + simulateTyping( 'Baz' ); + + expectOutput( 'Foo1Bar2Baz[]', + '

Foo1

Bar2

Baz{}

' ); + + editor.execute( 'undo' ); + + expectOutput( 'Foo1Bar2[]', + '

Foo1

Bar2

[]

' ); + + editor.execute( 'undo' ); + + expectOutput( 'Foo1Bar2[]', + '

Foo1

Bar2{}

' ); + + editor.execute( 'undo' ); + + expectOutput( 'Foo1Bar[]', + '

Foo1

Bar{}

' ); + + editor.execute( 'redo' ); + + expectOutput( 'Foo1Bar2[]', + '

Foo1

Bar2{}

' ); + } ); + + it( 'resets the buffer when attribute changes', () => { + setModelData( doc, 'Foo[] Bar' ); + + simulateTyping( ' ' ); + + boldView.fire( 'execute' ); + simulateTyping( 'B' ); + + italicView.fire( 'execute' ); + simulateTyping( 'a' ); + + boldView.fire( 'execute' ); + italicView.fire( 'execute' ); + simulateTyping( 'z' ); + + expectOutput( 'Foo <$text bold="true">B<$text italic="true"><$text bold="true">az[] Bar', + '

Foo Baz{} Bar

' ); + + editor.execute( 'undo' ); + + expectOutput( 'Foo <$text bold="true">B<$text italic="true"><$text bold="true">a<$text bold="true" italic="true">[] Bar', + '

Foo Ba{} Bar

' ); + + editor.execute( 'undo' ); + + expectOutput( 'Foo <$text bold="true">B[] Bar', + '

Foo B{} Bar

' ); + } ); + } ); +} ); From 85a16aad6f4de39498a2ef7b093854a63bf89c78 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Krzto=C5=84?= Date: Mon, 6 Mar 2017 13:22:22 +0100 Subject: [PATCH 7/9] Tests: import fix. --- tests/inputcommand-integration.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/inputcommand-integration.js b/tests/inputcommand-integration.js index 1dfbbe1..4a3501f 100644 --- a/tests/inputcommand-integration.js +++ b/tests/inputcommand-integration.js @@ -7,7 +7,7 @@ import ClassicTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/classictesteditor'; -import Typing from '@ckeditor/ckeditor5-typing/src/typing'; +import Typing from '../src/typing'; import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph'; import Undo from '@ckeditor/ckeditor5-undo/src/undo'; import Bold from '@ckeditor/ckeditor5-basic-styles/src/bold'; From a31847acb09a6c8ba3e4d23ee72dd484e0b68d19 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotrek=20Koszuli=C5=84ski?= Date: Thu, 9 Mar 2017 10:36:49 +0100 Subject: [PATCH 8/9] Fixed linting issue. --- tests/inputcommand-integration.js | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/tests/inputcommand-integration.js b/tests/inputcommand-integration.js index 4a3501f..affe81f 100644 --- a/tests/inputcommand-integration.js +++ b/tests/inputcommand-integration.js @@ -194,8 +194,13 @@ describe( 'InputCommand integration', () => { editor.execute( 'undo' ); - expectOutput( 'Foo <$text bold="true">B<$text italic="true"><$text bold="true">a<$text bold="true" italic="true">[] Bar', - '

Foo Ba{} Bar

' ); + expectOutput( + '' + + 'Foo <$text bold="true">B<$text italic="true"><$text bold="true">a' + + '<$text bold="true" italic="true">[] Bar' + + '', + '

Foo Ba{} Bar

' + ); editor.execute( 'undo' ); From df5eeeee861f92a5e02b513822129f0f9f1c8127 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotrek=20Koszuli=C5=84ski?= Date: Thu, 9 Mar 2017 10:47:58 +0100 Subject: [PATCH 9/9] Simplified manual test. --- tests/manual/20/1.js | 17 ++++++----------- tests/manual/21/1.js | 17 ++++++----------- tests/manual/40/1.md | 14 +++++++------- 3 files changed, 19 insertions(+), 29 deletions(-) diff --git a/tests/manual/20/1.js b/tests/manual/20/1.js index 962663f..d27220d 100644 --- a/tests/manual/20/1.js +++ b/tests/manual/20/1.js @@ -13,19 +13,14 @@ import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph'; import Undo from '@ckeditor/ckeditor5-undo/src/undo'; import Bold from '@ckeditor/ckeditor5-basic-styles/src/bold'; import Italic from '@ckeditor/ckeditor5-basic-styles/src/italic'; -import { getData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model'; - -window.setInterval( function() { - console.log( getData( window.editor.document ) ); -}, 3000 ); ClassicEditor.create( document.querySelector( '#editor' ), { plugins: [ Enter, Typing, Paragraph, Undo, Bold, Italic, Heading ], toolbar: [ 'headings', 'bold', 'italic', 'undo', 'redo' ] } ) - .then( editor => { - window.editor = editor; - } ) - .catch( err => { - console.error( err.stack ); - } ); +.then( editor => { + window.editor = editor; +} ) +.catch( err => { + console.error( err.stack ); +} ); diff --git a/tests/manual/21/1.js b/tests/manual/21/1.js index 962663f..d27220d 100644 --- a/tests/manual/21/1.js +++ b/tests/manual/21/1.js @@ -13,19 +13,14 @@ import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph'; import Undo from '@ckeditor/ckeditor5-undo/src/undo'; import Bold from '@ckeditor/ckeditor5-basic-styles/src/bold'; import Italic from '@ckeditor/ckeditor5-basic-styles/src/italic'; -import { getData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model'; - -window.setInterval( function() { - console.log( getData( window.editor.document ) ); -}, 3000 ); ClassicEditor.create( document.querySelector( '#editor' ), { plugins: [ Enter, Typing, Paragraph, Undo, Bold, Italic, Heading ], toolbar: [ 'headings', 'bold', 'italic', 'undo', 'redo' ] } ) - .then( editor => { - window.editor = editor; - } ) - .catch( err => { - console.error( err.stack ); - } ); +.then( editor => { + window.editor = editor; +} ) +.catch( err => { + console.error( err.stack ); +} ); diff --git a/tests/manual/40/1.md b/tests/manual/40/1.md index 9067e6a..08e6554 100644 --- a/tests/manual/40/1.md +++ b/tests/manual/40/1.md @@ -1,11 +1,11 @@ -### Issue [#40](https://github.com/ckeditor/ckeditor5-typing/issues/40) manual test +## Issue [#40](https://github.com/ckeditor/ckeditor5-typing/issues/40) manual test - - select fragment of Heading and Paragraph, - ```html -

Head{ing

-

Parag}raph

- ``` - - delete selected text. + * select a fragment of the heading and paragraph, + ```html +

Head{ing

+

Parag}raph

+ ``` +* delete the selected text. Expected result: ```html