Selaa lähdekoodia

Changed: MergeCellCommand value should be set to a mergable tableCell.

Maciej Gołaszewski 7 vuotta sitten
vanhempi
commit
0327df52b3

+ 31 - 20
packages/ckeditor5-table/src/commands/mergecellcommand.js

@@ -9,7 +9,7 @@
 
 import Command from '@ckeditor/ckeditor5-core/src/command';
 import Position from '@ckeditor/ckeditor5-engine/src/model/position';
-import Range from '../../../ckeditor5-engine/src/model/range';
+import Range from '@ckeditor/ckeditor5-engine/src/model/range';
 
 /**
  * The merge cell command.
@@ -31,25 +31,10 @@ export default class MergeCellCommand extends Command {
 	 * @inheritDoc
 	 */
 	refresh() {
-		this.isEnabled = this._checkEnabled();
-	}
-
-	_checkEnabled() {
-		const model = this.editor.model;
-		const doc = model.document;
-		const element = doc.selection.getFirstPosition().parent;
+		const cellToMerge = this._getCellToMerge();
 
-		const siblingToMerge = this.direction == 'right' ? element.nextSibling : element.previousSibling;
-
-		if ( !element.is( 'tableCell' ) || !siblingToMerge ) {
-			return false;
-		}
-
-		const rowspan = parseInt( element.getAttribute( 'rowspan' ) || 1 );
-
-		const nextCellRowspan = parseInt( siblingToMerge.getAttribute( 'rowspan' ) || 1 );
-
-		return nextCellRowspan === rowspan;
+		this.isEnabled = !!cellToMerge;
+		this.value = cellToMerge;
 	}
 
 	/**
@@ -62,7 +47,7 @@ export default class MergeCellCommand extends Command {
 		const doc = model.document;
 		const tableCell = doc.selection.getFirstPosition().parent;
 
-		const siblingToMerge = this.direction == 'right' ? tableCell.nextSibling : tableCell.previousSibling;
+		const siblingToMerge = this.value;
 
 		model.change( writer => {
 			writer.move( Range.createIn( siblingToMerge ), Position.createAt( tableCell, this.direction == 'right' ? 'end' : undefined ) );
@@ -74,4 +59,30 @@ export default class MergeCellCommand extends Command {
 			writer.setAttribute( 'colspan', colspan + nextTableCellColspan, tableCell );
 		} );
 	}
+
+	/**
+	 * Returns a cell that it mergable with current cell depending on command's direction.
+	 *
+	 * @returns {*}
+	 * @private
+	 */
+	_getCellToMerge() {
+		const model = this.editor.model;
+		const doc = model.document;
+		const element = doc.selection.getFirstPosition().parent;
+
+		const siblingToMerge = this.direction == 'right' ? element.nextSibling : element.previousSibling;
+
+		if ( !element.is( 'tableCell' ) || !siblingToMerge ) {
+			return;
+		}
+
+		const rowspan = parseInt( element.getAttribute( 'rowspan' ) || 1 );
+
+		const nextCellRowspan = parseInt( siblingToMerge.getAttribute( 'rowspan' ) || 1 );
+
+		if ( nextCellRowspan === rowspan ) {
+			return siblingToMerge;
+		}
+	}
 }

+ 82 - 1
packages/ckeditor5-table/tests/commands/mergecellcommand.js

@@ -13,13 +13,14 @@ import upcastTable from '../../src/converters/upcasttable';
 import { formatTable, formattedModelTable, modelTable } from '../_utils/utils';
 
 describe.only( 'MergeCellCommand', () => {
-	let editor, model, command;
+	let editor, model, command, root;
 
 	beforeEach( () => {
 		return ModelTestEditor.create()
 			.then( newEditor => {
 				editor = newEditor;
 				model = editor.model;
+				root = model.document.getRoot( 'main' );
 
 				const conversion = editor.conversion;
 				const schema = model.schema;
@@ -113,6 +114,46 @@ describe.only( 'MergeCellCommand', () => {
 			} );
 		} );
 
+		describe( 'value', () => {
+			it( 'should be set to mergeable sibling if in cell that has sibling on the right', () => {
+				setData( model, modelTable( [
+					[ '00[]', '01' ]
+				] ) );
+
+				expect( command.value ).to.equal( root.getNodeByPath( [ 0, 0, 1 ] ) );
+			} );
+
+			it( 'should be undefined if last cell of a row', () => {
+				setData( model, modelTable( [
+					[ '00', '01[]' ]
+				] ) );
+
+				expect( command.value ).to.be.undefined;
+			} );
+
+			it( 'should be set to mergeable sibling if in a cell that has sibling on the right with the same rowspan', () => {
+				setData( model, modelTable( [
+					[ { rowspan: 2, contents: '00[]' }, { rowspan: 2, contents: '01' } ]
+				] ) );
+
+				expect( command.value ).to.equal( root.getNodeByPath( [ 0, 0, 1 ] ) );
+			} );
+
+			it( 'should be undefined if in a cell that has sibling but with different rowspan', () => {
+				setData( model, modelTable( [
+					[ { rowspan: 2, contents: '00[]' }, { rowspan: 3, contents: '01' } ]
+				] ) );
+
+				expect( command.value ).to.be.undefined;
+			} );
+
+			it( 'should be undefined if not in a cell', () => {
+				setData( model, '<p>11[]</p>' );
+
+				expect( command.value ).to.be.undefined;
+			} );
+		} );
+
 		describe( 'execute()', () => {
 			it( 'should merge table cells ', () => {
 				setData( model, modelTable( [
@@ -173,6 +214,46 @@ describe.only( 'MergeCellCommand', () => {
 			} );
 		} );
 
+		describe( 'value', () => {
+			it( 'should be set to mergeable sibling if in cell that has sibling on the left', () => {
+				setData( model, modelTable( [
+					[ '00', '01[]' ]
+				] ) );
+
+				expect( command.value ).to.equal( root.getNodeByPath( [ 0, 0, 0 ] ) );
+			} );
+
+			it( 'should be undefined if first cell of a row', () => {
+				setData( model, modelTable( [
+					[ '00[]', '01' ]
+				] ) );
+
+				expect( command.value ).to.be.undefined;
+			} );
+
+			it( 'should be set to mergeable sibling if in a cell that has sibling on the left with the same rowspan', () => {
+				setData( model, modelTable( [
+					[ { rowspan: 2, contents: '00' }, { rowspan: 2, contents: '01[]' } ]
+				] ) );
+
+				expect( command.value ).to.equal( root.getNodeByPath( [ 0, 0, 0 ] ) );
+			} );
+
+			it( 'should be undefined if in a cell that has sibling but with different rowspan', () => {
+				setData( model, modelTable( [
+					[ { rowspan: 2, contents: '00' }, { rowspan: 3, contents: '01[]' } ]
+				] ) );
+
+				expect( command.value ).to.be.undefined;
+			} );
+
+			it( 'should be undefined if not in a cell', () => {
+				setData( model, '<p>11[]</p>' );
+
+				expect( command.value ).to.be.undefined;
+			} );
+		} );
+
 		describe( 'execute()', () => {
 			it( 'should merge table cells ', () => {
 				setData( model, modelTable( [