Procházet zdrojové kódy

Got rid of unnecessary file type checks (that's the caller's duty), superfluous tests and corrected test for inserting image at disallowed position.

Piotrek Koszuliński před 8 roky
rodič
revize
60982ee158

+ 5 - 6
packages/ckeditor5-upload/src/imageuploadcommand.js

@@ -7,7 +7,6 @@ import ModelElement from '@ckeditor/ckeditor5-engine/src/model/element';
 import ModelRange from '@ckeditor/ckeditor5-engine/src/model/range';
 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';
 
 /**
@@ -39,10 +38,6 @@ export default class ImageUploadCommand extends Command {
 		const selection = doc.selection;
 		const fileRepository = editor.plugins.get( FileRepository );
 
-		if ( !isImageType( file ) ) {
-			return;
-		}
-
 		doc.enqueueChanges( () => {
 			const imageElement = new ModelElement( 'image', {
 				uploadId: fileRepository.createLoader( file ).id
@@ -57,7 +52,11 @@ export default class ImageUploadCommand extends Command {
 			}
 
 			editor.data.insertContent( imageElement, insertAtSelection, batch );
-			selection.setRanges( [ ModelRange.createOn( imageElement ) ] );
+
+			// Inserting an image might've failed due to schema regulations.
+			if ( imageElement.parent ) {
+				selection.setRanges( [ ModelRange.createOn( imageElement ) ] );
+			}
 		} );
 	}
 }

+ 21 - 60
packages/ckeditor5-upload/tests/imageuploadcommand.js

@@ -39,69 +39,53 @@ describe( 'ImageUploadCommand', () => {
 	} );
 
 	describe( 'execute()', () => {
-		it( 'should insert image', () => {
+		it( 'should insert image at selection position', () => {
 			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( `[<image uploadId="${ id }"></image>]<paragraph>foo</paragraph>` );
+			expect( getModelData( doc ) )
+				.to.equal( `<paragraph>f</paragraph>[<image uploadId="${ id }"></image>]<paragraph>oo</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 insertAt = new ModelPosition( doc.getRoot(), [ 0, 2 ] ); // fo[]o
 
-			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>' );
-
-			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 +93,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 );
-		} );
 	} );
 } );