Browse Source

Fix: Change table attribute should only check existing view elements in the view.

Maciej Gołaszewski 7 years ago
parent
commit
eb8843a0cf

+ 3 - 1
packages/ckeditor5-table/src/converters/downcasttable.js

@@ -200,7 +200,9 @@ export function downcastAttributeChange( attribute ) {
 
 				const cell = conversionApi.mapper.toViewElement( tableCell );
 
-				if ( cell.name !== cellElementName ) {
+				// If in single change we're converting attribute changes and inserting cell the table cell might not be inserted into view
+				// because of child conversion is done after parent.
+				if ( cell && cell.name !== cellElementName ) {
 					conversionApi.writer.rename( cell, cellElementName );
 				}
 

+ 43 - 44
packages/ckeditor5-table/tests/converters/downcasttable.js

@@ -59,8 +59,8 @@ describe( 'downcastTable()', () => {
 				conversion.for( 'downcast' ).add( downcastInsertRow() );
 				conversion.for( 'downcast' ).add( downcastInsertCell() );
 
-				conversion.for( 'downcast' ).add( downcastAttributeChange( 'headingRows' ), { priority: 'low' } );
-				conversion.for( 'downcast' ).add( downcastAttributeChange( 'headingColumns' ), { priority: 'low' } );
+				conversion.for( 'downcast' ).add( downcastAttributeChange( 'headingRows' ) );
+				conversion.for( 'downcast' ).add( downcastAttributeChange( 'headingColumns' ), { priority: 'log' } );
 			} );
 	} );
 
@@ -460,48 +460,6 @@ describe( 'downcastTable()', () => {
 			] ) );
 		} );
 
-		// TODO: something broke after adding attribute converter :/
-		it.skip( 'should work with heading columns', () => {
-			setModelData( model, modelTable( [
-				[ { rowspan: 2, contents: '11' }, '12', '13', '14' ],
-				[ '22', '23', '24' ],
-				[ { colspan: 2, contents: '31' }, '33', '34' ]
-			], { headingColumns: 2 } ) );
-
-			const table = root.getChild( 0 );
-
-			model.change( writer => {
-				// Inserting column in heading columns so update table's attribute also
-				writer.setAttribute( 'headingColumns', 3, table );
-
-				writer.insertElement( 'tableCell', table.getChild( 0 ), 2 );
-				writer.insertElement( 'tableCell', table.getChild( 1 ), 1 );
-				writer.insertElement( 'tableCell', table.getChild( 2 ), 1 );
-			} );
-
-			expect( formatModelTable( getViewData( viewDocument, { withoutSelection: true } ) ) ).to.equal( formattedViewTable( [
-				[
-					{ isHeading: true, rowspan: 2, contents: '11' },
-					{ isHeading: true, contents: '12' },
-					{ isHeading: true, contents: '' },
-					'13',
-					'14'
-				],
-				[
-					{ isHeading: true, contents: '22' },
-					{ isHeading: true, contents: '' },
-					'23',
-					'24'
-				],
-				[
-					{ isHeading: true, colspan: 2, contents: '31' },
-					{ isHeading: true, contents: '' },
-					'33',
-					'34'
-				]
-			] ) );
-		} );
-
 		it( 'split cell simulation - simple', () => {
 			setModelData( model, modelTable( [
 				[ '11', '12' ],
@@ -712,5 +670,46 @@ describe( 'downcastTable()', () => {
 				'<table headingColumns="1"><tbody><tr><td>11</td></tr></tbody></table>'
 			);
 		} );
+
+		it( 'should work with adding table cells', () => {
+			setModelData( model, modelTable( [
+				[ { rowspan: 2, contents: '11' }, '12', '13', '14' ],
+				[ '22', '23', '24' ],
+				[ { colspan: 2, contents: '31' }, '33', '34' ]
+			], { headingColumns: 2 } ) );
+
+			const table = root.getChild( 0 );
+
+			model.change( writer => {
+				// Inserting column in heading columns so update table's attribute also
+				writer.setAttribute( 'headingColumns', 3, table );
+
+				writer.insertElement( 'tableCell', table.getChild( 0 ), 2 );
+				writer.insertElement( 'tableCell', table.getChild( 1 ), 1 );
+				writer.insertElement( 'tableCell', table.getChild( 2 ), 1 );
+			} );
+
+			expect( formatModelTable( getViewData( viewDocument, { withoutSelection: true } ) ) ).to.equal( formattedViewTable( [
+				[
+					{ isHeading: true, rowspan: 2, contents: '11' },
+					{ isHeading: true, contents: '12' },
+					{ isHeading: true, contents: '' },
+					'13',
+					'14'
+				],
+				[
+					{ isHeading: true, contents: '22' },
+					{ isHeading: true, contents: '' },
+					'23',
+					'24'
+				],
+				[
+					{ isHeading: true, colspan: 2, contents: '31' },
+					{ isHeading: true, contents: '' },
+					'33',
+					'34'
+				]
+			] ) );
+		} );
 	} );
 } );