Parcourir la source

Other: Improve readability of downcast attribute converters.

Maciej Gołaszewski il y a 7 ans
Parent
commit
b76b548ab9

+ 136 - 78
packages/ckeditor5-table/src/converters/downcast.js

@@ -33,9 +33,6 @@ export function downcastInsertTable( options = {} ) {
 		conversionApi.consumable.consume( table, 'attribute:headingRows:table' );
 		conversionApi.consumable.consume( table, 'attribute:headingColumns:table' );
 
-		// The <thead> and <tbody> elements are created on the fly when needed & cached by `getOrCreateTableSection()` function.
-		const tableSections = {};
-
 		const asWidget = options && options.asWidget;
 
 		const tableElement = conversionApi.writer.createContainerElement( 'table' );
@@ -51,7 +48,7 @@ export function downcastInsertTable( options = {} ) {
 		for ( const tableWalkerValue of tableWalker ) {
 			const { row, cell } = tableWalkerValue;
 
-			const tableSection = getOrCreateTableSection( getSectionName( tableWalkerValue ), tableElement, conversionApi, tableSections );
+			const tableSection = getOrCreateTableSection( getSectionName( tableWalkerValue ), tableElement, conversionApi );
 			const tableRow = table.getChild( row );
 
 			// Check if row was converted
@@ -141,92 +138,96 @@ export function downcastInsertCell( options = {} ) {
 }
 
 /**
- * Conversion helper that acts on attribute change for headingColumns and headingRows attributes.
+ * Conversion helper that acts on headingRows table attribute change.
  *
- * Depending on changed attributes this converter will:
- * - rename <td> to <th> elements or vice versa
- * - create <thead> or <tbody> elements
- * - remove empty <thead> or <tbody>
+ * This converter will:
+ * - Rename <td> to <th> elements or vice versa depending on headings.
+ * - Create <thead> or <tbody> elements if needed.
+ * - Remove empty <thead> or <tbody> if needed.
  *
  * @returns {Function} Conversion helper.
  */
-export function downcastAttributeChange( options ) {
-	const attribute = options.attribute;
+export function downcastTableHeadingRowsChange( options = {} ) {
 	const asWidget = !!options.asWidget;
 
-	return dispatcher => dispatcher.on( `attribute:${ attribute }:table`, ( evt, data, conversionApi ) => {
+	return dispatcher => dispatcher.on( 'attribute:headingRows:table', ( evt, data, conversionApi ) => {
 		const table = data.item;
 
 		if ( !conversionApi.consumable.consume( data.item, evt.name ) ) {
 			return;
 		}
 
-		const tableElement = conversionApi.mapper.toViewElement( table );
+		const viewTable = conversionApi.mapper.toViewElement( table );
 
-		const cachedTableSections = {};
+		const oldRows = data.attributeOldValue;
+		const newRows = data.attributeNewValue;
 
-		const tableWalker = new TableWalker( table );
-
-		for ( const tableWalkerValue of tableWalker ) {
-			const { row, cell } = tableWalkerValue;
-			const tableRow = table.getChild( row );
+		// The head section has grown so move rows from <tbody> to <thead>.
+		if ( newRows > oldRows ) {
+			// Filter out only those rows that are in wrong section.
+			const rowsToMove = Array.from( table.getChildren() ).filter( ( { index } ) => isBetween( index, oldRows - 1, newRows ) );
 
-			const trElement = conversionApi.mapper.toViewElement( tableRow );
+			const viewTableHead = getOrCreateTableSection( 'thead', viewTable, conversionApi );
+			moveViewRowsToTableSection( rowsToMove, viewTableHead, conversionApi, 'end' );
 
-			// The TR element might be not converted yet (ie when adding a row to a heading section).
-			// It will be converted by downcastInsertRow() conversion helper.
-			if ( !trElement ) {
-				continue;
+			// Rename all table cells from moved rows to 'th' as they lands in <thead>.
+			for ( const tableRow of rowsToMove ) {
+				for ( const tableCell of tableRow.getChildren() ) {
+					renameViewTableCell( tableCell, 'th', conversionApi, asWidget );
+				}
 			}
 
-			const desiredParentName = getSectionName( tableWalkerValue );
-
-			if ( desiredParentName !== trElement.parent.name ) {
-				let targetPosition;
-
-				if (
-					( desiredParentName == 'tbody' && row === data.attributeNewValue && data.attributeNewValue < data.attributeOldValue ) ||
-					row === 0
-				) {
-					const tableSection = getOrCreateTableSection( desiredParentName, tableElement, conversionApi, cachedTableSections );
+			// 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 <thead> to <tbody>.
+		else {
+			// Filter out only those rows that are in wrong section.
+			const rowsToMove = Array.from( table.getChildren() )
+				.filter( ( { index } ) => isBetween( index, newRows - 1, oldRows ) )
+				.reverse(); // The rows will be moved from <thead> to <tbody> in reverse order at the beginning of a <tbody>.
 
-					targetPosition = ViewPosition.createAt( tableSection, 'start' );
-				} else {
-					const previousTr = conversionApi.mapper.toViewElement( table.getChild( row - 1 ) );
+			const viewTableBody = getOrCreateTableSection( 'tbody', viewTable, conversionApi );
+			moveViewRowsToTableSection( rowsToMove, viewTableBody, conversionApi );
 
-					targetPosition = ViewPosition.createAfter( previousTr );
-				}
+			// Check if cells moved from <thead> to <tbody> requires renaming to <td> as this depends on current heading columns attribute.
+			const tableWalker = new TableWalker( table, { startRow: newRows ? newRows - 1 : newRows, endRow: oldRows - 1 } );
 
-				conversionApi.writer.move( ViewRange.createOn( trElement ), targetPosition );
+			for ( const tableWalkerValue of tableWalker ) {
+				renameViewTableCellIfRequired( tableWalkerValue, conversionApi, asWidget );
 			}
 
-			// Check whether current columnIndex is overlapped by table cells from previous rows.
-			const desiredCellElementName = getCellElementName( tableWalkerValue );
-
-			const viewCell = conversionApi.mapper.toViewElement( cell );
+			// 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 );
+		}
 
-			// If in single change we're converting attribute changes and inserting cell the table cell might not be inserted into view
-			// because of child conversion is done after parent.
-			if ( viewCell && viewCell.name !== desiredCellElementName ) {
-				let renamedCell;
+		function isBetween( index, lower, upper ) {
+			return index > lower && index < upper;
+		}
+	}, { priority: 'normal' } );
+}
 
-				if ( asWidget ) {
-					const editable = conversionApi.writer.createEditableElement( desiredCellElementName, viewCell.getAttributes() );
-					renamedCell = toWidgetEditable( editable, conversionApi.writer );
+/**
+ * Conversion helper that acts on headingColumns table attribute change.
+ *
+ * Depending on changed attributes this converter will rename <td> to <th> elements or vice versa depending of cell column index.
+ *
+ * @returns {Function} Conversion helper.
+ */
+export function downcastTableHeadingColumnsChange( options = {} ) {
+	const asWidget = !!options.asWidget;
 
-					conversionApi.writer.insert( ViewPosition.createAfter( viewCell ), renamedCell );
-					conversionApi.writer.move( ViewRange.createIn( viewCell ), ViewPosition.createAt( renamedCell ) );
-					conversionApi.writer.remove( ViewRange.createOn( viewCell ) );
-				} else {
-					renamedCell = conversionApi.writer.rename( viewCell, desiredCellElementName );
-				}
+	return dispatcher => dispatcher.on( 'attribute:headingColumns:table', ( evt, data, conversionApi ) => {
+		const table = data.item;
 
-				conversionApi.mapper.bindElements( cell, renamedCell );
-			}
+		if ( !conversionApi.consumable.consume( data.item, evt.name ) ) {
+			return;
 		}
 
-		removeTableSectionIfEmpty( 'thead', tableElement, conversionApi );
-		removeTableSectionIfEmpty( 'tbody', tableElement, conversionApi );
+		// TODO: column walk only?
+		for ( const tableWalkerValue of new TableWalker( table ) ) {
+			renameViewTableCellIfRequired( tableWalkerValue, conversionApi, asWidget );
+		}
 	}, { priority: 'normal' } );
 }
 
@@ -267,11 +268,56 @@ export function downcastRemoveRow() {
 	}, { priority: 'higher' } );
 }
 
+// Renames table cell in the view to given element name.
+//
+// @param {module:engine/model/element~Element} tableCell
+// @param {String} desiredCellElementName
+// @param {Object} conversionApi
+// @param {Boolean} asWidget
+function renameViewTableCell( tableCell, desiredCellElementName, conversionApi, asWidget ) {
+	const viewCell = conversionApi.mapper.toViewElement( tableCell );
+
+	let renamedCell;
+
+	if ( asWidget ) {
+		const editable = conversionApi.writer.createEditableElement( desiredCellElementName, viewCell.getAttributes() );
+		renamedCell = toWidgetEditable( editable, conversionApi.writer );
+
+		conversionApi.writer.insert( ViewPosition.createAfter( viewCell ), renamedCell );
+		conversionApi.writer.move( ViewRange.createIn( viewCell ), ViewPosition.createAt( renamedCell ) );
+		conversionApi.writer.remove( ViewRange.createOn( viewCell ) );
+	} else {
+		renamedCell = conversionApi.writer.rename( viewCell, desiredCellElementName );
+	}
+
+	conversionApi.mapper.bindElements( tableCell, renamedCell );
+}
+
+// Renames a table cell element in a view according to it's location in table.
+//
+// @param {module:table/tablewalker~TableWalkerValue} tableWalkerValue
+// @param {Object} conversionApi
+// @param {Boolean} asWidget
+function renameViewTableCellIfRequired( tableWalkerValue, conversionApi, asWidget ) {
+	const { cell } = tableWalkerValue;
+
+	// Check whether current columnIndex is overlapped by table cells from previous rows.
+	const desiredCellElementName = getCellElementName( tableWalkerValue );
+
+	const viewCell = conversionApi.mapper.toViewElement( cell );
+
+	// If in single change we're converting attribute changes and inserting cell the table cell might not be inserted into view
+	// because of child conversion is done after parent.
+	if ( viewCell && viewCell.name !== desiredCellElementName ) {
+		renameViewTableCell( cell, desiredCellElementName, conversionApi, asWidget );
+	}
+}
+
 // Creates a table cell element in a view.
 //
 // @param {module:table/tablewalker~TableWalkerValue} tableWalkerValue
 // @param {module:engine/view/position~Position} insertPosition
-// @param conversionApi
+// @param {Object} conversionApi
 function createViewTableCellElement( tableWalkerValue, insertPosition, conversionApi, options ) {
 	const tableCell = tableWalkerValue.cell;
 
@@ -288,6 +334,12 @@ function createViewTableCellElement( tableWalkerValue, insertPosition, conversio
 }
 
 // Creates or returns an existing tr element from a view.
+//
+// @param {module:engine/view/element~Element} tableRow
+// @param {Number} rowIndex
+// @param {module:engine/view/element~Element} tableSection
+// @param {Object} conversionApi
+// @returns {module:engine/view/element~Element}
 function getOrCreateTr( tableRow, rowIndex, tableSection, conversionApi ) {
 	let trElement = conversionApi.mapper.toViewElement( tableRow );
 
@@ -342,29 +394,21 @@ function getSectionName( tableWalkerValue ) {
 // Creates or returns an existing <tbody> or <thead> element witch caching.
 //
 // @param {String} sectionName
-// @param {module:engine/view/element~Element} tableElement
-// @param conversionApi
+// @param {module:engine/view/element~Element} viewTable
+// @param {Object} conversionApi
 // @param {Object} cachedTableSection An object on which store cached elements.
 // @return {module:engine/view/containerelement~ContainerElement}
-function getOrCreateTableSection( sectionName, tableElement, conversionApi, cachedTableSections = {} ) {
-	if ( cachedTableSections[ sectionName ] ) {
-		return cachedTableSections[ sectionName ];
-	}
-
-	cachedTableSections[ sectionName ] = getExistingTableSectionElement( sectionName, tableElement );
+function getOrCreateTableSection( sectionName, viewTable, conversionApi ) {
+	const viewTableSection = getExistingTableSectionElement( sectionName, viewTable );
 
-	if ( !cachedTableSections[ sectionName ] ) {
-		cachedTableSections[ sectionName ] = createTableSection( sectionName, tableElement, conversionApi );
-	}
-
-	return cachedTableSections[ sectionName ];
+	return viewTableSection ? viewTableSection : createTableSection( sectionName, viewTable, conversionApi );
 }
 
 // Finds an existing <tbody> or <thead> element or returns undefined.
 //
 // @param {String} sectionName
 // @param {module:engine/view/element~Element} tableElement
-// @param conversionApi
+// @param {Object} conversionApi
 function getExistingTableSectionElement( sectionName, tableElement ) {
 	for ( const tableSection of tableElement.getChildren() ) {
 		if ( tableSection.name == sectionName ) {
@@ -377,7 +421,7 @@ function getExistingTableSectionElement( sectionName, tableElement ) {
 //
 // @param {String} sectionName
 // @param {module:engine/view/element~Element} tableElement
-// @param conversionApi
+// @param {Object} conversionApi
 // @return {module:engine/view/containerelement~ContainerElement}
 function createTableSection( sectionName, tableElement, conversionApi ) {
 	const tableChildElement = conversionApi.writer.createContainerElement( sectionName );
@@ -391,7 +435,7 @@ function createTableSection( sectionName, tableElement, conversionApi ) {
 //
 // @param {String} sectionName
 // @param {module:engine/view/element~Element} tableElement
-// @param conversionApi
+// @param {Object} conversionApi
 function removeTableSectionIfEmpty( sectionName, tableElement, conversionApi ) {
 	const tableSection = getExistingTableSectionElement( sectionName, tableElement );
 
@@ -399,3 +443,17 @@ function removeTableSectionIfEmpty( sectionName, tableElement, conversionApi ) {
 		conversionApi.writer.remove( ViewRange.createOn( tableSection ) );
 	}
 }
+
+// Moves view table rows associated with passed model rows to provided table section element.
+//
+// @param {Array.<module:engine/model/element~Element>} rowsToMove
+// @param {module:engine/view/element~Element} viewTableSection
+// @param {Object} conversionApi
+// @param {Number|'end'|'before'|'after'} [offset=0] Offset or one of the flags.
+function moveViewRowsToTableSection( rowsToMove, viewTableSection, conversionApi, offset ) {
+	for ( const tableRow of rowsToMove ) {
+		const viewTableRow = conversionApi.mapper.toViewElement( tableRow );
+
+		conversionApi.writer.move( ViewRange.createOn( viewTableRow ), ViewPosition.createAt( viewTableSection, offset ) );
+	}
+}

+ 7 - 6
packages/ckeditor5-table/src/tableediting.js

@@ -14,11 +14,12 @@ import { keyCodes } from '@ckeditor/ckeditor5-utils/src/keyboard';
 
 import upcastTable from './converters/upcasttable';
 import {
-	downcastAttributeChange,
 	downcastInsertCell,
 	downcastInsertRow,
 	downcastInsertTable,
-	downcastRemoveRow
+	downcastRemoveRow,
+	downcastTableHeadingColumnsChange,
+	downcastTableHeadingRowsChange
 } from './converters/downcast';
 import InsertTableCommand from './commands/inserttablecommand';
 import InsertRowCommand from './commands/insertrowcommand';
@@ -91,10 +92,10 @@ export default class TablesEditing extends Plugin {
 		conversion.attributeToAttribute( { model: 'colspan', view: 'colspan' } );
 		conversion.attributeToAttribute( { model: 'rowspan', view: 'rowspan' } );
 
-		conversion.for( 'editingDowncast' ).add( downcastAttributeChange( { attribute: 'headingRows', asWidget: true } ) );
-		conversion.for( 'dataDowncast' ).add( downcastAttributeChange( { attribute: 'headingRows' } ) );
-		conversion.for( 'editingDowncast' ).add( downcastAttributeChange( { attribute: 'headingColumns', asWidget: true } ) );
-		conversion.for( 'dataDowncast' ).add( downcastAttributeChange( { attribute: 'headingColumns' } ) );
+		conversion.for( 'editingDowncast' ).add( downcastTableHeadingColumnsChange( { asWidget: true } ) );
+		conversion.for( 'dataDowncast' ).add( downcastTableHeadingColumnsChange() );
+		conversion.for( 'editingDowncast' ).add( downcastTableHeadingRowsChange( { asWidget: true } ) );
+		conversion.for( 'dataDowncast' ).add( downcastTableHeadingRowsChange() );
 
 		editor.commands.add( 'insertTable', new InsertTableCommand( editor ) );
 		editor.commands.add( 'insertRowAbove', new InsertRowCommand( editor, { order: 'above' } ) );

+ 19 - 3
packages/ckeditor5-table/tests/commands/insertcolumncommand.js

@@ -8,7 +8,14 @@ import { setData, getData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model
 import { upcastElementToElement } from '@ckeditor/ckeditor5-engine/src/conversion/upcast-converters';
 
 import InsertColumnCommand from '../../src/commands/insertcolumncommand';
-import { downcastInsertTable } from '../../src/converters/downcast';
+import {
+	downcastInsertCell,
+	downcastInsertRow,
+	downcastInsertTable,
+	downcastRemoveRow,
+	downcastTableHeadingColumnsChange,
+	downcastTableHeadingRowsChange
+} from '../../src/converters/downcast';
 import upcastTable from '../../src/converters/upcasttable';
 import { formatTable, formattedModelTable, modelTable } from '../_utils/utils';
 import TableUtils from '../../src/tableutils';
@@ -54,15 +61,24 @@ describe( 'InsertColumnCommand', () => {
 			conversion.for( 'upcast' ).add( upcastTable() );
 			conversion.for( 'downcast' ).add( downcastInsertTable() );
 
-			// Table row upcast only since downcast conversion is done in `downcastTable()`.
-			conversion.for( 'upcast' ).add( upcastElementToElement( { model: 'tableRow', view: 'tr' } ) );
+			// Insert row conversion.
+			conversion.for( 'downcast' ).add( downcastInsertRow() );
+
+			// Remove row conversion.
+			conversion.for( 'downcast' ).add( downcastRemoveRow() );
 
 			// Table cell conversion.
+			conversion.for( 'downcast' ).add( downcastInsertCell() );
+
 			conversion.for( 'upcast' ).add( upcastElementToElement( { model: 'tableCell', view: 'td' } ) );
 			conversion.for( 'upcast' ).add( upcastElementToElement( { model: 'tableCell', view: 'th' } ) );
 
+			// Table attributes conversion.
 			conversion.attributeToAttribute( { model: 'colspan', view: 'colspan' } );
 			conversion.attributeToAttribute( { model: 'rowspan', view: 'rowspan' } );
+
+			conversion.for( 'downcast' ).add( downcastTableHeadingColumnsChange() );
+			conversion.for( 'downcast' ).add( downcastTableHeadingRowsChange() );
 		} );
 	} );
 

+ 19 - 3
packages/ckeditor5-table/tests/commands/insertrowcommand.js

@@ -8,7 +8,14 @@ import { setData, getData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model
 import { upcastElementToElement } from '@ckeditor/ckeditor5-engine/src/conversion/upcast-converters';
 
 import InsertRowCommand from '../../src/commands/insertrowcommand';
-import { downcastInsertTable } from '../../src/converters/downcast';
+import {
+	downcastInsertCell,
+	downcastInsertRow,
+	downcastInsertTable,
+	downcastRemoveRow,
+	downcastTableHeadingColumnsChange,
+	downcastTableHeadingRowsChange
+} from '../../src/converters/downcast';
 import upcastTable from '../../src/converters/upcasttable';
 import { formatTable, formattedModelTable, modelTable } from '../_utils/utils';
 import TableUtils from '../../src/tableutils';
@@ -54,15 +61,24 @@ describe( 'InsertRowCommand', () => {
 			conversion.for( 'upcast' ).add( upcastTable() );
 			conversion.for( 'downcast' ).add( downcastInsertTable() );
 
-			// Table row upcast only since downcast conversion is done in `downcastTable()`.
-			conversion.for( 'upcast' ).add( upcastElementToElement( { model: 'tableRow', view: 'tr' } ) );
+			// Insert row conversion.
+			conversion.for( 'downcast' ).add( downcastInsertRow() );
+
+			// Remove row conversion.
+			conversion.for( 'downcast' ).add( downcastRemoveRow() );
 
 			// Table cell conversion.
+			conversion.for( 'downcast' ).add( downcastInsertCell() );
+
 			conversion.for( 'upcast' ).add( upcastElementToElement( { model: 'tableCell', view: 'td' } ) );
 			conversion.for( 'upcast' ).add( upcastElementToElement( { model: 'tableCell', view: 'th' } ) );
 
+			// Table attributes conversion.
 			conversion.attributeToAttribute( { model: 'colspan', view: 'colspan' } );
 			conversion.attributeToAttribute( { model: 'rowspan', view: 'rowspan' } );
+
+			conversion.for( 'downcast' ).add( downcastTableHeadingColumnsChange() );
+			conversion.for( 'downcast' ).add( downcastTableHeadingRowsChange() );
 		} );
 	} );
 

+ 19 - 3
packages/ckeditor5-table/tests/commands/inserttablecommand.js

@@ -8,7 +8,14 @@ import { setData, getData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model
 import { upcastElementToElement } from '@ckeditor/ckeditor5-engine/src/conversion/upcast-converters';
 
 import InsertTableCommand from '../../src/commands/inserttablecommand';
-import { downcastInsertTable } from '../../src/converters/downcast';
+import {
+	downcastInsertCell,
+	downcastInsertRow,
+	downcastInsertTable,
+	downcastRemoveRow,
+	downcastTableHeadingColumnsChange,
+	downcastTableHeadingRowsChange
+} from '../../src/converters/downcast';
 import upcastTable from '../../src/converters/upcasttable';
 import TableUtils from '../../src/tableutils';
 
@@ -54,15 +61,24 @@ describe( 'InsertTableCommand', () => {
 			conversion.for( 'upcast' ).add( upcastTable() );
 			conversion.for( 'downcast' ).add( downcastInsertTable() );
 
-			// Table row upcast only since downcast conversion is done in `downcastTable()`.
-			conversion.for( 'upcast' ).add( upcastElementToElement( { model: 'tableRow', view: 'tr' } ) );
+			// Insert row conversion.
+			conversion.for( 'downcast' ).add( downcastInsertRow() );
+
+			// Remove row conversion.
+			conversion.for( 'downcast' ).add( downcastRemoveRow() );
 
 			// Table cell conversion.
+			conversion.for( 'downcast' ).add( downcastInsertCell() );
+
 			conversion.for( 'upcast' ).add( upcastElementToElement( { model: 'tableCell', view: 'td' } ) );
 			conversion.for( 'upcast' ).add( upcastElementToElement( { model: 'tableCell', view: 'th' } ) );
 
+			// Table attributes conversion.
 			conversion.attributeToAttribute( { model: 'colspan', view: 'colspan' } );
 			conversion.attributeToAttribute( { model: 'rowspan', view: 'rowspan' } );
+
+			conversion.for( 'downcast' ).add( downcastTableHeadingColumnsChange() );
+			conversion.for( 'downcast' ).add( downcastTableHeadingRowsChange() );
 		} );
 	} );
 

+ 19 - 3
packages/ckeditor5-table/tests/commands/mergecellcommand.js

@@ -8,7 +8,14 @@ import { setData, getData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model
 import { upcastElementToElement } from '@ckeditor/ckeditor5-engine/src/conversion/upcast-converters';
 
 import MergeCellCommand from '../../src/commands/mergecellcommand';
-import { downcastInsertTable } from '../../src/converters/downcast';
+import {
+	downcastInsertCell,
+	downcastInsertRow,
+	downcastInsertTable,
+	downcastRemoveRow,
+	downcastTableHeadingColumnsChange,
+	downcastTableHeadingRowsChange
+} from '../../src/converters/downcast';
 import upcastTable from '../../src/converters/upcasttable';
 import { formatTable, formattedModelTable, modelTable } from '../_utils/utils';
 
@@ -53,15 +60,24 @@ describe( 'MergeCellCommand', () => {
 				conversion.for( 'upcast' ).add( upcastTable() );
 				conversion.for( 'downcast' ).add( downcastInsertTable() );
 
-				// Table row upcast only since downcast conversion is done in `downcastTable()`.
-				conversion.for( 'upcast' ).add( upcastElementToElement( { model: 'tableRow', view: 'tr' } ) );
+				// Insert row conversion.
+				conversion.for( 'downcast' ).add( downcastInsertRow() );
+
+				// Remove row conversion.
+				conversion.for( 'downcast' ).add( downcastRemoveRow() );
 
 				// Table cell conversion.
+				conversion.for( 'downcast' ).add( downcastInsertCell() );
+
 				conversion.for( 'upcast' ).add( upcastElementToElement( { model: 'tableCell', view: 'td' } ) );
 				conversion.for( 'upcast' ).add( upcastElementToElement( { model: 'tableCell', view: 'th' } ) );
 
+				// Table attributes conversion.
 				conversion.attributeToAttribute( { model: 'colspan', view: 'colspan' } );
 				conversion.attributeToAttribute( { model: 'rowspan', view: 'rowspan' } );
+
+				conversion.for( 'downcast' ).add( downcastTableHeadingColumnsChange() );
+				conversion.for( 'downcast' ).add( downcastTableHeadingRowsChange() );
 			} );
 	} );
 

+ 19 - 3
packages/ckeditor5-table/tests/commands/removecolumncommand.js

@@ -8,7 +8,14 @@ import { setData, getData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model
 import { upcastElementToElement } from '@ckeditor/ckeditor5-engine/src/conversion/upcast-converters';
 
 import RemoveColumnCommand from '../../src/commands/removecolumncommand';
-import { downcastInsertTable } from '../../src/converters/downcast';
+import {
+	downcastInsertCell,
+	downcastInsertRow,
+	downcastInsertTable,
+	downcastRemoveRow,
+	downcastTableHeadingColumnsChange,
+	downcastTableHeadingRowsChange
+} from '../../src/converters/downcast';
 import upcastTable from '../../src/converters/upcasttable';
 import { formatTable, formattedModelTable, modelTable } from '../_utils/utils';
 import TableUtils from '../../src/tableutils';
@@ -55,15 +62,24 @@ describe( 'RemoveColumnCommand', () => {
 			conversion.for( 'upcast' ).add( upcastTable() );
 			conversion.for( 'downcast' ).add( downcastInsertTable() );
 
-			// Table row upcast only since downcast conversion is done in `downcastTable()`.
-			conversion.for( 'upcast' ).add( upcastElementToElement( { model: 'tableRow', view: 'tr' } ) );
+			// Insert row conversion.
+			conversion.for( 'downcast' ).add( downcastInsertRow() );
+
+			// Remove row conversion.
+			conversion.for( 'downcast' ).add( downcastRemoveRow() );
 
 			// Table cell conversion.
+			conversion.for( 'downcast' ).add( downcastInsertCell() );
+
 			conversion.for( 'upcast' ).add( upcastElementToElement( { model: 'tableCell', view: 'td' } ) );
 			conversion.for( 'upcast' ).add( upcastElementToElement( { model: 'tableCell', view: 'th' } ) );
 
+			// Table attributes conversion.
 			conversion.attributeToAttribute( { model: 'colspan', view: 'colspan' } );
 			conversion.attributeToAttribute( { model: 'rowspan', view: 'rowspan' } );
+
+			conversion.for( 'downcast' ).add( downcastTableHeadingColumnsChange() );
+			conversion.for( 'downcast' ).add( downcastTableHeadingRowsChange() );
 		} );
 	} );
 

+ 19 - 3
packages/ckeditor5-table/tests/commands/removerowcommand.js

@@ -8,7 +8,14 @@ import { setData, getData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model
 import { upcastElementToElement } from '@ckeditor/ckeditor5-engine/src/conversion/upcast-converters';
 
 import RemoveRowCommand from '../../src/commands/removerowcommand';
-import { downcastInsertTable } from '../../src/converters/downcast';
+import {
+	downcastInsertCell,
+	downcastInsertRow,
+	downcastInsertTable,
+	downcastRemoveRow,
+	downcastTableHeadingColumnsChange,
+	downcastTableHeadingRowsChange
+} from '../../src/converters/downcast';
 import upcastTable from '../../src/converters/upcasttable';
 import { formatTable, formattedModelTable, modelTable } from '../_utils/utils';
 
@@ -53,15 +60,24 @@ describe( 'RemoveRowCommand', () => {
 				conversion.for( 'upcast' ).add( upcastTable() );
 				conversion.for( 'downcast' ).add( downcastInsertTable() );
 
-				// Table row upcast only since downcast conversion is done in `downcastTable()`.
-				conversion.for( 'upcast' ).add( upcastElementToElement( { model: 'tableRow', view: 'tr' } ) );
+				// Insert row conversion.
+				conversion.for( 'downcast' ).add( downcastInsertRow() );
+
+				// Remove row conversion.
+				conversion.for( 'downcast' ).add( downcastRemoveRow() );
 
 				// Table cell conversion.
+				conversion.for( 'downcast' ).add( downcastInsertCell() );
+
 				conversion.for( 'upcast' ).add( upcastElementToElement( { model: 'tableCell', view: 'td' } ) );
 				conversion.for( 'upcast' ).add( upcastElementToElement( { model: 'tableCell', view: 'th' } ) );
 
+				// Table attributes conversion.
 				conversion.attributeToAttribute( { model: 'colspan', view: 'colspan' } );
 				conversion.attributeToAttribute( { model: 'rowspan', view: 'rowspan' } );
+
+				conversion.for( 'downcast' ).add( downcastTableHeadingColumnsChange() );
+				conversion.for( 'downcast' ).add( downcastTableHeadingRowsChange() );
 			} );
 	} );
 

+ 19 - 3
packages/ckeditor5-table/tests/commands/settableheaderscommand.js

@@ -8,7 +8,14 @@ import { setData, getData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model
 import { upcastElementToElement } from '@ckeditor/ckeditor5-engine/src/conversion/upcast-converters';
 
 import SetTableHeadersCommand from '../../src/commands/settableheaderscommand';
-import { downcastInsertTable } from '../../src/converters/downcast';
+import {
+	downcastInsertCell,
+	downcastInsertRow,
+	downcastInsertTable,
+	downcastRemoveRow,
+	downcastTableHeadingColumnsChange,
+	downcastTableHeadingRowsChange
+} from '../../src/converters/downcast';
 import upcastTable from '../../src/converters/upcasttable';
 import { formatTable, formattedModelTable, modelTable } from '../_utils/utils';
 
@@ -53,15 +60,24 @@ describe( 'SetTableHeadersCommand', () => {
 				conversion.for( 'upcast' ).add( upcastTable() );
 				conversion.for( 'downcast' ).add( downcastInsertTable() );
 
-				// Table row upcast only since downcast conversion is done in `downcastTable()`.
-				conversion.for( 'upcast' ).add( upcastElementToElement( { model: 'tableRow', view: 'tr' } ) );
+				// Insert row conversion.
+				conversion.for( 'downcast' ).add( downcastInsertRow() );
+
+				// Remove row conversion.
+				conversion.for( 'downcast' ).add( downcastRemoveRow() );
 
 				// Table cell conversion.
+				conversion.for( 'downcast' ).add( downcastInsertCell() );
+
 				conversion.for( 'upcast' ).add( upcastElementToElement( { model: 'tableCell', view: 'td' } ) );
 				conversion.for( 'upcast' ).add( upcastElementToElement( { model: 'tableCell', view: 'th' } ) );
 
+				// Table attributes conversion.
 				conversion.attributeToAttribute( { model: 'colspan', view: 'colspan' } );
 				conversion.attributeToAttribute( { model: 'rowspan', view: 'rowspan' } );
+
+				conversion.for( 'downcast' ).add( downcastTableHeadingColumnsChange() );
+				conversion.for( 'downcast' ).add( downcastTableHeadingRowsChange() );
 			} );
 	} );
 

+ 19 - 3
packages/ckeditor5-table/tests/commands/splitcellcommand.js

@@ -8,7 +8,14 @@ import { setData, getData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model
 import { upcastElementToElement } from '@ckeditor/ckeditor5-engine/src/conversion/upcast-converters';
 
 import SplitCellCommand from '../../src/commands/splitcellcommand';
-import { downcastInsertTable } from '../../src/converters/downcast';
+import {
+	downcastInsertCell,
+	downcastInsertRow,
+	downcastInsertTable,
+	downcastRemoveRow,
+	downcastTableHeadingColumnsChange,
+	downcastTableHeadingRowsChange
+} from '../../src/converters/downcast';
 import upcastTable from '../../src/converters/upcasttable';
 import { formatTable, formattedModelTable, modelTable } from '../_utils/utils';
 import TableUtils from '../../src/tableutils';
@@ -55,15 +62,24 @@ describe( 'SplitCellCommand', () => {
 			conversion.for( 'upcast' ).add( upcastTable() );
 			conversion.for( 'downcast' ).add( downcastInsertTable() );
 
-			// Table row upcast only since downcast conversion is done in `downcastTable()`.
-			conversion.for( 'upcast' ).add( upcastElementToElement( { model: 'tableRow', view: 'tr' } ) );
+			// Insert row conversion.
+			conversion.for( 'downcast' ).add( downcastInsertRow() );
+
+			// Remove row conversion.
+			conversion.for( 'downcast' ).add( downcastRemoveRow() );
 
 			// Table cell conversion.
+			conversion.for( 'downcast' ).add( downcastInsertCell() );
+
 			conversion.for( 'upcast' ).add( upcastElementToElement( { model: 'tableCell', view: 'td' } ) );
 			conversion.for( 'upcast' ).add( upcastElementToElement( { model: 'tableCell', view: 'th' } ) );
 
+			// Table attributes conversion.
 			conversion.attributeToAttribute( { model: 'colspan', view: 'colspan' } );
 			conversion.attributeToAttribute( { model: 'rowspan', view: 'rowspan' } );
+
+			conversion.for( 'downcast' ).add( downcastTableHeadingColumnsChange() );
+			conversion.for( 'downcast' ).add( downcastTableHeadingRowsChange() );
 		} );
 	} );
 

+ 193 - 105
packages/ckeditor5-table/tests/converters/downcast.js

@@ -8,11 +8,12 @@ import { getData as getViewData } from '@ckeditor/ckeditor5-engine/src/dev-utils
 import { setData as setModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model';
 
 import {
-	downcastAttributeChange,
 	downcastInsertCell,
 	downcastInsertRow,
 	downcastInsertTable,
-	downcastRemoveRow
+	downcastRemoveRow,
+	downcastTableHeadingColumnsChange,
+	downcastTableHeadingRowsChange
 } from '../../src/converters/downcast';
 import { formatTable, formattedViewTable, modelTable, viewTable } from '../_utils/utils';
 
@@ -63,8 +64,8 @@ describe( 'downcast converters', () => {
 
 				conversion.for( 'downcast' ).add( downcastRemoveRow() );
 
-				conversion.for( 'downcast' ).add( downcastAttributeChange( { attribute: 'headingRows' } ) );
-				conversion.for( 'downcast' ).add( downcastAttributeChange( { attribute: 'headingColumns' } ) );
+				conversion.for( 'downcast' ).add( downcastTableHeadingRowsChange() );
+				conversion.for( 'downcast' ).add( downcastTableHeadingColumnsChange() );
 			} );
 	} );
 
@@ -751,105 +752,7 @@ describe( 'downcast converters', () => {
 		} );
 	} );
 
-	describe( 'downcastAttributeChange()', () => {
-		it( 'should work for adding heading rows', () => {
-			setModelData( model, modelTable( [
-				[ '11', '12' ],
-				[ '21', '22' ],
-				[ '31', '32' ]
-			] ) );
-
-			const table = root.getChild( 0 );
-
-			model.change( writer => {
-				writer.setAttribute( 'headingRows', 2, table );
-			} );
-
-			expect( formatTable( getViewData( viewDocument, { withoutSelection: true } ) ) ).to.equal( formattedViewTable( [
-				[ '11', '12' ],
-				[ '21', '22' ],
-				[ '31', '32' ]
-			], { headingRows: 2 } ) );
-		} );
-
-		it( 'should work for changing number of heading rows to a bigger number', () => {
-			setModelData( model, modelTable( [
-				[ '11', '12' ],
-				[ '21', '22' ],
-				[ '31', '32' ]
-			], { headingRows: 1 } ) );
-
-			const table = root.getChild( 0 );
-
-			model.change( writer => {
-				writer.setAttribute( 'headingRows', 2, table );
-			} );
-
-			expect( formatTable( getViewData( viewDocument, { withoutSelection: true } ) ) ).to.equal( formattedViewTable( [
-				[ '11', '12' ],
-				[ '21', '22' ],
-				[ '31', '32' ]
-			], { headingRows: 2 } ) );
-		} );
-
-		it( 'should work for changing number of heading rows to a smaller number', () => {
-			setModelData( model, modelTable( [
-				[ '11', '12' ],
-				[ '21', '22' ],
-				[ '31', '32' ],
-				[ '41', '42' ]
-			], { headingRows: 3 } ) );
-
-			const table = root.getChild( 0 );
-
-			model.change( writer => {
-				writer.setAttribute( 'headingRows', 2, table );
-			} );
-
-			expect( formatTable( getViewData( viewDocument, { withoutSelection: true } ) ) ).to.equal( formattedViewTable( [
-				[ '11', '12' ],
-				[ '21', '22' ],
-				[ '31', '32' ],
-				[ '41', '42' ]
-			], { headingRows: 2 } ) );
-		} );
-
-		it( 'should work for removing heading rows', () => {
-			setModelData( model, modelTable( [
-				[ '11', '12' ],
-				[ '21', '22' ]
-			], { headingRows: 2 } ) );
-
-			const table = root.getChild( 0 );
-
-			model.change( writer => {
-				writer.removeAttribute( 'headingRows', table );
-			} );
-
-			expect( getViewData( viewDocument, { withoutSelection: true } ) ).to.equal( viewTable( [
-				[ '11', '12' ],
-				[ '21', '22' ]
-			] ) );
-		} );
-
-		it( 'should work for making heading rows without tbody', () => {
-			setModelData( model, modelTable( [
-				[ '11', '12' ],
-				[ '21', '22' ]
-			] ) );
-
-			const table = root.getChild( 0 );
-
-			model.change( writer => {
-				writer.setAttribute( 'headingRows', 2, table );
-			} );
-
-			expect( getViewData( viewDocument, { withoutSelection: true } ) ).to.equal( viewTable( [
-				[ '11', '12' ],
-				[ '21', '22' ]
-			], { headingRows: 2 } ) );
-		} );
-
+	describe( 'downcastTableHeadingColumnsChange()', () => {
 		it( 'should work for adding heading columns', () => {
 			setModelData( model, modelTable( [
 				[ '11', '12' ],
@@ -977,6 +880,191 @@ describe( 'downcast converters', () => {
 			] ) );
 		} );
 
+		describe( 'asWidget', () => {
+			beforeEach( () => {
+				return VirtualTestEditor.create()
+					.then( newEditor => {
+						editor = newEditor;
+						model = editor.model;
+						doc = model.document;
+						root = doc.getRoot( 'main' );
+						viewDocument = editor.editing.view;
+
+						const conversion = editor.conversion;
+						const schema = model.schema;
+
+						schema.register( 'table', {
+							allowWhere: '$block',
+							allowAttributes: [ 'headingRows', 'headingColumns' ],
+							isBlock: true,
+							isObject: true
+						} );
+
+						schema.register( 'tableRow', {
+							allowIn: 'table',
+							allowAttributes: [],
+							isBlock: true,
+							isLimit: true
+						} );
+
+						schema.register( 'tableCell', {
+							allowIn: 'tableRow',
+							allowContentOf: '$block',
+							allowAttributes: [ 'colspan', 'rowspan' ],
+							isBlock: true,
+							isLimit: true
+						} );
+
+						conversion.for( 'downcast' ).add( downcastInsertTable( { asWidget: true } ) );
+						conversion.for( 'downcast' ).add( downcastInsertRow( { asWidget: true } ) );
+						conversion.for( 'downcast' ).add( downcastInsertCell( { asWidget: true } ) );
+
+						conversion.for( 'downcast' ).add( downcastTableHeadingColumnsChange( { asWidget: true } ) );
+						conversion.for( 'downcast' ).add( downcastTableHeadingRowsChange( { asWidget: true } ) );
+
+						conversion.attributeToAttribute( { model: 'colspan', view: 'colspan' } );
+						conversion.attributeToAttribute( { model: 'rowspan', view: 'rowspan' } );
+					} );
+			} );
+
+			it( 'should create renamed cell as a widget', () => {
+				setModelData( model,
+					'<table>' +
+					'<tableRow><tableCell>foo</tableCell></tableRow>' +
+					'</table>'
+				);
+
+				const table = root.getChild( 0 );
+
+				model.change( writer => {
+					writer.setAttribute( 'headingRows', 1, table );
+				} );
+
+				expect( getViewData( viewDocument, { withoutSelection: true } ) ).to.equal(
+					'<table class="ck-widget" contenteditable="false">' +
+					'<thead>' +
+					'<tr><th class="ck-editor__editable ck-editor__nested-editable" contenteditable="true">foo</th></tr>' +
+					'</thead>' +
+					'</table>'
+				);
+			} );
+		} );
+	} );
+
+	describe( 'downcastTableHeadingRowsChange()', () => {
+		it( 'should work for adding heading rows', () => {
+			setModelData( model, modelTable( [
+				[ '11', '12' ],
+				[ '21', '22' ],
+				[ '31', '32' ]
+			] ) );
+
+			const table = root.getChild( 0 );
+
+			model.change( writer => {
+				writer.setAttribute( 'headingRows', 2, table );
+			} );
+
+			expect( formatTable( getViewData( viewDocument, { withoutSelection: true } ) ) ).to.equal( formattedViewTable( [
+				[ '11', '12' ],
+				[ '21', '22' ],
+				[ '31', '32' ]
+			], { headingRows: 2 } ) );
+		} );
+
+		it( 'should work for changing number of heading rows to a bigger number', () => {
+			setModelData( model, modelTable( [
+				[ '11', '12' ],
+				[ '21', '22' ],
+				[ '31', '32' ]
+			], { headingRows: 1 } ) );
+
+			const table = root.getChild( 0 );
+
+			model.change( writer => {
+				writer.setAttribute( 'headingRows', 2, table );
+			} );
+
+			expect( formatTable( getViewData( viewDocument, { withoutSelection: true } ) ) ).to.equal( formattedViewTable( [
+				[ '11', '12' ],
+				[ '21', '22' ],
+				[ '31', '32' ]
+			], { headingRows: 2 } ) );
+		} );
+
+		it( 'should work for changing number of heading rows to a smaller number', () => {
+			setModelData( model, modelTable( [
+				[ '11', '12' ],
+				[ '21', '22' ],
+				[ '31', '32' ],
+				[ '41', '42' ]
+			], { headingRows: 3 } ) );
+
+			const table = root.getChild( 0 );
+
+			model.change( writer => {
+				writer.setAttribute( 'headingRows', 2, table );
+			} );
+
+			expect( formatTable( getViewData( viewDocument, { withoutSelection: true } ) ) ).to.equal( formattedViewTable( [
+				[ '11', '12' ],
+				[ '21', '22' ],
+				[ '31', '32' ],
+				[ '41', '42' ]
+			], { headingRows: 2 } ) );
+		} );
+
+		it( 'should work for removing heading rows', () => {
+			setModelData( model, modelTable( [
+				[ '11', '12' ],
+				[ '21', '22' ]
+			], { headingRows: 2 } ) );
+
+			const table = root.getChild( 0 );
+
+			model.change( writer => {
+				writer.removeAttribute( 'headingRows', table );
+			} );
+
+			expect( getViewData( viewDocument, { withoutSelection: true } ) ).to.equal( viewTable( [
+				[ '11', '12' ],
+				[ '21', '22' ]
+			] ) );
+		} );
+
+		it( 'should work for making heading rows without tbody', () => {
+			setModelData( model, modelTable( [
+				[ '11', '12' ],
+				[ '21', '22' ]
+			] ) );
+
+			const table = root.getChild( 0 );
+
+			model.change( writer => {
+				writer.setAttribute( 'headingRows', 2, table );
+			} );
+
+			expect( getViewData( viewDocument, { withoutSelection: true } ) ).to.equal( viewTable( [
+				[ '11', '12' ],
+				[ '21', '22' ]
+			], { headingRows: 2 } ) );
+		} );
+
+		it( 'should be possible to overwrite', () => {
+			editor.conversion.attributeToAttribute( { model: 'headingRows', view: 'headingRows', converterPriority: 'high' } );
+			setModelData( model, modelTable( [ [ '11' ] ] ) );
+
+			const table = root.getChild( 0 );
+
+			model.change( writer => {
+				writer.setAttribute( 'headingRows', 1, table );
+			} );
+
+			expect( getViewData( viewDocument, { withoutSelection: true } ) ).to.equal(
+				'<table headingRows="1"><tbody><tr><td>11</td></tr></tbody></table>'
+			);
+		} );
+
 		it( 'should work with adding table rows at the beginning of a table', () => {
 			setModelData( model, modelTable( [
 				[ '00', '01' ],
@@ -1041,8 +1129,8 @@ describe( 'downcast converters', () => {
 						conversion.for( 'downcast' ).add( downcastInsertRow( { asWidget: true } ) );
 						conversion.for( 'downcast' ).add( downcastInsertCell( { asWidget: true } ) );
 
-						conversion.for( 'downcast' ).add( downcastAttributeChange( { attribute: 'headingRows', asWidget: true } ) );
-						conversion.for( 'downcast' ).add( downcastAttributeChange( { attribute: 'headingColumns', asWidget: true } ) );
+						conversion.for( 'downcast' ).add( downcastTableHeadingColumnsChange( { asWidget: true } ) );
+						conversion.for( 'downcast' ).add( downcastTableHeadingRowsChange( { asWidget: true } ) );
 
 						conversion.attributeToAttribute( { model: 'colspan', view: 'colspan' } );
 						conversion.attributeToAttribute( { model: 'rowspan', view: 'rowspan' } );

+ 19 - 3
packages/ckeditor5-table/tests/tableutils.js

@@ -7,7 +7,14 @@ import ModelTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/modeltestedit
 import { setData, getData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model';
 import { upcastElementToElement } from '@ckeditor/ckeditor5-engine/src/conversion/upcast-converters';
 
-import { downcastInsertTable } from '../src/converters/downcast';
+import {
+	downcastInsertCell,
+	downcastInsertRow,
+	downcastInsertTable,
+	downcastRemoveRow,
+	downcastTableHeadingColumnsChange,
+	downcastTableHeadingRowsChange
+} from '../src/converters/downcast';
 import upcastTable from '../src/converters/upcasttable';
 import { formatTable, formattedModelTable, modelTable } from './_utils/utils';
 import TableUtils from '../src/tableutils';
@@ -55,15 +62,24 @@ describe( 'TableUtils', () => {
 			conversion.for( 'upcast' ).add( upcastTable() );
 			conversion.for( 'downcast' ).add( downcastInsertTable() );
 
-			// Table row upcast only since downcast conversion is done in `downcastTable()`.
-			conversion.for( 'upcast' ).add( upcastElementToElement( { model: 'tableRow', view: 'tr' } ) );
+			// Insert row conversion.
+			conversion.for( 'downcast' ).add( downcastInsertRow() );
+
+			// Remove row conversion.
+			conversion.for( 'downcast' ).add( downcastRemoveRow() );
 
 			// Table cell conversion.
+			conversion.for( 'downcast' ).add( downcastInsertCell() );
+
 			conversion.for( 'upcast' ).add( upcastElementToElement( { model: 'tableCell', view: 'td' } ) );
 			conversion.for( 'upcast' ).add( upcastElementToElement( { model: 'tableCell', view: 'th' } ) );
 
+			// Table attributes conversion.
 			conversion.attributeToAttribute( { model: 'colspan', view: 'colspan' } );
 			conversion.attributeToAttribute( { model: 'rowspan', view: 'rowspan' } );
+
+			conversion.for( 'downcast' ).add( downcastTableHeadingColumnsChange() );
+			conversion.for( 'downcast' ).add( downcastTableHeadingRowsChange() );
 		} );
 	} );