diff --git a/packages/ckeditor5-image/src/imageresize/imageresizehandles.js b/packages/ckeditor5-image/src/imageresize/imageresizehandles.js index c5e1725c60f..be553d0d68f 100644 --- a/packages/ckeditor5-image/src/imageresize/imageresizehandles.js +++ b/packages/ckeditor5-image/src/imageresize/imageresizehandles.js @@ -9,6 +9,7 @@ import Plugin from '@ckeditor/ckeditor5-core/src/plugin'; import WidgetResize from '@ckeditor/ckeditor5-widget/src/widgetresize'; +import ImageLoadObserver from '../image/imageloadobserver'; /** * The image resize by handles feature. @@ -37,21 +38,46 @@ export default class ImageResizeHandles extends Plugin { * @inheritDoc */ init() { + const command = this.editor.commands.get( 'imageResize' ); + this.bind( 'isEnabled' ).to( command ); + + this._setupResizerCreator(); + } + + /** + * Attaches the listeners responsible for creating a resizer for each image. + * + * @private + */ + _setupResizerCreator() { const editor = this.editor; - const command = editor.commands.get( 'imageResize' ); + const editingView = editor.editing.view; - this.bind( 'isEnabled' ).to( command ); + editingView.addObserver( ImageLoadObserver ); + + this.listenTo( editingView.document, 'imageLoaded', ( evt, domEvent ) => { + const imageView = editor.editing.view.domConverter.domToView( domEvent.target ); + const widgetView = imageView.findAncestor( 'figure' ); + let resizer = this.editor.plugins.get( WidgetResize ).getResizerByViewElement( widgetView ); + + if ( resizer ) { + // There are rare cases when image will be triggered multiple times for the same widget, e.g. when + // image's src was changed after upload (https://github.com/ckeditor/ckeditor5/pull/8108#issuecomment-708302992). + resizer.redraw(); + + return; + } - editor.editing.downcastDispatcher.on( 'insert:image', ( evt, data, conversionApi ) => { - const widget = conversionApi.mapper.toViewElement( data.item ); + const mapper = editor.editing.mapper; + const imageModel = mapper.toModelElement( widgetView ); - const resizer = editor.plugins + resizer = editor.plugins .get( WidgetResize ) .attachTo( { unit: editor.config.get( 'image.resizeUnit' ), - modelElement: data.item, - viewElement: widget, + modelElement: imageModel, + viewElement: widgetView, editor, getHandleHost( domWidgetElement ) { @@ -62,7 +88,7 @@ export default class ImageResizeHandles extends Plugin { }, // TODO consider other positions. isCentered() { - const imageStyle = data.item.getAttribute( 'imageStyle' ); + const imageStyle = imageModel.getAttribute( 'imageStyle' ); return !imageStyle || imageStyle == 'full' || imageStyle == 'alignCenter'; }, @@ -73,14 +99,14 @@ export default class ImageResizeHandles extends Plugin { } ); resizer.on( 'updateSize', () => { - if ( !widget.hasClass( 'image_resized' ) ) { - editor.editing.view.change( writer => { - writer.addClass( 'image_resized', widget ); + if ( !widgetView.hasClass( 'image_resized' ) ) { + editingView.change( writer => { + writer.addClass( 'image_resized', widgetView ); } ); } } ); resizer.bind( 'isEnabled' ).to( this ); - }, { priority: 'low' } ); + } ); } } diff --git a/packages/ckeditor5-image/tests/imageresize/_utils/utils.js b/packages/ckeditor5-image/tests/imageresize/_utils/utils.js new file mode 100644 index 00000000000..f4344aac024 --- /dev/null +++ b/packages/ckeditor5-image/tests/imageresize/_utils/utils.js @@ -0,0 +1,37 @@ +/** + * @license Copyright (c) 2003-2020, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license + */ + +import ImageLoadObserver from '../../../src/image/imageloadobserver'; + +// A 100x50 black png image +export const IMAGE_SRC_FIXTURE = 'data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAGQAAAAyCAQAAAAAPLY1AAAAQklEQVR42u3PQREAAAgDoK1/' + + 'aM3g14MGNJMXKiIiIiIiIiIiIiIiIiIiIiIiIiIiIiIiIiIiIiIiIiIiIiIiIiJysRFNMgH0RpujAAAAAElFTkSuQmCC'; + +export async function waitForAllImagesLoaded( editor ) { + // Returns a promise that waits for all the images in editor to be loaded. + // This is needed because resizers are created only after images are loaded. + const root = editor.model.document.getRoot(); + const editingView = editor.editing.view; + const images = new Set(); + + for ( const curModel of root.getChildren() ) { + if ( curModel.is( 'element', 'image' ) ) { + const imageView = editor.editing.mapper.toViewElement( curModel ); + images.add( editingView.domConverter.viewToDom( imageView ).querySelector( 'img' ) ); + } + } + + editingView.addObserver( ImageLoadObserver ); + + return new Promise( resolve => { + editingView.document.on( 'imageLoaded', ( evt, domEvent ) => { + images.delete( domEvent.target ); + + if ( images.size === 0 ) { + resolve(); + } + } ); + } ); +} diff --git a/packages/ckeditor5-image/tests/imageresize/imageresizeediting.js b/packages/ckeditor5-image/tests/imageresize/imageresizeediting.js index 963782b9f2d..649f9bc0299 100644 --- a/packages/ckeditor5-image/tests/imageresize/imageresizeediting.js +++ b/packages/ckeditor5-image/tests/imageresize/imageresizeediting.js @@ -18,12 +18,9 @@ import ImageStyle from '../../src/imagestyle'; import { setData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model'; import { focusEditor } from '@ckeditor/ckeditor5-widget/tests/widgetresize/_utils/utils'; +import { IMAGE_SRC_FIXTURE } from './_utils/utils'; describe( 'ImageResizeEditing', () => { - // 100x50 black png image - const IMAGE_SRC_FIXTURE = 'data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAGQAAAAyCAQAAAAAPLY1AAAAQklEQVR42u3PQREAAAgDoK1/' + - 'aM3g14MGNJMXKiIiIiIiIiIiIiIiIiIiIiIiIiIiIiIiIiIiIiIiIiIiIiIiIiJysRFNMgH0RpujAAAAAElFTkSuQmCC'; - let editor, editorElement; beforeEach( () => { diff --git a/packages/ckeditor5-image/tests/imageresize/imageresizehandles.js b/packages/ckeditor5-image/tests/imageresize/imageresizehandles.js index 833c8f120fe..b99b62bcd40 100644 --- a/packages/ckeditor5-image/tests/imageresize/imageresizehandles.js +++ b/packages/ckeditor5-image/tests/imageresize/imageresizehandles.js @@ -30,12 +30,9 @@ import { } from '@ckeditor/ckeditor5-widget/tests/widgetresize/_utils/utils'; import WidgetResize from '@ckeditor/ckeditor5-widget/src/widgetresize'; +import { IMAGE_SRC_FIXTURE, waitForAllImagesLoaded } from './_utils/utils'; describe( 'ImageResizeHandles', () => { - // 100x50 black png image - const IMAGE_SRC_FIXTURE = 'data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAGQAAAAyCAQAAAAAPLY1AAAAQklEQVR42u3PQREAAAgDoK1/' + - 'aM3g14MGNJMXKiIiIiIiIiIiIiIiIiIiIiIiIiIiIiIiIiIiIiIiIiIiIiIiIiJysRFNMgH0RpujAAAAAElFTkSuQmCC'; - let widget, editor, view, viewDocument, editorElement; beforeEach( () => { @@ -62,7 +59,7 @@ describe( 'ImageResizeHandles', () => { const attachToSpy = sinon.spy( localEditor.plugins.get( WidgetResize ), 'attachTo' ); - setData( localEditor.model, `[]` ); + await setModelAndWaitForImages( localEditor, `[]` ); expect( attachToSpy.args[ 0 ][ 0 ] ).to.have.a.property( 'unit', '%' ); @@ -79,7 +76,7 @@ describe( 'ImageResizeHandles', () => { it( 'uses the command on commit', async () => { const spy = sinon.spy( editor.commands.get( 'imageResize' ), 'execute' ); - setData( editor.model, `foo[]` ); + await setModelAndWaitForImages( editor, `foo[]` ); widget = viewDocument.getRoot().getChild( 1 ); const domParts = getWidgetDomParts( editor, widget, 'bottom-left' ); @@ -92,7 +89,7 @@ describe( 'ImageResizeHandles', () => { } ); it( 'disables the resizer if the command is disabled', async () => { - setData( editor.model, `foo[]` ); + await setModelAndWaitForImages( editor, `foo[]` ); const resizer = getSelectedImageResizer( editor ); @@ -124,6 +121,8 @@ describe( 'ImageResizeHandles', () => { editor.model.insertContent( writer.createElement( 'image', { src: IMAGE_SRC_FIXTURE } ) ); } ); + await waitForAllImagesLoaded( editor ); + const resizer = getSelectedImageResizer( editor ); const resizerWrapper = editor.ui.getEditableElement().querySelector( '.ck-widget__resizer' ); @@ -136,7 +135,8 @@ describe( 'ImageResizeHandles', () => { beforeEach( async () => { editor = await createEditor(); - setData( editor.model, `foo[]` ); + await setModelAndWaitForImages( editor, + `foo[]` ); widget = viewDocument.getRoot().getChild( 1 ); } ); @@ -178,7 +178,7 @@ describe( 'ImageResizeHandles', () => { beforeEach( async () => { editor = await createEditor(); - setData( editor.model, `foo[]` ); + await setModelAndWaitForImages( editor, `foo[]` ); widget = viewDocument.getRoot().getChild( 1 ); } ); @@ -200,9 +200,9 @@ describe( 'ImageResizeHandles', () => { // Toggle _visibleResizer to force synchronous redraw. Otherwise you'd need to wait ~200ms for // throttled redraw to take place, making tests slower. - const visibleResizer = plugin._visibleResizer; - plugin._visibleResizer = null; - plugin._visibleResizer = visibleResizer; + for ( const [ , resizer ] of plugin._resizers.entries() ) { + resizer.redraw(); + } const resizerWrapper = document.querySelector( '.ck-widget__resizer' ); const shadowBoundingRect = resizerWrapper.getBoundingClientRect(); @@ -210,13 +210,37 @@ describe( 'ImageResizeHandles', () => { expect( shadowBoundingRect.width ).to.equal( 100 ); expect( shadowBoundingRect.height ).to.equal( 50 ); } ); + + it( 'doesn\'t show resizers when undoing to multiple images', async () => { + // Based on https://github.com/ckeditor/ckeditor5/pull/8108#issuecomment-695949745. + await setModelAndWaitForImages( editor, + `[]` ); + + const paragraph = editor.model.change( writer => { + return writer.createElement( 'paragraph' ); + } ); + editor.model.insertContent( paragraph ); + + // Undo to go back to two, selected images. + editor.commands.get( 'undo' ).execute(); + + for ( let i = 0; i < 2; i++ ) { + widget = viewDocument.getRoot().getChild( i ); + const domImage = getWidgetDomParts( editor, widget, 'bottom-right' ).widget.querySelector( 'img' ); + viewDocument.fire( 'imageLoaded', { target: domImage } ); + + const domResizeWrapper = getWidgetDomParts( editor, widget, 'bottom-left' ).resizeWrapper; + + expect( domResizeWrapper.getBoundingClientRect().height ).to.equal( 0 ); + } + } ); } ); describe( 'table integration', () => { it( 'works when resizing in a table', async () => { editor = await createEditor(); - setData( editor.model, + await setModelAndWaitForImages( editor, '' + `[]` + '
' @@ -238,6 +262,36 @@ describe( 'ImageResizeHandles', () => { } ); } ); + it( 'doesn\'t create multiple resizers for a single image widget', async () => { + // https://github.com/ckeditor/ckeditor5/pull/8108#issuecomment-708302992 + editor = await createEditor(); + await setModelAndWaitForImages( editor, `[]` ); + widget = viewDocument.getRoot().getChild( 0 ); + + const domParts = getWidgetDomParts( editor, widget, 'bottom-right' ); + const alternativeImageFixture = + 'data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNk+A8AAQUBAScY42YAAAAASUVORK5CYII='; + + // Change the image so that load event triggers for the same img element again. + domParts.widget.querySelector( 'img' ).src = alternativeImageFixture; + await waitForAllImagesLoaded( editor ); + + expect( domParts.widget.querySelectorAll( '.ck-widget__resizer' ).length ).to.equal( 1 ); + } ); + + it( 'only creates a resizer after the image is loaded', async () => { + // https://github.com/ckeditor/ckeditor5/issues/8088 + editor = await createEditor(); + setData( editor.model, `[]` ); + widget = viewDocument.getRoot().getChild( 0 ); + const domParts = getWidgetDomParts( editor, widget, 'bottom-right' ); + + expect( domParts.widget.querySelectorAll( '.ck-widget__resizer' ).length ).to.equal( 0 ); + + await waitForAllImagesLoaded( editor ); + expect( domParts.widget.querySelectorAll( '.ck-widget__resizer' ).length ).to.equal( 1 ); + } ); + describe( 'srcset integration', () => { // The image is 96x96 pixels. const imageBaseUrl = '/assets/sample.png'; @@ -272,11 +326,13 @@ describe( 'ImageResizeHandles', () => { ` ); + await waitForAllImagesLoaded( editor ); + widget = viewDocument.getRoot().getChild( 0 ); model = editor.model.document.getRoot().getChild( 0 ); } ); - it( 'works with images containing srcset', () => { + it( 'works with images containing srcset', async () => { const domParts = getWidgetDomParts( editor, widget, 'bottom-right' ); const initialPosition = getHandleCenterPoint( domParts.widget, 'bottom-right' ); const finalPointerPosition = initialPosition.clone().moveBy( -20, -20 ); @@ -289,7 +345,7 @@ describe( 'ImageResizeHandles', () => { expect( model.getAttribute( 'width' ) ).to.equal( '76px' ); } ); - it( 'retains width after removing srcset', () => { + it( 'retains width after removing srcset', async () => { const domParts = getWidgetDomParts( editor, widget, 'bottom-right' ); const initialPosition = getHandleCenterPoint( domParts.widget, 'bottom-right' ); const finalPointerPosition = initialPosition.clone().moveBy( -16, -16 ); @@ -339,7 +395,7 @@ describe( 'ImageResizeHandles', () => { } } ); - setData( editor.model, `foo[]` ); + await setModelAndWaitForImages( editor, `foo[]` ); widget = viewDocument.getRoot().getChild( 1 ); @@ -395,7 +451,7 @@ describe( 'ImageResizeHandles', () => { } function getSelectedImageResizer( editor ) { - return editor.plugins.get( 'WidgetResize' )._getResizerByViewElement( + return editor.plugins.get( 'WidgetResize' ).getResizerByViewElement( editor.editing.view.document.selection.getSelectedElement() ); } @@ -415,4 +471,9 @@ describe( 'ImageResizeHandles', () => { return newEditor; } ); } + + async function setModelAndWaitForImages( editor, data ) { + setData( editor.model, data ); + return waitForAllImagesLoaded( editor ); + } } ); diff --git a/packages/ckeditor5-widget/src/widgetresize.js b/packages/ckeditor5-widget/src/widgetresize.js index 4e9b4e9fd1e..1094f299e23 100644 --- a/packages/ckeditor5-widget/src/widgetresize.js +++ b/packages/ckeditor5-widget/src/widgetresize.js @@ -41,11 +41,10 @@ export default class WidgetResize extends Plugin { /** * The currently visible resizer. * - * @protected * @observable - * @member {module:widget/widgetresize/resizer~Resizer|null} #_visibleResizer + * @member {module:widget/widgetresize/resizer~Resizer|null} #visibleResizer */ - this.set( '_visibleResizer', null ); + this.set( 'visibleResizer', null ); /** * References an active resizer. @@ -82,8 +81,8 @@ export default class WidgetResize extends Plugin { this._observer.listenTo( domDocument, 'mouseup', this._mouseUpListener.bind( this ) ); const redrawFocusedResizer = () => { - if ( this._visibleResizer ) { - this._visibleResizer.redraw(); + if ( this.visibleResizer ) { + this.visibleResizer.redraw(); } }; @@ -91,7 +90,7 @@ export default class WidgetResize extends Plugin { // Redraws occurring upon a change of visible resizer must not be throttled, as it is crucial for the initial // render. Without it the resizer frame would be misaligned with resizing host for a fraction of second. - this.on( 'change:_visibleResizer', redrawFocusedResizer ); + this.on( 'change:visibleResizer', redrawFocusedResizer ); // Redrawing on any change of the UI of the editor (including content changes). this.editor.ui.on( 'update', redrawFocusedResizerThrottled ); @@ -104,7 +103,7 @@ export default class WidgetResize extends Plugin { viewSelection.on( 'change', () => { const selectedElement = viewSelection.getSelectedElement(); - this._visibleResizer = this._getResizerByViewElement( selectedElement ) || null; + this.visibleResizer = this.getResizerByViewElement( selectedElement ) || null; } ); } @@ -149,9 +148,28 @@ export default class WidgetResize extends Plugin { this._resizers.set( options.viewElement, resizer ); + const viewSelection = this.editor.editing.view.document.selection; + const selectedElement = viewSelection.getSelectedElement(); + + // It could be that the element the resizer is created for is currently focused. In that + // case it should become visible. + if ( this.getResizerByViewElement( selectedElement ) == resizer ) { + this.visibleResizer = resizer; + } + return resizer; } + /** + * Returns a resizer created for a given view element (widget element). + * + * @param {module:engine/view/containerelement~ContainerElement} viewElement View element associated with the resizer. + * @returns {module:widget/widgetresize/resizer~Resizer|undefined} + */ + getResizerByViewElement( viewElement ) { + return this._resizers.get( viewElement ); + } + /** * Returns a resizer that contains a given resize handle. * @@ -167,17 +185,6 @@ export default class WidgetResize extends Plugin { } } - /** - * Returns a resizer created for a given view element (widget element). - * - * @protected - * @param {module:engine/view/containerelement~ContainerElement} viewElement - * @returns {module:widget/widgetresize/resizer~Resizer} - */ - _getResizerByViewElement( viewElement ) { - return this._resizers.get( viewElement ); - } - /** * @protected * @param {module:utils/eventinfo~EventInfo} event diff --git a/packages/ckeditor5-widget/tests/widgetresize-integration.js b/packages/ckeditor5-widget/tests/widgetresize-integration.js index 6b52d006411..353d9a2d810 100644 --- a/packages/ckeditor5-widget/tests/widgetresize-integration.js +++ b/packages/ckeditor5-widget/tests/widgetresize-integration.js @@ -11,6 +11,7 @@ import { setData as setModelData } from '@ckeditor/ckeditor5-engine/src/dev-util import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils'; import Image from '@ckeditor/ckeditor5-image/src/image'; import ImageResize from '@ckeditor/ckeditor5-image/src/imageresize'; +import { waitForAllImagesLoaded } from '@ckeditor/ckeditor5-image/tests/imageresize/_utils/utils'; describe( 'WidgetResize - integration', () => { let editor, model, view, viewDocument, editorElement; @@ -36,11 +37,13 @@ describe( 'WidgetResize - integration', () => { return editor.destroy(); } ); - it( 'should not fire viewDocument#mousedown events after starting resizing', () => { + it( 'should not fire viewDocument#mousedown events after starting resizing', async () => { const eventSpy = sinon.spy().named( 'ViewDocument#mousedown' ); setModelData( model, '[]' ); + await waitForAllImagesLoaded( editor ); + const resizeSquareUI = [ ...viewDocument.getRoot().getChild( 0 ).getChildren() ] .find( element => element.hasClass( 'ck-widget__resizer' ) ); diff --git a/packages/ckeditor5-widget/tests/widgetresize.js b/packages/ckeditor5-widget/tests/widgetresize.js index f1020cd4e7f..7d12c7870c6 100644 --- a/packages/ckeditor5-widget/tests/widgetresize.js +++ b/packages/ckeditor5-widget/tests/widgetresize.js @@ -468,20 +468,43 @@ describe( 'WidgetResize', () => { } ); describe( 'attachTo()', () => { - it( 'works without WidgetToolbarRepository plugin', async () => { - const localEditorElement = createEditorElement(); - const localEditor = await ClassicEditor.create( localEditorElement, { + let localEditorElement, localEditor; + + beforeEach( async () => { + localEditorElement = createEditorElement(); + localEditor = await ClassicEditor.create( localEditorElement, { plugins: [ WidgetResize, simpleWidgetPlugin ] } ); + } ); + + afterEach( () => { + localEditorElement.remove(); + return localEditor.destroy(); + } ); + + it( 'works without WidgetToolbarRepository plugin', async () => { + setModelData( localEditor.model, '[]' ); + + localEditor.plugins.get( WidgetResize ).attachTo( gerResizerOptions( localEditor ) ); + // Nothing should be thrown. + } ); + it( 'sets the visible resizer if associated widget is already focused', async () => { setModelData( localEditor.model, '[]' ); - const resizerOptions = { - modelElement: localEditor.model.document.getRoot().getChild( 0 ), - viewElement: localEditor.editing.view.document.getRoot().getChild( 0 ), - editor: localEditor, + const widgetResizePlugin = localEditor.plugins.get( WidgetResize ); + const resizer = widgetResizePlugin.attachTo( gerResizerOptions( localEditor ) ); + + expect( widgetResizePlugin.visibleResizer ).to.eql( resizer ); + } ); + + function gerResizerOptions( editor ) { + return { + modelElement: editor.model.document.getRoot().getChild( 0 ), + viewElement: editor.editing.view.document.getRoot().getChild( 0 ), + editor, isCentered: () => false, getHandleHost( domWidgetElement ) { @@ -493,13 +516,7 @@ describe( 'WidgetResize', () => { onCommit: commitStub }; - - localEditor.plugins.get( WidgetResize ).attachTo( resizerOptions ); - // Nothing should be thrown. - // And clean up. - localEditorElement.remove(); - return localEditor.destroy(); - } ); + } } ); describe( 'init()', () => { diff --git a/packages/ckeditor5-widget/tests/widgetresize/_utils/utils.js b/packages/ckeditor5-widget/tests/widgetresize/_utils/utils.js index c8759f1532c..222cc9383d9 100644 --- a/packages/ckeditor5-widget/tests/widgetresize/_utils/utils.js +++ b/packages/ckeditor5-widget/tests/widgetresize/_utils/utils.js @@ -152,6 +152,5 @@ export function getHandleCenterPoint( domWrapper, handlePosition ) { } export function focusEditor( editor ) { - editor.focus(); editor.ui.focusTracker.isFocused = true; }