Merge pull request #232 from ckeditor/t/ckeditor5/1243

Other: Image feature should insert image the same way as other widgets. BREAKING CHANGE: Removed `findOptimalInsertionPosition()` from utils. The method can be found in Widget feature in `widget/utils` module.
ckeditor · Sep 18, 2018 · 26638f5 · 26638f5
2 parents b9dad37 + 7ec3ce4
commit 26638f5
Show file tree

Hide file tree

Showing 5 changed files with 9 additions and 139 deletions.
diff --git a/src/imageupload/imageuploadcommand.js b/src/imageupload/imageuploadcommand.js
@@ -26,8 +26,8 @@ export default class ImageUploadCommand extends Command {
 	 * @param {File} options.file The image file to upload.
 	 * @param {module:engine/model/position~Position} [options.insertAt] The position at which the image should be inserted.
 	 * 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 the UX perspective.
+	 * Note: You can use the {@link module:widget/utils~findOptimalInsertionPosition} function
+	 * to calculate (e.g. based on the current selection) a position which is more optimal from the UX perspective.
 	 */
 	execute( options ) {
 		const editor = this.editor;

diff --git a/src/imageupload/imageuploadediting.js b/src/imageupload/imageuploadediting.js
@@ -12,8 +12,8 @@ import FileRepository from '@ckeditor/ckeditor5-upload/src/filerepository';
 import ImageUploadCommand from '../../src/imageupload/imageuploadcommand';
 import Notification from '@ckeditor/ckeditor5-ui/src/notification/notification';
 import ModelSelection from '@ckeditor/ckeditor5-engine/src/model/selection';
-import ModelRange from '@ckeditor/ckeditor5-engine/src/model/range';
-import { isImageType, findOptimalInsertionPosition } from '../../src/imageupload/utils';
+import { isImageType } from '../../src/imageupload/utils';
+import { findOptimalInsertionPosition } from '@ckeditor/ckeditor5-widget/src/utils';
 
 /**
  * The editing part of the image upload feature.
@@ -73,9 +73,8 @@ export default class ImageUploadEditing extends Plugin {
 						}
 
 						const imageElement = writer.createElement( 'image', { uploadId: loader.id } );
-						const targetSelection = new ModelSelection( [ new ModelRange( insertAt ) ] );
 
-						editor.model.insertContent( imageElement, targetSelection );
+						editor.model.insertContent( imageElement, insertAt );
 
 						// Inserting an image might've failed due to schema regulations.
 						if ( imageElement.parent ) {

diff --git a/src/imageupload/imageuploadui.js b/src/imageupload/imageuploadui.js
@@ -10,7 +10,8 @@
 import Plugin from '@ckeditor/ckeditor5-core/src/plugin';
 import FileDialogButtonView from '@ckeditor/ckeditor5-upload/src/ui/filedialogbuttonview';
 import imageIcon from '@ckeditor/ckeditor5-core/theme/icons/image.svg';
-import { isImageType, findOptimalInsertionPosition } from './utils';
+import { isImageType } from './utils';
+import { findOptimalInsertionPosition } from '@ckeditor/ckeditor5-widget/src/utils';
 
 /**
  * The image upload button plugin.

diff --git a/src/imageupload/utils.js b/src/imageupload/utils.js
@@ -4,11 +4,9 @@
  */
 
 /**
- * @module upload/utils
+ * @module image/imageupload/utils
  */
 
-import ModelPosition from '@ckeditor/ckeditor5-engine/src/model/position';
-
 /**
  * Checks if a given file is an image.
  *
@@ -20,48 +18,3 @@ 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 the middle of a paragraph, the position before this paragraph
- * will be returned so that it is not split. If the selection is at the end of a paragraph,
- * the position after this paragraph will be returned.
- *
- * Note: If the selection is placed in an empty block, that block will be returned. If that position
- * is then passed to {@link module:engine/model/model~Model#insertContent},
- * the block will be fully replaced by the image.
- *
- * @param {module:engine/model/selection~Selection|module:engine/model/documentselection~DocumentSelection} selection
- * The 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;
-}
diff --git a/tests/imageupload/utils.js b/tests/imageupload/utils.js
@@ -3,9 +3,7 @@
  * For licensing, see LICENSE.md.
  */
 
-import { isImageType, findOptimalInsertionPosition } from '../../src/imageupload/utils';
-import Model from '@ckeditor/ckeditor5-engine/src/model/model';
-import { setData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model';
+import { isImageType } from '../../src/imageupload/utils';
 
 describe( 'upload utils', () => {
 	describe( 'isImageType()', () => {
@@ -30,85 +28,4 @@ describe( 'upload utils', () => {
 			expect( isImageType( { type: 'video/mpeg' } ) ).to.be.false;
 		} );
 	} );
-
-	describe( 'findOptimalInsertionPosition()', () => {
-		let model, doc;
-
-		beforeEach( () => {
-			model = new Model();
-			doc = model.document;
-
-			doc.createRoot();
-
-			model.schema.register( 'paragraph', { inheritAllFrom: '$block' } );
-			model.schema.register( 'image' );
-			model.schema.register( 'span' );
-
-			model.schema.extend( 'image', {
-				allowIn: '$root',
-				isObject: true
-			} );
-
-			model.schema.extend( 'span', { allowIn: 'paragraph' } );
-			model.schema.extend( '$text', { allowIn: 'span' } );
-		} );
-
-		it( 'returns position after selected element', () => {
-			setData( model, '<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( model, '<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( model, '<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( model, '<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( model, '<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( model, '<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', () => {
-			model.schema.extend( '$text', { allowIn: '$root' } );
-			setData( model, 'foo[]bar' );
-
-			const pos = findOptimalInsertionPosition( doc.selection );
-
-			expect( pos.path ).to.deep.equal( [ 3 ] );
-		} );
-	} );
 } );