瀏覽代碼

Fix: Merge cell horizontally should not be possible on overlapped cells.

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

+ 28 - 3
packages/ckeditor5-table/src/commands/mergecellcommand.js

@@ -12,6 +12,7 @@ import Position from '@ckeditor/ckeditor5-engine/src/model/position';
 import Range from '@ckeditor/ckeditor5-engine/src/model/range';
 import TableWalker from '../tablewalker';
 import { updateNumericAttribute } from './utils';
+import TableUtils from '../tableutils';
 
 /**
  * The merge cell command.
@@ -130,8 +131,12 @@ export default class MergeCellCommand extends Command {
 			return;
 		}
 
+		const tableUtils = this.editor.plugins.get( TableUtils );
+
 		// First get the cell on proper direction.
-		const cellToMerge = this.isHorizontal ? getHorizontalCell( element, this.direction ) : getVerticalCell( element, this.direction );
+		const cellToMerge = this.isHorizontal ?
+			getHorizontalCell( element, this.direction, tableUtils ) :
+			getVerticalCell( element, this.direction );
 
 		if ( !cellToMerge ) {
 			return;
@@ -154,8 +159,28 @@ export default class MergeCellCommand extends Command {
 // @param {module:engine/model/element~Element} tableCell
 // @param {String} direction
 // @returns {module:engine/model/node~Node|null}
-function getHorizontalCell( tableCell, direction ) {
-	return direction == 'right' ? tableCell.nextSibling : tableCell.previousSibling;
+function getHorizontalCell( tableCell, direction, tableUtils ) {
+	const horizontalCell = direction == 'right' ? tableCell.nextSibling : tableCell.previousSibling;
+
+	if ( !horizontalCell ) {
+		return;
+	}
+
+	// Sort cells:
+	const cellOnLeft = direction == 'right' ? tableCell : horizontalCell;
+	const cellOnRight = direction == 'right' ? horizontalCell : tableCell;
+
+	// Get their column indexes:
+	const { column: leftCellColumn } = tableUtils.getCellLocation( cellOnLeft );
+	const { column: rightCellColumn } = tableUtils.getCellLocation( cellOnRight );
+
+	const leftCellSpan = parseInt( cellOnLeft.getAttribute( 'colspan' ) || 1 );
+
+	// The cell on the right must have index that is distant to the cell on the left by the left cell's width (colspan).
+	const cellsAreTouching = leftCellColumn + leftCellSpan === rightCellColumn;
+
+	// If the right cell's column index is different it means that there are rowspanned cells between them.
+	return cellsAreTouching ? horizontalCell : undefined;
 }
 
 // Returns the cell that can be merged vertically.

+ 75 - 37
packages/ckeditor5-table/tests/commands/mergecellcommand.js

@@ -18,60 +18,62 @@ import {
 } from '../../src/converters/downcast';
 import upcastTable from '../../src/converters/upcasttable';
 import { formatTable, formattedModelTable, modelTable } from '../_utils/utils';
+import TableUtils from '../../src/tableutils';
 
 describe( 'MergeCellCommand', () => {
 	let editor, model, command, root;
 
 	beforeEach( () => {
-		return ModelTestEditor.create()
-			.then( newEditor => {
-				editor = newEditor;
-				model = editor.model;
-				root = model.document.getRoot( 'main' );
+		return ModelTestEditor.create( {
+			plugins: [ TableUtils ]
+		} ).then( newEditor => {
+			editor = newEditor;
+			model = editor.model;
+			root = model.document.getRoot( 'main' );
 
-				const conversion = editor.conversion;
-				const schema = model.schema;
+			const conversion = editor.conversion;
+			const schema = model.schema;
 
-				schema.register( 'table', {
-					allowWhere: '$block',
-					allowAttributes: [ 'headingRows' ],
-					isObject: true
-				} );
+			schema.register( 'table', {
+				allowWhere: '$block',
+				allowAttributes: [ 'headingRows' ],
+				isObject: true
+			} );
 
-				schema.register( 'tableRow', { allowIn: 'table' } );
+			schema.register( 'tableRow', { allowIn: 'table' } );
 
-				schema.register( 'tableCell', {
-					allowIn: 'tableRow',
-					allowContentOf: '$block',
-					allowAttributes: [ 'colspan', 'rowspan' ],
-					isLimit: true
-				} );
+			schema.register( 'tableCell', {
+				allowIn: 'tableRow',
+				allowContentOf: '$block',
+				allowAttributes: [ 'colspan', 'rowspan' ],
+				isLimit: true
+			} );
 
-				model.schema.register( 'p', { inheritAllFrom: '$block' } );
+			model.schema.register( 'p', { inheritAllFrom: '$block' } );
 
-				// Table conversion.
-				conversion.for( 'upcast' ).add( upcastTable() );
-				conversion.for( 'downcast' ).add( downcastInsertTable() );
+			// Table conversion.
+			conversion.for( 'upcast' ).add( upcastTable() );
+			conversion.for( 'downcast' ).add( downcastInsertTable() );
 
-				// Insert row conversion.
-				conversion.for( 'downcast' ).add( downcastInsertRow() );
+			// Insert row conversion.
+			conversion.for( 'downcast' ).add( downcastInsertRow() );
 
-				// Remove row conversion.
-				conversion.for( 'downcast' ).add( downcastRemoveRow() );
+			// Remove row conversion.
+			conversion.for( 'downcast' ).add( downcastRemoveRow() );
 
-				// Table cell conversion.
-				conversion.for( 'downcast' ).add( downcastInsertCell() );
+			// Table cell conversion.
+			conversion.for( 'downcast' ).add( downcastInsertCell() );
 
-				conversion.for( 'upcast' ).add( upcastElementToElement( { model: 'tableCell', view: 'td' } ) );
-				conversion.for( 'upcast' ).add( upcastElementToElement( { model: 'tableCell', view: 'th' } ) );
+			conversion.for( 'upcast' ).add( upcastElementToElement( { model: 'tableCell', view: 'td' } ) );
+			conversion.for( 'upcast' ).add( upcastElementToElement( { model: 'tableCell', view: 'th' } ) );
 
-				// Table attributes conversion.
-				conversion.attributeToAttribute( { model: 'colspan', view: 'colspan' } );
-				conversion.attributeToAttribute( { model: 'rowspan', view: 'rowspan' } );
+			// Table attributes conversion.
+			conversion.attributeToAttribute( { model: 'colspan', view: 'colspan' } );
+			conversion.attributeToAttribute( { model: 'rowspan', view: 'rowspan' } );
 
-				conversion.for( 'downcast' ).add( downcastTableHeadingColumnsChange() );
-				conversion.for( 'downcast' ).add( downcastTableHeadingRowsChange() );
-			} );
+			conversion.for( 'downcast' ).add( downcastTableHeadingColumnsChange() );
+			conversion.for( 'downcast' ).add( downcastTableHeadingRowsChange() );
+		} );
 	} );
 
 	afterEach( () => {
@@ -116,6 +118,24 @@ describe( 'MergeCellCommand', () => {
 				expect( command.isEnabled ).to.be.false;
 			} );
 
+			it( 'should be false when next cell is rowspanned', () => {
+				setData( model, modelTable( [
+					[ '00', { rowspan: 3, contents: '01' }, '02' ],
+					[ '10[]', '12' ],
+					[ '20', '22' ]
+				] ) );
+
+				expect( command.isEnabled ).to.be.false;
+			} );
+
+			it( 'should be true when current cell is colspanned', () => {
+				setData( model, modelTable( [
+					[ { colspan: 2, contents: '00[]' }, '02' ]
+				] ) );
+
+				expect( command.isEnabled ).to.be.true;
+			} );
+
 			it( 'should be false if not in a cell', () => {
 				setData( model, '<p>11[]</p>' );
 
@@ -216,6 +236,24 @@ describe( 'MergeCellCommand', () => {
 				expect( command.isEnabled ).to.be.false;
 			} );
 
+			it( 'should be false when next cell is rowspanned', () => {
+				setData( model, modelTable( [
+					[ '00', { rowspan: 3, contents: '01' }, '02' ],
+					[ '10', '12[]' ],
+					[ '20', '22' ]
+				] ) );
+
+				expect( command.isEnabled ).to.be.false;
+			} );
+
+			it( 'should be true when mergeable cell is colspanned', () => {
+				setData( model, modelTable( [
+					[ { colspan: 2, contents: '00' }, '02[]' ]
+				] ) );
+
+				expect( command.isEnabled ).to.be.true;
+			} );
+
 			it( 'should be false if not in a cell', () => {
 				setData( model, '<p>11[]</p>' );