Skip to content

Commit

Permalink
Merge pull request #8108 from ckeditor/i/7548
Browse files Browse the repository at this point in the history 
Fix (link): Fixed a quick image flicker of image resize frame when inserting an image. Closes #8088.

Other (widget): `WidgetResize#visibleResizer` and `WidgetResize#getResizerByViewElement()` are now public. See #8088.

Other (widget): `WidgetResize` will now automatically set `WidgetResize#visibleResizer` when calling `WidgetResize#attachTo()` if the corresponding resizer's element is focused during the call. See #8088.
  • Loading branch information
@Reinmar
Reinmar authored Nov 4, 2020
2 parents 2c03820 + 9e5317c commit 021227d
Show file tree
Hide file tree
Showing 8 changed files with 214 additions and 67 deletions.
50 changes: 38 additions & 12 deletions packages/ckeditor5-image/src/imageresize/imageresizehandles.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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 ) {
Expand All @@ -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';
},
Expand All @@ -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' } );
} );
}
}
37 changes: 37 additions & 0 deletions packages/ckeditor5-image/tests/imageresize/_utils/utils.js
View file
Original file line number Diff line number Diff line change
@@ -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();
}
} );
} );
}
5 changes: 1 addition & 4 deletions packages/ckeditor5-image/tests/imageresize/imageresizeediting.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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( () => {
Expand Down
95 changes: 78 additions & 17 deletions packages/ckeditor5-image/tests/imageresize/imageresizehandles.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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( () => {
Expand All @@ -62,7 +59,7 @@ describe( 'ImageResizeHandles', () => {

const attachToSpy = sinon.spy( localEditor.plugins.get( WidgetResize ), 'attachTo' );

setData( localEditor.model, `[<image imageStyle="side" src="${ IMAGE_SRC_FIXTURE }"></image>]` );
await setModelAndWaitForImages( localEditor, `[<image imageStyle="side" src="${ IMAGE_SRC_FIXTURE }"></image>]` );

expect( attachToSpy.args[ 0 ][ 0 ] ).to.have.a.property( 'unit', '%' );

Expand All @@ -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, `<paragraph>foo</paragraph>[<image src="${ IMAGE_SRC_FIXTURE }"></image>]` );
await setModelAndWaitForImages( editor, `<paragraph>foo</paragraph>[<image src="${ IMAGE_SRC_FIXTURE }"></image>]` );
widget = viewDocument.getRoot().getChild( 1 );
const domParts = getWidgetDomParts( editor, widget, 'bottom-left' );

Expand All @@ -92,7 +89,7 @@ describe( 'ImageResizeHandles', () => {
} );

it( 'disables the resizer if the command is disabled', async () => {
setData( editor.model, `<paragraph>foo</paragraph>[<image src="${ IMAGE_SRC_FIXTURE }"></image>]` );
await setModelAndWaitForImages( editor, `<paragraph>foo</paragraph>[<image src="${ IMAGE_SRC_FIXTURE }"></image>]` );

const resizer = getSelectedImageResizer( editor );

Expand Down Expand Up @@ -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' );

Expand All @@ -136,7 +135,8 @@ describe( 'ImageResizeHandles', () => {
beforeEach( async () => {
editor = await createEditor();

setData( editor.model, `<paragraph>foo</paragraph>[<image imageStyle="side" src="${ IMAGE_SRC_FIXTURE }"></image>]` );
await setModelAndWaitForImages( editor,
`<paragraph>foo</paragraph>[<image imageStyle="side" src="${ IMAGE_SRC_FIXTURE }"></image>]` );

widget = viewDocument.getRoot().getChild( 1 );
} );
Expand Down Expand Up @@ -178,7 +178,7 @@ describe( 'ImageResizeHandles', () => {
beforeEach( async () => {
editor = await createEditor();

setData( editor.model, `<paragraph>foo</paragraph>[<image src="${ IMAGE_SRC_FIXTURE }"></image>]` );
await setModelAndWaitForImages( editor, `<paragraph>foo</paragraph>[<image src="${ IMAGE_SRC_FIXTURE }"></image>]` );

widget = viewDocument.getRoot().getChild( 1 );
} );
Expand All @@ -200,23 +200,47 @@ 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();

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,
`[<image src="${ IMAGE_SRC_FIXTURE }"></image><image src="${ IMAGE_SRC_FIXTURE }"></image>]` );

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,
'<table>' +
`<tableRow><tableCell>[<image src="${ IMAGE_SRC_FIXTURE }"></image>]</tableCell></tableRow>` +
'</table>'
Expand All @@ -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, `[<image src="${ IMAGE_SRC_FIXTURE }"></image>]` );
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, `[<image src="${ IMAGE_SRC_FIXTURE }"></image>]` );
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';
Expand Down Expand Up @@ -272,11 +326,13 @@ describe( 'ImageResizeHandles', () => {
</figure>`
);

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 );
Expand All @@ -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 );
Expand Down Expand Up @@ -339,7 +395,7 @@ describe( 'ImageResizeHandles', () => {
}
} );

setData( editor.model, `<paragraph>foo</paragraph>[<image src="${ IMAGE_SRC_FIXTURE }"></image>]` );
await setModelAndWaitForImages( editor, `<paragraph>foo</paragraph>[<image src="${ IMAGE_SRC_FIXTURE }"></image>]` );

widget = viewDocument.getRoot().getChild( 1 );

Expand Down Expand Up @@ -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()
);
}
Expand All @@ -415,4 +471,9 @@ describe( 'ImageResizeHandles', () => {
return newEditor;
} );
}

async function setModelAndWaitForImages( editor, data ) {
setData( editor.model, data );
return waitForAllImagesLoaded( editor );
}
} );
Loading

0 comments on commit 021227d

Please sign in to comment.