Merge branch 'master' into i/6202-remove-env.isEdge

ckeditor · Apr 20, 2020 · 9b10433 · 9b10433
2 parents 9961e6c + 5e1fd28
commit 9b10433
Show file tree

Hide file tree

Showing 15 changed files with 1,212 additions and 408 deletions.
diff --git a/src/commands/removecolumncommand.js b/src/commands/removecolumncommand.js
@@ -10,7 +10,8 @@
 import Command from '@ckeditor/ckeditor5-core/src/command';
 
 import TableWalker from '../tablewalker';
-import { getSelectionAffectedTableCells } from '../utils';
+import { getColumnIndexes, getSelectionAffectedTableCells } from '../utils';
+import { findAncestor } from './utils';
 
 /**
  * The remove column command.
@@ -32,15 +33,12 @@ export default class RemoveColumnCommand extends Command {
 		const firstCell = selectedCells[ 0 ];
 
 		if ( firstCell ) {
-			const table = firstCell.parent.parent;
+			const table = findAncestor( 'table', firstCell );
 			const tableColumnCount = this.editor.plugins.get( 'TableUtils' ).getColumns( table );
 
-			const tableMap = [ ...new TableWalker( table ) ];
-			const columnIndexes = tableMap.filter( entry => selectedCells.includes( entry.cell ) ).map( el => el.column ).sort();
-			const minColumnIndex = columnIndexes[ 0 ];
-			const maxColumnIndex = columnIndexes[ columnIndexes.length - 1 ];
+			const { first, last } = getColumnIndexes( selectedCells );
 
-			this.isEnabled = maxColumnIndex - minColumnIndex < ( tableColumnCount - 1 );
+			this.isEnabled = last - first < ( tableColumnCount - 1 );
 		} else {
 			this.isEnabled = false;
 		}

diff --git a/src/commands/removerowcommand.js b/src/commands/removerowcommand.js
@@ -60,17 +60,23 @@ export default class RemoveRowCommand extends Command {
 
 		const columnIndexToFocus = this.editor.plugins.get( 'TableUtils' ).getCellLocation( firstCell ).column;
 
-		model.change( writer => {
+		// Use single batch to modify table in steps but in one undo step.
+		const batch = model.createBatch();
+
+		model.enqueueChange( batch, writer => {
 			// This prevents the "model-selection-range-intersects" error, caused by removing row selected cells.
 			writer.setSelection( writer.createSelection( table, 'on' ) );
 
 			const rowsToRemove = removedRowIndexes.last - removedRowIndexes.first + 1;
 
 			this.editor.plugins.get( 'TableUtils' ).removeRows( table, {
 				at: removedRowIndexes.first,
-				rows: rowsToRemove
+				rows: rowsToRemove,
+				batch
 			} );
+		} );
 
+		model.enqueueChange( batch, writer => {
 			const cellToFocus = getCellToFocus( table, removedRowIndexes.first, columnIndexToFocus );
 
 			writer.setSelection( writer.createPositionAt( cellToFocus, 0 ) );

diff --git a/src/commands/setheadercolumncommand.js b/src/commands/setheadercolumncommand.js
@@ -9,11 +9,8 @@
 
 import Command from '@ckeditor/ckeditor5-core/src/command';
 
-import {
-	updateNumericAttribute,
-	isHeadingColumnCell
-} from './utils';
-import { getSelectionAffectedTableCells } from '../utils';
+import { findAncestor, isHeadingColumnCell, updateNumericAttribute } from './utils';
+import { getColumnIndexes, getSelectionAffectedTableCells } from '../utils';
 
 /**
  * The header column command.
@@ -66,26 +63,19 @@ export default class SetHeaderColumnCommand extends Command {
 	 * the `forceValue` parameter instead of the current model state.
 	 */
 	execute( options = {} ) {
-		const model = this.editor.model;
-		const tableUtils = this.editor.plugins.get( 'TableUtils' );
-
-		const selectedCells = getSelectionAffectedTableCells( model.document.selection );
-		const firstCell = selectedCells[ 0 ];
-		const lastCell = selectedCells[ selectedCells.length - 1 ];
-		const tableRow = firstCell.parent;
-		const table = tableRow.parent;
-
-		const [ selectedColumnMin, selectedColumnMax ] =
-			// Returned cells might not necessary be in order, so make sure to sort it.
-			[ tableUtils.getCellLocation( firstCell ).column, tableUtils.getCellLocation( lastCell ).column ].sort();
-
 		if ( options.forceValue === this.value ) {
 			return;
 		}
 
-		const headingColumnsToSet = this.value ? selectedColumnMin : selectedColumnMax + 1;
+		const model = this.editor.model;
+		const selectedCells = getSelectionAffectedTableCells( model.document.selection );
+		const { first, last } = getColumnIndexes( selectedCells );
+
+		const headingColumnsToSet = this.value ? first : last + 1;
 
 		model.change( writer => {
+			const table = findAncestor( 'table', selectedCells[ 0 ] );
+
 			updateNumericAttribute( 'headingColumns', headingColumnsToSet, table, writer, 0 );
 		} );
 	}

diff --git a/src/commands/setheaderrowcommand.js b/src/commands/setheaderrowcommand.js
@@ -9,8 +9,8 @@
 
 import Command from '@ckeditor/ckeditor5-core/src/command';
 
-import { createEmptyTableCell, updateNumericAttribute } from './utils';
-import { getSelectionAffectedTableCells } from '../utils';
+import { createEmptyTableCell, findAncestor, updateNumericAttribute } from './utils';
+import { getRowIndexes, getSelectionAffectedTableCells } from '../utils';
 import TableWalker from '../tablewalker';
 
 /**
@@ -62,24 +62,16 @@ export default class SetHeaderRowCommand extends Command {
 	 * the `forceValue` parameter instead of the current model state.
 	 */
 	execute( options = {} ) {
-		const model = this.editor.model;
-
-		const selectedCells = getSelectionAffectedTableCells( model.document.selection );
-		const firstCell = selectedCells[ 0 ];
-		const lastCell = selectedCells[ selectedCells.length - 1 ];
-		const table = firstCell.parent.parent;
-
-		const currentHeadingRows = table.getAttribute( 'headingRows' ) || 0;
-
-		const [ selectedRowMin, selectedRowMax ] =
-			// Returned cells might not necessary be in order, so make sure to sort it.
-			[ firstCell.parent.index, lastCell.parent.index ].sort();
-
 		if ( options.forceValue === this.value ) {
 			return;
 		}
+		const model = this.editor.model;
+		const selectedCells = getSelectionAffectedTableCells( model.document.selection );
+		const table = findAncestor( 'table', selectedCells[ 0 ] );
 
-		const headingRowsToSet = this.value ? selectedRowMin : selectedRowMax + 1;
+		const { first, last } = getRowIndexes( selectedCells );
+		const headingRowsToSet = this.value ? first : last + 1;
+		const currentHeadingRows = table.getAttribute( 'headingRows' ) || 0;
 
 		model.change( writer => {
 			if ( headingRowsToSet ) {

diff --git a/src/converters/downcast.js b/src/converters/downcast.js
@@ -209,9 +209,6 @@ export function downcastTableHeadingRowsChange( options = {} ) {
 					renameViewTableCell( tableCell, 'th', conversionApi, asWidget );
 				}
 			}
-
-			// Cleanup: this will remove any empty section from the view which may happen when moving all rows from a table section.
-			removeTableSectionIfEmpty( 'tbody', viewTable, conversionApi );
 		}
 		// The head section has shrunk so move rows from <thead> to <tbody>.
 		else {
@@ -234,11 +231,12 @@ export function downcastTableHeadingRowsChange( options = {} ) {
 			for ( const tableWalkerValue of tableWalker ) {
 				renameViewTableCellIfRequired( tableWalkerValue, tableAttributes, conversionApi, asWidget );
 			}
-
-			// Cleanup: this will remove any empty section from the view which may happen when moving all rows from a table section.
-			removeTableSectionIfEmpty( 'thead', viewTable, conversionApi );
 		}
 
+		// Cleanup: Ensure that thead & tbody sections are removed if left empty after moving rows. See #6437, #6391.
+		removeTableSectionIfEmpty( 'thead', viewTable, conversionApi );
+		removeTableSectionIfEmpty( 'tbody', viewTable, conversionApi );
+
 		function isBetween( index, lower, upper ) {
 			return index > lower && index < upper;
 		}
@@ -298,6 +296,7 @@ export function downcastRemoveRow() {
 		const viewStart = mapper.toViewPosition( data.position ).getLastMatchingPosition( value => !value.item.is( 'tr' ) );
 		const viewItem = viewStart.nodeAfter;
 		const tableSection = viewItem.parent;
+		const viewTable = tableSection.parent;
 
 		// Remove associated <tr> from the view.
 		const removeRange = viewWriter.createRangeOn( viewItem );
@@ -307,11 +306,9 @@ export function downcastRemoveRow() {
 			mapper.unbindViewElement( child );
 		}
 
-		// Check if table section has any children left - if not remove it from the view.
-		if ( !tableSection.childCount ) {
-			// No need to unbind anything as table section is not represented in the model.
-			viewWriter.remove( viewWriter.createRangeOn( tableSection ) );
-		}
+		// Cleanup: Ensure that thead & tbody sections are removed if left empty after removing rows. See #6437, #6391.
+		removeTableSectionIfEmpty( 'thead', viewTable, conversionApi );
+		removeTableSectionIfEmpty( 'tbody', viewTable, conversionApi );
 	}, { priority: 'higher' } );
 }