Skip to content

Commit

Permalink
Merge pull request #7373 from ckeditor/i/6819
Browse files Browse the repository at this point in the history
Other (engine): Added Writer#cloneElement(). Closes #6819.

Internal (table): Replaced Element._clone() usages with Writer#cloneElement().
  • Loading branch information
jodator authored Jun 4, 2020
2 parents fd31445 + fda2593 commit 4c71140
Show file tree
Hide file tree
Showing 6 changed files with 51 additions and 4 deletions.
2 changes: 1 addition & 1 deletion packages/ckeditor5-engine/src/model/element.js
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ export default class Element extends Node {

/**
* Creates a copy of this element and returns it. Created element has the same name and attributes as the original element.
* If clone is deep, the original element's children are also cloned. If not, then empty element is removed.
* If clone is deep, the original element's children are also cloned. If not, then empty element is returned.
*
* @protected
* @param {Boolean} [deep=false] If set to `true` clones element and all its children recursively. When set to `false`,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ export default function getSelectedContent( model, selection ) {
if ( item.is( 'textProxy' ) ) {
writer.appendText( item.data, item.getAttributes(), frag );
} else {
writer.append( item._clone( true ), frag );
writer.append( writer.cloneElement( item, true ), frag );
}
}

Expand Down
12 changes: 12 additions & 0 deletions packages/ckeditor5-engine/src/model/writer.js
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,18 @@ export default class Writer {
return new DocumentFragment();
}

/**
* Creates a copy of the element and returns it. Created element has the same name and attributes as the original element.
* If clone is deep, the original element's children are also cloned. If not, then empty element is returned.
*
* @param {module:engine/model/element~Element} element The element to clone.
* @param {Boolean} [deep=true] If set to `true` clones element and all its children recursively. When set to `false`,
* element will be cloned without any child.
*/
cloneElement( element, deep = true ) {
return element._clone( deep );
}

/**
* Inserts item on given position.
*
Expand Down
35 changes: 35 additions & 0 deletions packages/ckeditor5-engine/tests/model/writer.js
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,35 @@ describe( 'Writer', () => {
} );
} );

describe( 'cloneElement()', () => {
it( 'should make deep copy of element', () => {
const element = createElement( 'foo', { 'abc': '123' } );

insertElement( createElement( 'bar', { 'xyz': '789' } ), element );

const clonedElement = cloneElement( element );

expect( clonedElement ).to.not.equal( element );
expect( clonedElement.getChild( 0 ) ).to.not.equal( element.getChild( 0 ) );
expect( clonedElement.toJSON() ).to.deep.equal( element.toJSON() );
} );

it( 'should make shallow copy of element', () => {
const element = createElement( 'foo', { 'abc': '123' } );

insertElement( createElement( 'bar', { 'xyz': '789' } ), element );

const elementJson = element.toJSON();
delete elementJson.children;

const clonedElement = cloneElement( element, false );

expect( clonedElement ).to.not.equal( element );
expect( clonedElement.childCount ).to.equal( 0 );
expect( clonedElement.toJSON() ).to.deep.equal( elementJson );
} );
} );

describe( 'insert()', () => {
it( 'should insert node at given position', () => {
const parent = createDocumentFragment();
Expand Down Expand Up @@ -2902,6 +2931,12 @@ describe( 'Writer', () => {
} );
}

function cloneElement( element, deep ) {
return model.change( writer => {
return writer.cloneElement( element, deep );
} );
}

function insert( item, itemOrPosition, offset ) {
model.enqueueChange( batch, writer => {
writer.insert( item, itemOrPosition, offset );
Expand Down
2 changes: 1 addition & 1 deletion packages/ckeditor5-table/src/tableclipboard.js
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ function replaceSelectedCellsWithPasted( pastedTable, pastedDimensions, selected

// Clone cell to insert (to duplicate its attributes and children).
// Cloning is required to support repeating pasted table content when inserting to a bigger selection.
const cellToInsert = pastedCell._clone( true );
const cellToInsert = writer.cloneElement( pastedCell );

// Trim the cell if it's row/col-spans would exceed selection area.
trimTableCellIfNeeded( cellToInsert, row, column, selection.lastRow, selection.lastColumn, writer );
Expand Down
2 changes: 1 addition & 1 deletion packages/ckeditor5-table/src/utils/structure.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ export function cropTableToDimensions( sourceTable, cropDimensions, writer ) {
}
// Otherwise clone the cell with all children and trim if it exceeds cropped area.
else {
const tableCellCopy = tableCell._clone( true );
const tableCellCopy = writer.cloneElement( tableCell );

writer.append( tableCellCopy, row );

Expand Down

0 comments on commit 4c71140

Please sign in to comment.