diff --git a/src/commands/removerowcommand.js b/src/commands/removerowcommand.js index 641d5b25..e4737791 100644 --- a/src/commands/removerowcommand.js +++ b/src/commands/removerowcommand.js @@ -60,7 +60,10 @@ export default class RemoveRowCommand extends Command { const columnIndexToFocus = this.editor.plugins.get( 'TableUtils' ).getCellLocation( firstCell ).column; - model.change( writer => { + // Use single batch to modify table in steps but in one undo step. + const batch = model.createBatch(); + + model.enqueueChange( batch, writer => { // This prevents the "model-selection-range-intersects" error, caused by removing row selected cells. writer.setSelection( writer.createSelection( table, 'on' ) ); @@ -68,9 +71,12 @@ export default class RemoveRowCommand extends Command { this.editor.plugins.get( 'TableUtils' ).removeRows( table, { at: removedRowIndexes.first, - rows: rowsToRemove + rows: rowsToRemove, + batch } ); + } ); + model.enqueueChange( batch, writer => { const cellToFocus = getCellToFocus( table, removedRowIndexes.first, columnIndexToFocus ); writer.setSelection( writer.createPositionAt( cellToFocus, 0 ) ); diff --git a/src/converters/downcast.js b/src/converters/downcast.js index a32f8998..cbd9ad0a 100644 --- a/src/converters/downcast.js +++ b/src/converters/downcast.js @@ -209,9 +209,6 @@ export function downcastTableHeadingRowsChange( options = {} ) { renameViewTableCell( tableCell, 'th', conversionApi, asWidget ); } } - - // Cleanup: this will remove any empty section from the view which may happen when moving all rows from a table section. - removeTableSectionIfEmpty( 'tbody', viewTable, conversionApi ); } // The head section has shrunk so move rows from to . else { @@ -234,11 +231,12 @@ export function downcastTableHeadingRowsChange( options = {} ) { for ( const tableWalkerValue of tableWalker ) { renameViewTableCellIfRequired( tableWalkerValue, tableAttributes, conversionApi, asWidget ); } - - // Cleanup: this will remove any empty section from the view which may happen when moving all rows from a table section. - removeTableSectionIfEmpty( 'thead', viewTable, conversionApi ); } + // Cleanup: Ensure that thead & tbody sections are removed if left empty after moving rows. See #6437, #6391. + removeTableSectionIfEmpty( 'thead', viewTable, conversionApi ); + removeTableSectionIfEmpty( 'tbody', viewTable, conversionApi ); + function isBetween( index, lower, upper ) { return index > lower && index < upper; } @@ -298,6 +296,7 @@ export function downcastRemoveRow() { const viewStart = mapper.toViewPosition( data.position ).getLastMatchingPosition( value => !value.item.is( 'tr' ) ); const viewItem = viewStart.nodeAfter; const tableSection = viewItem.parent; + const viewTable = tableSection.parent; // Remove associated from the view. const removeRange = viewWriter.createRangeOn( viewItem ); @@ -307,11 +306,9 @@ export function downcastRemoveRow() { mapper.unbindViewElement( child ); } - // Check if table section has any children left - if not remove it from the view. - if ( !tableSection.childCount ) { - // No need to unbind anything as table section is not represented in the model. - viewWriter.remove( viewWriter.createRangeOn( tableSection ) ); - } + // Cleanup: Ensure that thead & tbody sections are removed if left empty after removing rows. See #6437, #6391. + removeTableSectionIfEmpty( 'thead', viewTable, conversionApi ); + removeTableSectionIfEmpty( 'tbody', viewTable, conversionApi ); }, { priority: 'higher' } ); } diff --git a/src/tableutils.js b/src/tableutils.js index dad1ab2b..91f50ee7 100644 --- a/src/tableutils.js +++ b/src/tableutils.js @@ -287,6 +287,7 @@ export default class TableUtils extends Plugin { const rowsToRemove = options.rows || 1; const first = options.at; const last = first + rowsToRemove - 1; + const batch = options.batch || 'default'; // Removing rows from table requires most calculations to be done prior to changing table structure. @@ -294,7 +295,7 @@ export default class TableUtils extends Plugin { const { cellsToMove, cellsToTrim } = getCellsToMoveAndTrimOnRemoveRow( table, first, last ); // 2. Execution - model.change( writer => { + model.enqueueChange( batch, writer => { // 2a. Move cells from removed rows that extends over a removed section - must be done before removing rows. // This will fill any gaps in a rows below that previously were empty because of row-spanned cells. const rowAfterRemovedSection = last + 1; @@ -311,7 +312,7 @@ export default class TableUtils extends Plugin { } // 2d. Adjust heading rows if removed rows were in a heading section. - updateHeadingRows( table, first, last, model, writer.batch ); + updateHeadingRows( table, first, last, model, batch ); } ); } diff --git a/tests/commands/removerowcommand.js b/tests/commands/removerowcommand.js index fcd80606..75fb74ae 100644 --- a/tests/commands/removerowcommand.js +++ b/tests/commands/removerowcommand.js @@ -9,22 +9,19 @@ import { getData as getViewData } from '@ckeditor/ckeditor5-engine/src/dev-utils import RemoveRowCommand from '../../src/commands/removerowcommand'; import TableSelection from '../../src/tableselection'; -import { defaultConversion, defaultSchema, modelTable, viewTable } from '../_utils/utils'; +import { modelTable, viewTable } from '../_utils/utils'; import { assertEqualMarkup } from '@ckeditor/ckeditor5-utils/tests/_utils/utils'; +import TableEditing from '../../src/tableediting'; +import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph'; describe( 'RemoveRowCommand', () => { let editor, model, command; - beforeEach( () => { - return VirtualTestEditor.create( { plugins: [ TableSelection ] } ) - .then( newEditor => { - editor = newEditor; - model = editor.model; - command = new RemoveRowCommand( editor ); + beforeEach( async () => { + editor = await VirtualTestEditor.create( { plugins: [ Paragraph, TableEditing, TableSelection ] } ); - defaultSchema( model.schema ); - defaultConversion( editor.conversion ); - } ); + model = editor.model; + command = new RemoveRowCommand( editor ); } ); afterEach( () => { @@ -290,11 +287,11 @@ describe( 'RemoveRowCommand', () => { [ '[]40', '41' ] ], { headingRows: 1 } ) ); - // The view should also be properly downcasted. + // The editing view should also be properly downcasted. assertEqualMarkup( getViewData( editor.editing.view, { withoutSelection: true } ), viewTable( [ [ '00', '01' ], [ '40', '41' ] - ], { headingRows: 1 } ) ); + ], { headingRows: 1, asWidget: true } ) ); } ); it( 'should support removing mixed heading and cell rows', () => { @@ -509,8 +506,8 @@ describe( 'RemoveRowCommand', () => { setData( model, modelTable( [ [ { rowspan: 4, contents: '00' }, { rowspan: 3, contents: '01' }, { rowspan: 2, contents: '02' }, '03', '04' ], [ { rowspan: 2, contents: '13' }, '14' ], - [ '22[]', '23', '24' ], - [ '30', '31', '32', '33', '34' ] + [ '22[]', '24' ], + [ '31', '32', '33', '34' ] ] ) ); command.execute(); @@ -518,7 +515,7 @@ describe( 'RemoveRowCommand', () => { assertEqualMarkup( getData( model ), modelTable( [ [ { rowspan: 3, contents: '00' }, { rowspan: 2, contents: '01' }, { rowspan: 2, contents: '02' }, '03', '04' ], [ '13', '14' ], - [ '30', '31', '[]32', '33', '34' ] + [ '31', '32', '[]33', '34' ] ] ) ); } ); diff --git a/tests/converters/downcast.js b/tests/converters/downcast.js index e40ded71..8160f4b0 100644 --- a/tests/converters/downcast.js +++ b/tests/converters/downcast.js @@ -4,13 +4,15 @@ */ import VirtualTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/virtualtesteditor'; -import { getData as getViewData } from '@ckeditor/ckeditor5-engine/src/dev-utils/view'; -import { setData as setModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model'; +import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph'; -import { defaultConversion, defaultSchema, modelTable, viewTable } from '../_utils/utils'; +import TableEditing from '../../src/tableediting'; import env from '@ckeditor/ckeditor5-utils/src/env'; import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils'; +import { getData as getViewData } from '@ckeditor/ckeditor5-engine/src/dev-utils/view'; +import { setData as setModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model'; import { assertEqualMarkup } from '@ckeditor/ckeditor5-utils/tests/_utils/utils'; +import { defaultConversion, defaultSchema, modelTable, viewTable } from '../_utils/utils'; function paragraphInTableCell() { return dispatcher => dispatcher.on( 'insert:paragraph', ( evt, data, conversionApi ) => { @@ -42,24 +44,25 @@ describe( 'downcast converters', () => { testUtils.createSinonSandbox(); - beforeEach( () => { - // Most tests assume non-edge environment but we do not set `contenteditable=false` on Edge so stub `env.isEdge`. - testUtils.sinon.stub( env, 'isEdge' ).get( () => false ); + describe( 'downcastInsertTable()', () => { + // The beforeEach is duplicated due to ckeditor/ckeditor5#6574. New test are written using TableEditing. + beforeEach( () => { + // Most tests assume non-edge environment but we do not set `contenteditable=false` on Edge so stub `env.isEdge`. + testUtils.sinon.stub( env, 'isEdge' ).get( () => false ); - return VirtualTestEditor.create() - .then( newEditor => { - editor = newEditor; - model = editor.model; - doc = model.document; - root = doc.getRoot( 'main' ); - view = editor.editing.view; + return VirtualTestEditor.create() + .then( newEditor => { + editor = newEditor; + model = editor.model; + doc = model.document; + root = doc.getRoot( 'main' ); + view = editor.editing.view; - defaultSchema( model.schema ); - defaultConversion( editor.conversion ); - } ); - } ); + defaultSchema( model.schema ); + defaultConversion( editor.conversion ); + } ); + } ); - describe( 'downcastInsertTable()', () => { it( 'should create table with tbody', () => { setModelData( model, modelTable( [ [ '' ] ] ) ); @@ -355,6 +358,24 @@ describe( 'downcast converters', () => { } ); describe( 'downcastInsertRow()', () => { + // The beforeEach is duplicated due to ckeditor/ckeditor5#6574. New test are written using TableEditing. + beforeEach( () => { + // Most tests assume non-edge environment but we do not set `contenteditable=false` on Edge so stub `env.isEdge`. + testUtils.sinon.stub( env, 'isEdge' ).get( () => false ); + + return VirtualTestEditor.create() + .then( newEditor => { + editor = newEditor; + model = editor.model; + doc = model.document; + root = doc.getRoot( 'main' ); + view = editor.editing.view; + + defaultSchema( model.schema ); + defaultConversion( editor.conversion ); + } ); + } ); + it( 'should react to changed rows', () => { setModelData( model, modelTable( [ [ '00', '01' ] @@ -592,6 +613,24 @@ describe( 'downcast converters', () => { } ); describe( 'downcastInsertCell()', () => { + // The beforeEach is duplicated due to ckeditor/ckeditor5#6574. New test are written using TableEditing. + beforeEach( () => { + // Most tests assume non-edge environment but we do not set `contenteditable=false` on Edge so stub `env.isEdge`. + testUtils.sinon.stub( env, 'isEdge' ).get( () => false ); + + return VirtualTestEditor.create() + .then( newEditor => { + editor = newEditor; + model = editor.model; + doc = model.document; + root = doc.getRoot( 'main' ); + view = editor.editing.view; + + defaultSchema( model.schema ); + defaultConversion( editor.conversion ); + } ); + } ); + it( 'should add tableCell on proper index in tr', () => { setModelData( model, modelTable( [ [ '00', '01' ] @@ -736,6 +775,24 @@ describe( 'downcast converters', () => { } ); describe( 'downcastTableHeadingColumnsChange()', () => { + // The beforeEach is duplicated due to ckeditor/ckeditor5#6574. New test are written using TableEditing. + beforeEach( () => { + // Most tests assume non-edge environment but we do not set `contenteditable=false` on Edge so stub `env.isEdge`. + testUtils.sinon.stub( env, 'isEdge' ).get( () => false ); + + return VirtualTestEditor.create() + .then( newEditor => { + editor = newEditor; + model = editor.model; + doc = model.document; + root = doc.getRoot( 'main' ); + view = editor.editing.view; + + defaultSchema( model.schema ); + defaultConversion( editor.conversion ); + } ); + } ); + it( 'should work for adding heading columns', () => { setModelData( model, modelTable( [ [ '00', '01' ], @@ -895,7 +952,7 @@ describe( 'downcast converters', () => { assertEqualMarkup( getViewData( view, { withoutSelection: true } ), '
' + - '
' + + '
' + '' + '' + '' + @@ -912,190 +969,201 @@ describe( 'downcast converters', () => { } ); describe( 'downcastTableHeadingRowsChange()', () => { - it( 'should work for adding heading rows', () => { - setModelData( model, modelTable( [ - [ '00', '01' ], - [ '10', '11' ], - [ '20', '21' ] - ] ) ); - - const table = root.getChild( 0 ); - - model.change( writer => { - writer.setAttribute( 'headingRows', 2, table ); - } ); + // The beforeEach is duplicated due to ckeditor/ckeditor5#6574. New test are written using TableEditing. + beforeEach( () => { + // Most tests assume non-edge environment but we do not set `contenteditable=false` on Edge so stub `env.isEdge`. + testUtils.sinon.stub( env, 'isEdge' ).get( () => false ); - assertEqualMarkup( getViewData( view, { withoutSelection: true } ), viewTable( [ - [ '00', '01' ], - [ '10', '11' ], - [ '20', '21' ] - ], { headingRows: 2 } ) ); + return VirtualTestEditor.create( { plugins: [ Paragraph, TableEditing ] } ) + .then( newEditor => { + editor = newEditor; + model = editor.model; + doc = model.document; + root = doc.getRoot( 'main' ); + view = editor.editing.view; + } ); } ); - it( 'should work for changing number of heading rows to a bigger number', () => { - setModelData( model, modelTable( [ - [ '00', '01' ], - [ '10', '11' ], - [ '20', '21' ] - ], { headingRows: 1 } ) ); + // The heading rows change downcast conversion is not executed in data pipeline. + describe( 'editing pipeline', () => { + it( 'should work for adding heading rows', () => { + setModelData( model, modelTable( [ + [ '00', '01' ], + [ '10', '11' ], + [ '20', '21' ] + ] ) ); - const table = root.getChild( 0 ); + const table = root.getChild( 0 ); - model.change( writer => { - writer.setAttribute( 'headingRows', 2, table ); + model.change( writer => { + writer.setAttribute( 'headingRows', 2, table ); + } ); + + assertEqualMarkup( getViewData( view, { withoutSelection: true } ), viewTable( [ + [ '00', '01' ], + [ '10', '11' ], + [ '20', '21' ] + ], { headingRows: 2, asWidget: true } ) ); } ); - assertEqualMarkup( getViewData( view, { withoutSelection: true } ), viewTable( [ - [ '00', '01' ], - [ '10', '11' ], - [ '20', '21' ] - ], { headingRows: 2 } ) ); - } ); + it( 'should work for changing number of heading rows to a bigger number', () => { + setModelData( model, modelTable( [ + [ '00', '01' ], + [ '10', '11' ], + [ '20', '21' ] + ], { headingRows: 1 } ) ); - it( 'should work for changing number of heading rows to a smaller number', () => { - setModelData( model, modelTable( [ - [ '00', '01' ], - [ '10', '11' ], - [ '20', '21' ], - [ '30', '31' ] - ], { headingRows: 3 } ) ); + const table = root.getChild( 0 ); - const table = root.getChild( 0 ); + model.change( writer => { + writer.setAttribute( 'headingRows', 2, table ); + } ); - model.change( writer => { - writer.setAttribute( 'headingRows', 2, table ); + assertEqualMarkup( getViewData( view, { withoutSelection: true } ), viewTable( [ + [ '00', '01' ], + [ '10', '11' ], + [ '20', '21' ] + ], { headingRows: 2, asWidget: true } ) ); } ); - assertEqualMarkup( getViewData( view, { withoutSelection: true } ), viewTable( [ - [ '00', '01' ], - [ '10', '11' ], - [ '20', '21' ], - [ '30', '31' ] - ], { headingRows: 2 } ) ); - } ); + it( 'should work for changing number of heading rows to a smaller number', () => { + setModelData( model, modelTable( [ + [ '00', '01' ], + [ '10', '11' ], + [ '20', '21' ], + [ '30', '31' ] + ], { headingRows: 3 } ) ); - it( 'should work for removing heading rows', () => { - setModelData( model, modelTable( [ - [ '00', '01' ], - [ '10', '11' ] - ], { headingRows: 2 } ) ); + const table = root.getChild( 0 ); - const table = root.getChild( 0 ); + model.change( writer => { + writer.setAttribute( 'headingRows', 2, table ); + } ); - model.change( writer => { - writer.removeAttribute( 'headingRows', table ); + assertEqualMarkup( getViewData( view, { withoutSelection: true } ), viewTable( [ + [ '00', '01' ], + [ '10', '11' ], + [ '20', '21' ], + [ '30', '31' ] + ], { headingRows: 2, asWidget: true } ) ); } ); - assertEqualMarkup( getViewData( view, { withoutSelection: true } ), viewTable( [ - [ '00', '01' ], - [ '10', '11' ] - ] ) ); - } ); + it( 'should work for removing heading rows', () => { + setModelData( model, modelTable( [ + [ '00', '01' ], + [ '10', '11' ] + ], { headingRows: 2 } ) ); - it( 'should work for making heading rows without tbody', () => { - setModelData( model, modelTable( [ - [ '00', '01' ], - [ '10', '11' ] - ] ) ); + const table = root.getChild( 0 ); - const table = root.getChild( 0 ); + model.change( writer => { + writer.removeAttribute( 'headingRows', table ); + } ); - model.change( writer => { - writer.setAttribute( 'headingRows', 2, table ); + assertEqualMarkup( getViewData( view, { withoutSelection: true } ), viewTable( [ + [ '00', '01' ], + [ '10', '11' ] + ], { asWidget: true } ) ); } ); - assertEqualMarkup( getViewData( view, { withoutSelection: true } ), viewTable( [ - [ '00', '01' ], - [ '10', '11' ] - ], { headingRows: 2 } ) ); - } ); + it( 'should work for making heading rows without tbody', () => { + setModelData( model, modelTable( [ + [ '00', '01' ], + [ '10', '11' ] + ] ) ); - it( 'should be possible to overwrite', () => { - editor.conversion.attributeToAttribute( { model: 'headingRows', view: 'headingRows', converterPriority: 'high' } ); - setModelData( model, modelTable( [ [ '00' ] ] ) ); + const table = root.getChild( 0 ); - const table = root.getChild( 0 ); + model.change( writer => { + writer.setAttribute( 'headingRows', 2, table ); + } ); - model.change( writer => { - writer.setAttribute( 'headingRows', 1, table ); + assertEqualMarkup( getViewData( view, { withoutSelection: true } ), viewTable( [ + [ '00', '01' ], + [ '10', '11' ] + ], { headingRows: 2, asWidget: true } ) ); } ); - assertEqualMarkup( getViewData( view, { withoutSelection: true } ), - '
' + - '
' + - '' + - '' + - '' + - '
00
' + - '
' - ); - } ); - - it( 'should work with adding table rows at the beginning of a table', () => { - setModelData( model, modelTable( [ - [ '00', '01' ], - [ '10', '11' ] - ], { headingRows: 1 } ) ); - - const table = root.getChild( 0 ); + it( 'should be possible to overwrite', () => { + editor.conversion.attributeToAttribute( { + model: 'headingRows', + view: 'headingRows', + converterPriority: 'high' + } ); + setModelData( model, modelTable( [ [ '00' ] ] ) ); - model.change( writer => { - writer.setAttribute( 'headingRows', 2, table ); + const table = root.getChild( 0 ); - const tableRow = writer.createElement( 'tableRow' ); + model.change( writer => { + writer.setAttribute( 'headingRows', 1, table ); + } ); - writer.insert( tableRow, table, 0 ); - writer.insertElement( 'tableCell', tableRow, 'end' ); - writer.insertElement( 'tableCell', tableRow, 'end' ); + assertEqualMarkup( getViewData( view, { withoutSelection: true } ), + '
' + + '
' + + '' + + '' + + '' + + '' + + '' + + '' + + '
' + + '00' + + '
' + + '
' + ); } ); - assertEqualMarkup( getViewData( view, { withoutSelection: true } ), viewTable( [ - [ '', '' ], - [ '00', '01' ], - [ '10', '11' ] - ], { headingRows: 2 } ) ); - } ); + it( 'should work with adding table rows at the beginning of a table', () => { + setModelData( model, modelTable( [ + [ '00', '01' ], + [ '10', '11' ] + ], { headingRows: 1 } ) ); - it( 'should work with adding a table row and expanding heading', () => { - setModelData( model, modelTable( [ - [ '00', '01' ], - [ '10', '11' ], - [ '20', '21' ] - ], { headingRows: 1 } ) ); + const table = root.getChild( 0 ); - const table = root.getChild( 0 ); + model.change( writer => { + writer.setAttribute( 'headingRows', 2, table ); - model.change( writer => { - writer.setAttribute( 'headingRows', 2, table ); + const tableRow = writer.createElement( 'tableRow' ); - const tableRow = writer.createElement( 'tableRow' ); + writer.insert( tableRow, table, 0 ); + writer.insertElement( 'tableCell', tableRow, 'end' ); + writer.insertElement( 'tableCell', tableRow, 'end' ); + } ); - writer.insert( tableRow, table, 1 ); - writer.insertElement( 'tableCell', tableRow, 'end' ); - writer.insertElement( 'tableCell', tableRow, 'end' ); + assertEqualMarkup( getViewData( view, { withoutSelection: true } ), viewTable( [ + [ '', '' ], + [ '00', '01' ], + [ '10', '11' ] + ], { headingRows: 2, asWidget: true } ) ); } ); - assertEqualMarkup( getViewData( view, { withoutSelection: true } ), viewTable( [ - [ '00', '01' ], - [ '', '' ], - [ '10', '11' ], - [ '20', '21' ] - ], { headingRows: 2 } ) ); - } ); + it( 'should work with adding a table row and expanding heading', () => { + setModelData( model, modelTable( [ + [ '00', '01' ], + [ '10', '11' ], + [ '20', '21' ] + ], { headingRows: 1 } ) ); - describe( 'options.asWidget=true', () => { - beforeEach( () => { - return VirtualTestEditor.create() - .then( newEditor => { - editor = newEditor; - model = editor.model; - doc = model.document; - root = doc.getRoot( 'main' ); - view = editor.editing.view; + const table = root.getChild( 0 ); - defaultSchema( model.schema ); - defaultConversion( editor.conversion, true ); - } ); + model.change( writer => { + writer.setAttribute( 'headingRows', 2, table ); + + const tableRow = writer.createElement( 'tableRow' ); + + writer.insert( tableRow, table, 1 ); + writer.insertElement( 'tableCell', tableRow, 'end' ); + writer.insertElement( 'tableCell', tableRow, 'end' ); + } ); + + assertEqualMarkup( getViewData( view, { withoutSelection: true } ), viewTable( [ + [ '00', '01' ], + [ '', '' ], + [ '10', '11' ], + [ '20', '21' ] + ], { headingRows: 2, asWidget: true } ) ); } ); it( 'should create renamed cell as a widget', () => { @@ -1126,107 +1194,257 @@ describe( 'downcast converters', () => { } ); describe( 'downcastRemoveRow()', () => { - it( 'should react to removed row from the beginning of a tbody', () => { - setModelData( model, modelTable( [ - [ '00[]', '01' ], - [ '10', '11' ] - ] ) ); + // The beforeEach is duplicated due to ckeditor/ckeditor5#6574. New test are written using TableEditing. + beforeEach( async () => { + // Most tests assume non-edge environment but we do not set `contenteditable=false` on Edge so stub `env.isEdge`. + testUtils.sinon.stub( env, 'isEdge' ).get( () => false ); - const table = root.getChild( 0 ); + editor = await VirtualTestEditor.create( { plugins: [ Paragraph, TableEditing ] } ); - model.change( writer => { - writer.remove( table.getChild( 1 ) ); + model = editor.model; + root = model.document.getRoot( 'main' ); + view = editor.editing.view; + } ); + + // The remove row downcast conversion is not executed in data pipeline. + describe( 'editing pipeline', () => { + it( 'should react to removed row from the beginning of a body rows (no heading rows)', () => { + setModelData( model, modelTable( [ + [ '00[]', '01' ], + [ '10', '11' ] + ] ) ); + + const table = root.getChild( 0 ); + + model.change( writer => { + writer.remove( table.getChild( 1 ) ); + } ); + + assertEqualMarkup( getViewData( view, { withoutSelection: true } ), + '
' + + '
' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '
' + + '00' + + '' + + '01' + + '
' + + '
' + ); } ); - assertEqualMarkup( getViewData( view, { withoutSelection: true } ), viewTable( [ - [ '00', '01' ] - ] ) ); - } ); + it( 'should react to removed row form the end of a body rows (no heading rows)', () => { + setModelData( model, modelTable( [ + [ '00[]', '01' ], + [ '10', '11' ] + ] ) ); - it( 'should react to removed row form the end of a tbody', () => { - setModelData( model, modelTable( [ - [ '00[]', '01' ], - [ '10', '11' ] - ] ) ); + const table = root.getChild( 0 ); - const table = root.getChild( 0 ); + model.change( writer => { + writer.remove( table.getChild( 0 ) ); + } ); - model.change( writer => { - writer.remove( table.getChild( 0 ) ); + assertEqualMarkup( getViewData( view, { withoutSelection: true } ), + '
' + + '
' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '
' + + '10' + + '' + + '11' + + '
' + + '
' + ); } ); - assertEqualMarkup( getViewData( view, { withoutSelection: true } ), viewTable( [ - [ '10', '11' ] - ] ) ); - } ); + it( 'should react to removed row from the beginning of a heading rows (no body rows)', () => { + setModelData( model, modelTable( [ + [ '00[]', '01' ], + [ '10', '11' ] + ], { headingRows: 2 } ) ); - it( 'should react to removed row from the beginning of a thead', () => { - setModelData( model, modelTable( [ - [ '00[]', '01' ], - [ '10', '11' ] - ], { headingRows: 2 } ) ); + const table = root.getChild( 0 ); - const table = root.getChild( 0 ); + model.change( writer => { + // Removing row from a heading section changes requires changing heading rows attribute. + writer.setAttribute( 'headingRows', 1, table ); + writer.remove( table.getChild( 0 ) ); + } ); - model.change( writer => { - writer.remove( table.getChild( 1 ) ); + assertEqualMarkup( getViewData( view, { withoutSelection: true } ), + '
' + + '
' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '
' + + '10' + + '' + + '11' + + '
' + + '
' + ); } ); - assertEqualMarkup( getViewData( view, { withoutSelection: true } ), viewTable( [ - [ '00', '01' ] - ], { headingRows: 2 } ) ); - } ); + it( 'should react to removed row form the end of a heading rows (no body rows)', () => { + setModelData( model, modelTable( [ + [ '00[]', '01' ], + [ '10', '11' ] + ], { headingRows: 2 } ) ); - it( 'should react to removed row form the end of a thead', () => { - setModelData( model, modelTable( [ - [ '00[]', '01' ], - [ '10', '11' ] - ], { headingRows: 2 } ) ); + const table = root.getChild( 0 ); - const table = root.getChild( 0 ); + model.change( writer => { + // Removing row from a heading section changes requires changing heading rows attribute. + writer.setAttribute( 'headingRows', 1, table ); + writer.remove( table.getChild( 1 ) ); + } ); - model.change( writer => { - writer.remove( table.getChild( 0 ) ); + assertEqualMarkup( getViewData( view, { withoutSelection: true } ), + '
' + + '
' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '
' + + '00' + + '' + + '01' + + '
' + + '
' + ); } ); - assertEqualMarkup( getViewData( view, { withoutSelection: true } ), viewTable( [ - [ '10', '11' ] - ], { headingRows: 2 } ) ); - } ); + it( 'should react to removed row form the end of a heading rows (first cell in body has colspan)', () => { + setModelData( model, modelTable( [ + [ '00[]', '01', '02', '03' ], + [ { rowspan: 2, colspan: 2, contents: '10' }, '12', '13' ], + [ '22', '23' ] + ], { headingRows: 1 } ) ); - it( 'should remove empty thead section if a last row was removed from thead', () => { - setModelData( model, modelTable( [ - [ '00[]', '01' ], - [ '10', '11' ] - ], { headingRows: 1 } ) ); + const table = root.getChild( 0 ); - const table = root.getChild( 0 ); + model.change( writer => { + // Removing row from a heading section changes requires changing heading rows attribute. + writer.remove( table.getChild( 0 ) ); + writer.setAttribute( 'headingRows', 0, table ); + } ); - model.change( writer => { - writer.setAttribute( 'headingRows', 0, table ); - writer.remove( table.getChild( 0 ) ); + assertEqualMarkup( getViewData( view, { withoutSelection: true } ), + '
' + + '
' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '
' + + '10' + + '' + + '12' + + '' + + '13' + + '
' + + '22' + + '' + + '23' + + '
' + + '
' + ); } ); - assertEqualMarkup( getViewData( view, { withoutSelection: true } ), viewTable( [ - [ '10', '11' ] - ] ) ); - } ); + it( 'should remove empty thead if a last row was removed from a heading rows (has heading and body)', () => { + setModelData( model, modelTable( [ + [ '00[]', '01' ], + [ '10', '11' ] + ], { headingRows: 1 } ) ); - it( 'should remove empty tbody section if a last row was removed from tbody', () => { - setModelData( model, modelTable( [ - [ '00[]', '01' ], - [ '10', '11' ] - ], { headingRows: 1 } ) ); + const table = root.getChild( 0 ); - const table = root.getChild( 0 ); + model.change( writer => { + // Removing row from a heading section changes requires changing heading rows attribute. + writer.removeAttribute( 'headingRows', table ); + writer.remove( table.getChild( 0 ) ); + } ); - model.change( writer => { - writer.remove( table.getChild( 1 ) ); + assertEqualMarkup( getViewData( view, { withoutSelection: true } ), + '
' + + '
' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '
' + + '10' + + '' + + '11' + + '
' + + '
' + ); } ); - assertEqualMarkup( getViewData( view, { withoutSelection: true } ), viewTable( [ - [ '00', '01' ] - ], { headingRows: 1 } ) ); + it( 'should remove empty tbody if a last row was removed a body rows (has heading and body)', () => { + setModelData( model, modelTable( [ + [ '00[]', '01' ], + [ '10', '11' ] + ], { headingRows: 1 } ) ); + + const table = root.getChild( 0 ); + + model.change( writer => { + writer.remove( table.getChild( 1 ) ); + } ); + + assertEqualMarkup( getViewData( view, { withoutSelection: true } ), + '
' + + '
' + + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '
' + + '00' + + '' + + '01' + + '
' + + '
' + ); + } ); } ); } ); diff --git a/tests/tableutils.js b/tests/tableutils.js index b02d8c4d..870412b1 100644 --- a/tests/tableutils.js +++ b/tests/tableutils.js @@ -1174,7 +1174,7 @@ describe( 'TableUtils', () => { ] ) ); } ); - it( 'should create one undo step (1 batch)', () => { + it( 'should re-use batch to create one undo step', () => { setData( model, modelTable( [ [ '00', '01' ], [ '10', '11' ], @@ -1190,7 +1190,9 @@ describe( 'TableUtils', () => { createdBatches.add( operation.batch ); } ); - tableUtils.removeRows( root.getChild( 0 ), { at: 0, rows: 2 } ); + const batch = model.createBatch(); + + tableUtils.removeRows( root.getChild( 0 ), { at: 0, rows: 2, batch } ); expect( createdBatches.size ).to.equal( 1 ); } );