Răsfoiți Sursa

Merge branch 'master' into i/6127

Piotrek Koszuliński 5 ani în urmă
părinte
comite
3d713658ff

+ 75 - 39
packages/ckeditor5-table/src/commands/removecolumncommand.js

@@ -11,7 +11,7 @@ import Command from '@ckeditor/ckeditor5-core/src/command';
 
 import TableWalker from '../tablewalker';
 import { updateNumericAttribute } from './utils';
-import { getTableCellsContainingSelection } from '../utils';
+import { getSelectionAffectedTableCells } from '../utils';
 
 /**
  * The remove column command.
@@ -29,66 +29,102 @@ export default class RemoveColumnCommand extends Command {
 	 * @inheritDoc
 	 */
 	refresh() {
-		const editor = this.editor;
-		const selection = editor.model.document.selection;
-		const tableUtils = editor.plugins.get( 'TableUtils' );
-		const tableCell = getTableCellsContainingSelection( selection )[ 0 ];
-
-		this.isEnabled = !!tableCell && tableUtils.getColumns( tableCell.parent.parent ) > 1;
+		const selectedCells = getSelectionAffectedTableCells( this.editor.model.document.selection );
+		const firstCell = selectedCells[ 0 ];
+
+		if ( firstCell ) {
+			const table = firstCell.parent.parent;
+			const tableColumnCount = this.editor.plugins.get( 'TableUtils' ).getColumns( table );
+
+			const tableMap = [ ...new TableWalker( table ) ];
+			const columnIndexes = tableMap.filter( entry => selectedCells.includes( entry.cell ) ).map( el => el.column ).sort();
+			const minColumnIndex = columnIndexes[ 0 ];
+			const maxColumnIndex = columnIndexes[ columnIndexes.length - 1 ];
+
+			this.isEnabled = maxColumnIndex - minColumnIndex < ( tableColumnCount - 1 );
+		} else {
+			this.isEnabled = false;
+		}
 	}
 
 	/**
 	 * @inheritDoc
 	 */
 	execute() {
-		const model = this.editor.model;
-		const selection = model.document.selection;
-
-		const tableCell = getTableCellsContainingSelection( selection )[ 0 ];
-		const tableRow = tableCell.parent;
-		const table = tableRow.parent;
-
-		const headingColumns = table.getAttribute( 'headingColumns' ) || 0;
+		const [ firstCell, lastCell ] = getBoundaryCells( this.editor.model.document.selection );
+		const table = firstCell.parent.parent;
 
 		// Cache the table before removing or updating colspans.
 		const tableMap = [ ...new TableWalker( table ) ];
 
-		// Get column index of removed column.
-		const cellData = tableMap.find( value => value.cell === tableCell );
-		const removedColumn = cellData.column;
-		const selectionRow = cellData.row;
-		const cellToFocus = getCellToFocus( tableCell );
-
-		model.change( writer => {
-			// Update heading columns attribute if removing a row from head section.
-			if ( headingColumns && selectionRow <= headingColumns ) {
-				writer.setAttribute( 'headingColumns', headingColumns - 1, table );
-			}
-
-			for ( const { cell, column, colspan } of tableMap ) {
-				// If colspaned cell overlaps removed column decrease it's span.
-				if ( column <= removedColumn && colspan > 1 && column + colspan > removedColumn ) {
-					updateNumericAttribute( 'colspan', colspan - 1, cell, writer );
-				} else if ( column === removedColumn ) {
-					// The cell in removed column has colspan of 1.
-					writer.remove( cell );
+		// Store column indexes of removed columns.
+		const removedColumnIndexes = {
+			first: tableMap.find( value => value.cell === firstCell ).column,
+			last: tableMap.find( value => value.cell === lastCell ).column
+		};
+
+		const cellsToFocus = getCellToFocus( firstCell, lastCell );
+
+		this.editor.model.change( writer => {
+			// A temporary workaround to avoid the "model-selection-range-intersects" error.
+			writer.setSelection( writer.createRangeOn( table ) );
+
+			adjustHeadingColumns( table, removedColumnIndexes, writer );
+
+			for (
+				let removedColumnIndex = removedColumnIndexes.last;
+				removedColumnIndex >= removedColumnIndexes.first;
+				removedColumnIndex--
+			) {
+				for ( const { cell, column, colspan } of tableMap ) {
+					// 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 ) {
+						// The cell in removed column has colspan of 1.
+						writer.remove( cell );
+					}
 				}
 			}
 
-			writer.setSelection( writer.createPositionAt( cellToFocus, 0 ) );
+			writer.setSelection( writer.createPositionAt( cellsToFocus.reverse().filter( item => item != null )[ 0 ], 0 ) );
 		} );
 	}
 }
 
+// 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. It should be a next sibling to selection visually stay in place but:
 // - selection is on last table cell it will return previous cell.
 // - table cell is spanned over 2+ columns - it will be truncated so the selection should stay in that cell.
-function getCellToFocus( tableCell ) {
-	const colspan = parseInt( tableCell.getAttribute( 'colspan' ) || 1 );
+function getCellToFocus( firstCell, lastCell ) {
+	const colspan = parseInt( lastCell.getAttribute( 'colspan' ) || 1 );
 
 	if ( colspan > 1 ) {
-		return tableCell;
+		return [ firstCell, lastCell ];
 	}
 
-	return tableCell.nextSibling ? tableCell.nextSibling : tableCell.previousSibling;
+	// return lastCell.nextSibling ? lastCell.nextSibling : lastCell.previousSibling;
+	return [ firstCell.previousSibling, lastCell.nextSibling ];
+}
+
+// Returns helper object returning the first and the last cell contained in given selection, based on DOM order.
+function getBoundaryCells( selection ) {
+	const referenceCells = getSelectionAffectedTableCells( selection );
+	const firstCell = referenceCells[ 0 ];
+	const lastCell = referenceCells.pop();
+
+	const returnValue = [ firstCell, lastCell ];
+
+	return firstCell.isBefore( lastCell ) ? returnValue : returnValue.reverse();
 }

+ 109 - 57
packages/ckeditor5-table/src/commands/removerowcommand.js

@@ -11,7 +11,7 @@ import Command from '@ckeditor/ckeditor5-core/src/command';
 
 import TableWalker from '../tablewalker';
 import { updateNumericAttribute } from './utils';
-import { getTableCellsContainingSelection } from '../utils';
+import { getSelectionAffectedTableCells } from '../utils';
 
 /**
  * The remove row command.
@@ -29,83 +29,127 @@ export default class RemoveRowCommand extends Command {
 	 * @inheritDoc
 	 */
 	refresh() {
-		const model = this.editor.model;
-		const doc = model.document;
-		const tableCell = getTableCellsContainingSelection( doc.selection )[ 0 ];
+		const selectedCells = getSelectionAffectedTableCells( this.editor.model.document.selection );
+		const firstCell = selectedCells[ 0 ];
 
-		this.isEnabled = !!tableCell && tableCell.parent.parent.childCount > 1;
+		if ( firstCell ) {
+			const table = firstCell.parent.parent;
+			const tableRowCount = this.editor.plugins.get( 'TableUtils' ).getRows( table );
+
+			const tableMap = [ ...new TableWalker( table ) ];
+			const rowIndexes = tableMap.filter( entry => selectedCells.includes( entry.cell ) ).map( el => el.row );
+			const minRowIndex = rowIndexes[ 0 ];
+			const maxRowIndex = rowIndexes[ rowIndexes.length - 1 ];
+
+			this.isEnabled = maxRowIndex - minRowIndex < ( tableRowCount - 1 );
+		} else {
+			this.isEnabled = false;
+		}
 	}
 
 	/**
 	 * @inheritDoc
 	 */
 	execute() {
-		const model = this.editor.model;
-		const selection = model.document.selection;
-		const tableCell = getTableCellsContainingSelection( selection )[ 0 ];
-		const tableRow = tableCell.parent;
-		const table = tableRow.parent;
+		const referenceCells = getSelectionAffectedTableCells( this.editor.model.document.selection );
+		const removedRowIndexes = getRowIndexes( referenceCells );
+
+		const firstCell = referenceCells[ 0 ];
+		const table = firstCell.parent.parent;
+		const tableMap = [ ...new TableWalker( table, { endRow: removedRowIndexes.last } ) ];
+		const batch = this.editor.model.createBatch();
+		const columnIndexToFocus = getColumnIndexToFocus( tableMap, firstCell );
+
+		// Doing multiple model.enqueueChange() calls, to get around ckeditor/ckeditor5#6391.
+		// Ideally we want to do this in a single model.change() block.
+		this.editor.model.enqueueChange( batch, writer => {
+			// This prevents the "model-selection-range-intersects" error, caused by removing row selected cells.
+			writer.setSelection( writer.createSelection( table, 'on' ) );
+		} );
 
-		const removedRow = table.getChildIndex( tableRow );
+		let cellToFocus;
 
-		const tableMap = [ ...new TableWalker( table, { endRow: removedRow } ) ];
+		for ( let i = removedRowIndexes.last; i >= removedRowIndexes.first; i-- ) {
+			this.editor.model.enqueueChange( batch, writer => {
+				const removedRowIndex = i;
+				this._removeRow( removedRowIndex, table, writer, tableMap );
 
-		const cellData = tableMap.find( value => value.cell === tableCell );
+				cellToFocus = getCellToFocus( table, removedRowIndex, columnIndexToFocus );
+			} );
+		}
 
+		this.editor.model.enqueueChange( batch, writer => {
+			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
+	 * @param {module:engine/model/element~Element[]} tableMap Table map retrieved from {@link module:table/tablewalker~TableWalker}.
+	 */
+	_removeRow( removedRowIndex, table, writer, tableMap ) {
+		const cellsToMove = new Map();
+		const tableRow = table.getChild( removedRowIndex );
 		const headingRows = table.getAttribute( 'headingRows' ) || 0;
 
-		const columnToFocus = cellData.column;
+		if ( headingRows && removedRowIndex < headingRows ) {
+			updateNumericAttribute( 'headingRows', headingRows - 1, table, writer, 0 );
+		}
 
-		model.change( writer => {
-			if ( headingRows && removedRow <= headingRows ) {
-				updateNumericAttribute( 'headingRows', headingRows - 1, table, writer, 0 );
+		// 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;
 			}
-
-			const cellsToMove = new Map();
-
-			// Get cells from removed row that are spanned over multiple rows.
-			tableMap
-				.filter( ( { row, rowspan } ) => row === removedRow && 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 <= removedRow - 1 && row + rowspan > removedRow )
-				.forEach( ( { cell, rowspan } ) => updateNumericAttribute( 'rowspan', rowspan - 1, cell, writer ) );
-
-			// Move cells to another row.
-			const targetRow = removedRow + 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;
-				}
+			else {
+				previousCell = cell;
 			}
+		}
 
-			writer.remove( tableRow );
-
-			const cellToFocus = getCellToFocus( table, removedRow, columnToFocus );
-			writer.setSelection( writer.createPositionAt( cellToFocus, 0 ) );
-		} );
+		writer.remove( tableRow );
 	}
 }
 
+// Returns a helper object with first and last row index contained in given `referenceCells`.
+function getRowIndexes( referenceCells ) {
+	const allIndexesSorted = referenceCells.map( cell => cell.parent.index ).sort();
+
+	return {
+		first: allIndexesSorted[ 0 ],
+		last: allIndexesSorted[ allIndexesSorted.length - 1 ]
+	};
+}
+
 // Returns a cell that should be focused before removing the row, belonging to the same column as the currently focused cell.
-function getCellToFocus( table, removedRow, columnToFocus ) {
-	const row = table.getChild( removedRow );
+// * If the row was not the last one, the cell to focus will be in the row that followed it (before removal).
+// * If the row was the last one, the cell to focus will be in the row that preceded it (before removal).
+function getCellToFocus( table, removedRowIndex, columnToFocus ) {
+	const row = table.getChild( removedRowIndex ) || table.getChild( table.childCount - 1 );
 
 	// Default to first table cell.
 	let cellToFocus = row.getChild( 0 );
@@ -119,4 +163,12 @@ function getCellToFocus( table, removedRow, columnToFocus ) {
 		cellToFocus = tableCell;
 		column += parseInt( tableCell.getAttribute( 'colspan' ) || 1 );
 	}
+
+	return cellToFocus;
+}
+
+// Returns the index of column that should be focused after rows are removed.
+function getColumnIndexToFocus( tableMap, firstCell ) {
+	const firstCellData = tableMap.find( value => value.cell === firstCell );
+	return firstCellData.column;
 }

+ 16 - 0
packages/ckeditor5-table/src/tableutils.js

@@ -550,6 +550,22 @@ export default class TableUtils extends Plugin {
 			return columns + columnWidth;
 		}, 0 );
 	}
+
+	/**
+	 * Returns the number of rows for a given table.
+	 *
+	 *		editor.plugins.get( 'TableUtils' ).getRows( table );
+	 *
+	 * @param {module:engine/model/element~Element} table The table to analyze.
+	 * @returns {Number}
+	 */
+	getRows( table ) {
+		return [ ...table.getChildren() ].reduce( ( rows, row ) => {
+			const currentRowCount = parseInt( row.getChild( 0 ).getAttribute( 'rowspan' ) || 1 );
+
+			return rows + currentRowCount;
+		}, 0 );
+	}
 }
 
 // Creates empty rows at the given index in an existing table.

+ 234 - 1
packages/ckeditor5-table/tests/commands/removecolumncommand.js

@@ -7,6 +7,7 @@ import ModelTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/modeltestedit
 import { getData, setData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model';
 
 import RemoveColumnCommand from '../../src/commands/removecolumncommand';
+import TableSelection from '../../src/tableselection';
 import { defaultConversion, defaultSchema, modelTable } from '../_utils/utils';
 import TableUtils from '../../src/tableutils';
 import { assertEqualMarkup } from '@ckeditor/ckeditor5-utils/tests/_utils/utils';
@@ -17,7 +18,7 @@ describe( 'RemoveColumnCommand', () => {
 	beforeEach( () => {
 		return ModelTestEditor
 			.create( {
-				plugins: [ TableUtils ]
+				plugins: [ TableUtils, TableSelection ]
 			} )
 			.then( newEditor => {
 				editor = newEditor;
@@ -43,6 +44,22 @@ describe( 'RemoveColumnCommand', () => {
 			expect( command.isEnabled ).to.be.true;
 		} );
 
+		it( 'should be true if selection contains multiple cells', () => {
+			setData( model, modelTable( [
+				[ '00', '01', '02' ],
+				[ '10', '11', '12' ]
+			] ) );
+
+			const tableSelection = editor.plugins.get( TableSelection );
+			const modelRoot = model.document.getRoot();
+			tableSelection._setCellSelection(
+				modelRoot.getNodeByPath( [ 0, 0, 0 ] ),
+				modelRoot.getNodeByPath( [ 0, 0, 1 ] )
+			);
+
+			expect( command.isEnabled ).to.be.true;
+		} );
+
 		it( 'should be false if selection is inside table with one column only', () => {
 			setData( model, modelTable( [
 				[ '00' ],
@@ -53,6 +70,22 @@ describe( 'RemoveColumnCommand', () => {
 			expect( command.isEnabled ).to.be.false;
 		} );
 
+		it( 'should be false if all columns are selected', () => {
+			setData( model, modelTable( [
+				[ '00', '01', '02' ],
+				[ '10', '11', '12' ]
+			] ) );
+
+			const tableSelection = editor.plugins.get( TableSelection );
+			const modelRoot = model.document.getRoot();
+			tableSelection._setCellSelection(
+				modelRoot.getNodeByPath( [ 0, 0, 0 ] ),
+				modelRoot.getNodeByPath( [ 0, 0, 2 ] )
+			);
+
+			expect( command.isEnabled ).to.be.false;
+		} );
+
 		it( 'should be false if selection is outside a table', () => {
 			setData( model, '<paragraph>11[]</paragraph>' );
 
@@ -93,6 +126,206 @@ describe( 'RemoveColumnCommand', () => {
 			] ) );
 		} );
 
+		describe( 'with multiple cells selected', () => {
+			it( 'should properly remove the first column', () => {
+				setData( model, modelTable( [
+					[ '00', '01' ],
+					[ '10', '11' ],
+					[ '20', '21' ],
+					[ '30', '31' ]
+				] ) );
+
+				const tableSelection = editor.plugins.get( TableSelection );
+				const modelRoot = model.document.getRoot();
+				tableSelection._setCellSelection(
+					modelRoot.getNodeByPath( [ 0, 1, 0 ] ),
+					modelRoot.getNodeByPath( [ 0, 2, 0 ] )
+				);
+
+				command.execute();
+
+				assertEqualMarkup( getData( model ), modelTable( [
+					[ '01' ],
+					[ '11' ],
+					[ '[]21' ],
+					[ '31' ]
+				] ) );
+			} );
+
+			it( 'should properly remove a middle column', () => {
+				setData( model, modelTable( [
+					[ '00', '01', '02' ],
+					[ '10', '11', '12' ],
+					[ '20', '21', '22' ],
+					[ '30', '31', '32' ]
+				] ) );
+
+				const tableSelection = editor.plugins.get( TableSelection );
+				const modelRoot = model.document.getRoot();
+				tableSelection._setCellSelection(
+					modelRoot.getNodeByPath( [ 0, 1, 1 ] ),
+					modelRoot.getNodeByPath( [ 0, 2, 1 ] )
+				);
+
+				command.execute();
+
+				assertEqualMarkup( getData( model ), modelTable( [
+					[ '00', '02' ],
+					[ '10', '12' ],
+					[ '20', '[]22' ],
+					[ '30', '32' ]
+				] ) );
+			} );
+
+			it( 'should properly remove the last column', () => {
+				setData( model, modelTable( [
+					[ '00', '01' ],
+					[ '10', '11' ],
+					[ '20', '21' ],
+					[ '30', '31' ]
+				] ) );
+
+				const tableSelection = editor.plugins.get( TableSelection );
+				const modelRoot = model.document.getRoot();
+				tableSelection._setCellSelection(
+					modelRoot.getNodeByPath( [ 0, 1, 1 ] ),
+					modelRoot.getNodeByPath( [ 0, 2, 1 ] )
+				);
+
+				command.execute();
+
+				assertEqualMarkup( getData( model ), modelTable( [
+					[ '00' ],
+					[ '[]10' ],
+					[ '20' ],
+					[ '30' ]
+				] ) );
+			} );
+
+			it( 'should properly remove two first columns', () => {
+				setData( model, modelTable( [
+					[ '00', '01', '02' ],
+					[ '10', '11', '12' ],
+					[ '20', '21', '22' ],
+					[ '30', '31', '32' ]
+				] ) );
+
+				const tableSelection = editor.plugins.get( TableSelection );
+				const modelRoot = model.document.getRoot();
+				tableSelection._setCellSelection(
+					modelRoot.getNodeByPath( [ 0, 1, 0 ] ),
+					modelRoot.getNodeByPath( [ 0, 1, 1 ] )
+				);
+
+				command.execute();
+
+				assertEqualMarkup( getData( model ), 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' ]
+				] ) );
+
+				const tableSelection = editor.plugins.get( TableSelection );
+				const modelRoot = model.document.getRoot();
+				tableSelection._setCellSelection(
+					modelRoot.getNodeByPath( [ 0, 1, 1 ] ),
+					modelRoot.getNodeByPath( [ 0, 2, 2 ] )
+				);
+
+				command.execute();
+
+				assertEqualMarkup( getData( model ), modelTable( [
+					[ '00', '03' ],
+					[ '10', '13' ],
+					[ '20', '[]23' ],
+					[ '30', '33' ]
+				] ) );
+			} );
+
+			it( 'should properly remove two middle columns with reversed selection', () => {
+				setData( model, modelTable( [
+					[ '00', '01', '02', '03' ],
+					[ '10', '11', '12', '13' ],
+					[ '20', '21', '22', '23' ],
+					[ '30', '31', '32', '33' ]
+				] ) );
+
+				const tableSelection = editor.plugins.get( TableSelection );
+				const modelRoot = model.document.getRoot();
+				tableSelection._setCellSelection(
+					modelRoot.getNodeByPath( [ 0, 2, 2 ] ),
+					modelRoot.getNodeByPath( [ 0, 1, 1 ] )
+				);
+
+				command.execute();
+
+				assertEqualMarkup( getData( model ), modelTable( [
+					[ '00', '03' ],
+					[ '10', '13' ],
+					[ '20', '[]23' ],
+					[ '30', '33' ]
+				] ) );
+			} );
+
+			it( 'should properly remove two last columns', () => {
+				// There's no handling for selection in case like that.
+				setData( model, modelTable( [
+					[ '00', '01', '02' ],
+					[ '10', '11', '12' ],
+					[ '20', '21', '22' ],
+					[ '30', '31', '32' ]
+				] ) );
+
+				const tableSelection = editor.plugins.get( TableSelection );
+				const modelRoot = model.document.getRoot();
+				tableSelection._setCellSelection(
+					modelRoot.getNodeByPath( [ 0, 1, 1 ] ),
+					modelRoot.getNodeByPath( [ 0, 2, 2 ] )
+				);
+
+				command.execute();
+
+				assertEqualMarkup( getData( model ), modelTable( [
+					[ '00' ],
+					[ '[]10' ],
+					[ '20' ],
+					[ '30' ]
+				] ) );
+			} );
+
+			it( 'should properly remove multiple heading columns', () => {
+				// There's no handling for selection in case like that.
+				setData( model, modelTable( [
+					[ '00', '01', '02', '03', '04' ],
+					[ '10', '11', '12', '13', '14' ]
+				], { headingColumns: 3 } ) );
+
+				const tableSelection = editor.plugins.get( TableSelection );
+				const modelRoot = model.document.getRoot();
+				tableSelection._setCellSelection(
+					modelRoot.getNodeByPath( [ 0, 0, 1 ] ),
+					modelRoot.getNodeByPath( [ 0, 1, 3 ] )
+				);
+
+				command.execute();
+
+				assertEqualMarkup( getData( model ), modelTable( [
+					[ '00', '04' ],
+					[ '10', '[]14' ]
+				], { headingColumns: 1 } ) );
+			} );
+		} );
+
 		it( 'should change heading columns if removing a heading column', () => {
 			setData( model, modelTable( [
 				[ '00', '01' ],

+ 245 - 1
packages/ckeditor5-table/tests/commands/removerowcommand.js

@@ -7,6 +7,7 @@ import ModelTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/modeltestedit
 import { getData, setData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model';
 
 import RemoveRowCommand from '../../src/commands/removerowcommand';
+import TableSelection from '../../src/tableselection';
 import { defaultConversion, defaultSchema, modelTable } from '../_utils/utils';
 import { assertEqualMarkup } from '@ckeditor/ckeditor5-utils/tests/_utils/utils';
 
@@ -14,7 +15,7 @@ describe( 'RemoveRowCommand', () => {
 	let editor, model, command;
 
 	beforeEach( () => {
-		return ModelTestEditor.create()
+		return ModelTestEditor.create( { plugins: [ TableSelection ] } )
 			.then( newEditor => {
 				editor = newEditor;
 				model = editor.model;
@@ -39,6 +40,23 @@ describe( 'RemoveRowCommand', () => {
 			expect( command.isEnabled ).to.be.true;
 		} );
 
+		it( 'should be true if selection contains multiple cells', () => {
+			setData( model, modelTable( [
+				[ '00', '01' ],
+				[ '10', '11' ],
+				[ '20', '21' ]
+			] ) );
+
+			const tableSelection = editor.plugins.get( TableSelection );
+			const modelRoot = model.document.getRoot();
+			tableSelection._setCellSelection(
+				modelRoot.getNodeByPath( [ 0, 0, 0 ] ),
+				modelRoot.getNodeByPath( [ 0, 0, 1 ] )
+			);
+
+			expect( command.isEnabled ).to.be.true;
+		} );
+
 		it( 'should be false if selection is inside table with one row only', () => {
 			setData( model, modelTable( [
 				[ '00[]', '01' ]
@@ -47,6 +65,22 @@ describe( 'RemoveRowCommand', () => {
 			expect( command.isEnabled ).to.be.false;
 		} );
 
+		it( 'should be false if all the rows are selected', () => {
+			setData( model, modelTable( [
+				[ '00', '01' ],
+				[ '10', '11' ]
+			] ) );
+
+			const tableSelection = editor.plugins.get( TableSelection );
+			const modelRoot = model.document.getRoot();
+			tableSelection._setCellSelection(
+				modelRoot.getNodeByPath( [ 0, 0, 0 ] ),
+				modelRoot.getNodeByPath( [ 0, 1, 0 ] )
+			);
+
+			expect( command.isEnabled ).to.be.false;
+		} );
+
 		it( 'should be false if selection is outside a table', () => {
 			setData( model, '<paragraph>11[]</paragraph>' );
 
@@ -70,6 +104,188 @@ describe( 'RemoveRowCommand', () => {
 			] ) );
 		} );
 
+		describe( 'with multiple rows selected', () => {
+			it( 'should properly remove middle rows', () => {
+				setData( model, modelTable( [
+					[ '00', '01' ],
+					[ '10', '11' ],
+					[ '20', '21' ],
+					[ '30', '31' ]
+				] ) );
+
+				const tableSelection = editor.plugins.get( TableSelection );
+				const modelRoot = model.document.getRoot();
+				tableSelection._setCellSelection(
+					modelRoot.getNodeByPath( [ 0, 1, 0 ] ),
+					modelRoot.getNodeByPath( [ 0, 2, 0 ] )
+				);
+
+				command.execute();
+
+				assertEqualMarkup( getData( model ), modelTable( [
+					[ '00', '01' ],
+					[ '[]30', '31' ]
+				] ) );
+			} );
+
+			it( 'should properly remove middle rows in reversed order', () => {
+				setData( model, modelTable( [
+					[ '00', '01' ],
+					[ '10', '11' ],
+					[ '20', '21' ],
+					[ '30', '31' ]
+				] ) );
+
+				const tableSelection = editor.plugins.get( TableSelection );
+				const modelRoot = model.document.getRoot();
+				tableSelection._setCellSelection(
+					modelRoot.getNodeByPath( [ 0, 2, 0 ] ),
+					modelRoot.getNodeByPath( [ 0, 1, 0 ] )
+				);
+
+				command.execute();
+
+				assertEqualMarkup( getData( model ), modelTable( [
+					[ '00', '01' ],
+					[ '[]30', '31' ]
+				] ) );
+			} );
+
+			it( 'should properly remove tailing rows', () => {
+				setData( model, modelTable( [
+					[ '00', '01' ],
+					[ '10', '11' ],
+					[ '20', '21' ],
+					[ '30', '31' ]
+				] ) );
+
+				const tableSelection = editor.plugins.get( TableSelection );
+				const modelRoot = model.document.getRoot();
+				tableSelection._setCellSelection(
+					modelRoot.getNodeByPath( [ 0, 2, 0 ] ),
+					modelRoot.getNodeByPath( [ 0, 3, 0 ] )
+				);
+
+				command.execute();
+
+				assertEqualMarkup( getData( model ), modelTable( [
+					[ '00', '01' ],
+					[ '[]10', '11' ]
+				] ) );
+			} );
+
+			it( 'should properly remove beginning rows', () => {
+				setData( model, modelTable( [
+					[ '00', '01' ],
+					[ '10', '11' ],
+					[ '20', '21' ],
+					[ '30', '31' ]
+				] ) );
+
+				const tableSelection = editor.plugins.get( TableSelection );
+				const modelRoot = model.document.getRoot();
+				tableSelection._setCellSelection(
+					modelRoot.getNodeByPath( [ 0, 0, 0 ] ),
+					modelRoot.getNodeByPath( [ 0, 1, 0 ] )
+				);
+
+				command.execute();
+
+				assertEqualMarkup( getData( model ), modelTable( [
+					[ '[]20', '21' ],
+					[ '30', '31' ]
+				] ) );
+			} );
+
+			it( 'should support removing multiple headings', () => {
+				setData( model, modelTable( [
+					[ '00', '01' ],
+					[ '10', '11' ],
+					[ '20', '21' ],
+					[ '30', '31' ]
+				], { headingRows: 3 } ) );
+
+				const tableSelection = editor.plugins.get( TableSelection );
+				const modelRoot = model.document.getRoot();
+				tableSelection._setCellSelection(
+					modelRoot.getNodeByPath( [ 0, 0, 0 ] ),
+					modelRoot.getNodeByPath( [ 0, 1, 0 ] )
+				);
+
+				command.execute();
+
+				assertEqualMarkup( getData( model ), modelTable( [
+					[ '[]20', '21' ],
+					[ '30', '31' ]
+				], { headingRows: 1 } ) );
+			} );
+
+			it( 'should support removing mixed heading and cell rows', () => {
+				setData( model, modelTable( [
+					[ '00', '01' ],
+					[ '10', '11' ],
+					[ '20', '21' ]
+				], { headingRows: 1 } ) );
+
+				const tableSelection = editor.plugins.get( TableSelection );
+				const modelRoot = model.document.getRoot();
+				tableSelection._setCellSelection(
+					modelRoot.getNodeByPath( [ 0, 0, 0 ] ),
+					modelRoot.getNodeByPath( [ 0, 1, 0 ] )
+				);
+
+				command.execute();
+
+				assertEqualMarkup( getData( model ), modelTable( [
+					[ '[]20', '21' ]
+				] ) );
+			} );
+		} );
+
+		describe( 'with entire row selected', () => {
+			it( 'should remove a row if all its cells are selected', () => {
+				setData( model, modelTable( [
+					[ '00', '01' ],
+					[ '10', '11' ],
+					[ '20', '21' ]
+				] ) );
+
+				const tableSelection = editor.plugins.get( TableSelection );
+				const modelRoot = model.document.getRoot();
+				tableSelection._setCellSelection(
+					modelRoot.getNodeByPath( [ 0, 1, 0 ] ),
+					modelRoot.getNodeByPath( [ 0, 1, 1 ] )
+				);
+
+				command.execute();
+
+				assertEqualMarkup( getData( model ), modelTable( [
+					[ '00', '01' ],
+					[ '[]20', '21' ]
+				] ) );
+			} );
+
+			it( 'should properly remove row if reversed selection is made', () => {
+				setData( model, modelTable( [
+					[ '00', '01' ],
+					[ '10', '11' ]
+				] ) );
+
+				const tableSelection = editor.plugins.get( TableSelection );
+				const modelRoot = model.document.getRoot();
+				tableSelection._setCellSelection(
+					modelRoot.getNodeByPath( [ 0, 0, 1 ] ),
+					modelRoot.getNodeByPath( [ 0, 0, 0 ] )
+				);
+
+				command.execute();
+
+				assertEqualMarkup( getData( model ), modelTable( [
+					[ '10', '[]11' ]
+				] ) );
+			} );
+		} );
+
 		it( 'should remove a given row from a table start', () => {
 			setData( model, modelTable( [
 				[ '[]00', '01' ],
@@ -85,6 +301,34 @@ describe( 'RemoveRowCommand', () => {
 			] ) );
 		} );
 
+		it( 'should remove a given row from a table start when selection is at the end', () => {
+			setData( model, modelTable( [
+				[ '00', '01[]' ],
+				[ '10', '11' ],
+				[ '20', '21' ]
+			] ) );
+
+			command.execute();
+
+			assertEqualMarkup( getData( model ), modelTable( [
+				[ '10', '[]11' ],
+				[ '20', '21' ]
+			] ) );
+		} );
+
+		it( 'should remove last row', () => {
+			setData( model, modelTable( [
+				[ '00', '01' ],
+				[ '[]10', '11' ]
+			] ) );
+
+			command.execute();
+
+			assertEqualMarkup( getData( model ), modelTable( [
+				[ '[]00', '01' ]
+			] ) );
+		} );
+
 		it( 'should change heading rows if removing a heading row', () => {
 			setData( model, modelTable( [
 				[ '00', '01' ],

+ 30 - 0
packages/ckeditor5-table/tests/tableutils.js

@@ -699,4 +699,34 @@ describe( 'TableUtils', () => {
 			expect( tableUtils.getColumns( root.getNodeByPath( [ 0 ] ) ) ).to.equal( 5 );
 		} );
 	} );
+
+	describe( 'getRows()', () => {
+		it( 'should return proper number of columns for simple table', () => {
+			setData( model, modelTable( [
+				[ '00', '01' ],
+				[ '10', '11' ]
+			] ) );
+
+			expect( tableUtils.getRows( root.getNodeByPath( [ 0 ] ) ) ).to.equal( 2 );
+		} );
+
+		it( 'should return proper number of columns for a table with header', () => {
+			setData( model, modelTable( [
+				[ '00', '01' ],
+				[ '10', '11' ]
+			], { headingRows: 1 } ) );
+
+			expect( tableUtils.getRows( root.getNodeByPath( [ 0 ] ) ) ).to.equal( 2 );
+		} );
+
+		it( 'should return proper number of columns for rowspan table', () => {
+			setData( model, modelTable( [
+				[ '00', '01' ],
+				[ { rowspan: 2, contents: '10' }, '11' ],
+				[ '21' ]
+			] ) );
+
+			expect( tableUtils.getRows( root.getNodeByPath( [ 0 ] ) ) ).to.equal( 4 );
+		} );
+	} );
 } );