From da907b4a929526b02e747f3290de49ac726960ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Tue, 26 Jun 2018 09:24:22 +0200 Subject: [PATCH 01/12] Introduced EditorUI class with an #update event. --- src/editor/editorui.js | 102 ++++++++++++++++++++++++++++++++++++ src/editor/editorui.jsdoc | 44 ---------------- tests/editor/editorui.js | 106 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 208 insertions(+), 44 deletions(-) create mode 100644 src/editor/editorui.js delete mode 100644 src/editor/editorui.jsdoc create mode 100644 tests/editor/editorui.js diff --git a/src/editor/editorui.js b/src/editor/editorui.js new file mode 100644 index 00000000..8ef9a082 --- /dev/null +++ b/src/editor/editorui.js @@ -0,0 +1,102 @@ +/** + * @license Copyright (c) 2003-2018, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md. + */ + +/** + * @module core/editor/editorui + */ + +import ComponentFactory from '@ckeditor/ckeditor5-ui/src/componentfactory'; +import FocusTracker from '@ckeditor/ckeditor5-utils/src/focustracker'; + +import EmitterMixin from '@ckeditor/ckeditor5-utils/src/emittermixin'; +import mix from '@ckeditor/ckeditor5-utils/src/mix'; +import throttle from '@ckeditor/ckeditor5-utils/src/lib/lodash/throttle'; + +/** + * Class providing the minimal interface that is required to successfully bootstrap any editor UI. + * + * @mixes module:utils/emittermixin~EmitterMixin + */ +export default class EditorUI { + /** + * Creates an instance of the editor UI class. + * + * @param {module:core/editor/editor~Editor} editor The editor instance. + * @param {module:ui/editorui/editoruiview~EditorUIView} view The view of the UI. + */ + constructor( editor, view ) { + /** + * Editor that the UI belongs to. + * + * @readonly + * @member {module:core/editor/editor~Editor} #editor + */ + this.editor = editor; + + /** + * The main (top–most) view of the editor UI. + * + * @readonly + * @member {module:ui/editorui/editoruiview~EditorUIView} #view + */ + this.view = view; + + /** + * Instance of the {@link module:ui/componentfactory~ComponentFactory}, a registry used by plugins + * to register factories of specific UI components. + * + * @readonly + * @member {module:ui/componentfactory~ComponentFactory} #componentFactory + */ + this.componentFactory = new ComponentFactory( editor ); + + /** + * Keeps information about editor UI focus and propagates it among various plugins and components, + * unifying them in a uniform focus group. + * + * @readonly + * @member {module:utils/focustracker~FocusTracker} #focusTracker + */ + this.focusTracker = new FocusTracker(); + + /** + * Fires throttled {@link module:core/editor/editorui~EditorUI#event:update} event. + * + * @protected + * @type {Function} + */ + this._throttledUpdate = throttle( () => this.fire( 'update' ), 200 ); + + // Informs UI components that should be refreshed after layout change. + this.listenTo( editor.editing.view.document, 'layoutChanged', () => this._throttledUpdate() ); + } + + /** + * Triggers the UI update. + */ + update() { + this._throttledUpdate(); + } + + /** + * Destroys the UI. + */ + destroy() { + this.stopListening(); + this._throttledUpdate.cancel(); + this.view.destroy(); + } + + /** + * Fired whenever UI and all related components should be refreshed. + * + * It is fired after each {@link module:engine/view/document~Document#event:layoutChanged} event + * besides it can be fired manually through {@link module:core/editor/editorui~EditorUI#update} method. + * + * @event core/editor/editorui~EditorUI#event:update + */ +} + +mix( EditorUI, EmitterMixin ); diff --git a/src/editor/editorui.jsdoc b/src/editor/editorui.jsdoc deleted file mode 100644 index 97f25782..00000000 --- a/src/editor/editorui.jsdoc +++ /dev/null @@ -1,44 +0,0 @@ -/** - * @license Copyright (c) 2003-2018, CKSource - Frederico Knabben. All rights reserved. - * For licensing, see LICENSE.md. - */ - -/** - * @module core/editor/editorui - */ - -/** - * Minimal interface that is required to successfully bootstrap any editor UI. - * - * @interface EditorUI - */ - -/** - * Editor that the UI belongs to. - * - * @readonly - * @member {module:core/editor/editor~Editor} #editor - */ - -/** - * The main (top–most) view of the editor UI. - * - * @readonly - * @member {module:ui/editorui/editoruiview~EditorUIView} #view - */ - -/** - * Instance of the {@link module:ui/componentfactory~ComponentFactory}, a registry used by plugins - * to register factories of specific UI components. - * - * @readonly - * @member {module:ui/componentfactory~ComponentFactory} #componentFactory - */ - -/** - * Keeps information about editor UI focus and propagates it among various plugins and components, - * unifying them in a uniform focus group. - * - * @readonly - * @member {module:utils/focustracker~FocusTracker} #focusTracker - */ diff --git a/tests/editor/editorui.js b/tests/editor/editorui.js new file mode 100644 index 00000000..9012abf7 --- /dev/null +++ b/tests/editor/editorui.js @@ -0,0 +1,106 @@ +/** + * @license Copyright (c) 2003-2018, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md. + */ + +import EditorUI from '../../src/editor/editorui'; +import Editor from '../../src/editor/editor'; + +import FocusTracker from '@ckeditor/ckeditor5-utils/src/focustracker'; +import ComponentFactory from '@ckeditor/ckeditor5-ui/src/componentfactory'; +import View from '@ckeditor/ckeditor5-ui/src/view'; + +import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils'; + +testUtils.createSinonSandbox(); + +describe( 'EditorUI', () => { + let editor, view, ui; + + beforeEach( () => { + editor = new Editor(); + view = new View(); + ui = new EditorUI( editor, view ); + } ); + + afterEach( () => { + return Promise.all( [ + editor.destroy(), + ui.destroy() + ] ); + } ); + + describe( 'constructor()', () => { + it( 'sets #editor', () => { + expect( ui.editor ).to.equal( editor ); + } ); + + it( 'sets #view', () => { + expect( ui.view ).to.equal( view ); + } ); + + it( 'creates #componentFactory factory', () => { + expect( ui.componentFactory ).to.be.instanceOf( ComponentFactory ); + } ); + + it( 'creates #focusTracker', () => { + expect( ui.focusTracker ).to.be.instanceOf( FocusTracker ); + } ); + + it( 'should fire throttled update event after viewDocument#layoutChanged', () => { + const spy = sinon.spy(); + + ui.on( 'update', spy ); + + editor.editing.view.document.fire( 'layoutChanged' ); + + sinon.assert.calledOnce( spy ); + + editor.editing.view.document.fire( 'layoutChanged' ); + + sinon.assert.calledOnce( spy ); + + ui._throttledUpdate.flush(); + + sinon.assert.calledTwice( spy ); + } ); + } ); + + describe( 'update()', () => { + it( 'should fire throttled update event', () => { + const spy = sinon.spy(); + + ui.on( 'update', spy ); + + ui.update(); + + sinon.assert.calledOnce( spy ); + + ui.update(); + + sinon.assert.calledOnce( spy ); + + ui._throttledUpdate.flush(); + + sinon.assert.calledTwice( spy ); + } ); + } ); + + describe( 'destroy()', () => { + it( 'stops listening', () => { + const spy = sinon.spy( ui, 'stopListening' ); + + ui.destroy(); + + sinon.assert.called( spy ); + } ); + + it( 'destroys the #view', () => { + const spy = sinon.spy( view, 'destroy' ); + + ui.destroy(); + + sinon.assert.called( spy ); + } ); + } ); +} ); From 823f212b054548881ba1607ce2c79e2ebfa434da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Tue, 26 Jun 2018 09:33:05 +0200 Subject: [PATCH 02/12] Improved EditorUI test. --- tests/editor/editorui.js | 29 +++++++++++++++++++++++------ 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/tests/editor/editorui.js b/tests/editor/editorui.js index 9012abf7..857f667e 100644 --- a/tests/editor/editorui.js +++ b/tests/editor/editorui.js @@ -31,19 +31,19 @@ describe( 'EditorUI', () => { } ); describe( 'constructor()', () => { - it( 'sets #editor', () => { + it( 'should set #editor', () => { expect( ui.editor ).to.equal( editor ); } ); - it( 'sets #view', () => { + it( 'should set #view', () => { expect( ui.view ).to.equal( view ); } ); - it( 'creates #componentFactory factory', () => { + it( 'should create #componentFactory factory', () => { expect( ui.componentFactory ).to.be.instanceOf( ComponentFactory ); } ); - it( 'creates #focusTracker', () => { + it( 'should create #focusTracker', () => { expect( ui.focusTracker ).to.be.instanceOf( FocusTracker ); } ); @@ -87,7 +87,7 @@ describe( 'EditorUI', () => { } ); describe( 'destroy()', () => { - it( 'stops listening', () => { + it( 'should stop listening', () => { const spy = sinon.spy( ui, 'stopListening' ); ui.destroy(); @@ -95,12 +95,29 @@ describe( 'EditorUI', () => { sinon.assert.called( spy ); } ); - it( 'destroys the #view', () => { + it( 'should destroy the #view', () => { const spy = sinon.spy( view, 'destroy' ); ui.destroy(); sinon.assert.called( spy ); } ); + + it( 'should cancel throttled update event', () => { + const spy = sinon.spy(); + + ui.on( 'update', spy ); + + ui.update(); + + sinon.assert.calledOnce( spy ); + + ui.destroy(); + + ui.update(); + ui._throttledUpdate.flush(); + + sinon.assert.calledOnce( spy ); + } ); } ); } ); From 96c3f719aa9b63b916a617f6adad09754e08f022 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Tue, 26 Jun 2018 09:33:39 +0200 Subject: [PATCH 03/12] Simplified the UI of ClassicTestEditor. --- tests/_utils-tests/classictesteditor.js | 6 +-- tests/_utils-tests/classictesteditorui.js | 46 ----------------- tests/_utils/classictesteditor.js | 4 +- tests/_utils/classictesteditorui.js | 63 ----------------------- 4 files changed, 5 insertions(+), 114 deletions(-) delete mode 100644 tests/_utils-tests/classictesteditorui.js delete mode 100644 tests/_utils/classictesteditorui.js diff --git a/tests/_utils-tests/classictesteditor.js b/tests/_utils-tests/classictesteditor.js index e105504d..ab77ede8 100644 --- a/tests/_utils-tests/classictesteditor.js +++ b/tests/_utils-tests/classictesteditor.js @@ -11,7 +11,7 @@ import ClassicTestEditor from '../../tests/_utils/classictesteditor'; import Plugin from '../../src/plugin'; import HtmlDataProcessor from '@ckeditor/ckeditor5-engine/src/dataprocessor/htmldataprocessor'; -import ClassicTestEditorUI from '../../tests/_utils/classictesteditorui'; +import EditorUI from '../../src/editor/editorui'; import BoxedEditorUIView from '@ckeditor/ckeditor5-ui/src/editorui/boxed/boxededitoruiview'; import InlineEditableUIView from '@ckeditor/ckeditor5-ui/src/editableui/inline/inlineeditableuiview'; @@ -39,7 +39,7 @@ describe( 'ClassicTestEditor', () => { expect( editor ).to.be.instanceof( Editor ); expect( editor.config.get( 'foo' ) ).to.equal( 1 ); expect( editor.element ).to.equal( editorElement ); - expect( editor.ui ).to.be.instanceOf( ClassicTestEditorUI ); + expect( editor.ui ).to.be.instanceOf( EditorUI ); expect( editor.ui.view ).to.be.instanceOf( BoxedEditorUIView ); expect( editor.data.processor ).to.be.instanceof( HtmlDataProcessor ); } ); @@ -82,7 +82,7 @@ describe( 'ClassicTestEditor', () => { it( 'creates and initializes the UI', () => { return ClassicTestEditor.create( editorElement, { foo: 1 } ) .then( editor => { - expect( editor.ui ).to.be.instanceOf( ClassicTestEditorUI ); + expect( editor.ui ).to.be.instanceOf( EditorUI ); expect( editor.ui.view ).to.be.instanceOf( BoxedEditorUIView ); } ); } ); diff --git a/tests/_utils-tests/classictesteditorui.js b/tests/_utils-tests/classictesteditorui.js deleted file mode 100644 index a070c2db..00000000 --- a/tests/_utils-tests/classictesteditorui.js +++ /dev/null @@ -1,46 +0,0 @@ -/** - * @license Copyright (c) 2003-2018, CKSource - Frederico Knabben. All rights reserved. - * For licensing, see LICENSE.md. - */ - -import ComponentFactory from '@ckeditor/ckeditor5-ui/src/componentfactory'; -import FocusTracker from '@ckeditor/ckeditor5-utils/src/focustracker'; -import ClassicTestEditorUI from '../../tests/_utils/classictesteditorui'; -import View from '@ckeditor/ckeditor5-ui/src/view'; - -describe( 'ClassicTestEditorUI', () => { - let editor, view, ui; - - beforeEach( () => { - editor = {}; - view = new View(); - ui = new ClassicTestEditorUI( editor, view ); - } ); - - describe( 'constructor()', () => { - it( 'sets #editor', () => { - expect( ui.editor ).to.equal( editor ); - } ); - - it( 'sets #view', () => { - expect( ui.view ).to.equal( view ); - } ); - - it( 'creates #componentFactory factory', () => { - expect( ui.componentFactory ).to.be.instanceOf( ComponentFactory ); - } ); - - it( 'creates #focusTracker', () => { - expect( ui.focusTracker ).to.be.instanceOf( FocusTracker ); - } ); - } ); - - describe( 'destroy()', () => { - it( 'destroys the #view', () => { - const spy = sinon.spy( view, 'destroy' ); - - ui.destroy(); - sinon.assert.calledOnce( spy ); - } ); - } ); -} ); diff --git a/tests/_utils/classictesteditor.js b/tests/_utils/classictesteditor.js index 8a7d4c7a..15a0a6ba 100644 --- a/tests/_utils/classictesteditor.js +++ b/tests/_utils/classictesteditor.js @@ -7,7 +7,7 @@ import Editor from '../../src/editor/editor'; import ElementApiMixin from '../../src/editor/utils/elementapimixin'; import DataApiMixin from '../../src/editor/utils/dataapimixin'; import HtmlDataProcessor from '@ckeditor/ckeditor5-engine/src/dataprocessor/htmldataprocessor'; -import ClassicTestEditorUI from './classictesteditorui'; +import EditorUI from '../../src/editor/editorui'; import BoxedEditorUIView from '@ckeditor/ckeditor5-ui/src/editorui/boxed/boxededitoruiview'; import ElementReplacer from '@ckeditor/ckeditor5-utils/src/elementreplacer'; import InlineEditableUIView from '@ckeditor/ckeditor5-ui/src/editableui/inline/inlineeditableuiview'; @@ -33,7 +33,7 @@ export default class ClassicTestEditor extends Editor { // Use the HTML data processor in this editor. this.data.processor = new HtmlDataProcessor(); - this.ui = new ClassicTestEditorUI( this, new BoxedEditorUIView( this.locale ) ); + this.ui = new EditorUI( this, new BoxedEditorUIView( this.locale ) ); // Expose properties normally exposed by the ClassicEditorUI. this.ui.view.editable = new InlineEditableUIView( this.ui.view.locale ); diff --git a/tests/_utils/classictesteditorui.js b/tests/_utils/classictesteditorui.js deleted file mode 100644 index 1251f315..00000000 --- a/tests/_utils/classictesteditorui.js +++ /dev/null @@ -1,63 +0,0 @@ -/** - * @license Copyright (c) 2003-2018, CKSource - Frederico Knabben. All rights reserved. - * For licensing, see LICENSE.md. - */ - -import ComponentFactory from '@ckeditor/ckeditor5-ui/src/componentfactory'; -import FocusTracker from '@ckeditor/ckeditor5-utils/src/focustracker'; - -/** - * A simplified classic editor UI class. Useful for testing features. - * - * @memberOf tests.core._utils - * @extends ui.View - */ -export default class ClassicTestEditorUI { - /** - * Creates an instance of the test editor UI class. - * - * @param {core.editor.Editor} editor The editor instance. - * @param {ui.editorUI.EditorUIView} view View of the ui. - */ - constructor( editor, view ) { - /** - * Editor that the UI belongs to. - * - * @readonly - * @member {core.editor.Editor} tests.core._utils.ClassicTestEditorUI#editor - */ - this.editor = editor; - - /** - * View of the ui. - * - * @readonly - * @member {ui.editorUI.EditorUIView} tests.core._utils.ClassicTestEditorUI#view - */ - this.view = view; - - /** - * Instance of the {@link ui.ComponentFactory}. - * - * @readonly - * @member {ui.ComponentFactory} tests.core._utils.ClassicTestEditorUI#componentFactory - */ - this.componentFactory = new ComponentFactory( editor ); - - /** - * Keeps information about editor focus. - * - * @member {utils.FocusTracker} tests.core._utils.ClassicTestEditorUI#focusTracker - */ - this.focusTracker = new FocusTracker(); - } - - /** - * Destroys the UI. - * - * @returns {Promise} A Promise resolved when the destruction process is finished. - */ - destroy() { - this.view.destroy(); - } -} From f7dfa0c23dcb4e95ca4cb6877419fca3464879e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Tue, 26 Jun 2018 10:17:40 +0200 Subject: [PATCH 04/12] Used relative path for a local import. --- tests/editor/editorui.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/editor/editorui.js b/tests/editor/editorui.js index 857f667e..9ade3bfa 100644 --- a/tests/editor/editorui.js +++ b/tests/editor/editorui.js @@ -10,7 +10,7 @@ import FocusTracker from '@ckeditor/ckeditor5-utils/src/focustracker'; import ComponentFactory from '@ckeditor/ckeditor5-ui/src/componentfactory'; import View from '@ckeditor/ckeditor5-ui/src/view'; -import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils'; +import testUtils from '../_utils/utils'; testUtils.createSinonSandbox(); From 2ffd3db7602f7b8d6c0032e47d9f4c2d5356d601 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Tue, 26 Jun 2018 10:17:52 +0200 Subject: [PATCH 05/12] Minor improvement. --- src/editor/editorui.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/editor/editorui.js b/src/editor/editorui.js index 8ef9a082..e6365f56 100644 --- a/src/editor/editorui.js +++ b/src/editor/editorui.js @@ -70,7 +70,7 @@ export default class EditorUI { this._throttledUpdate = throttle( () => this.fire( 'update' ), 200 ); // Informs UI components that should be refreshed after layout change. - this.listenTo( editor.editing.view.document, 'layoutChanged', () => this._throttledUpdate() ); + this.listenTo( editor.editing.view.document, 'layoutChanged', () => this.update() ); } /** From b2c08c5ed5b5cbd735bf3b887ae191f6147eb304 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Tue, 26 Jun 2018 13:07:25 +0200 Subject: [PATCH 06/12] Reduced throttle factor of ui#update event. --- src/editor/editorui.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/editor/editorui.js b/src/editor/editorui.js index e6365f56..487ffd02 100644 --- a/src/editor/editorui.js +++ b/src/editor/editorui.js @@ -67,14 +67,14 @@ export default class EditorUI { * @protected * @type {Function} */ - this._throttledUpdate = throttle( () => this.fire( 'update' ), 200 ); + this._throttledUpdate = throttle( () => this.fire( 'update' ), 50 ); // Informs UI components that should be refreshed after layout change. this.listenTo( editor.editing.view.document, 'layoutChanged', () => this.update() ); } /** - * Triggers the UI update. + * Fires the UI update. */ update() { this._throttledUpdate(); From 27b6bb69ef90790ee58e0db32033761f70885e56 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Tue, 26 Jun 2018 18:18:32 +0200 Subject: [PATCH 07/12] Docs: Fixed invalid docs. --- src/editor/editorui.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/editor/editorui.js b/src/editor/editorui.js index 487ffd02..3382c357 100644 --- a/src/editor/editorui.js +++ b/src/editor/editorui.js @@ -95,7 +95,7 @@ export default class EditorUI { * It is fired after each {@link module:engine/view/document~Document#event:layoutChanged} event * besides it can be fired manually through {@link module:core/editor/editorui~EditorUI#update} method. * - * @event core/editor/editorui~EditorUI#event:update + * @event update */ } From a596cfa928c7ab8df407803585a02655c405e9a8 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Wed, 27 Jun 2018 16:06:20 +0200 Subject: [PATCH 08/12] Docs: Improvements in the EditorUI class. --- src/editor/editorui.js | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/src/editor/editorui.js b/src/editor/editorui.js index 3382c357..331980fb 100644 --- a/src/editor/editorui.js +++ b/src/editor/editorui.js @@ -15,7 +15,7 @@ import mix from '@ckeditor/ckeditor5-utils/src/mix'; import throttle from '@ckeditor/ckeditor5-utils/src/lib/lodash/throttle'; /** - * Class providing the minimal interface that is required to successfully bootstrap any editor UI. + * A class providing the minimal interface that is required to successfully bootstrap any editor UI. * * @mixes module:utils/emittermixin~EmitterMixin */ @@ -28,7 +28,7 @@ export default class EditorUI { */ constructor( editor, view ) { /** - * Editor that the UI belongs to. + * The editor that the UI belongs to. * * @readonly * @member {module:core/editor/editor~Editor} #editor @@ -44,7 +44,7 @@ export default class EditorUI { this.view = view; /** - * Instance of the {@link module:ui/componentfactory~ComponentFactory}, a registry used by plugins + * An instance of the {@link module:ui/componentfactory~ComponentFactory}, a registry used by plugins * to register factories of specific UI components. * * @readonly @@ -53,8 +53,8 @@ export default class EditorUI { this.componentFactory = new ComponentFactory( editor ); /** - * Keeps information about editor UI focus and propagates it among various plugins and components, - * unifying them in a uniform focus group. + * Stores the information about the editor UI focus and propagates it so various plugins and components + * are unified as a focus group. * * @readonly * @member {module:utils/focustracker~FocusTracker} #focusTracker @@ -74,7 +74,7 @@ export default class EditorUI { } /** - * Fires the UI update. + * Fires the (throttled) {@link module:core/editor/editorui~EditorUI#event:update} event. */ update() { this._throttledUpdate(); @@ -90,10 +90,13 @@ export default class EditorUI { } /** - * Fired whenever UI and all related components should be refreshed. + * Fired whenever the UI (all related components) should be refreshed. * - * It is fired after each {@link module:engine/view/document~Document#event:layoutChanged} event - * besides it can be fired manually through {@link module:core/editor/editorui~EditorUI#update} method. + * **Note:**: The event is fired after each {@link module:engine/view/document~Document#event:layoutChanged}. + * It can also be fired manually via the {@link module:core/editor/editorui~EditorUI#update} method. + * + * **Note:**: This event is not fired immediately but throttled to improve the performance of the UI + * and the overall responsiveness of the editor. * * @event update */ From 5be1d27e9ce554cf45c22e8dd9abedfb58a0e080 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Wed, 27 Jun 2018 17:06:37 +0200 Subject: [PATCH 09/12] Docs: Improved docs of the EditorUI. --- src/editor/editorui.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/editor/editorui.js b/src/editor/editorui.js index 331980fb..5fcb28d8 100644 --- a/src/editor/editorui.js +++ b/src/editor/editorui.js @@ -95,7 +95,7 @@ export default class EditorUI { * **Note:**: The event is fired after each {@link module:engine/view/document~Document#event:layoutChanged}. * It can also be fired manually via the {@link module:core/editor/editorui~EditorUI#update} method. * - * **Note:**: This event is not fired immediately but throttled to improve the performance of the UI + * **Note:**: Successive `update` events will throttled (50ms) to improve the performance of the UI * and the overall responsiveness of the editor. * * @event update From 1c2cc8bdbcfeea4aa0fa5234f3aa0be17b65e688 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Thu, 28 Jun 2018 11:39:21 +0200 Subject: [PATCH 10/12] Changed throttle factor. --- src/editor/editorui.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/editor/editorui.js b/src/editor/editorui.js index 5fcb28d8..aef954f1 100644 --- a/src/editor/editorui.js +++ b/src/editor/editorui.js @@ -67,7 +67,7 @@ export default class EditorUI { * @protected * @type {Function} */ - this._throttledUpdate = throttle( () => this.fire( 'update' ), 50 ); + this._throttledUpdate = throttle( () => this.fire( 'update' ), 10 ); // Informs UI components that should be refreshed after layout change. this.listenTo( editor.editing.view.document, 'layoutChanged', () => this.update() ); @@ -95,7 +95,7 @@ export default class EditorUI { * **Note:**: The event is fired after each {@link module:engine/view/document~Document#event:layoutChanged}. * It can also be fired manually via the {@link module:core/editor/editorui~EditorUI#update} method. * - * **Note:**: Successive `update` events will throttled (50ms) to improve the performance of the UI + * **Note:**: Successive `update` events will throttled (10ms) to improve the performance of the UI * and the overall responsiveness of the editor. * * @event update From 3ab8d2cace5cd7650392d6a32705349b5c760d88 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Thu, 28 Jun 2018 12:27:40 +0200 Subject: [PATCH 11/12] Decreased EditorUI#update from 10ms to 5ms to prevent the floating UI wiggle. --- src/editor/editorui.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/editor/editorui.js b/src/editor/editorui.js index aef954f1..f21dbd73 100644 --- a/src/editor/editorui.js +++ b/src/editor/editorui.js @@ -67,7 +67,7 @@ export default class EditorUI { * @protected * @type {Function} */ - this._throttledUpdate = throttle( () => this.fire( 'update' ), 10 ); + this._throttledUpdate = throttle( () => this.fire( 'update' ), 5 ); // Informs UI components that should be refreshed after layout change. this.listenTo( editor.editing.view.document, 'layoutChanged', () => this.update() ); @@ -95,7 +95,7 @@ export default class EditorUI { * **Note:**: The event is fired after each {@link module:engine/view/document~Document#event:layoutChanged}. * It can also be fired manually via the {@link module:core/editor/editorui~EditorUI#update} method. * - * **Note:**: Successive `update` events will throttled (10ms) to improve the performance of the UI + * **Note:**: Successive `update` events will throttled (5ms) to improve the performance of the UI * and the overall responsiveness of the editor. * * @event update From 051d76bd72be3f523cb53c47442a0bd7fcae0384 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Thu, 28 Jun 2018 13:10:25 +0200 Subject: [PATCH 12/12] Removed throttle when firing update event. --- src/editor/editorui.js | 17 ++--------------- tests/editor/editorui.js | 29 ++--------------------------- 2 files changed, 4 insertions(+), 42 deletions(-) diff --git a/src/editor/editorui.js b/src/editor/editorui.js index f21dbd73..e2d9e754 100644 --- a/src/editor/editorui.js +++ b/src/editor/editorui.js @@ -12,7 +12,6 @@ import FocusTracker from '@ckeditor/ckeditor5-utils/src/focustracker'; import EmitterMixin from '@ckeditor/ckeditor5-utils/src/emittermixin'; import mix from '@ckeditor/ckeditor5-utils/src/mix'; -import throttle from '@ckeditor/ckeditor5-utils/src/lib/lodash/throttle'; /** * A class providing the minimal interface that is required to successfully bootstrap any editor UI. @@ -61,23 +60,15 @@ export default class EditorUI { */ this.focusTracker = new FocusTracker(); - /** - * Fires throttled {@link module:core/editor/editorui~EditorUI#event:update} event. - * - * @protected - * @type {Function} - */ - this._throttledUpdate = throttle( () => this.fire( 'update' ), 5 ); - // Informs UI components that should be refreshed after layout change. this.listenTo( editor.editing.view.document, 'layoutChanged', () => this.update() ); } /** - * Fires the (throttled) {@link module:core/editor/editorui~EditorUI#event:update} event. + * Fires the {@link module:core/editor/editorui~EditorUI#event:update} event. */ update() { - this._throttledUpdate(); + this.fire( 'update' ); } /** @@ -85,7 +76,6 @@ export default class EditorUI { */ destroy() { this.stopListening(); - this._throttledUpdate.cancel(); this.view.destroy(); } @@ -95,9 +85,6 @@ export default class EditorUI { * **Note:**: The event is fired after each {@link module:engine/view/document~Document#event:layoutChanged}. * It can also be fired manually via the {@link module:core/editor/editorui~EditorUI#update} method. * - * **Note:**: Successive `update` events will throttled (5ms) to improve the performance of the UI - * and the overall responsiveness of the editor. - * * @event update */ } diff --git a/tests/editor/editorui.js b/tests/editor/editorui.js index 9ade3bfa..8b616513 100644 --- a/tests/editor/editorui.js +++ b/tests/editor/editorui.js @@ -47,7 +47,7 @@ describe( 'EditorUI', () => { expect( ui.focusTracker ).to.be.instanceOf( FocusTracker ); } ); - it( 'should fire throttled update event after viewDocument#layoutChanged', () => { + it( 'should fire update event after viewDocument#layoutChanged', () => { const spy = sinon.spy(); ui.on( 'update', spy ); @@ -58,16 +58,12 @@ describe( 'EditorUI', () => { editor.editing.view.document.fire( 'layoutChanged' ); - sinon.assert.calledOnce( spy ); - - ui._throttledUpdate.flush(); - sinon.assert.calledTwice( spy ); } ); } ); describe( 'update()', () => { - it( 'should fire throttled update event', () => { + it( 'should fire update event', () => { const spy = sinon.spy(); ui.on( 'update', spy ); @@ -78,10 +74,6 @@ describe( 'EditorUI', () => { ui.update(); - sinon.assert.calledOnce( spy ); - - ui._throttledUpdate.flush(); - sinon.assert.calledTwice( spy ); } ); } ); @@ -102,22 +94,5 @@ describe( 'EditorUI', () => { sinon.assert.called( spy ); } ); - - it( 'should cancel throttled update event', () => { - const spy = sinon.spy(); - - ui.on( 'update', spy ); - - ui.update(); - - sinon.assert.calledOnce( spy ); - - ui.destroy(); - - ui.update(); - ui._throttledUpdate.flush(); - - sinon.assert.calledOnce( spy ); - } ); } ); } );