瀏覽代碼

Refactor RemoveColumnCommand & move getColumns() to TableUtils plugin.

Maciej Gołaszewski 7 年之前
父節點
當前提交
7d0c065436

+ 23 - 23
packages/ckeditor5-table/src/commands/removecolumncommand.js

@@ -4,12 +4,13 @@
  */
 
 /**
- * @module table/commands/splitcell
+ * @module table/commands/removecolumncommand
  */
 
 import Command from '@ckeditor/ckeditor5-core/src/command';
+
 import TableWalker from '../tablewalker';
-import { getColumns } from './utils';
+import TableUtils from '../tableutils';
 
 /**
  * The split cell command.
@@ -21,12 +22,13 @@ export default class RemoveColumnCommand extends Command {
 	 * @inheritDoc
 	 */
 	refresh() {
-		const model = this.editor.model;
-		const doc = model.document;
+		const editor = this.editor;
+		const selection = editor.model.document.selection;
+		const tableUtils = editor.plugins.get( TableUtils );
 
-		const element = doc.selection.getFirstPosition().parent;
+		const selectedElement = selection.getFirstPosition().parent;
 
-		this.isEnabled = element.is( 'tableCell' ) && getColumns( element.parent.parent ) > 1;
+		this.isEnabled = selectedElement.is( 'tableCell' ) && tableUtils.getColumns( selectedElement.parent.parent ) > 1;
 	}
 
 	/**
@@ -34,44 +36,42 @@ export default class RemoveColumnCommand extends Command {
 	 */
 	execute() {
 		const model = this.editor.model;
-		const document = model.document;
-		const selection = document.selection;
+		const selection = model.document.selection;
 
 		const firstPosition = selection.getFirstPosition();
+
 		const tableCell = firstPosition.parent;
 		const tableRow = tableCell.parent;
-
 		const table = tableRow.parent;
 
-		const rowIndex = tableRow.index;
-
 		model.change( writer => {
-			const headingColumns = ( table.getAttribute( 'headingColumns' ) || 0 );
+			const headingColumns = parseInt( table.getAttribute( 'headingColumns' ) || 0 );
+			const rowIndex = table.getChildIndex( tableRow );
 
 			if ( headingColumns && rowIndex <= headingColumns ) {
 				writer.setAttribute( 'headingColumns', headingColumns - 1, table );
 			}
 
 			// Cache the table before removing or updating colspans.
-			const currentTableState = [ ...new TableWalker( table ) ];
+			const tableMap = [ ...new TableWalker( table ) ];
 
 			// Get column index of removed column.
-			const removedColumn = currentTableState.filter( value => value.cell === tableCell ).pop().column;
-
-			for ( const tableWalkerValue of currentTableState ) {
-				const column = tableWalkerValue.column;
-				const colspan = tableWalkerValue.colspan;
+			const cellData = tableMap.find( value => value.cell === tableCell );
+			const removedColumn = cellData.column;
 
+			for ( const { cell, column, colspan } of tableMap ) {
+				// If colspaned cell overlaps removed column decrease it's span.
 				if ( column <= removedColumn && colspan > 1 && column + colspan > removedColumn ) {
-					const colspanToSet = tableWalkerValue.colspan - 1;
+					const colspanToSet = colspan - 1;
 
 					if ( colspanToSet > 1 ) {
-						writer.setAttribute( 'colspan', colspanToSet, tableWalkerValue.cell );
+						writer.setAttribute( 'colspan', colspanToSet, cell );
 					} else {
-						writer.removeAttribute( 'colspan', tableWalkerValue.cell );
+						writer.removeAttribute( 'colspan', cell );
 					}
-				} else if ( column == removedColumn ) {
-					writer.remove( tableWalkerValue.cell );
+				} else if ( column === removedColumn ) {
+					// The cell in removed column has colspan of 1.
+					writer.remove( cell );
 				}
 			}
 		} );

+ 0 - 16
packages/ckeditor5-table/src/commands/utils.js

@@ -24,19 +24,3 @@ export function getParentTable( position ) {
 		parent = parent.parent;
 	}
 }
-
-/**
- * Returns number of columns for given table.
- *
- * @param {module:engine/model/element} table
- * @returns {Number}
- */
-export function getColumns( table ) {
-	const row = table.getChild( 0 );
-
-	return [ ...row.getChildren() ].reduce( ( columns, row ) => {
-		const columnWidth = parseInt( row.getAttribute( 'colspan' ) ) || 1;
-
-		return columns + ( columnWidth );
-	}, 0 );
-}

+ 23 - 6
packages/ckeditor5-table/src/tableutils.js

@@ -4,18 +4,19 @@
  */
 
 /**
- * @module table/commands/insertrowcommand
+ * @module table/tableutils
  */
 
 import Plugin from '@ckeditor/ckeditor5-core/src/plugin';
-import TableWalker from './tablewalker';
-import { getColumns, getParentTable } from './commands/utils';
 import Position from '@ckeditor/ckeditor5-engine/src/model/position';
 
+import TableWalker from './tablewalker';
+import { getParentTable } from './commands/utils';
+
 /**
  * The table utils plugin.
  *
- * @extends module:core/command~Command
+ * @extends module:core/plugin~Plugin
  */
 export default class TableUtils extends Plugin {
 	/**
@@ -66,7 +67,7 @@ export default class TableUtils extends Plugin {
 
 		const headingRows = table.getAttribute( 'headingRows' ) || 0;
 
-		const columns = getColumns( table );
+		const columns = this.getColumns( table );
 
 		model.change( writer => {
 			if ( headingRows > insertAt ) {
@@ -107,7 +108,7 @@ export default class TableUtils extends Plugin {
 		const insertAt = parseInt( options.at ) || 0;
 
 		model.change( writer => {
-			const tableColumns = getColumns( table );
+			const tableColumns = this.getColumns( table );
 
 			// Inserting at the end and at the begging of a table doesn't require to calculate anything special.
 			if ( insertAt === 0 || tableColumns <= insertAt ) {
@@ -226,6 +227,22 @@ export default class TableUtils extends Plugin {
 			createEmptyRows( writer, table, rowIndex + 1, cellNumber - 1, 1 );
 		} );
 	}
+
+	/**
+	 * Returns number of columns for given table.
+	 *
+	 * @param {module:engine/model/element} table
+	 * @returns {Number}
+	 */
+	getColumns( table ) {
+		const row = table.getChild( 0 );
+
+		return [ ...row.getChildren() ].reduce( ( columns, row ) => {
+			const columnWidth = parseInt( row.getAttribute( 'colspan' ) ) || 1;
+
+			return columns + ( columnWidth );
+		}, 0 );
+	}
 }
 
 // Creates empty rows at given index in an existing table.

+ 49 - 47
packages/ckeditor5-table/tests/commands/removecolumncommand.js

@@ -11,58 +11,60 @@ import RemoveColumnCommand from '../../src/commands/removecolumncommand';
 import { downcastInsertTable } from '../../src/converters/downcast';
 import upcastTable from '../../src/converters/upcasttable';
 import { formatTable, formattedModelTable, modelTable } from '../_utils/utils';
+import TableUtils from '../../src/tableutils';
 
 describe( 'RemoveColumnCommand', () => {
 	let editor, model, command;
 
 	beforeEach( () => {
-		return ModelTestEditor.create()
-			.then( newEditor => {
-				editor = newEditor;
-				model = editor.model;
-				command = new RemoveColumnCommand( editor );
-
-				const conversion = editor.conversion;
-				const schema = model.schema;
-
-				schema.register( 'table', {
-					allowWhere: '$block',
-					allowAttributes: [ 'headingRows', 'headingColumns' ],
-					isBlock: true,
-					isObject: true
-				} );
-
-				schema.register( 'tableRow', {
-					allowIn: 'table',
-					allowAttributes: [],
-					isBlock: true,
-					isLimit: true
-				} );
-
-				schema.register( 'tableCell', {
-					allowIn: 'tableRow',
-					allowContentOf: '$block',
-					allowAttributes: [ 'colspan', 'rowspan' ],
-					isBlock: true,
-					isLimit: true
-				} );
-
-				model.schema.register( 'p', { inheritAllFrom: '$block' } );
-
-				// Table conversion.
-				conversion.for( 'upcast' ).add( upcastTable() );
-				conversion.for( 'downcast' ).add( downcastInsertTable() );
-
-				// Table row upcast only since downcast conversion is done in `downcastTable()`.
-				conversion.for( 'upcast' ).add( upcastElementToElement( { model: 'tableRow', view: 'tr' } ) );
-
-				// Table cell conversion.
-				conversion.for( 'upcast' ).add( upcastElementToElement( { model: 'tableCell', view: 'td' } ) );
-				conversion.for( 'upcast' ).add( upcastElementToElement( { model: 'tableCell', view: 'th' } ) );
-
-				conversion.attributeToAttribute( { model: 'colspan', view: 'colspan' } );
-				conversion.attributeToAttribute( { model: 'rowspan', view: 'rowspan' } );
+		return ModelTestEditor.create( {
+			plugins: [ TableUtils ]
+		} ).then( newEditor => {
+			editor = newEditor;
+			model = editor.model;
+			command = new RemoveColumnCommand( editor );
+
+			const conversion = editor.conversion;
+			const schema = model.schema;
+
+			schema.register( 'table', {
+				allowWhere: '$block',
+				allowAttributes: [ 'headingRows', 'headingColumns' ],
+				isBlock: true,
+				isObject: true
 			} );
+
+			schema.register( 'tableRow', {
+				allowIn: 'table',
+				allowAttributes: [],
+				isBlock: true,
+				isLimit: true
+			} );
+
+			schema.register( 'tableCell', {
+				allowIn: 'tableRow',
+				allowContentOf: '$block',
+				allowAttributes: [ 'colspan', 'rowspan' ],
+				isBlock: true,
+				isLimit: true
+			} );
+
+			model.schema.register( 'p', { inheritAllFrom: '$block' } );
+
+			// Table conversion.
+			conversion.for( 'upcast' ).add( upcastTable() );
+			conversion.for( 'downcast' ).add( downcastInsertTable() );
+
+			// Table row upcast only since downcast conversion is done in `downcastTable()`.
+			conversion.for( 'upcast' ).add( upcastElementToElement( { model: 'tableRow', view: 'tr' } ) );
+
+			// Table cell conversion.
+			conversion.for( 'upcast' ).add( upcastElementToElement( { model: 'tableCell', view: 'td' } ) );
+			conversion.for( 'upcast' ).add( upcastElementToElement( { model: 'tableCell', view: 'th' } ) );
+
+			conversion.attributeToAttribute( { model: 'colspan', view: 'colspan' } );
+			conversion.attributeToAttribute( { model: 'rowspan', view: 'rowspan' } );
+		} );
 	} );
 
 	afterEach( () => {
@@ -166,7 +168,7 @@ describe( 'RemoveColumnCommand', () => {
 			] ) );
 		} );
 
-		it( 'should move colspaned cells to row below removing it\'s column', () => {
+		it( 'should decrease colspan of cells that are on removed column', () => {
 			setData( model, modelTable( [
 				[ { colspan: 3, contents: '[]00' }, '03' ],
 				[ { colspan: 2, contents: '10' }, '13' ],

+ 2 - 14
packages/ckeditor5-table/tests/commands/utils.js

@@ -10,10 +10,10 @@ import { upcastElementToElement } from '@ckeditor/ckeditor5-engine/src/conversio
 import { downcastInsertTable } from '../../src/converters/downcast';
 import upcastTable from '../../src/converters/upcasttable';
 import { modelTable } from '../_utils/utils';
-import { getColumns, getParentTable } from '../../src/commands/utils';
+import { getParentTable } from '../../src/commands/utils';
 
 describe( 'commands utils', () => {
-	let editor, model, root;
+	let editor, model;
 
 	beforeEach( () => {
 		return ModelTestEditor.create()
@@ -21,8 +21,6 @@ describe( 'commands utils', () => {
 				editor = newEditor;
 				model = editor.model;
 
-				root = model.document.getRoot( 'main' );
-
 				const conversion = editor.conversion;
 				const schema = model.schema;
 
@@ -86,14 +84,4 @@ describe( 'commands utils', () => {
 			expect( parentTable.is( 'table' ) ).to.be.true;
 		} );
 	} );
-
-	describe( 'getColumns()', () => {
-		it( 'should return proper number of columns', () => {
-			setData( model, modelTable( [
-				[ '00', { colspan: 3, contents: '01' }, '04' ]
-			] ) );
-
-			expect( getColumns( root.getNodeByPath( [ 0 ] ) ) ).to.equal( 5 );
-		} );
-	} );
 } );

+ 10 - 0
packages/ckeditor5-table/tests/tableutils.js

@@ -541,4 +541,14 @@ describe( 'TableUtils', () => {
 			] ) );
 		} );
 	} );
+
+	describe( 'getColumns()', () => {
+		it( 'should return proper number of columns', () => {
+			setData( model, modelTable( [
+				[ '00', { colspan: 3, contents: '01' }, '04' ]
+			] ) );
+
+			expect( tableUtils.getColumns( root.getNodeByPath( [ 0 ] ) ) ).to.equal( 5 );
+		} );
+	} );
 } );