Merge pull request #28 from ckeditor/t/7

Fix: Image will be inserted after the block if the selection is placed at the block's end. Closes #7.

Author: Reinmar

src/imageuploadcommand.js

@@ -44,20 +44,7 @@ export default class ImageUploadCommand extends Command {
 		}
 
 		doc.enqueueChanges( () => {
-			let insertPosition;
-			const selectedElement = selection.getSelectedElement();
-
-			// If selected element is placed directly in root - put image after it.
-			if ( selectedElement && selectedElement.parent.is( 'rootElement' ) ) {
-				insertPosition = ModelPosition.createAfter( selectedElement );
-			} else {
-				// If selection is inside some block - put image before it.
-				const firstBlock = doc.selection.getSelectedBlocks().next().value;
-
-				if ( firstBlock ) {
-					insertPosition = ModelPosition.createBefore( firstBlock );
-				}
-			}
+			const insertPosition = getInsertionPosition( doc );
 
 			// No position to insert.
 			if ( !insertPosition ) {
@@ -77,3 +64,33 @@ export default class ImageUploadCommand extends Command {
 		} );
 	}
 }
+
+/**
+ * Returns correct image insertion position.
+ *
+ * @param {module:engine/model/document~Document} doc
+ * @returns {module:engine/model/position~Position|undefined}
+ */
+function getInsertionPosition( doc ) {
+	const selection = doc.selection;
+	const selectedElement = selection.getSelectedElement();
+
+	// If selected element is placed directly in root - return position after that element.
+	if ( selectedElement && selectedElement.parent.is( 'rootElement' ) ) {
+		return ModelPosition.createAfter( selectedElement );
+	}
+
+	const firstBlock = doc.selection.getSelectedBlocks().next().value;
+
+	if ( firstBlock ) {
+		const positionAfter = ModelPosition.createAfter( firstBlock );
+
+		// If selection is at the end of the block - return position after the block.
+		if ( selection.focus.isTouching( positionAfter ) ) {
+			return positionAfter;
+		}
+
+		// Otherwise return position before the block.
+		return ModelPosition.createBefore( firstBlock );
+	}
+}

tests/imageuploadcommand.js

@@ -39,11 +39,31 @@ describe( 'ImageUploadCommand', () => {
 
 	describe( '_doExecute', () => {
 		it( 'should insert image', () => {
+			const file = createNativeFileMock();
+			setModelData( document, '<paragraph>[]foo</paragraph>' );
+
+			command._doExecute( { file } );
+
+			const id = fileRepository.getLoader( file ).id;
+			expect( getModelData( document ) ).to.equal( `[<image uploadId="${ id }"></image>]<paragraph>foo</paragraph>` );
+		} );
+
+		it( 'should insert image after block if selection is at its end', () => {
 			const file = createNativeFileMock();
 			setModelData( document, '<paragraph>foo[]</paragraph>' );
 
 			command._doExecute( { file } );
 
+			const id = fileRepository.getLoader( file ).id;
+			expect( getModelData( document ) ).to.equal( `<paragraph>foo</paragraph>[<image uploadId="${ id }"></image>]` );
+		} );
+
+		it( 'should insert image before block if selection is in the middle', () => {
+			const file = createNativeFileMock();
+			setModelData( document, '<paragraph>f{}oo</paragraph>' );
+
+			command._doExecute( { file } );
+
 			const id = fileRepository.getLoader( file ).id;
 			expect( getModelData( document ) ).to.equal( `[<image uploadId="${ id }"></image>]<paragraph>foo</paragraph>` );
 		} );
@@ -87,7 +107,7 @@ describe( 'ImageUploadCommand', () => {
 			const file = createNativeFileMock();
 			const spy = sinon.spy( batch, 'insert' );
 
-			setModelData( document, '<paragraph>foo[]</paragraph>' );
+			setModelData( document, '<paragraph>[]foo</paragraph>' );
 
 			command._doExecute( { batch, file } );
 			const id = fileRepository.getLoader( file ).id;

tests/imageuploadengine.js

@@ -61,7 +61,7 @@ describe( 'ImageUploadEngine', () => {
 		const spy = sinon.spy( editor, 'execute' );
 		const fileMock = createNativeFileMock();
 		const dataTransfer = new DataTransfer( { files: [ fileMock ] } );
-		setModelData( document, '<paragraph>foo bar baz[]</paragraph>' );
+		setModelData( document, '<paragraph>[]foo bar baz</paragraph>' );
 
 		viewDocument.fire( 'clipboardInput', { dataTransfer } );
 

tests/imageuploadprogress.js

@@ -53,7 +53,7 @@ describe( 'ImageUploadProgress', () => {
 	} );
 
 	it( 'should convert image\'s "reading" uploadStatus attribute', () => {
-		setModelData( document, '<paragraph>foo[]</paragraph>' );
+		setModelData( document, '<paragraph>[]foo</paragraph>' );
 		editor.execute( 'imageUpload', { file: createNativeFileMock() } );
 
 		expect( getViewData( viewDocument ) ).to.equal(
@@ -64,7 +64,7 @@ describe( 'ImageUploadProgress', () => {
 	} );
 
 	it( 'should convert image\'s "uploading" uploadStatus attribute', ( done ) => {
-		setModelData( document, '<paragraph>foo[]</paragraph>' );
+		setModelData( document, '<paragraph>[]foo</paragraph>' );
 		editor.execute( 'imageUpload', { file: createNativeFileMock() } );
 
 		document.once( 'changesDone', () => {
@@ -82,7 +82,7 @@ describe( 'ImageUploadProgress', () => {
 	} );
 
 	it( 'should update progressbar width on progress', ( done ) => {
-		setModelData( document, '<paragraph>foo[]</paragraph>' );
+		setModelData( document, '<paragraph>[]foo</paragraph>' );
 		editor.execute( 'imageUpload', { file: createNativeFileMock() } );
 
 		document.once( 'changesDone', () => {
@@ -102,7 +102,7 @@ describe( 'ImageUploadProgress', () => {
 	} );
 
 	it( 'should convert image\'s "complete" uploadStatus attribute', ( done ) => {
-		setModelData( document, '<paragraph>foo[]</paragraph>' );
+		setModelData( document, '<paragraph>[]foo</paragraph>' );
 		editor.execute( 'imageUpload', { file: createNativeFileMock() } );
 
 		document.once( 'changesDone', () => {
@@ -126,7 +126,7 @@ describe( 'ImageUploadProgress', () => {
 		const uploadProgress = editor.plugins.get( ImageUploadProgress );
 		uploadProgress.placeholder = base64Sample;
 
-		setModelData( document, '<paragraph>foo[]</paragraph>' );
+		setModelData( document, '<paragraph>[]foo</paragraph>' );
 		editor.execute( 'imageUpload', { file: createNativeFileMock() } );
 
 		expect( getViewData( viewDocument ) ).to.equal(
@@ -141,7 +141,7 @@ describe( 'ImageUploadProgress', () => {
 			consumable.consume( data.item, eventNameToConsumableType( evt.name ) );
 		}, { priority: 'highest' } );
 
-		setModelData( document, '<paragraph>foo[]</paragraph>' );
+		setModelData( document, '<paragraph>[]foo</paragraph>' );
 		editor.execute( 'imageUpload', { file: createNativeFileMock() } );
 
 		expect( getViewData( viewDocument ) ).to.equal(