8
0
Просмотр исходного кода

Merge pull request #47 from ckeditor/t/42

Fix: Fixed two issues related to dropping images. First, when dropping a file into an empty paragraph, that paragraph should be replaced with that image. Second, drop position should be read correctly when the editor is focused upon drop. Closes #42. Closes #29.

BREAKING CHANGE: `UploadImageCommand` doesn't optimize the drop position itself anymore. Instead, a separate `findOptimalInsertionPosition()` function was introduced.

BREAKING CHANGE: `UploadImageCommand` doesn't verify the type of file anymore. This needs to be done by the caller.
Piotrek Koszuliński 8 лет назад
Родитель
Сommit
58c7356f02

+ 6 - 1
packages/ckeditor5-upload/src/imageuploadbutton.js

@@ -11,6 +11,7 @@ import Plugin from '@ckeditor/ckeditor5-core/src/plugin';
 import ImageUploadEngine from './imageuploadengine';
 import FileDialogButtonView from './ui/filedialogbuttonview';
 import imageIcon from '@ckeditor/ckeditor5-core/theme/icons/image.svg';
+import { isImageType, findOptimalInsertionPosition } from './utils';
 
 /**
  * Image upload button plugin.
@@ -50,7 +51,11 @@ export default class ImageUploadButton extends Plugin {
 
 			view.on( 'done', ( evt, files ) => {
 				for ( const file of files ) {
-					editor.execute( 'imageUpload', { file } );
+					const insertAt = findOptimalInsertionPosition( editor.document.selection );
+
+					if ( isImageType( file ) ) {
+						editor.execute( 'imageUpload', { file, insertAt } );
+					}
 				}
 			} );
 

+ 17 - 43
packages/ckeditor5-upload/src/imageuploadcommand.js

@@ -3,13 +3,10 @@
  * For licensing, see LICENSE.md.
  */
 
-import ModelDocumentFragment from '@ckeditor/ckeditor5-engine/src/model/documentfragment';
 import ModelElement from '@ckeditor/ckeditor5-engine/src/model/element';
 import ModelRange from '@ckeditor/ckeditor5-engine/src/model/range';
-import ModelPosition from '@ckeditor/ckeditor5-engine/src/model/position';
 import ModelSelection from '@ckeditor/ckeditor5-engine/src/model/selection';
 import FileRepository from './filerepository';
-import { isImageType } from './utils';
 import Command from '@ckeditor/ckeditor5-core/src/command';
 
 /**
@@ -29,7 +26,9 @@ export default class ImageUploadCommand extends Command {
 	 * @param {Object} options Options for executed command.
 	 * @param {File} options.file Image file to upload.
 	 * @param {module:engine/model/position~Position} [options.insertAt] Position at which the image should be inserted.
-	 * If the position won't be specified the image will be inserted next to the selection.
+	 * If the position is not specified the image will be inserted into the current selection.
+	 * Note: You can use the {@link module:upload/utils~findOptimalInsertionPosition} function to calculate
+	 * (e.g. based on the current selection) a position which is more optimal from UX perspective.
 	 * @param {module:engine/model/batch~Batch} [options.batch] Batch to collect all the change steps.
 	 * New batch will be created if this option is not set.
 	 */
@@ -41,28 +40,25 @@ export default class ImageUploadCommand extends Command {
 		const selection = doc.selection;
 		const fileRepository = editor.plugins.get( FileRepository );
 
-		if ( !isImageType( file ) ) {
-			return;
-		}
-
 		doc.enqueueChanges( () => {
-			const insertAt = options.insertAt || getInsertionPosition( doc );
-
-			// No position to insert.
-			if ( !insertAt ) {
-				return;
-			}
-
 			const imageElement = new ModelElement( 'image', {
 				uploadId: fileRepository.createLoader( file ).id
 			} );
-			const documentFragment = new ModelDocumentFragment( [ imageElement ] );
-			const range = new ModelRange( insertAt );
-			const insertSelection = new ModelSelection();
 
-			insertSelection.setRanges( [ range ] );
-			editor.data.insertContent( documentFragment, insertSelection, batch );
-			selection.setRanges( [ ModelRange.createOn( imageElement ) ] );
+			let insertAtSelection;
+
+			if ( options.insertAt ) {
+				insertAtSelection = new ModelSelection( [ new ModelRange( options.insertAt ) ] );
+			} else {
+				insertAtSelection = doc.selection;
+			}
+
+			editor.data.insertContent( imageElement, insertAtSelection, batch );
+
+			// Inserting an image might've failed due to schema regulations.
+			if ( imageElement.parent ) {
+				selection.setRanges( [ ModelRange.createOn( imageElement ) ] );
+			}
 		} );
 	}
 }
@@ -71,26 +67,4 @@ export default class ImageUploadCommand extends Command {
 //
 // @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 );
-	}
-}

+ 13 - 2
packages/ckeditor5-upload/src/imageuploadengine.js

@@ -11,7 +11,8 @@ import Plugin from '@ckeditor/ckeditor5-core/src/plugin';
 import FileRepository from './filerepository';
 import ImageUploadCommand from './imageuploadcommand';
 import Notification from '@ckeditor/ckeditor5-ui/src/notification/notification';
-import { isImageType } from './utils';
+import ModelSelection from '@ckeditor/ckeditor5-engine/src/model/selection';
+import { isImageType, findOptimalInsertionPosition } from './utils';
 
 /**
  * Image upload engine plugin.
@@ -45,11 +46,21 @@ export default class ImageUploadEngine extends Plugin {
 
 		// Execute imageUpload command when image is dropped or pasted.
 		editor.editing.view.on( 'clipboardInput', ( evt, data ) => {
+			let targetModelSelection = new ModelSelection(
+				data.targetRanges.map( viewRange => editor.editing.mapper.toModelRange( viewRange ) )
+			);
+
 			for ( const file of data.dataTransfer.files ) {
+				const insertAt = findOptimalInsertionPosition( targetModelSelection );
+
 				if ( isImageType( file ) ) {
-					editor.execute( 'imageUpload', { file } );
+					editor.execute( 'imageUpload', { file, insertAt } );
 					evt.stop();
 				}
+
+				// Use target ranges only for the first image. Then, use that image position
+				// so we keep adding the next ones after the previous one.
+				targetModelSelection = doc.selection;
 			}
 		} );
 

+ 46 - 0
packages/ckeditor5-upload/src/utils.js

@@ -7,6 +7,8 @@
  * @module upload/utils
  */
 
+import ModelPosition from '@ckeditor/ckeditor5-engine/src/model/position';
+
 /**
  * Checks if given file is an image.
  *
@@ -19,3 +21,47 @@ export function isImageType( file ) {
 	return types.test( file.type );
 }
 
+/**
+ * Returns a model position which is optimal (in terms of UX) for inserting an image.
+ *
+ * For instance, if a selection is in a middle of a paragraph, position before this paragraph
+ * will be returned, so that it's not split. If the selection is at the end of a paragraph,
+ * position after this paragraph will be returned.
+ *
+ * Note: If selection is placed in an empty block, that block will be returned. If that position
+ * is then passed to {@link module:engine/controller/datacontroller~DataController#insertContent}
+ * that block will be fully replaced by the image.
+ *
+ * @param {module:engine/model/selection~Selection} selection Selection based on which the
+ * insertion position should be calculated.
+ * @returns {module:engine/model/position~Position} The optimal position.
+ */
+export function findOptimalInsertionPosition( selection ) {
+	const selectedElement = selection.getSelectedElement();
+
+	if ( selectedElement ) {
+		return ModelPosition.createAfter( selectedElement );
+	}
+
+	const firstBlock = selection.getSelectedBlocks().next().value;
+
+	if ( firstBlock ) {
+		// If inserting into an empty block – return position in that block. It will get
+		// replaced with the image by insertContent(). #42.
+		if ( firstBlock.isEmpty ) {
+			return ModelPosition.createAt( 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 );
+	}
+
+	return selection.focus;
+}

+ 76 - 6
packages/ckeditor5-upload/tests/imageuploadbutton.js

@@ -6,27 +6,50 @@
 /* globals document */
 
 import ClassicEditor from '@ckeditor/ckeditor5-editor-classic/src/classiceditor';
+
 import Image from '@ckeditor/ckeditor5-image/src/image';
 import FileDialogButtonView from '../src/ui/filedialogbuttonview';
+import FileRepository from '../src/filerepository';
 import ImageUploadButton from '../src/imageuploadbutton';
 import ImageUploadEngine from '../src/imageuploadengine';
-import { createNativeFileMock } from './_utils/mocks';
+import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph';
+import Notification from '@ckeditor/ckeditor5-ui/src/notification/notification';
+
+import { createNativeFileMock, AdapterMock } from './_utils/mocks';
+import { setData as setModelData, getData as getModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model';
 
 describe( 'ImageUploadButton', () => {
-	let editor;
+	let editor, doc, editorElement, fileRepository;
 
 	beforeEach( () => {
-		const editorElement = document.createElement( 'div' );
+		editorElement = document.createElement( 'div' );
 		document.body.appendChild( editorElement );
 
-		return ClassicEditor.create( editorElement, {
-			plugins: [ Image, ImageUploadButton ]
-		} )
+		return ClassicEditor
+			.create( editorElement, {
+				plugins: [ Paragraph, Image, ImageUploadButton, FileRepository ]
+			} )
 			.then( newEditor => {
 				editor = newEditor;
+				doc = editor.document;
+
+				fileRepository = editor.plugins.get( FileRepository );
+				fileRepository.createAdapter = loader => {
+					return new AdapterMock( loader );
+				};
+
+				// Hide all notifications (prevent alert() calls).
+				const notification = editor.plugins.get( Notification );
+				notification.on( 'show', evt => evt.stop() );
 			} );
 	} );
 
+	afterEach( () => {
+		editorElement.remove();
+
+		return editor.destroy();
+	} );
+
 	it( 'should include ImageUploadEngine', () => {
 		expect( editor.plugins.get( ImageUploadEngine ) ).to.be.instanceOf( ImageUploadEngine );
 	} );
@@ -60,5 +83,52 @@ describe( 'ImageUploadButton', () => {
 		expect( executeStub.firstCall.args[ 0 ] ).to.equal( 'imageUpload' );
 		expect( executeStub.firstCall.args[ 1 ].file ).to.equal( files[ 0 ] );
 	} );
+
+	it( 'should optimize the insertion position', () => {
+		const button = editor.ui.componentFactory.create( 'insertImage' );
+		const files = [ createNativeFileMock() ];
+
+		setModelData( doc, '<paragraph>f[]oo</paragraph>' );
+
+		button.fire( 'done', files );
+
+		const id = fileRepository.getLoader( files[ 0 ] ).id;
+
+		expect( getModelData( doc ) ).to.equal(
+			`[<image uploadId="${ id }" uploadStatus="reading"></image>]` +
+			'<paragraph>foo</paragraph>'
+		);
+	} );
+
+	it( 'should correctly insert multiple files', () => {
+		const button = editor.ui.componentFactory.create( 'insertImage' );
+		const files = [ createNativeFileMock(), createNativeFileMock() ];
+
+		setModelData( doc, '<paragraph>foo[]</paragraph><paragraph>bar</paragraph>' );
+
+		button.fire( 'done', files );
+
+		const id1 = fileRepository.getLoader( files[ 0 ] ).id;
+		const id2 = fileRepository.getLoader( files[ 1 ] ).id;
+
+		expect( getModelData( doc ) ).to.equal(
+			'<paragraph>foo</paragraph>' +
+			`<image uploadId="${ id1 }" uploadStatus="reading"></image>` +
+			`[<image uploadId="${ id2 }" uploadStatus="reading"></image>]` +
+			'<paragraph>bar</paragraph>'
+		);
+	} );
+
+	it( 'should not execute imageUpload if the file is not an image', () => {
+		const executeStub = sinon.stub( editor, 'execute' );
+		const button = editor.ui.componentFactory.create( 'insertImage' );
+		const file = {
+			type: 'media/mp3',
+			size: 1024
+		};
+
+		button.fire( 'done', [ file ] );
+		sinon.assert.notCalled( executeStub );
+	} );
 } );
 

+ 27 - 64
packages/ckeditor5-upload/tests/imageuploadcommand.js

@@ -14,7 +14,7 @@ import buildModelConverter from '@ckeditor/ckeditor5-engine/src/conversion/build
 import ModelPosition from '@ckeditor/ckeditor5-engine/src/model/position';
 
 describe( 'ImageUploadCommand', () => {
-	let editor, command, adapterMock, doc, fileRepository;
+	let editor, command, doc, fileRepository;
 
 	beforeEach( () => {
 		return VirtualTestEditor.create( {
@@ -25,9 +25,7 @@ describe( 'ImageUploadCommand', () => {
 			command = new ImageUploadCommand( editor );
 			fileRepository = editor.plugins.get( FileRepository );
 			fileRepository.createAdapter = loader => {
-				adapterMock = new AdapterMock( loader );
-
-				return adapterMock;
+				return new AdapterMock( loader );
 			};
 
 			doc = editor.document;
@@ -38,70 +36,58 @@ describe( 'ImageUploadCommand', () => {
 		} );
 	} );
 
+	afterEach( () => {
+		return editor.destroy();
+	} );
+
 	describe( 'execute()', () => {
-		it( 'should insert image', () => {
+		it( 'should insert image at selection position (includes deleting selected content)', () => {
 			const file = createNativeFileMock();
-			setModelData( doc, '<paragraph>[]foo</paragraph>' );
+			setModelData( doc, '<paragraph>f[o]o</paragraph>' );
 
 			command.execute( { file } );
 
 			const id = fileRepository.getLoader( file ).id;
-			expect( getModelData( doc ) ).to.equal( `[<image uploadId="${ id }"></image>]<paragraph>foo</paragraph>` );
+			expect( getModelData( doc ) )
+				.to.equal( `<paragraph>f</paragraph>[<image uploadId="${ id }"></image>]<paragraph>o</paragraph>` );
 		} );
 
-		it( 'should insert image after block if selection is at its end', () => {
+		it( 'should insert directly at specified position (options.insertAt)', () => {
 			const file = createNativeFileMock();
-			setModelData( doc, '<paragraph>foo[]</paragraph>' );
+			setModelData( doc, '<paragraph>f[]oo</paragraph>' );
 
-			command.execute( { file } );
-
-			const id = fileRepository.getLoader( file ).id;
-			expect( getModelData( doc ) ).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( doc, '<paragraph>f{}oo</paragraph>' );
+			const insertAt = new ModelPosition( doc.getRoot(), [ 0, 2 ] ); // fo[]o
 
-			command.execute( { file } );
+			command.execute( { file, insertAt } );
 
 			const id = fileRepository.getLoader( file ).id;
-			expect( getModelData( doc ) ).to.equal( `[<image uploadId="${ id }"></image>]<paragraph>foo</paragraph>` );
+			expect( getModelData( doc ) )
+				.to.equal( `<paragraph>fo</paragraph>[<image uploadId="${ id }"></image>]<paragraph>o</paragraph>` );
 		} );
 
-		it( 'should insert image after other image', () => {
+		it( 'should allow to provide batch instance (options.batch)', () => {
+			const batch = doc.batch();
 			const file = createNativeFileMock();
-			setModelData( doc, '[<image src="image.png"></image>]' );
+			const spy = sinon.spy( batch, 'insert' );
 
-			command.execute( { file } );
+			setModelData( doc, '<paragraph>[]foo</paragraph>' );
 
+			command.execute( { batch, file } );
 			const id = fileRepository.getLoader( file ).id;
-			expect( getModelData( doc ) ).to.equal( `<image src="image.png"></image>[<image uploadId="${ id }"></image>]` );
-		} );
-
-		it( 'should allow to insert image at some custom position (options.insertAt)', () => {
-			const file = createNativeFileMock();
-			setModelData( doc, '<paragraph>[foo]</paragraph><paragraph>bar</paragraph><paragraph>bom</paragraph>' );
-
-			const customPosition = new ModelPosition( doc.getRoot(), [ 2 ] ); // <p>foo</p><p>bar</p>^<p>bom</p>
 
-			command.execute( { file, insertAt: customPosition } );
-
-			const id = fileRepository.getLoader( file ).id;
-			expect( getModelData( doc ) ).to.equal(
-				'<paragraph>foo</paragraph><paragraph>bar</paragraph>' +
-				`[<image uploadId="${ id }"></image>]` +
-				'<paragraph>bom</paragraph>'
-			);
+			expect( getModelData( doc ) ).to.equal( `[<image uploadId="${ id }"></image>]<paragraph>foo</paragraph>` );
+			sinon.assert.calledOnce( spy );
 		} );
 
-		it( 'should not insert image when proper insert position cannot be found', () => {
+		it( 'should not insert image nor crash when image could not be inserted', () => {
 			const file = createNativeFileMock();
 			doc.schema.registerItem( 'other' );
+			doc.schema.allow( { name: '$text', inside: 'other' } );
 			doc.schema.allow( { name: 'other', inside: '$root' } );
+			doc.schema.limits.add( 'other' );
 			buildModelConverter().for( editor.editing.modelToView )
 				.fromElement( 'other' )
-				.toElement( 'span' );
+				.toElement( 'p' );
 
 			setModelData( doc, '<other>[]</other>' );
 
@@ -109,28 +95,5 @@ describe( 'ImageUploadCommand', () => {
 
 			expect( getModelData( doc ) ).to.equal( '<other>[]</other>' );
 		} );
-
-		it( 'should not insert non-image', () => {
-			const file = createNativeFileMock();
-			file.type = 'audio/mpeg3';
-			setModelData( doc, '<paragraph>foo[]</paragraph>' );
-			command.execute( { file } );
-
-			expect( getModelData( doc ) ).to.equal( '<paragraph>foo[]</paragraph>' );
-		} );
-
-		it( 'should allow to provide batch instance', () => {
-			const batch = doc.batch();
-			const file = createNativeFileMock();
-			const spy = sinon.spy( batch, 'insert' );
-
-			setModelData( doc, '<paragraph>[]foo</paragraph>' );
-
-			command.execute( { batch, file } );
-			const id = fileRepository.getLoader( file ).id;
-
-			expect( getModelData( doc ) ).to.equal( `[<image uploadId="${ id }"></image>]<paragraph>foo</paragraph>` );
-			sinon.assert.calledOnce( spy );
-		} );
 	} );
 } );

+ 94 - 38
packages/ckeditor5-upload/tests/imageuploadengine.js

@@ -12,18 +12,22 @@ import ImageUploadCommand from '../src/imageuploadcommand';
 import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph';
 import UndoEngine from '@ckeditor/ckeditor5-undo/src/undoengine';
 import DataTransfer from '@ckeditor/ckeditor5-clipboard/src/datatransfer';
+
 import FileRepository from '../src/filerepository';
 import { AdapterMock, createNativeFileMock, NativeFileReaderMock } from './_utils/mocks';
+
 import { setData as setModelData, getData as getModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model';
 import { getData as getViewData } from '@ckeditor/ckeditor5-engine/src/dev-utils/view';
 import { eventNameToConsumableType } from '@ckeditor/ckeditor5-engine/src/conversion/model-to-view-converters';
+import Range from '@ckeditor/ckeditor5-engine/src/model/range';
+
 import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils';
 import Notification from '@ckeditor/ckeditor5-ui/src/notification/notification';
 
 describe( 'ImageUploadEngine', () => {
 	// eslint-disable-next-line max-len
 	const base64Sample = 'data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNk+A8AAQUBAScY42YAAAAASUVORK5CYII=';
-	let editor, document, fileRepository, viewDocument, nativeReaderMock, loader, adapterMock;
+	let editor, doc, fileRepository, viewDocument, nativeReaderMock, loader, adapterMock;
 	testUtils.createSinonSandbox();
 
 	beforeEach( () => {
@@ -38,7 +42,7 @@ describe( 'ImageUploadEngine', () => {
 		} )
 		.then( newEditor => {
 			editor = newEditor;
-			document = editor.document;
+			doc = editor.document;
 			viewDocument = editor.editing.view;
 
 			fileRepository = editor.plugins.get( FileRepository );
@@ -52,7 +56,7 @@ describe( 'ImageUploadEngine', () => {
 	} );
 
 	it( 'should register proper schema rules', () => {
-		expect( document.schema.check( { name: 'image', attributes: [ 'uploadId' ], inside: '$root' } ) ).to.be.true;
+		expect( doc.schema.check( { name: 'image', attributes: [ 'uploadId' ], inside: '$root' } ) ).to.be.true;
 	} );
 
 	it( 'should register imageUpload command', () => {
@@ -63,16 +67,64 @@ 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( doc, '<paragraph>[]foo</paragraph>' );
+
+		const targetRange = Range.createFromParentsAndOffsets( doc.getRoot(), 1, doc.getRoot(), 1 );
+		const targetViewRange = editor.editing.mapper.toViewRange( targetRange );
 
-		viewDocument.fire( 'clipboardInput', { dataTransfer } );
+		viewDocument.fire( 'clipboardInput', { dataTransfer, targetRanges: [ targetViewRange ] } );
 
 		sinon.assert.calledOnce( spy );
 		sinon.assert.calledWith( spy, 'imageUpload' );
 
 		const id = fileRepository.getLoader( fileMock ).id;
-		expect( getModelData( document ) ).to.equal(
-			`[<image uploadId="${ id }" uploadStatus="reading"></image>]<paragraph>foo bar baz</paragraph>`
+		expect( getModelData( doc ) ).to.equal(
+			`<paragraph>foo</paragraph>[<image uploadId="${ id }" uploadStatus="reading"></image>]`
+		);
+	} );
+
+	it( 'should execute imageUpload command with an optimized position when image is pasted', () => {
+		const spy = sinon.spy( editor, 'execute' );
+		const fileMock = createNativeFileMock();
+		const dataTransfer = new DataTransfer( { files: [ fileMock ] } );
+		setModelData( doc, '<paragraph>[]foo</paragraph>' );
+
+		const paragraph = doc.getRoot().getChild( 0 );
+		const targetRange = Range.createFromParentsAndOffsets( paragraph, 1, paragraph, 1 ); // f[]oo
+		const targetViewRange = editor.editing.mapper.toViewRange( targetRange );
+
+		viewDocument.fire( 'clipboardInput', { dataTransfer, targetRanges: [ targetViewRange ] } );
+
+		sinon.assert.calledOnce( spy );
+		sinon.assert.calledWith( spy, 'imageUpload' );
+
+		const id = fileRepository.getLoader( fileMock ).id;
+		expect( getModelData( doc ) ).to.equal(
+			`[<image uploadId="${ id }" uploadStatus="reading"></image>]<paragraph>foo</paragraph>`
+		);
+	} );
+
+	it( 'should execute imageUpload command when multiple files image are pasted', () => {
+		const spy = sinon.spy( editor, 'execute' );
+		const files = [ createNativeFileMock(), createNativeFileMock() ];
+		const dataTransfer = new DataTransfer( { files } );
+		setModelData( doc, '<paragraph>[]foo</paragraph>' );
+
+		const targetRange = Range.createFromParentsAndOffsets( doc.getRoot(), 1, doc.getRoot(), 1 );
+		const targetViewRange = editor.editing.mapper.toViewRange( targetRange );
+
+		viewDocument.fire( 'clipboardInput', { dataTransfer, targetRanges: [ targetViewRange ] } );
+
+		sinon.assert.calledTwice( spy );
+		sinon.assert.calledWith( spy, 'imageUpload' );
+
+		const id1 = fileRepository.getLoader( files[ 0 ] ).id;
+		const id2 = fileRepository.getLoader( files[ 1 ] ).id;
+
+		expect( getModelData( doc ) ).to.equal(
+			'<paragraph>foo</paragraph>' +
+			`<image uploadId="${ id1 }" uploadStatus="reading"></image>` +
+			`[<image uploadId="${ id2 }" uploadStatus="reading"></image>]`
 		);
 	} );
 
@@ -84,9 +136,13 @@ describe( 'ImageUploadEngine', () => {
 			size: 1024
 		};
 		const dataTransfer = new DataTransfer( { files: [ fileMock ] } );
-		setModelData( document, '<paragraph>foo bar baz[]</paragraph>' );
 
-		viewDocument.fire( 'clipboardInput', { dataTransfer } );
+		setModelData( doc, '<paragraph>foo[]</paragraph>' );
+
+		const targetRange = Range.createFromParentsAndOffsets( doc.getRoot(), 1, doc.getRoot(), 1 );
+		const targetViewRange = editor.editing.mapper.toViewRange( targetRange );
+
+		viewDocument.fire( 'clipboardInput', { dataTransfer, targetRanges: [ targetViewRange ] } );
 
 		sinon.assert.notCalled( spy );
 	} );
@@ -96,7 +152,7 @@ describe( 'ImageUploadEngine', () => {
 			consumable.consume( data.item, eventNameToConsumableType( evt.name ) );
 		}, { priority: 'high' } );
 
-		setModelData( document, '<image uploadId="1234"></image>' );
+		setModelData( doc, '<image uploadId="1234"></image>' );
 
 		expect( getViewData( viewDocument ) ).to.equal(
 			'[<figure class="image ck-widget" contenteditable="false">' +
@@ -106,10 +162,10 @@ describe( 'ImageUploadEngine', () => {
 
 	it( 'should use read data once it is present', done => {
 		const file = createNativeFileMock();
-		setModelData( document, '<paragraph>{}foo bar</paragraph>' );
+		setModelData( doc, '<paragraph>{}foo bar</paragraph>' );
 		editor.execute( 'imageUpload', { file } );
 
-		document.once( 'changesDone', () => {
+		doc.once( 'changesDone', () => {
 			expect( getViewData( viewDocument ) ).to.equal(
 				'[<figure class="image ck-widget" contenteditable="false">' +
 				`<img src="${ base64Sample }"></img>` +
@@ -126,11 +182,11 @@ describe( 'ImageUploadEngine', () => {
 
 	it( 'should replace read data with server response once it is present', done => {
 		const file = createNativeFileMock();
-		setModelData( document, '<paragraph>{}foo bar</paragraph>' );
+		setModelData( doc, '<paragraph>{}foo bar</paragraph>' );
 		editor.execute( 'imageUpload', { file } );
 
-		document.once( 'changesDone', () => {
-			document.once( 'changesDone', () => {
+		doc.once( 'changesDone', () => {
+			doc.once( 'changesDone', () => {
 				expect( getViewData( viewDocument ) ).to.equal(
 					'[<figure class="image ck-widget" contenteditable="false"><img src="image.png"></img></figure>]<p>foo bar</p>'
 				);
@@ -157,7 +213,7 @@ describe( 'ImageUploadEngine', () => {
 			done();
 		}, { priority: 'high' } );
 
-		setModelData( document, '<paragraph>{}foo bar</paragraph>' );
+		setModelData( doc, '<paragraph>{}foo bar</paragraph>' );
 		editor.execute( 'imageUpload', { file } );
 
 		nativeReaderMock.mockError( 'Reading error.' );
@@ -173,7 +229,7 @@ describe( 'ImageUploadEngine', () => {
 			evt.stop();
 		}, { priority: 'high' } );
 
-		setModelData( document, '<paragraph>{}foo bar</paragraph>' );
+		setModelData( doc, '<paragraph>{}foo bar</paragraph>' );
 		editor.execute( 'imageUpload', { file } );
 		nativeReaderMock.abort();
 
@@ -184,7 +240,7 @@ describe( 'ImageUploadEngine', () => {
 	} );
 
 	it( 'should do nothing if image does not have uploadId', () => {
-		setModelData( document, '<image src="image.png"></image>' );
+		setModelData( doc, '<image src="image.png"></image>' );
 
 		expect( getViewData( viewDocument ) ).to.equal(
 			'[<figure class="image ck-widget" contenteditable="false"><img src="image.png"></img></figure>]'
@@ -195,7 +251,7 @@ describe( 'ImageUploadEngine', () => {
 		const file = createNativeFileMock();
 		const spy = testUtils.sinon.spy();
 		const notification = editor.plugins.get( Notification );
-		setModelData( document, '<paragraph>{}foo bar</paragraph>' );
+		setModelData( doc, '<paragraph>{}foo bar</paragraph>' );
 
 		notification.on( 'show:warning', evt => {
 			spy();
@@ -204,9 +260,9 @@ describe( 'ImageUploadEngine', () => {
 
 		editor.execute( 'imageUpload', { file } );
 
-		document.once( 'changesDone', () => {
-			document.once( 'changesDone', () => {
-				expect( getModelData( document ) ).to.equal( '<paragraph>[]foo bar</paragraph>' );
+		doc.once( 'changesDone', () => {
+			doc.once( 'changesDone', () => {
+				expect( getModelData( doc ) ).to.equal( '<paragraph>[]foo bar</paragraph>' );
 				sinon.assert.calledOnce( spy );
 
 				done();
@@ -218,16 +274,16 @@ describe( 'ImageUploadEngine', () => {
 
 	it( 'should abort upload if image is removed', () => {
 		const file = createNativeFileMock();
-		setModelData( document, '<paragraph>{}foo bar</paragraph>' );
+		setModelData( doc, '<paragraph>{}foo bar</paragraph>' );
 		editor.execute( 'imageUpload', { file } );
 		const abortSpy = testUtils.sinon.spy( loader, 'abort' );
 
 		expect( loader.status ).to.equal( 'reading' );
 		nativeReaderMock.mockSuccess( base64Sample );
 
-		const image = document.getRoot().getChild( 0 );
-		document.enqueueChanges( () => {
-			const batch = document.batch();
+		const image = doc.getRoot().getChild( 0 );
+		doc.enqueueChanges( () => {
+			const batch = doc.batch();
 
 			batch.remove( image );
 		} );
@@ -239,7 +295,7 @@ describe( 'ImageUploadEngine', () => {
 	it( 'image should be permanently removed if it is removed by user during upload', done => {
 		const file = createNativeFileMock();
 		const notification = editor.plugins.get( Notification );
-		setModelData( document, '<paragraph>{}foo bar</paragraph>' );
+		setModelData( doc, '<paragraph>{}foo bar</paragraph>' );
 
 		// Prevent popping up alert window.
 		notification.on( 'show:warning', evt => {
@@ -248,23 +304,23 @@ describe( 'ImageUploadEngine', () => {
 
 		editor.execute( 'imageUpload', { file } );
 
-		document.once( 'changesDone', () => {
+		doc.once( 'changesDone', () => {
 			// This is called after "manual" remove.
-			document.once( 'changesDone', () => {
+			doc.once( 'changesDone', () => {
 				// This is called after attributes are removed.
 				let undone = false;
 
-				document.once( 'changesDone', () => {
+				doc.once( 'changesDone', () => {
 					if ( !undone ) {
 						undone = true;
 
 						// This is called after abort remove.
-						expect( getModelData( document ) ).to.equal( '<paragraph>[]foo bar</paragraph>' );
+						expect( getModelData( doc ) ).to.equal( '<paragraph>[]foo bar</paragraph>' );
 
 						editor.execute( 'undo' );
 
 						// Expect that the image has not been brought back.
-						expect( getModelData( document ) ).to.equal( '<paragraph>[]foo bar</paragraph>' );
+						expect( getModelData( doc ) ).to.equal( '<paragraph>[]foo bar</paragraph>' );
 
 						done();
 					}
@@ -272,9 +328,9 @@ describe( 'ImageUploadEngine', () => {
 			} );
 		} );
 
-		const image = document.getRoot().getChild( 0 );
-		document.enqueueChanges( () => {
-			const batch = document.batch();
+		const image = doc.getRoot().getChild( 0 );
+		doc.enqueueChanges( () => {
+			const batch = doc.batch();
 
 			batch.remove( image );
 		} );
@@ -282,11 +338,11 @@ describe( 'ImageUploadEngine', () => {
 
 	it( 'should create responsive image if server return multiple images', done => {
 		const file = createNativeFileMock();
-		setModelData( document, '<paragraph>{}foo bar</paragraph>' );
+		setModelData( doc, '<paragraph>{}foo bar</paragraph>' );
 		editor.execute( 'imageUpload', { file } );
 
-		document.once( 'changesDone', () => {
-			document.once( 'changesDone', () => {
+		doc.once( 'changesDone', () => {
+			doc.once( 'changesDone', () => {
 				expect( getViewData( viewDocument ) ).to.equal(
 					'[<figure class="image ck-widget" contenteditable="false">' +
 						'<img sizes="100vw" src="image.png" srcset="image-500.png 500w, image-800.png 800w"></img>' +

+ 83 - 3
packages/ckeditor5-upload/tests/utils.js

@@ -3,10 +3,12 @@
  * For licensing, see LICENSE.md.
  */
 
-import { isImageType } from '../src/utils';
+import { isImageType, findOptimalInsertionPosition } from '../src/utils';
+import Document from '@ckeditor/ckeditor5-engine/src/model/document';
+import { setData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model';
 
-describe( 'utils', () => {
-	describe( 'isImageType', () => {
+describe( 'upload utils', () => {
+	describe( 'isImageType()', () => {
 		it( 'should return true for png mime type', () => {
 			expect( isImageType( { type: 'image/png' } ) ).to.be.true;
 		} );
@@ -28,4 +30,82 @@ describe( 'utils', () => {
 			expect( isImageType( { type: 'video/mpeg' } ) ).to.be.false;
 		} );
 	} );
+
+	describe( 'findOptimalInsertionPosition()', () => {
+		let doc;
+
+		beforeEach( () => {
+			doc = new Document();
+
+			doc.createRoot();
+
+			doc.schema.registerItem( 'paragraph', '$block' );
+			doc.schema.registerItem( 'image' );
+			doc.schema.registerItem( 'span' );
+
+			doc.schema.allow( { name: 'image', inside: '$root' } );
+			doc.schema.objects.add( 'image' );
+
+			doc.schema.allow( { name: 'span', inside: 'paragraph' } );
+			doc.schema.allow( { name: '$text', inside: 'span' } );
+		} );
+
+		it( 'returns position after selected element', () => {
+			setData( doc, '<paragraph>x</paragraph>[<image></image>]<paragraph>y</paragraph>' );
+
+			const pos = findOptimalInsertionPosition( doc.selection );
+
+			expect( pos.path ).to.deep.equal( [ 2 ] );
+		} );
+
+		it( 'returns position inside empty block', () => {
+			setData( doc, '<paragraph>x</paragraph><paragraph>[]</paragraph><paragraph>y</paragraph>' );
+
+			const pos = findOptimalInsertionPosition( doc.selection );
+
+			expect( pos.path ).to.deep.equal( [ 1, 0 ] );
+		} );
+
+		it( 'returns position before block if at the beginning of that block', () => {
+			setData( doc, '<paragraph>x</paragraph><paragraph>[]foo</paragraph><paragraph>y</paragraph>' );
+
+			const pos = findOptimalInsertionPosition( doc.selection );
+
+			expect( pos.path ).to.deep.equal( [ 1 ] );
+		} );
+
+		it( 'returns position before block if in the middle of that block', () => {
+			setData( doc, '<paragraph>x</paragraph><paragraph>f[]oo</paragraph><paragraph>y</paragraph>' );
+
+			const pos = findOptimalInsertionPosition( doc.selection );
+
+			expect( pos.path ).to.deep.equal( [ 1 ] );
+		} );
+
+		it( 'returns position after block if at the end of that block', () => {
+			setData( doc, '<paragraph>x</paragraph><paragraph>foo[]</paragraph><paragraph>y</paragraph>' );
+
+			const pos = findOptimalInsertionPosition( doc.selection );
+
+			expect( pos.path ).to.deep.equal( [ 2 ] );
+		} );
+
+		// Checking if isTouching() was used.
+		it( 'returns position after block if at the end of that block (deeply nested)', () => {
+			setData( doc, '<paragraph>x</paragraph><paragraph>foo<span>bar[]</span></paragraph><paragraph>y</paragraph>' );
+
+			const pos = findOptimalInsertionPosition( doc.selection );
+
+			expect( pos.path ).to.deep.equal( [ 2 ] );
+		} );
+
+		it( 'returns selection focus if not in a block', () => {
+			doc.schema.allow( { name: '$text', inside: '$root' } );
+			setData( doc, 'foo[]bar' );
+
+			const pos = findOptimalInsertionPosition( doc.selection );
+
+			expect( pos.path ).to.deep.equal( [ 3 ] );
+		} );
+	} );
 } );