From ba4dab88d0f8f7eebee924d5b56e620136d62f6e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Tue, 26 Jun 2018 15:15:20 +0200 Subject: [PATCH 1/2] Used UI#update event instead of View#render to attach the UI components. --- src/toolbar/balloon/balloontoolbar.js | 7 ++-- src/toolbar/block/blocktoolbar.js | 2 +- tests/toolbar/balloon/balloontoolbar.js | 10 ++--- tests/toolbar/block/blocktoolbar.js | 52 +++++++++++++++++++++---- 4 files changed, 53 insertions(+), 18 deletions(-) diff --git a/src/toolbar/balloon/balloontoolbar.js b/src/toolbar/balloon/balloontoolbar.js index f1e4c62a..827946bd 100644 --- a/src/toolbar/balloon/balloontoolbar.js +++ b/src/toolbar/balloon/balloontoolbar.js @@ -167,9 +167,8 @@ export default class BalloonToolbar extends Plugin { return; } - // Update the toolbar position upon change (e.g. external document changes) - // while it's visible. - this.listenTo( this.editor.editing.view, 'render', () => { + // Update the toolbar position when the editor ui should be refreshed. + this.listenTo( this.editor.ui, 'update', () => { this._balloon.updatePosition( this._getBalloonPositionData() ); } ); @@ -186,7 +185,7 @@ export default class BalloonToolbar extends Plugin { */ hide() { if ( this._balloon.hasView( this.toolbarView ) ) { - this.stopListening( this.editor.editing.view, 'render' ); + this.stopListening( this.editor.ui, 'update' ); this._balloon.remove( this.toolbarView ); } } diff --git a/src/toolbar/block/blocktoolbar.js b/src/toolbar/block/blocktoolbar.js index e11e6d28..6d0bbb16 100644 --- a/src/toolbar/block/blocktoolbar.js +++ b/src/toolbar/block/blocktoolbar.js @@ -230,7 +230,7 @@ export default class BlockToolbar extends Plugin { } } ); - this.listenTo( view, 'render', () => { + this.listenTo( editor.ui, 'update', () => { // Get first selected block, button will be attached to this element. modelTarget = Array.from( model.document.selection.getSelectedBlocks() )[ 0 ]; diff --git a/tests/toolbar/balloon/balloontoolbar.js b/tests/toolbar/balloon/balloontoolbar.js index 1d7f7aa3..906ce89a 100644 --- a/tests/toolbar/balloon/balloontoolbar.js +++ b/tests/toolbar/balloon/balloontoolbar.js @@ -275,17 +275,17 @@ describe( 'BalloonToolbar', () => { expect( targetRect ).to.deep.equal( backwardSelectionRect ); } ); - it( 'should update balloon position on view#change event while balloon is added to the #_balloon', () => { + it( 'should update balloon position on ui#update event while balloon is added to the #_balloon', () => { setData( model, 'b[a]r' ); const spy = sandbox.spy( balloon, 'updatePosition' ); - editingView.fire( 'render' ); + editor.ui.fire( 'update' ); balloonToolbar.show(); sinon.assert.notCalled( spy ); - editingView.fire( 'render' ); + editor.ui.fire( 'update' ); sinon.assert.calledOnce( spy ); } ); @@ -369,7 +369,7 @@ describe( 'BalloonToolbar', () => { sinon.assert.calledWithExactly( removeBalloonSpy, balloonToolbar.toolbarView ); } ); - it( 'should stop update balloon position on ViewDocument#change event', () => { + it( 'should stop update balloon position on ui#update event', () => { setData( model, 'b[a]r' ); const spy = sandbox.spy( balloon, 'updatePosition' ); @@ -377,7 +377,7 @@ describe( 'BalloonToolbar', () => { balloonToolbar.show(); balloonToolbar.hide(); - editingView.fire( 'render' ); + editor.ui.fire( 'update' ); sinon.assert.notCalled( spy ); } ); diff --git a/tests/toolbar/block/blocktoolbar.js b/tests/toolbar/block/blocktoolbar.js index 9e26cc92..b190ab27 100644 --- a/tests/toolbar/block/blocktoolbar.js +++ b/tests/toolbar/block/blocktoolbar.js @@ -211,26 +211,38 @@ describe( 'BlockToolbar', () => { setData( editor.model, 'foo[]bar' ); + // "Flush" throttled event. + editor.ui.fire( 'update' ); + expect( blockToolbar.buttonView.isVisible ).to.be.true; } ); it( 'should display the button when the first selected block is an object', () => { setData( editor.model, '[foo]' ); + // "Flush" throttled event. + editor.ui.fire( 'update' ); + expect( blockToolbar.buttonView.isVisible ).to.be.true; } ); it( 'should display the button when the selection is inside the object', () => { setData( editor.model, 'f[]oo' ); + // "Flush" throttled event. + editor.ui.fire( 'update' ); + expect( blockToolbar.buttonView.isVisible ).to.be.true; } ); - it( 'should not display the button when the selection is placed in a root element', () => { + it( 'should not display the button when the selection is placed in the root element', () => { editor.model.schema.extend( '$text', { allowIn: '$root' } ); setData( editor.model, 'foo[]bar' ); + // "Flush" throttled event. + editor.ui.fire( 'update' ); + expect( blockToolbar.buttonView.isVisible ).to.be.false; } ); @@ -247,6 +259,9 @@ describe( 'BlockToolbar', () => { setData( editor.model, '[]' ); + // "Flush" throttled event. + editor.ui.fire( 'update' ); + expect( blockToolbar.buttonView.isVisible ).to.be.false; element.remove(); @@ -267,6 +282,9 @@ describe( 'BlockToolbar', () => { 'of the selected block #1', () => { setData( editor.model, 'foo[]bar' ); + // "Flush" throttled event. + editor.ui.fire( 'update' ); + const target = editor.ui.view.editableElement.querySelector( 'p' ); const styleMock = testUtils.sinon.stub( window, 'getComputedStyle' ); @@ -291,7 +309,7 @@ describe( 'BlockToolbar', () => { height: 100 } ); - editor.editing.view.fire( 'render' ); + editor.ui.fire( 'update' ); expect( blockToolbar.buttonView.top ).to.equal( 470 ); expect( blockToolbar.buttonView.left ).to.equal( 100 ); @@ -301,6 +319,9 @@ describe( 'BlockToolbar', () => { 'of the selected block #2', () => { setData( editor.model, 'foo[]bar' ); + // "Flush" throttled event. + editor.ui.fire( 'update' ); + const target = editor.ui.view.editableElement.querySelector( 'p' ); const styleMock = testUtils.sinon.stub( window, 'getComputedStyle' ); @@ -326,24 +347,24 @@ describe( 'BlockToolbar', () => { height: 100 } ); - editor.editing.view.fire( 'render' ); + editor.ui.fire( 'update' ); expect( blockToolbar.buttonView.top ).to.equal( 472 ); expect( blockToolbar.buttonView.left ).to.equal( 100 ); } ); - it( 'should reposition the #panelView when open on view#render', () => { + it( 'should reposition the #panelView when open on ui#update', () => { blockToolbar.panelView.isVisible = false; const spy = testUtils.sinon.spy( blockToolbar.panelView, 'pin' ); - editor.editing.view.fire( 'render' ); + editor.ui.fire( 'update' ); sinon.assert.notCalled( spy ); blockToolbar.panelView.isVisible = true; - editor.editing.view.fire( 'render' ); + editor.ui.fire( 'update' ); sinon.assert.calledWith( spy, { target: blockToolbar.buttonView.element, @@ -351,12 +372,12 @@ describe( 'BlockToolbar', () => { } ); } ); - it( 'should not reset the toolbar focus on view#render', () => { + it( 'should not reset the toolbar focus on ui#update', () => { blockToolbar.panelView.isVisible = true; const spy = testUtils.sinon.spy( blockToolbar.toolbarView, 'focus' ); - editor.editing.view.fire( 'render' ); + editor.ui.fire( 'update' ); sinon.assert.notCalled( spy ); } ); @@ -380,6 +401,9 @@ describe( 'BlockToolbar', () => { it( 'should hide the UI when editor switches to readonly when the panel is not visible', () => { setData( editor.model, 'foo[]bar' ); + // "Flush" throttled event. + editor.ui.fire( 'update' ); + blockToolbar.buttonView.isVisible = true; blockToolbar.panelView.isVisible = false; @@ -392,6 +416,9 @@ describe( 'BlockToolbar', () => { it( 'should not hide button when the editor switches to readonly when the panel is visible', () => { setData( editor.model, 'foo[]bar' ); + // "Flush" throttled event. + editor.ui.fire( 'update' ); + blockToolbar.buttonView.isVisible = true; blockToolbar.panelView.isVisible = true; @@ -409,12 +436,18 @@ describe( 'BlockToolbar', () => { // Place the selection outside of any block because the toolbar will not be shown in this case. setData( editor.model, '[]bar' ); + // "Flush" throttled event. + editor.ui.fire( 'update' ); + window.dispatchEvent( new Event( 'resize' ) ); sinon.assert.notCalled( spy ); setData( editor.model, 'ba[]r' ); + // "Flush" throttled event. + editor.ui.fire( 'update' ); + spy.resetHistory(); window.dispatchEvent( new Event( 'resize' ) ); @@ -423,6 +456,9 @@ describe( 'BlockToolbar', () => { setData( editor.model, '[]bar' ); + // "Flush" throttled event. + editor.ui.fire( 'update' ); + spy.resetHistory(); window.dispatchEvent( new Event( 'resize' ) ); From 299e5f4694a509899cb4f111f649d33542ac0563 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Thu, 28 Jun 2018 13:12:28 +0200 Subject: [PATCH 2/2] Simplified the tests after removing throttle from the EditorUI#update event. --- tests/toolbar/block/blocktoolbar.js | 36 ----------------------------- 1 file changed, 36 deletions(-) diff --git a/tests/toolbar/block/blocktoolbar.js b/tests/toolbar/block/blocktoolbar.js index b190ab27..a5194195 100644 --- a/tests/toolbar/block/blocktoolbar.js +++ b/tests/toolbar/block/blocktoolbar.js @@ -211,27 +211,18 @@ describe( 'BlockToolbar', () => { setData( editor.model, 'foo[]bar' ); - // "Flush" throttled event. - editor.ui.fire( 'update' ); - expect( blockToolbar.buttonView.isVisible ).to.be.true; } ); it( 'should display the button when the first selected block is an object', () => { setData( editor.model, '[foo]' ); - // "Flush" throttled event. - editor.ui.fire( 'update' ); - expect( blockToolbar.buttonView.isVisible ).to.be.true; } ); it( 'should display the button when the selection is inside the object', () => { setData( editor.model, 'f[]oo' ); - // "Flush" throttled event. - editor.ui.fire( 'update' ); - expect( blockToolbar.buttonView.isVisible ).to.be.true; } ); @@ -240,9 +231,6 @@ describe( 'BlockToolbar', () => { setData( editor.model, 'foo[]bar' ); - // "Flush" throttled event. - editor.ui.fire( 'update' ); - expect( blockToolbar.buttonView.isVisible ).to.be.false; } ); @@ -259,9 +247,6 @@ describe( 'BlockToolbar', () => { setData( editor.model, '[]' ); - // "Flush" throttled event. - editor.ui.fire( 'update' ); - expect( blockToolbar.buttonView.isVisible ).to.be.false; element.remove(); @@ -282,9 +267,6 @@ describe( 'BlockToolbar', () => { 'of the selected block #1', () => { setData( editor.model, 'foo[]bar' ); - // "Flush" throttled event. - editor.ui.fire( 'update' ); - const target = editor.ui.view.editableElement.querySelector( 'p' ); const styleMock = testUtils.sinon.stub( window, 'getComputedStyle' ); @@ -319,9 +301,6 @@ describe( 'BlockToolbar', () => { 'of the selected block #2', () => { setData( editor.model, 'foo[]bar' ); - // "Flush" throttled event. - editor.ui.fire( 'update' ); - const target = editor.ui.view.editableElement.querySelector( 'p' ); const styleMock = testUtils.sinon.stub( window, 'getComputedStyle' ); @@ -401,9 +380,6 @@ describe( 'BlockToolbar', () => { it( 'should hide the UI when editor switches to readonly when the panel is not visible', () => { setData( editor.model, 'foo[]bar' ); - // "Flush" throttled event. - editor.ui.fire( 'update' ); - blockToolbar.buttonView.isVisible = true; blockToolbar.panelView.isVisible = false; @@ -416,9 +392,6 @@ describe( 'BlockToolbar', () => { it( 'should not hide button when the editor switches to readonly when the panel is visible', () => { setData( editor.model, 'foo[]bar' ); - // "Flush" throttled event. - editor.ui.fire( 'update' ); - blockToolbar.buttonView.isVisible = true; blockToolbar.panelView.isVisible = true; @@ -436,18 +409,12 @@ describe( 'BlockToolbar', () => { // Place the selection outside of any block because the toolbar will not be shown in this case. setData( editor.model, '[]bar' ); - // "Flush" throttled event. - editor.ui.fire( 'update' ); - window.dispatchEvent( new Event( 'resize' ) ); sinon.assert.notCalled( spy ); setData( editor.model, 'ba[]r' ); - // "Flush" throttled event. - editor.ui.fire( 'update' ); - spy.resetHistory(); window.dispatchEvent( new Event( 'resize' ) ); @@ -456,9 +423,6 @@ describe( 'BlockToolbar', () => { setData( editor.model, '[]bar' ); - // "Flush" throttled event. - editor.ui.fire( 'update' ); - spy.resetHistory(); window.dispatchEvent( new Event( 'resize' ) );