浏览代码

Fix: Merge left and right commands should be always enabled if execution does not cross the heading column boundary.

Aleksander Nowodzinski 5 年之前
父节点
当前提交
14cd7bed80

+ 10 - 6
packages/ckeditor5-table/src/commands/mergecellcommand.js

@@ -9,7 +9,11 @@
 
 
 import Command from '@ckeditor/ckeditor5-core/src/command';
 import Command from '@ckeditor/ckeditor5-core/src/command';
 import TableWalker from '../tablewalker';
 import TableWalker from '../tablewalker';
-import { findAncestor, updateNumericAttribute } from './utils';
+import {
+	findAncestor,
+	updateNumericAttribute,
+	isHeadingColumnCell
+} from './utils';
 
 
 /**
 /**
  * The merge cell command.
  * The merge cell command.
@@ -171,13 +175,13 @@ function getHorizontalCell( tableCell, direction, tableUtils ) {
 	const { column: rightCellColumn } = tableUtils.getCellLocation( cellOnRight );
 	const { column: rightCellColumn } = tableUtils.getCellLocation( cellOnRight );
 
 
 	const leftCellSpan = parseInt( cellOnLeft.getAttribute( 'colspan' ) || 1 );
 	const leftCellSpan = parseInt( cellOnLeft.getAttribute( 'colspan' ) || 1 );
-	const rightCellSpan = parseInt( cellOnRight.getAttribute( 'colspan' ) || 1 );
 
 
-	// We cannot merge cells if the result will extend over heading section.
-	const isMergeWithBodyCell = direction == 'right' && ( rightCellColumn + rightCellSpan > headingColumns );
-	const isMergeWithHeadCell = direction == 'left' && ( leftCellColumn + leftCellSpan > headingColumns - 1 );
+	const isCellOnLeftInHeadingColumn = isHeadingColumnCell( tableUtils, cellOnLeft, table );
+	const isCellOnRightInRegularColumn = !isHeadingColumnCell( tableUtils, cellOnRight, table );
 
 
-	if ( headingColumns && ( isMergeWithBodyCell || isMergeWithHeadCell ) ) {
+	// We cannot merge heading columns cells with regular cells.
+	// We cannot merge regular cells with heading column cells.
+	if ( headingColumns && isCellOnLeftInHeadingColumn && isCellOnRightInRegularColumn ) {
 		return;
 		return;
 	}
 	}
 
 

+ 60 - 40
packages/ckeditor5-table/tests/commands/mergecellcommand.js

@@ -95,44 +95,46 @@ describe( 'MergeCellCommand', () => {
 				expect( command.isEnabled ).to.be.false;
 				expect( command.isEnabled ).to.be.false;
 			} );
 			} );
 
 
-			it( 'should be false if mergeable cell is in other table section then current cell', () => {
-				setData( model, modelTable( [
-					[ '00[]', '01' ]
-				], { headingColumns: 1 } ) );
+			describe( 'when the heading section is in the table', () => {
+				it( 'should be false if mergeable cell is in other table section then current cell', () => {
+					setData( model, modelTable( [
+						[ '00[]', '01' ]
+					], { headingColumns: 1 } ) );
 
 
-				expect( command.isEnabled ).to.be.false;
-			} );
+					expect( command.isEnabled ).to.be.false;
+				} );
 
 
-			it( 'should be false if merged cell would cross heading section (mergeable cell with colspan)', () => {
-				setData( model, modelTable( [
-					[ '00[]', { colspan: 2, contents: '01' }, '02', '03' ]
-				], { headingColumns: 2 } ) );
+				it( 'should be true if merged cell would not cross heading section (mergeable cell with colspan)', () => {
+					setData( model, modelTable( [
+						[ '00[]', { colspan: 2, contents: '01' }, '02', '03' ]
+					], { headingColumns: 3 } ) );
 
 
-				expect( command.isEnabled ).to.be.false;
-			} );
+					expect( command.isEnabled ).to.be.true;
+				} );
 
 
-			it( 'should be true if merged cell would not cross heading section (mergeable cell with colspan)', () => {
-				setData( model, modelTable( [
-					[ '00[]', { colspan: 2, contents: '01' }, '02', '03' ]
-				], { headingColumns: 3 } ) );
+				it( 'should be false if merged cell would cross heading section (current cell with colspan)', () => {
+					setData( model, modelTable( [
+						[ { colspan: 2, contents: '00[]' }, '01', '02', '03' ]
+					], { headingColumns: 2 } ) );
 
 
-				expect( command.isEnabled ).to.be.true;
-			} );
+					expect( command.isEnabled ).to.be.false;
+				} );
 
 
-			it( 'should be false if merged cell would cross heading section (current cell with colspan)', () => {
-				setData( model, modelTable( [
-					[ { colspan: 2, contents: '00[]' }, '01', '02', '03' ]
-				], { headingColumns: 2 } ) );
+				it( 'should be true if merged cell would not cross heading section (current cell with colspan)', () => {
+					setData( model, modelTable( [
+						[ { colspan: 2, contents: '00[]' }, '01', '02', '03' ]
+					], { headingColumns: 3 } ) );
 
 
-				expect( command.isEnabled ).to.be.false;
-			} );
+					expect( command.isEnabled ).to.be.true;
+				} );
 
 
-			it( 'should be true if merged cell would not cross heading section (current cell with colspan)', () => {
-				setData( model, modelTable( [
-					[ { colspan: 2, contents: '00[]' }, '01', '02', '03' ]
-				], { headingColumns: 3 } ) );
+				it( 'should be true if merged cell would not cross the section boundary (regular section)', () => {
+					setData( model, modelTable( [
+						[ '00', '01', '02[]', '03' ]
+					], { headingColumns: 1 } ) );
 
 
-				expect( command.isEnabled ).to.be.true;
+					expect( command.isEnabled ).to.be.true;
+				} );
 			} );
 			} );
 		} );
 		} );
 
 
@@ -315,20 +317,38 @@ describe( 'MergeCellCommand', () => {
 				expect( command.isEnabled ).to.be.false;
 				expect( command.isEnabled ).to.be.false;
 			} );
 			} );
 
 
-			it( 'should be false if mergeable cell is in other table section then current cell', () => {
-				setData( model, modelTable( [
-					[ '00', '01[]' ]
-				], { headingColumns: 1 } ) );
+			describe( 'when the heading section is in the table', () => {
+				it( 'should be false if mergeable cell is in other table section then current cell', () => {
+					setData( model, modelTable( [
+						[ '00', '01[]' ]
+					], { headingColumns: 1 } ) );
 
 
-				expect( command.isEnabled ).to.be.false;
-			} );
+					expect( command.isEnabled ).to.be.false;
+				} );
 
 
-			it( 'should be false if merged cell would cross heading section (mergeable cell with colspan)', () => {
-				setData( model, modelTable( [
-					[ { colspan: 2, contents: '00' }, '02[]', '03' ]
-				], { headingColumns: 2 } ) );
+				it( 'should be false if merged cell would cross heading section (mergeable cell with colspan)', () => {
+					setData( model, modelTable( [
+						[ { colspan: 2, contents: '00' }, '02[]', '03' ]
+					], { headingColumns: 2 } ) );
 
 
-				expect( command.isEnabled ).to.be.false;
+					expect( command.isEnabled ).to.be.false;
+				} );
+
+				it( 'should be true if merged cell would not cross the section boundary (in regular section)', () => {
+					setData( model, modelTable( [
+						[ '00', '01', '02[]', '03' ]
+					], { headingColumns: 1 } ) );
+
+					expect( command.isEnabled ).to.be.true;
+				} );
+
+				it( 'should be true if merged cell would not cross the section boundary (in heading section)', () => {
+					setData( model, modelTable( [
+						[ '00', '01[]', '02', '03' ]
+					], { headingColumns: 2 } ) );
+
+					expect( command.isEnabled ).to.be.true;
+				} );
 			} );
 			} );
 		} );
 		} );