Skip to content
This repository has been archived by the owner on Jun 26, 2020. It is now read-only.

Commit

Permalink
Merge pull request #209 from ckeditor/t/208
Browse files Browse the repository at this point in the history 
Fix: Made image upload by drag&drop work when the `ImageUploadCommand` is disabled. Closes #208.
  • Loading branch information
@Reinmar
Reinmar authored Jun 4, 2018
2 parents 937809c + 282bec0 commit 6908ec6
Show file tree
Hide file tree
Showing 3 changed files with 130 additions and 29 deletions.
5 changes: 2 additions & 3 deletions src/imageupload/imageuploadcommand.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
* For licensing, see LICENSE.md.
*/

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 '@ckeditor/ckeditor5-upload/src/filerepository';
Expand Down Expand Up @@ -44,7 +43,7 @@ export default class ImageUploadCommand extends Command {
return;
}

const imageElement = new ModelElement( 'image', {
const imageElement = writer.createElement( 'image', {
uploadId: loader.id
} );

Expand All @@ -60,7 +59,7 @@ export default class ImageUploadCommand extends Command {

// Inserting an image might've failed due to schema regulations.
if ( imageElement.parent ) {
writer.setSelection( ModelRange.createOn( imageElement ) );
writer.setSelection( imageElement, 'on' );
}
} );
}
Expand Down
27 changes: 23 additions & 4 deletions src/imageupload/imageuploadediting.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ 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';

/**
Expand Down Expand Up @@ -45,7 +46,7 @@ export default class ImageUploadEditing extends Plugin {
editor.commands.add( 'imageUpload', new ImageUploadCommand( editor ) );

// Execute imageUpload command when image is dropped or pasted.
editor.editing.view.document.on( 'clipboardInput', ( evt, data ) => {
this.listenTo( editor.editing.view.document, 'clipboardInput', ( evt, data ) => {
// Skip if non empty HTML data is included.
// https://github.com/ckeditor/ckeditor5-upload/issues/68
if ( isHtmlIncluded( data.dataTransfer ) ) {
Expand All @@ -57,10 +58,28 @@ export default class ImageUploadEditing extends Plugin {
);

for ( const file of data.dataTransfer.files ) {
const insertAt = findOptimalInsertionPosition( targetModelSelection );

if ( isImageType( file ) ) {
editor.execute( 'imageUpload', { file, insertAt } );
const insertAt = findOptimalInsertionPosition( targetModelSelection );

editor.model.change( writer => {
const loader = fileRepository.createLoader( file );

// Do not throw when upload adapter is not set. FileRepository will log an error anyway.
if ( !loader ) {
return;
}

const imageElement = writer.createElement( 'image', { uploadId: loader.id } );
const targetSelection = new ModelSelection( [ new ModelRange( insertAt ) ] );

editor.model.insertContent( imageElement, targetSelection );

// Inserting an image might've failed due to schema regulations.
if ( imageElement.parent ) {
writer.setSelection( imageElement, 'on' );
}
} );

evt.stop();
}

Expand Down
127 changes: 105 additions & 22 deletions tests/imageupload/imageuploadediting.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import VirtualTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/virtualtesteditor';

import Plugin from '@ckeditor/ckeditor5-core/src/plugin';
import Clipboard from '@ckeditor/ckeditor5-clipboard/src/clipboard';
import ImageEditing from '../../src/image/imageediting';
import ImageUploadEditing from '../../src/imageupload/imageuploadediting';
import ImageUploadCommand from '../../src/imageupload/imageuploadcommand';
Expand All @@ -23,6 +24,7 @@ import { getData as getViewData } from '@ckeditor/ckeditor5-engine/src/dev-utils
import Range from '@ckeditor/ckeditor5-engine/src/model/range';
import Position from '@ckeditor/ckeditor5-engine/src/model/position';

import log from '@ckeditor/ckeditor5-utils/src/log';
import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils';
import Notification from '@ckeditor/ckeditor5-ui/src/notification/notification';

Expand Down Expand Up @@ -65,6 +67,10 @@ describe( 'ImageUploadEditing', () => {
} );
} );

afterEach( () => {
return editor.destroy();
} );

it( 'should register proper schema rules', () => {
expect( model.schema.checkAttribute( [ '$root', 'image' ], 'uploadId' ) ).to.be.true;
} );
Expand All @@ -73,8 +79,7 @@ describe( 'ImageUploadEditing', () => {
expect( editor.commands.get( 'imageUpload' ) ).to.be.instanceOf( ImageUploadCommand );
} );

it( 'should execute imageUpload command when image is pasted', () => {
const spy = sinon.spy( editor, 'execute' );
it( 'should insert image when is pasted', () => {
const fileMock = createNativeFileMock();
const dataTransfer = new DataTransfer( { files: [ fileMock ], types: [ 'Files' ] } );
setModelData( model, '<paragraph>[]foo</paragraph>' );
Expand All @@ -84,17 +89,13 @@ describe( 'ImageUploadEditing', () => {

viewDocument.fire( 'clipboardInput', { dataTransfer, targetRanges: [ targetViewRange ] } );

sinon.assert.calledOnce( spy );
sinon.assert.calledWith( spy, 'imageUpload' );

const id = fileRepository.getLoader( fileMock ).id;
expect( getModelData( model ) ).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' );
it( 'should insert image at optimized position when is pasted', () => {
const fileMock = createNativeFileMock();
const dataTransfer = new DataTransfer( { files: [ fileMock ], types: [ 'Files' ] } );
setModelData( model, '<paragraph>[]foo</paragraph>' );
Expand All @@ -105,17 +106,13 @@ describe( 'ImageUploadEditing', () => {

viewDocument.fire( 'clipboardInput', { dataTransfer, targetRanges: [ targetViewRange ] } );

sinon.assert.calledOnce( spy );
sinon.assert.calledWith( spy, 'imageUpload' );

const id = fileRepository.getLoader( fileMock ).id;
expect( getModelData( model ) ).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' );
it( 'should insert multiple image files when are pasted', () => {
const files = [ createNativeFileMock(), createNativeFileMock() ];
const dataTransfer = new DataTransfer( { files, types: [ 'Files' ] } );
setModelData( model, '<paragraph>[]foo</paragraph>' );
Expand All @@ -125,9 +122,6 @@ describe( 'ImageUploadEditing', () => {

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;

Expand All @@ -138,8 +132,56 @@ describe( 'ImageUploadEditing', () => {
);
} );

it( 'should not execute imageUpload command when file is not an image', () => {
const spy = sinon.spy( editor, 'execute' );
it( 'should insert image when is pasted on allowed position when ImageUploadCommand is disabled', () => {
const fileMock = createNativeFileMock();
const dataTransfer = new DataTransfer( { files: [ fileMock ], types: [ 'Files' ] } );
setModelData( model, '<paragraph>[]foo</paragraph>' );

const command = editor.commands.get( 'imageUpload' );

command.on( 'beforeChange:isEnabled', evt => {
evt.return = false;
evt.stop();
}, { priority: 'highest' } );

command.isEnabled = false;

const targetRange = Range.createFromParentsAndOffsets( doc.getRoot(), 1, doc.getRoot(), 1 );
const targetViewRange = editor.editing.mapper.toViewRange( targetRange );

viewDocument.fire( 'clipboardInput', { dataTransfer, targetRanges: [ targetViewRange ] } );

const id = fileRepository.getLoader( fileMock ).id;
expect( getModelData( model ) ).to.equal(
`<paragraph>foo</paragraph>[<image uploadId="${ id }" uploadStatus="reading"></image>]`
);
} );

it( 'should not insert image when editor is in read-only mode', () => {
// Clipboard plugin is required for this test.
return VirtualTestEditor
.create( {
plugins: [ ImageEditing, ImageUploadEditing, Paragraph, UploadAdapterPluginMock, Clipboard ]
} )
.then( editor => {
const fileMock = createNativeFileMock();
const dataTransfer = new DataTransfer( { files: [ fileMock ], types: [ 'Files' ] } );
setModelData( editor.model, '<paragraph>[]foo</paragraph>' );

const targetRange = editor.model.document.selection.getFirstRange();
const targetViewRange = editor.editing.mapper.toViewRange( targetRange );

editor.isReadOnly = true;

editor.editing.view.document.fire( 'clipboardInput', { dataTransfer, targetRanges: [ targetViewRange ] } );

expect( getModelData( editor.model ) ).to.equal( '<paragraph>[]foo</paragraph>' );

return editor.destroy();
} );
} );

it( 'should not insert image when file is not an image', () => {
const viewDocument = editor.editing.view.document;
const fileMock = {
type: 'media/mp3',
Expand All @@ -149,16 +191,15 @@ describe( 'ImageUploadEditing', () => {

setModelData( model, '<paragraph>foo[]</paragraph>' );

const targetRange = Range.createFromParentsAndOffsets( doc.getRoot(), 1, doc.getRoot(), 1 );
const targetRange = doc.selection.getFirstRange();
const targetViewRange = editor.editing.mapper.toViewRange( targetRange );

viewDocument.fire( 'clipboardInput', { dataTransfer, targetRanges: [ targetViewRange ] } );

sinon.assert.notCalled( spy );
expect( getModelData( model ) ).to.equal( '<paragraph>foo[]</paragraph>' );
} );

it( 'should not execute imageUpload command when there is non-empty HTML content pasted', () => {
const spy = sinon.spy( editor, 'execute' );
it( 'should not insert image when there is non-empty HTML content pasted', () => {
const fileMock = createNativeFileMock();
const dataTransfer = new DataTransfer( {
files: [ fileMock ],
Expand All @@ -172,7 +213,49 @@ describe( 'ImageUploadEditing', () => {

viewDocument.fire( 'clipboardInput', { dataTransfer, targetRanges: [ targetViewRange ] } );

sinon.assert.notCalled( spy );
expect( getModelData( model ) ).to.equal( '<paragraph>[]foo</paragraph>' );
} );

it( 'should not insert image nor crash when pasted image could not be inserted', () => {
model.schema.register( 'other', {
allowIn: '$root',
isLimit: true
} );
model.schema.extend( '$text', { allowIn: 'other' } );

editor.conversion.elementToElement( { model: 'other', view: 'p' } );

setModelData( model, '<other>[]</other>' );

const fileMock = createNativeFileMock();
const dataTransfer = new DataTransfer( { files: [ fileMock ], types: [ 'Files' ] } );

const targetRange = doc.selection.getFirstRange();
const targetViewRange = editor.editing.mapper.toViewRange( targetRange );

viewDocument.fire( 'clipboardInput', { dataTransfer, targetRanges: [ targetViewRange ] } );

expect( getModelData( model ) ).to.equal( '<other>[]</other>' );
} );

it( 'should not throw when upload adapter is not set (FileRepository will log an error anyway) when image is pasted', () => {
const fileMock = createNativeFileMock();
const dataTransfer = new DataTransfer( { files: [ fileMock ], types: [ 'Files' ] } );
const logStub = testUtils.sinon.stub( log, 'error' );

setModelData( model, '<paragraph>[]foo</paragraph>' );

fileRepository.createUploadAdapter = undefined;

const targetRange = Range.createFromParentsAndOffsets( doc.getRoot(), 1, doc.getRoot(), 1 );
const targetViewRange = editor.editing.mapper.toViewRange( targetRange );

expect( () => {
viewDocument.fire( 'clipboardInput', { dataTransfer, targetRanges: [ targetViewRange ] } );
} ).to.not.throw();

expect( getModelData( model ) ).to.equal( '<paragraph>[]foo</paragraph>' );
sinon.assert.calledOnce( logStub );
} );

// https://github.com/ckeditor/ckeditor5-upload/issues/70
Expand Down

0 comments on commit 6908ec6

Please sign in to comment.