diff --git a/packages/ckeditor5-table/src/converters/table-cell-refresh-post-fixer.js b/packages/ckeditor5-table/src/converters/table-cell-refresh-post-fixer.js index d8d12f682b0..9440a0b615f 100644 --- a/packages/ckeditor5-table/src/converters/table-cell-refresh-post-fixer.js +++ b/packages/ckeditor5-table/src/converters/table-cell-refresh-post-fixer.js @@ -28,10 +28,21 @@ function tableCellRefreshPostFixer( model ) { // Stores cells to be refreshed so the table cell will be refreshed once for multiple changes. const cellsToRefresh = new Set(); + // Counting the paragraph inserts to verify if it increased to more than one paragraph in the current differ. + let insertCount = 0; + for ( const change of differ.getChanges() ) { const parent = change.type == 'insert' || change.type == 'remove' ? change.position.parent : change.range.start.parent; - if ( parent.is( 'tableCell' ) && checkRefresh( parent, change.type ) ) { + if ( !parent.is( 'tableCell' ) ) { + continue; + } + + if ( change.type == 'insert' ) { + insertCount++; + } + + if ( checkRefresh( parent, change.type, insertCount ) ) { cellsToRefresh.add( parent ); } } @@ -60,7 +71,8 @@ function tableCellRefreshPostFixer( model ) { // // @param {module:engine/model/element~Element} tableCell The table cell to check. // @param {String} type Type of change. -function checkRefresh( tableCell, type ) { +// @param {Number} insertCount The number of inserts in differ. +function checkRefresh( tableCell, type, insertCount ) { const hasInnerParagraph = Array.from( tableCell.getChildren() ).some( child => child.is( 'paragraph' ) ); // If there is no paragraph in table cell then the view doesn't require refreshing. @@ -83,5 +95,7 @@ function checkRefresh( tableCell, type ) { // // - another element is added to a single paragraph (childCount becomes >= 2) // - another element is removed and a single paragraph is left (childCount == 1) - return tableCell.childCount <= ( type == 'insert' ? 2 : 1 ); + // + // Change is not needed if there were multiple blocks before change. + return tableCell.childCount <= ( type == 'insert' ? insertCount + 1 : 1 ); } diff --git a/packages/ckeditor5-table/tests/converters/table-cell-refresh-post-fixer.js b/packages/ckeditor5-table/tests/converters/table-cell-refresh-post-fixer.js index d32dfe2a495..a23ba63af0a 100644 --- a/packages/ckeditor5-table/tests/converters/table-cell-refresh-post-fixer.js +++ b/packages/ckeditor5-table/tests/converters/table-cell-refresh-post-fixer.js @@ -55,18 +55,16 @@ describe( 'Table cell refresh post-fixer', () => { return editor.destroy(); } ); - it( 'should rename to

when adding more elements to the same table cell', () => { + it( 'should rename to

when adding element to the same table cell', () => { editor.setData( viewTable( [ [ '

00

' ] ] ) ); const table = root.getChild( 0 ); model.change( writer => { const nodeByPath = table.getNodeByPath( [ 0, 0, 0 ] ); - const paragraph = writer.createElement( 'paragraph' ); writer.insert( paragraph, nodeByPath, 'after' ); - writer.setSelection( nodeByPath.nextSibling, 0 ); } ); @@ -76,18 +74,37 @@ describe( 'Table cell refresh post-fixer', () => { sinon.assert.calledOnce( refreshItemSpy ); } ); - it( 'should rename to

on adding other block element to the same table cell', () => { + it( 'should rename to

when adding more elements to the same table cell', () => { editor.setData( viewTable( [ [ '

00

' ] ] ) ); const table = root.getChild( 0 ); model.change( writer => { const nodeByPath = table.getNodeByPath( [ 0, 0, 0 ] ); + const paragraph1 = writer.createElement( 'paragraph' ); + const paragraph2 = writer.createElement( 'paragraph' ); - const paragraph = writer.createElement( 'block' ); + writer.insert( paragraph1, nodeByPath, 'after' ); + writer.insert( paragraph2, nodeByPath, 'after' ); + writer.setSelection( nodeByPath.nextSibling, 0 ); + } ); - writer.insert( paragraph, nodeByPath, 'after' ); + assertEqualMarkup( getViewData( view, { withoutSelection: true } ), viewTable( [ + [ '

00

' ] + ], { asWidget: true } ) ); + sinon.assert.calledOnce( refreshItemSpy ); + } ); + it( 'should rename to

on adding other block element to the same table cell', () => { + editor.setData( viewTable( [ [ '

00

' ] ] ) ); + + const table = root.getChild( 0 ); + + model.change( writer => { + const nodeByPath = table.getNodeByPath( [ 0, 0, 0 ] ); + const block = writer.createElement( 'block' ); + + writer.insert( block, nodeByPath, 'after' ); writer.setSelection( nodeByPath.nextSibling, 0 ); } ); @@ -97,6 +114,27 @@ describe( 'Table cell refresh post-fixer', () => { sinon.assert.calledOnce( refreshItemSpy ); } ); + it( 'should rename to

on adding multiple other block elements to the same table cell', () => { + editor.setData( viewTable( [ [ '

00

' ] ] ) ); + + const table = root.getChild( 0 ); + + model.change( writer => { + const nodeByPath = table.getNodeByPath( [ 0, 0, 0 ] ); + const block1 = writer.createElement( 'block' ); + const block2 = writer.createElement( 'block' ); + + writer.insert( block1, nodeByPath, 'after' ); + writer.insert( block2, nodeByPath, 'after' ); + writer.setSelection( nodeByPath.nextSibling, 0 ); + } ); + + assertEqualMarkup( getViewData( view, { withoutSelection: true } ), viewTable( [ + [ '

00

' ] + ], { asWidget: true } ) ); + sinon.assert.calledOnce( refreshItemSpy ); + } ); + it( 'should properly rename the same element on consecutive changes', () => { editor.setData( viewTable( [ [ '

00

' ] ] ) ); @@ -140,7 +178,7 @@ describe( 'Table cell refresh post-fixer', () => { sinon.assert.calledOnce( refreshItemSpy ); } ); - it( 'should rename

to when removing all but one paragraph inside table cell', () => { + it( 'should rename

to when removing one of two paragraphs inside table cell', () => { editor.setData( viewTable( [ [ '

00

foo

' ] ] ) ); const table = root.getChild( 0 ); @@ -155,6 +193,22 @@ describe( 'Table cell refresh post-fixer', () => { sinon.assert.calledOnce( refreshItemSpy ); } ); + it( 'should rename

to when removing all but one paragraph inside table cell', () => { + editor.setData( viewTable( [ [ '

00

foo

bar

' ] ] ) ); + + const table = root.getChild( 0 ); + + model.change( writer => { + writer.remove( table.getNodeByPath( [ 0, 0, 1 ] ) ); + writer.remove( table.getNodeByPath( [ 0, 0, 1 ] ) ); + } ); + + assertEqualMarkup( getViewData( view, { withoutSelection: true } ), viewTable( [ + [ '00' ] + ], { asWidget: true } ) ); + sinon.assert.calledOnce( refreshItemSpy ); + } ); + it( 'should rename

to when removing attribute from ', () => { editor.setData( '

00

' ); @@ -250,6 +304,21 @@ describe( 'Table cell refresh post-fixer', () => { sinon.assert.notCalled( refreshItemSpy ); } ); + it( 'should do nothing on adding to existing paragraphs', () => { + editor.setData( viewTable( [ [ '

a

b

' ] ] ) ); + + const table = root.getChild( 0 ); + + model.change( writer => { + writer.insertElement( 'paragraph', table.getNodeByPath( [ 0, 0, 1 ] ), 'after' ); + } ); + + assertEqualMarkup( getViewData( view, { withoutSelection: true } ), viewTable( [ + [ '

a

b

' ] + ], { asWidget: true } ) ); + sinon.assert.notCalled( refreshItemSpy ); + } ); + it( 'should do nothing when setting attribute on block item other then ', () => { editor.setData( viewTable( [ [ '
foo
' ] ] ) );