Browse Source

Added normalization of pasted table.

Kuba Niegowski 5 years ago
parent
commit
c0c0d74513

+ 24 - 12
packages/ckeditor5-table/src/converters/downcast.js

@@ -56,10 +56,8 @@ export function downcastInsertTable( options = {} ) {
 		for ( const tableSlot of tableWalker ) {
 			const { row, cell } = tableSlot;
 
-			const tableSection = getOrCreateTableSection( getSectionName( row, tableAttributes ), tableElement, conversionApi );
 			const tableRow = table.getChild( row );
-
-			const trElement = viewRows.get( row ) || createTr( tableRow, row, tableSection, conversionApi );
+			const trElement = viewRows.get( row ) || createTr( tableElement, tableRow, row, tableAttributes, conversionApi );
 			viewRows.set( row, trElement );
 
 			// Consume table cell - it will be always consumed as we convert whole table at once.
@@ -70,6 +68,16 @@ export function downcastInsertTable( options = {} ) {
 			createViewTableCellElement( tableSlot, tableAttributes, insertPosition, conversionApi, options );
 		}
 
+		// Insert empty TR elements if there are any rows without anchored cells. Since the model is always normalized
+		// this can happen only in the document fragment that only part of the table is down-casted.
+		for ( const tableRow of table.getChildren() ) {
+			const rowIndex = tableRow.index;
+
+			if ( !viewRows.has( rowIndex ) ) {
+				viewRows.set( rowIndex, createTr( tableElement, tableRow, rowIndex, tableAttributes, conversionApi ) );
+			}
+		}
+
 		const viewPosition = conversionApi.mapper.toViewPosition( data.range.start );
 
 		conversionApi.mapper.bindElements( table, asWidget ? tableWidget : figureElement );
@@ -110,9 +118,7 @@ export function downcastInsertRow() {
 		const viewRows = new Map();
 
 		for ( const tableSlot of tableWalker ) {
-			const tableSection = getOrCreateTableSection( getSectionName( row, tableAttributes ), tableElement, conversionApi );
-
-			const trElement = viewRows.get( row ) || createTr( tableRow, row, tableSection, conversionApi );
+			const trElement = viewRows.get( row ) || createTr( tableElement, tableRow, row, tableAttributes, conversionApi );
 			viewRows.set( row, trElement );
 
 			// Consume table cell - it will be always consumed as we convert whole row at once.
@@ -425,22 +431,28 @@ function createViewTableCellElement( tableSlot, tableAttributes, insertPosition,
 
 // Creates a `<tr>` view element.
 //
-// @param {module:engine/view/element~Element} tableRow
+// @param {module:engine/view/element~Element} tableElement
+// @param {module:engine/model/element~Element} tableRow
 // @param {Number} rowIndex
-// @param {module:engine/view/element~Element} tableSection
+// @param {{headingColumns, headingRows}} tableAttributes
 // @param {module:engine/conversion/downcastdispatcher~DowncastConversionApi} conversionApi
 // @returns {module:engine/view/element~Element}
-function createTr( tableRow, rowIndex, tableSection, conversionApi ) {
+function createTr( tableElement, tableRow, rowIndex, tableAttributes, conversionApi ) {
 	// Will always consume since we're converting <tableRow> element from a parent <table>.
 	conversionApi.consumable.consume( tableRow, 'insert' );
 
-	const trElement = conversionApi.writer.createContainerElement( 'tr' );
+	const trElement = tableRow.isEmpty ?
+		conversionApi.writer.createEmptyElement( 'tr' ) :
+		conversionApi.writer.createContainerElement( 'tr' );
+
 	conversionApi.mapper.bindElements( tableRow, trElement );
 
-	const headingRows = tableRow.parent.getAttribute( 'headingRows' ) || 0;
-	const offset = headingRows > 0 && rowIndex >= headingRows ? rowIndex - headingRows : rowIndex;
+	const headingRows = tableAttributes.headingRows;
+	const tableSection = getOrCreateTableSection( getSectionName( rowIndex, tableAttributes ), tableElement, conversionApi );
 
+	const offset = headingRows > 0 && rowIndex >= headingRows ? rowIndex - headingRows : rowIndex;
 	const position = conversionApi.writer.createPositionAt( tableSection, offset );
+
 	conversionApi.writer.insert( position, trElement );
 
 	return trElement;

+ 3 - 3
packages/ckeditor5-table/src/converters/upcasttable.js

@@ -91,16 +91,16 @@ export default function upcastTable() {
 }
 
 /**
- * Conversion helper that skips empty <tr> from upcasting.
+ * Conversion helper that skips empty <tr> from upcasting at the beginning of the table.
  *
- * Empty row is considered a table model error.
+ * Empty row is considered a table model error if there are no cells spanned over that row.
  *
  * @returns {Function} Conversion helper.
  */
 export function skipEmptyTableRow() {
 	return dispatcher => {
 		dispatcher.on( 'element:tr', ( evt, data ) => {
-			if ( data.viewItem.isEmpty ) {
+			if ( data.viewItem.isEmpty && data.modelCursor.index == 0 ) {
 				evt.stop();
 			}
 		}, { priority: 'high' } );

+ 10 - 76
packages/ckeditor5-table/src/tableclipboard.js

@@ -19,10 +19,11 @@ import { getColumnIndexes, getRowIndexes, getSelectionAffectedTableCells, isSele
 import {
 	cropTableToDimensions,
 	getHorizontallyOverlappingCells,
-	getVerticallyOverlappingCells,
+	getVerticallyOverlappingCells, removeEmptyRowsColumns,
 	splitHorizontally,
 	splitVertically,
-	trimTableCellIfNeeded
+	trimTableCellIfNeeded,
+	adjustLastRowIndex, adjustLastColumnIndex
 } from './utils/structure';
 
 /**
@@ -113,16 +114,18 @@ export default class TableClipboard extends Plugin {
 		const model = this.editor.model;
 		const tableUtils = this.editor.plugins.get( TableUtils );
 
-		const selectedTableCells = getSelectionAffectedTableCells( model.document.selection );
+		// We might need to crop table before inserting so reference might change.
+		let pastedTable = getTableIfOnlyTableInContent( content, model );
 
-		if ( !selectedTableCells.length ) {
+		if ( !pastedTable ) {
 			return;
 		}
 
-		// We might need to crop table before inserting so reference might change.
-		let pastedTable = getTableIfOnlyTableInContent( content, model );
+		const selectedTableCells = getSelectionAffectedTableCells( model.document.selection );
+
+		if ( !selectedTableCells.length ) {
+			removeEmptyRowsColumns( pastedTable, tableUtils );
 
-		if ( !pastedTable ) {
 			return;
 		}
 
@@ -513,72 +516,3 @@ function isAffectedBySelection( index, span, limit ) {
 
 	return isInsideSelection || overlapsSelectionFromOutside;
 }
-
-// Returns adjusted last row index if selection covers part of a row with empty slots (spanned by other cells).
-// The rowIndexes.last is equal to last row index but selection might be bigger.
-//
-// This happens *only* on rectangular selection so we analyze a case like this:
-//
-//    +---+---+---+---+
-//  0 | a | b | c | d |
-//    +   +   +---+---+
-//  1 |   | e | f | g |
-//    +   +---+   +---+
-//  2 |   | h |   | i | <- last row, each cell has rowspan = 2,
-//    +   +   +   +   +    so we need to return 3, not 2
-//  3 |   |   |   |   |
-//    +---+---+---+---+
-function adjustLastRowIndex( table, dimensions ) {
-	const lastRowMap = Array.from( new TableWalker( table, {
-		startColumn: dimensions.firstColumn,
-		endColumn: dimensions.lastColumn,
-		row: dimensions.lastRow
-	} ) );
-
-	const everyCellHasSingleRowspan = lastRowMap.every( ( { cellHeight } ) => cellHeight === 1 );
-
-	// It is a "flat" row, so the last row index is OK.
-	if ( everyCellHasSingleRowspan ) {
-		return dimensions.lastRow;
-	}
-
-	// Otherwise get any cell's rowspan and adjust the last row index.
-	const rowspanAdjustment = lastRowMap[ 0 ].cellHeight - 1;
-	return dimensions.lastRow + rowspanAdjustment;
-}
-
-// Returns adjusted last column index if selection covers part of a column with empty slots (spanned by other cells).
-// The columnIndexes.last is equal to last column index but selection might be bigger.
-//
-// This happens *only* on rectangular selection so we analyze a case like this:
-//
-//   0   1   2   3
-// +---+---+---+---+
-// | a             |
-// +---+---+---+---+
-// | b | c | d     |
-// +---+---+---+---+
-// | e     | f     |
-// +---+---+---+---+
-// | g | h         |
-// +---+---+---+---+
-//           ^
-//          last column, each cell has colspan = 2, so we need to return 3, not 2
-function adjustLastColumnIndex( table, dimensions ) {
-	const lastColumnMap = Array.from( new TableWalker( table, {
-		startRow: dimensions.firstRow,
-		endRow: dimensions.lastRow,
-		column: dimensions.lastColumn
-	} ) );
-
-	const everyCellHasSingleColspan = lastColumnMap.every( ( { cellWidth } ) => cellWidth === 1 );
-
-	// It is a "flat" column, so the last column index is OK.
-	if ( everyCellHasSingleColspan ) {
-		return dimensions.lastColumn;
-	}
-
-	// Otherwise get any cell's colspan and adjust the last column index.
-	const colspanAdjustment = lastColumnMap[ 0 ].cellWidth - 1;
-	return dimensions.lastColumn + colspanAdjustment;
-}

+ 26 - 8
packages/ckeditor5-table/src/tableselection.js

@@ -14,8 +14,8 @@ import TableWalker from './tablewalker';
 import TableUtils from './tableutils';
 
 import { findAncestor } from './utils/common';
-import { cropTableToDimensions } from './utils/structure';
-import { getColumnIndexes, getRowIndexes, getSelectedTableCells } from './utils/selection';
+import { cropTableToDimensions, adjustLastRowIndex, adjustLastColumnIndex } from './utils/structure';
+import { getColumnIndexes, getRowIndexes, getSelectedTableCells, isSelectionRectangular } from './utils/selection';
 
 import '../theme/tableselection.css';
 
@@ -90,17 +90,35 @@ export default class TableSelection extends Plugin {
 
 		return this.editor.model.change( writer => {
 			const documentFragment = writer.createDocumentFragment();
+			const tableUtils = this.editor.plugins.get( 'TableUtils' );
 
-			const { first: startColumn, last: endColumn } = getColumnIndexes( selectedCells );
-			const { first: startRow, last: endRow } = getRowIndexes( selectedCells );
+			const { first: firstColumn, last: lastColumn } = getColumnIndexes( selectedCells );
+			const { first: firstRow, last: lastRow } = getRowIndexes( selectedCells );
 
 			const sourceTable = findAncestor( 'table', selectedCells[ 0 ] );
 
+			let adjustedLastRow = lastRow;
+			let adjustedLastColumn = lastColumn;
+
+			// If the selection is rectangular there could be a case of all cells in the last row/column spanned over
+			// next row/column so the real lastRow/lastColumn should be updated.
+			if ( isSelectionRectangular( selectedCells, tableUtils ) ) {
+				const dimensions = {
+					firstColumn,
+					lastColumn,
+					firstRow,
+					lastRow
+				};
+
+				adjustedLastRow = adjustLastRowIndex( sourceTable, dimensions );
+				adjustedLastColumn = adjustLastColumnIndex( sourceTable, dimensions );
+			}
+
 			const cropDimensions = {
-				startRow,
-				startColumn,
-				endRow,
-				endColumn
+				startRow: firstRow,
+				startColumn: firstColumn,
+				endRow: adjustedLastRow,
+				endColumn: adjustedLastColumn
 			};
 
 			const table = cropTableToDimensions( sourceTable, cropDimensions, writer );

+ 91 - 2
packages/ckeditor5-table/src/utils/structure.js

@@ -328,7 +328,7 @@ function addHeadingsToCroppedTable( croppedTable, sourceTable, startRow, startCo
  * @protected
  * @param {module:engine/model/element~Element} table
  * @param {module:table/tableutils~TableUtils} tableUtils
- * @return {Boolean} True if removed some columns.
+ * @returns {Boolean} True if removed some columns.
  */
 export function removeEmptyColumns( table, tableUtils ) {
 	const width = tableUtils.getColumns( table );
@@ -381,7 +381,7 @@ export function removeEmptyColumns( table, tableUtils ) {
  * @param {module:engine/model/element~Element} table
  * @param {module:table/tableutils~TableUtils} tableUtils
  * @param {module:engine/model/batch~Batch|null} [batch] Batch that should be used for removing empty rows.
- * @return {Boolean} True if removed some rows.
+ * @returns {Boolean} True if removed some rows.
  */
 export function removeEmptyRows( table, tableUtils, batch ) {
 	const emptyRows = [];
@@ -438,3 +438,92 @@ export function removeEmptyRowsColumns( table, tableUtils, batch ) {
 		removeEmptyRows( table, tableUtils, batch );
 	}
 }
+
+/**
+ * Returns adjusted last row index if selection covers part of a row with empty slots (spanned by other cells).
+ * The `dimensions.lastRow` is equal to last row index but selection might be bigger.
+ *
+ * This happens *only* on rectangular selection so we analyze a case like this:
+ *
+ *        +---+---+---+---+
+ *      0 | a | b | c | d |
+ *        +   +   +---+---+
+ *      1 |   | e | f | g |
+ *        +   +---+   +---+
+ *      2 |   | h |   | i | <- last row, each cell has rowspan = 2,
+ *        +   +   +   +   +    so we need to return 3, not 2
+ *      3 |   |   |   |   |
+ *        +---+---+---+---+
+ *
+ * @param {module:engine/model/element~Element} table
+ * @param {Object} dimensions
+ * @param {Number} dimensions.firstRow
+ * @param {Number} dimensions.firstColumn
+ * @param {Number} dimensions.lastRow
+ * @param {Number} dimensions.lastColumn
+ * @returns {Number} Adjusted last row index.
+ */
+export function adjustLastRowIndex( table, dimensions ) {
+	const lastRowMap = Array.from( new TableWalker( table, {
+		startColumn: dimensions.firstColumn,
+		endColumn: dimensions.lastColumn,
+		row: dimensions.lastRow
+	} ) );
+
+	const everyCellHasSingleRowspan = lastRowMap.every( ( { cellHeight } ) => cellHeight === 1 );
+
+	// It is a "flat" row, so the last row index is OK.
+	if ( everyCellHasSingleRowspan ) {
+		return dimensions.lastRow;
+	}
+
+	// Otherwise get any cell's rowspan and adjust the last row index.
+	const rowspanAdjustment = lastRowMap[ 0 ].cellHeight - 1;
+	return dimensions.lastRow + rowspanAdjustment;
+}
+
+/**
+ * Returns adjusted last column index if selection covers part of a column with empty slots (spanned by other cells).
+ * The `dimensions.lastColumn` is equal to last column index but selection might be bigger.
+ *
+ * This happens *only* on rectangular selection so we analyze a case like this:
+ *
+ *       0   1   2   3
+ *     +---+---+---+---+
+ *     | a             |
+ *     +---+---+---+---+
+ *     | b | c | d     |
+ *     +---+---+---+---+
+ *     | e     | f     |
+ *     +---+---+---+---+
+ *     | g | h         |
+ *     +---+---+---+---+
+ *               ^
+ *              last column, each cell has colspan = 2, so we need to return 3, not 2
+ *
+ * @param {module:engine/model/element~Element} table
+ * @param {Object} dimensions
+ * @param {Number} dimensions.firstRow
+ * @param {Number} dimensions.firstColumn
+ * @param {Number} dimensions.lastRow
+ * @param {Number} dimensions.lastColumn
+ * @returns {Number} Adjusted last column index.
+ */
+export function adjustLastColumnIndex( table, dimensions ) {
+	const lastColumnMap = Array.from( new TableWalker( table, {
+		startRow: dimensions.firstRow,
+		endRow: dimensions.lastRow,
+		column: dimensions.lastColumn
+	} ) );
+
+	const everyCellHasSingleColspan = lastColumnMap.every( ( { cellWidth } ) => cellWidth === 1 );
+
+	// It is a "flat" column, so the last column index is OK.
+	if ( everyCellHasSingleColspan ) {
+		return dimensions.lastColumn;
+	}
+
+	// Otherwise get any cell's colspan and adjust the last column index.
+	const colspanAdjustment = lastColumnMap[ 0 ].cellWidth - 1;
+	return dimensions.lastColumn + colspanAdjustment;
+}

+ 26 - 0
packages/ckeditor5-table/tests/tableclipboard-copy-cut.js

@@ -191,6 +191,32 @@ describe( 'table clipboard', () => {
 			] ) );
 		} );
 
+		it( 'should output empty tr elements if needed by row-spanned cells', () => {
+			// +----+----+----+
+			// | 00 | 01 | 02 |
+			// +----+----+----+
+			// | 10 | 11 | 12 |
+			// +    +    +----+
+			// |    |    | 22 |
+			// +----+----+----+
+			setModelData( model, modelTable( [
+				[ '00', '01', '02' ],
+				[ { contents: '10', rowspan: 2 }, { contents: '11', rowspan: 2 }, '12' ],
+				[ '22' ]
+			] ) );
+
+			tableSelection.setCellSelection(
+				modelRoot.getNodeByPath( [ 0, 0, 0 ] ),
+				modelRoot.getNodeByPath( [ 0, 1, 1 ] )
+			);
+
+			assertClipboardContentOnMethod( 'copy', viewTable( [
+				[ '00', '01' ],
+				[ { contents: '10', rowspan: 2 }, { contents: '11', rowspan: 2 } ],
+				[] // The empty tr
+			] ) );
+		} );
+
 		it( 'should fix selected table to a selection rectangle (hardcore case)', () => {
 			// This test check how previous simple rules run together (mixed prepending and trimming).
 			// In the example below a selection is set from cell "21" to "77"

+ 19 - 8
packages/ckeditor5-table/tests/tableclipboard-paste.js

@@ -124,28 +124,39 @@ describe( 'table clipboard', () => {
 			] ) );
 		} );
 
-		it( 'should not alter model.insertContent if selection is outside table', () => {
+		it( 'should normalize pasted table if selection is outside table', () => {
 			model.change( writer => {
 				writer.insertElement( 'paragraph', modelRoot.getChild( 0 ), 'before' );
 				writer.setSelection( modelRoot.getChild( 0 ), 'before' );
 			} );
 
+			const table = viewTable( [
+				[ 'aa', 'ab', { contents: 'ac', rowspan: 3 } ],
+				[ { contents: 'ba', rowspan: 2 }, { contents: 'bb', rowspan: 2 } ]
+			] );
+
 			const data = {
 				dataTransfer: createDataTransfer(),
 				preventDefault: sinon.spy(),
 				stopPropagation: sinon.spy()
 			};
-			data.dataTransfer.setData( 'text/html', '<p>foo</p>' );
+			data.dataTransfer.setData( 'text/html', table );
 			viewDocument.fire( 'paste', data );
 
 			editor.isReadOnly = false;
 
-			assertEqualMarkup( getModelData( model ), '<paragraph>foo[]</paragraph>' + modelTable( [
-				[ '00', '01', '02', '03' ],
-				[ '10', '11', '12', '13' ],
-				[ '20', '21', '22', '23' ],
-				[ '30', '31', '32', '33' ]
-			] ) );
+			assertEqualMarkup( getModelData( model ),
+				'[' + modelTable( [
+					[ 'aa', 'ab', { contents: 'ac', rowspan: 2 } ],
+					[ 'ba', 'bb' ]
+				] ) + ']' +
+				modelTable( [
+					[ '00', '01', '02', '03' ],
+					[ '10', '11', '12', '13' ],
+					[ '20', '21', '22', '23' ],
+					[ '30', '31', '32', '33' ]
+				] )
+			);
 		} );
 
 		it( 'should not alter model.insertContent if no table pasted', () => {

+ 93 - 0
packages/ckeditor5-table/tests/tableselection.js

@@ -236,6 +236,99 @@ describe( 'TableSelection', () => {
 				[ '21', '22' ]
 			] ) );
 		} );
+
+		it( 'should adjust the selection dimensions if it\'s rectangular but last row has only row-spanned cells', () => {
+			// +----+----+----+
+			// | 00 | 01 | 02 |
+			// +----+----+----+
+			// | 10 | 11 | 12 |
+			// +    +    +----+
+			// |    |    | 22 |
+			// +----+----+----+
+			setModelData( model, modelTable( [
+				[ '00', '01', '02' ],
+				[ { contents: '10', rowspan: 2 }, { contents: '11', rowspan: 2 }, '12' ],
+				[ '22' ]
+			] ) );
+
+			tableSelection.setCellSelection(
+				modelRoot.getNodeByPath( [ 0, 0, 0 ] ),
+				modelRoot.getNodeByPath( [ 0, 1, 1 ] )
+			);
+
+			// +----+----+
+			// | 00 | 01 |
+			// +----+----+
+			// | 10 | 11 |
+			// +    +    +
+			// |    |    |
+			// +----+----+
+			expect( stringifyModel( tableSelection.getSelectionAsFragment() ) ).to.equal( modelTable( [
+				[ '00', '01' ],
+				[ { contents: '10', rowspan: 2 }, { contents: '11', rowspan: 2 } ],
+				[] // This is an empty row that should be here to properly handle pasting of this table fragment.
+			] ) );
+		} );
+
+		it( 'should adjust the selection dimensions if it\'s rectangular but last column has only col-spanned cells', () => {
+			// +----+----+----+
+			// | 00 | 01      |
+			// +----+----+----+
+			// | 10 | 11      |
+			// +----+----+----+
+			// | 20 | 21 | 22 |
+			// +----+----+----+
+			setModelData( model, modelTable( [
+				[ '00', { contents: '01', colspan: 2 } ],
+				[ '10', { contents: '11', colspan: 2 } ],
+				[ '20', '21', '22' ]
+			] ) );
+
+			tableSelection.setCellSelection(
+				modelRoot.getNodeByPath( [ 0, 0, 0 ] ),
+				modelRoot.getNodeByPath( [ 0, 1, 1 ] )
+			);
+
+			// +----+----+----+
+			// | 00 | 01      |
+			// +----+----+----+
+			// | 10 | 11      |
+			// +----+----+----+
+			expect( stringifyModel( tableSelection.getSelectionAsFragment() ) ).to.equal( modelTable( [
+				[ '00', { contents: '01', colspan: 2 } ],
+				[ '10', { contents: '11', colspan: 2 } ]
+			] ) );
+		} );
+
+		it( 'should crop table fragment to rectangular selection', () => {
+			// +----+----+----+
+			// | 00 | 01      |
+			// +----+----+----+
+			// | 10 | 11 | 12 |
+			// +    +----+----+
+			// |    | 21 | 22 |
+			// +----+----+----+
+			setModelData( model, modelTable( [
+				[ '00', { contents: '01', colspan: 2 } ],
+				[ { contents: '10', rowspan: 2 }, '11', '12' ],
+				[ '21', '22' ]
+			] ) );
+
+			tableSelection.setCellSelection(
+				modelRoot.getNodeByPath( [ 0, 0, 0 ] ),
+				modelRoot.getNodeByPath( [ 0, 1, 1 ] )
+			);
+
+			// +----+----+
+			// | 00 | 01 |
+			// +----+----+
+			// | 10 | 11 |
+			// +----+----+
+			expect( stringifyModel( tableSelection.getSelectionAsFragment() ) ).to.equal( modelTable( [
+				[ '00', '01' ],
+				[ '10', '11' ]
+			] ) );
+		} );
 	} );
 
 	describe( 'delete content', () => {