Skip to content
This repository was archived by the owner on Jun 26, 2020. It is now read-only.

Commit 72b6315

Browse files
ReinmarReinmar
authored
Merge pull request #82 from ckeditor/t/68
Fix: Merge cell horizontally should not be possible on overlapped cells. Closes #68.
2 parents b0991f4 + 185bc89 commit 72b6315

File tree

9 files changed

+377
-298
lines changed

9 files changed

+377
-298
lines changed

‎src/commands/mergecellcommand.js‎

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import Position from '@ckeditor/ckeditor5-engine/src/model/position';
1212
import Range from '@ckeditor/ckeditor5-engine/src/model/range';
1313
import TableWalker from '../tablewalker';
1414
import { updateNumericAttribute } from './utils';
15+
import TableUtils from '../tableutils';
1516

1617
/**
1718
* The merge cell command.
@@ -130,8 +131,12 @@ export default class MergeCellCommand extends Command {
130131
return;
131132
}
132133

134+
const tableUtils = this.editor.plugins.get( TableUtils );
135+
133136
// First get the cell on proper direction.
134-
const cellToMerge = this.isHorizontal ? getHorizontalCell( element, this.direction ) : getVerticalCell( element, this.direction );
137+
const cellToMerge = this.isHorizontal ?
138+
getHorizontalCell( element, this.direction, tableUtils ) :
139+
getVerticalCell( element, this.direction );
135140

136141
if ( !cellToMerge ) {
137142
return;
@@ -154,8 +159,28 @@ export default class MergeCellCommand extends Command {
154159
// @param {module:engine/model/element~Element} tableCell
155160
// @param {String} direction
156161
// @returns {module:engine/model/node~Node|null}
157-
function getHorizontalCell( tableCell, direction ) {
158-
return direction == 'right' ? tableCell.nextSibling : tableCell.previousSibling;
162+
function getHorizontalCell( tableCell, direction, tableUtils ) {
163+
const horizontalCell = direction == 'right' ? tableCell.nextSibling : tableCell.previousSibling;
164+
165+
if ( !horizontalCell ) {
166+
return;
167+
}
168+
169+
// Sort cells:
170+
const cellOnLeft = direction == 'right' ? tableCell : horizontalCell;
171+
const cellOnRight = direction == 'right' ? horizontalCell : tableCell;
172+
173+
// Get their column indexes:
174+
const { column: leftCellColumn } = tableUtils.getCellLocation( cellOnLeft );
175+
const { column: rightCellColumn } = tableUtils.getCellLocation( cellOnRight );
176+
177+
const leftCellSpan = parseInt( cellOnLeft.getAttribute( 'colspan' ) || 1 );
178+
179+
// The cell on the right must have index that is distant to the cell on the left by the left cell's width (colspan).
180+
const cellsAreTouching = leftCellColumn + leftCellSpan === rightCellColumn;
181+
182+
// If the right cell's column index is different it means that there are rowspanned cells between them.
183+
return cellsAreTouching ? horizontalCell : undefined;
159184
}
160185

161186
// Returns the cell that can be merged vertically.

‎tests/commands/insertcolumncommand.js‎

Lines changed: 41 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -24,55 +24,57 @@ describe( 'InsertColumnCommand', () => {
2424
let editor, model, command;
2525

2626
beforeEach( () => {
27-
return ModelTestEditor.create( {
28-
plugins: [ TableUtils ]
29-
} ).then( newEditor => {
30-
editor = newEditor;
31-
model = editor.model;
32-
33-
const conversion = editor.conversion;
34-
const schema = model.schema;
35-
36-
schema.register( 'table', {
37-
allowWhere: '$block',
38-
allowAttributes: [ 'headingRows' ],
39-
isObject: true
40-
} );
27+
return ModelTestEditor
28+
.create( {
29+
plugins: [ TableUtils ]
30+
} )
31+
.then( newEditor => {
32+
editor = newEditor;
33+
model = editor.model;
4134

42-
schema.register( 'tableRow', { allowIn: 'table' } );
35+
const conversion = editor.conversion;
36+
const schema = model.schema;
4337

44-
schema.register( 'tableCell', {
45-
allowIn: 'tableRow',
46-
allowContentOf: '$block',
47-
allowAttributes: [ 'colspan', 'rowspan' ],
48-
isLimit: true
49-
} );
38+
schema.register( 'table', {
39+
allowWhere: '$block',
40+
allowAttributes: [ 'headingRows' ],
41+
isObject: true
42+
} );
5043

51-
model.schema.register( 'p', { inheritAllFrom: '$block' } );
44+
schema.register( 'tableRow', { allowIn: 'table' } );
5245

53-
// Table conversion.
54-
conversion.for( 'upcast' ).add( upcastTable() );
55-
conversion.for( 'downcast' ).add( downcastInsertTable() );
46+
schema.register( 'tableCell', {
47+
allowIn: 'tableRow',
48+
allowContentOf: '$block',
49+
allowAttributes: [ 'colspan', 'rowspan' ],
50+
isLimit: true
51+
} );
5652

57-
// Insert row conversion.
58-
conversion.for( 'downcast' ).add( downcastInsertRow() );
53+
model.schema.register( 'p', { inheritAllFrom: '$block' } );
5954

60-
// Remove row conversion.
61-
conversion.for( 'downcast' ).add( downcastRemoveRow() );
55+
// Table conversion.
56+
conversion.for( 'upcast' ).add( upcastTable() );
57+
conversion.for( 'downcast' ).add( downcastInsertTable() );
6258

63-
// Table cell conversion.
64-
conversion.for( 'downcast' ).add( downcastInsertCell() );
59+
// Insert row conversion.
60+
conversion.for( 'downcast' ).add( downcastInsertRow() );
6561

66-
conversion.for( 'upcast' ).add( upcastElementToElement( { model: 'tableCell', view: 'td' } ) );
67-
conversion.for( 'upcast' ).add( upcastElementToElement( { model: 'tableCell', view: 'th' } ) );
62+
// Remove row conversion.
63+
conversion.for( 'downcast' ).add( downcastRemoveRow() );
6864

69-
// Table attributes conversion.
70-
conversion.attributeToAttribute( { model: 'colspan', view: 'colspan' } );
71-
conversion.attributeToAttribute( { model: 'rowspan', view: 'rowspan' } );
65+
// Table cell conversion.
66+
conversion.for( 'downcast' ).add( downcastInsertCell() );
7267

73-
conversion.for( 'downcast' ).add( downcastTableHeadingColumnsChange() );
74-
conversion.for( 'downcast' ).add( downcastTableHeadingRowsChange() );
75-
} );
68+
conversion.for( 'upcast' ).add( upcastElementToElement( { model: 'tableCell', view: 'td' } ) );
69+
conversion.for( 'upcast' ).add( upcastElementToElement( { model: 'tableCell', view: 'th' } ) );
70+
71+
// Table attributes conversion.
72+
conversion.attributeToAttribute( { model: 'colspan', view: 'colspan' } );
73+
conversion.attributeToAttribute( { model: 'rowspan', view: 'rowspan' } );
74+
75+
conversion.for( 'downcast' ).add( downcastTableHeadingColumnsChange() );
76+
conversion.for( 'downcast' ).add( downcastTableHeadingRowsChange() );
77+
} );
7678
} );
7779

7880
afterEach( () => {

‎tests/commands/insertrowcommand.js‎

Lines changed: 41 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -24,55 +24,57 @@ describe( 'InsertRowCommand', () => {
2424
let editor, model, command;
2525

2626
beforeEach( () => {
27-
return ModelTestEditor.create( {
28-
plugins: [ TableUtils ]
29-
} ).then( newEditor => {
30-
editor = newEditor;
31-
model = editor.model;
32-
33-
const conversion = editor.conversion;
34-
const schema = model.schema;
35-
36-
schema.register( 'table', {
37-
allowWhere: '$block',
38-
allowAttributes: [ 'headingRows' ],
39-
isObject: true
40-
} );
27+
return ModelTestEditor
28+
.create( {
29+
plugins: [ TableUtils ]
30+
} )
31+
.then( newEditor => {
32+
editor = newEditor;
33+
model = editor.model;
4134

42-
schema.register( 'tableRow', { allowIn: 'table' } );
35+
const conversion = editor.conversion;
36+
const schema = model.schema;
4337

44-
schema.register( 'tableCell', {
45-
allowIn: 'tableRow',
46-
allowContentOf: '$block',
47-
allowAttributes: [ 'colspan', 'rowspan' ],
48-
isLimit: true
49-
} );
38+
schema.register( 'table', {
39+
allowWhere: '$block',
40+
allowAttributes: [ 'headingRows' ],
41+
isObject: true
42+
} );
5043

51-
model.schema.register( 'p', { inheritAllFrom: '$block' } );
44+
schema.register( 'tableRow', { allowIn: 'table' } );
5245

53-
// Table conversion.
54-
conversion.for( 'upcast' ).add( upcastTable() );
55-
conversion.for( 'downcast' ).add( downcastInsertTable() );
46+
schema.register( 'tableCell', {
47+
allowIn: 'tableRow',
48+
allowContentOf: '$block',
49+
allowAttributes: [ 'colspan', 'rowspan' ],
50+
isLimit: true
51+
} );
5652

57-
// Insert row conversion.
58-
conversion.for( 'downcast' ).add( downcastInsertRow() );
53+
model.schema.register( 'p', { inheritAllFrom: '$block' } );
5954

60-
// Remove row conversion.
61-
conversion.for( 'downcast' ).add( downcastRemoveRow() );
55+
// Table conversion.
56+
conversion.for( 'upcast' ).add( upcastTable() );
57+
conversion.for( 'downcast' ).add( downcastInsertTable() );
6258

63-
// Table cell conversion.
64-
conversion.for( 'downcast' ).add( downcastInsertCell() );
59+
// Insert row conversion.
60+
conversion.for( 'downcast' ).add( downcastInsertRow() );
6561

66-
conversion.for( 'upcast' ).add( upcastElementToElement( { model: 'tableCell', view: 'td' } ) );
67-
conversion.for( 'upcast' ).add( upcastElementToElement( { model: 'tableCell', view: 'th' } ) );
62+
// Remove row conversion.
63+
conversion.for( 'downcast' ).add( downcastRemoveRow() );
6864

69-
// Table attributes conversion.
70-
conversion.attributeToAttribute( { model: 'colspan', view: 'colspan' } );
71-
conversion.attributeToAttribute( { model: 'rowspan', view: 'rowspan' } );
65+
// Table cell conversion.
66+
conversion.for( 'downcast' ).add( downcastInsertCell() );
7267

73-
conversion.for( 'downcast' ).add( downcastTableHeadingColumnsChange() );
74-
conversion.for( 'downcast' ).add( downcastTableHeadingRowsChange() );
75-
} );
68+
conversion.for( 'upcast' ).add( upcastElementToElement( { model: 'tableCell', view: 'td' } ) );
69+
conversion.for( 'upcast' ).add( upcastElementToElement( { model: 'tableCell', view: 'th' } ) );
70+
71+
// Table attributes conversion.
72+
conversion.attributeToAttribute( { model: 'colspan', view: 'colspan' } );
73+
conversion.attributeToAttribute( { model: 'rowspan', view: 'rowspan' } );
74+
75+
conversion.for( 'downcast' ).add( downcastTableHeadingColumnsChange() );
76+
conversion.for( 'downcast' ).add( downcastTableHeadingRowsChange() );
77+
} );
7678
} );
7779

7880
afterEach( () => {

‎tests/commands/inserttablecommand.js‎

Lines changed: 42 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -25,56 +25,58 @@ describe( 'InsertTableCommand', () => {
2525
let editor, model, command;
2626

2727
beforeEach( () => {
28-
return ModelTestEditor.create( {
29-
plugins: [ TableUtils ]
30-
} ).then( newEditor => {
31-
editor = newEditor;
32-
model = editor.model;
33-
command = new InsertTableCommand( editor );
34-
35-
const conversion = editor.conversion;
36-
const schema = model.schema;
37-
38-
schema.register( 'table', {
39-
allowWhere: '$block',
40-
allowAttributes: [ 'headingRows' ],
41-
isObject: true
42-
} );
28+
return ModelTestEditor
29+
.create( {
30+
plugins: [ TableUtils ]
31+
} )
32+
.then( newEditor => {
33+
editor = newEditor;
34+
model = editor.model;
35+
command = new InsertTableCommand( editor );
4336

44-
schema.register( 'tableRow', { allowIn: 'table' } );
37+
const conversion = editor.conversion;
38+
const schema = model.schema;
4539

46-
schema.register( 'tableCell', {
47-
allowIn: 'tableRow',
48-
allowContentOf: '$block',
49-
allowAttributes: [ 'colspan', 'rowspan' ],
50-
isLimit: true
51-
} );
40+
schema.register( 'table', {
41+
allowWhere: '$block',
42+
allowAttributes: [ 'headingRows' ],
43+
isObject: true
44+
} );
5245

53-
model.schema.register( 'p', { inheritAllFrom: '$block' } );
46+
schema.register( 'tableRow', { allowIn: 'table' } );
5447

55-
// Table conversion.
56-
conversion.for( 'upcast' ).add( upcastTable() );
57-
conversion.for( 'downcast' ).add( downcastInsertTable() );
48+
schema.register( 'tableCell', {
49+
allowIn: 'tableRow',
50+
allowContentOf: '$block',
51+
allowAttributes: [ 'colspan', 'rowspan' ],
52+
isLimit: true
53+
} );
5854

59-
// Insert row conversion.
60-
conversion.for( 'downcast' ).add( downcastInsertRow() );
55+
model.schema.register( 'p', { inheritAllFrom: '$block' } );
6156

62-
// Remove row conversion.
63-
conversion.for( 'downcast' ).add( downcastRemoveRow() );
57+
// Table conversion.
58+
conversion.for( 'upcast' ).add( upcastTable() );
59+
conversion.for( 'downcast' ).add( downcastInsertTable() );
6460

65-
// Table cell conversion.
66-
conversion.for( 'downcast' ).add( downcastInsertCell() );
61+
// Insert row conversion.
62+
conversion.for( 'downcast' ).add( downcastInsertRow() );
6763

68-
conversion.for( 'upcast' ).add( upcastElementToElement( { model: 'tableCell', view: 'td' } ) );
69-
conversion.for( 'upcast' ).add( upcastElementToElement( { model: 'tableCell', view: 'th' } ) );
64+
// Remove row conversion.
65+
conversion.for( 'downcast' ).add( downcastRemoveRow() );
7066

71-
// Table attributes conversion.
72-
conversion.attributeToAttribute( { model: 'colspan', view: 'colspan' } );
73-
conversion.attributeToAttribute( { model: 'rowspan', view: 'rowspan' } );
67+
// Table cell conversion.
68+
conversion.for( 'downcast' ).add( downcastInsertCell() );
7469

75-
conversion.for( 'downcast' ).add( downcastTableHeadingColumnsChange() );
76-
conversion.for( 'downcast' ).add( downcastTableHeadingRowsChange() );
77-
} );
70+
conversion.for( 'upcast' ).add( upcastElementToElement( { model: 'tableCell', view: 'td' } ) );
71+
conversion.for( 'upcast' ).add( upcastElementToElement( { model: 'tableCell', view: 'th' } ) );
72+
73+
// Table attributes conversion.
74+
conversion.attributeToAttribute( { model: 'colspan', view: 'colspan' } );
75+
conversion.attributeToAttribute( { model: 'rowspan', view: 'rowspan' } );
76+
77+
conversion.for( 'downcast' ).add( downcastTableHeadingColumnsChange() );
78+
conversion.for( 'downcast' ).add( downcastTableHeadingRowsChange() );
79+
} );
7880
} );
7981

8082
afterEach( () => {

0 commit comments

Comments
 (0)