From c3ef549a5fce2b84f0463d5644ac7f3cc8f088fd Mon Sep 17 00:00:00 2001 From: Kamil Piechaczek Date: Wed, 19 Jul 2017 15:30:30 +0200 Subject: [PATCH 01/18] =?UTF-8?q?Improved=20the=20typing=20=E2=80=93=20whe?= =?UTF-8?q?n=20the=20user=20held=20the=20Backspace=20or=20pressed=20the=20?= =?UTF-8?q?Backspace=20inside=20the=20empty=20heading=20element.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/delete.js | 2 +- src/deletecommand.js | 90 +++++++++++++++++++++++++++++++++- src/deleteobserver.js | 11 +++++ tests/delete.js | 10 ++-- tests/deletecommand.js | 106 +++++++++++++++++++++++++++++++++++----- tests/deleteobserver.js | 97 ++++++++++++++++++++++++++++++++++++ 6 files changed, 297 insertions(+), 19 deletions(-) diff --git a/src/delete.js b/src/delete.js index 03fe599..c5b5c2f 100644 --- a/src/delete.js +++ b/src/delete.js @@ -34,7 +34,7 @@ export default class Delete extends Plugin { editor.commands.add( 'delete', new DeleteCommand( editor, 'backward' ) ); this.listenTo( editingView, 'delete', ( evt, data ) => { - editor.execute( data.direction == 'forward' ? 'forwardDelete' : 'delete', { unit: data.unit } ); + editor.execute( data.direction == 'forward' ? 'forwardDelete' : 'delete', { unit: data.unit, sequence: data.sequence } ); data.preventDefault(); } ); } diff --git a/src/deletecommand.js b/src/deletecommand.js index 805d026..79ff2fd 100644 --- a/src/deletecommand.js +++ b/src/deletecommand.js @@ -9,6 +9,9 @@ import Command from '@ckeditor/ckeditor5-core/src/command'; import Selection from '@ckeditor/ckeditor5-engine/src/model/selection'; +import Element from '@ckeditor/ckeditor5-engine/src/model/element'; +import Position from '@ckeditor/ckeditor5-engine/src/model/position'; +import Range from '@ckeditor/ckeditor5-engine/src/model/range'; import ChangeBuffer from './changebuffer'; import count from '@ckeditor/ckeditor5-utils/src/count'; @@ -54,8 +57,8 @@ export default class DeleteCommand extends Command { * * @fires execute * @param {Object} [options] The command options. - * @param {'character'} [options.unit='character'] See {@link module:engine/controller/modifyselection~modifySelection}'s - * options. + * @param {'character'} [options.unit='character'] See {@link module:engine/controller/modifyselection~modifySelection}'s options. + * @param {Number} [options.sequence=1] See the {@link module:engine/view/document~Document#event:delete} event data. */ execute( options = {} ) { const doc = this.editor.document; @@ -73,6 +76,12 @@ export default class DeleteCommand extends Command { // If selection is still collapsed, then there's nothing to delete. if ( selection.isCollapsed ) { + const sequence = options.sequence || 1; + + if ( this._shouldEntireContentBeReplacedWithParagraph( { sequence } ) ) { + this._replaceEntireContentWithParagraph(); + } + return; } @@ -92,4 +101,81 @@ export default class DeleteCommand extends Command { this._buffer.unlock(); } ); } + + /** + * If the user keeps Backspace or Delete key, we do nothing because the user can clear + * the whole element without removing them. + * + * But, if the user pressed and released the key, we want to replace the entire content with a paragraph if: + * - the entire content is selected, + * - the paragraph is allowed in the common ancestor, + * - other paragraph does not occur in the editor. + * + * @private + * @param {Object} options + * @param {Number} options.sequence A number that describes which sequence of the same event is fired. + * @returns {Boolean} + */ + _shouldEntireContentBeReplacedWithParagraph( options ) { + // Does nothing if user pressed and held the "Backspace" or "Delete" key. + if ( options.sequence > 1 ) { + return false; + } + + const document = this.editor.document; + const selection = document.selection; + const limitElement = getLimitElement( document.schema, selection ); + const limitStartPosition = Position.createAt( limitElement ); + const limitEndPosition = Position.createAt( limitElement, 'end' ); + + if ( + !limitStartPosition.isTouching( selection.getFirstPosition() ) || + !limitEndPosition.isTouching( selection.getLastPosition() ) + ) { + return false; + } + + if ( !document.schema.check( { name: 'paragraph', inside: limitElement.name } ) ) { + return false; + } + + // Does nothing if editor contains an empty paragraph. + if ( selection.getFirstRange().getCommonAncestor().name === 'paragraph' ) { + return false; + } + + return true; + } + + /** + * The entire content is replaced with the paragraph. Selection is moved inside the paragraph. + * + * @private + */ + _replaceEntireContentWithParagraph() { + const document = this.editor.document; + const selection = document.selection; + const limitElement = getLimitElement( document.schema, selection ); + const paragraph = new Element( 'paragraph' ); + + this._buffer.batch.remove( Range.createIn( limitElement ) ); + this._buffer.batch.insert( Position.createAt( limitElement ), paragraph ); + + selection.collapse( paragraph ); + } +} + +// Returns the lowest limit element defined in `Schema.limits` for passed selection. +function getLimitElement( schema, selection ) { + let element = selection.getFirstRange().getCommonAncestor(); + + while ( !schema.limits.has( element.name ) ) { + if ( element.parent ) { + element = element.parent; + } else { + break; + } + } + + return element; } diff --git a/src/deleteobserver.js b/src/deleteobserver.js index 096430f..e091051 100644 --- a/src/deleteobserver.js +++ b/src/deleteobserver.js @@ -20,6 +20,14 @@ export default class DeleteObserver extends Observer { constructor( document ) { super( document ); + let sequence = 0; + + document.on( 'keyup', ( evt, data ) => { + if ( data.keyCode == keyCodes.delete || data.keyCode == keyCodes.backspace ) { + sequence = 0; + } + } ); + document.on( 'keydown', ( evt, data ) => { const deleteData = {}; @@ -34,6 +42,7 @@ export default class DeleteObserver extends Observer { } deleteData.unit = data.altKey ? 'word' : deleteData.unit; + deleteData.sequence = ++sequence; document.fire( 'delete', new DomEventData( document, data.domEvent, deleteData ) ); } ); @@ -55,4 +64,6 @@ export default class DeleteObserver extends Observer { * @param {module:engine/view/observer/domeventdata~DomEventData} data * @param {'forward'|'delete'} data.direction The direction in which the deletion should happen. * @param {'character'|'word'} data.unit The "amount" of content that should be deleted. + * @param {Number} data.sequence A number that describes which sequence of the same event is fired. + * It helps detect the key was pressed and held. */ diff --git a/tests/delete.js b/tests/delete.js index ae97305..0ea5fac 100644 --- a/tests/delete.js +++ b/tests/delete.js @@ -34,21 +34,23 @@ describe( 'Delete feature', () => { view.fire( 'delete', new DomEventData( editingView, domEvt, { direction: 'forward', - unit: 'character' + unit: 'character', + sequence: 1 } ) ); expect( spy.calledOnce ).to.be.true; - expect( spy.calledWithMatch( 'forwardDelete', { unit: 'character' } ) ).to.be.true; + expect( spy.calledWithMatch( 'forwardDelete', { unit: 'character', sequence: 1 } ) ).to.be.true; expect( domEvt.preventDefault.calledOnce ).to.be.true; view.fire( 'delete', new DomEventData( editingView, getDomEvent(), { direction: 'backward', - unit: 'character' + unit: 'character', + sequence: 5 } ) ); expect( spy.calledTwice ).to.be.true; - expect( spy.calledWithMatch( 'delete', { unit: 'character' } ) ).to.be.true; + expect( spy.calledWithMatch( 'delete', { unit: 'character', sequence: 5 } ) ).to.be.true; } ); function getDomEvent() { diff --git a/tests/deletecommand.js b/tests/deletecommand.js index 1727c8c..c5ce81a 100644 --- a/tests/deletecommand.js +++ b/tests/deletecommand.js @@ -22,7 +22,8 @@ describe( 'DeleteCommand', () => { const command = new DeleteCommand( editor, 'backward' ); editor.commands.add( 'delete', command ); - doc.schema.registerItem( 'p', '$block' ); + doc.schema.registerItem( 'paragraph', '$block' ); + doc.schema.registerItem( 'heading1', '$block' ); } ); } ); @@ -38,7 +39,7 @@ describe( 'DeleteCommand', () => { describe( 'execute()', () => { it( 'uses enqueueChanges', () => { - setData( doc, '

foo[]bar

' ); + setData( doc, 'foo[]bar' ); const spy = testUtils.sinon.spy( doc, 'enqueueChanges' ); @@ -48,7 +49,7 @@ describe( 'DeleteCommand', () => { } ); it( 'locks buffer when executing', () => { - setData( doc, '

foo[]bar

' ); + setData( doc, 'foo[]bar' ); const buffer = editor.commands.get( 'delete' )._buffer; const lockSpy = testUtils.sinon.spy( buffer, 'lock' ); @@ -61,38 +62,38 @@ describe( 'DeleteCommand', () => { } ); it( 'deletes previous character when selection is collapsed', () => { - setData( doc, '

foo[]bar

' ); + setData( doc, 'foo[]bar' ); editor.execute( 'delete' ); - expect( getData( doc, { selection: true } ) ).to.equal( '

fo[]bar

' ); + expect( getData( doc, { selection: true } ) ).to.equal( 'fo[]bar' ); } ); it( 'deletes selection contents', () => { - setData( doc, '

fo[ob]ar

' ); + setData( doc, 'fo[ob]ar' ); editor.execute( 'delete' ); - expect( getData( doc, { selection: true } ) ).to.equal( '

fo[]ar

' ); + expect( getData( doc, { selection: true } ) ).to.equal( 'fo[]ar' ); } ); it( 'merges elements', () => { - setData( doc, '

foo

[]bar

' ); + setData( doc, 'foo[]bar' ); editor.execute( 'delete' ); - expect( getData( doc, { selection: true } ) ).to.equal( '

foo[]bar

' ); + expect( getData( doc, { selection: true } ) ).to.equal( 'foo[]bar' ); } ); it( 'does not try to delete when selection is at the boundary', () => { const spy = sinon.spy(); editor.data.on( 'deleteContent', spy ); - setData( doc, '

[]foo

' ); + setData( doc, '[]foo' ); editor.execute( 'delete' ); - expect( getData( doc, { selection: true } ) ).to.equal( '

[]foo

' ); + expect( getData( doc, { selection: true } ) ).to.equal( '[]foo' ); expect( spy.callCount ).to.equal( 0 ); } ); @@ -100,7 +101,7 @@ describe( 'DeleteCommand', () => { const spy = sinon.spy(); editor.data.on( 'modifySelection', spy ); - setData( doc, '

foo[]bar

' ); + setData( doc, 'foo[]bar' ); editor.commands.get( 'delete' ).direction = 'forward'; @@ -112,5 +113,86 @@ describe( 'DeleteCommand', () => { expect( modifyOpts ).to.have.property( 'direction', 'forward' ); expect( modifyOpts ).to.have.property( 'unit', 'word' ); } ); + + it( 'leaves an empty paragraph after removing the whole content from editor', () => { + setData( doc, '[Header 1Some text.]' ); + + editor.execute( 'delete' ); + + expect( getData( doc, { selection: true } ) ).to.equal( '[]' ); + } ); + + it( 'leaves an empty paragraph after removing the whole content inside limit element', () => { + doc.schema.registerItem( 'section', '$root' ); + doc.schema.limits.add( 'section' ); + doc.schema.allow( { name: 'section', inside: '$root' } ); + + setData( doc, + 'Foo' + + '
' + + '[Header 1' + + 'Some text.]' + + '
' + + 'Bar.' + ); + + editor.execute( 'delete' ); + + expect( getData( doc, { selection: true } ) ).to.equal( + 'Foo' + + '
' + + '[]' + + '
' + + 'Bar.' + ); + } ); + + it( 'leaves an empty paragraph after removing the whole content when root element was not added as Schema.limits', () => { + doc.schema.limits.delete( '$root' ); + + setData( doc, '[]' ); + + editor.execute( 'delete' ); + + expect( getData( doc ) ).to.equal( '[]' ); + } ); + + it( 'replaces an empty element with paragraph', () => { + setData( doc, '[]' ); + + editor.execute( 'delete' ); + + expect( getData( doc, { selection: true } ) ).to.equal( '[]' ); + } ); + + it( 'does not replace an element when Backspace or Delete key is held', () => { + setData( doc, 'Bar[]' ); + + for ( let sequence = 1; sequence < 10; ++sequence ) { + editor.execute( 'delete', { sequence } ); + } + + expect( getData( doc, { selection: true } ) ).to.equal( '[]' ); + } ); + + it( 'does not replace an element if a paragraph is a common ancestor', () => { + setData( doc, '[]' ); + + const element = doc.selection.getFirstRange().getCommonAncestor(); + + editor.execute( 'delete' ); + + expect( element ).is.equal( doc.selection.getFirstRange().getCommonAncestor() ); + } ); + + it( 'does not replace an element if a paragraph is not allowed in current position', () => { + doc.schema.disallow( { name: 'paragraph', inside: '$root' } ); + + setData( doc, '[]' ); + + editor.execute( 'delete' ); + + expect( getData( doc, { selection: true } ) ).to.equal( '[]' ); + } ); } ); } ); diff --git a/tests/deleteobserver.js b/tests/deleteobserver.js index ad32de4..3b76c07 100644 --- a/tests/deleteobserver.js +++ b/tests/deleteobserver.js @@ -40,6 +40,7 @@ describe( 'DeleteObserver', () => { const data = spy.args[ 0 ][ 1 ]; expect( data ).to.have.property( 'direction', 'forward' ); expect( data ).to.have.property( 'unit', 'character' ); + expect( data ).to.have.property( 'sequence', 1 ); } ); it( 'is fired with a proper direction and unit', () => { @@ -57,6 +58,7 @@ describe( 'DeleteObserver', () => { const data = spy.args[ 0 ][ 1 ]; expect( data ).to.have.property( 'direction', 'backward' ); expect( data ).to.have.property( 'unit', 'word' ); + expect( data ).to.have.property( 'sequence', 1 ); } ); it( 'is not fired on keydown when keyCode does not match backspace or delete', () => { @@ -70,6 +72,101 @@ describe( 'DeleteObserver', () => { expect( spy.calledOnce ).to.be.false; } ); + + it( 'is fired with a proper sequence number', () => { + const spy = sinon.spy(); + + viewDocument.on( 'delete', spy ); + + // Simulate that a user keeps the "Delete" key. + for ( let i = 0; i < 5; ++i ) { + viewDocument.fire( 'keydown', new DomEventData( viewDocument, getDomEvent(), { + keyCode: getCode( 'delete' ) + } ) ); + } + + expect( spy.callCount ).to.equal( 5 ); + + expect( spy.args[ 0 ][ 1 ] ).to.have.property( 'sequence', 1 ); + expect( spy.args[ 1 ][ 1 ] ).to.have.property( 'sequence', 2 ); + expect( spy.args[ 2 ][ 1 ] ).to.have.property( 'sequence', 3 ); + expect( spy.args[ 3 ][ 1 ] ).to.have.property( 'sequence', 4 ); + expect( spy.args[ 4 ][ 1 ] ).to.have.property( 'sequence', 5 ); + } ); + + it( 'clears the sequence when the key was released', () => { + const spy = sinon.spy(); + + viewDocument.on( 'delete', spy ); + + // Simulate that a user keeps the "Delete" key. + for ( let i = 0; i < 3; ++i ) { + viewDocument.fire( 'keydown', new DomEventData( viewDocument, getDomEvent(), { + keyCode: getCode( 'delete' ) + } ) ); + } + + // Then the user has released the key. + viewDocument.fire( 'keyup', new DomEventData( viewDocument, getDomEvent(), { + keyCode: getCode( 'delete' ) + } ) ); + + // And pressed it once again. + viewDocument.fire( 'keydown', new DomEventData( viewDocument, getDomEvent(), { + keyCode: getCode( 'delete' ) + } ) ); + + expect( spy.callCount ).to.equal( 4 ); + + expect( spy.args[ 0 ][ 1 ] ).to.have.property( 'sequence', 1 ); + expect( spy.args[ 1 ][ 1 ] ).to.have.property( 'sequence', 2 ); + expect( spy.args[ 2 ][ 1 ] ).to.have.property( 'sequence', 3 ); + expect( spy.args[ 3 ][ 1 ] ).to.have.property( 'sequence', 1 ); + } ); + + it( 'works fine with Backspace key', () => { + const spy = sinon.spy(); + + viewDocument.on( 'delete', spy ); + + viewDocument.fire( 'keydown', new DomEventData( viewDocument, getDomEvent(), { + keyCode: getCode( 'backspace' ) + } ) ); + + viewDocument.fire( 'keyup', new DomEventData( viewDocument, getDomEvent(), { + keyCode: getCode( 'backspace' ) + } ) ); + + viewDocument.fire( 'keydown', new DomEventData( viewDocument, getDomEvent(), { + keyCode: getCode( 'backspace' ) + } ) ); + + expect( spy.callCount ).to.equal( 2 ); + + expect( spy.args[ 0 ][ 1 ] ).to.have.property( 'sequence', 1 ); + expect( spy.args[ 1 ][ 1 ] ).to.have.property( 'sequence', 1 ); + } ); + + it( 'does not reset the sequence if other than Backspace or Delete key was released', () => { + const spy = sinon.spy(); + + viewDocument.on( 'delete', spy ); + + viewDocument.fire( 'keydown', new DomEventData( viewDocument, getDomEvent(), { + keyCode: getCode( 'delete' ) + } ) ); + + viewDocument.fire( 'keyup', new DomEventData( viewDocument, getDomEvent(), { + keyCode: getCode( 'A' ) + } ) ); + + viewDocument.fire( 'keydown', new DomEventData( viewDocument, getDomEvent(), { + keyCode: getCode( 'delete' ) + } ) ); + + expect( spy.args[ 0 ][ 1 ] ).to.have.property( 'sequence', 1 ); + expect( spy.args[ 1 ][ 1 ] ).to.have.property( 'sequence', 2 ); + } ); } ); function getDomEvent() { From 22bc2ab69e426bc35f9f1f79b739a52a6578e018 Mon Sep 17 00:00:00 2001 From: Kamil Piechaczek Date: Fri, 21 Jul 2017 10:14:49 +0200 Subject: [PATCH 02/18] Added more TCs to manual tests. [skip ci] --- tests/manual/delete.md | 6 +++++- tests/manual/input.md | 3 ++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/tests/manual/delete.md b/tests/manual/delete.md index 2f48fb9..fefdd24 100644 --- a/tests/manual/delete.md +++ b/tests/manual/delete.md @@ -3,4 +3,8 @@ Check: * collapsed selection (by letter, by word, whole line), -* non-collapsed selections. +* non-collapsed selections, +* put the selection at the end of **Heading 1**, **press and hold** the Backspace. +After releasing the key you should be able typing inside the header. +* clear the whole editor and choose **Heading 1** from the dropdown. Press the Backspace. +After typing, your text should be wrapped in a paragraph. diff --git a/tests/manual/input.md b/tests/manual/input.md index c1d9adf..2093292 100644 --- a/tests/manual/input.md +++ b/tests/manual/input.md @@ -3,7 +3,8 @@ Check: * normal typing, -* typing into non-collapsed selection. +* typing into non-collapsed selection, +* typing when the entire content is selected - new content should be wrapped in a paragraph. ### IME From 3380d32c3b3b30dee4bd175df54d2832475a92d5 Mon Sep 17 00:00:00 2001 From: Kamil Piechaczek Date: Mon, 24 Jul 2017 20:10:27 +0200 Subject: [PATCH 03/18] Improved docs. [skip ci] --- src/deletecommand.js | 3 +-- src/deleteobserver.js | 4 ++-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/deletecommand.js b/src/deletecommand.js index 79ff2fd..3220fa9 100644 --- a/src/deletecommand.js +++ b/src/deletecommand.js @@ -112,8 +112,7 @@ export default class DeleteCommand extends Command { * - other paragraph does not occur in the editor. * * @private - * @param {Object} options - * @param {Number} options.sequence A number that describes which sequence of the same event is fired. + * @param {Number} sequence A number describing which subsequent delete event it is without the key being released. * @returns {Boolean} */ _shouldEntireContentBeReplacedWithParagraph( options ) { diff --git a/src/deleteobserver.js b/src/deleteobserver.js index e091051..32e61dd 100644 --- a/src/deleteobserver.js +++ b/src/deleteobserver.js @@ -64,6 +64,6 @@ export default class DeleteObserver extends Observer { * @param {module:engine/view/observer/domeventdata~DomEventData} data * @param {'forward'|'delete'} data.direction The direction in which the deletion should happen. * @param {'character'|'word'} data.unit The "amount" of content that should be deleted. - * @param {Number} data.sequence A number that describes which sequence of the same event is fired. - * It helps detect the key was pressed and held. + * @param {Number} data.sequence A number describing which subsequent delete event it is without the key being released. + * If it's 2 or more it means that the key was pressed and hold. */ From 6558d59fd9d90fe6807810f8e80209150f9c27c6 Mon Sep 17 00:00:00 2001 From: Kamil Piechaczek Date: Mon, 24 Jul 2017 20:13:15 +0200 Subject: [PATCH 04/18] Simplified the private API of `DeleteCommand#_shouldEntireContentBeReplacedWithParagraph()` method. --- src/deletecommand.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/deletecommand.js b/src/deletecommand.js index 3220fa9..aa0fed9 100644 --- a/src/deletecommand.js +++ b/src/deletecommand.js @@ -78,7 +78,7 @@ export default class DeleteCommand extends Command { if ( selection.isCollapsed ) { const sequence = options.sequence || 1; - if ( this._shouldEntireContentBeReplacedWithParagraph( { sequence } ) ) { + if ( this._shouldEntireContentBeReplacedWithParagraph( sequence ) ) { this._replaceEntireContentWithParagraph(); } @@ -115,9 +115,9 @@ export default class DeleteCommand extends Command { * @param {Number} sequence A number describing which subsequent delete event it is without the key being released. * @returns {Boolean} */ - _shouldEntireContentBeReplacedWithParagraph( options ) { + _shouldEntireContentBeReplacedWithParagraph( sequence ) { // Does nothing if user pressed and held the "Backspace" or "Delete" key. - if ( options.sequence > 1 ) { + if ( sequence > 1 ) { return false; } From ef6a30ebb5f5424aa23a8f27cec2061fafd6f998 Mon Sep 17 00:00:00 2001 From: Kamil Piechaczek Date: Mon, 24 Jul 2017 20:13:45 +0200 Subject: [PATCH 05/18] Simplified the code in order to avoid repeating the code with the engine. --- src/deletecommand.js | 40 +++++++++++++--------------------------- tests/deletecommand.js | 3 ++- 2 files changed, 15 insertions(+), 28 deletions(-) diff --git a/src/deletecommand.js b/src/deletecommand.js index aa0fed9..e4e848f 100644 --- a/src/deletecommand.js +++ b/src/deletecommand.js @@ -9,8 +9,6 @@ import Command from '@ckeditor/ckeditor5-core/src/command'; import Selection from '@ckeditor/ckeditor5-engine/src/model/selection'; -import Element from '@ckeditor/ckeditor5-engine/src/model/element'; -import Position from '@ckeditor/ckeditor5-engine/src/model/position'; import Range from '@ckeditor/ckeditor5-engine/src/model/range'; import ChangeBuffer from './changebuffer'; import count from '@ckeditor/ckeditor5-utils/src/count'; @@ -107,9 +105,12 @@ export default class DeleteCommand extends Command { * the whole element without removing them. * * But, if the user pressed and released the key, we want to replace the entire content with a paragraph if: - * - the entire content is selected, - * - the paragraph is allowed in the common ancestor, - * - other paragraph does not occur in the editor. + * + * * the entire content is selected, + * * the paragraph is allowed in the common ancestor, + * * other paragraph does not occur in the editor. + * + * Rest of the checks are done in {@link module:engine/controller/datacontroller~DataController#deleteContent} method. * * @private * @param {Number} sequence A number describing which subsequent delete event it is without the key being released. @@ -123,23 +124,10 @@ export default class DeleteCommand extends Command { const document = this.editor.document; const selection = document.selection; - const limitElement = getLimitElement( document.schema, selection ); - const limitStartPosition = Position.createAt( limitElement ); - const limitEndPosition = Position.createAt( limitElement, 'end' ); - - if ( - !limitStartPosition.isTouching( selection.getFirstPosition() ) || - !limitEndPosition.isTouching( selection.getLastPosition() ) - ) { - return false; - } + const commonAncestor = selection.getFirstRange().getCommonAncestor(); - if ( !document.schema.check( { name: 'paragraph', inside: limitElement.name } ) ) { - return false; - } - - // Does nothing if editor contains an empty paragraph. - if ( selection.getFirstRange().getCommonAncestor().name === 'paragraph' ) { + // Does nothing if editor already contains an empty paragraph. + if ( commonAncestor.name === 'paragraph' ) { return false; } @@ -152,15 +140,13 @@ export default class DeleteCommand extends Command { * @private */ _replaceEntireContentWithParagraph() { - const document = this.editor.document; + const editor = this.editor; + const document = editor.document; const selection = document.selection; const limitElement = getLimitElement( document.schema, selection ); - const paragraph = new Element( 'paragraph' ); - - this._buffer.batch.remove( Range.createIn( limitElement ) ); - this._buffer.batch.insert( Position.createAt( limitElement ), paragraph ); - selection.collapse( paragraph ); + selection.setRanges( [ Range.createIn( limitElement ) ], selection.isBackward ); + editor.data.deleteContent( selection, this._buffer.batch, { skipParentsCheck: true } ); } } diff --git a/tests/deletecommand.js b/tests/deletecommand.js index c5ce81a..5699b6d 100644 --- a/tests/deletecommand.js +++ b/tests/deletecommand.js @@ -185,13 +185,14 @@ describe( 'DeleteCommand', () => { expect( element ).is.equal( doc.selection.getFirstRange().getCommonAncestor() ); } ); - it( 'does not replace an element if a paragraph is not allowed in current position', () => { + xit( 'does not replace an element if a paragraph is not allowed in current position', () => { doc.schema.disallow( { name: 'paragraph', inside: '$root' } ); setData( doc, '[]' ); editor.execute( 'delete' ); + // Returned data: '[]' instead of the heading element. expect( getData( doc, { selection: true } ) ).to.equal( '[]' ); } ); } ); From 60bccf5dfb6c2455b0c1aa4cc6b62491ae93926e Mon Sep 17 00:00:00 2001 From: Kamil Piechaczek Date: Tue, 25 Jul 2017 08:08:53 +0200 Subject: [PATCH 06/18] Revert: ef6a30e. It was not good idea to follow this way. --- src/deletecommand.js | 37 ++++++++++++++++++++++++++----------- tests/deletecommand.js | 2 +- 2 files changed, 27 insertions(+), 12 deletions(-) diff --git a/src/deletecommand.js b/src/deletecommand.js index e4e848f..91b9921 100644 --- a/src/deletecommand.js +++ b/src/deletecommand.js @@ -9,6 +9,8 @@ import Command from '@ckeditor/ckeditor5-core/src/command'; import Selection from '@ckeditor/ckeditor5-engine/src/model/selection'; +import Element from '@ckeditor/ckeditor5-engine/src/model/element'; +import Position from '@ckeditor/ckeditor5-engine/src/model/position'; import Range from '@ckeditor/ckeditor5-engine/src/model/range'; import ChangeBuffer from './changebuffer'; import count from '@ckeditor/ckeditor5-utils/src/count'; @@ -106,11 +108,9 @@ export default class DeleteCommand extends Command { * * But, if the user pressed and released the key, we want to replace the entire content with a paragraph if: * - * * the entire content is selected, - * * the paragraph is allowed in the common ancestor, - * * other paragraph does not occur in the editor. - * - * Rest of the checks are done in {@link module:engine/controller/datacontroller~DataController#deleteContent} method. + * * the entire content is selected, + * * the paragraph is allowed in the common ancestor, + * * other paragraph does not occur in the editor. * * @private * @param {Number} sequence A number describing which subsequent delete event it is without the key being released. @@ -124,10 +124,23 @@ export default class DeleteCommand extends Command { const document = this.editor.document; const selection = document.selection; - const commonAncestor = selection.getFirstRange().getCommonAncestor(); + const limitElement = getLimitElement( document.schema, selection ); + const limitStartPosition = Position.createAt( limitElement ); + const limitEndPosition = Position.createAt( limitElement, 'end' ); + + if ( + !limitStartPosition.isTouching( selection.getFirstPosition() ) || + !limitEndPosition.isTouching( selection.getLastPosition() ) + ) { + return false; + } + + if ( !document.schema.check( { name: 'paragraph', inside: limitElement.name } ) ) { + return false; + } // Does nothing if editor already contains an empty paragraph. - if ( commonAncestor.name === 'paragraph' ) { + if ( selection.getFirstRange().getCommonAncestor().name === 'paragraph' ) { return false; } @@ -140,13 +153,15 @@ export default class DeleteCommand extends Command { * @private */ _replaceEntireContentWithParagraph() { - const editor = this.editor; - const document = editor.document; + const document = this.editor.document; const selection = document.selection; const limitElement = getLimitElement( document.schema, selection ); + const paragraph = new Element( 'paragraph' ); + + this._buffer.batch.remove( Range.createIn( limitElement ) ); + this._buffer.batch.insert( Position.createAt( limitElement ), paragraph ); - selection.setRanges( [ Range.createIn( limitElement ) ], selection.isBackward ); - editor.data.deleteContent( selection, this._buffer.batch, { skipParentsCheck: true } ); + selection.collapse( paragraph ); } } diff --git a/tests/deletecommand.js b/tests/deletecommand.js index 5699b6d..2c811ee 100644 --- a/tests/deletecommand.js +++ b/tests/deletecommand.js @@ -185,7 +185,7 @@ describe( 'DeleteCommand', () => { expect( element ).is.equal( doc.selection.getFirstRange().getCommonAncestor() ); } ); - xit( 'does not replace an element if a paragraph is not allowed in current position', () => { + it( 'does not replace an element if a paragraph is not allowed in current position', () => { doc.schema.disallow( { name: 'paragraph', inside: '$root' } ); setData( doc, '[]' ); From 291f0db2883e2a301cc10b341cd90740ccd0a1ee Mon Sep 17 00:00:00 2001 From: Kamil Piechaczek Date: Mon, 31 Jul 2017 14:31:07 +0200 Subject: [PATCH 07/18] Removed "getLimitElement()" function because it is already available as a method in Schema. --- src/deletecommand.js | 19 ++----------------- 1 file changed, 2 insertions(+), 17 deletions(-) diff --git a/src/deletecommand.js b/src/deletecommand.js index af43b25..ee009f3 100644 --- a/src/deletecommand.js +++ b/src/deletecommand.js @@ -131,7 +131,7 @@ export default class DeleteCommand extends Command { const document = this.editor.document; const selection = document.selection; - const limitElement = getLimitElement( document.schema, selection ); + const limitElement = document.schema.getLimitElement( selection ); const limitStartPosition = Position.createAt( limitElement ); const limitEndPosition = Position.createAt( limitElement, 'end' ); @@ -162,7 +162,7 @@ export default class DeleteCommand extends Command { _replaceEntireContentWithParagraph() { const document = this.editor.document; const selection = document.selection; - const limitElement = getLimitElement( document.schema, selection ); + const limitElement = document.schema.getLimitElement( selection ); const paragraph = new Element( 'paragraph' ); this._buffer.batch.remove( Range.createIn( limitElement ) ); @@ -171,18 +171,3 @@ export default class DeleteCommand extends Command { selection.collapse( paragraph ); } } - -// Returns the lowest limit element defined in `Schema.limits` for passed selection. -function getLimitElement( schema, selection ) { - let element = selection.getFirstRange().getCommonAncestor(); - - while ( !schema.limits.has( element.name ) ) { - if ( element.parent ) { - element = element.parent; - } else { - break; - } - } - - return element; -} From 52cde4769fa8cba434243e9ddc85f19fe4ffe5aa Mon Sep 17 00:00:00 2001 From: Kamil Piechaczek Date: Thu, 10 Aug 2017 09:18:42 +0200 Subject: [PATCH 08/18] Simplified the code (used "Selection#isEntireContentSelected()"). --- src/deletecommand.js | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/deletecommand.js b/src/deletecommand.js index ee009f3..fb128f8 100644 --- a/src/deletecommand.js +++ b/src/deletecommand.js @@ -132,13 +132,8 @@ export default class DeleteCommand extends Command { const document = this.editor.document; const selection = document.selection; const limitElement = document.schema.getLimitElement( selection ); - const limitStartPosition = Position.createAt( limitElement ); - const limitEndPosition = Position.createAt( limitElement, 'end' ); - if ( - !limitStartPosition.isTouching( selection.getFirstPosition() ) || - !limitEndPosition.isTouching( selection.getLastPosition() ) - ) { + if ( !selection.isEntireContentSelected( limitElement ) ) { return false; } From 4b364b9bfa07796ce4abece560a62012462a27ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotrek=20Koszuli=C5=84ski?= Date: Wed, 16 Aug 2017 13:44:17 +0200 Subject: [PATCH 09/18] Code style. --- tests/manual/delete.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/manual/delete.md b/tests/manual/delete.md index fefdd24..b9933ab 100644 --- a/tests/manual/delete.md +++ b/tests/manual/delete.md @@ -4,7 +4,7 @@ Check: * collapsed selection (by letter, by word, whole line), * non-collapsed selections, -* put the selection at the end of **Heading 1**, **press and hold** the Backspace. +* put the selection at the end of **Heading 1**, **press and hold** the Backspace. After releasing the key you should be able typing inside the header. * clear the whole editor and choose **Heading 1** from the dropdown. Press the Backspace. -After typing, your text should be wrapped in a paragraph. +After typing, your text should be wrapped in a paragraph. From 5e1608bacb49a77b580cf2293548bf445e1aeb3d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotrek=20Koszuli=C5=84ski?= Date: Thu, 17 Aug 2017 11:24:26 +0200 Subject: [PATCH 10/18] Improved docs. --- src/deletecommand.js | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/deletecommand.js b/src/deletecommand.js index fb128f8..6512e99 100644 --- a/src/deletecommand.js +++ b/src/deletecommand.js @@ -58,7 +58,8 @@ export default class DeleteCommand extends Command { * @fires execute * @param {Object} [options] The command options. * @param {'character'} [options.unit='character'] See {@link module:engine/controller/modifyselection~modifySelection}'s options. - * @param {Number} [options.sequence=1] See the {@link module:engine/view/document~Document#event:delete} event data. + * @param {Number} [options.sequence=1] A number describing which subsequent delete event it is without the key being released. + * See the {@link module:engine/view/document~Document#event:delete} event data. */ execute( options = {} ) { const doc = this.editor.document; @@ -110,15 +111,18 @@ export default class DeleteCommand extends Command { } /** - * If the user keeps Backspace or Delete key, we do nothing because the user can clear - * the whole element without removing them. + * If the user keeps Backspace or Delete key pressed, the content of the current + * editable will be cleared. However, this will not yet lead to resetting the remaining block to a paragraph + * (which happens e.g. when the user does Ctrl + A, Backspace). * - * But, if the user pressed and released the key, we want to replace the entire content with a paragraph if: + * But, if the user pressed the key again, we want to replace the entire content with a paragraph if: * * * the entire content is selected, * * the paragraph is allowed in the common ancestor, * * other paragraph does not occur in the editor. * + * See https://github.com/ckeditor/ckeditor5-typing/issues/61. + * * @private * @param {Number} sequence A number describing which subsequent delete event it is without the key being released. * @returns {Boolean} From 2b254e7e3226fc1337801f3b946f5844a69dd8ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotrek=20Koszuli=C5=84ski?= Date: Thu, 17 Aug 2017 11:38:17 +0200 Subject: [PATCH 11/18] =?UTF-8?q?Code=20refactoring=20=E2=80=93=20fixed=20?= =?UTF-8?q?semantics.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/deletecommand.js | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/src/deletecommand.js b/src/deletecommand.js index 6512e99..cd40097 100644 --- a/src/deletecommand.js +++ b/src/deletecommand.js @@ -82,14 +82,15 @@ export default class DeleteCommand extends Command { dataController.modifySelection( selection, { direction: this.direction, unit: options.unit } ); } - // If selection is still collapsed, then there's nothing to delete. - if ( selection.isCollapsed ) { - const sequence = options.sequence || 1; + // Check if deleting in an empty editor. See #61. + if ( this._shouldEntireContentBeReplacedWithParagraph( options.sequence || 1 ) ) { + this._replaceEntireContentWithParagraph(); - if ( this._shouldEntireContentBeReplacedWithParagraph( sequence ) ) { - this._replaceEntireContentWithParagraph(); - } + return; + } + // If selection is still collapsed, then there's nothing to delete. + if ( selection.isCollapsed ) { return; } @@ -115,9 +116,10 @@ export default class DeleteCommand extends Command { * editable will be cleared. However, this will not yet lead to resetting the remaining block to a paragraph * (which happens e.g. when the user does Ctrl + A, Backspace). * - * But, if the user pressed the key again, we want to replace the entire content with a paragraph if: + * But, if the user pressed the key in an empty editable for the first time, + * we want to replace the entire content with a paragraph if: * - * * the entire content is selected, + * * the current limit element is empty, * * the paragraph is allowed in the common ancestor, * * other paragraph does not occur in the editor. * @@ -136,8 +138,11 @@ export default class DeleteCommand extends Command { const document = this.editor.document; const selection = document.selection; const limitElement = document.schema.getLimitElement( selection ); + // If a collapsed selection contains the whole content it means that the content is empty + // (from the user perspective). + const limitElementIsEmpty = selection.isCollapsed && selection.isEntireContentSelected( limitElement ); - if ( !selection.isEntireContentSelected( limitElement ) ) { + if ( !limitElementIsEmpty ) { return false; } From 45e9beb3ff3615e5801512cd491cb335e62c29ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotrek=20Koszuli=C5=84ski?= Date: Thu, 17 Aug 2017 11:42:10 +0200 Subject: [PATCH 12/18] Aligned Selection usage to changes in the engine. --- src/deletecommand.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/deletecommand.js b/src/deletecommand.js index cd40097..7865dd4 100644 --- a/src/deletecommand.js +++ b/src/deletecommand.js @@ -172,6 +172,6 @@ export default class DeleteCommand extends Command { this._buffer.batch.remove( Range.createIn( limitElement ) ); this._buffer.batch.insert( Position.createAt( limitElement ), paragraph ); - selection.collapse( paragraph ); + selection.setCollapsedAt( paragraph ); } } From 54b4ad8549665905d095ed3f7c905998daad835f Mon Sep 17 00:00:00 2001 From: Kamil Piechaczek Date: Thu, 17 Aug 2017 11:56:22 +0200 Subject: [PATCH 13/18] Improved the docs and adjusted to changes in engine (Selection API). --- src/deletecommand.js | 13 ++++++++----- tests/deletecommand.js | 42 +++++++++++++++++++++++++++++------------- 2 files changed, 37 insertions(+), 18 deletions(-) diff --git a/src/deletecommand.js b/src/deletecommand.js index 7865dd4..d47dca0 100644 --- a/src/deletecommand.js +++ b/src/deletecommand.js @@ -120,8 +120,8 @@ export default class DeleteCommand extends Command { * we want to replace the entire content with a paragraph if: * * * the current limit element is empty, - * * the paragraph is allowed in the common ancestor, - * * other paragraph does not occur in the editor. + * * the paragraph is allowed in the limit element, + * * other empty paragraph does not occur in the limit element. * * See https://github.com/ckeditor/ckeditor5-typing/issues/61. * @@ -138,9 +138,10 @@ export default class DeleteCommand extends Command { const document = this.editor.document; const selection = document.selection; const limitElement = document.schema.getLimitElement( selection ); + // If a collapsed selection contains the whole content it means that the content is empty // (from the user perspective). - const limitElementIsEmpty = selection.isCollapsed && selection.isEntireContentSelected( limitElement ); + const limitElementIsEmpty = selection.isCollapsed && selection.containsEntireContent( limitElement ); if ( !limitElementIsEmpty ) { return false; @@ -150,8 +151,10 @@ export default class DeleteCommand extends Command { return false; } - // Does nothing if editor already contains an empty paragraph. - if ( selection.getFirstRange().getCommonAncestor().name === 'paragraph' ) { + const limitElementFirstChild = limitElement.getChild( 0 ); + + // Does nothing if limit element already contains an empty paragraph. + if ( limitElementFirstChild && limitElementFirstChild.name === 'paragraph' ) { return false; } diff --git a/tests/deletecommand.js b/tests/deletecommand.js index 99cff7b..406a832 100644 --- a/tests/deletecommand.js +++ b/tests/deletecommand.js @@ -147,7 +147,7 @@ describe( 'DeleteCommand', () => { editor.execute( 'delete' ); - expect( getData( doc, { selection: true } ) ).to.equal( '[]' ); + expect( getData( doc ) ).to.equal( '[]' ); } ); it( 'leaves an empty paragraph after removing the whole content inside limit element', () => { @@ -157,24 +157,40 @@ describe( 'DeleteCommand', () => { setData( doc, 'Foo' + - '
' + - '[Header 1' + - 'Some text.]' + - '
' + + '
' + + '[Header 1' + + 'Some text.]' + + '
' + 'Bar.' ); editor.execute( 'delete' ); - expect( getData( doc, { selection: true } ) ).to.equal( + expect( getData( doc ) ).to.equal( 'Foo' + '
' + - '[]' + + '[]' + '
' + 'Bar.' ); } ); + it( 'leaves an empty paragraph after removing another paragraph from block element', () => { + doc.schema.registerItem( 'section', '$block' ); + doc.schema.registerItem( 'blockQuote', '$block' ); + doc.schema.limits.add( 'section' ); + doc.schema.allow( { name: 'section', inside: '$root' } ); + doc.schema.allow( { name: 'paragraph', inside: 'section' } ); + doc.schema.allow( { name: 'blockQuote', inside: 'section' } ); + doc.schema.allow( { name: 'paragraph', inside: 'blockQuote' } ); + + setData( doc, '
[]
' ); + + editor.execute( 'delete' ); + + expect( getData( doc ) ).to.equal( '
[]
' ); + } ); + it( 'leaves an empty paragraph after removing the whole content when root element was not added as Schema.limits', () => { doc.schema.limits.delete( '$root' ); @@ -190,7 +206,7 @@ describe( 'DeleteCommand', () => { editor.execute( 'delete' ); - expect( getData( doc, { selection: true } ) ).to.equal( '[]' ); + expect( getData( doc ) ).to.equal( '[]' ); } ); it( 'does not replace an element when Backspace or Delete key is held', () => { @@ -200,17 +216,17 @@ describe( 'DeleteCommand', () => { editor.execute( 'delete', { sequence } ); } - expect( getData( doc, { selection: true } ) ).to.equal( '[]' ); + expect( getData( doc ) ).to.equal( '[]' ); } ); - it( 'does not replace an element if a paragraph is a common ancestor', () => { + it( 'does not replace with paragraph in another paragraph already occurs in limit element', () => { setData( doc, '[]' ); - const element = doc.selection.getFirstRange().getCommonAncestor(); + const element = doc.getRoot().getNodeByPath( [ 0 ] ); editor.execute( 'delete' ); - expect( element ).is.equal( doc.selection.getFirstRange().getCommonAncestor() ); + expect( element ).is.equal( doc.getRoot().getNodeByPath( [ 0 ] ) ); } ); it( 'does not replace an element if a paragraph is not allowed in current position', () => { @@ -221,7 +237,7 @@ describe( 'DeleteCommand', () => { editor.execute( 'delete' ); // Returned data: '[]' instead of the heading element. - expect( getData( doc, { selection: true } ) ).to.equal( '[]' ); + expect( getData( doc ) ).to.equal( '[]' ); } ); } ); } ); From 3347a7dc8dd84d97d9476afb49be06739aea076c Mon Sep 17 00:00:00 2001 From: Kamil Piechaczek Date: Fri, 18 Aug 2017 11:02:04 +0200 Subject: [PATCH 14/18] Removed additional options from "getData()" function. --- tests/deletecommand.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/deletecommand.js b/tests/deletecommand.js index 406a832..465c668 100644 --- a/tests/deletecommand.js +++ b/tests/deletecommand.js @@ -66,7 +66,7 @@ describe( 'DeleteCommand', () => { editor.execute( 'delete' ); - expect( getData( doc, { selection: true } ) ).to.equal( 'fo[]bar' ); + expect( getData( doc ) ).to.equal( 'fo[]bar' ); } ); it( 'deletes selection contents', () => { @@ -74,7 +74,7 @@ describe( 'DeleteCommand', () => { editor.execute( 'delete' ); - expect( getData( doc, { selection: true } ) ).to.equal( 'fo[]ar' ); + expect( getData( doc ) ).to.equal( 'fo[]ar' ); } ); it( 'merges elements', () => { @@ -82,7 +82,7 @@ describe( 'DeleteCommand', () => { editor.execute( 'delete' ); - expect( getData( doc, { selection: true } ) ).to.equal( 'foo[]bar' ); + expect( getData( doc ) ).to.equal( 'foo[]bar' ); } ); it( 'does not try to delete when selection is at the boundary', () => { @@ -93,7 +93,7 @@ describe( 'DeleteCommand', () => { editor.execute( 'delete' ); - expect( getData( doc, { selection: true } ) ).to.equal( '[]foo' ); + expect( getData( doc ) ).to.equal( '[]foo' ); expect( spy.callCount ).to.equal( 0 ); } ); From e1d027a7c236e2512d0263d739617b1402d71265 Mon Sep 17 00:00:00 2001 From: Kamil Piechaczek Date: Fri, 18 Aug 2017 13:26:25 +0200 Subject: [PATCH 15/18] Removed invalid comment. [skip ci] --- tests/deletecommand.js | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/deletecommand.js b/tests/deletecommand.js index 465c668..c35a33a 100644 --- a/tests/deletecommand.js +++ b/tests/deletecommand.js @@ -236,7 +236,6 @@ describe( 'DeleteCommand', () => { editor.execute( 'delete' ); - // Returned data: '[]' instead of the heading element. expect( getData( doc ) ).to.equal( '[]' ); } ); } ); From 7c030100c9e5c8b5e298bb60f51096877ff4ab13 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotrek=20Koszuli=C5=84ski?= Date: Tue, 22 Aug 2017 14:48:04 +0200 Subject: [PATCH 16/18] Code style. --- tests/manual/input.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/manual/input.md b/tests/manual/input.md index 2093292..9f65f20 100644 --- a/tests/manual/input.md +++ b/tests/manual/input.md @@ -4,7 +4,7 @@ Check: * normal typing, * typing into non-collapsed selection, -* typing when the entire content is selected - new content should be wrapped in a paragraph. +* typing when the entire content is selected - new content should be wrapped in a paragraph. ### IME From 7bf6feda98772b7384a589a80b50f8767b1163aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotrek=20Koszuli=C5=84ski?= Date: Tue, 22 Aug 2017 14:54:54 +0200 Subject: [PATCH 17/18] Fixed tests. --- tests/deletecommand.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/deletecommand.js b/tests/deletecommand.js index 0c65311..24ebe12 100644 --- a/tests/deletecommand.js +++ b/tests/deletecommand.js @@ -46,11 +46,11 @@ describe( 'DeleteCommand', () => { // We expect that command is executed in enqueue changes block. Since we are already in // an enqueued block, the command execution will be postponed. Hence, no changes. - expect( getData( doc ) ).to.equal( '

foo[]bar

' ); + expect( getData( doc ) ).to.equal( 'foo[]bar' ); } ); // After all enqueued changes are done, the command execution is reflected. - expect( getData( doc ) ).to.equal( '

fo[]bar

' ); + expect( getData( doc ) ).to.equal( 'fo[]bar' ); } ); it( 'locks buffer when executing', () => { From 9f57e6f57274c3ff032f620b94d77a7e675bfb66 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotrek=20Koszuli=C5=84ski?= Date: Tue, 22 Aug 2017 15:26:36 +0200 Subject: [PATCH 18/18] Improved comments. --- src/deletecommand.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/deletecommand.js b/src/deletecommand.js index d47dca0..4b6274d 100644 --- a/src/deletecommand.js +++ b/src/deletecommand.js @@ -121,7 +121,7 @@ export default class DeleteCommand extends Command { * * * the current limit element is empty, * * the paragraph is allowed in the limit element, - * * other empty paragraph does not occur in the limit element. + * * the limit doesn't already have a paragraph inside. * * See https://github.com/ckeditor/ckeditor5-typing/issues/61. * @@ -153,7 +153,9 @@ export default class DeleteCommand extends Command { const limitElementFirstChild = limitElement.getChild( 0 ); - // Does nothing if limit element already contains an empty paragraph. + // Does nothing if the limit element already contains only a paragraph. + // We ignore the case when paragraph might have some inline elements (

[]

) + // because we don't support such cases yet and it's unclear whether inlineWidget shouldn't be a limit itself. if ( limitElementFirstChild && limitElementFirstChild.name === 'paragraph' ) { return false; }