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 #228 from ckeditor/t/225
Browse files Browse the repository at this point in the history 
Other: The ImageUploadCommand should check whether it can be executed in the selection context. Closes #225. Closes #227. Closes #235.

BREAKING CHANGE: The `options.file` property was renamed to options.files for 'imageUpload' command.
BREAKING CHANGE: The `options.insertAt` property for 'imageUpload' command was removed. The command will now use model's selection.
  • Loading branch information
@jodator
jodator authored Sep 28, 2018
2 parents 647f09d + 678d6ac commit 4c1f27f
Show file tree
Hide file tree
Showing 7 changed files with 208 additions and 111 deletions.
101 changes: 72 additions & 29 deletions src/imageupload/imageuploadcommand.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.<File>} 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;
}
39 changes: 12 additions & 27 deletions src/imageupload/imageuploadediting.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down
9 changes: 3 additions & 6 deletions src/imageupload/imageuploadui.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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 } );
}
} );

Expand Down
105 changes: 85 additions & 20 deletions tests/imageupload/imageuploadcommand.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand All @@ -43,7 +42,6 @@ describe( 'ImageUploadCommand', () => {
.then( newEditor => {
editor = newEditor;
model = editor.model;
doc = model.document;

command = new ImageUploadCommand( editor );

Expand All @@ -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, '<paragraph>f[o]o</paragraph>' );
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( `<paragraph>f</paragraph>[<image uploadId="${ id }"></image>]<paragraph>o</paragraph>` );
it( 'should be true when the selection is in empty block', () => {
setModelData( model, '<paragraph>[]</paragraph>' );

expect( command.isEnabled ).to.be.true;
} );

it( 'should insert directly at specified position (options.insertAt)', () => {
const file = createNativeFileMock();
setModelData( model, '<paragraph>f[]oo</paragraph>' );
it( 'should be true when the selection directly in a paragraph', () => {
setModelData( model, '<paragraph>foo[]</paragraph>' );
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, '<block>foo[]</block>' );
expect( command.isEnabled ).to.be.true;
} );

it( 'should be false when the selection is on other image', () => {
setModelData( model, '[<image></image>]' );
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, '<image><caption>[]</caption></image>' );
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, '[<object></object>]' );

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, '<object>[]</object>' );

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, '<block><paragraph>[]</paragraph></block>' );

expect( command.isEnabled ).to.be.false;
} );
} );

describe( 'execute()', () => {
it( 'should insert image at selection position as other widgets', () => {
const file = createNativeFileMock();
setModelData( model, '<paragraph>f[o]o</paragraph>' );

command.execute( { file, insertAt } );
command.execute( { files: file } );

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

it( 'should use parent batch', () => {
Expand All @@ -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 );
} );
Expand All @@ -108,7 +173,7 @@ describe( 'ImageUploadCommand', () => {

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

command.execute( { file } );
command.execute( { files: file } );

expect( getModelData( model ) ).to.equal( '<other>[]</other>' );
} );
Expand All @@ -123,7 +188,7 @@ describe( 'ImageUploadCommand', () => {
setModelData( model, '<paragraph>fo[]o</paragraph>' );

expect( () => {
command.execute( { file } );
command.execute( { files: file } );
} ).to.not.throw();

expect( getModelData( model ) ).to.equal( '<paragraph>fo[]o</paragraph>' );
Expand Down
Loading

0 comments on commit 4c1f27f

Please sign in to comment.