Skip to content

Commit

Permalink
Fix upcast
Browse files Browse the repository at this point in the history
  • Loading branch information
Mati365 committed Nov 18, 2024
1 parent 7c23f16 commit 94a51d3
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 8 deletions.
38 changes: 30 additions & 8 deletions packages/ckeditor5-image/src/imageupload/imageuploadediting.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ import {
type DataTransfer,
type ViewElement,
type NodeAttributes,
type DowncastAttributeEvent
type DowncastAttributeEvent,
type UpcastElementEvent
} from 'ckeditor5/src/engine.js';

import { Notification } from 'ckeditor5/src/ui.js';
Expand Down Expand Up @@ -116,13 +117,34 @@ export default class ImageUploadEditing extends Plugin {
},
model: 'uploadId'
} )
.attributeToAttribute( {
view: {
name: 'img',
key: 'data-ck-upload-id'
},
model: 'uploadId'
} );

// Handle the case when the image is not fully uploaded yet but it's being moved.
// See more: https://github.com/ckeditor/ckeditor5/pull/17327
.add( dispatcher => dispatcher.on<UpcastElementEvent>( 'element:img', ( evt, data, conversionApi ) => {
const uploadId = data.viewItem.getAttribute( 'data-ck-upload-id' );

if ( !uploadId ) {
return;
}

if ( !data.modelRange ) {
Object.assign( data, conversionApi.convertChildren( data.viewItem, data.modelCursor ) );
}

const [ modelElement ] = Array.from( data.modelRange!.getItems() );
const loader = fileRepository.loaders.get( uploadId as string );

if ( modelElement ) {
// Handle case when `uploadId` is set on the image element but the loader is not present in the registry.
// It may happen when the image was successfully uploaded and the loader was removed from the registry.
// It's still present in the `_uploadedImages` map though. It's why we do not place this line in the condition below.
conversionApi.writer.setAttribute( 'uploadId', uploadId, modelElement );

if ( loader && loader.data ) {
conversionApi.writer.setAttribute( 'uploadStatus', loader.status, modelElement );
}
}
} ) );

// Handle pasted images.
// For every image file, a new file loader is created and a placeholder image is
Expand Down
24 changes: 24 additions & 0 deletions packages/ckeditor5-image/tests/imageupload/imageuploadediting.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils.js';

import FileRepository from '@ckeditor/ckeditor5-upload/src/filerepository.js';
import { UploadAdapterMock, createNativeFileMock, NativeFileReaderMock } from '@ckeditor/ckeditor5-upload/tests/_utils/mocks.js';
import UpcastDispatcher from '@ckeditor/ckeditor5-engine/src/conversion/upcastdispatcher.js';

import { setData as setModelData, getData as getModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model.js';
import { getData as getViewData, stringify as stringifyView } from '@ckeditor/ckeditor5-engine/src/dev-utils/view.js';
Expand Down Expand Up @@ -1684,6 +1685,29 @@ describe( 'ImageUploadEditing', () => {
} );
} );

describe( 'data upcast of `data-ck-upload-id` attribute', () => {
it( 'should upcast `data-ck-upload-id` attribute', () => {
editor.setData( '<p><img data-ck-upload-id="123"></p>' );

expect( getModelData( model, { withoutSelection: true } ) ).to.equal(
'<paragraph><imageInline uploadId="123"></imageInline></paragraph>'
);
} );

it( 'should upcast `uploadStatus` if image is present in registry', () => {
sinon.stub( fileRepository.loaders, 'get' ).withArgs( '123' ).returns( {
status: 'uploading',
data: {}
} );

editor.setData( '<p><img data-ck-upload-id="123"></p>' );

expect( getModelData( model, { withoutSelection: true } ) ).to.equal(
'<paragraph><imageInline uploadId="123" uploadStatus="uploading"></imageInline></paragraph>'
);
} );
} );

// 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

0 comments on commit 94a51d3

Please sign in to comment.