diff --git a/packages/ckeditor5-table/src/commands/removecolumncommand.js b/packages/ckeditor5-table/src/commands/removecolumncommand.js index d21aa7a3cdf..0e159ed5167 100644 --- a/packages/ckeditor5-table/src/commands/removecolumncommand.js +++ b/packages/ckeditor5-table/src/commands/removecolumncommand.js @@ -10,7 +10,6 @@ import Command from '@ckeditor/ckeditor5-core/src/command'; import TableWalker from '../tablewalker'; -import { updateNumericAttribute } from './utils'; import { getSelectionAffectedTableCells } from '../utils'; /** @@ -69,59 +68,16 @@ export default class RemoveColumnCommand extends Command { // A temporary workaround to avoid the "model-selection-range-intersects" error. writer.setSelection( writer.createRangeOn( table ) ); - adjustHeadingColumns( table, removedColumnIndexes, writer ); + const columnsToRemove = removedColumnIndexes.last - removedColumnIndexes.first + 1; - for ( - let removedColumnIndex = removedColumnIndexes.last; - removedColumnIndex >= removedColumnIndexes.first; - removedColumnIndex-- - ) { - this._removeColumn( removedColumnIndex, table, writer ); - } + this.editor.plugins.get( 'TableUtils' ).removeColumns( table, { + at: removedColumnIndexes.first, + columns: columnsToRemove + } ); writer.setSelection( writer.createPositionAt( cellToFocus, 0 ) ); } ); } - - /** - * Removes a column from the given `table`. - * - * @private - * @param {Number} removedColumnIndex Index of the column that should be removed. - * @param {module:engine/model/element~Element} table - * @param {module:engine/model/writer~Writer} writer - */ - _removeColumn( removedColumnIndex, table, writer ) { - for ( const { cell, column, colspan } of new TableWalker( table ) ) { - // If colspaned cell overlaps removed column decrease its span. - if ( column <= removedColumnIndex && colspan > 1 && column + colspan > removedColumnIndex ) { - updateNumericAttribute( 'colspan', colspan - 1, cell, writer ); - } else if ( column === removedColumnIndex ) { - const cellRow = cell.parent; - - // The cell in removed column has colspan of 1. - writer.remove( cell ); - - // If the cell was the last one in the row, get rid of the entire row. - // https://github.com/ckeditor/ckeditor5/issues/6429 - if ( !cellRow.childCount ) { - writer.remove( cellRow ); - } - } - } - } -} - -// Updates heading columns attribute if removing a row from head section. -function adjustHeadingColumns( table, removedColumnIndexes, writer ) { - const headingColumns = table.getAttribute( 'headingColumns' ) || 0; - - if ( headingColumns && removedColumnIndexes.first <= headingColumns ) { - const headingsRemoved = Math.min( headingColumns - 1 /* Other numbers are 0-based */, removedColumnIndexes.last ) - - removedColumnIndexes.first + 1; - - writer.setAttribute( 'headingColumns', headingColumns - headingsRemoved, table ); - } } // Returns a proper table cell to focus after removing a column. diff --git a/packages/ckeditor5-table/src/tableutils.js b/packages/ckeditor5-table/src/tableutils.js index efe541c6ae5..a1e1b17bd66 100644 --- a/packages/ckeditor5-table/src/tableutils.js +++ b/packages/ckeditor5-table/src/tableutils.js @@ -307,6 +307,66 @@ export default class TableUtils extends Plugin { } ); } + /** + * Removes columns from the given `table`. + * + * This method re-calculates the table geometry including the `colspan` attribute of table cells overlapping removed columns + * and table headings values. + * + * editor.plugins.get( 'TableUtils' ).removeColumns( table, { at: 1, columns: 2 } ); + * + * Executing the above code in the context of the table on the left will transform its structure as presented on the right: + * + * 0 1 2 3 4 0 1 2 + * ┌───────────────┬───┐ ┌───────┬───┐ + * │ a │ b │ │ a │ b │ + * │ ├───┤ │ ├───┤ + * │ │ c │ │ │ c │ + * ├───┬───┬───┬───┼───┤ will give: ├───┬───┼───┤ + * │ d │ e │ f │ g │ h │ │ d │ g │ h │ + * ├───┼───┼───┤ ├───┤ ├───┤ ├───┤ + * │ i │ j │ k │ │ l │ │ i │ │ l │ + * ├───┴───┴───┴───┴───┤ ├───┴───┴───┤ + * │ m │ │ m │ + * └───────────────────┘ └───────────┘ + * ^---- remove from here, `at` = 1, `columns` = 2 + * + * @param {module:engine/model/element~Element} table + * @param {Object} options + * @param {Number} options.at The row index at which the removing columns will start. + * @param {Number} [options.columns=1] The number of columns to remove. + */ + removeColumns( table, options ) { + const model = this.editor.model; + const first = options.at; + const columnsToRemove = options.columns || 1; + const last = options.at + columnsToRemove - 1; + + model.change( writer => { + adjustHeadingColumns( table, { first, last }, writer ); + + for ( let removedColumnIndex = last; removedColumnIndex >= first; removedColumnIndex-- ) { + for ( const { cell, column, colspan } of [ ...new TableWalker( table ) ] ) { + // If colspaned cell overlaps removed column decrease its span. + if ( column <= removedColumnIndex && colspan > 1 && column + colspan > removedColumnIndex ) { + updateNumericAttribute( 'colspan', colspan - 1, cell, writer ); + } else if ( column === removedColumnIndex ) { + const cellRow = cell.parent; + + // The cell in removed column has colspan of 1. + writer.remove( cell ); + + // If the cell was the last one in the row, get rid of the entire row. + // https://github.com/ckeditor/ckeditor5/issues/6429 + if ( !cellRow.childCount ) { + this.removeRows( table, { at: cellRow.index } ); + } + } + } + } + } ); + } + /** * Divides a table cell vertically into several ones. * @@ -715,3 +775,15 @@ function getNewHeadingRowsValue( first, last, headingRows ) { return first; } + +// Updates heading columns attribute if removing a row from head section. +function adjustHeadingColumns( table, removedColumnIndexes, writer ) { + const headingColumns = table.getAttribute( 'headingColumns' ) || 0; + + if ( headingColumns && removedColumnIndexes.first < headingColumns ) { + const headingsRemoved = Math.min( headingColumns - 1 /* Other numbers are 0-based */, removedColumnIndexes.last ) - + removedColumnIndexes.first + 1; + + writer.setAttribute( 'headingColumns', headingColumns - headingsRemoved, table ); + } +} diff --git a/packages/ckeditor5-table/tests/commands/removecolumncommand.js b/packages/ckeditor5-table/tests/commands/removecolumncommand.js index ad76d108292..487a2f16d9f 100644 --- a/packages/ckeditor5-table/tests/commands/removecolumncommand.js +++ b/packages/ckeditor5-table/tests/commands/removecolumncommand.js @@ -367,21 +367,21 @@ describe( 'RemoveColumnCommand', () => { it( 'should decrease colspan of table cells from previous column', () => { setData( model, modelTable( [ - [ { colspan: 4, contents: '00' }, '03' ], - [ { colspan: 3, contents: '10' }, '13' ], - [ { colspan: 2, contents: '20' }, '22[]', '23' ], - [ '30', { colspan: 2, contents: '31' }, '33' ], - [ '40', '41', '42', '43' ] + [ { colspan: 4, contents: '00' }, '04' ], + [ { colspan: 3, contents: '10' }, '13', '14' ], + [ { colspan: 2, contents: '20' }, '22[]', '23', '24' ], + [ '30', { colspan: 2, contents: '31' }, '33', '34' ], + [ '40', '41', '42', '43', '44' ] ] ) ); command.execute(); assertEqualMarkup( getData( model ), modelTable( [ - [ { colspan: 3, contents: '00' }, '03' ], - [ { colspan: 2, contents: '10' }, '13' ], - [ { colspan: 2, contents: '20' }, '[]23' ], - [ '30', '31', '33' ], - [ '40', '41', '43' ] + [ { colspan: 3, contents: '00' }, '04' ], + [ { colspan: 2, contents: '10' }, '13', '14' ], + [ { colspan: 2, contents: '20' }, '[]23', '24' ], + [ '30', '31', '33', '34' ], + [ '40', '41', '43', '44' ] ] ) ); } ); @@ -389,7 +389,7 @@ describe( 'RemoveColumnCommand', () => { it( 'should decrease colspan of cells that are on removed column', () => { setData( model, modelTable( [ [ { colspan: 3, contents: '[]00' }, '03' ], - [ { colspan: 2, contents: '10' }, '13' ], + [ { colspan: 2, contents: '10' }, '12', '13' ], [ '20', '21', '22', '23' ] ] ) ); @@ -397,7 +397,7 @@ describe( 'RemoveColumnCommand', () => { assertEqualMarkup( getData( model ), modelTable( [ [ { colspan: 2, contents: '[]00' }, '03' ], - [ '10', '13' ], + [ '10', '12', '13' ], [ '21', '22', '23' ] ] ) ); } ); @@ -418,47 +418,47 @@ describe( 'RemoveColumnCommand', () => { ] ) ); } ); - it( 'should work property if the rowspan is in the first column (#1)', () => { + it( 'should work property if the rowspan is in the first column (the other cell in row is selected)', () => { setData( model, modelTable( [ [ { rowspan: 2, contents: '00' }, '[]01' ], - [ '10' ] + [ '11' ] ] ) ); command.execute(); assertEqualMarkup( getData( model ), modelTable( [ - [ { rowspan: 2, contents: '[]00' } ] + [ '[]00' ] ] ) ); } ); - it( 'should work property if the rowspan is in the first column (#2)', () => { + it( 'should work property if the rowspan is in the first column (the cell in row below is selected)', () => { setData( model, modelTable( [ [ { rowspan: 2, contents: '00' }, '01' ], - [ '[]10' ] + [ '[]11' ] ] ) ); command.execute(); assertEqualMarkup( getData( model ), modelTable( [ - [ { rowspan: 2, contents: '[]00' } ] + [ '[]00' ] ] ) ); } ); - it( 'should work property if the rowspan is in the first column (#3)', () => { + it( 'should work property if the rowspan is in the first column (the cell with rowspan is selected)', () => { setData( model, modelTable( [ [ { rowspan: 2, contents: '00[]' }, '01' ], - [ '10' ] + [ '11' ] ] ) ); command.execute(); assertEqualMarkup( getData( model ), modelTable( [ [ '[]01' ], - [ '10' ] + [ '11' ] ] ) ); } ); - it( 'should work property if the rowspan is in the last column (#1)', () => { + it( 'should work property if the rowspan is in the last column (the other cell in row is selected)', () => { setData( model, modelTable( [ [ '[]00', { rowspan: 2, contents: '01' } ], [ '10' ] @@ -467,11 +467,11 @@ describe( 'RemoveColumnCommand', () => { command.execute(); assertEqualMarkup( getData( model ), modelTable( [ - [ { rowspan: 2, contents: '[]01' } ] + [ '[]01' ] ] ) ); } ); - it( 'should work property if the rowspan is in the last column (#2)', () => { + it( 'should work property if the rowspan is in the last column (the cell in row below is selected)', () => { setData( model, modelTable( [ [ '00', { rowspan: 2, contents: '01' } ], [ '[]10' ] @@ -480,11 +480,11 @@ describe( 'RemoveColumnCommand', () => { command.execute(); assertEqualMarkup( getData( model ), modelTable( [ - [ { rowspan: 2, contents: '[]01' } ] + [ '[]01' ] ] ) ); } ); - it( 'should work property if the rowspan is in the last column (#3)', () => { + it( 'should work property if the rowspan is in the last column (the cell with rowspan is selected)', () => { setData( model, modelTable( [ [ '00', { rowspan: 2, contents: '[]01' } ], [ '10' ] @@ -497,5 +497,20 @@ describe( 'RemoveColumnCommand', () => { [ '10' ] ] ) ); } ); + + it( 'should remove column if removing row with one column - other columns are spanned', () => { + setData( model, modelTable( [ + [ '[]00', { rowspan: 2, contents: '01' }, { rowspan: 2, contents: '02' } ], + [ '10' ], + [ '20', '21', '22' ] + ] ) ); + + command.execute(); + + assertEqualMarkup( getData( model ), modelTable( [ + [ '[]01', '02' ], + [ '21', '22' ] + ] ) ); + } ); } ); } ); diff --git a/packages/ckeditor5-table/tests/tableutils.js b/packages/ckeditor5-table/tests/tableutils.js index ab229677169..ae1c4f56fab 100644 --- a/packages/ckeditor5-table/tests/tableutils.js +++ b/packages/ckeditor5-table/tests/tableutils.js @@ -960,4 +960,248 @@ describe( 'TableUtils', () => { } ); } ); } ); + + describe( 'removeColumns()', () => { + describe( 'single row', () => { + it( 'should remove a given column', () => { + setData( model, modelTable( [ + [ '00', '01', '02' ], + [ '10', '11', '12' ], + [ '20', '21', '22' ] + ] ) ); + + tableUtils.removeColumns( root.getNodeByPath( [ 0 ] ), { at: 1 } ); + + assertEqualMarkup( getData( model, { withoutSelection: true } ), modelTable( [ + [ '00', '02' ], + [ '10', '12' ], + [ '20', '22' ] + ] ) ); + } ); + + it( 'should remove a given column from a table start', () => { + setData( model, modelTable( [ + [ '00', '01' ], + [ '10', '11' ], + [ '20', '21' ] + ] ) ); + + tableUtils.removeColumns( root.getNodeByPath( [ 0 ] ), { at: 0 } ); + + assertEqualMarkup( getData( model, { withoutSelection: true } ), modelTable( [ + [ '01' ], + [ '11' ], + [ '21' ] + ] ) ); + } ); + + it( 'should change heading columns if removing a heading column', () => { + setData( model, modelTable( [ + [ '00', '01' ], + [ '10', '11' ], + [ '20', '21' ] + ], { headingColumns: 2 } ) ); + + tableUtils.removeColumns( root.getNodeByPath( [ 0 ] ), { at: 0 } ); + + assertEqualMarkup( getData( model, { withoutSelection: true } ), modelTable( [ + [ '01' ], + [ '11' ], + [ '21' ] + ], { headingColumns: 1 } ) ); + } ); + + it( 'should decrease colspan of table cells from previous column', () => { + setData( model, modelTable( [ + [ { colspan: 4, contents: '00' }, '04' ], + [ { colspan: 3, contents: '10' }, '13', '14' ], + [ { colspan: 2, contents: '20' }, '22', '23', '24' ], + [ '30', { colspan: 2, contents: '31' }, '33', '34' ], + [ '40', '41', '42', '43', '44' ] + ] ) ); + + tableUtils.removeColumns( root.getNodeByPath( [ 0 ] ), { at: 2 } ); + + assertEqualMarkup( getData( model, { withoutSelection: true } ), modelTable( [ + [ { colspan: 3, contents: '00' }, '04' ], + [ { colspan: 2, contents: '10' }, '13', '14' ], + [ { colspan: 2, contents: '20' }, '23', '24' ], + [ '30', '31', '33', '34' ], + [ '40', '41', '43', '44' ] + + ] ) ); + } ); + + it( 'should decrease colspan of cells that are on removed column', () => { + setData( model, modelTable( [ + [ { colspan: 3, contents: '00' }, '03' ], + [ { colspan: 2, contents: '10' }, '12', '13' ], + [ '20', '21', '22', '23' ] + ] ) ); + + tableUtils.removeColumns( root.getNodeByPath( [ 0 ] ), { at: 0 } ); + + assertEqualMarkup( getData( model, { withoutSelection: true } ), modelTable( [ + [ { colspan: 2, contents: '00' }, '03' ], + [ '10', '12', '13' ], + [ '21', '22', '23' ] + ] ) ); + } ); + + it( 'should remove column with rowspan (first column)', () => { + setData( model, modelTable( [ + [ { rowspan: 2, contents: '00' }, '01' ], + [ '11' ] + ] ) ); + + tableUtils.removeColumns( root.getNodeByPath( [ 0 ] ), { at: 0 } ); + + assertEqualMarkup( getData( model, { withoutSelection: true } ), modelTable( [ + [ '01' ], + [ '11' ] + ] ) ); + } ); + + it( 'should remove column with rowspan (last column)', () => { + setData( model, modelTable( [ + [ '00', { rowspan: 2, contents: '01' } ], + [ '10' ] + ] ) ); + + tableUtils.removeColumns( root.getNodeByPath( [ 0 ] ), { at: 1 } ); + + assertEqualMarkup( getData( model, { withoutSelection: true } ), modelTable( [ + [ '00' ], + [ '10' ] + ] ) ); + } ); + + it( 'should remove column if other column is rowspanned (last column)', () => { + setData( model, modelTable( [ + [ '00', { rowspan: 2, contents: '01' } ], + [ '10' ] + ] ) ); + + tableUtils.removeColumns( root.getNodeByPath( [ 0 ] ), { at: 0 } ); + + assertEqualMarkup( getData( model, { withoutSelection: true } ), modelTable( [ + [ '01' ] + ] ) ); + } ); + + it( 'should remove column if other column is rowspanned (first column)', () => { + setData( model, modelTable( [ + [ { rowspan: 2, contents: '00' }, '01' ], + [ '11' ] + ] ) ); + + tableUtils.removeColumns( root.getNodeByPath( [ 0 ] ), { at: 1 } ); + + assertEqualMarkup( getData( model, { withoutSelection: true } ), modelTable( [ + [ '00' ] + ] ) ); + } ); + + it( 'should remove column if removing row with one column - other columns are spanned', () => { + setData( model, modelTable( [ + [ '00', { rowspan: 2, contents: '01' }, { rowspan: 2, contents: '02' } ], + [ '10' ], + [ '20', '21', '22' ] + ] ) ); + + tableUtils.removeColumns( root.getNodeByPath( [ 0 ] ), { at: 0 } ); + + assertEqualMarkup( getData( model, { withoutSelection: true } ), modelTable( [ + [ '01', '02' ], + [ '21', '22' ] + ] ) ); + } ); + } ); + + describe( 'multiple columns', () => { + it( 'should properly remove two first columns', () => { + setData( model, modelTable( [ + [ '00', '01', '02' ], + [ '10', '11', '12' ], + [ '20', '21', '22' ], + [ '30', '31', '32' ] + ] ) ); + + tableUtils.removeColumns( root.getNodeByPath( [ 0 ] ), { at: 0, columns: 2 } ); + + assertEqualMarkup( getData( model, { withoutSelection: true } ), modelTable( [ + [ '02' ], + [ '12' ], + [ '22' ], + [ '32' ] + ] ) ); + } ); + + it( 'should properly remove two middle columns', () => { + setData( model, modelTable( [ + [ '00', '01', '02', '03' ], + [ '10', '11', '12', '13' ], + [ '20', '21', '22', '23' ], + [ '30', '31', '32', '33' ] + ] ) ); + + tableUtils.removeColumns( root.getNodeByPath( [ 0 ] ), { at: 1, columns: 2 } ); + + assertEqualMarkup( getData( model, { withoutSelection: true } ), modelTable( [ + [ '00', '03' ], + [ '10', '13' ], + [ '20', '23' ], + [ '30', '33' ] + ] ) ); + } ); + + it( 'should properly remove two last columns', () => { + setData( model, modelTable( [ + [ '00', '01', '02' ], + [ '10', '11', '12' ], + [ '20', '21', '22' ], + [ '30', '31', '32' ] + ] ) ); + + tableUtils.removeColumns( root.getNodeByPath( [ 0 ] ), { at: 1, columns: 2 } ); + + assertEqualMarkup( getData( model, { withoutSelection: true } ), modelTable( [ + [ '00' ], + [ '10' ], + [ '20' ], + [ '30' ] + ] ) ); + } ); + + it( 'should properly remove multiple heading columns', () => { + setData( model, modelTable( [ + [ '00', '01', '02', '03', '04' ], + [ '10', '11', '12', '13', '14' ] + ], { headingColumns: 3 } ) ); + + tableUtils.removeColumns( root.getNodeByPath( [ 0 ] ), { at: 1, columns: 3 } ); + + assertEqualMarkup( getData( model, { withoutSelection: true } ), modelTable( [ + [ '00', '04' ], + [ '10', '14' ] + ], { headingColumns: 1 } ) ); + } ); + + it( 'should properly calculate truncated colspans', () => { + setData( model, modelTable( [ + [ { contents: '00', colspan: 3 } ], + [ '10', '11', '12' ], + [ '20', '21', '22' ] + ] ) ); + + tableUtils.removeColumns( root.getNodeByPath( [ 0 ] ), { at: 0, columns: 2 } ); + + assertEqualMarkup( getData( model, { withoutSelection: true } ), modelTable( [ + [ '00' ], + [ '12' ], + [ '22' ] + ] ) ); + } ); + } ); + } ); } );