Przeglądaj źródła

Internal: Removing table column also modifies heading columns accordingly if there's a multiple cells selected.

Marek Lewandowski 5 lat temu
rodzic
commit
f8d0001aad

+ 13 - 7
packages/ckeditor5-table/src/commands/removecolumncommand.js

@@ -46,14 +46,11 @@ export default class RemoveColumnCommand extends Command {
 		const tableRow = firstCell.parent;
 		const table = tableRow.parent;
 
-		const headingColumns = table.getAttribute( 'headingColumns' ) || 0;
-
 		// Cache the table before removing or updating colspans.
 		const tableMap = [ ...new TableWalker( table ) ];
 
 		// Get column index of removed column.
 		const firstCellData = tableMap.find( value => value.cell === firstCell );
-		const selectionRow = firstCellData.row;
 		const cellsToFocus = getCellToFocus( firstCell, lastCell );
 
 		const removedColumnIndexes = {
@@ -65,10 +62,7 @@ export default class RemoveColumnCommand extends Command {
 			// A temporary workaround to avoid the "model-selection-range-intersects" error.
 			writer.setSelection( writer.createSelection( table, 'on' ) );
 
-			// Update heading columns attribute if removing a row from head section.
-			if ( headingColumns && selectionRow <= headingColumns ) {
-				writer.setAttribute( 'headingColumns', headingColumns - 1, table );
-			}
+			adjustHeadingColumns( table, removedColumnIndexes, writer );
 
 			for (
 				let removedColumnIndex = removedColumnIndexes.last;
@@ -110,6 +104,18 @@ export default class RemoveColumnCommand extends Command {
 	}
 }
 
+// Updates heading columns attribute if removing a row from head section.
+function adjustHeadingColumns( table, removedColumnIndexes, writer ) {
+	const headingColumns = table.getAttribute( 'headingColumns' ) || 0;
+
+	if ( headingColumns && removedColumnIndexes.first <= headingColumns ) {
+		const headingsRemoved = Math.min( headingColumns - 1 /* Other numbers are 0-based */, removedColumnIndexes.last ) -
+			removedColumnIndexes.first + 1;
+
+		writer.setAttribute( 'headingColumns', headingColumns - headingsRemoved, table );
+	}
+}
+
 // Returns a proper table cell to focus after removing a column. It should be a next sibling to selection visually stay in place but:
 // - selection is on last table cell it will return previous cell.
 // - table cell is spanned over 2+ columns - it will be truncated so the selection should stay in that cell.

+ 20 - 0
packages/ckeditor5-table/tests/commands/removecolumncommand.js

@@ -247,6 +247,26 @@ describe( 'RemoveColumnCommand', () => {
 					[ '30' ]
 				] ) );
 			} );
+
+			it( 'should properly remove multiple heading columns', () => {
+				// There's no handling for selection in case like that.
+				setData( model, modelTable( [
+					[ '00', '01', '02', '03', '04' ],
+					[ '10', '11', '12', '13', '14' ]
+				], { headingColumns: 3 } ) );
+
+				const tableSelection = editor.plugins.get( TableSelection );
+				const modelRoot = model.document.getRoot();
+				tableSelection.startSelectingFrom( modelRoot.getNodeByPath( [ 0, 0, 1 ] ) );
+				tableSelection.setSelectingTo( modelRoot.getNodeByPath( [ 0, 1, 3 ] ) );
+
+				command.execute();
+
+				assertEqualMarkup( getData( model ), modelTable( [
+					[ '00', '04' ],
+					[ '10', '[]14' ]
+				], { headingColumns: 1 } ) );
+			} );
 		} );
 
 		it( 'should change heading columns if removing a heading column', () => {