Merge pull request #230 from ckeditor/i/6113

Feature: Introduced the `TableSelection` plugin. Closes ckeditor/ckeditor5#6113.

src/commands/removecolumncommand.js

@@ -51,18 +51,19 @@ export default class RemoveColumnCommand extends Command {
 		const table = tableRow.parent;
 
 		const headingColumns = table.getAttribute( 'headingColumns' ) || 0;
-		const row = table.getChildIndex( tableRow );
 
 		// 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 && row <= headingColumns ) {
+			if ( headingColumns && selectionRow <= headingColumns ) {
 				writer.setAttribute( 'headingColumns', headingColumns - 1, table );
 			}
 
@@ -75,6 +76,21 @@ export default class RemoveColumnCommand extends Command {
 					writer.remove( cell );
 				}
 			}
+
+			writer.setSelection( writer.createPositionAt( cellToFocus, 0 ) );
 		} );
 	}
 }
+
+// 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 );
+
+	if ( colspan > 1 ) {
+		return tableCell;
+	}
+
+	return tableCell.nextSibling ? tableCell.nextSibling : tableCell.previousSibling;
+}

src/commands/removerowcommand.js

@@ -48,30 +48,35 @@ export default class RemoveRowCommand extends Command {
 		const tableRow = tableCell.parent;
 		const table = tableRow.parent;
 
-		const currentRow = table.getChildIndex( tableRow );
+		const removedRow = table.getChildIndex( tableRow );
+
+		const tableMap = [ ...new TableWalker( table, { endRow: removedRow } ) ];
+
+		const cellData = tableMap.find( value => value.cell === tableCell );
+
 		const headingRows = table.getAttribute( 'headingRows' ) || 0;
 
+		const columnToFocus = cellData.column;
+
 		model.change( writer => {
-			if ( headingRows && currentRow <= headingRows ) {
+			if ( headingRows && removedRow <= headingRows ) {
 				updateNumericAttribute( 'headingRows', headingRows - 1, table, writer, 0 );
 			}
 
-			const tableMap = [ ...new TableWalker( table, { endRow: currentRow } ) ];
-
 			const cellsToMove = new Map();
 
 			// Get cells from removed row that are spanned over multiple rows.
 			tableMap
-				.filter( ( { row, rowspan } ) => row === currentRow && rowspan > 1 )
+				.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 <= currentRow - 1 && row + rowspan > currentRow )
+				.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 = currentRow + 1;
+			const targetRow = removedRow + 1;
 			const tableWalker = new TableWalker( table, { includeSpanned: true, startRow: targetRow, endRow: targetRow } );
 
 			let previousCell;
@@ -93,6 +98,27 @@ export default class RemoveRowCommand extends Command {
 			}
 
 			writer.remove( tableRow );
+
+			const cellToFocus = getCellToFocus( table, removedRow, columnToFocus );
+			writer.setSelection( writer.createPositionAt( cellToFocus, 0 ) );
 		} );
 	}
 }
+
+// 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 );
+
+	// Default to first table cell.
+	let cellToFocus = row.getChild( 0 );
+	let column = 0;
+
+	for ( const tableCell of row.getChildren() ) {
+		if ( column > columnToFocus ) {
+			return cellToFocus;
+		}
+
+		cellToFocus = tableCell;
+		column += parseInt( tableCell.getAttribute( 'colspan' ) || 1 );
+	}
+}

src/commands/utils.js

@@ -10,14 +10,15 @@
 import { isObject } from 'lodash-es';
 
 /**
- * Returns the parent element of the given name. Returns undefined if the position is not inside the desired parent.
+ * Returns the parent element of the given name. Returns undefined if the position or the element is not inside the desired parent.
  *
  * @param {String} parentName The name of the parent element to find.
- * @param {module:engine/model/position~Position|module:engine/model/position~Position} position The position to start searching.
+ * @param {module:engine/model/position~Position|module:engine/model/position~Position} positionOrElement The position or
+ * the parentElement to start searching.
  * @returns {module:engine/model/element~Element|module:engine/model/documentfragment~DocumentFragment}
  */
-export function findAncestor( parentName, position ) {
-	let parent = position.parent;
+export function findAncestor( parentName, positionOrElement ) {
+	let parent = positionOrElement.parent;
 
 	while ( parent ) {
 		if ( parent.name === parentName ) {

src/tableediting.js

@@ -75,7 +75,7 @@ export default class TableEditing extends Plugin {
 		schema.register( 'tableCell', {
 			allowIn: 'tableRow',
 			allowAttributes: [ 'colspan', 'rowspan' ],
-			isLimit: true
+			isObject: true
 		} );
 
 		// Allow all $block content inside table cell.