diff --git a/src/imagetextalternative/imagetextalternativeui.js b/src/imagetextalternative/imagetextalternativeui.js index d18ee9df..4a8a1cdc 100644 --- a/src/imagetextalternative/imagetextalternativeui.js +++ b/src/imagetextalternative/imagetextalternativeui.js @@ -115,7 +115,7 @@ export default class ImageTextAlternativeUI extends Plugin { } ); // Reposition the balloon or hide the form if an image widget is no longer selected. - this.listenTo( view, 'render', () => { + this.listenTo( editor.ui, 'update', () => { if ( !isImageWidgetSelected( viewDocument.selection ) ) { this._hideForm( true ); } else if ( this._isVisible ) { diff --git a/src/imagetoolbar.js b/src/imagetoolbar.js index 0bc6651f..35e60867 100644 --- a/src/imagetoolbar.js +++ b/src/imagetoolbar.js @@ -93,11 +93,11 @@ export default class ImageToolbar extends Plugin { this._toolbar.fillFromConfig( toolbarConfig, editor.ui.componentFactory ); // Show balloon panel each time image widget is selected. - this.listenTo( editor.editing.view, 'render', () => { + this.listenTo( editor.ui, 'update', () => { this._checkIsVisible(); } ); - // There is no render method after focus is back in editor, we need to check if balloon panel should be visible. + // UI#update is not fired after focus is back in editor, we need to check if balloon panel should be visible. this.listenTo( editor.ui.focusTracker, 'change:isFocused', () => { this._checkIsVisible(); }, { priority: 'low' } ); @@ -111,14 +111,10 @@ export default class ImageToolbar extends Plugin { _checkIsVisible() { const editor = this.editor; - if ( !editor.ui.focusTracker.isFocused ) { + if ( !editor.ui.focusTracker.isFocused || !isImageWidgetSelected( editor.editing.view.document.selection ) ) { this._hideToolbar(); } else { - if ( isImageWidgetSelected( editor.editing.view.document.selection ) ) { - this._showToolbar(); - } else { - this._hideToolbar(); - } + this._showToolbar(); } } @@ -132,14 +128,12 @@ export default class ImageToolbar extends Plugin { if ( this._isVisible ) { repositionContextualBalloon( editor ); - } else { - if ( !this._balloon.hasView( this._toolbar ) ) { - this._balloon.add( { - view: this._toolbar, - position: getBalloonPositionData( editor ), - balloonClassName - } ); - } + } else if ( !this._balloon.hasView( this._toolbar ) ) { + this._balloon.add( { + view: this._toolbar, + position: getBalloonPositionData( editor ), + balloonClassName + } ); } } diff --git a/tests/image/imageediting.js b/tests/image/imageediting.js index 30fc34a7..4eb8d1c3 100644 --- a/tests/image/imageediting.js +++ b/tests/image/imageediting.js @@ -3,7 +3,10 @@ * For licensing, see LICENSE.md. */ +/* global document, Event */ + import VirtualTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/virtualtesteditor'; +import ClassicTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/classictesteditor'; import ImageEditing from '../../src/image/imageediting'; import ImageLoadObserver from '../../src/image/imageloadobserver'; import { getData as getModelData, setData as setModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model'; @@ -12,7 +15,7 @@ import { isImageWidget } from '../../src/image/utils'; import normalizeHtml from '@ckeditor/ckeditor5-utils/tests/_utils/normalizehtml'; describe( 'ImageEditing', () => { - let editor, model, document, view, viewDocument; + let editor, model, doc, view, viewDocument; beforeEach( () => { return VirtualTestEditor @@ -22,7 +25,7 @@ describe( 'ImageEditing', () => { .then( newEditor => { editor = newEditor; model = editor.model; - document = model.document; + doc = model.document; view = editor.editing.view; viewDocument = view.document; } ); @@ -48,6 +51,28 @@ describe( 'ImageEditing', () => { expect( view.getObserver( ImageLoadObserver ) ).to.be.instanceOf( ImageLoadObserver ); } ); + // See https://github.com/ckeditor/ckeditor5-image/issues/142. + it( 'should update the ui after image has been loaded in the DOM', () => { + const element = document.createElement( 'div' ); + document.body.appendChild( element ); + + ClassicTestEditor.create( element, { + plugins: [ ImageEditing ] + } ).then( editor => { + editor.data.set( '
bar
' ); + + const spy = sinon.spy(); + + editor.ui.on( 'update', spy ); + + element.querySelector( 'img' ).dispatchEvent( new Event( 'load' ) ); + + sinon.assert.calledOnce( spy ); + + return editor.destroy().then( () => element.remove() ); + } ); + } ); + describe( 'conversion in data pipeline', () => { describe( 'model to view', () => { it( 'should convert', () => { @@ -120,7 +145,7 @@ describe( 'ImageEditing', () => { 'srcset=\'{ "foo":"bar" }\'>' + '' ); - const image = document.getRoot().getChild( 0 ); + const image = doc.getRoot().getChild( 0 ); model.change( writer => { writer.removeAttribute( 'srcset', image ); } ); @@ -400,7 +425,7 @@ describe( 'ImageEditing', () => { it( 'should convert attribute change', () => { setModelData( model, 'alt text' ); - const image = document.getRoot().getChild( 0 ); + const image = doc.getRoot().getChild( 0 ); model.change( writer => { writer.setAttribute( 'alt', 'new text', image ); @@ -413,7 +438,7 @@ describe( 'ImageEditing', () => { it( 'should convert attribute removal', () => { setModelData( model, 'alt text' ); - const image = document.getRoot().getChild( 0 ); + const image = doc.getRoot().getChild( 0 ); model.change( writer => { writer.removeAttribute( 'alt', image ); @@ -425,7 +450,7 @@ describe( 'ImageEditing', () => { it( 'should not convert change if is already consumed', () => { setModelData( model, 'alt text' ); - const image = document.getRoot().getChild( 0 ); + const image = doc.getRoot().getChild( 0 ); editor.editing.downcastDispatcher.on( 'attribute:alt:image', ( evt, data, conversionApi ) => { conversionApi.consumable.consume( data.item, 'attribute:alt' ); @@ -463,7 +488,7 @@ describe( 'ImageEditing', () => { 'srcset=\'{ "foo":"bar" }\'>' + '' ); - const image = document.getRoot().getChild( 0 ); + const image = doc.getRoot().getChild( 0 ); model.change( writer => { writer.removeAttribute( 'srcset', image ); } ); @@ -492,7 +517,7 @@ describe( 'ImageEditing', () => { it( 'should remove sizes and srcsset attribute when srcset attribute is removed from model', () => { setModelData( model, '' ); - const image = document.getRoot().getChild( 0 ); + const image = doc.getRoot().getChild( 0 ); model.change( writer => { writer.removeAttribute( 'srcset', image ); @@ -512,7 +537,7 @@ describe( 'ImageEditing', () => { 'srcset=\'{ "data": "small.png 148w, big.png 1024w", "width": "1024" }\'>' + '' ); - const image = document.getRoot().getChild( 0 ); + const image = doc.getRoot().getChild( 0 ); model.change( writer => { writer.removeAttribute( 'srcset', image ); diff --git a/tests/imagetextalternative/imagetextalternativeui.js b/tests/imagetextalternative/imagetextalternativeui.js index 71455446..e0ef96d8 100644 --- a/tests/imagetextalternative/imagetextalternativeui.js +++ b/tests/imagetextalternative/imagetextalternativeui.js @@ -156,14 +156,14 @@ describe( 'ImageTextAlternative', () => { expect( balloon.visibleView ).to.equal( lastView ); } ); - describe( 'integration with the editor selection (#render event)', () => { + describe( 'integration with the editor selection (ui#update event)', () => { it( 'should re-position the form', () => { setData( model, '[]' ); button.fire( 'execute' ); const spy = sinon.spy( balloon, 'updatePosition' ); - editor.editing.view.fire( 'render' ); + editor.ui.fire( 'update' ); sinon.assert.calledOnce( spy ); } ); @@ -174,11 +174,13 @@ describe( 'ImageTextAlternative', () => { const removeSpy = sinon.spy( balloon, 'remove' ); const focusSpy = sinon.spy( editor.editing.view, 'focus' ); - // EnqueueChange automatically fires #render event. model.enqueueChange( 'transparent', writer => { writer.remove( doc.selection.getFirstRange() ); } ); + // Because the update event is throttled we need to "flush" it. + editor.ui.fire( 'update' ); + sinon.assert.calledWithExactly( removeSpy, form ); sinon.assert.calledOnce( focusSpy ); } ); diff --git a/tests/imagetoolbar.js b/tests/imagetoolbar.js index f704ca9e..c94a9344 100644 --- a/tests/imagetoolbar.js +++ b/tests/imagetoolbar.js @@ -17,7 +17,7 @@ import View from '@ckeditor/ckeditor5-ui/src/view'; import { setData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model'; describe( 'ImageToolbar', () => { - let editor, model, doc, editingView, plugin, toolbar, balloon, editorElement; + let editor, model, doc, plugin, toolbar, balloon, editorElement; beforeEach( () => { editorElement = global.document.createElement( 'div' ); @@ -36,7 +36,6 @@ describe( 'ImageToolbar', () => { doc = model.document; plugin = editor.plugins.get( ImageToolbar ); toolbar = plugin._toolbar; - editingView = editor.editing.view; balloon = editor.plugins.get( 'ContextualBalloon' ); } ); } ); @@ -79,6 +78,9 @@ describe( 'ImageToolbar', () => { setData( model, '[]' ); + // "Flush" throttled update event. + editor.ui.fire( 'update' ); + sinon.assert.calledWithMatch( spy, { view: toolbar, balloonClassName: 'ck-toolbar-container' @@ -112,17 +114,18 @@ describe( 'ImageToolbar', () => { } ); } ); - describe( 'integration with the editor selection (#change event)', () => { + describe( 'integration with the editor selection', () => { beforeEach( () => { editor.ui.focusTracker.isFocused = true; } ); - it( 'should show the toolbar on render when the image is selected', () => { + it( 'should show the toolbar on ui#update when the image is selected', () => { setData( model, '[foo]' ); expect( balloon.visibleView ).to.be.null; - editingView.change( () => {} ); + editor.ui.fire( 'update' ); + expect( balloon.visibleView ).to.be.null; model.change( writer => { @@ -132,16 +135,22 @@ describe( 'ImageToolbar', () => { ); } ); + // "Flush" throttled update event. + editor.ui.fire( 'update' ); expect( balloon.visibleView ).to.equal( toolbar ); // Make sure successive change does not throw, e.g. attempting // to insert the toolbar twice. - editingView.change( () => {} ); + editor.ui.fire( 'update' ); expect( balloon.visibleView ).to.equal( toolbar ); } ); it( 'should not engage when the toolbar is in the balloon yet invisible', () => { setData( model, '[]' ); + + // "Flush" throttled update event. + editor.ui.fire( 'update' ); + expect( balloon.visibleView ).to.equal( toolbar ); const lastView = new View(); @@ -156,13 +165,17 @@ describe( 'ImageToolbar', () => { expect( balloon.visibleView ).to.equal( lastView ); - editingView.change( () => {} ); + editor.ui.fire( 'update' ); + expect( balloon.visibleView ).to.equal( lastView ); } ); - it( 'should hide the toolbar on render if the image is de–selected', () => { + it( 'should hide the toolbar on ui#update if the image is de–selected', () => { setData( model, 'foo[]' ); + // "Flush" throttled update event. + editor.ui.fire( 'update' ); + expect( balloon.visibleView ).to.equal( toolbar ); model.change( writer => { @@ -172,11 +185,13 @@ describe( 'ImageToolbar', () => { ); } ); + // "Flush" throttled update event. + editor.ui.fire( 'update' ); expect( balloon.visibleView ).to.be.null; // Make sure successive change does not throw, e.g. attempting // to remove the toolbar twice. - editingView.change( () => {} ); + editor.ui.fire( 'update' ); expect( balloon.visibleView ).to.be.null; } ); } ); diff --git a/tests/manual/tickets/142/1.html b/tests/manual/tickets/142/1.html new file mode 100644 index 00000000..c77a92b8 --- /dev/null +++ b/tests/manual/tickets/142/1.html @@ -0,0 +1,6 @@ +

+ +
+

lorem ipsum

+

foo bar biz

+
diff --git a/tests/manual/tickets/142/1.js b/tests/manual/tickets/142/1.js new file mode 100644 index 00000000..8c8b7740 --- /dev/null +++ b/tests/manual/tickets/142/1.js @@ -0,0 +1,41 @@ +/** + * @license Copyright (c) 2003-2017, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md. + */ + +/* global document, console, window, setTimeout */ + +import ClassicEditor from '@ckeditor/ckeditor5-editor-classic/src/classiceditor'; +import Enter from '@ckeditor/ckeditor5-enter/src/enter'; +import Typing from '@ckeditor/ckeditor5-typing/src/typing'; +import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph'; +import Link from '@ckeditor/ckeditor5-link/src/link'; +import Image from '../../../../src/image'; +import ImageCaption from '../../../../src/imagecaption'; + +ClassicEditor + .create( document.querySelector( '#editor' ), { + plugins: [ Enter, Typing, Paragraph, Link, Image, ImageCaption ], + toolbar: [], + } ) + .then( editor => { + window.editor = editor; + + const doc = editor.model.document; + + document.querySelector( '.start' ).addEventListener( 'click', () => { + let image; + + editor.model.change( writer => { + image = writer.createElement( 'image', { src: 'sample-small.jpg' } ); + writer.insert( image, doc.getRoot().getChild( 0 ), 'after' ); + } ); + + setTimeout( () => { + editor.ui.view.element.querySelector( 'img' ).src = '../../sample.jpg'; + }, 3000 ); + } ); + } ) + .catch( err => { + console.error( err.stack ); + } ); diff --git a/tests/manual/tickets/142/1.md b/tests/manual/tickets/142/1.md new file mode 100644 index 00000000..f5ba38cb --- /dev/null +++ b/tests/manual/tickets/142/1.md @@ -0,0 +1,4 @@ +### Update the UI after image load [#142](https://github.com/ckeditor/ckeditor5-image/issues/142) + +1. Click **Insert image** then quickly select the link in the content (the link editing balloon should show up). +2. Observe if the balloon remains attached to the link after replacing image source to the biggest one (replacing source is made using DOM API). diff --git a/tests/manual/tickets/142/sample-small.jpg b/tests/manual/tickets/142/sample-small.jpg new file mode 100644 index 00000000..8e5b6aba Binary files /dev/null and b/tests/manual/tickets/142/sample-small.jpg differ diff --git a/tests/manual/tickets/142/sample.jpg b/tests/manual/tickets/142/sample.jpg new file mode 100644 index 00000000..b77d07e7 Binary files /dev/null and b/tests/manual/tickets/142/sample.jpg differ