From 831431f28bcae3374a5225f253f0838c445abec1 Mon Sep 17 00:00:00 2001 From: panr Date: Tue, 14 Jan 2020 15:12:41 +0100 Subject: [PATCH 01/24] Fix: Set inline toolbar's max-width to be the same as editable's width and make it groupable --- src/inlineeditorui.js | 7 +++++++ src/inlineeditoruiview.js | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/inlineeditorui.js b/src/inlineeditorui.js index cac847a..0007800 100644 --- a/src/inlineeditorui.js +++ b/src/inlineeditorui.js @@ -11,6 +11,7 @@ import EditorUI from '@ckeditor/ckeditor5-core/src/editor/editorui'; import enableToolbarKeyboardFocus from '@ckeditor/ckeditor5-ui/src/toolbar/enabletoolbarkeyboardfocus'; import normalizeToolbarConfig from '@ckeditor/ckeditor5-ui/src/toolbar/normalizetoolbarconfig'; import { enablePlaceholder } from '@ckeditor/ckeditor5-engine/src/view/placeholder'; +import Rect from '@ckeditor/ckeditor5-utils/src/dom/rect'; /** * The inline editor UI class. @@ -139,6 +140,12 @@ export default class InlineEditorUI extends EditorUI { target: editableElement, positions: view.panelPositions } ); + + // Set toolbar's max-width only once. + if ( !toolbar.element.style.maxWidth ) { + const editableRect = new Rect( editableElement ); + toolbar.element.style.maxWidth = `${ editableRect.width }px`; + } } } ); diff --git a/src/inlineeditoruiview.js b/src/inlineeditoruiview.js index 0c89f32..5f60175 100644 --- a/src/inlineeditoruiview.js +++ b/src/inlineeditoruiview.js @@ -35,7 +35,7 @@ export default class InlineEditorUIView extends EditorUIView { * @readonly * @member {module:ui/toolbar/toolbarview~ToolbarView} */ - this.toolbar = new ToolbarView( locale ); + this.toolbar = new ToolbarView( locale, { shouldGroupWhenFull: true } ); /** * The offset from the top edge of the web browser's viewport which makes the From 27e8f78e4c6dcb1d6026629fa85cb65613b1c250 Mon Sep 17 00:00:00 2001 From: panr Date: Wed, 15 Jan 2020 13:09:58 +0100 Subject: [PATCH 02/24] Tests: Add tests to cover setting max-width for toolbar --- tests/inlineeditorui.js | 50 ++++++++++++++++++++++++++++++++++++----- 1 file changed, 45 insertions(+), 5 deletions(-) diff --git a/tests/inlineeditorui.js b/tests/inlineeditorui.js index 4bab7a5..82a0a1d 100644 --- a/tests/inlineeditorui.js +++ b/tests/inlineeditorui.js @@ -83,22 +83,62 @@ describe( 'InlineEditorUI', () => { } ); // https://github.com/ckeditor/ckeditor5-editor-inline/issues/4 - it( 'pin() is called on editor.ui#update', () => { + it( 'pin() is called on editor.ui#update when editable element is in the DOM', () => { + const spy = sinon.stub( view.panel, 'pin' ); + + viewElement.ownerDocument.body.append( viewElement ); + view.panel.show(); + + expect( viewElement.ownerDocument.body.contains( viewElement ) ).to.be.true; + + editor.ui.fire( 'update' ); + sinon.assert.calledOnce( spy ); + sinon.assert.calledWithExactly( spy, { + target: view.editable.element, + positions: sinon.match.array + } ); + } ); + + it( 'pin() is not called on editor.ui#update when panel is hidden', () => { const spy = sinon.stub( view.panel, 'pin' ); view.panel.hide(); editor.ui.fire( 'update' ); sinon.assert.notCalled( spy ); + } ); + + it( 'pin() is not called on editor.ui#update when panel is visible but editable element is not in the DOM', () => { + const spy = sinon.stub( view.panel, 'pin' ); view.panel.show(); + expect( !viewElement.ownerDocument.body.contains( viewElement ) ).to.be.true; + editor.ui.fire( 'update' ); + sinon.assert.notCalled( spy ); + } ); + + it( 'toolbar max-width is set on editor.ui#update only once when editable element is in the DOM', () => { + const spy = sinon.spy( editor.ui, '_setToolbarMaxWidth' ); + testUtils.sinon.stub( viewElement, 'getBoundingClientRect' ).returns( { width: 100 } ); + + viewElement.ownerDocument.body.append( viewElement ); + view.panel.show(); + + expect( view.toolbar.element.style.maxWidth ).to.be.equal( '' ); + + editor.ui.fire( 'update' ); + + expect( view.toolbar.element.style.maxWidth ).to.be.equal( '100px' ); + + editor.ui.fire( 'update' ); + sinon.assert.calledOnce( spy ); - sinon.assert.calledWithExactly( spy, { - target: view.editable.element, - positions: sinon.match.array - } ); + } ); + + it( 'toolbar should group items by default', () => { + expect( view.toolbar.options.shouldGroupWhenFull ).to.be.true; } ); } ); From 28b64ee0d49d6086683e58d498cbcd7382890307 Mon Sep 17 00:00:00 2001 From: panr Date: Wed, 15 Jan 2020 13:28:37 +0100 Subject: [PATCH 03/24] Fix: Refactor setting max-width for toolbar to handle tests scenarios --- src/inlineeditorui.js | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/src/inlineeditorui.js b/src/inlineeditorui.js index 0007800..1053546 100644 --- a/src/inlineeditorui.js +++ b/src/inlineeditorui.js @@ -133,6 +133,10 @@ export default class InlineEditorUI extends EditorUI { // https://github.com/ckeditor/ckeditor5-editor-inline/issues/4 view.listenTo( editor.ui, 'update', () => { + if ( !editableElement.ownerDocument.body.contains( editableElement ) ) { + return; + } + // Don't pin if the panel is not already visible. It prevents the panel // showing up when there's no focus in the UI. if ( view.panel.isVisible ) { @@ -142,9 +146,8 @@ export default class InlineEditorUI extends EditorUI { } ); // Set toolbar's max-width only once. - if ( !toolbar.element.style.maxWidth ) { - const editableRect = new Rect( editableElement ); - toolbar.element.style.maxWidth = `${ editableRect.width }px`; + if ( toolbar.element.style.maxWidth === '' ) { + this._setToolbarMaxWidth( new Rect( editableElement ).width ); } } } ); @@ -182,4 +185,16 @@ export default class InlineEditorUI extends EditorUI { } ); } } + + /** + * Set max-width for toolbar. + * + * @private + */ + _setToolbarMaxWidth( width ) { + const view = this.view; + const toolbar = view.toolbar; + + toolbar.element.style.maxWidth = `${ width }px`; + } } From 1e71bb7a8827db831466b4879aad1ff3b8d890e0 Mon Sep 17 00:00:00 2001 From: panr Date: Wed, 15 Jan 2020 13:42:05 +0100 Subject: [PATCH 04/24] Tests: Fix tests DOM leaks --- tests/inlineeditorui.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/inlineeditorui.js b/tests/inlineeditorui.js index 82a0a1d..4ba0ba2 100644 --- a/tests/inlineeditorui.js +++ b/tests/inlineeditorui.js @@ -97,6 +97,8 @@ describe( 'InlineEditorUI', () => { target: view.editable.element, positions: sinon.match.array } ); + + viewElement.remove(); } ); it( 'pin() is not called on editor.ui#update when panel is hidden', () => { @@ -135,6 +137,8 @@ describe( 'InlineEditorUI', () => { editor.ui.fire( 'update' ); sinon.assert.calledOnce( spy ); + + viewElement.remove(); } ); it( 'toolbar should group items by default', () => { From 1cdf63c22e4c86e2ff02ce023b70c168aa629a40 Mon Sep 17 00:00:00 2001 From: panr Date: Wed, 22 Jan 2020 14:53:17 +0100 Subject: [PATCH 05/24] WIP: Refactor code --- src/inlineeditorui.js | 16 +++++++++------- tests/inlineeditorui.js | 23 +++++++++-------------- 2 files changed, 18 insertions(+), 21 deletions(-) diff --git a/src/inlineeditorui.js b/src/inlineeditorui.js index 1053546..6dc14e0 100644 --- a/src/inlineeditorui.js +++ b/src/inlineeditorui.js @@ -11,7 +11,7 @@ import EditorUI from '@ckeditor/ckeditor5-core/src/editor/editorui'; import enableToolbarKeyboardFocus from '@ckeditor/ckeditor5-ui/src/toolbar/enabletoolbarkeyboardfocus'; import normalizeToolbarConfig from '@ckeditor/ckeditor5-ui/src/toolbar/normalizetoolbarconfig'; import { enablePlaceholder } from '@ckeditor/ckeditor5-engine/src/view/placeholder'; -import Rect from '@ckeditor/ckeditor5-utils/src/dom/rect'; +import getResizeObserver from '@ckeditor/ckeditor5-utils/src/dom/getresizeobserver'; /** * The inline editor UI class. @@ -144,11 +144,6 @@ export default class InlineEditorUI extends EditorUI { target: editableElement, positions: view.panelPositions } ); - - // Set toolbar's max-width only once. - if ( toolbar.element.style.maxWidth === '' ) { - this._setToolbarMaxWidth( new Rect( editableElement ).width ); - } } } ); @@ -160,6 +155,13 @@ export default class InlineEditorUI extends EditorUI { originKeystrokeHandler: editor.keystrokes, toolbar } ); + + // Set toolbar's max-width on the initialization and update it on the editable resize. + const widthObserver = getResizeObserver( ( [ entry ] ) => { + this._setToolbarMaxWidth( entry.contentRect.width ); + } ); + + widthObserver.observe( editableElement ); } /** @@ -195,6 +197,6 @@ export default class InlineEditorUI extends EditorUI { const view = this.view; const toolbar = view.toolbar; - toolbar.element.style.maxWidth = `${ width }px`; + toolbar.maxWidth = width; } } diff --git a/tests/inlineeditorui.js b/tests/inlineeditorui.js index 4ba0ba2..11e10ca 100644 --- a/tests/inlineeditorui.js +++ b/tests/inlineeditorui.js @@ -121,24 +121,19 @@ describe( 'InlineEditorUI', () => { sinon.assert.notCalled( spy ); } ); - it( 'toolbar max-width is set on editor.ui#update only once when editable element is in the DOM', () => { - const spy = sinon.spy( editor.ui, '_setToolbarMaxWidth' ); - testUtils.sinon.stub( viewElement, 'getBoundingClientRect' ).returns( { width: 100 } ); + // TODO: HELP? + it( 'toolbar max-width is set to the value of width of the editable element', () => { + document.body.appendChild( viewElement ); - viewElement.ownerDocument.body.append( viewElement ); - view.panel.show(); - - expect( view.toolbar.element.style.maxWidth ).to.be.equal( '' ); - - editor.ui.fire( 'update' ); + viewElement.style.width = '400px'; - expect( view.toolbar.element.style.maxWidth ).to.be.equal( '100px' ); + expect( viewElement.ownerDocument.body.contains( viewElement ) ).to.be.true; + expect( view.toolbar.element.ownerDocument.body.contains( view.toolbar.element ) ).to.be.true; + expect( view.toolbar.element.style.maxWidth ).to.be.equal( '400px' ); - editor.ui.fire( 'update' ); + document.body.removeChild( viewElement ); - sinon.assert.calledOnce( spy ); - - viewElement.remove(); + expect( viewElement ).to.not.exist; } ); it( 'toolbar should group items by default', () => { From e2a99cda4d7d6420c764b8afedfed08b700f2d19 Mon Sep 17 00:00:00 2001 From: panr Date: Thu, 23 Jan 2020 12:28:22 +0100 Subject: [PATCH 06/24] Refactor inline editor toolbar helper for setting max-width --- src/inlineeditorui.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/inlineeditorui.js b/src/inlineeditorui.js index 6dc14e0..f742ef3 100644 --- a/src/inlineeditorui.js +++ b/src/inlineeditorui.js @@ -12,6 +12,9 @@ import enableToolbarKeyboardFocus from '@ckeditor/ckeditor5-ui/src/toolbar/enabl import normalizeToolbarConfig from '@ckeditor/ckeditor5-ui/src/toolbar/normalizetoolbarconfig'; import { enablePlaceholder } from '@ckeditor/ckeditor5-engine/src/view/placeholder'; import getResizeObserver from '@ckeditor/ckeditor5-utils/src/dom/getresizeobserver'; +import toUnit from '@ckeditor/ckeditor5-utils/src/dom/tounit'; + +const toPx = toUnit( 'px' ); /** * The inline editor UI class. @@ -197,6 +200,6 @@ export default class InlineEditorUI extends EditorUI { const view = this.view; const toolbar = view.toolbar; - toolbar.maxWidth = width; + toolbar.maxWidth = toPx( width ); } } From 78c6947dbb79a994bf8128a0ed6d199aa970bd57 Mon Sep 17 00:00:00 2001 From: panr Date: Thu, 23 Jan 2020 12:28:58 +0100 Subject: [PATCH 07/24] Rewrite and refactor tests for inline editor toolbar and panel --- tests/inlineeditorui.js | 33 +++++++++++++++++++++------------ 1 file changed, 21 insertions(+), 12 deletions(-) diff --git a/tests/inlineeditorui.js b/tests/inlineeditorui.js index 11e10ca..7134b68 100644 --- a/tests/inlineeditorui.js +++ b/tests/inlineeditorui.js @@ -3,7 +3,7 @@ * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license */ -/* globals document, Event */ +/* globals document, Event, setTimeout */ import View from '@ckeditor/ckeditor5-ui/src/view'; @@ -53,6 +53,12 @@ describe( 'InlineEditorUI', () => { } ); describe( 'panel', () => { + afterEach( () => { + if ( document.body.contains( viewElement ) ) { + document.body.removeChild( viewElement ); + } + } ); + it( 'binds view.panel#isVisible to editor.ui#focusTracker', () => { ui.focusTracker.isFocused = false; expect( view.panel.isVisible ).to.be.false; @@ -86,10 +92,10 @@ describe( 'InlineEditorUI', () => { it( 'pin() is called on editor.ui#update when editable element is in the DOM', () => { const spy = sinon.stub( view.panel, 'pin' ); - viewElement.ownerDocument.body.append( viewElement ); + document.body.appendChild( viewElement ); view.panel.show(); - expect( viewElement.ownerDocument.body.contains( viewElement ) ).to.be.true; + expect( document.body.contains( viewElement ) ).to.be.true; editor.ui.fire( 'update' ); sinon.assert.calledOnce( spy ); @@ -115,25 +121,28 @@ describe( 'InlineEditorUI', () => { view.panel.show(); - expect( !viewElement.ownerDocument.body.contains( viewElement ) ).to.be.true; + expect( document.body.contains( viewElement ) ).to.be.false; editor.ui.fire( 'update' ); sinon.assert.notCalled( spy ); } ); - // TODO: HELP? - it( 'toolbar max-width is set to the value of width of the editable element', () => { + it( 'toolbar max-width is set to the value of width of the editable element, otherwise it can be wider then editor', done => { document.body.appendChild( viewElement ); - viewElement.style.width = '400px'; + expect( document.body.contains( viewElement ) ).to.be.true; - expect( viewElement.ownerDocument.body.contains( viewElement ) ).to.be.true; - expect( view.toolbar.element.ownerDocument.body.contains( view.toolbar.element ) ).to.be.true; - expect( view.toolbar.element.style.maxWidth ).to.be.equal( '400px' ); + viewElement.style.width = '400px'; - document.body.removeChild( viewElement ); + // Unfortunately we have to wait for async ResizeObserver execution. + // ResizeObserver which has been called after changing width of viewElement, + // needs 2x requestAnimationFrame or timeout to update a layout. + // See more: https://twitter.com/paul_irish/status/912693347315150849/photo/1 + setTimeout( () => { + expect( view.toolbar.maxWidth ).to.be.equal( '400px' ); - expect( viewElement ).to.not.exist; + done(); + }, 100 ); } ); it( 'toolbar should group items by default', () => { From 790e1279b0de48a9e1865ff09cb646fc05d50161 Mon Sep 17 00:00:00 2001 From: panr Date: Thu, 23 Jan 2020 16:26:54 +0100 Subject: [PATCH 08/24] Add config option for inline toolbar to set if shouldn't group items --- src/inlineeditor.js | 4 +++- src/inlineeditorui.js | 13 ++++++++----- src/inlineeditoruiview.js | 4 ++-- 3 files changed, 13 insertions(+), 8 deletions(-) diff --git a/src/inlineeditor.js b/src/inlineeditor.js index 1979ef4..23ee024 100644 --- a/src/inlineeditor.js +++ b/src/inlineeditor.js @@ -73,7 +73,9 @@ export default class InlineEditor extends Editor { secureSourceElement( this ); } - const view = new InlineEditorUIView( this.locale, this.editing.view, this.sourceElement ); + const shouldToolbarGroupWhenFull = !this.config.get( 'toolbar.shouldNotGroupWhenFull' ); + + const view = new InlineEditorUIView( this.locale, this.editing.view, this.sourceElement, { shouldToolbarGroupWhenFull } ); this.ui = new InlineEditorUI( this, view ); attachToForm( this ); diff --git a/src/inlineeditorui.js b/src/inlineeditorui.js index f742ef3..a4eea65 100644 --- a/src/inlineeditorui.js +++ b/src/inlineeditorui.js @@ -159,12 +159,15 @@ export default class InlineEditorUI extends EditorUI { toolbar } ); - // Set toolbar's max-width on the initialization and update it on the editable resize. - const widthObserver = getResizeObserver( ( [ entry ] ) => { - this._setToolbarMaxWidth( entry.contentRect.width ); - } ); + // Set toolbar's max-width on the initialization and update it on the editable resize, + // if 'shouldToolbarGroupWhenFull' in config is set to 'true'. + if ( !this._toolbarConfig.shouldNotGroupWhenFull ) { + const widthObserver = getResizeObserver( ( [ entry ] ) => { + this._setToolbarMaxWidth( entry.contentRect.width ); + } ); - widthObserver.observe( editableElement ); + widthObserver.observe( editableElement ); + } } /** diff --git a/src/inlineeditoruiview.js b/src/inlineeditoruiview.js index 5f60175..089be47 100644 --- a/src/inlineeditoruiview.js +++ b/src/inlineeditoruiview.js @@ -26,7 +26,7 @@ export default class InlineEditorUIView extends EditorUIView { * @param {HTMLElement} [editableElement] The editable element. If not specified, it will be automatically created by * {@link module:ui/editableui/editableuiview~EditableUIView}. Otherwise, the given element will be used. */ - constructor( locale, editingView, editableElement ) { + constructor( locale, editingView, editableElement, options = {} ) { super( locale ); /** @@ -35,7 +35,7 @@ export default class InlineEditorUIView extends EditorUIView { * @readonly * @member {module:ui/toolbar/toolbarview~ToolbarView} */ - this.toolbar = new ToolbarView( locale, { shouldGroupWhenFull: true } ); + this.toolbar = new ToolbarView( locale, { shouldGroupWhenFull: options.shouldToolbarGroupWhenFull } ); /** * The offset from the top edge of the web browser's viewport which makes the From 24ee2179629345bb5ac3e5693b946185decc6817 Mon Sep 17 00:00:00 2001 From: panr Date: Thu, 23 Jan 2020 16:27:41 +0100 Subject: [PATCH 09/24] Update test for shouldNotGroupWhenFull --- tests/inlineeditor.js | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/inlineeditor.js b/tests/inlineeditor.js index 547ad41..6efe8d1 100644 --- a/tests/inlineeditor.js +++ b/tests/inlineeditor.js @@ -183,6 +183,21 @@ describe( 'InlineEditor', () => { } ); } ); + it( 'inline toolbar should group items by default', () => { + const editorElement = document.createElement( 'div' ); + return InlineEditor.create( editorElement, { + toolbar: { + shouldNotGroupWhenFull: true + } + } ).then( editor => { + expect( editor.ui.view.toolbar.options.shouldGroupWhenFull ).to.be.false; + + return editor.destroy(); + } ).then( () => { + editorElement.remove(); + } ); + } ); + it( 'throws if initial data is passed in Editor#create and config.initialData is also used', done => { InlineEditor.create( '

Hello world!

', { initialData: '

I am evil!

', From 1fd53c59ac1ddca598217a6fc90e3b6de2588e44 Mon Sep 17 00:00:00 2001 From: panr Date: Thu, 23 Jan 2020 16:34:19 +0100 Subject: [PATCH 10/24] Change description of the inline editor max-width test --- tests/inlineeditorui.js | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/tests/inlineeditorui.js b/tests/inlineeditorui.js index 7134b68..fc0e326 100644 --- a/tests/inlineeditorui.js +++ b/tests/inlineeditorui.js @@ -127,7 +127,7 @@ describe( 'InlineEditorUI', () => { sinon.assert.notCalled( spy ); } ); - it( 'toolbar max-width is set to the value of width of the editable element, otherwise it can be wider then editor', done => { + it( 'should set inline toolbar max-width to the width of the editable element', done => { document.body.appendChild( viewElement ); expect( document.body.contains( viewElement ) ).to.be.true; @@ -144,10 +144,6 @@ describe( 'InlineEditorUI', () => { done(); }, 100 ); } ); - - it( 'toolbar should group items by default', () => { - expect( view.toolbar.options.shouldGroupWhenFull ).to.be.true; - } ); } ); describe( 'editable', () => { From f00ad89afbc95052490f799bb24aa3023dcb8137 Mon Sep 17 00:00:00 2001 From: panr Date: Thu, 23 Jan 2020 16:49:33 +0100 Subject: [PATCH 11/24] Fix test description for shouldNotGroupWhenFull check --- tests/inlineeditor.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/inlineeditor.js b/tests/inlineeditor.js index 6efe8d1..edae0c0 100644 --- a/tests/inlineeditor.js +++ b/tests/inlineeditor.js @@ -183,7 +183,7 @@ describe( 'InlineEditor', () => { } ); } ); - it( 'inline toolbar should group items by default', () => { + it( 'inline toolbar should not group items when shouldNotGroupWhenFull option is enabled', () => { const editorElement = document.createElement( 'div' ); return InlineEditor.create( editorElement, { toolbar: { From ffb077a5435af2ec5a9e5d22f15819f0dbe27857 Mon Sep 17 00:00:00 2001 From: panr Date: Thu, 23 Jan 2020 17:12:41 +0100 Subject: [PATCH 12/24] Increase test timeout while waiting for ResizeObserver --- tests/inlineeditorui.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/inlineeditorui.js b/tests/inlineeditorui.js index fc0e326..174f465 100644 --- a/tests/inlineeditorui.js +++ b/tests/inlineeditorui.js @@ -142,7 +142,7 @@ describe( 'InlineEditorUI', () => { expect( view.toolbar.maxWidth ).to.be.equal( '400px' ); done(); - }, 100 ); + }, 500 ); } ); } ); From 36f9063919cd3cdf422596f592f840dc800960ad Mon Sep 17 00:00:00 2001 From: panr Date: Wed, 29 Jan 2020 17:03:06 +0100 Subject: [PATCH 13/24] Remove helper and obsolete code checking if editable element is in the DOM --- src/inlineeditorui.js | 28 ++++++++++------------------ 1 file changed, 10 insertions(+), 18 deletions(-) diff --git a/src/inlineeditorui.js b/src/inlineeditorui.js index a4eea65..998141a 100644 --- a/src/inlineeditorui.js +++ b/src/inlineeditorui.js @@ -11,6 +11,7 @@ import EditorUI from '@ckeditor/ckeditor5-core/src/editor/editorui'; import enableToolbarKeyboardFocus from '@ckeditor/ckeditor5-ui/src/toolbar/enabletoolbarkeyboardfocus'; import normalizeToolbarConfig from '@ckeditor/ckeditor5-ui/src/toolbar/normalizetoolbarconfig'; import { enablePlaceholder } from '@ckeditor/ckeditor5-engine/src/view/placeholder'; +import Rect from '@ckeditor/ckeditor5-utils/src/dom/rect'; import getResizeObserver from '@ckeditor/ckeditor5-utils/src/dom/getresizeobserver'; import toUnit from '@ckeditor/ckeditor5-utils/src/dom/tounit'; @@ -136,10 +137,6 @@ export default class InlineEditorUI extends EditorUI { // https://github.com/ckeditor/ckeditor5-editor-inline/issues/4 view.listenTo( editor.ui, 'update', () => { - if ( !editableElement.ownerDocument.body.contains( editableElement ) ) { - return; - } - // Don't pin if the panel is not already visible. It prevents the panel // showing up when there's no focus in the UI. if ( view.panel.isVisible ) { @@ -162,8 +159,15 @@ export default class InlineEditorUI extends EditorUI { // Set toolbar's max-width on the initialization and update it on the editable resize, // if 'shouldToolbarGroupWhenFull' in config is set to 'true'. if ( !this._toolbarConfig.shouldNotGroupWhenFull ) { - const widthObserver = getResizeObserver( ( [ entry ] ) => { - this._setToolbarMaxWidth( entry.contentRect.width ); + const widthObserver = getResizeObserver( () => { + // We need to check if there's already the editable element in the DOM. + // Otherwise the `Rect` instance will complain that source (editableElement) is not available + // to obtain the element's geometry. + if ( !editableElement.ownerDocument.body.contains( editableElement ) ) { + return; + } + + this.view.toolbar.maxWidth = toPx( new Rect( editableElement ).width ); } ); widthObserver.observe( editableElement ); @@ -193,16 +197,4 @@ export default class InlineEditorUI extends EditorUI { } ); } } - - /** - * Set max-width for toolbar. - * - * @private - */ - _setToolbarMaxWidth( width ) { - const view = this.view; - const toolbar = view.toolbar; - - toolbar.maxWidth = toPx( width ); - } } From babb31379e7098476d8362ff2eda548be91fc430 Mon Sep 17 00:00:00 2001 From: panr Date: Wed, 29 Jan 2020 17:04:23 +0100 Subject: [PATCH 14/24] Refactor the line where passing shouldToolbarGroupWhenFull option to the InlineEditorUIView instance --- src/inlineeditor.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/inlineeditor.js b/src/inlineeditor.js index 23ee024..c248b64 100644 --- a/src/inlineeditor.js +++ b/src/inlineeditor.js @@ -75,7 +75,9 @@ export default class InlineEditor extends Editor { const shouldToolbarGroupWhenFull = !this.config.get( 'toolbar.shouldNotGroupWhenFull' ); - const view = new InlineEditorUIView( this.locale, this.editing.view, this.sourceElement, { shouldToolbarGroupWhenFull } ); + const view = new InlineEditorUIView( this.locale, this.editing.view, this.sourceElement, { + shouldToolbarGroupWhenFull + } ); this.ui = new InlineEditorUI( this, view ); attachToForm( this ); From 7428430a7089bcc71ba5cf65b8e912cb01ec7831 Mon Sep 17 00:00:00 2001 From: panr Date: Wed, 29 Jan 2020 17:06:20 +0100 Subject: [PATCH 15/24] Refactor the line where passing shouldGroupWhenFull option to the ToolbarView instance --- src/inlineeditoruiview.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/inlineeditoruiview.js b/src/inlineeditoruiview.js index 089be47..5fbfa75 100644 --- a/src/inlineeditoruiview.js +++ b/src/inlineeditoruiview.js @@ -35,7 +35,9 @@ export default class InlineEditorUIView extends EditorUIView { * @readonly * @member {module:ui/toolbar/toolbarview~ToolbarView} */ - this.toolbar = new ToolbarView( locale, { shouldGroupWhenFull: options.shouldToolbarGroupWhenFull } ); + this.toolbar = new ToolbarView( locale, { + shouldGroupWhenFull: options.shouldToolbarGroupWhenFull + } ); /** * The offset from the top edge of the web browser's viewport which makes the From 8fe4e582d47084857ebbb09b41950c994281dc9d Mon Sep 17 00:00:00 2001 From: panr Date: Wed, 29 Jan 2020 17:06:56 +0100 Subject: [PATCH 16/24] Fix tests after changes --- tests/inlineeditorui.js | 35 ++++++++++------------------------- 1 file changed, 10 insertions(+), 25 deletions(-) diff --git a/tests/inlineeditorui.js b/tests/inlineeditorui.js index 174f465..3995d68 100644 --- a/tests/inlineeditorui.js +++ b/tests/inlineeditorui.js @@ -13,10 +13,14 @@ import InlineEditorUIView from '../src/inlineeditoruiview'; import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph'; import VirtualTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/virtualtesteditor'; +import Rect from '@ckeditor/ckeditor5-utils/src/dom/rect'; import { keyCodes } from '@ckeditor/ckeditor5-utils/src/keyboard'; import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils'; import { assertBinding } from '@ckeditor/ckeditor5-utils/tests/_utils/utils'; import { isElement } from 'lodash-es'; +import toUnit from '@ckeditor/ckeditor5-utils/src/dom/tounit'; + +const toPx = toUnit( 'px' ); describe( 'InlineEditorUI', () => { let editor, view, ui, viewElement; @@ -53,12 +57,6 @@ describe( 'InlineEditorUI', () => { } ); describe( 'panel', () => { - afterEach( () => { - if ( document.body.contains( viewElement ) ) { - document.body.removeChild( viewElement ); - } - } ); - it( 'binds view.panel#isVisible to editor.ui#focusTracker', () => { ui.focusTracker.isFocused = false; expect( view.panel.isVisible ).to.be.false; @@ -89,22 +87,17 @@ describe( 'InlineEditorUI', () => { } ); // https://github.com/ckeditor/ckeditor5-editor-inline/issues/4 - it( 'pin() is called on editor.ui#update when editable element is in the DOM', () => { + it( 'pin() is called on editor.ui#update', () => { const spy = sinon.stub( view.panel, 'pin' ); - document.body.appendChild( viewElement ); view.panel.show(); - expect( document.body.contains( viewElement ) ).to.be.true; - editor.ui.fire( 'update' ); sinon.assert.calledOnce( spy ); sinon.assert.calledWithExactly( spy, { target: view.editable.element, positions: sinon.match.array } ); - - viewElement.remove(); } ); it( 'pin() is not called on editor.ui#update when panel is hidden', () => { @@ -116,22 +109,11 @@ describe( 'InlineEditorUI', () => { sinon.assert.notCalled( spy ); } ); - it( 'pin() is not called on editor.ui#update when panel is visible but editable element is not in the DOM', () => { - const spy = sinon.stub( view.panel, 'pin' ); - - view.panel.show(); - - expect( document.body.contains( viewElement ) ).to.be.false; - - editor.ui.fire( 'update' ); - sinon.assert.notCalled( spy ); - } ); - + // Sometimes this test can fail, due to the fact that it have to wait for async ResizeObserver execution. it( 'should set inline toolbar max-width to the width of the editable element', done => { document.body.appendChild( viewElement ); expect( document.body.contains( viewElement ) ).to.be.true; - viewElement.style.width = '400px'; // Unfortunately we have to wait for async ResizeObserver execution. @@ -139,7 +121,10 @@ describe( 'InlineEditorUI', () => { // needs 2x requestAnimationFrame or timeout to update a layout. // See more: https://twitter.com/paul_irish/status/912693347315150849/photo/1 setTimeout( () => { - expect( view.toolbar.maxWidth ).to.be.equal( '400px' ); + const editableWidthWithPadding = toPx( new Rect( viewElement ).width ); + expect( view.toolbar.maxWidth ).to.be.equal( editableWidthWithPadding ); + + document.body.removeChild( viewElement ); done(); }, 500 ); From c76c812994be9dcb0f2390acf5b34c0312615f06 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Mon, 3 Feb 2020 13:22:41 +0100 Subject: [PATCH 17/24] Tests: Improved test description. --- tests/inlineeditor.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/inlineeditor.js b/tests/inlineeditor.js index edae0c0..b2bd6de 100644 --- a/tests/inlineeditor.js +++ b/tests/inlineeditor.js @@ -183,8 +183,9 @@ describe( 'InlineEditor', () => { } ); } ); - it( 'inline toolbar should not group items when shouldNotGroupWhenFull option is enabled', () => { + it( 'should pass the config.toolbar.shouldNotGroupWhenFull configuration to the view', () => { const editorElement = document.createElement( 'div' ); + return InlineEditor.create( editorElement, { toolbar: { shouldNotGroupWhenFull: true From 8fd5ef94b3a69884c43e7a1803ea27d13c2f1505 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Mon, 3 Feb 2020 14:49:39 +0100 Subject: [PATCH 18/24] Docs. --- src/inlineeditorui.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/inlineeditorui.js b/src/inlineeditorui.js index 998141a..6945e0a 100644 --- a/src/inlineeditorui.js +++ b/src/inlineeditorui.js @@ -156,7 +156,7 @@ export default class InlineEditorUI extends EditorUI { toolbar } ); - // Set toolbar's max-width on the initialization and update it on the editable resize, + // Set the toolbar's max-width on the initialization and update it on the editable resize, // if 'shouldToolbarGroupWhenFull' in config is set to 'true'. if ( !this._toolbarConfig.shouldNotGroupWhenFull ) { const widthObserver = getResizeObserver( () => { From 1893b1cb9a920df9e20a0054d3e0cab0b1e99fc4 Mon Sep 17 00:00:00 2001 From: panr Date: Tue, 4 Feb 2020 15:27:02 +0100 Subject: [PATCH 19/24] WIP: Move setting toolbar max-width to inlineeditoruiview --- src/inlineeditorui.js | 22 ---------------------- src/inlineeditoruiview.js | 26 ++++++++++++++++++++++++++ tests/inlineeditorui.js | 27 +-------------------------- tests/inlineeditoruiview.js | 31 +++++++++++++++++++++++++++++++ 4 files changed, 58 insertions(+), 48 deletions(-) diff --git a/src/inlineeditorui.js b/src/inlineeditorui.js index 6945e0a..cac847a 100644 --- a/src/inlineeditorui.js +++ b/src/inlineeditorui.js @@ -11,11 +11,6 @@ import EditorUI from '@ckeditor/ckeditor5-core/src/editor/editorui'; import enableToolbarKeyboardFocus from '@ckeditor/ckeditor5-ui/src/toolbar/enabletoolbarkeyboardfocus'; import normalizeToolbarConfig from '@ckeditor/ckeditor5-ui/src/toolbar/normalizetoolbarconfig'; import { enablePlaceholder } from '@ckeditor/ckeditor5-engine/src/view/placeholder'; -import Rect from '@ckeditor/ckeditor5-utils/src/dom/rect'; -import getResizeObserver from '@ckeditor/ckeditor5-utils/src/dom/getresizeobserver'; -import toUnit from '@ckeditor/ckeditor5-utils/src/dom/tounit'; - -const toPx = toUnit( 'px' ); /** * The inline editor UI class. @@ -155,23 +150,6 @@ export default class InlineEditorUI extends EditorUI { originKeystrokeHandler: editor.keystrokes, toolbar } ); - - // Set the toolbar's max-width on the initialization and update it on the editable resize, - // if 'shouldToolbarGroupWhenFull' in config is set to 'true'. - if ( !this._toolbarConfig.shouldNotGroupWhenFull ) { - const widthObserver = getResizeObserver( () => { - // We need to check if there's already the editable element in the DOM. - // Otherwise the `Rect` instance will complain that source (editableElement) is not available - // to obtain the element's geometry. - if ( !editableElement.ownerDocument.body.contains( editableElement ) ) { - return; - } - - this.view.toolbar.maxWidth = toPx( new Rect( editableElement ).width ); - } ); - - widthObserver.observe( editableElement ); - } } /** diff --git a/src/inlineeditoruiview.js b/src/inlineeditoruiview.js index 5fbfa75..b45b068 100644 --- a/src/inlineeditoruiview.js +++ b/src/inlineeditoruiview.js @@ -11,6 +11,11 @@ import EditorUIView from '@ckeditor/ckeditor5-ui/src/editorui/editoruiview'; import InlineEditableUIView from '@ckeditor/ckeditor5-ui/src/editableui/inline/inlineeditableuiview'; import BalloonPanelView from '@ckeditor/ckeditor5-ui/src/panel/balloon/balloonpanelview'; import ToolbarView from '@ckeditor/ckeditor5-ui/src/toolbar/toolbarview'; +import Rect from '@ckeditor/ckeditor5-utils/src/dom/rect'; +import getResizeObserver from '@ckeditor/ckeditor5-utils/src/dom/getresizeobserver'; +import toUnit from '@ckeditor/ckeditor5-utils/src/dom/tounit'; + +const toPx = toUnit( 'px' ); /** * Inline editor UI view. Uses an nline editable and a floating toolbar. @@ -25,6 +30,7 @@ export default class InlineEditorUIView extends EditorUIView { * @param {module:engine/view/view~View} editingView The editing view instance this view is related to. * @param {HTMLElement} [editableElement] The editable element. If not specified, it will be automatically created by * {@link module:ui/editableui/editableuiview~EditableUIView}. Otherwise, the given element will be used. + * @param {Object} [options={}] Configuration options for the view instance. */ constructor( locale, editingView, editableElement, options = {} ) { super( locale ); @@ -146,6 +152,26 @@ export default class InlineEditorUIView extends EditorUIView { this.body.add( this.panel ); this.registerChild( this.editable ); this.panel.content.add( this.toolbar ); + + const options = this.toolbar.options; + + // Set toolbar's max-width on the initialization and update it on the editable resize, + // if 'shouldToolbarGroupWhenFull' in config is set to 'true'. + if ( options.shouldGroupWhenFull ) { + const editableElement = this.editable.element; + const widthObserver = getResizeObserver( () => { + // We need to check if there's already the editable element in the DOM. + // Otherwise the `Rect` instance will complain that source (editableElement) is not available + // to obtain the element's geometry. + if ( !editableElement.ownerDocument.body.contains( editableElement ) ) { + return; + } + + this.toolbar.maxWidth = toPx( new Rect( editableElement ).width ); + } ); + + widthObserver.observe( editableElement ); + } } /** diff --git a/tests/inlineeditorui.js b/tests/inlineeditorui.js index 3995d68..85269d6 100644 --- a/tests/inlineeditorui.js +++ b/tests/inlineeditorui.js @@ -3,7 +3,7 @@ * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license */ -/* globals document, Event, setTimeout */ +/* globals document, Event */ import View from '@ckeditor/ckeditor5-ui/src/view'; @@ -13,14 +13,10 @@ import InlineEditorUIView from '../src/inlineeditoruiview'; import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph'; import VirtualTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/virtualtesteditor'; -import Rect from '@ckeditor/ckeditor5-utils/src/dom/rect'; import { keyCodes } from '@ckeditor/ckeditor5-utils/src/keyboard'; import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils'; import { assertBinding } from '@ckeditor/ckeditor5-utils/tests/_utils/utils'; import { isElement } from 'lodash-es'; -import toUnit from '@ckeditor/ckeditor5-utils/src/dom/tounit'; - -const toPx = toUnit( 'px' ); describe( 'InlineEditorUI', () => { let editor, view, ui, viewElement; @@ -108,27 +104,6 @@ describe( 'InlineEditorUI', () => { editor.ui.fire( 'update' ); sinon.assert.notCalled( spy ); } ); - - // Sometimes this test can fail, due to the fact that it have to wait for async ResizeObserver execution. - it( 'should set inline toolbar max-width to the width of the editable element', done => { - document.body.appendChild( viewElement ); - - expect( document.body.contains( viewElement ) ).to.be.true; - viewElement.style.width = '400px'; - - // Unfortunately we have to wait for async ResizeObserver execution. - // ResizeObserver which has been called after changing width of viewElement, - // needs 2x requestAnimationFrame or timeout to update a layout. - // See more: https://twitter.com/paul_irish/status/912693347315150849/photo/1 - setTimeout( () => { - const editableWidthWithPadding = toPx( new Rect( viewElement ).width ); - expect( view.toolbar.maxWidth ).to.be.equal( editableWidthWithPadding ); - - document.body.removeChild( viewElement ); - - done(); - }, 500 ); - } ); } ); describe( 'editable', () => { diff --git a/tests/inlineeditoruiview.js b/tests/inlineeditoruiview.js index a18ae70..b21ebb5 100644 --- a/tests/inlineeditoruiview.js +++ b/tests/inlineeditoruiview.js @@ -10,9 +10,16 @@ import BalloonPanelView from '@ckeditor/ckeditor5-ui/src/panel/balloon/balloonpa import InlineEditableUIView from '@ckeditor/ckeditor5-ui/src/editableui/inline/inlineeditableuiview'; import Locale from '@ckeditor/ckeditor5-utils/src/locale'; import createRoot from '@ckeditor/ckeditor5-engine/tests/view/_utils/createroot.js'; +import Rect from '@ckeditor/ckeditor5-utils/src/dom/rect'; +import toUnit from '@ckeditor/ckeditor5-utils/src/dom/tounit'; +import global from '@ckeditor/ckeditor5-utils/src/dom/global'; + +const toPx = toUnit( 'px' ); import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils'; +/* globals setTimeout */ + describe( 'InlineEditorUIView', () => { let locale, view, editingView, editingViewRoot; @@ -91,6 +98,30 @@ describe( 'InlineEditorUIView', () => { it( 'sets the default value of the #viewportTopOffset attribute', () => { expect( view.viewportTopOffset ).to.equal( 0 ); } ); + + // Sometimes this test can fail, due to the fact that it have to wait for async ResizeObserver execution. + it( 'max-width should be set to the width of the editable element', done => { + const viewElement = view.editable.element; + global.document.body.appendChild( viewElement ); + + expect( global.document.body.contains( viewElement ) ).to.be.true; + + viewElement.style.width = '400px'; + + // Unfortunately we have to wait for async ResizeObserver execution. + // ResizeObserver which has been called after changing width of viewElement, + // needs 2x requestAnimationFrame or timeout to update a layout. + // See more: https://twitter.com/paul_irish/status/912693347315150849/photo/1 + setTimeout( () => { + const editableWidthWithPadding = toPx( new Rect( viewElement ).width ); + + expect( view.toolbar.maxWidth ).to.be.equal( editableWidthWithPadding ); + + global.document.body.removeChild( viewElement ); + + done(); + }, 500 ); + } ); } ); describe( '#panel', () => { From 466b92eb28135c399b22542e7164bff8540ee852 Mon Sep 17 00:00:00 2001 From: panr Date: Tue, 4 Feb 2020 21:58:48 +0100 Subject: [PATCH 20/24] Fix render() tests --- tests/inlineeditoruiview.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/inlineeditoruiview.js b/tests/inlineeditoruiview.js index b21ebb5..d03a957 100644 --- a/tests/inlineeditoruiview.js +++ b/tests/inlineeditoruiview.js @@ -83,6 +83,9 @@ describe( 'InlineEditorUIView', () => { describe( 'render()', () => { beforeEach( () => { + // We need to set this option explicitly before render phase, + // otherwise it is `undefined` and toolbar items grouping will be skipped in tests. + view.toolbar.options.shouldGroupWhenFull = true; view.render(); } ); @@ -102,8 +105,8 @@ describe( 'InlineEditorUIView', () => { // Sometimes this test can fail, due to the fact that it have to wait for async ResizeObserver execution. it( 'max-width should be set to the width of the editable element', done => { const viewElement = view.editable.element; - global.document.body.appendChild( viewElement ); + global.document.body.appendChild( viewElement ); expect( global.document.body.contains( viewElement ) ).to.be.true; viewElement.style.width = '400px'; @@ -114,7 +117,6 @@ describe( 'InlineEditorUIView', () => { // See more: https://twitter.com/paul_irish/status/912693347315150849/photo/1 setTimeout( () => { const editableWidthWithPadding = toPx( new Rect( viewElement ).width ); - expect( view.toolbar.maxWidth ).to.be.equal( editableWidthWithPadding ); global.document.body.removeChild( viewElement ); From 9c2ae9c47d974bd61f06d47ede76baae6ae09335 Mon Sep 17 00:00:00 2001 From: panr Date: Tue, 25 Feb 2020 10:36:34 +0100 Subject: [PATCH 21/24] Fix reference to the new ResizeObserver class --- src/inlineeditoruiview.js | 26 +++++++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/src/inlineeditoruiview.js b/src/inlineeditoruiview.js index b45b068..d7fcb69 100644 --- a/src/inlineeditoruiview.js +++ b/src/inlineeditoruiview.js @@ -12,7 +12,7 @@ import InlineEditableUIView from '@ckeditor/ckeditor5-ui/src/editableui/inline/i import BalloonPanelView from '@ckeditor/ckeditor5-ui/src/panel/balloon/balloonpanelview'; import ToolbarView from '@ckeditor/ckeditor5-ui/src/toolbar/toolbarview'; import Rect from '@ckeditor/ckeditor5-utils/src/dom/rect'; -import getResizeObserver from '@ckeditor/ckeditor5-utils/src/dom/getresizeobserver'; +import ResizeObserver from '@ckeditor/ckeditor5-utils/src/dom/resizeobserver'; import toUnit from '@ckeditor/ckeditor5-utils/src/dom/tounit'; const toPx = toUnit( 'px' ); @@ -141,6 +141,17 @@ export default class InlineEditorUIView extends EditorUIView { * @member {module:ui/editableui/inline/inlineeditableuiview~InlineEditableUIView} */ this.editable = new InlineEditableUIView( locale, editingView, editableElement ); + + /** + * An instance of the resize observer that helps dynamically determine the geometry of the toolbar + * and manage items that do not fit into a single row. + * + * **Note:** Created in {@link #render}. + * + * @readonly + * @member {module:utils/dom/getresizeobserver~ResizeObserver} + */ + this.resizeObserver = null; } /** @@ -159,7 +170,7 @@ export default class InlineEditorUIView extends EditorUIView { // if 'shouldToolbarGroupWhenFull' in config is set to 'true'. if ( options.shouldGroupWhenFull ) { const editableElement = this.editable.element; - const widthObserver = getResizeObserver( () => { + this.resizeObserver = new ResizeObserver( editableElement, () => { // We need to check if there's already the editable element in the DOM. // Otherwise the `Rect` instance will complain that source (editableElement) is not available // to obtain the element's geometry. @@ -169,8 +180,17 @@ export default class InlineEditorUIView extends EditorUIView { this.toolbar.maxWidth = toPx( new Rect( editableElement ).width ); } ); + } + } + + /** + * @inheritDoc + */ + destroy() { + super.destroy(); - widthObserver.observe( editableElement ); + if ( this.resizeObserver ) { + this.resizeObserver.destroy(); } } From eaa5e314b27b330c05fb4729b92099207244ab64 Mon Sep 17 00:00:00 2001 From: panr Date: Wed, 26 Feb 2020 12:59:15 +0100 Subject: [PATCH 22/24] Update InlineEditorUIView docs & make resizeObserver a private field --- src/inlineeditoruiview.js | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/src/inlineeditoruiview.js b/src/inlineeditoruiview.js index d7fcb69..1386da2 100644 --- a/src/inlineeditoruiview.js +++ b/src/inlineeditoruiview.js @@ -31,6 +31,9 @@ export default class InlineEditorUIView extends EditorUIView { * @param {HTMLElement} [editableElement] The editable element. If not specified, it will be automatically created by * {@link module:ui/editableui/editableuiview~EditableUIView}. Otherwise, the given element will be used. * @param {Object} [options={}] Configuration options for the view instance. + * @param {Boolean} [options.shouldToolbarGroupWhenFull] When set `true` enables automatic items grouping + * in the main {@link module:editor-inline/inlineeditoruiview~InlineEditorUIView#toolbar toolbar}. + * See {@link module:ui/toolbar/toolbarview~ToolbarOptions#shouldGroupWhenFull} to learn more. */ constructor( locale, editingView, editableElement, options = {} ) { super( locale ); @@ -148,10 +151,10 @@ export default class InlineEditorUIView extends EditorUIView { * * **Note:** Created in {@link #render}. * - * @readonly - * @member {module:utils/dom/getresizeobserver~ResizeObserver} + * @private + * @member {module:utils/dom/resizeobserver~ResizeObserver} */ - this.resizeObserver = null; + this._resizeObserver = null; } /** @@ -170,14 +173,8 @@ export default class InlineEditorUIView extends EditorUIView { // if 'shouldToolbarGroupWhenFull' in config is set to 'true'. if ( options.shouldGroupWhenFull ) { const editableElement = this.editable.element; - this.resizeObserver = new ResizeObserver( editableElement, () => { - // We need to check if there's already the editable element in the DOM. - // Otherwise the `Rect` instance will complain that source (editableElement) is not available - // to obtain the element's geometry. - if ( !editableElement.ownerDocument.body.contains( editableElement ) ) { - return; - } + this._resizeObserver = new ResizeObserver( editableElement, () => { this.toolbar.maxWidth = toPx( new Rect( editableElement ).width ); } ); } @@ -189,8 +186,8 @@ export default class InlineEditorUIView extends EditorUIView { destroy() { super.destroy(); - if ( this.resizeObserver ) { - this.resizeObserver.destroy(); + if ( this._resizeObserver ) { + this._resizeObserver.destroy(); } } From d672f3fc063e208bd080c9dbcced0e333398d67e Mon Sep 17 00:00:00 2001 From: panr Date: Wed, 26 Feb 2020 13:00:30 +0100 Subject: [PATCH 23/24] Change toolbar max-width test by removing timeout --- tests/inlineeditoruiview.js | 42 ++++++++++++++++++++++++------------- 1 file changed, 28 insertions(+), 14 deletions(-) diff --git a/tests/inlineeditoruiview.js b/tests/inlineeditoruiview.js index d03a957..801eabd 100644 --- a/tests/inlineeditoruiview.js +++ b/tests/inlineeditoruiview.js @@ -13,15 +13,17 @@ import createRoot from '@ckeditor/ckeditor5-engine/tests/view/_utils/createroot. import Rect from '@ckeditor/ckeditor5-utils/src/dom/rect'; import toUnit from '@ckeditor/ckeditor5-utils/src/dom/tounit'; import global from '@ckeditor/ckeditor5-utils/src/dom/global'; +import ResizeObserver from '@ckeditor/ckeditor5-utils/src/dom/resizeobserver'; const toPx = toUnit( 'px' ); import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils'; -/* globals setTimeout */ +/* globals */ describe( 'InlineEditorUIView', () => { let locale, view, editingView, editingViewRoot; + let resizeCallback; testUtils.createSinonSandbox(); @@ -31,6 +33,20 @@ describe( 'InlineEditorUIView', () => { editingViewRoot = createRoot( editingView.document ); view = new InlineEditorUIView( locale, editingView ); view.editable.name = editingViewRoot.rootName; + + // Make sure other tests of the editor do not affect tests that follow. + // Without it, if an instance of ResizeObserver already exists somewhere undestroyed + // in DOM, the following DOM mock will have no effect. + ResizeObserver._observerInstance = null; + + testUtils.sinon.stub( global.window, 'ResizeObserver' ).callsFake( callback => { + resizeCallback = callback; + + return { + observe: sinon.spy(), + unobserve: sinon.spy() + }; + } ); } ); describe( 'constructor()', () => { @@ -102,27 +118,25 @@ describe( 'InlineEditorUIView', () => { expect( view.viewportTopOffset ).to.equal( 0 ); } ); - // Sometimes this test can fail, due to the fact that it have to wait for async ResizeObserver execution. - it( 'max-width should be set to the width of the editable element', done => { + it( 'max-width should be set to the width of the editable element', () => { const viewElement = view.editable.element; + // View element should be inside the body, otherwise the `Rect` instance will complain + // that it's not available in the DOM. global.document.body.appendChild( viewElement ); - expect( global.document.body.contains( viewElement ) ).to.be.true; viewElement.style.width = '400px'; - // Unfortunately we have to wait for async ResizeObserver execution. - // ResizeObserver which has been called after changing width of viewElement, - // needs 2x requestAnimationFrame or timeout to update a layout. - // See more: https://twitter.com/paul_irish/status/912693347315150849/photo/1 - setTimeout( () => { - const editableWidthWithPadding = toPx( new Rect( viewElement ).width ); - expect( view.toolbar.maxWidth ).to.be.equal( editableWidthWithPadding ); + resizeCallback( [ { + target: viewElement, + contentRect: new Rect( viewElement ) + } ] ); + + const editableWidthWithPadding = toPx( new Rect( viewElement ).width ); - global.document.body.removeChild( viewElement ); + expect( view.toolbar.maxWidth ).to.be.equal( editableWidthWithPadding ); - done(); - }, 500 ); + viewElement.remove(); } ); } ); From e6f4e314d2d6a702767484e120cc8deef3226a7b Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Wed, 26 Feb 2020 13:36:27 +0100 Subject: [PATCH 24/24] Tests: Refactoring in InlineEditorUIView tests. --- tests/inlineeditoruiview.js | 70 +++++++++++++++++++++++++------------ 1 file changed, 48 insertions(+), 22 deletions(-) diff --git a/tests/inlineeditoruiview.js b/tests/inlineeditoruiview.js index 801eabd..f80e21e 100644 --- a/tests/inlineeditoruiview.js +++ b/tests/inlineeditoruiview.js @@ -19,8 +19,6 @@ const toPx = toUnit( 'px' ); import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils'; -/* globals */ - describe( 'InlineEditorUIView', () => { let locale, view, editingView, editingViewRoot; let resizeCallback; @@ -49,6 +47,10 @@ describe( 'InlineEditorUIView', () => { } ); } ); + afterEach( () => { + view.destroy(); + } ); + describe( 'constructor()', () => { describe( '#toolbar', () => { it( 'is created', () => { @@ -62,6 +64,16 @@ describe( 'InlineEditorUIView', () => { it( 'is not rendered', () => { expect( view.toolbar.isRendered ).to.be.false; } ); + + it( 'should have the shouldGroupWhenFull option set based on constructor options', () => { + const view = new InlineEditorUIView( locale, editingView, null, { + shouldToolbarGroupWhenFull: true + } ); + + expect( view.toolbar.options.shouldGroupWhenFull ).to.be.true; + + view.destroy(); + } ); } ); describe( '#panel', () => { @@ -99,16 +111,9 @@ describe( 'InlineEditorUIView', () => { describe( 'render()', () => { beforeEach( () => { - // We need to set this option explicitly before render phase, - // otherwise it is `undefined` and toolbar items grouping will be skipped in tests. - view.toolbar.options.shouldGroupWhenFull = true; view.render(); } ); - afterEach( () => { - view.destroy(); - } ); - describe( '#toolbar', () => { it( 'is given the right CSS classes', () => { expect( view.toolbar.element.classList.contains( 'ck-toolbar_floating' ) ).to.be.true; @@ -118,25 +123,46 @@ describe( 'InlineEditorUIView', () => { expect( view.viewportTopOffset ).to.equal( 0 ); } ); - it( 'max-width should be set to the width of the editable element', () => { - const viewElement = view.editable.element; + describe( 'automatic resizing when shouldToolbarGroupWhenFull is "true"', () => { + it( 'should set and update toolbar max-width according to the width of the editable element', () => { + const locale = new Locale(); + const editingView = new EditingView(); + const editingViewRoot = createRoot( editingView.document ); + const view = new InlineEditorUIView( locale, editingView, null, { + shouldToolbarGroupWhenFull: true + } ); + view.editable.name = editingViewRoot.rootName; + view.render(); - // View element should be inside the body, otherwise the `Rect` instance will complain - // that it's not available in the DOM. - global.document.body.appendChild( viewElement ); + const editableElement = view.editable.element; - viewElement.style.width = '400px'; + // View element should be inside the body, otherwise the `Rect` instance will complain + // that it's not available in the DOM. + global.document.body.appendChild( editableElement ); - resizeCallback( [ { - target: viewElement, - contentRect: new Rect( viewElement ) - } ] ); + editableElement.style.width = '400px'; - const editableWidthWithPadding = toPx( new Rect( viewElement ).width ); + resizeCallback( [ { + target: editableElement, + contentRect: new Rect( editableElement ) + } ] ); - expect( view.toolbar.maxWidth ).to.be.equal( editableWidthWithPadding ); + // Include paddings. + expect( view.toolbar.maxWidth ).to.be.equal( toPx( new Rect( editableElement ).width ) ); - viewElement.remove(); + editableElement.style.width = '200px'; + + resizeCallback( [ { + target: editableElement, + contentRect: new Rect( editableElement ) + } ] ); + + // Include paddings. + expect( view.toolbar.maxWidth ).to.be.equal( toPx( new Rect( editableElement ).width ) ); + + editableElement.remove(); + view.destroy(); + } ); } ); } );