diff --git a/src/imageupload/imageuploadcommand.js b/src/imageupload/imageuploadcommand.js index f67b1f0d..6b61cf9d 100644 --- a/src/imageupload/imageuploadcommand.js +++ b/src/imageupload/imageuploadcommand.js @@ -3,10 +3,9 @@ * For licensing, see LICENSE.md. */ -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'; import Command from '@ckeditor/ckeditor5-core/src/command'; +import { findOptimalInsertionPosition } from '@ckeditor/ckeditor5-widget/src/utils'; /** * @module image/imageupload/imageuploadcommand @@ -18,49 +17,93 @@ import Command from '@ckeditor/ckeditor5-core/src/command'; * @extends module:core/command~Command */ export default class ImageUploadCommand extends Command { + /** + * @inheritDoc + */ + refresh() { + const model = this.editor.model; + const selection = model.document.selection; + const schema = model.schema; + + this.isEnabled = isImageAllowedInParent( selection, schema ) && checkSelectionWithObject( selection, schema ); + } + /** * Executes the command. * * @fires execute * @param {Object} options Options for the executed 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:widget/utils~findOptimalInsertionPosition} function - * to calculate (e.g. based on the current selection) a position which is more optimal from the UX perspective. + * @param {File|Array.} options.files The image file or an array of image files to upload. */ execute( options ) { const editor = this.editor; - const doc = editor.model.document; - const file = options.file; - const fileRepository = editor.plugins.get( FileRepository ); editor.model.change( writer => { - const loader = fileRepository.createLoader( file ); + const filesToUpload = Array.isArray( options.files ) ? options.files : [ options.files ]; - // Do not throw when upload adapter is not set. FileRepository will log an error anyway. - if ( !loader ) { - return; + for ( const file of filesToUpload ) { + uploadImage( writer, editor, file ); } + } ); + } +} - const imageElement = writer.createElement( 'image', { - uploadId: loader.id - } ); +// Handles uploading single file. +// +// @param {module:engine/model/writer~writer} writer +// @param {module:core/editor/editor~Editor} editor +// @param {File} file +function uploadImage( writer, editor, file ) { + const doc = editor.model.document; + const fileRepository = editor.plugins.get( FileRepository ); - let insertAtSelection; + const loader = fileRepository.createLoader( file ); - if ( options.insertAt ) { - insertAtSelection = new ModelSelection( [ new ModelRange( options.insertAt ) ] ); - } else { - insertAtSelection = doc.selection; - } + // Do not throw when upload adapter is not set. FileRepository will log an error anyway. + if ( !loader ) { + return; + } - editor.model.insertContent( imageElement, insertAtSelection ); + const imageElement = writer.createElement( 'image', { uploadId: loader.id } ); - // Inserting an image might've failed due to schema regulations. - if ( imageElement.parent ) { - writer.setSelection( imageElement, 'on' ); - } - } ); + const insertAtSelection = findOptimalInsertionPosition( doc.selection ); + + editor.model.insertContent( imageElement, insertAtSelection ); + + // Inserting an image might've failed due to schema regulations. + if ( imageElement.parent ) { + writer.setSelection( imageElement, 'on' ); } } + +// Checks if image is allowed by schema in optimal insertion parent. +function isImageAllowedInParent( selection, schema ) { + const parent = getInsertImageParent( selection ); + + return schema.checkChild( parent, 'image' ); +} + +// Additional check for when the command should be disabled: +// - selection is on object +// - selection is inside object +function checkSelectionWithObject( selection, schema ) { + const selectedElement = selection.getSelectedElement(); + + const isSelectionOnObject = !!selectedElement && schema.isObject( selectedElement ); + const isSelectionInObject = !![ ...selection.focus.getAncestors() ].find( ancestor => schema.isObject( ancestor ) ); + + return !isSelectionOnObject && !isSelectionInObject; +} + +// Returns a node that will be used to insert image with `model.insertContent` to check if image can be placed there. +function getInsertImageParent( selection ) { + const insertAt = findOptimalInsertionPosition( selection ); + + let parent = insertAt.parent; + + if ( !parent.is( '$root' ) ) { + parent = parent.parent; + } + + return parent; +} diff --git a/src/imageupload/imageuploadediting.js b/src/imageupload/imageuploadediting.js index 01874287..f529f764 100644 --- a/src/imageupload/imageuploadediting.js +++ b/src/imageupload/imageuploadediting.js @@ -13,7 +13,6 @@ 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 { isImageType } from '../../src/imageupload/utils'; -import { findOptimalInsertionPosition } from '@ckeditor/ckeditor5-widget/src/utils'; /** * The editing part of the image upload feature. @@ -56,39 +55,25 @@ export default class ImageUploadEditing extends Plugin { return; } - let targetModelSelection = new ModelSelection( + const images = Array.from( data.dataTransfer.files ).filter( isImageType ); + + const targetModelSelection = new ModelSelection( data.targetRanges.map( viewRange => editor.editing.mapper.toModelRange( viewRange ) ) ); - for ( const file of data.dataTransfer.files ) { - if ( isImageType( file ) ) { - 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; - } + editor.model.change( writer => { + // Set selection to paste target. + writer.setSelection( targetModelSelection ); - const imageElement = writer.createElement( 'image', { uploadId: loader.id } ); - - editor.model.insertContent( imageElement, insertAt ); + if ( images.length ) { + evt.stop(); - // Inserting an image might've failed due to schema regulations. - if ( imageElement.parent ) { - writer.setSelection( imageElement, 'on' ); - } + // Upload images after the selection has changed in order to ensure the command's state is refreshed. + editor.model.enqueueChange( 'default', () => { + editor.execute( 'imageUpload', { files: images } ); } ); - - 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; - } + } ); } ); // Prevents from the browser redirecting to the dropped image. diff --git a/src/imageupload/imageuploadui.js b/src/imageupload/imageuploadui.js index 2c1cbeb9..d6912d76 100644 --- a/src/imageupload/imageuploadui.js +++ b/src/imageupload/imageuploadui.js @@ -11,7 +11,6 @@ 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 } from './utils'; -import { findOptimalInsertionPosition } from '@ckeditor/ckeditor5-widget/src/utils'; /** * The image upload button plugin. @@ -49,12 +48,10 @@ export default class ImageUploadUI extends Plugin { view.buttonView.bind( 'isEnabled' ).to( command ); view.on( 'done', ( evt, files ) => { - for ( const file of Array.from( files ) ) { - const insertAt = findOptimalInsertionPosition( editor.model.document.selection ); + const imagesToUpload = Array.from( files ).filter( isImageType ); - if ( isImageType( file ) ) { - editor.execute( 'imageUpload', { file, insertAt } ); - } + if ( imagesToUpload.length ) { + editor.execute( 'imageUpload', { files: imagesToUpload } ); } } ); diff --git a/tests/imageupload/imageuploadcommand.js b/tests/imageupload/imageuploadcommand.js index fd105af0..eaeb7dfd 100644 --- a/tests/imageupload/imageuploadcommand.js +++ b/tests/imageupload/imageuploadcommand.js @@ -15,14 +15,13 @@ import { setData as setModelData, getData as getModelData } from '@ckeditor/cked import Image from '../../src/image/imageediting'; import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph'; import { downcastElementToElement } from '@ckeditor/ckeditor5-engine/src/conversion/downcast-converters'; -import ModelPosition from '@ckeditor/ckeditor5-engine/src/model/position'; import log from '@ckeditor/ckeditor5-utils/src/log'; import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils'; describe( 'ImageUploadCommand', () => { - let editor, command, model, doc, fileRepository; + let editor, command, model, fileRepository; testUtils.createSinonSandbox(); @@ -43,7 +42,6 @@ describe( 'ImageUploadCommand', () => { .then( newEditor => { editor = newEditor; model = editor.model; - doc = model.document; command = new ImageUploadCommand( editor ); @@ -56,29 +54,96 @@ describe( 'ImageUploadCommand', () => { return editor.destroy(); } ); - describe( 'execute()', () => { - it( 'should insert image at selection position (includes deleting selected content)', () => { - const file = createNativeFileMock(); - setModelData( model, 'f[o]o' ); + describe( 'isEnabled', () => { + it( 'should be true when the selection directly in the root', () => { + model.enqueueChange( 'transparent', () => { + setModelData( model, '[]' ); - command.execute( { file } ); + command.refresh(); + expect( command.isEnabled ).to.be.true; + } ); + } ); - const id = fileRepository.getLoader( file ).id; - expect( getModelData( model ) ) - .to.equal( `f[]o` ); + it( 'should be true when the selection is in empty block', () => { + setModelData( model, '[]' ); + + expect( command.isEnabled ).to.be.true; } ); - it( 'should insert directly at specified position (options.insertAt)', () => { - const file = createNativeFileMock(); - setModelData( model, 'f[]oo' ); + it( 'should be true when the selection directly in a paragraph', () => { + setModelData( model, 'foo[]' ); + expect( command.isEnabled ).to.be.true; + } ); + + it( 'should be true when the selection directly in a block', () => { + model.schema.register( 'block', { inheritAllFrom: '$block' } ); + model.schema.extend( '$text', { allowIn: 'block' } ); + editor.conversion.for( 'downcast' ).add( downcastElementToElement( { model: 'block', view: 'block' } ) ); + + setModelData( model, 'foo[]' ); + expect( command.isEnabled ).to.be.true; + } ); + + it( 'should be false when the selection is on other image', () => { + setModelData( model, '[]' ); + expect( command.isEnabled ).to.be.false; + } ); + + it( 'should be false when the selection is inside other image', () => { + model.schema.register( 'caption', { + allowIn: 'image', + allowContentOf: '$block', + isLimit: true + } ); + editor.conversion.for( 'downcast' ).add( downcastElementToElement( { model: 'caption', view: 'figcaption' } ) ); + setModelData( model, '[]' ); + expect( command.isEnabled ).to.be.false; + } ); + + it( 'should be false when the selection is on other object', () => { + model.schema.register( 'object', { isObject: true, allowIn: '$root' } ); + editor.conversion.for( 'downcast' ).add( downcastElementToElement( { model: 'object', view: 'object' } ) ); + setModelData( model, '[]' ); + + expect( command.isEnabled ).to.be.false; + } ); - const insertAt = new ModelPosition( doc.getRoot(), [ 0, 2 ] ); // fo[]o + it( 'should be false when the selection is inside other object', () => { + model.schema.register( 'object', { isObject: true, allowIn: '$root' } ); + model.schema.extend( '$text', { allowIn: 'object' } ); + editor.conversion.for( 'downcast' ).add( downcastElementToElement( { model: 'object', view: 'object' } ) ); + setModelData( model, '[]' ); + + expect( command.isEnabled ).to.be.false; + } ); + + it( 'should be false when schema disallows image', () => { + model.schema.register( 'block', { inheritAllFrom: '$block' } ); + model.schema.extend( 'paragraph', { allowIn: 'block' } ); + // Block image in block. + model.schema.addChildCheck( ( context, childDefinition ) => { + if ( childDefinition.name === 'image' && context.last.name === 'block' ) { + return false; + } + } ); + editor.conversion.for( 'downcast' ).add( downcastElementToElement( { model: 'block', view: 'block' } ) ); + + setModelData( model, '[]' ); + + expect( command.isEnabled ).to.be.false; + } ); + } ); + + describe( 'execute()', () => { + it( 'should insert image at selection position as other widgets', () => { + const file = createNativeFileMock(); + setModelData( model, 'f[o]o' ); - command.execute( { file, insertAt } ); + command.execute( { files: file } ); const id = fileRepository.getLoader( file ).id; expect( getModelData( model ) ) - .to.equal( `fo[]o` ); + .to.equal( `[]foo` ); } ); it( 'should use parent batch', () => { @@ -89,7 +154,7 @@ describe( 'ImageUploadCommand', () => { model.change( writer => { expect( writer.batch.operations ).to.length( 0 ); - command.execute( { file } ); + command.execute( { files: file } ); expect( writer.batch.operations ).to.length.above( 0 ); } ); @@ -108,7 +173,7 @@ describe( 'ImageUploadCommand', () => { setModelData( model, '[]' ); - command.execute( { file } ); + command.execute( { files: file } ); expect( getModelData( model ) ).to.equal( '[]' ); } ); @@ -123,7 +188,7 @@ describe( 'ImageUploadCommand', () => { setModelData( model, 'fo[]o' ); expect( () => { - command.execute( { file } ); + command.execute( { files: file } ); } ).to.not.throw(); expect( getModelData( model ) ).to.equal( 'fo[]o' ); diff --git a/tests/imageupload/imageuploadediting.js b/tests/imageupload/imageuploadediting.js index e8440197..f85d84a4 100644 --- a/tests/imageupload/imageuploadediting.js +++ b/tests/imageupload/imageuploadediting.js @@ -137,27 +137,23 @@ describe( 'ImageUploadEditing', () => { } ); it( 'should insert image when is pasted on allowed position when ImageUploadCommand is disabled', () => { + setModelData( model, 'foo[]' ); + const fileMock = createNativeFileMock(); const dataTransfer = new DataTransfer( { files: [ fileMock ], types: [ 'Files' ] } ); - setModelData( model, '[]foo' ); const command = editor.commands.get( 'imageUpload' ); - command.on( 'set:isEnabled', evt => { - evt.return = false; - evt.stop(); - }, { priority: 'highest' } ); - - command.isEnabled = false; + expect( command.isEnabled ).to.be.false; - const targetRange = Range.createFromParentsAndOffsets( doc.getRoot(), 1, doc.getRoot(), 1 ); + const targetRange = Range.createFromParentsAndOffsets( doc.getRoot(), 0, doc.getRoot(), 0 ); const targetViewRange = editor.editing.mapper.toViewRange( targetRange ); viewDocument.fire( 'clipboardInput', { dataTransfer, targetRanges: [ targetViewRange ] } ); const id = fileRepository.getLoader( fileMock ).id; expect( getModelData( model ) ).to.equal( - `foo[]` + `[]foo` ); } ); @@ -258,7 +254,7 @@ describe( 'ImageUploadEditing', () => { viewDocument.fire( 'clipboardInput', { dataTransfer, targetRanges: [ targetViewRange ] } ); } ).to.not.throw(); - expect( getModelData( model ) ).to.equal( '[]foo' ); + expect( getModelData( model ) ).to.equal( 'foo[]' ); sinon.assert.calledOnce( logStub ); } ); @@ -300,7 +296,7 @@ describe( 'ImageUploadEditing', () => { it( 'should use read data once it is present', done => { const file = createNativeFileMock(); setModelData( model, '{}foo bar' ); - editor.execute( 'imageUpload', { file } ); + editor.execute( 'imageUpload', { files: file } ); model.once( '_change', () => { expect( getViewData( view ) ).to.equal( @@ -320,7 +316,7 @@ describe( 'ImageUploadEditing', () => { it( 'should replace read data with server response once it is present', done => { const file = createNativeFileMock(); setModelData( model, '{}foo bar' ); - editor.execute( 'imageUpload', { file } ); + editor.execute( 'imageUpload', { files: file } ); model.document.once( 'change', () => { model.document.once( 'change', () => { @@ -351,7 +347,7 @@ describe( 'ImageUploadEditing', () => { }, { priority: 'high' } ); setModelData( model, '{}foo bar' ); - editor.execute( 'imageUpload', { file } ); + editor.execute( 'imageUpload', { files: file } ); nativeReaderMock.mockError( 'Reading error.' ); } ); @@ -367,7 +363,7 @@ describe( 'ImageUploadEditing', () => { }, { priority: 'high' } ); setModelData( model, '{}foo bar' ); - editor.execute( 'imageUpload', { file } ); + editor.execute( 'imageUpload', { files: file } ); nativeReaderMock.abort(); setTimeout( () => { @@ -391,7 +387,7 @@ describe( 'ImageUploadEditing', () => { } ); setModelData( model, '{}foo bar' ); - editor.execute( 'imageUpload', { file } ); + editor.execute( 'imageUpload', { files: file } ); sinon.assert.calledOnce( loadSpy ); @@ -428,7 +424,7 @@ describe( 'ImageUploadEditing', () => { evt.stop(); }, { priority: 'high' } ); - editor.execute( 'imageUpload', { file } ); + editor.execute( 'imageUpload', { files: file } ); model.document.once( 'change', () => { model.document.once( 'change', () => { @@ -445,7 +441,7 @@ describe( 'ImageUploadEditing', () => { it( 'should abort upload if image is removed', () => { const file = createNativeFileMock(); setModelData( model, '{}foo bar' ); - editor.execute( 'imageUpload', { file } ); + editor.execute( 'imageUpload', { files: file } ); const abortSpy = testUtils.sinon.spy( loader, 'abort' ); @@ -464,7 +460,7 @@ describe( 'ImageUploadEditing', () => { it( 'should not abort and not restart upload when image is moved', () => { const file = createNativeFileMock(); setModelData( model, '{}foo bar' ); - editor.execute( 'imageUpload', { file } ); + editor.execute( 'imageUpload', { files: file } ); const abortSpy = testUtils.sinon.spy( loader, 'abort' ); const loadSpy = testUtils.sinon.spy( loader, 'read' ); @@ -489,7 +485,7 @@ describe( 'ImageUploadEditing', () => { evt.stop(); }, { priority: 'high' } ); - editor.execute( 'imageUpload', { file } ); + editor.execute( 'imageUpload', { files: file } ); model.document.once( 'change', () => { // This is called after "manual" remove. @@ -525,7 +521,7 @@ describe( 'ImageUploadEditing', () => { it( 'should create responsive image if server return multiple images', done => { const file = createNativeFileMock(); setModelData( model, '{}foo bar' ); - editor.execute( 'imageUpload', { file } ); + editor.execute( 'imageUpload', { files: file } ); model.document.once( 'change', () => { model.document.once( 'change', () => { diff --git a/tests/imageupload/imageuploadprogress.js b/tests/imageupload/imageuploadprogress.js index 6715af20..ffef57b9 100644 --- a/tests/imageupload/imageuploadprogress.js +++ b/tests/imageupload/imageuploadprogress.js @@ -74,7 +74,7 @@ describe( 'ImageUploadProgress', () => { it( 'should convert image\'s "reading" uploadStatus attribute', () => { setModelData( model, '[]foo' ); - editor.execute( 'imageUpload', { file: createNativeFileMock() } ); + editor.execute( 'imageUpload', { files: createNativeFileMock() } ); expect( getViewData( view ) ).to.equal( '[
' + @@ -86,7 +86,7 @@ describe( 'ImageUploadProgress', () => { it( 'should convert image\'s "uploading" uploadStatus attribute', done => { setModelData( model, '[]foo' ); - editor.execute( 'imageUpload', { file: createNativeFileMock() } ); + editor.execute( 'imageUpload', { files: createNativeFileMock() } ); model.document.once( 'change', () => { expect( getViewData( view ) ).to.equal( @@ -167,7 +167,7 @@ describe( 'ImageUploadProgress', () => { it( 'should update progressbar width on progress', done => { setModelData( model, '[]foo' ); - editor.execute( 'imageUpload', { file: createNativeFileMock() } ); + editor.execute( 'imageUpload', { files: createNativeFileMock() } ); model.document.once( 'change', () => { adapterMock.mockProgress( 40, 100 ); @@ -189,7 +189,7 @@ describe( 'ImageUploadProgress', () => { const clock = testUtils.sinon.useFakeTimers(); setModelData( model, '[]foo' ); - editor.execute( 'imageUpload', { file: createNativeFileMock() } ); + editor.execute( 'imageUpload', { files: createNativeFileMock() } ); model.document.once( 'change', () => { model.document.once( 'change', () => { @@ -222,7 +222,7 @@ describe( 'ImageUploadProgress', () => { uploadProgress.placeholder = base64Sample; setModelData( model, '[]foo' ); - editor.execute( 'imageUpload', { file: createNativeFileMock() } ); + editor.execute( 'imageUpload', { files: createNativeFileMock() } ); expect( getViewData( view ) ).to.equal( '[
' + @@ -238,7 +238,7 @@ describe( 'ImageUploadProgress', () => { }, { priority: 'highest' } ); setModelData( model, '[]foo' ); - editor.execute( 'imageUpload', { file: createNativeFileMock() } ); + editor.execute( 'imageUpload', { files: createNativeFileMock() } ); expect( getViewData( view ) ).to.equal( '[
]

foo

' @@ -276,7 +276,7 @@ describe( 'ImageUploadProgress', () => { testUtils.sinon.stub( env, 'isEdge' ).get( () => true ); setModelData( model, '[]foo' ); - editor.execute( 'imageUpload', { file: createNativeFileMock() } ); + editor.execute( 'imageUpload', { files: createNativeFileMock() } ); model.document.once( 'change', () => { model.document.once( 'change', () => { diff --git a/tests/imageupload/imageuploadui.js b/tests/imageupload/imageuploadui.js index aa1abe8f..68326d4a 100644 --- a/tests/imageupload/imageuploadui.js +++ b/tests/imageupload/imageuploadui.js @@ -99,7 +99,18 @@ describe( 'ImageUploadUI', () => { button.fire( 'done', files ); sinon.assert.calledOnce( executeStub ); expect( executeStub.firstCall.args[ 0 ] ).to.equal( 'imageUpload' ); - expect( executeStub.firstCall.args[ 1 ].file ).to.equal( files[ 0 ] ); + expect( executeStub.firstCall.args[ 1 ].files ).to.deep.equal( files ); + } ); + + it( 'should execute imageUpload command with multiple files', () => { + const executeStub = sinon.stub( editor, 'execute' ); + const button = editor.ui.componentFactory.create( 'imageUpload' ); + const files = [ createNativeFileMock(), createNativeFileMock(), createNativeFileMock() ]; + + button.fire( 'done', files ); + sinon.assert.calledOnce( executeStub ); + expect( executeStub.firstCall.args[ 0 ] ).to.equal( 'imageUpload' ); + expect( executeStub.firstCall.args[ 1 ].files ).to.deep.equal( files ); } ); it( 'should optimize the insertion position', () => { @@ -160,6 +171,6 @@ describe( 'ImageUploadUI', () => { button.fire( 'done', files ); sinon.assert.calledOnce( executeStub ); expect( executeStub.firstCall.args[ 0 ] ).to.equal( 'imageUpload' ); - expect( executeStub.firstCall.args[ 1 ].file ).to.equal( files[ 0 ] ); + expect( executeStub.firstCall.args[ 1 ].files ).to.deep.equal( [ files[ 0 ] ] ); } ); } );