diff --git a/packages/ckeditor5-table/package.json b/packages/ckeditor5-table/package.json
index 1da63085388..1b060882ec1 100644
--- a/packages/ckeditor5-table/package.json
+++ b/packages/ckeditor5-table/package.json
@@ -30,7 +30,8 @@
"@ckeditor/ckeditor5-typing": "^19.0.0",
"@ckeditor/ckeditor5-undo": "^19.0.0",
"@ckeditor/ckeditor5-utils": "^19.0.0",
- "json-diff": "^0.5.4"
+ "json-diff": "^0.5.4",
+ "lodash-es": "^4.17.10"
},
"engines": {
"node": ">=8.0.0",
diff --git a/packages/ckeditor5-table/src/commands/mergecellcommand.js b/packages/ckeditor5-table/src/commands/mergecellcommand.js
index 31f518dd7fb..f96651c8b3c 100644
--- a/packages/ckeditor5-table/src/commands/mergecellcommand.js
+++ b/packages/ckeditor5-table/src/commands/mergecellcommand.js
@@ -9,10 +9,7 @@
import Command from '@ckeditor/ckeditor5-core/src/command';
import TableWalker from '../tablewalker';
-import {
- updateNumericAttribute,
- isHeadingColumnCell
-} from './utils';
+import { isHeadingColumnCell, findAncestor } from './utils';
import { getTableCellsContainingSelection } from '../utils';
/**
@@ -83,6 +80,7 @@ export default class MergeCellCommand extends Command {
const model = this.editor.model;
const doc = model.document;
const tableCell = getTableCellsContainingSelection( doc.selection )[ 0 ];
+
const cellToMerge = this.value;
const direction = this.direction;
@@ -108,7 +106,10 @@ export default class MergeCellCommand extends Command {
// Remove empty row after merging.
if ( !removedTableCellRow.childCount ) {
- removeEmptyRow( removedTableCellRow, writer );
+ const tableUtils = this.editor.plugins.get( 'TableUtils' );
+ const table = findAncestor( 'table', removedTableCellRow );
+
+ tableUtils.removeRows( table, { at: removedTableCellRow.index, batch: writer.batch } );
}
} );
}
@@ -243,26 +244,6 @@ function getVerticalCell( tableCell, direction ) {
return cellToMergeData && cellToMergeData.cell;
}
-// Properly removes an empty row from a table. It will update the `rowspan` attribute of cells that overlap the removed row.
-//
-// @param {module:engine/model/element~Element} removedTableCellRow
-// @param {module:engine/model/writer~Writer} writer
-function removeEmptyRow( removedTableCellRow, writer ) {
- const table = removedTableCellRow.parent;
-
- const removedRowIndex = table.getChildIndex( removedTableCellRow );
-
- for ( const { cell, row, rowspan } of new TableWalker( table, { endRow: removedRowIndex } ) ) {
- const overlapsRemovedRow = row + rowspan - 1 >= removedRowIndex;
-
- if ( overlapsRemovedRow ) {
- updateNumericAttribute( 'rowspan', rowspan - 1, cell, writer );
- }
- }
-
- writer.remove( removedTableCellRow );
-}
-
// Merges two table cells. It will ensure that after merging cells with an empty paragraph, the resulting table cell will only have one
// paragraph. If one of the merged table cells is empty, the merged table cell will have the contents of the non-empty table cell.
// If both are empty, the merged table cell will have only one empty paragraph.
diff --git a/packages/ckeditor5-table/src/commands/mergecellscommand.js b/packages/ckeditor5-table/src/commands/mergecellscommand.js
index c42982acff3..2c3ee36d498 100644
--- a/packages/ckeditor5-table/src/commands/mergecellscommand.js
+++ b/packages/ckeditor5-table/src/commands/mergecellscommand.js
@@ -8,7 +8,6 @@
*/
import Command from '@ckeditor/ckeditor5-core/src/command';
-import TableWalker from '../tablewalker';
import { findAncestor, updateNumericAttribute } from './utils';
import TableUtils from '../tableutils';
import { getColumnIndexes, getRowIndexes, getSelectedTableCells } from '../utils';
@@ -57,38 +56,27 @@ export default class MergeCellsCommand extends Command {
updateNumericAttribute( 'colspan', mergeWidth, firstTableCell, writer );
updateNumericAttribute( 'rowspan', mergeHeight, firstTableCell, writer );
+ const emptyRowsIndexes = [];
+
for ( const tableCell of selectedTableCells ) {
const tableRow = tableCell.parent;
- mergeTableCells( tableCell, firstTableCell, writer );
- removeRowIfEmpty( tableRow, writer );
- }
- writer.setSelection( firstTableCell, 'in' );
- } );
- }
-}
+ mergeTableCells( tableCell, firstTableCell, writer );
-// Properly removes an empty row from a table. Updates the `rowspan` attribute of cells that overlap the removed row.
-//
-// @param {module:engine/model/element~Element} row
-// @param {module:engine/model/writer~Writer} writer
-function removeRowIfEmpty( row, writer ) {
- if ( row.childCount ) {
- return;
- }
+ if ( !tableRow.childCount ) {
+ emptyRowsIndexes.push( tableRow.index );
+ }
+ }
- const table = row.parent;
- const removedRowIndex = table.getChildIndex( row );
+ if ( emptyRowsIndexes.length ) {
+ const table = findAncestor( 'table', firstTableCell );
- for ( const { cell, row, rowspan } of new TableWalker( table, { endRow: removedRowIndex } ) ) {
- const overlapsRemovedRow = row + rowspan - 1 >= removedRowIndex;
+ emptyRowsIndexes.reverse().forEach( row => tableUtils.removeRows( table, { at: row, batch: writer.batch } ) );
+ }
- if ( overlapsRemovedRow ) {
- updateNumericAttribute( 'rowspan', rowspan - 1, cell, writer );
- }
+ writer.setSelection( firstTableCell, 'in' );
+ } );
}
-
- writer.remove( row );
}
// Merges two table cells. It will ensure that after merging cells with empty paragraphs the resulting table cell will only have one
diff --git a/packages/ckeditor5-table/src/tableutils.js b/packages/ckeditor5-table/src/tableutils.js
index 91f50ee76f4..2799b099ca1 100644
--- a/packages/ckeditor5-table/src/tableutils.js
+++ b/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 the table require that most calculations to be done prior to changing table structure.
+ // Preparations must be done in the same 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,20 @@ 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.
+ // See https://github.com/ckeditor/ckeditor5/issues/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:
diff --git a/packages/ckeditor5-table/tests/_utils/utils.js b/packages/ckeditor5-table/tests/_utils/utils.js
index 56ab29e6947..c7a33cdd493 100644
--- a/packages/ckeditor5-table/tests/_utils/utils.js
+++ b/packages/ckeditor5-table/tests/_utils/utils.js
@@ -507,6 +507,9 @@ export function createTableAsciiArt( model, table ) {
const { row: lastRow, column: lastColumn } = tableMap[ tableMap.length - 1 ];
const columns = lastColumn + 1;
+ const headingRows = parseInt( table.getAttribute( 'headingRows' ) ) || 0;
+ const headingColumns = parseInt( table.getAttribute( 'headingColumns' ) ) || 0;
+
let result = '';
for ( let row = 0; row <= lastRow; row++ ) {
@@ -539,6 +542,10 @@ export function createTableAsciiArt( model, table ) {
if ( column == lastColumn ) {
gridLine += '+';
contentLine += '|';
+
+ if ( headingRows && row == headingRows ) {
+ gridLine += ' <-- heading rows';
+ }
}
}
result += gridLine + '\n';
@@ -546,6 +553,14 @@ export function createTableAsciiArt( model, table ) {
if ( row == lastRow ) {
result += `+${ '----+'.repeat( columns ) }`;
+
+ if ( headingRows && row == headingRows - 1 ) {
+ result += ' <-- heading rows';
+ }
+
+ if ( headingColumns > 0 ) {
+ result += `\n${ ' '.repeat( headingColumns ) }^-- heading columns`;
+ }
}
}
diff --git a/packages/ckeditor5-table/tests/commands/mergecellcommand.js b/packages/ckeditor5-table/tests/commands/mergecellcommand.js
index 2399d6e0fe3..77bc8c66d47 100644
--- a/packages/ckeditor5-table/tests/commands/mergecellcommand.js
+++ b/packages/ckeditor5-table/tests/commands/mergecellcommand.js
@@ -701,6 +701,53 @@ describe( 'MergeCellCommand', () => {
[ '40', '41' ]
] ) );
} );
+
+ it( 'should adjust heading rows if empty row was removed ', () => {
+ // +----+----+
+ // | 00 | 01 |
+ // + +----+
+ // | | 11 |
+ // +----+----+ <-- heading rows
+ // | 20 | 21 |
+ // +----+----+
+ setData( model, modelTable( [
+ [ { contents: '00', rowspan: 2 }, '[]01' ],
+ [ '11' ],
+ [ '20', '21' ]
+ ], { headingRows: 2 } ) );
+
+ command.execute();
+
+ assertEqualMarkup( getData( model ), modelTable( [
+ [ '00', '[0111]' ],
+ [ '20', '21' ]
+ ], { headingRows: 1 } ) );
+ } );
+
+ it( 'should create one undo step (1 batch)', () => {
+ // +----+----+
+ // | 00 | 01 |
+ // + +----+
+ // | | 11 |
+ // +----+----+ <-- heading rows
+ // | 20 | 21 |
+ // +----+----+
+ setData( model, modelTable( [
+ [ { contents: '00', rowspan: 2 }, '[]01' ],
+ [ '11' ],
+ [ '20', '21' ]
+ ], { headingRows: 2 } ) );
+
+ const createdBatches = new Set();
+
+ model.on( 'applyOperation', ( evt, [ operation ] ) => {
+ createdBatches.add( operation.batch );
+ } );
+
+ command.execute();
+
+ expect( createdBatches.size ).to.equal( 1 );
+ } );
} );
} );
@@ -959,6 +1006,53 @@ describe( 'MergeCellCommand', () => {
[ '40', '41' ]
] ) );
} );
+
+ it( 'should adjust heading rows if empty row was removed ', () => {
+ // +----+----+
+ // | 00 | 01 |
+ // + +----+
+ // | | 11 |
+ // +----+----+ <-- heading rows
+ // | 20 | 21 |
+ // +----+----+
+ setData( model, modelTable( [
+ [ { contents: '00', rowspan: 2 }, '01' ],
+ [ '[]11' ],
+ [ '20', '21' ]
+ ], { headingRows: 2 } ) );
+
+ command.execute();
+
+ assertEqualMarkup( getData( model ), modelTable( [
+ [ '00', '[0111]' ],
+ [ '20', '21' ]
+ ], { headingRows: 1 } ) );
+ } );
+
+ it( 'should create one undo step (1 batch)', () => {
+ // +----+----+
+ // | 00 | 01 |
+ // + +----+
+ // | | 11 |
+ // +----+----+ <-- heading rows
+ // | 20 | 21 |
+ // +----+----+
+ setData( model, modelTable( [
+ [ { contents: '00', rowspan: 2 }, '01' ],
+ [ '[]11' ],
+ [ '20', '21' ]
+ ], { headingRows: 2 } ) );
+
+ const createdBatches = new Set();
+
+ model.on( 'applyOperation', ( evt, [ operation ] ) => {
+ createdBatches.add( operation.batch );
+ } );
+
+ command.execute();
+
+ expect( createdBatches.size ).to.equal( 1 );
+ } );
} );
} );
} );
diff --git a/packages/ckeditor5-table/tests/commands/mergecellscommand.js b/packages/ckeditor5-table/tests/commands/mergecellscommand.js
index f6c3951442c..da1523e36b4 100644
--- a/packages/ckeditor5-table/tests/commands/mergecellscommand.js
+++ b/packages/ckeditor5-table/tests/commands/mergecellscommand.js
@@ -514,6 +514,103 @@ describe( 'MergeCellsCommand', () => {
] ) );
} );
+ it( 'should decrease heading rows if some heading rows were removed', () => {
+ setData( model, modelTable( [
+ [ '00' ],
+ [ '10' ],
+ [ '20' ]
+ ], { headingRows: 2 } ) );
+
+ selectNodes( [
+ [ 0, 0, 0 ],
+ [ 0, 1, 0 ]
+ ] );
+
+ command.execute();
+
+ assertEqualMarkup( getData( model ), modelTable( [
+ [
+ '[0010]'
+ ],
+ [ '20' ]
+ ], { headingRows: 1 } ) );
+ } );
+
+ it( 'should decrease heading rows if multiple heading rows were removed', () => {
+ // +----+----+
+ // | 00 | 01 |
+ // + +----+
+ // | | 11 |
+ // +----+----+
+ // | 20 | 21 |
+ // +----+----+
+ // | 30 | 31 |
+ // + +----+
+ // | | 41 |
+ // +----+----+ <-- heading rows
+ // | 50 | 51 |
+ // +----+----+
+ setData( model, modelTable( [
+ [ { contents: '00', rowspan: 2 }, '01' ],
+ [ '11' ],
+ [ '20', '21' ],
+ [ { contents: '30', rowspan: 2 }, '31' ],
+ [ '41' ],
+ [ '50', '51' ]
+ ], { headingRows: 5 } ) );
+
+ selectNodes( [
+ [ 0, 0, 1 ],
+ [ 0, 1, 0 ],
+ [ 0, 2, 1 ],
+ [ 0, 3, 1 ],
+ [ 0, 4, 0 ]
+ ] );
+
+ command.execute();
+
+ const contents = [ '[01', '11', '21', '31', '41]' ].map( content => `${ content }` ).join( '' );
+
+ // +----+----+
+ // | 00 | 01 |
+ // +----+ +
+ // | 20 | |
+ // +----+ +
+ // | 30 | |
+ // +----+----+ <-- heading rows
+ // | 50 | 51 |
+ // +----+----+
+ assertEqualMarkup( getData( model ), modelTable( [
+ [ '00', { contents, rowspan: 3 } ],
+ [ '20' ],
+ [ '30' ],
+ [ '50', '51' ]
+ ], { headingRows: 3 } ) );
+ } );
+
+ it( 'should create one undo step (1 batch)', () => {
+ setData( model, modelTable( [
+ [ '00' ],
+ [ '10' ],
+ [ '20' ]
+ ], { headingRows: 2 } ) );
+
+ selectNodes( [
+ [ 0, 0, 0 ],
+ [ 0, 1, 0 ]
+ ] );
+
+ const createdBatches = new Set();
+
+ model.on( 'applyOperation', ( evt, [ operation ] ) => {
+ createdBatches.add( operation.batch );
+ } );
+
+ command.execute();
+
+ expect( createdBatches.size ).to.equal( 1 );
+ } );
+
it( 'should decrease rowspan if cell overlaps removed row', () => {
setData( model, modelTable( [
[ '00', { rowspan: 2, contents: '01' }, { rowspan: 3, contents: '02' } ],
diff --git a/packages/ckeditor5-table/tests/manual/tablemocking.js b/packages/ckeditor5-table/tests/manual/tablemocking.js
index 89b613e6b70..be5ad3e006c 100644
--- a/packages/ckeditor5-table/tests/manual/tablemocking.js
+++ b/packages/ckeditor5-table/tests/manual/tablemocking.js
@@ -11,6 +11,7 @@ import ClassicEditor from '@ckeditor/ckeditor5-editor-classic/src/classiceditor'
import { setData as setModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model';
import { diffString } from 'json-diff';
+import { debounce } from 'lodash-es';
import ArticlePluginSet from '@ckeditor/ckeditor5-core/tests/_utils/articlepluginset';
import TableWalker from '../../src/tablewalker';
@@ -67,7 +68,7 @@ ClassicEditor
updateAsciiAndDiff();
} );
- editor.model.document.on( 'change:data', updateAsciiAndDiff );
+ editor.model.document.on( 'change:data', debounce( () => updateAsciiAndDiff(), 100 ) );
updateAsciiAndDiff();
function updateAsciiAndDiff() {