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

Commit

Permalink
Merge pull request #93 from ckeditor/t/92
Browse files Browse the repository at this point in the history
Fix: Downcast converter for table attributes should work with not converted child elements. Closes #92.
  • Loading branch information
scofalik authored Jul 27, 2018
2 parents bce6766 + cccc774 commit a3ea18d
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 2 deletions.
16 changes: 14 additions & 2 deletions src/converters/downcast.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 ) {
Expand Down Expand Up @@ -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
Expand All @@ -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 ) );
}
}
}

Expand Down
27 changes: 27 additions & 0 deletions tests/converters/downcast.js
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down

0 comments on commit a3ea18d

Please sign in to comment.