From c9da83ce463b5d38063236f09bb2a5edba026f0a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Go=C5=82aszewski?= Date: Tue, 29 Oct 2019 16:33:34 +0100 Subject: [PATCH 1/2] BlockAutoformat should not react to text after inline element. --- src/blockautoformatediting.js | 24 +++++++++++++++++++++- tests/autoformat.js | 36 +++++++++++++++++++++++++++++++++ tests/blockautoformatediting.js | 28 +++++++++++++++++++++++++ 3 files changed, 87 insertions(+), 1 deletion(-) diff --git a/src/blockautoformatediting.js b/src/blockautoformatediting.js index 92e5f7f..9f53ca7 100644 --- a/src/blockautoformatediting.js +++ b/src/blockautoformatediting.js @@ -94,7 +94,9 @@ export default class BlockAutoformatEditing { return; } - const match = pattern.exec( item.data ); + const text = getStartText( editor.model.createRangeIn( item.parent ) ); + + const match = pattern.exec( text ); if ( !match ) { return; @@ -119,3 +121,23 @@ export default class BlockAutoformatEditing { } ); } } + +/** + * Returns the text from given renge up to the first inline element. + * + * @param {module:engine/model/range~Range} range + * @returns {String} + */ +export function getStartText( range ) { + let text = ''; + + for ( const item of range.getItems() ) { + if ( !( item.is( 'text' ) || item.is( 'textProxy' ) ) ) { + return text; + } + + text = text + item.data; + } + + return text; +} diff --git a/tests/autoformat.js b/tests/autoformat.js index 22a4543..bca08db 100644 --- a/tests/autoformat.js +++ b/tests/autoformat.js @@ -81,6 +81,15 @@ describe( 'Autoformat', () => { expect( getData( model ) ).to.equal( '- []' ); } ); + + it( 'should not replace asterisk character after ', () => { + setData( model, 'Foo*[]' ); + model.change( writer => { + writer.insertText( ' ', doc.selection.getFirstPosition() ); + } ); + + expect( getData( model ) ).to.equal( 'Foo* []' ); + } ); } ); describe( 'Numbered list', () => { @@ -128,6 +137,15 @@ describe( 'Autoformat', () => { expect( getData( model ) ).to.equal( '3. []' ); } ); + + it( 'should not replace digit character after ', () => { + setData( model, 'Foo1.[]' ); + model.change( writer => { + writer.insertText( ' ', doc.selection.getFirstPosition() ); + } ); + + expect( getData( model ) ).to.equal( 'Foo1. []' ); + } ); } ); describe( 'Heading', () => { @@ -212,6 +230,15 @@ describe( 'Autoformat', () => { expect( getData( model ) ).to.equal( '# []' ); } ); + + it( 'should not replace hash character after ', () => { + setData( model, 'Foo#[]' ); + model.change( writer => { + writer.insertText( ' ', doc.selection.getFirstPosition() ); + } ); + + expect( getData( model ) ).to.equal( 'Foo# []' ); + } ); } ); describe( 'Block quote', () => { @@ -250,6 +277,15 @@ describe( 'Autoformat', () => { expect( getData( model ) ).to.equal( '1. > []' ); } ); + + it( 'should not replace greater-than character after ', () => { + setData( model, 'Foo>[]' ); + model.change( writer => { + writer.insertText( ' ', doc.selection.getFirstPosition() ); + } ); + + expect( getData( model ) ).to.equal( 'Foo> []' ); + } ); } ); describe( 'Inline autoformat', () => { diff --git a/tests/blockautoformatediting.js b/tests/blockautoformatediting.js index 2a62c58..6af16a6 100644 --- a/tests/blockautoformatediting.js +++ b/tests/blockautoformatediting.js @@ -128,6 +128,34 @@ describe( 'BlockAutoformatEditing', () => { sinon.assert.notCalled( spy ); } ); + it( 'should not call callback when after inline element', () => { + // Configure the schema. + model.schema.register( 'softBreak', { + allowWhere: '$text', + isInline: true + } ); + editor.conversion.for( 'upcast' ) + .elementToElement( { + model: 'softBreak', + view: 'br' + } ); + editor.conversion.for( 'downcast' ) + .elementToElement( { + model: 'softBreak', + view: ( modelElement, viewWriter ) => viewWriter.createEmptyElement( 'br' ) + } ); + + const spy = testUtils.sinon.spy(); + new BlockAutoformatEditing( editor, /^[*]\s/, spy ); // eslint-disable-line no-new + + setData( model, '*[]' ); + model.change( writer => { + writer.insertText( ' ', doc.selection.getFirstPosition() ); + } ); + + sinon.assert.notCalled( spy ); + } ); + it( 'should stop if callback returned false', () => { new BlockAutoformatEditing( editor, /^[*]\s$/, () => false ); // eslint-disable-line no-new From c049bf5a6cf96bdac1babfae3a025c51020a7f5a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Go=C5=82aszewski?= Date: Mon, 4 Nov 2019 14:17:13 +0100 Subject: [PATCH 2/2] Simplify block autoformat trigger logic. --- src/blockautoformatediting.js | 34 +++++++-------------------------- tests/blockautoformatediting.js | 29 ++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 27 deletions(-) diff --git a/src/blockautoformatediting.js b/src/blockautoformatediting.js index 9f53ca7..09593fb 100644 --- a/src/blockautoformatediting.js +++ b/src/blockautoformatediting.js @@ -88,16 +88,16 @@ export default class BlockAutoformatEditing { return; } - const item = entry.position.textNode || entry.position.nodeAfter; + const blockToFormat = entry.position.parent; - if ( !item.parent.is( 'paragraph' ) ) { + // Block formatting should trigger only if the entire content of a paragraph is a single text node... (see ckeditor5#5671). + if ( !blockToFormat.is( 'paragraph' ) || blockToFormat.childCount !== 1 ) { return; } - const text = getStartText( editor.model.createRangeIn( item.parent ) ); - - const match = pattern.exec( text ); + const match = pattern.exec( blockToFormat.getChild( 0 ).data ); + // ...and this text node's data match the pattern. if ( !match ) { return; } @@ -105,8 +105,8 @@ export default class BlockAutoformatEditing { // Use enqueueChange to create new batch to separate typing batch from the auto-format changes. editor.model.enqueueChange( writer => { // Matched range. - const start = writer.createPositionAt( item.parent, 0 ); - const end = writer.createPositionAt( item.parent, match[ 0 ].length ); + const start = writer.createPositionAt( blockToFormat, 0 ); + const end = writer.createPositionAt( blockToFormat, match[ 0 ].length ); const range = new LiveRange( start, end ); const wasChanged = callback( { match } ); @@ -121,23 +121,3 @@ export default class BlockAutoformatEditing { } ); } } - -/** - * Returns the text from given renge up to the first inline element. - * - * @param {module:engine/model/range~Range} range - * @returns {String} - */ -export function getStartText( range ) { - let text = ''; - - for ( const item of range.getItems() ) { - if ( !( item.is( 'text' ) || item.is( 'textProxy' ) ) ) { - return text; - } - - text = text + item.data; - } - - return text; -} diff --git a/tests/blockautoformatediting.js b/tests/blockautoformatediting.js index 6af16a6..27b5c6c 100644 --- a/tests/blockautoformatediting.js +++ b/tests/blockautoformatediting.js @@ -156,6 +156,35 @@ describe( 'BlockAutoformatEditing', () => { sinon.assert.notCalled( spy ); } ); + it( 'should not call callback when after inline element (typing after softBreak in a "matching" paragraph)', () => { + // Configure the schema. + model.schema.register( 'softBreak', { + allowWhere: '$text', + isInline: true + } ); + editor.conversion.for( 'upcast' ) + .elementToElement( { + model: 'softBreak', + view: 'br' + } ); + editor.conversion.for( 'downcast' ) + .elementToElement( { + model: 'softBreak', + view: ( modelElement, viewWriter ) => viewWriter.createEmptyElement( 'br' ) + } ); + + const spy = testUtils.sinon.spy(); + new BlockAutoformatEditing( editor, /^[*]\s/, spy ); // eslint-disable-line no-new + + setData( model, '* []' ); + + model.change( writer => { + writer.insertText( ' ', doc.selection.getFirstPosition() ); + } ); + + sinon.assert.notCalled( spy ); + } ); + it( 'should stop if callback returned false', () => { new BlockAutoformatEditing( editor, /^[*]\s$/, () => false ); // eslint-disable-line no-new