diff --git a/src/commands/removerowcommand.js b/src/commands/removerowcommand.js index 7e716a5a..641d5b25 100644 --- a/src/commands/removerowcommand.js +++ b/src/commands/removerowcommand.js @@ -9,8 +9,7 @@ import Command from '@ckeditor/ckeditor5-core/src/command'; -import TableWalker from '../tablewalker'; -import { findAncestor, updateNumericAttribute } from './utils'; +import { findAncestor } from './utils'; import { getRowIndexes, getSelectionAffectedTableCells } from '../utils'; /** @@ -65,77 +64,18 @@ export default class RemoveRowCommand extends Command { // This prevents the "model-selection-range-intersects" error, caused by removing row selected cells. writer.setSelection( writer.createSelection( table, 'on' ) ); - let cellToFocus; + const rowsToRemove = removedRowIndexes.last - removedRowIndexes.first + 1; - for ( let i = removedRowIndexes.last; i >= removedRowIndexes.first; i-- ) { - const removedRowIndex = i; - this._removeRow( removedRowIndex, table, writer ); + this.editor.plugins.get( 'TableUtils' ).removeRows( table, { + at: removedRowIndexes.first, + rows: rowsToRemove + } ); - cellToFocus = getCellToFocus( table, removedRowIndex, columnIndexToFocus ); - } - - const model = this.editor.model; - const headingRows = table.getAttribute( 'headingRows' ) || 0; - - if ( headingRows && removedRowIndexes.first < headingRows ) { - const newRows = getNewHeadingRowsValue( removedRowIndexes, headingRows ); - - // Must be done after the changes in table structure (removing rows). - // Otherwise the downcast converter for headingRows attribute will fail. ckeditor/ckeditor5#6391. - model.enqueueChange( writer.batch, writer => { - updateNumericAttribute( 'headingRows', newRows, table, writer, 0 ); - } ); - } + const cellToFocus = getCellToFocus( table, removedRowIndexes.first, columnIndexToFocus ); writer.setSelection( writer.createPositionAt( cellToFocus, 0 ) ); } ); } - - /** - * Removes a row from the given `table`. - * - * @private - * @param {Number} removedRowIndex Index of the row that should be removed. - * @param {module:engine/model/element~Element} table - * @param {module:engine/model/writer~Writer} writer - */ - _removeRow( removedRowIndex, table, writer ) { - const cellsToMove = new Map(); - const tableRow = table.getChild( removedRowIndex ); - const tableMap = [ ...new TableWalker( table, { endRow: removedRowIndex } ) ]; - - // Get cells from removed row that are spanned over multiple rows. - tableMap - .filter( ( { row, rowspan } ) => row === removedRowIndex && rowspan > 1 ) - .forEach( ( { column, cell, rowspan } ) => cellsToMove.set( column, { cell, rowspanToSet: rowspan - 1 } ) ); - - // Reduce rowspan on cells that are above removed row and overlaps removed row. - tableMap - .filter( ( { row, rowspan } ) => row <= removedRowIndex - 1 && row + rowspan > removedRowIndex ) - .forEach( ( { cell, rowspan } ) => updateNumericAttribute( 'rowspan', rowspan - 1, cell, writer ) ); - - // Move cells to another row. - const targetRow = removedRowIndex + 1; - const tableWalker = new TableWalker( table, { includeSpanned: true, startRow: targetRow, endRow: targetRow } ); - let previousCell; - - for ( const { row, column, cell } of [ ...tableWalker ] ) { - if ( cellsToMove.has( column ) ) { - const { cell: cellToMove, rowspanToSet } = cellsToMove.get( column ); - const targetPosition = previousCell ? - writer.createPositionAfter( previousCell ) : - writer.createPositionAt( table.getChild( row ), 0 ); - writer.move( writer.createRangeOn( cellToMove ), targetPosition ); - updateNumericAttribute( 'rowspan', rowspanToSet, cellToMove, writer ); - previousCell = cellToMove; - } - else { - previousCell = cell; - } - } - - writer.remove( tableRow ); - } } // Returns a cell that should be focused before removing the row, belonging to the same column as the currently focused cell. @@ -159,12 +99,3 @@ function getCellToFocus( table, removedRowIndex, columnToFocus ) { return cellToFocus; } - -// Calculates a new heading rows value for removing rows from heading section. -function getNewHeadingRowsValue( removedRowIndexes, headingRows ) { - if ( removedRowIndexes.last < headingRows ) { - return headingRows - ( ( removedRowIndexes.last - removedRowIndexes.first ) + 1 ); - } - - return removedRowIndexes.first; -} diff --git a/src/tableutils.js b/src/tableutils.js index 14cf4b88..efe541c6 100644 --- a/src/tableutils.js +++ b/src/tableutils.js @@ -253,6 +253,60 @@ export default class TableUtils extends Plugin { } ); } + /** + * Removes rows from the given `table`. + * + * This method re-calculates the table geometry including `rowspan` attribute of table cells overlapping removed rows + * and table headings values. + * + * editor.plugins.get( 'TableUtils' ).removeRows( table, { at: 1, rows: 2 } ); + * + * Executing the above code in the context of the table on the left will transform its structure as presented on the right: + * + * row index + * ┌───┬───┬───┐ `at` = 1 ┌───┬───┬───┐ + * 0 │ a │ b │ c │ `rows` = 2 │ a │ b │ c │ 0 + * │ ├───┼───┤ │ ├───┼───┤ + * 1 │ │ d │ e │ <-- remove from here │ │ h │ i │ 1 + * │ ├───┼───┤ will give: ├───┼───┼───┤ + * 2 │ │ f │ g │ │ j │ k │ l │ 2 + * │ ├───┼───┤ └───┴───┴───┘ + * 3 │ │ h │ i │ + * ├───┼───┼───┤ + * 4 │ j │ k │ l │ + * └───┴───┴───┘ + * + * @param {module:engine/model/element~Element} table + * @param {Object} options + * @param {Number} options.at The row index at which the removing rows will start. + * @param {Number} [options.rows=1] The number of rows to remove. + */ + removeRows( table, options ) { + const model = this.editor.model; + const first = options.at; + const rowsToRemove = options.rows || 1; + + const last = first + rowsToRemove - 1; + + model.change( writer => { + for ( let i = last; i >= first; i-- ) { + removeRow( table, i, writer ); + } + + const headingRows = table.getAttribute( 'headingRows' ) || 0; + + if ( headingRows && first < headingRows ) { + const newRows = getNewHeadingRowsValue( first, last, headingRows ); + + // Must be done after the changes in table structure (removing rows). + // Otherwise the downcast converter for headingRows attribute will fail. ckeditor/ckeditor5#6391. + model.enqueueChange( writer.batch, writer => { + updateNumericAttribute( 'headingRows', newRows, table, writer, 0 ); + } ); + } + } ); + } + /** * Divides a table cell vertically into several ones. * @@ -615,3 +669,49 @@ function breakSpanEvenly( span, numberOfCells ) { return { newCellsSpan, updatedSpan }; } + +function removeRow( table, rowIndex, writer ) { + const cellsToMove = new Map(); + const tableRow = table.getChild( rowIndex ); + const tableMap = [ ...new TableWalker( table, { endRow: rowIndex } ) ]; + + // Get cells from removed row that are spanned over multiple rows. + tableMap + .filter( ( { row, rowspan } ) => row === rowIndex && rowspan > 1 ) + .forEach( ( { column, cell, rowspan } ) => cellsToMove.set( column, { cell, rowspanToSet: rowspan - 1 } ) ); + + // Reduce rowspan on cells that are above removed row and overlaps removed row. + tableMap + .filter( ( { row, rowspan } ) => row <= rowIndex - 1 && row + rowspan > rowIndex ) + .forEach( ( { cell, rowspan } ) => updateNumericAttribute( 'rowspan', rowspan - 1, cell, writer ) ); + + // Move cells to another row. + const targetRow = rowIndex + 1; + const tableWalker = new TableWalker( table, { includeSpanned: true, startRow: targetRow, endRow: targetRow } ); + let previousCell; + + for ( const { row, column, cell } of [ ...tableWalker ] ) { + if ( cellsToMove.has( column ) ) { + const { cell: cellToMove, rowspanToSet } = cellsToMove.get( column ); + const targetPosition = previousCell ? + writer.createPositionAfter( previousCell ) : + writer.createPositionAt( table.getChild( row ), 0 ); + writer.move( writer.createRangeOn( cellToMove ), targetPosition ); + updateNumericAttribute( 'rowspan', rowspanToSet, cellToMove, writer ); + previousCell = cellToMove; + } else { + previousCell = cell; + } + } + + writer.remove( tableRow ); +} + +// Calculates a new heading rows value for removing rows from heading section. +function getNewHeadingRowsValue( first, last, headingRows ) { + if ( last < headingRows ) { + return headingRows - ( last - first + 1 ); + } + + return first; +} diff --git a/tests/commands/removerowcommand.js b/tests/commands/removerowcommand.js index fde382ed..21ccb315 100644 --- a/tests/commands/removerowcommand.js +++ b/tests/commands/removerowcommand.js @@ -464,7 +464,7 @@ describe( 'RemoveRowCommand', () => { setData( model, modelTable( [ [ { rowspan: 3, contents: '[]00' }, { rowspan: 2, contents: '01' }, '02' ], [ '12' ], - [ '22' ], + [ '21', '22' ], [ '30', '31', '32' ] ] ) ); @@ -472,7 +472,7 @@ describe( 'RemoveRowCommand', () => { assertEqualMarkup( getData( model ), modelTable( [ [ { rowspan: 2, contents: '[]00' }, '01', '12' ], - [ '22' ], + [ '21', '22' ], [ '30', '31', '32' ] ] ) ); } ); diff --git a/tests/tableutils.js b/tests/tableutils.js index a9f1199c..ab229677 100644 --- a/tests/tableutils.js +++ b/tests/tableutils.js @@ -747,4 +747,217 @@ describe( 'TableUtils', () => { expect( tableUtils.getRows( root.getNodeByPath( [ 0 ] ) ) ).to.equal( 3 ); } ); } ); + + describe( 'removeRows()', () => { + describe( 'single row', () => { + it( 'should remove a given row from a table start', () => { + setData( model, modelTable( [ + [ '00', '01' ], + [ '10', '11' ], + [ '20', '21' ] + ] ) ); + + tableUtils.removeRows( root.getChild( 0 ), { at: 0 } ); + + assertEqualMarkup( getData( model, { withoutSelection: true } ), modelTable( [ + [ '10', '11' ], + [ '20', '21' ] + ] ) ); + } ); + + it( 'should remove last row', () => { + setData( model, modelTable( [ + [ '00', '01' ], + [ '10', '11' ] + ] ) ); + + tableUtils.removeRows( root.getChild( 0 ), { at: 1 } ); + + assertEqualMarkup( getData( model, { withoutSelection: true } ), modelTable( [ + [ '00', '01' ] + ] ) ); + } ); + + it( 'should change heading rows if removing a heading row', () => { + setData( model, modelTable( [ + [ '00', '01' ], + [ '10', '11' ], + [ '20', '21' ] + ], { headingRows: 2 } ) ); + + tableUtils.removeRows( root.getChild( 0 ), { at: 1 } ); + + assertEqualMarkup( getData( model, { withoutSelection: true } ), modelTable( [ + [ '00', '01' ], + [ '20', '21' ] + ], { headingRows: 1 } ) ); + } ); + + it( 'should decrease rowspan of table cells from previous rows', () => { + 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' ] + ] ) ); + + tableUtils.removeRows( root.getChild( 0 ), { at: 2 } ); + + assertEqualMarkup( getData( model, { withoutSelection: true } ), modelTable( [ + [ { rowspan: 3, contents: '00' }, { rowspan: 2, contents: '01' }, { rowspan: 2, contents: '02' }, '03', '04' ], + [ '13', '14' ], + [ '30', '31', '32', '33', '34' ] + ] ) ); + } ); + + it( 'should move rowspaned cells to row below removing it\'s row', () => { + setData( model, modelTable( [ + [ { rowspan: 3, contents: '00' }, { rowspan: 2, contents: '01' }, '02' ], + [ '12' ], + [ '21', '22' ], + [ '30', '31', '32' ] + ] ) ); + + tableUtils.removeRows( root.getChild( 0 ), { at: 0 } ); + + assertEqualMarkup( getData( model, { withoutSelection: true } ), modelTable( [ + [ { rowspan: 2, contents: '00' }, '01', '12' ], + [ '21', '22' ], + [ '30', '31', '32' ] + ] ) ); + } ); + } ); + + describe( 'many rows', () => { + it( 'should properly remove middle rows', () => { + setData( model, modelTable( [ + [ '00', '01' ], + [ '10', '11' ], + [ '20', '21' ], + [ '30', '31' ] + ] ) ); + + tableUtils.removeRows( root.getChild( 0 ), { at: 1, rows: 2 } ); + + assertEqualMarkup( getData( model, { withoutSelection: true } ), modelTable( [ + [ '00', '01' ], + [ '30', '31' ] + ] ) ); + } ); + + it( 'should properly remove tailing rows', () => { + setData( model, modelTable( [ + [ '00', '01' ], + [ '10', '11' ], + [ '20', '21' ], + [ '30', '31' ] + ] ) ); + + tableUtils.removeRows( root.getChild( 0 ), { at: 2, rows: 2 } ); + + assertEqualMarkup( getData( model, { withoutSelection: true } ), modelTable( [ + [ '00', '01' ], + [ '10', '11' ] + ] ) ); + } ); + + it( 'should properly remove beginning rows', () => { + setData( model, modelTable( [ + [ '00', '01' ], + [ '10', '11' ], + [ '20', '21' ], + [ '30', '31' ] + ] ) ); + + tableUtils.removeRows( root.getChild( 0 ), { at: 0, rows: 2 } ); + + assertEqualMarkup( getData( model, { withoutSelection: true } ), modelTable( [ + [ '20', '21' ], + [ '30', '31' ] + ] ) ); + } ); + + it( 'should support removing multiple headings (removed rows in heading section)', () => { + setData( model, modelTable( [ + [ '00', '01' ], + [ '10', '11' ], + [ '20', '21' ], + [ '30', '31' ] + ], { headingRows: 3 } ) ); + + tableUtils.removeRows( root.getChild( 0 ), { at: 0, rows: 2 } ); + + assertEqualMarkup( getData( model, { withoutSelection: true } ), modelTable( [ + [ '20', '21' ], + [ '30', '31' ] + ], { headingRows: 1 } ) ); + } ); + + it( 'should support removing multiple headings (removed rows in heading and body section)', () => { + setData( model, modelTable( [ + [ '00', '01' ], + [ '10', '11' ], + [ '20', '21' ], + [ '30', '31' ], + [ '40', '41' ] + ], { headingRows: 3 } ) ); + + tableUtils.removeRows( root.getChild( 0 ), { at: 1, rows: 3 } ); + + assertEqualMarkup( getData( model, { withoutSelection: true } ), modelTable( [ + [ '00', '01' ], + [ '40', '41' ] + ], { headingRows: 1 } ) ); + } ); + + it( 'should support removing mixed heading and cell rows', () => { + setData( model, modelTable( [ + [ '00', '01' ], + [ '10', '11' ], + [ '20', '21' ] + ], { headingRows: 1 } ) ); + + tableUtils.removeRows( root.getChild( 0 ), { at: 0, rows: 2 } ); + + assertEqualMarkup( getData( model, { withoutSelection: true } ), modelTable( [ + [ '20', '21' ] + ] ) ); + } ); + + it( 'should properly calculate truncated rowspans', () => { + setData( model, modelTable( [ + [ '00', { contents: '01', rowspan: 3 } ], + [ '10' ], + [ '20' ] + ] ) ); + + tableUtils.removeRows( root.getChild( 0 ), { at: 0, rows: 2 } ); + + assertEqualMarkup( getData( model, { withoutSelection: true } ), modelTable( [ + [ '20', '01' ] + ] ) ); + } ); + + it( 'should create one undo step (1 batch)', () => { + setData( model, modelTable( [ + [ '00', '01' ], + [ '10', '11' ], + [ '20', '21' ], + [ '30', '31' ] + ], { headingRows: 3 } ) ); + + const createdBatches = new Set(); + + model.on( 'applyOperation', ( evt, args ) => { + const operation = args[ 0 ]; + + createdBatches.add( operation.batch ); + } ); + + tableUtils.removeRows( root.getChild( 0 ), { at: 0, rows: 2 } ); + + expect( createdBatches.size ).to.equal( 1 ); + } ); + } ); + } ); } );