Kaynağa Gözat

Merge branch 'master' into t/13

Aleksander Nowodzinski 7 yıl önce
ebeveyn
işleme
7f20d27fb3

+ 5 - 1
packages/ckeditor5-table/src/commands/mergecellcommand.js

@@ -199,10 +199,14 @@ function getVerticalCell( tableCell, direction ) {
 		return;
 	}
 
+	const rowspan = parseInt( tableCell.getAttribute( 'rowspan' ) || 1 );
 	const headingRows = table.getAttribute( 'headingRows' ) || 0;
 
+	const isMergeWithBodyCell = direction == 'down' && ( rowIndex + rowspan ) === headingRows;
+	const isMergeWithHeadCell = direction == 'up' && rowIndex === headingRows;
+
 	// Don't search for mergeable cell if direction points out of the current table section.
-	if ( headingRows && ( ( direction == 'down' && rowIndex === headingRows - 1 ) || ( direction == 'up' && rowIndex === headingRows ) ) ) {
+	if ( headingRows && ( isMergeWithBodyCell || isMergeWithHeadCell ) ) {
 		return;
 	}
 

+ 3 - 6
packages/ckeditor5-table/src/commands/setheadercolumncommand.js

@@ -74,15 +74,12 @@ export default class SetHeaderColumnCommand extends Command {
 		const table = tableRow.parent;
 
 		const currentHeadingColumns = parseInt( table.getAttribute( 'headingColumns' ) || 0 );
+		const { column: selectionColumn } = tableUtils.getCellLocation( tableCell );
 
-		let { column } = tableUtils.getCellLocation( tableCell );
-
-		if ( column + 1 !== currentHeadingColumns ) {
-			column++;
-		}
+		const headingColumnsToSet = currentHeadingColumns > selectionColumn ? selectionColumn : selectionColumn + 1;
 
 		model.change( writer => {
-			updateNumericAttribute( 'headingColumns', column, table, writer, 0 );
+			updateNumericAttribute( 'headingColumns', headingColumnsToSet, table, writer, 0 );
 		} );
 	}
 

+ 6 - 8
packages/ckeditor5-table/src/commands/setheaderrowcommand.js

@@ -74,24 +74,22 @@ export default class SetHeaderRowCommand extends Command {
 		const table = tableRow.parent;
 
 		const currentHeadingRows = table.getAttribute( 'headingRows' ) || 0;
-		let rowIndex = tableRow.index;
+		const selectionRow = tableRow.index;
 
-		if ( rowIndex + 1 !== currentHeadingRows ) {
-			rowIndex++;
-		}
+		const headingRowsToSet = currentHeadingRows > selectionRow ? selectionRow : selectionRow + 1;
 
 		model.change( writer => {
-			if ( rowIndex ) {
+			if ( headingRowsToSet ) {
 				// Changing heading rows requires to check if any of a heading cell is overlapping vertically the table head.
 				// Any table cell that has a rowspan attribute > 1 will not exceed the table head so we need to fix it in rows below.
-				const cellsToSplit = getOverlappingCells( table, rowIndex, currentHeadingRows );
+				const cellsToSplit = getOverlappingCells( table, headingRowsToSet, currentHeadingRows );
 
 				for ( const cell of cellsToSplit ) {
-					splitHorizontally( cell, rowIndex, writer );
+					splitHorizontally( cell, headingRowsToSet, writer );
 				}
 			}
 
-			updateNumericAttribute( 'headingRows', rowIndex, table, writer, 0 );
+			updateNumericAttribute( 'headingRows', headingRowsToSet, table, writer, 0 );
 		} );
 	}
 

+ 14 - 2
packages/ckeditor5-table/src/converters/downcast.js

@@ -307,7 +307,9 @@ export function downcastRemoveRow() {
 	}, { priority: 'higher' } );
 }
 
-// Renames a table cell in the view to a given element name.
+// Renames an existing table cell in the view to a given element name.
+//
+// **Note** This method will not do anything if a view table cell was not yet converted.
 //
 // @param {module:engine/model/element~Element} tableCell
 // @param {String} desiredCellElementName
@@ -316,6 +318,11 @@ export function downcastRemoveRow() {
 function renameViewTableCell( tableCell, desiredCellElementName, conversionApi, asWidget ) {
 	const viewCell = conversionApi.mapper.toViewElement( tableCell );
 
+	// View cell might be not yet converted - skip it as it will be properly created by cell converter later on.
+	if ( !viewCell ) {
+		return;
+	}
+
 	let renamedCell;
 
 	if ( asWidget ) {
@@ -486,6 +493,8 @@ function removeTableSectionIfEmpty( sectionName, tableElement, conversionApi ) {
 
 // Moves view table rows associated with passed model rows to the provided table section element.
 //
+// **Note** This method will skip not converted table rows.
+//
 // @param {Array.<module:engine/model/element~Element>} rowsToMove
 // @param {module:engine/view/element~Element} viewTableSection
 // @param {Object} conversionApi
@@ -494,7 +503,10 @@ function moveViewRowsToTableSection( rowsToMove, viewTableSection, conversionApi
 	for ( const tableRow of rowsToMove ) {
 		const viewTableRow = conversionApi.mapper.toViewElement( tableRow );
 
-		conversionApi.writer.move( ViewRange.createOn( viewTableRow ), ViewPosition.createAt( viewTableSection, offset ) );
+		// View table row might be not yet converted - skip it as it will be properly created by cell converter later on.
+		if ( viewTableRow ) {
+			conversionApi.writer.move( ViewRange.createOn( viewTableRow ), ViewPosition.createAt( viewTableSection, offset ) );
+		}
 	}
 }
 

+ 10 - 0
packages/ckeditor5-table/tests/commands/mergecellcommand.js

@@ -413,6 +413,16 @@ describe( 'MergeCellCommand', () => {
 				expect( command.value ).to.be.undefined;
 			} );
 
+			it( 'should be undefined if mergable cell is in other table section', () => {
+				setData( model, modelTable( [
+					[ { rowspan: 2, contents: '00[]' }, '02' ],
+					[ '12' ],
+					[ '21', '22' ]
+				], { headingRows: 2 } ) );
+
+				expect( command.value ).to.be.undefined;
+			} );
+
 			it( 'should be undefined if not in a cell', () => {
 				setData( model, '<p>11[]</p>' );
 

+ 17 - 34
packages/ckeditor5-table/tests/commands/setheadercolumncommand.js

@@ -133,60 +133,43 @@ describe( 'HeaderColumnCommand', () => {
 	describe( 'execute()', () => {
 		it( 'should set heading columns attribute that cover column in which is selection', () => {
 			setData( model, modelTable( [
-				[ '00', '01' ],
-				[ '[]10', '11' ],
-				[ '20', '21' ]
+				[ '00', '01[]', '02', '03' ]
 			] ) );
 
 			command.execute();
 
 			expect( formatTable( getData( model ) ) ).to.equal( formattedModelTable( [
-				[ '00', '01' ],
-				[ '[]10', '11' ],
-				[ '20', '21' ]
-			], { headingColumns: 1 } ) );
+				[ '00', '01[]', '02', '03' ]
+			], { headingColumns: 2 } ) );
 		} );
 
-		it(
-			'should set heading columns attribute if currently selected column is a heading so the heading section is before this column',
-			() => {
-				setData( model, modelTable( [
-					[ '00', '01' ],
-					[ '[]10', '11' ],
-					[ '20', '21' ]
-				], { headingColumns: 2 } ) );
-
-				command.execute();
-
-				expect( formatTable( getData( model ) ) ).to.equal( formattedModelTable( [
-					[ '00', '01' ],
-					[ '[]10', '11' ],
-					[ '20', '21' ]
-				], { headingColumns: 1 } ) );
-			}
-		);
+		it( 'should set heading columns attribute below current selection column', () => {
+			setData( model, modelTable( [
+				[ '00', '01[]', '02', '03' ]
+			], { headingColumns: 3 } ) );
+
+			command.execute();
+
+			expect( formatTable( getData( model ) ) ).to.equal( formattedModelTable( [
+				[ '00', '01[]', '02', '03' ]
+			], { headingColumns: 1 } ) );
+		} );
 
 		it( 'should toggle of selected column', () => {
 			setData( model, modelTable( [
-				[ '00', '01' ],
-				[ '10', '11[]' ],
-				[ '20', '21' ]
+				[ '00', '01[]', '02', '03' ]
 			], { headingColumns: 2 } ) );
 
 			command.execute();
 
 			expect( formatTable( getData( model ) ) ).to.equal( formattedModelTable( [
-				[ '00', '01' ],
-				[ '10', '11[]' ],
-				[ '20', '21' ]
+				[ '00', '01[]', '02', '03' ]
 			], { headingColumns: 1 } ) );
 
 			command.execute();
 
 			expect( formatTable( getData( model ) ) ).to.equal( formattedModelTable( [
-				[ '00', '01' ],
-				[ '10', '11[]' ],
-				[ '20', '21' ]
+				[ '00', '01[]', '02', '03' ]
 			], { headingColumns: 2 } ) );
 		} );
 	} );

+ 57 - 32
packages/ckeditor5-table/tests/commands/setheaderrowcommand.js

@@ -135,60 +135,85 @@ describe( 'HeaderRowCommand', () => {
 	describe( 'execute()', () => {
 		it( 'should set heading rows attribute that cover row in which is selection', () => {
 			setData( model, modelTable( [
-				[ '00', '01' ],
-				[ '[]10', '11' ],
-				[ '20', '21' ]
+				[ '00' ],
+				[ '[]10' ],
+				[ '20' ],
+				[ '30' ]
 			] ) );
 
 			command.execute();
 
 			expect( formatTable( getData( model ) ) ).to.equal( formattedModelTable( [
-				[ '00', '01' ],
-				[ '[]10', '11' ],
-				[ '20', '21' ]
+				[ '00' ],
+				[ '[]10' ],
+				[ '20' ],
+				[ '30' ]
 			], { headingRows: 2 } ) );
 		} );
 
 		it( 'should toggle heading rows attribute', () => {
 			setData( model, modelTable( [
-				[ '[]00', '01' ],
-				[ '10', '11' ],
-				[ '20', '21' ]
-			] ) );
+				[ '00' ],
+				[ '[]10' ],
+				[ '20' ],
+				[ '30' ]
+			], { headingRows: 2 } ) );
 
 			command.execute();
 
 			expect( formatTable( getData( model ) ) ).to.equal( formattedModelTable( [
-				[ '[]00', '01' ],
-				[ '10', '11' ],
-				[ '20', '21' ]
+				[ '00' ],
+				[ '[]10' ],
+				[ '20' ],
+				[ '30' ]
 			], { headingRows: 1 } ) );
 
 			command.execute();
 
 			setData( model, modelTable( [
-				[ '[]00', '01' ],
-				[ '10', '11' ],
-				[ '20', '21' ]
-			] ) );
+				[ '00' ],
+				[ '[]10' ],
+				[ '20' ],
+				[ '30' ]
+			], { headingRows: 2 } ) );
 		} );
 
 		it( 'should set heading rows attribute if currently selected row is a heading so the heading section is below this row', () => {
 			setData( model, modelTable( [
-				[ '00', '01' ],
-				[ '[]10', '11' ],
-				[ '20', '21' ]
-			], { headingRows: 2 } ) );
+				[ '00' ],
+				[ '[]10' ],
+				[ '20' ],
+				[ '30' ]
+			], { headingRows: 3 } ) );
 
 			command.execute();
 
 			expect( formatTable( getData( model ) ) ).to.equal( formattedModelTable( [
-				[ '00', '01' ],
-				[ '[]10', '11' ],
-				[ '20', '21' ]
+				[ '00' ],
+				[ '[]10' ],
+				[ '20' ],
+				[ '30' ]
 			], { headingRows: 1 } ) );
 		} );
 
+		it( 'should unsetset heading rows attribute', () => {
+			setData( model, modelTable( [
+				[ '[]00' ],
+				[ '10' ],
+				[ '20' ],
+				[ '30' ]
+			], { headingRows: 3 } ) );
+
+			command.execute();
+
+			expect( formatTable( getData( model ) ) ).to.equal( formattedModelTable( [
+				[ '[]00' ],
+				[ '10' ],
+				[ '20' ],
+				[ '30' ]
+			] ) );
+		} );
+
 		it( 'should fix rowspaned cells on the edge of an table head section', () => {
 			setData( model, modelTable( [
 				[ '00', '01', '02' ],
@@ -229,29 +254,29 @@ describe( 'HeaderRowCommand', () => {
 
 		it( 'should fix rowspaned cells on the edge of an table head section when creating section', () => {
 			setData( model, modelTable( [
-				[ { rowspan: 2, contents: '[]00' }, '01' ],
-				[ '11' ]
+				[ { rowspan: 2, contents: '00' }, '01' ],
+				[ '[]11' ]
 			], { headingRows: 2 } ) );
 
 			command.execute();
 
 			expect( formatTable( getData( model ) ) ).to.equal( formattedModelTable( [
-				[ '[]00', '01' ],
-				[ '', '11' ]
+				[ '00', '01' ],
+				[ '', '[]11' ]
 			], { headingRows: 1 } ) );
 		} );
 
 		it( 'should fix rowspaned cells inside a row', () => {
 			setData( model, modelTable( [
-				[ '00', { rowspan: 2, contents: '[]01' } ],
-				[ '10' ]
+				[ '00', { rowspan: 2, contents: '01' } ],
+				[ '[]10' ]
 			], { headingRows: 2 } ) );
 
 			command.execute();
 
 			expect( formatTable( getData( model ) ) ).to.equal( formattedModelTable( [
-				[ '00', '[]01' ],
-				[ '10', '' ]
+				[ '00', '01' ],
+				[ '[]10', '' ]
 			], { headingRows: 1 } ) );
 		} );
 

+ 27 - 0
packages/ckeditor5-table/tests/converters/downcast.js

@@ -1053,6 +1053,33 @@ describe( 'downcast converters', () => {
 			], { headingRows: 2 } ) );
 		} );
 
+		it( 'should work with adding a table row and expanding heading', () => {
+			setModelData( model, modelTable( [
+				[ '00', '01' ],
+				[ '10', '11' ],
+				[ '20', '21' ]
+			], { headingRows: 1 } ) );
+
+			const table = root.getChild( 0 );
+
+			model.change( writer => {
+				writer.setAttribute( 'headingRows', 2, table );
+
+				const tableRow = writer.createElement( 'tableRow' );
+
+				writer.insert( tableRow, table, 1 );
+				writer.insertElement( 'tableCell', tableRow, 'end' );
+				writer.insertElement( 'tableCell', tableRow, 'end' );
+			} );
+
+			expect( formatTable( getViewData( viewDocument, { withoutSelection: true } ) ) ).to.equal( formattedViewTable( [
+				[ '00', '01' ],
+				[ '', '' ],
+				[ '10', '11' ],
+				[ '20', '21' ]
+			], { headingRows: 2 } ) );
+		} );
+
 		describe( 'asWidget', () => {
 			beforeEach( () => {
 				return VirtualTestEditor.create()