Merge pull request #1536 from ckeditor/t/1521

Other: Swapped parameters order in the `DowncastWriter#rename()` method. The `DowncastWriter#remove()` method now accepts range or item. Closes #1521.

BREAKING CHANGE: Swapped parameters order in the `DowncastWriter` (former `engine\view\writer`) `rename()`  method. See #1521.

Author: scofalik

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.
@@ -905,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 );

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( '<container:p>[foobar]</container:p>', '<container:p></container:p>', 'foobar', true );
+		} );
+
+		it( 'should not leave empty text nodes (as item)', () => {
+			test( '<container:p>{foobar}</container:p>', '<container:p></container:p>', 'foobar', true );
+		} );
+
+		it( 'should remove part of the text node (as item)', () => {
+			test( '<container:p>f{oob}ar</container:p>', '<container:p>far</container:p>', 'oob', true );
+		} );
+
+		it( 'should merge after removing (as item)', () => {
+			test(
+				'<container:p>' +
+					'<attribute:b view-priority="1">foo</attribute:b>[bar]<attribute:b view-priority="1">bazqux</attribute:b>' +
+				'</container:p>',
+				'<container:p><attribute:b view-priority="1">foobazqux</attribute:b></container:p>',
+				'bar',
+				true
+			);
+		} );
+
+		it( 'should remove EmptyElement (as item)', () => {
+			test(
+				'<container:p>foo[<empty:img></empty:img>]bar</container:p>',
+				'<container:p>foobar</container:p>',
+				'<empty:img></empty:img>',
+				true
+			);
+		} );
+
+		it( 'should remove UIElement (as item)', () => {
+			test(
+				'<container:p>foo[<ui:span></ui:span>]bar</container:p>',
+				'<container:p>foobar</container:p>',
+				'<ui:span></ui:span>',
+				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' );
+		} );
 	} );
 } );

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 ) );
 		} );