Skip to content

Commit

Permalink
Fix not working drag & drop on images that are in uploading state.
Browse files Browse the repository at this point in the history
  • Loading branch information
Mati365 committed Nov 8, 2024
1 parent faaa044 commit 228311c
Show file tree
Hide file tree
Showing 2 changed files with 219 additions and 1 deletion.
74 changes: 73 additions & 1 deletion packages/ckeditor5-image/src/imageupload/imageuploadediting.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ import {
type Writer,
type DataTransfer,
type ViewElement,
type NodeAttributes
type NodeAttributes,
type DowncastAttributeEvent
} from 'ckeditor5/src/engine.js';

import { Notification } from 'ckeditor5/src/ui.js';
Expand Down Expand Up @@ -66,6 +67,14 @@ export default class ImageUploadEditing extends Plugin {
*/
private readonly _uploadImageElements: Map<string, Element>;

/**
* An internal mapping of {@link module:upload/filerepository~FileLoader#id file loader UIDs} and
* upload responses for handling images dragged during their upload process. When such images are later
* dropped, their original upload IDs no longer exist in the registry (as the original upload completed).
* This map preserves the upload responses to properly handle such cases.
*/
private readonly _uploadedImages = new Map<string, UploadResponse>();

/**
* @inheritDoc
*/
Expand Down Expand Up @@ -106,6 +115,13 @@ export default class ImageUploadEditing extends Plugin {
key: 'uploadId'
},
model: 'uploadId'
} )
.attributeToAttribute( {
view: {
name: 'img',
key: 'data-ck-upload-id'
},
model: 'uploadId'
} );

// Handle pasted images.
Expand Down Expand Up @@ -217,6 +233,19 @@ export default class ImageUploadEditing extends Plugin {
const loader = fileRepository.loaders.get( uploadId );

if ( !loader ) {
// If the loader does not exist, it means that the image was already uploaded
// and the loader promise was removed from the registry. In that scenario we need
// to restore response object from the internal map.
if ( !isInsertedInGraveyard && this._uploadedImages.has( uploadId ) ) {
// Fire `uploadComplete` to set proper attributes on the image element.
this.fire<ImageUploadCompleteEvent>( 'uploadComplete', {
data: this._uploadedImages.get( uploadId )!,
imageElement: imageElement as Element
} );

this._uploadedImages.delete( uploadId );
}

continue;
}

Expand Down Expand Up @@ -274,12 +303,16 @@ export default class ImageUploadEditing extends Plugin {
schema.extend( 'imageBlock', {
allowAttributes: [ 'uploadId', 'uploadStatus' ]
} );

this._registerConverters( 'imageBlock' );
}

if ( this.editor.plugins.has( 'ImageInlineEditing' ) ) {
schema.extend( 'imageInline', {
allowAttributes: [ 'uploadId', 'uploadStatus' ]
} );

this._registerConverters( 'imageInline' );
}
}

Expand Down Expand Up @@ -360,6 +393,8 @@ export default class ImageUploadEditing extends Plugin {
}

this.fire<ImageUploadCompleteEvent>( 'uploadComplete', { data, imageElement } );

this._uploadedImages.set( loader.id, data );
} );

clean();
Expand Down Expand Up @@ -445,6 +480,43 @@ export default class ImageUploadEditing extends Plugin {
writer.setAttributes( attributes, image );
}
}

/**
* Registers image upload converters.
*
* @param imageType The type of the image.
*/
private _registerConverters( imageType: 'imageBlock' | 'imageInline' ) {
const { conversion, plugins } = this.editor;

const fileRepository = plugins.get( FileRepository );
const imageUtils = plugins.get( ImageUtils );

// It sets `data-ck-upload-id` attribute on the view image elements that are not fully uploaded.
// It avoids the situation when image disappears when it's being moved and upload is not finished yet.
// See more: https://github.com/ckeditor/ckeditor5/issues/16967
conversion.for( 'dataDowncast' ).add( dispatcher => {
dispatcher.on<DowncastAttributeEvent>( `attribute:uploadId:${ imageType }`, ( evt, data, conversionApi ) => {
if ( !conversionApi.consumable.test( data.item, evt.name ) ) {
return;
}

const loader = fileRepository.loaders.get( data.attributeNewValue as string );

if ( !loader || !loader.data ) {
return null;
}

const viewElement = conversionApi.mapper.toViewElement( data.item as Element )!;
const img = imageUtils.findViewImgElement( viewElement );

if ( img ) {
conversionApi.consumable.consume( data.item, evt.name );
conversionApi.writer.setAttribute( 'data-ck-upload-id', loader.id, img );
}
} );
} );
}
}

/**
Expand Down
146 changes: 146 additions & 0 deletions packages/ckeditor5-image/tests/imageupload/imageuploadediting.js
Original file line number Diff line number Diff line change
Expand Up @@ -1552,6 +1552,138 @@ describe( 'ImageUploadEditing', () => {
} );
} );

describe( 'data downcast conversion of images with uploading state', () => {
it( 'should dump the `data-ck-upload-id` into the data', async () => {
const onDispatch = sinon.spy( ( evt, data, conversionApi ) => {
const wasConsumed = conversionApi.consumable.test( data.item, 'attribute:uploadId:imageInline' );

expect( wasConsumed ).to.be.true;
} );

editor.conversion.for( 'downcast' ).add( dispatcher =>
dispatcher.on( 'attribute:uploadId:imageInline', onDispatch, { priority: 'high' } )
);

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

const file = createNativeFileMock();
editor.execute( 'uploadImage', { file } );
loader.file.then( () => nativeReaderMock.mockSuccess( base64Sample ) );

await timeout( 50 );

const uploadId = adapterMocks[ 0 ].loader.id;

expect( getModelData( editor.model ) ).to.be.equal(
`<paragraph>[<imageInline uploadId="${ uploadId }" uploadStatus="uploading"></imageInline>]foo</paragraph>`
);

expect( onDispatch ).to.be.calledOnce;
expect( editor.getData() ).to.be.equal(
`<p><img data-ck-upload-id="${ uploadId }">foo</p>`
);
} );

it( 'should not crash if uploadId of down casted image is not found in loaders repository', async () => {
setModelData( model, '<paragraph>[]foo</paragraph>' );

const file = createNativeFileMock();
editor.execute( 'uploadImage', { file } );
loader.file.then( () => nativeReaderMock.mockSuccess( base64Sample ) );

await timeout( 50 );

const uploadId = adapterMocks[ 0 ].loader.id;

sinon
.stub( fileRepository.loaders, 'get' )
.withArgs( uploadId )
.returns( null );

expect( getModelData( editor.model ) ).to.be.equal(
`<paragraph>[<imageInline uploadId="${ uploadId }" uploadStatus="uploading"></imageInline>]foo</paragraph>`
);

expect( editor.getData() ).to.be.equal( '<p><img>foo</p>' );
} );

it( 'should not downcast consumed uploadId image attribute', async () => {
editor.conversion.for( 'downcast' ).add( dispatcher =>
dispatcher.on( 'attribute:uploadId:imageInline', ( evt, data, conversionApi ) => {
conversionApi.consumable.consume( data.item, 'attribute:uploadId:imageInline' );
}, { priority: 'high' } )
);

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

const file = createNativeFileMock();
editor.execute( 'uploadImage', { file } );
loader.file.then( () => nativeReaderMock.mockSuccess( base64Sample ) );

await timeout( 50 );

const uploadId = adapterMocks[ 0 ].loader.id;

expect( getModelData( editor.model ) ).to.be.equal(
`<paragraph>[<imageInline uploadId="${ uploadId }" uploadStatus="uploading"></imageInline>]foo</paragraph>`
);

expect( editor.getData() ).to.be.equal( '<p><img>foo</p>' );
} );

it( 'should restore image from `_uploadedImages` if it was pasted from clipboard', async () => {
setModelData( model, '<paragraph>[]foo</paragraph>' );

const file = createNativeFileMock();
editor.execute( 'uploadImage', { file } );
loader.file.then( () => nativeReaderMock.mockSuccess( base64Sample ) );

await timeout( 50 );

// Let's copy image in uploading state.
const uploadId = adapterMocks[ 0 ].loader.id;
expect( getModelData( editor.model ) ).to.be.equal(
`<paragraph>[<imageInline uploadId="${ uploadId }" uploadStatus="uploading"></imageInline>]foo</paragraph>`
);

// Lets check if content of clipboard is correct.
const data = {
dataTransfer: createDataTransfer(),
preventDefault: () => {},
stopPropagation: () => {}
};

viewDocument.fire( 'copy', data );
expect( data.dataTransfer.getData( 'text/html' ) ).to.equal( `<img data-ck-upload-id="${ uploadId }">` );

// Let's resolve uploading status and ensure that image is loaded.
await new Promise( res => {
model.document.once( 'change', res, { priority: 'lowest' } );
loader.file.then( () => adapterMocks[ 0 ].mockSuccess( { default: '/assets/sample.png', 800: 'image-800.png' } ) );
} );

expect( editor.getData() ).to.be.equal(
'<p><img src="/assets/sample.png" srcset="image-800.png 800w" sizes="100vw" width="800">foo</p>'
);

// Make sure it's no longer present in registry, so image upload is completed.
expect( fileRepository.loaders.get( uploadId ) ).to.be.null;

// Let's paste the image from clipboard, it has upload id, which should be stored in plugin cache.
setModelData( model, '<paragraph>hello[]</paragraph>' );

viewDocument.fire( 'paste', {
dataTransfer: mockDataTransfer( `<img data-ck-upload-id=${ uploadId } />` ),
preventDefault: () => {},
stopPropagation: () => {}
} );

expect( editor.getData() ).to.be.equal(
'<p>hello<img src="/assets/sample.png" srcset="image-800.png 800w" sizes="100vw" width="800"></p>'
);
} );
} );

// Helper for validating clipboard and model data as a result of a paste operation. This function checks both clipboard
// data and model data synchronously (`expectedClipboardData`, `expectedModel`) and then the model data after `loader.file`
// promise is resolved (so model state after successful/failed file fetch attempt).
Expand Down Expand Up @@ -1685,3 +1817,17 @@ function base64ToBlob( base64Data ) {
function timeout( ms ) {
return new Promise( res => setTimeout( res, ms ) );
}

function createDataTransfer() {
const store = new Map();

return {
setData( type, data ) {
store.set( type, data );
},

getData( type ) {
return store.get( type );
}
};
}

0 comments on commit 228311c

Please sign in to comment.