Sfoglia il codice sorgente

Updating heading rows in one enqueued change block (to use the current table state for update).

Kuba Niegowski 5 anni fa
parent
commit
c17528862b

+ 3 - 5
packages/ckeditor5-table/src/commands/mergecellcommand.js

@@ -85,10 +85,7 @@ export default class MergeCellCommand extends Command {
 		const cellToMerge = this.value;
 		const direction = this.direction;
 
-		// Use single batch to modify table in steps but in one undo step.
-		const batch = model.createBatch();
-
-		model.enqueueChange( batch, writer => {
+		model.change( writer => {
 			const isMergeNext = direction == 'right' || direction == 'down';
 
 			// The merge mechanism is always the same so sort cells to be merged.
@@ -111,7 +108,8 @@ export default class MergeCellCommand extends Command {
 			// Remove empty row after merging.
 			if ( !removedTableCellRow.childCount ) {
 				const tableUtils = this.editor.plugins.get( 'TableUtils' );
-				tableUtils.removeRows( table, { at: table.getChildIndex( removedTableCellRow ), batch } );
+
+				tableUtils.removeRows( table, { at: table.getChildIndex( removedTableCellRow ), batch: writer.batch } );
 			}
 		} );
 	}

+ 2 - 5
packages/ckeditor5-table/src/commands/mergecellscommand.js

@@ -40,10 +40,7 @@ export default class MergeCellsCommand extends Command {
 		const model = this.editor.model;
 		const tableUtils = this.editor.plugins.get( TableUtils );
 
-		// Use single batch to modify table in steps but in one undo step.
-		const batch = model.createBatch();
-
-		model.enqueueChange( batch, writer => {
+		model.change( writer => {
 			const selectedTableCells = getSelectedTableCells( model.document.selection );
 
 			// All cells will be merged into the first one.
@@ -72,7 +69,7 @@ export default class MergeCellsCommand extends Command {
 				}
 			}
 
-			emptyRows.reverse().forEach( row => tableUtils.removeRows( table, { at: row, batch } ) );
+			emptyRows.reverse().forEach( row => tableUtils.removeRows( table, { at: row, batch: writer.batch } ) );
 
 			writer.setSelection( firstTableCell, 'in' );
 		} );

+ 21 - 15
packages/ckeditor5-table/src/tableutils.js

@@ -289,17 +289,21 @@ export default class TableUtils extends Plugin {
 		const last = first + rowsToRemove - 1;
 		const batch = options.batch || 'default';
 
-		// Removing rows from table requires most calculations to be done prior to changing table structure.
+		model.enqueueChange( batch, writer => {
+			// Removing rows from table requires most calculations to be done prior to changing table structure.
+			// Preparations must be done in enqueueChange callback to use the current table structure.
 
-		// 1. Preparation - get row-spanned cells that have to be modified after removing rows.
-		const { cellsToMove, cellsToTrim } = getCellsToMoveAndTrimOnRemoveRow( table, first, last );
+			// 1. Preparation - get row-spanned cells that have to be modified after removing rows.
+			const { cellsToMove, cellsToTrim } = getCellsToMoveAndTrimOnRemoveRow( table, first, last );
+
+			// 2. Execution
 
-		// 2. Execution
-		model.enqueueChange( batch, writer => {
 			// 2a. Move cells from removed rows that extends over a removed section - must be done before removing rows.
 			// This will fill any gaps in a rows below that previously were empty because of row-spanned cells.
-			const rowAfterRemovedSection = last + 1;
-			moveCellsToRow( table, rowAfterRemovedSection, cellsToMove, writer );
+			if ( cellsToMove.size ) {
+				const rowAfterRemovedSection = last + 1;
+				moveCellsToRow( table, rowAfterRemovedSection, cellsToMove, writer );
+			}
 
 			// 2b. Remove all required rows.
 			for ( let i = last; i >= first; i-- ) {
@@ -753,17 +757,19 @@ function adjustHeadingColumns( table, removedColumnIndexes, writer ) {
 
 // Calculates a new heading rows value for removing rows from heading section.
 function updateHeadingRows( table, first, last, model, batch ) {
-	const headingRows = table.getAttribute( 'headingRows' ) || 0;
+	// Must be done after the changes in table structure (removing rows).
+	// Otherwise the downcast converter for headingRows attribute will fail. ckeditor/ckeditor5#6391.
+	//
+	// Must be completely wrapped in enqueueChange to get the current table state (after applying other enqueued changes).
+	model.enqueueChange( batch, writer => {
+		const headingRows = table.getAttribute( 'headingRows' ) || 0;
 
-	if ( first < headingRows ) {
-		const newRows = last < headingRows ? headingRows - ( last - first + 1 ) : first;
+		if ( first < headingRows ) {
+			const newRows = last < headingRows ? headingRows - ( last - first + 1 ) : first;
 
-		// Must be done after the changes in table structure (removing rows).
-		// Otherwise the downcast converter for headingRows attribute will fail. ckeditor/ckeditor5#6391.
-		model.enqueueChange( batch, writer => {
 			updateNumericAttribute( 'headingRows', newRows, table, writer, 0 );
-		} );
-	}
+		}
+	} );
 }
 
 // Finds cells that will be: