Parcourir la source

Merge pull request #192 from ckeditor/t/191

Fix: Fixed image upload progress updates. Now each upload status is treated separately. Closes #191.
Oskar Wróbel il y a 7 ans
Parent
commit
943297577a

+ 104 - 27
packages/ckeditor5-image/src/imageupload/imageuploadprogress.js

@@ -65,16 +65,16 @@ export default class ImageUploadProgress extends Plugin {
 		}
 
 		const fileRepository = editor.plugins.get( FileRepository );
-		const placeholder = this.placeholder;
 		const status = data.attributeNewValue;
+		const placeholder = this.placeholder;
 		const viewFigure = editor.editing.mapper.toViewElement( modelImage );
 		const viewWriter = conversionApi.writer;
 
-		// Show placeholder with infinite progress bar on the top while image is read from disk.
 		if ( status == 'reading' ) {
-			viewWriter.addClass( [ 'ck-appear', 'ck-infinite-progress', 'ck-image-upload-placeholder' ], viewFigure );
-			const viewImg = viewFigure.getChild( 0 );
-			viewWriter.setAttribute( 'src', placeholder, viewImg );
+			// Start "appearing" effect and show placeholder with infinite progress bar on the top
+			// while image is read from disk.
+			_startAppearEffect( viewFigure, viewWriter );
+			_showPlaceholder( placeholder, viewFigure, viewWriter );
 
 			return;
 		}
@@ -83,39 +83,116 @@ export default class ImageUploadProgress extends Plugin {
 		if ( status == 'uploading' ) {
 			const loader = fileRepository.loaders.get( uploadId );
 
-			if ( loader ) {
-				const progressBar = createProgressBar( viewWriter );
-
-				viewWriter.removeClass( [ 'ck-infinite-progress', 'ck-image-upload-placeholder' ], viewFigure );
-				viewWriter.insert( ViewPosition.createAt( viewFigure, 'end' ), progressBar );
-
-				// Update progress bar width when uploadedPercent is changed.
-				loader.on( 'change:uploadedPercent', ( evt, name, value ) => {
-					editor.editing.view.change( writer => {
-						writer.setStyle( 'width', value + '%', progressBar );
-					} );
-				} );
+			// Start appear effect if needed - see https://github.com/ckeditor/ckeditor5-image/issues/191.
+			_startAppearEffect( viewFigure, viewWriter );
+
+			if ( !loader ) {
+				// There is no loader associated with uploadId - this means that image came from external changes.
+				// In such cases we still want to show the placeholder until image is fully uploaded.
+				// Show placeholder if needed - see https://github.com/ckeditor/ckeditor5-image/issues/191.
+				_showPlaceholder( placeholder, viewFigure, viewWriter );
+			} else {
+				// Hide placeholder and initialize progress bar showing upload progress.
+				_hidePlaceholder( viewFigure, viewWriter );
+				_showProgressBar( viewFigure, viewWriter, loader, editor.editing.view );
 			}
 
 			return;
 		}
 
-		// Hide progress bar and clean up classes.
-		const progressBar = getProgressBar( viewFigure );
-
-		if ( progressBar ) {
-			viewWriter.remove( ViewRange.createOn( progressBar ) );
-		} else {
-			viewWriter.removeClass( 'ck-infinite-progress', viewFigure );
-		}
-
-		viewWriter.removeClass( [ 'ck-appear', 'ck-image-upload-placeholder' ], viewFigure );
+		// Clean up.
+		_hideProgressBar( viewFigure, viewWriter );
+		_hidePlaceholder( viewFigure, viewWriter );
+		_stopAppearEffect( viewFigure, viewWriter );
 	}
 }
 
 // Symbol added to progress bar UIElement to distinguish it from other elements.
 const progressBarSymbol = Symbol( 'progress-bar' );
 
+// Adds ck-appear class to the image figure if one is not already applied.
+//
+// @param {module:engine/view/containerelement~ContainerElement} viewFigure
+// @param {module:engine/view/writer~Writer} writer
+function _startAppearEffect( viewFigure, writer ) {
+	if ( !viewFigure.hasClass( 'ck-appear' ) ) {
+		writer.addClass( 'ck-appear', viewFigure );
+	}
+}
+
+// Removes ck-appear class to the image figure if one is not already removed.
+//
+// @param {module:engine/view/containerelement~ContainerElement} viewFigure
+// @param {module:engine/view/writer~Writer} writer
+function _stopAppearEffect( viewFigure, writer ) {
+	writer.removeClass( 'ck-appear', viewFigure );
+}
+
+// Shows placeholder together with infinite progress bar on given image figure.
+//
+// @param {module:engine/view/containerelement~ContainerElement} viewFigure
+// @param {module:engine/view/writer~Writer} writer
+function _showPlaceholder( placeholder, viewFigure, writer ) {
+	if ( !viewFigure.hasClass( 'ck-image-upload-placeholder' ) ) {
+		writer.addClass( 'ck-image-upload-placeholder', viewFigure );
+	}
+
+	if ( !viewFigure.hasClass( 'ck-infinite-progress' ) ) {
+		writer.addClass( 'ck-infinite-progress', viewFigure );
+	}
+
+	const viewImg = viewFigure.getChild( 0 );
+
+	if ( viewImg.getAttribute( 'src' ) !== placeholder ) {
+		writer.setAttribute( 'src', placeholder, viewImg );
+	}
+}
+
+// Removes placeholder together with infinite progress bar on given image figure.
+//
+// @param {module:engine/view/containerelement~ContainerElement} viewFigure
+// @param {module:engine/view/writer~Writer} writer
+function _hidePlaceholder( viewFigure, writer ) {
+	if ( viewFigure.hasClass( 'ck-image-upload-placeholder' ) ) {
+		writer.removeClass( 'ck-image-upload-placeholder', viewFigure );
+	}
+
+	if ( viewFigure.hasClass( 'ck-infinite-progress' ) ) {
+		writer.removeClass( 'ck-infinite-progress', viewFigure );
+	}
+}
+
+// Shows progress bar displaying upload progress.
+// Attaches it to the file loader to update when upload percentace is changed.
+//
+// @param {module:engine/view/containerelement~ContainerElement} viewFigure
+// @param {module:engine/view/writer~Writer} writer
+// @param {module:upload/filerepository~FileLoader} loader
+// @param {module:engine/view/view~View} view
+function _showProgressBar( viewFigure, writer, loader, view ) {
+	const progressBar = createProgressBar( writer );
+	writer.insert( ViewPosition.createAt( viewFigure, 'end' ), progressBar );
+
+	// Update progress bar width when uploadedPercent is changed.
+	loader.on( 'change:uploadedPercent', ( evt, name, value ) => {
+		view.change( writer => {
+			writer.setStyle( 'width', value + '%', progressBar );
+		} );
+	} );
+}
+
+// Hides upload progress bar.
+//
+// @param {module:engine/view/containerelement~ContainerElement} viewFigure
+// @param {module:engine/view/writer~Writer} writer
+function _hideProgressBar( viewFigure, writer ) {
+	const progressBar = getProgressBar( viewFigure );
+
+	if ( progressBar ) {
+		writer.remove( ViewRange.createOn( progressBar ) );
+	}
+}
+
 // Create progress bar element using {@link module:engine/view/uielement~UIElement}.
 //
 // @private

+ 40 - 0
packages/ckeditor5-image/tests/imageupload/imageuploadprogress.js

@@ -97,6 +97,46 @@ describe( 'ImageUploadProgress', () => {
 		nativeReaderMock.mockSuccess( base64Sample );
 	} );
 
+	it( 'should work correctly when there is no "reading" status and go straight to "uploading"', () => {
+		const fileRepository = editor.plugins.get( FileRepository );
+		const file = createNativeFileMock();
+		const loader = fileRepository.createLoader( file );
+
+		setModelData( model, '<image></image>' );
+		const image = document.getRoot().getChild( 0 );
+
+		// Set attributes directly on image to simulate instant "uploading" status.
+		model.change( writer => {
+			writer.setAttribute( 'uploadStatus', 'uploading', image );
+			writer.setAttribute( 'uploadId', loader.id, image );
+			writer.setAttribute( 'src', 'image.png', image );
+		} );
+
+		expect( getViewData( view ) ).to.equal(
+			'[<figure class="ck-appear ck-widget image" contenteditable="false">' +
+				'<img src="image.png"></img>' +
+				'<div class="ck-progress-bar"></div>' +
+			'</figure>]'
+		);
+	} );
+
+	it( 'should work correctly when there is no "reading" status and go straight to "uploading" - external changes', () => {
+		setModelData( model, '<image></image>' );
+		const image = document.getRoot().getChild( 0 );
+
+		// Set attributes directly on image to simulate instant "uploading" status.
+		model.change( writer => {
+			writer.setAttribute( 'uploadStatus', 'uploading', image );
+			writer.setAttribute( 'uploadId', '12345', image );
+		} );
+
+		expect( getViewData( view ) ).to.equal(
+			'[<figure class="ck-appear ck-image-upload-placeholder ck-infinite-progress ck-widget image" contenteditable="false">' +
+				`<img src="data:image/svg+xml;utf8,${ imagePlaceholder }"></img>` +
+			'</figure>]'
+		);
+	} );
+
 	it( 'should update progressbar width on progress', done => {
 		setModelData( model, '<paragraph>[]foo</paragraph>' );
 		editor.execute( 'imageUpload', { file: createNativeFileMock() } );