瀏覽代碼

Fixed removing empty rows and columns when multiple rows and columns should be removed.

Kuba Niegowski 5 年之前
父節點
當前提交
66ebfe46ff

+ 1 - 1
packages/ckeditor5-table/src/commands/mergecellcommand.js

@@ -109,7 +109,7 @@ export default class MergeCellCommand extends Command {
 			const table = findAncestor( 'table', removedTableCellRow );
 
 			// Remove empty rows and columns after merging.
-			removeEmptyRowsColumns( table, tableUtils, writer.batch );
+			removeEmptyRowsColumns( table, tableUtils );
 		} );
 	}
 

+ 1 - 1
packages/ckeditor5-table/src/commands/mergecellscommand.js

@@ -60,7 +60,7 @@ export default class MergeCellsCommand extends Command {
 			const table = findAncestor( 'table', firstTableCell );
 
 			// Remove rows and columns that become empty (have no anchored cells).
-			removeEmptyRowsColumns( table, tableUtils, writer.batch );
+			removeEmptyRowsColumns( table, tableUtils );
 
 			writer.setSelection( firstTableCell, 'in' );
 		} );

+ 2 - 4
packages/ckeditor5-table/src/tableclipboard.js

@@ -205,7 +205,7 @@ function prepareTableForPasting( selectedTableCells, pastedDimensions, writer, t
 		selection.lastRow += pastedDimensions.height - 1;
 		selection.lastColumn += pastedDimensions.width - 1;
 
-		expandTableSize( selectedTable, selection.lastRow + 1, selection.lastColumn + 1, writer, tableUtils );
+		expandTableSize( selectedTable, selection.lastRow + 1, selection.lastColumn + 1, tableUtils );
 	}
 
 	// In case of expanding selection we do not reset the selection so in this case we will always try to fix selection
@@ -320,13 +320,12 @@ function replaceSelectedCellsWithPasted( pastedTable, pastedDimensions, selected
 }
 
 // Expand table (in place) to expected size.
-function expandTableSize( table, expectedHeight, expectedWidth, writer, tableUtils ) {
+function expandTableSize( table, expectedHeight, expectedWidth, tableUtils ) {
 	const tableWidth = tableUtils.getColumns( table );
 	const tableHeight = tableUtils.getRows( table );
 
 	if ( expectedWidth > tableWidth ) {
 		tableUtils.insertColumns( table, {
-			batch: writer.batch,
 			at: tableWidth,
 			columns: expectedWidth - tableWidth
 		} );
@@ -334,7 +333,6 @@ function expandTableSize( table, expectedHeight, expectedWidth, writer, tableUti
 
 	if ( expectedHeight > tableHeight ) {
 		tableUtils.insertRows( table, {
-			batch: writer.batch,
 			at: tableHeight,
 			rows: expectedHeight - tableHeight
 		} );

+ 13 - 5
packages/ckeditor5-table/src/tableutils.js

@@ -336,11 +336,15 @@ export default class TableUtils extends Plugin {
 				updateNumericAttribute( 'rowspan', rowspan, cell, writer );
 			}
 
-			// 2d. Remove empty columns (without anchored cells) if there are any.
-			removeEmptyColumns( table, this );
-
-			// 2e. Adjust heading rows if removed rows were in a heading section.
+			// 2d. Adjust heading rows if removed rows were in a heading section.
 			updateHeadingRows( table, first, last, model );
+
+			// 2e. Remove empty columns (without anchored cells) if there are any.
+			if ( !removeEmptyColumns( table, this ) ) {
+				// If there wasn't any empty columns then we still need to check if this wasn't called
+				// because of cleaning empty rows and we only removed one of them.
+				removeEmptyRows( table, this );
+			}
 		} );
 	}
 
@@ -395,7 +399,11 @@ export default class TableUtils extends Plugin {
 			}
 
 			// Remove empty rows that could appear after removing columns.
-			removeEmptyRows( table, this, writer.batch );
+			if ( !removeEmptyRows( table, this ) ) {
+				// If there wasn't any empty rows then we still need to check if this wasn't called
+				// because of cleaning empty columns and we only removed one of them.
+				removeEmptyColumns( table, this );
+			}
 		} );
 	}
 

+ 21 - 15
packages/ckeditor5-table/src/utils/structure.js

@@ -342,13 +342,17 @@ export function removeEmptyColumns( table, tableUtils ) {
 		return cellsCount ? result : [ ...result, column ];
 	}, [] );
 
-	// @if CK_DEBUG_TABLE // emptyColumns.length > 0 && console.log( `Removing empty columns: ${ emptyColumns.join( ', ' ) }.` );
+	if ( emptyColumns.length > 0 ) {
+		// Remove only last empty column because it will recurrently trigger removing empty rows.
+		const emptyColumn = emptyColumns[ emptyColumns.length - 1 ];
 
-	emptyColumns.reverse().forEach( column => {
-		tableUtils.removeColumns( table, { at: column } );
-	} );
+		// @if CK_DEBUG_TABLE // console.log( `Removing empty column: ${ emptyColumn }.` );
+		tableUtils.removeColumns( table, { at: emptyColumn } );
 
-	return emptyColumns.length > 0;
+		return true;
+	}
+
+	return false;
 }
 
 /**
@@ -380,10 +384,9 @@ export function removeEmptyColumns( table, tableUtils ) {
  * @protected
  * @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.
  * @returns {Boolean} True if removed some rows.
  */
-export function removeEmptyRows( table, tableUtils, batch ) {
+export function removeEmptyRows( table, tableUtils ) {
 	const emptyRows = [];
 
 	for ( let rowIndex = 0; rowIndex < table.childCount; rowIndex++ ) {
@@ -394,13 +397,17 @@ export function removeEmptyRows( table, tableUtils, batch ) {
 		}
 	}
 
-	// @if CK_DEBUG_TABLE // emptyRows.length > 0 && console.log( `Removing empty rows: ${ emptyRows.join( ', ' ) }.` );
+	if ( emptyRows.length > 0 ) {
+		// Remove only last empty row because it will recurrently trigger removing empty columns.
+		const emptyRow = emptyRows[ emptyRows.length - 1 ];
 
-	emptyRows.reverse().forEach( row => {
-		tableUtils.removeRows( table, { at: row, batch } );
-	} );
+		// @if CK_DEBUG_TABLE // console.log( `Removing empty row: ${ emptyRow }.` );
+		tableUtils.removeRows( table, { at: emptyRow } );
+
+		return true;
+	}
 
-	return emptyRows.length > 0;
+	return false;
 }
 
 /**
@@ -428,14 +435,13 @@ export function removeEmptyRows( table, tableUtils, batch ) {
  * @protected
  * @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.
  */
-export function removeEmptyRowsColumns( table, tableUtils, batch ) {
+export function removeEmptyRowsColumns( table, tableUtils ) {
 	const removedColumns = removeEmptyColumns( table, tableUtils );
 
 	// If there was some columns removed then cleaning empty rows was already triggered.
 	if ( !removedColumns ) {
-		removeEmptyRows( table, tableUtils, batch );
+		removeEmptyRows( table, tableUtils );
 	}
 }
 

+ 47 - 0
packages/ckeditor5-table/tests/commands/mergecellscommand.js

@@ -706,6 +706,53 @@ describe( 'MergeCellsCommand', () => {
 					]
 				] ) );
 			} );
+
+			it( 'should remove all empty rows and columns', () => {
+				setData( model, modelTable( [
+					[ '00', '01', '02' ],
+					[ '10', '11', '12' ],
+					[ '20', '21', '22' ]
+				] ) );
+
+				tableSelection.setCellSelection(
+					root.getNodeByPath( [ 0, 0, 0 ] ),
+					root.getNodeByPath( [ 0, 2, 2 ] )
+				);
+
+				command.execute();
+
+				assertEqualMarkup( getData( model ), modelTable( [
+					[
+						'<paragraph>[00</paragraph><paragraph>01</paragraph><paragraph>02</paragraph>' +
+						'<paragraph>10</paragraph><paragraph>11</paragraph><paragraph>12</paragraph>' +
+						'<paragraph>20</paragraph><paragraph>21</paragraph><paragraph>22]</paragraph>'
+					]
+				] ) );
+			} );
+
+			it( 'should remove all empty rows and columns (asymmetrical case)', () => {
+				setData( model, modelTable( [
+					[ '00', '01', { contents: '02', rowspan: 3 } ],
+					[ '10', '11' ],
+					[ '20', '21' ]
+				] ) );
+
+				tableSelection.setCellSelection(
+					root.getNodeByPath( [ 0, 0, 0 ] ),
+					root.getNodeByPath( [ 0, 2, 1 ] )
+				);
+
+				command.execute();
+
+				assertEqualMarkup( getData( model ), modelTable( [
+					[
+						'<paragraph>[00</paragraph><paragraph>01</paragraph>' +
+						'<paragraph>10</paragraph><paragraph>11</paragraph>' +
+						'<paragraph>20</paragraph><paragraph>21]</paragraph>',
+						'02'
+					]
+				] ) );
+			} );
 		} );
 	} );