From 7be0e23b7c3862966e83f90d66b2190eb69b203d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Krzto=C5=84?= Date: Mon, 10 Sep 2018 13:05:11 +0200 Subject: [PATCH 1/2] The 'DowncastWriter#remove()' method now accepts range or item. --- src/view/downcastwriter.js | 7 +++- tests/view/downcastwriter/remove.js | 58 +++++++++++++++++++++++++++-- 2 files changed, 60 insertions(+), 5 deletions(-) diff --git a/src/view/downcastwriter.js b/src/view/downcastwriter.js index 6bdc67c5b..3fae5e8a9 100644 --- a/src/view/downcastwriter.js +++ b/src/view/downcastwriter.js @@ -651,11 +651,14 @@ export default class DowncastWriter { * {@link module:engine/view/range~Range#start start} and {@link module:engine/view/range~Range#end end} positions are not placed inside * same parent container. * - * @param {module:engine/view/range~Range} range Range to remove from container. After removing, it will be updated + * @param {module:engine/view/range~Range|module:engine/view/item~Item} rangeOrItem Range to remove from container + * or an {@link module:engine/view/item~Item item} to remove. If range is provided, after removing, it will be updated * to a collapsed range showing the new position. * @returns {module:engine/view/documentfragment~DocumentFragment} Document fragment containing removed nodes. */ - remove( range ) { + remove( rangeOrItem ) { + const range = rangeOrItem instanceof Range ? rangeOrItem : Range.createOn( rangeOrItem ); + validateRangeContainer( range ); // If range is collapsed - nothing to remove. diff --git a/tests/view/downcastwriter/remove.js b/tests/view/downcastwriter/remove.js index e3e2ec479..0dd573505 100644 --- a/tests/view/downcastwriter/remove.js +++ b/tests/view/downcastwriter/remove.js @@ -24,12 +24,15 @@ describe( 'DowncastWriter', () => { // @param {String} input // @param {String} expectedResult // @param {String} expectedRemoved - function test( input, expectedResult, expectedRemoved ) { + // @param {Boolean} asItem + function test( input, expectedResult, expectedRemoved, asItem = false ) { const { view, selection } = parse( input ); const range = selection.getFirstRange(); - const removed = writer.remove( range ); - expect( stringify( view, range, { showType: true, showPriority: true } ) ).to.equal( expectedResult ); + const rangeOrItem = asItem ? Array.from( range.getItems() )[ 0 ] : range; + const removed = writer.remove( rangeOrItem ); + + expect( stringify( view, asItem ? null : range, { showType: true, showPriority: true } ) ).to.equal( expectedResult ); expect( stringify( removed, null, { showType: true, showPriority: true } ) ).to.equal( expectedRemoved ); } @@ -153,5 +156,54 @@ describe( 'DowncastWriter', () => { writer.remove( range ); } ).to.throw( CKEditorError, 'view-writer-cannot-break-ui-element' ); } ); + + it( 'should remove single text node (as item)', () => { + test( '[foobar]', '', 'foobar', true ); + } ); + + it( 'should not leave empty text nodes (as item)', () => { + test( '{foobar}', '', 'foobar', true ); + } ); + + it( 'should remove part of the text node (as item)', () => { + test( 'f{oob}ar', 'far', 'oob', true ); + } ); + + it( 'should merge after removing (as item)', () => { + test( + '' + + 'foo[bar]bazqux' + + '', + 'foobazqux', + 'bar', + true + ); + } ); + + it( 'should remove EmptyElement (as item)', () => { + test( + 'foo[]bar', + 'foobar', + '', + true + ); + } ); + + it( 'should remove UIElement (as item)', () => { + test( + 'foo[]bar', + 'foobar', + '', + true + ); + } ); + + it( 'should throw when item has no parent container', () => { + const el = new AttributeElement( 'b' ); + + expect( () => { + writer.remove( el ); + } ).to.throw( CKEditorError, 'view-position-before-root' ); + } ); } ); } ); From e50028e7818531c50f4217429c1f40d7f2086ac8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Krzto=C5=84?= Date: Mon, 10 Sep 2018 13:06:50 +0200 Subject: [PATCH 2/2] 'DowncastWriter#rename()' method params order changed. --- src/view/downcastwriter.js | 4 ++-- tests/view/downcastwriter/rename.js | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/view/downcastwriter.js b/src/view/downcastwriter.js index 3fae5e8a9..f3651fc66 100644 --- a/src/view/downcastwriter.js +++ b/src/view/downcastwriter.js @@ -908,10 +908,10 @@ export default class DowncastWriter { * * Since this function creates a new element and removes the given one, the new element is returned to keep reference. * - * @param {module:engine/view/containerelement~ContainerElement} viewElement Element to be renamed. * @param {String} newName New name for element. + * @param {module:engine/view/containerelement~ContainerElement} viewElement Element to be renamed. */ - rename( viewElement, newName ) { + rename( newName, viewElement ) { const newElement = new ContainerElement( newName, viewElement.getAttributes() ); this.insert( Position.createAfter( viewElement ), newElement ); diff --git a/tests/view/downcastwriter/rename.js b/tests/view/downcastwriter/rename.js index 78f36c8a2..951e112bb 100644 --- a/tests/view/downcastwriter/rename.js +++ b/tests/view/downcastwriter/rename.js @@ -24,7 +24,7 @@ describe( 'DowncastWriter', () => { it( 'should rename given element by inserting a new element in the place of the old one', () => { const text = foo.getChild( 0 ); - writer.rename( foo, 'bar' ); + writer.rename( 'bar', foo ); const bar = root.getChild( 0 ); @@ -35,7 +35,7 @@ describe( 'DowncastWriter', () => { } ); it( 'should return a reference to the inserted element', () => { - const bar = writer.rename( foo, 'bar' ); + const bar = writer.rename( 'bar', foo ); expect( bar ).to.equal( root.getChild( 0 ) ); } );