Browse Source

Added the edge case test of removing multiple empty rows after cells merge.

Kuba Niegowski 5 years ago
parent
commit
5aa707d71e

+ 2 - 1
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",

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

@@ -80,7 +80,6 @@ export default class MergeCellCommand extends Command {
 		const model = this.editor.model;
 		const doc = model.document;
 		const tableCell = getTableCellsContainingSelection( doc.selection )[ 0 ];
-		const table = findAncestor( 'table', tableCell );
 
 		const cellToMerge = this.value;
 		const direction = this.direction;
@@ -108,6 +107,7 @@ export default class MergeCellCommand extends Command {
 			// Remove empty row after merging.
 			if ( !removedTableCellRow.childCount ) {
 				const tableUtils = this.editor.plugins.get( 'TableUtils' );
+				const table = findAncestor( 'table', removedTableCellRow );
 
 				tableUtils.removeRows( table, { at: removedTableCellRow.index, batch: writer.batch } );
 			}

+ 7 - 4
packages/ckeditor5-table/src/commands/mergecellscommand.js

@@ -45,7 +45,6 @@ export default class MergeCellsCommand extends Command {
 
 			// All cells will be merged into the first one.
 			const firstTableCell = selectedTableCells.shift();
-			const table = findAncestor( 'table', firstTableCell );
 
 			// Set the selection in cell that other cells are being merged to prevent model-selection-range-intersects error in undo.
 			// See https://github.com/ckeditor/ckeditor5/issues/6634.
@@ -57,7 +56,7 @@ export default class MergeCellsCommand extends Command {
 			updateNumericAttribute( 'colspan', mergeWidth, firstTableCell, writer );
 			updateNumericAttribute( 'rowspan', mergeHeight, firstTableCell, writer );
 
-			const emptyRows = [];
+			const emptyRowsIndexes = [];
 
 			for ( const tableCell of selectedTableCells ) {
 				const tableRow = tableCell.parent;
@@ -65,11 +64,15 @@ export default class MergeCellsCommand extends Command {
 				mergeTableCells( tableCell, firstTableCell, writer );
 
 				if ( !tableRow.childCount ) {
-					emptyRows.push( tableRow.index );
+					emptyRowsIndexes.push( tableRow.index );
 				}
 			}
 
-			emptyRows.reverse().forEach( row => tableUtils.removeRows( table, { at: row, batch: writer.batch } ) );
+			if ( emptyRowsIndexes.length ) {
+				const table = findAncestor( 'table', firstTableCell );
+
+				emptyRowsIndexes.reverse().forEach( row => tableUtils.removeRows( table, { at: row, batch: writer.batch } ) );
+			}
 
 			writer.setSelection( firstTableCell, 'in' );
 		} );

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

@@ -536,6 +536,58 @@ describe( 'MergeCellsCommand', () => {
 				], { 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 => `<paragraph>${ content }</paragraph>` ).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' ],

+ 2 - 1
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() {