From 2d9f309bb9c08578a0d7e3b61263c4193f819589 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Mon, 20 Mar 2017 19:31:05 +0100 Subject: [PATCH 01/19] Added `stickTo` method to the BalloonPanelView. --- src/panel/balloon/balloonpanelview.js | 20 ++++++++++ tests/panel/balloon/balloonpanelview.js | 51 ++++++++++++++++++++++++- 2 files changed, 70 insertions(+), 1 deletion(-) diff --git a/src/panel/balloon/balloonpanelview.js b/src/panel/balloon/balloonpanelview.js index 948bf301..2b09e593 100644 --- a/src/panel/balloon/balloonpanelview.js +++ b/src/panel/balloon/balloonpanelview.js @@ -159,6 +159,26 @@ export default class BalloonPanelView extends View { Object.assign( this, { top, left, position } ); } + + /** + * Works exactly the same as {module:ui/panel/balloon/balloonpanelview~BalloonPanelView.attachTo} with one exception. + * Position of attached panel is constantly updated when any of element in document is scrolled. Thanks to this + * panel always sticks to the target element. See #170. + * + * @param {module:utils/dom/position~Options} options Positioning options compatible with + * {@link module:utils/dom/position~getOptimalPosition}. Default `positions` array is + * {@link module:ui/panel/balloon/balloonpanelview~BalloonPanelView.defaultPositions}. + */ + stickTo( options ) { + // First we need to attach the balloon panel to target element. + this.attachTo( options ); + + // Then we need to listen to scroll of eny element in the document and update position of the balloon panel. + this.listenTo( document, 'scroll', () => this.attachTo( options ), { useCapture: true } ); + + // After all we need to clean up the listener. + this.once( 'change:isVisible', () => this.stopListening( document, 'scroll' ) ); + } } /** diff --git a/tests/panel/balloon/balloonpanelview.js b/tests/panel/balloon/balloonpanelview.js index 3922c7b8..d7c90e50 100644 --- a/tests/panel/balloon/balloonpanelview.js +++ b/tests/panel/balloon/balloonpanelview.js @@ -3,7 +3,7 @@ * For licensing, see LICENSE.md. */ -/* global window, document */ +/* global window, document, Event */ import global from '@ckeditor/ckeditor5-utils/src/dom/global'; import ViewCollection from '../../../src/viewcollection'; @@ -409,6 +409,55 @@ describe( 'BalloonPanelView', () => { } ); } ); } ); + + describe( 'stickTo()', () => { + let attachToSpy, target, limiter; + + beforeEach( () => { + attachToSpy = testUtils.sinon.spy( view, 'attachTo' ); + limiter = document.createElement( 'div' ); + target = document.createElement( 'div' ); + + document.body.appendChild( limiter ); + document.body.appendChild( target ); + } ); + + afterEach( () => { + attachToSpy.restore(); + limiter.remove(); + target.remove(); + } ); + + it( 'should attach balloon to the target constantly when any of element in document is scrolled', () => { + view.stickTo( { target, limiter } ); + + expect( attachToSpy.calledOnce ).to.true; + expect( attachToSpy.lastCall.args[ 0 ] ).to.deep.equal( { target, limiter } ); + + document.dispatchEvent( new Event( 'scroll' ) ); + + expect( attachToSpy.calledTwice ).to.true; + expect( attachToSpy.lastCall.args[ 0 ] ).to.deep.equal( { target, limiter } ); + + limiter.dispatchEvent( new Event( 'scroll' ) ); + + expect( attachToSpy.calledThrice ).to.true; + expect( attachToSpy.lastCall.args[ 0 ] ).to.deep.equal( { target, limiter } ); + } ); + + it( 'should stop attach when balloon is hidden', () => { + view.stickTo( { target, limiter } ); + + expect( attachToSpy.calledOnce ).to.true; + + view.hide(); + + document.dispatchEvent( new Event( 'scroll' ) ); + + // Still once. + expect( attachToSpy.calledOnce ).to.true; + } ); + } ); } ); function mockBoundingBox( element, data ) { From 3a6b7414bcb05173865bc3a55d75cfbd669fc209 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Fri, 31 Mar 2017 10:34:50 +0200 Subject: [PATCH 02/19] Limited balloon panel updaing on scroll. --- src/panel/balloon/balloonpanelview.js | 27 +++++++++++++++++-------- tests/panel/balloon/balloonpanelview.js | 21 ++++++++++++------- 2 files changed, 33 insertions(+), 15 deletions(-) diff --git a/src/panel/balloon/balloonpanelview.js b/src/panel/balloon/balloonpanelview.js index 2b09e593..df6714af 100644 --- a/src/panel/balloon/balloonpanelview.js +++ b/src/panel/balloon/balloonpanelview.js @@ -15,6 +15,7 @@ import { getOptimalPosition } from '@ckeditor/ckeditor5-utils/src/dom/position'; import toUnit from '@ckeditor/ckeditor5-utils/src/dom/tounit'; const toPx = toUnit( 'px' ); +const defaultLimiterElement = document.body; /** * The balloon panel view class. @@ -151,7 +152,7 @@ export default class BalloonPanelView extends View { defaultPositions.ne, defaultPositions.nw ], - limiter: document.body, + limiter: defaultLimiterElement, fitInViewport: true }, options ); @@ -162,22 +163,32 @@ export default class BalloonPanelView extends View { /** * Works exactly the same as {module:ui/panel/balloon/balloonpanelview~BalloonPanelView.attachTo} with one exception. - * Position of attached panel is constantly updated when any of element in document is scrolled. Thanks to this - * panel always sticks to the target element. See #170. + * Position of attached panel is constantly updated when any of parents of the target or limiter elements are scrolled + * or when browser window is resized. Thanks to this panel always sticks to the target element. See #170. * * @param {module:utils/dom/position~Options} options Positioning options compatible with * {@link module:utils/dom/position~getOptimalPosition}. Default `positions` array is * {@link module:ui/panel/balloon/balloonpanelview~BalloonPanelView.defaultPositions}. */ - stickTo( options ) { - // First we need to attach the balloon panel to target element. + keepAttachedTo( options ) { + // First we need to attach the balloon panel to the target element. this.attachTo( options ); - // Then we need to listen to scroll of eny element in the document and update position of the balloon panel. - this.listenTo( document, 'scroll', () => this.attachTo( options ), { useCapture: true } ); + const target = options.target; + const limiter = options.limiter || defaultLimiterElement; + + // Then we need to listen on scroll event of eny element in the document. + this.listenTo( document, 'scroll', ( evt, domEvt ) => { + // And update position if scrolled element contains related to the balloon elements. + if ( domEvt.target.contains( target ) || domEvt.target.contains( limiter ) ) { + this.attachTo( options ); + } + }, { useCapture: true } ); // After all we need to clean up the listener. - this.once( 'change:isVisible', () => this.stopListening( document, 'scroll' ) ); + this.once( 'change:isVisible', () => { + this.stopListening( document, 'scroll' ); + } ); } } diff --git a/tests/panel/balloon/balloonpanelview.js b/tests/panel/balloon/balloonpanelview.js index d7c90e50..b15f5f2f 100644 --- a/tests/panel/balloon/balloonpanelview.js +++ b/tests/panel/balloon/balloonpanelview.js @@ -410,26 +410,27 @@ describe( 'BalloonPanelView', () => { } ); } ); - describe( 'stickTo()', () => { - let attachToSpy, target, limiter; + describe( 'keepAttachedTo()', () => { + let attachToSpy, target, limiter, notRelatedElement; beforeEach( () => { attachToSpy = testUtils.sinon.spy( view, 'attachTo' ); limiter = document.createElement( 'div' ); target = document.createElement( 'div' ); + notRelatedElement = document.createElement( 'div' ); document.body.appendChild( limiter ); - document.body.appendChild( target ); + document.body.appendChild( notRelatedElement ); } ); afterEach( () => { attachToSpy.restore(); limiter.remove(); - target.remove(); + notRelatedElement.remove(); } ); - it( 'should attach balloon to the target constantly when any of element in document is scrolled', () => { - view.stickTo( { target, limiter } ); + it( 'should attach balloon to the target constantly when any of element containing target element is scrolled', () => { + view.keepAttachedTo( { target, limiter } ); expect( attachToSpy.calledOnce ).to.true; expect( attachToSpy.lastCall.args[ 0 ] ).to.deep.equal( { target, limiter } ); @@ -443,10 +444,16 @@ describe( 'BalloonPanelView', () => { expect( attachToSpy.calledThrice ).to.true; expect( attachToSpy.lastCall.args[ 0 ] ).to.deep.equal( { target, limiter } ); + + notRelatedElement.dispatchEvent( new Event( 'scroll' ) ); + + // Nothing's changed. + expect( attachToSpy.calledThrice ).to.true; + expect( attachToSpy.lastCall.args[ 0 ] ).to.deep.equal( { target, limiter } ); } ); it( 'should stop attach when balloon is hidden', () => { - view.stickTo( { target, limiter } ); + view.keepAttachedTo( { target, limiter } ); expect( attachToSpy.calledOnce ).to.true; From 923316096c0051f79e6a804941240d78bd3d578c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Fri, 31 Mar 2017 10:36:11 +0200 Subject: [PATCH 03/19] Updated panel position on resize. --- src/panel/balloon/balloonpanelview.js | 6 +++++- tests/panel/balloon/balloonpanelview.js | 25 +++++++++++++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/src/panel/balloon/balloonpanelview.js b/src/panel/balloon/balloonpanelview.js index df6714af..4cdcaaec 100644 --- a/src/panel/balloon/balloonpanelview.js +++ b/src/panel/balloon/balloonpanelview.js @@ -7,7 +7,7 @@ * @module ui/panel/balloon/balloonpanelview */ -/* globals document */ +/* globals document, window */ import View from '../../view'; import Template from '../../template'; @@ -185,9 +185,13 @@ export default class BalloonPanelView extends View { } }, { useCapture: true } ); + // We need to listen on window resize event and update position. + this.listenTo( window, 'resize', () => this.attachTo( options ) ); + // After all we need to clean up the listener. this.once( 'change:isVisible', () => { this.stopListening( document, 'scroll' ); + this.stopListening( window, 'resize' ); } ); } } diff --git a/tests/panel/balloon/balloonpanelview.js b/tests/panel/balloon/balloonpanelview.js index b15f5f2f..e7eecc71 100644 --- a/tests/panel/balloon/balloonpanelview.js +++ b/tests/panel/balloon/balloonpanelview.js @@ -464,6 +464,31 @@ describe( 'BalloonPanelView', () => { // Still once. expect( attachToSpy.calledOnce ).to.true; } ); + + it( 'should attach balloon to the target constantly when browser window is resized', () => { + view.keepAttachedTo( { target, limiter } ); + + expect( attachToSpy.calledOnce ).to.true; + expect( attachToSpy.lastCall.args[ 0 ] ).to.deep.equal( { target, limiter } ); + + window.dispatchEvent( new Event( 'resize' ) ); + + expect( attachToSpy.calledTwice ).to.true; + expect( attachToSpy.lastCall.args[ 0 ] ).to.deep.equal( { target, limiter } ); + } ); + + it( 'should stop attach when balloon is hidden', () => { + view.keepAttachedTo( { target, limiter } ); + + expect( attachToSpy.calledOnce ).to.true; + + view.hide(); + + window.dispatchEvent( new Event( 'resize' ) ); + + // Still once. + expect( attachToSpy.calledOnce ).to.true; + } ); } ); } ); From a5c1dabd5f0a07d4ffcdbd6f08c1ba27579b0332 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Fri, 31 Mar 2017 13:18:58 +0200 Subject: [PATCH 04/19] Added manual test for BalloonPanelView#keepAttachedTo. --- tests/manual/panel/balloon/balloonpanel.html | 104 +++++++++++++++++++ tests/manual/panel/balloon/balloonpanel.js | 86 +++++++++++++++ tests/manual/panel/balloon/balloonpanel.md | 4 + 3 files changed, 194 insertions(+) create mode 100644 tests/manual/panel/balloon/balloonpanel.html create mode 100644 tests/manual/panel/balloon/balloonpanel.js create mode 100644 tests/manual/panel/balloon/balloonpanel.md diff --git a/tests/manual/panel/balloon/balloonpanel.html b/tests/manual/panel/balloon/balloonpanel.html new file mode 100644 index 00000000..5c09995d --- /dev/null +++ b/tests/manual/panel/balloon/balloonpanel.html @@ -0,0 +1,104 @@ +
+
+
+
+

+

+

+

+

+

+

+

+

+

+

+

+

+

+

Balloon is attached to the TARGET element.

+

+

+

+

+

+

+

+

+

+

+

+

+

+

+
+
+ +
+
+

+

+

+

+

+

+

+

+

+

+

+

+

+

+

Balloon sticks to the TARGET element.

+

+

+

+

+

+

+

+

+

+

+

+

+

+

+
+
+
+
+ + diff --git a/tests/manual/panel/balloon/balloonpanel.js b/tests/manual/panel/balloon/balloonpanel.js new file mode 100644 index 00000000..12de5230 --- /dev/null +++ b/tests/manual/panel/balloon/balloonpanel.js @@ -0,0 +1,86 @@ +/** + * @license Copyright (c) 2003-2017, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md. + */ + +/* globals window, document, console:false */ + +import ClassicEditor from '@ckeditor/ckeditor5-editor-classic/src/classic'; +import ArticlePresets from '@ckeditor/ckeditor5-presets/src/article'; +import BalloonPanelView from '@ckeditor/ckeditor5-ui/src/panel/balloon/balloonpanelview'; +import View from '@ckeditor/ckeditor5-ui/src/view'; +import Template from '@ckeditor/ckeditor5-ui/src/template'; + +// Content of the balloon panel. +class BalloonContentView extends View { + constructor() { + super(); + + this.template = new Template( { + tag: 'div', + attributes: { + class: 'balloon-content' + }, + children: [ + { + text: 'Balloon' + } + ] + } ); + } +} + +// Set initial scroll of one of the container element. +document.querySelector( '.container-a' ).scrollTop = 450; + +// Init editor with balloon attached to the target element. +ClassicEditor.create( document.querySelector( '#editor-attach' ), { + plugins: [ ArticlePresets ], + toolbar: [ 'bold', 'italic', 'undo', 'redo' ] +} ) +.then( editor => { + const panel = new BalloonPanelView(); + + panel.content.add( new BalloonContentView() ); + editor.ui.view.body.add( panel ); + + editor.ui.view.element.querySelector( '.ck-editor__editable' ).scrollTop = 360; + + panel.init().then( () => { + panel.attachTo( { + target: editor.ui.view.element.querySelector( '.ck-editor__editable p strong' ), + limiter: editor.ui.view.editableElement + } ); + } ); + + window.attachEditor = editor; +} ) +.catch( err => { + console.error( err.stack ); +} ); + +// Init editor with balloon sticked to the target element. +ClassicEditor.create( document.querySelector( '#editor-stick' ), { + plugins: [ ArticlePresets ], + toolbar: [ 'bold', 'italic', 'undo', 'redo' ] +} ) +.then( editor => { + const panel = new BalloonPanelView(); + + panel.content.add( new BalloonContentView() ); + editor.ui.view.body.add( panel ); + + editor.ui.view.element.querySelector( '.ck-editor__editable' ).scrollTop = 360; + + panel.init().then( () => { + panel.keepAttachedTo( { + target: editor.ui.view.element.querySelector( '.ck-editor__editable p strong' ), + limiter: editor.ui.view.editableElement + } ); + } ); + + window.stickEditor = editor; +} ) +.catch( err => { + console.error( err.stack ); +} ); diff --git a/tests/manual/panel/balloon/balloonpanel.md b/tests/manual/panel/balloon/balloonpanel.md new file mode 100644 index 00000000..904f1286 --- /dev/null +++ b/tests/manual/panel/balloon/balloonpanel.md @@ -0,0 +1,4 @@ +## BalloonPanelView `attachTo` vs `keepAttachedTo` + +Scroll editable elements and container (horizontally as well). Balloon in the left editor should float but balloon in the +right editor should stick to the target element. From 4d1443fa444a27f17e46ba8991f109c0c045ec80 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Fri, 31 Mar 2017 13:25:19 +0200 Subject: [PATCH 05/19] Used global namespace for global dom object. --- src/panel/balloon/balloonpanelview.js | 13 ++++++------ tests/panel/balloon/balloonpanelview.js | 27 ++++++++++--------------- 2 files changed, 17 insertions(+), 23 deletions(-) diff --git a/src/panel/balloon/balloonpanelview.js b/src/panel/balloon/balloonpanelview.js index 4cdcaaec..1dda04f9 100644 --- a/src/panel/balloon/balloonpanelview.js +++ b/src/panel/balloon/balloonpanelview.js @@ -7,15 +7,14 @@ * @module ui/panel/balloon/balloonpanelview */ -/* globals document, window */ - import View from '../../view'; import Template from '../../template'; import { getOptimalPosition } from '@ckeditor/ckeditor5-utils/src/dom/position'; import toUnit from '@ckeditor/ckeditor5-utils/src/dom/tounit'; +import global from '@ckeditor/ckeditor5-utils/src/dom/global'; const toPx = toUnit( 'px' ); -const defaultLimiterElement = document.body; +const defaultLimiterElement = global.document.body; /** * The balloon panel view class. @@ -178,7 +177,7 @@ export default class BalloonPanelView extends View { const limiter = options.limiter || defaultLimiterElement; // Then we need to listen on scroll event of eny element in the document. - this.listenTo( document, 'scroll', ( evt, domEvt ) => { + this.listenTo( global.document, 'scroll', ( evt, domEvt ) => { // And update position if scrolled element contains related to the balloon elements. if ( domEvt.target.contains( target ) || domEvt.target.contains( limiter ) ) { this.attachTo( options ); @@ -186,12 +185,12 @@ export default class BalloonPanelView extends View { }, { useCapture: true } ); // We need to listen on window resize event and update position. - this.listenTo( window, 'resize', () => this.attachTo( options ) ); + this.listenTo( global.window, 'resize', () => this.attachTo( options ) ); // After all we need to clean up the listener. this.once( 'change:isVisible', () => { - this.stopListening( document, 'scroll' ); - this.stopListening( window, 'resize' ); + this.stopListening( global.document, 'scroll' ); + this.stopListening( global.window, 'resize' ); } ); } } diff --git a/tests/panel/balloon/balloonpanelview.js b/tests/panel/balloon/balloonpanelview.js index e7eecc71..a053dac9 100644 --- a/tests/panel/balloon/balloonpanelview.js +++ b/tests/panel/balloon/balloonpanelview.js @@ -22,18 +22,6 @@ describe( 'BalloonPanelView', () => { view.set( 'maxWidth', 200 ); - windowStub = { - innerWidth: 1000, - innerHeight: 1000, - scrollX: 0, - scrollY: 0, - getComputedStyle: ( el ) => { - return window.getComputedStyle( el ); - } - }; - - testUtils.sinon.stub( global, 'window', windowStub ); - return view.init(); } ); @@ -160,11 +148,18 @@ describe( 'BalloonPanelView', () => { height: 100 } ); - // Make sure that limiter is fully visible in viewport. - Object.assign( windowStub, { + // Mock window dimensions. + windowStub = { innerWidth: 500, - innerHeight: 500 - } ); + innerHeight: 500, + scrollX: 0, + scrollY: 0, + getComputedStyle: ( el ) => { + return window.getComputedStyle( el ); + } + }; + + testUtils.sinon.stub( global, 'window', windowStub ); } ); it( 'should use default options', () => { From e078d4813faf958008acd765847711545212dc9e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Fri, 31 Mar 2017 13:41:05 +0200 Subject: [PATCH 06/19] Moved manual test to tickets. --- .../{panel/balloon/balloonpanel.html => tickets/170/1.html} | 0 tests/manual/{panel/balloon/balloonpanel.js => tickets/170/1.js} | 0 tests/manual/{panel/balloon/balloonpanel.md => tickets/170/1.md} | 0 3 files changed, 0 insertions(+), 0 deletions(-) rename tests/manual/{panel/balloon/balloonpanel.html => tickets/170/1.html} (100%) rename tests/manual/{panel/balloon/balloonpanel.js => tickets/170/1.js} (100%) rename tests/manual/{panel/balloon/balloonpanel.md => tickets/170/1.md} (100%) diff --git a/tests/manual/panel/balloon/balloonpanel.html b/tests/manual/tickets/170/1.html similarity index 100% rename from tests/manual/panel/balloon/balloonpanel.html rename to tests/manual/tickets/170/1.html diff --git a/tests/manual/panel/balloon/balloonpanel.js b/tests/manual/tickets/170/1.js similarity index 100% rename from tests/manual/panel/balloon/balloonpanel.js rename to tests/manual/tickets/170/1.js diff --git a/tests/manual/panel/balloon/balloonpanel.md b/tests/manual/tickets/170/1.md similarity index 100% rename from tests/manual/panel/balloon/balloonpanel.md rename to tests/manual/tickets/170/1.md From a3f093463f841a09f895a8e9eb81f91f13d53972 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Mon, 3 Apr 2017 16:26:04 +0200 Subject: [PATCH 07/19] Added full url to the ticket in docs. --- src/panel/balloon/balloonpanelview.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/panel/balloon/balloonpanelview.js b/src/panel/balloon/balloonpanelview.js index 1dda04f9..3404a5e2 100644 --- a/src/panel/balloon/balloonpanelview.js +++ b/src/panel/balloon/balloonpanelview.js @@ -163,7 +163,8 @@ export default class BalloonPanelView extends View { /** * Works exactly the same as {module:ui/panel/balloon/balloonpanelview~BalloonPanelView.attachTo} with one exception. * Position of attached panel is constantly updated when any of parents of the target or limiter elements are scrolled - * or when browser window is resized. Thanks to this panel always sticks to the target element. See #170. + * or when browser window is resized. Thanks to this panel always sticks to the target element. + * See https://github.com/ckeditor/ckeditor5-ui/issues/170. * * @param {module:utils/dom/position~Options} options Positioning options compatible with * {@link module:utils/dom/position~getOptimalPosition}. Default `positions` array is From fc6ec1aba6ee2e9e1105e0e98125e2e5d61c926a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Mon, 3 Apr 2017 16:40:53 +0200 Subject: [PATCH 08/19] Simplified manual test. --- tests/manual/tickets/170/1.html | 75 +++++---------------------------- tests/manual/tickets/170/1.js | 29 ++----------- 2 files changed, 15 insertions(+), 89 deletions(-) diff --git a/tests/manual/tickets/170/1.html b/tests/manual/tickets/170/1.html index 5c09995d..ee75c1c5 100644 --- a/tests/manual/tickets/170/1.html +++ b/tests/manual/tickets/170/1.html @@ -1,70 +1,14 @@ -
-
+
+
-

-

-

-

-

-

-

-

-

-

-

-

-

-

Balloon is attached to the TARGET element.

-

-

-

-

-

-

-

-

-

-

-

-

-

-

-

-

-

-

-

-

-

-

-

-

-

-

-

-

Balloon sticks to the TARGET element.

-

-

-

-

-

-

-

-

-

-

-

-

-

-

@@ -75,22 +19,25 @@ height: 100%; } - .balloon-content { - padding: 10px; - font-size: 16px; + .ck-balloon-panel { + padding: 2em; } .ck-editor__editable { max-height: 300px; } - .container-a { + .ck-editor__editable p { + margin: 470px 0; + } + + .container-outer { height: 500px; width: 750px; overflow: scroll; } - .container-b { + .container-inner { padding: 500px 30px 30px; display: flex; width: 1000px; @@ -98,7 +45,7 @@ background: #e1e1e1; } - .container-b > div { + .container-inner > div { margin: 0 20px; } diff --git a/tests/manual/tickets/170/1.js b/tests/manual/tickets/170/1.js index 12de5230..ccfa0362 100644 --- a/tests/manual/tickets/170/1.js +++ b/tests/manual/tickets/170/1.js @@ -8,30 +8,9 @@ import ClassicEditor from '@ckeditor/ckeditor5-editor-classic/src/classic'; import ArticlePresets from '@ckeditor/ckeditor5-presets/src/article'; import BalloonPanelView from '@ckeditor/ckeditor5-ui/src/panel/balloon/balloonpanelview'; -import View from '@ckeditor/ckeditor5-ui/src/view'; -import Template from '@ckeditor/ckeditor5-ui/src/template'; -// Content of the balloon panel. -class BalloonContentView extends View { - constructor() { - super(); - - this.template = new Template( { - tag: 'div', - attributes: { - class: 'balloon-content' - }, - children: [ - { - text: 'Balloon' - } - ] - } ); - } -} - -// Set initial scroll of one of the container element. -document.querySelector( '.container-a' ).scrollTop = 450; +// Set initial scroll for the outer container element. +document.querySelector( '.container-outer' ).scrollTop = 450; // Init editor with balloon attached to the target element. ClassicEditor.create( document.querySelector( '#editor-attach' ), { @@ -41,7 +20,7 @@ ClassicEditor.create( document.querySelector( '#editor-attach' ), { .then( editor => { const panel = new BalloonPanelView(); - panel.content.add( new BalloonContentView() ); + panel.element.innerHTML = 'Balloon content.'; editor.ui.view.body.add( panel ); editor.ui.view.element.querySelector( '.ck-editor__editable' ).scrollTop = 360; @@ -67,7 +46,7 @@ ClassicEditor.create( document.querySelector( '#editor-stick' ), { .then( editor => { const panel = new BalloonPanelView(); - panel.content.add( new BalloonContentView() ); + panel.element.innerHTML = 'Balloon content.'; editor.ui.view.body.add( panel ); editor.ui.view.element.querySelector( '.ck-editor__editable' ).scrollTop = 360; From 724a5903b51e1092a063a2cab2e03846ee352333 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Mon, 3 Apr 2017 16:44:45 +0200 Subject: [PATCH 09/19] Reduced similar test cases. --- tests/panel/balloon/balloonpanelview.js | 18 +++--------------- 1 file changed, 3 insertions(+), 15 deletions(-) diff --git a/tests/panel/balloon/balloonpanelview.js b/tests/panel/balloon/balloonpanelview.js index a053dac9..89db17cb 100644 --- a/tests/panel/balloon/balloonpanelview.js +++ b/tests/panel/balloon/balloonpanelview.js @@ -424,7 +424,7 @@ describe( 'BalloonPanelView', () => { notRelatedElement.remove(); } ); - it( 'should attach balloon to the target constantly when any of element containing target element is scrolled', () => { + it( 'should attach balloon to the target constantly when any of the related element is scrolled', () => { view.keepAttachedTo( { target, limiter } ); expect( attachToSpy.calledOnce ).to.true; @@ -447,19 +447,6 @@ describe( 'BalloonPanelView', () => { expect( attachToSpy.lastCall.args[ 0 ] ).to.deep.equal( { target, limiter } ); } ); - it( 'should stop attach when balloon is hidden', () => { - view.keepAttachedTo( { target, limiter } ); - - expect( attachToSpy.calledOnce ).to.true; - - view.hide(); - - document.dispatchEvent( new Event( 'scroll' ) ); - - // Still once. - expect( attachToSpy.calledOnce ).to.true; - } ); - it( 'should attach balloon to the target constantly when browser window is resized', () => { view.keepAttachedTo( { target, limiter } ); @@ -472,7 +459,7 @@ describe( 'BalloonPanelView', () => { expect( attachToSpy.lastCall.args[ 0 ] ).to.deep.equal( { target, limiter } ); } ); - it( 'should stop attach when balloon is hidden', () => { + it( 'should stop attaching when the balloon is hidden', () => { view.keepAttachedTo( { target, limiter } ); expect( attachToSpy.calledOnce ).to.true; @@ -480,6 +467,7 @@ describe( 'BalloonPanelView', () => { view.hide(); window.dispatchEvent( new Event( 'resize' ) ); + window.dispatchEvent( new Event( 'scroll' ) ); // Still once. expect( attachToSpy.calledOnce ).to.true; From f3840aefbc5126f019410f6d216a001db77468a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Mon, 3 Apr 2017 17:05:05 +0200 Subject: [PATCH 10/19] Cleaned view listeners in destroy method. --- src/panel/balloon/balloonpanelview.js | 12 +++++++++++- tests/panel/balloon/balloonpanelview.js | 14 ++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/src/panel/balloon/balloonpanelview.js b/src/panel/balloon/balloonpanelview.js index 3404a5e2..803ec060 100644 --- a/src/panel/balloon/balloonpanelview.js +++ b/src/panel/balloon/balloonpanelview.js @@ -188,12 +188,22 @@ export default class BalloonPanelView extends View { // We need to listen on window resize event and update position. this.listenTo( global.window, 'resize', () => this.attachTo( options ) ); - // After all we need to clean up the listener. + // After all we need to clean up the listeners. this.once( 'change:isVisible', () => { this.stopListening( global.document, 'scroll' ); this.stopListening( global.window, 'resize' ); } ); } + + /** + * @inheritDoc + */ + destroy() { + this.stopListening( global.document, 'scroll' ); + this.stopListening( global.window, 'resize' ); + + return super.destroy(); + } } /** diff --git a/tests/panel/balloon/balloonpanelview.js b/tests/panel/balloon/balloonpanelview.js index 89db17cb..7ad87ed3 100644 --- a/tests/panel/balloon/balloonpanelview.js +++ b/tests/panel/balloon/balloonpanelview.js @@ -472,6 +472,20 @@ describe( 'BalloonPanelView', () => { // Still once. expect( attachToSpy.calledOnce ).to.true; } ); + + it( 'should stop attaching when the view will be destroyed', () => { + view.keepAttachedTo( { target, limiter } ); + + expect( attachToSpy.calledOnce ).to.true; + + view.destroy(); + + window.dispatchEvent( new Event( 'resize' ) ); + window.dispatchEvent( new Event( 'scroll' ) ); + + // Still once. + expect( attachToSpy.calledOnce ).to.true; + } ); } ); } ); From 8692e48c3e5e55e83746979ecae7665c1b4e1848 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Mon, 3 Apr 2017 17:05:26 +0200 Subject: [PATCH 11/19] Added test for default limiter. --- tests/panel/balloon/balloonpanelview.js | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tests/panel/balloon/balloonpanelview.js b/tests/panel/balloon/balloonpanelview.js index 7ad87ed3..b1878d93 100644 --- a/tests/panel/balloon/balloonpanelview.js +++ b/tests/panel/balloon/balloonpanelview.js @@ -486,6 +486,16 @@ describe( 'BalloonPanelView', () => { // Still once. expect( attachToSpy.calledOnce ).to.true; } ); + + it( 'should set default limiter as document.body', () => { + view.keepAttachedTo( { target } ); + + expect( attachToSpy.calledOnce ).to.true; + + document.body.dispatchEvent( new Event( 'scroll' ) ); + + expect( attachToSpy.calledTwice ).to.true; + } ); } ); } ); From 3941c564cd9b1cb3b2cac52bef21ccee30db1db4 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Tue, 4 Apr 2017 10:25:19 +0200 Subject: [PATCH 12/19] Docs: Enhanced BalloonPanelView#keepAttachedTo() docs. --- src/panel/balloon/balloonpanelview.js | 10 +++++++--- tests/panel/balloon/balloonpanelview.js | 8 ++++---- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/src/panel/balloon/balloonpanelview.js b/src/panel/balloon/balloonpanelview.js index 803ec060..9c99c58e 100644 --- a/src/panel/balloon/balloonpanelview.js +++ b/src/panel/balloon/balloonpanelview.js @@ -161,9 +161,13 @@ export default class BalloonPanelView extends View { } /** - * Works exactly the same as {module:ui/panel/balloon/balloonpanelview~BalloonPanelView.attachTo} with one exception. - * Position of attached panel is constantly updated when any of parents of the target or limiter elements are scrolled - * or when browser window is resized. Thanks to this panel always sticks to the target element. + * Works the same way as {module:ui/panel/balloon/balloonpanelview~BalloonPanelView.attachTo} + * except that the position of the panel is continuously updated when any ancestor of the + * {@link module:utils/dom/position~Options#target} or {@link module:utils/dom/position~Options#limiter} + * is being scrolled or when the browser window is being resized. + * + * Thanks to this, the panel always sticks to the {@link module:utils/dom/position~Options#target}. + * * See https://github.com/ckeditor/ckeditor5-ui/issues/170. * * @param {module:utils/dom/position~Options} options Positioning options compatible with diff --git a/tests/panel/balloon/balloonpanelview.js b/tests/panel/balloon/balloonpanelview.js index b1878d93..9f3202dc 100644 --- a/tests/panel/balloon/balloonpanelview.js +++ b/tests/panel/balloon/balloonpanelview.js @@ -424,7 +424,7 @@ describe( 'BalloonPanelView', () => { notRelatedElement.remove(); } ); - it( 'should attach balloon to the target constantly when any of the related element is scrolled', () => { + it( 'should keep the balloon attached to the target when any of the related elements is scrolled', () => { view.keepAttachedTo( { target, limiter } ); expect( attachToSpy.calledOnce ).to.true; @@ -447,7 +447,7 @@ describe( 'BalloonPanelView', () => { expect( attachToSpy.lastCall.args[ 0 ] ).to.deep.equal( { target, limiter } ); } ); - it( 'should attach balloon to the target constantly when browser window is resized', () => { + it( 'should keep the balloon attached to the target when the browser window is being resized', () => { view.keepAttachedTo( { target, limiter } ); expect( attachToSpy.calledOnce ).to.true; @@ -473,7 +473,7 @@ describe( 'BalloonPanelView', () => { expect( attachToSpy.calledOnce ).to.true; } ); - it( 'should stop attaching when the view will be destroyed', () => { + it( 'should stop attaching once the view is destroyed', () => { view.keepAttachedTo( { target, limiter } ); expect( attachToSpy.calledOnce ).to.true; @@ -487,7 +487,7 @@ describe( 'BalloonPanelView', () => { expect( attachToSpy.calledOnce ).to.true; } ); - it( 'should set default limiter as document.body', () => { + it( 'should set document.body as the default limiter', () => { view.keepAttachedTo( { target } ); expect( attachToSpy.calledOnce ).to.true; From af93584ae7c6133986922d76ea5b670662fae77a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Tue, 4 Apr 2017 16:09:46 +0200 Subject: [PATCH 13/19] Added support for Range and Rect as a target. --- src/panel/balloon/balloonpanelview.js | 9 +++++-- tests/panel/balloon/balloonpanelview.js | 31 +++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 2 deletions(-) diff --git a/src/panel/balloon/balloonpanelview.js b/src/panel/balloon/balloonpanelview.js index 9c99c58e..10c88dcf 100644 --- a/src/panel/balloon/balloonpanelview.js +++ b/src/panel/balloon/balloonpanelview.js @@ -10,6 +10,8 @@ import View from '../../view'; import Template from '../../template'; import { getOptimalPosition } from '@ckeditor/ckeditor5-utils/src/dom/position'; +import isRange from '@ckeditor/ckeditor5-utils/src/dom/isrange'; +import isElement from '@ckeditor/ckeditor5-utils/src/lib/lodash/isElement'; import toUnit from '@ckeditor/ckeditor5-utils/src/dom/tounit'; import global from '@ckeditor/ckeditor5-utils/src/dom/global'; @@ -183,8 +185,11 @@ export default class BalloonPanelView extends View { // Then we need to listen on scroll event of eny element in the document. this.listenTo( global.document, 'scroll', ( evt, domEvt ) => { - // And update position if scrolled element contains related to the balloon elements. - if ( domEvt.target.contains( target ) || domEvt.target.contains( limiter ) ) { + // We need to take HTMLElement related to the target if it is possible. + const targetNode = isElement( target ) ? target : isRange( target ) ? target.commonAncestorContainer : null; + + // We need to update position if scrolled element contains related to the balloon elements. + if ( ( targetNode && domEvt.target.contains( targetNode ) ) || domEvt.target.contains( limiter ) ) { this.attachTo( options ); } }, { useCapture: true } ); diff --git a/tests/panel/balloon/balloonpanelview.js b/tests/panel/balloon/balloonpanelview.js index 9f3202dc..d1260f43 100644 --- a/tests/panel/balloon/balloonpanelview.js +++ b/tests/panel/balloon/balloonpanelview.js @@ -9,6 +9,7 @@ import global from '@ckeditor/ckeditor5-utils/src/dom/global'; import ViewCollection from '../../../src/viewcollection'; import BalloonPanelView from '../../../src/panel/balloon/balloonpanelview'; import ButtonView from '../../../src/button/buttonview'; +import Rect from '@ckeditor/ckeditor5-utils/src/dom/rect'; import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils'; import * as positionUtils from '@ckeditor/ckeditor5-utils/src/dom/position'; @@ -496,6 +497,36 @@ describe( 'BalloonPanelView', () => { expect( attachToSpy.calledTwice ).to.true; } ); + + it( 'should work for Range as a target', () => { + const element = document.createElement( 'div' ); + const range = document.createRange(); + + element.appendChild( document.createTextNode( 'foo bar' ) ); + document.body.appendChild( element ); + range.selectNodeContents( element ); + + view.keepAttachedTo( { target: range } ); + + expect( attachToSpy.calledOnce ).to.true; + + element.dispatchEvent( new Event( 'scroll' ) ); + + expect( attachToSpy.calledTwice ).to.true; + } ); + + it( 'should work for Rect as a target', () => { + // Just check if this normally works without errors. + const rect = new Rect( {} ); + + view.keepAttachedTo( { target: rect, limiter } ); + + expect( attachToSpy.calledOnce ).to.true; + + limiter.dispatchEvent( new Event( 'scroll' ) ); + + expect( attachToSpy.calledTwice ).to.true; + } ); } ); } ); From 4a31312dd4471511426abc1c17cb2ff298e77bda Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Tue, 4 Apr 2017 16:19:37 +0200 Subject: [PATCH 14/19] Using sinon assertions in tests. --- tests/panel/balloon/balloonpanelview.js | 44 ++++++++++++------------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/tests/panel/balloon/balloonpanelview.js b/tests/panel/balloon/balloonpanelview.js index d1260f43..eb2c6a28 100644 --- a/tests/panel/balloon/balloonpanelview.js +++ b/tests/panel/balloon/balloonpanelview.js @@ -428,42 +428,42 @@ describe( 'BalloonPanelView', () => { it( 'should keep the balloon attached to the target when any of the related elements is scrolled', () => { view.keepAttachedTo( { target, limiter } ); - expect( attachToSpy.calledOnce ).to.true; - expect( attachToSpy.lastCall.args[ 0 ] ).to.deep.equal( { target, limiter } ); + sinon.assert.calledOnce( attachToSpy ); + sinon.assert.calledWith( attachToSpy.lastCall, { target, limiter } ); document.dispatchEvent( new Event( 'scroll' ) ); - expect( attachToSpy.calledTwice ).to.true; - expect( attachToSpy.lastCall.args[ 0 ] ).to.deep.equal( { target, limiter } ); + sinon.assert.calledTwice( attachToSpy ); + sinon.assert.calledWith( attachToSpy.lastCall, { target, limiter } ); limiter.dispatchEvent( new Event( 'scroll' ) ); - expect( attachToSpy.calledThrice ).to.true; - expect( attachToSpy.lastCall.args[ 0 ] ).to.deep.equal( { target, limiter } ); + sinon.assert.calledThrice( attachToSpy ); + sinon.assert.calledWith( attachToSpy.lastCall, { target, limiter } ); notRelatedElement.dispatchEvent( new Event( 'scroll' ) ); // Nothing's changed. - expect( attachToSpy.calledThrice ).to.true; - expect( attachToSpy.lastCall.args[ 0 ] ).to.deep.equal( { target, limiter } ); + sinon.assert.calledThrice( attachToSpy ); + sinon.assert.calledWith( attachToSpy.lastCall, { target, limiter } ); } ); it( 'should keep the balloon attached to the target when the browser window is being resized', () => { view.keepAttachedTo( { target, limiter } ); - expect( attachToSpy.calledOnce ).to.true; - expect( attachToSpy.lastCall.args[ 0 ] ).to.deep.equal( { target, limiter } ); + sinon.assert.calledOnce( attachToSpy ); + sinon.assert.calledWith( attachToSpy.lastCall, { target, limiter } ); window.dispatchEvent( new Event( 'resize' ) ); - expect( attachToSpy.calledTwice ).to.true; - expect( attachToSpy.lastCall.args[ 0 ] ).to.deep.equal( { target, limiter } ); + sinon.assert.calledTwice( attachToSpy ); + sinon.assert.calledWith( attachToSpy.lastCall, { target, limiter } ); } ); it( 'should stop attaching when the balloon is hidden', () => { view.keepAttachedTo( { target, limiter } ); - expect( attachToSpy.calledOnce ).to.true; + sinon.assert.calledOnce( attachToSpy ); view.hide(); @@ -471,13 +471,13 @@ describe( 'BalloonPanelView', () => { window.dispatchEvent( new Event( 'scroll' ) ); // Still once. - expect( attachToSpy.calledOnce ).to.true; + sinon.assert.calledOnce( attachToSpy ); } ); it( 'should stop attaching once the view is destroyed', () => { view.keepAttachedTo( { target, limiter } ); - expect( attachToSpy.calledOnce ).to.true; + sinon.assert.calledOnce( attachToSpy ); view.destroy(); @@ -485,17 +485,17 @@ describe( 'BalloonPanelView', () => { window.dispatchEvent( new Event( 'scroll' ) ); // Still once. - expect( attachToSpy.calledOnce ).to.true; + sinon.assert.calledOnce( attachToSpy ); } ); it( 'should set document.body as the default limiter', () => { view.keepAttachedTo( { target } ); - expect( attachToSpy.calledOnce ).to.true; + sinon.assert.calledOnce( attachToSpy ); document.body.dispatchEvent( new Event( 'scroll' ) ); - expect( attachToSpy.calledTwice ).to.true; + sinon.assert.calledTwice( attachToSpy ); } ); it( 'should work for Range as a target', () => { @@ -508,11 +508,11 @@ describe( 'BalloonPanelView', () => { view.keepAttachedTo( { target: range } ); - expect( attachToSpy.calledOnce ).to.true; + sinon.assert.calledOnce( attachToSpy ); element.dispatchEvent( new Event( 'scroll' ) ); - expect( attachToSpy.calledTwice ).to.true; + sinon.assert.calledTwice( attachToSpy ); } ); it( 'should work for Rect as a target', () => { @@ -521,11 +521,11 @@ describe( 'BalloonPanelView', () => { view.keepAttachedTo( { target: rect, limiter } ); - expect( attachToSpy.calledOnce ).to.true; + sinon.assert.calledOnce( attachToSpy ); limiter.dispatchEvent( new Event( 'scroll' ) ); - expect( attachToSpy.calledTwice ).to.true; + sinon.assert.calledTwice( attachToSpy ); } ); } ); } ); From dd62d007ddef0b514d720e72b368caa76dce09fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Tue, 4 Apr 2017 16:21:04 +0200 Subject: [PATCH 15/19] Added full ticket url. --- tests/panel/balloon/balloonpanelview.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/panel/balloon/balloonpanelview.js b/tests/panel/balloon/balloonpanelview.js index eb2c6a28..b2a4fb8b 100644 --- a/tests/panel/balloon/balloonpanelview.js +++ b/tests/panel/balloon/balloonpanelview.js @@ -259,7 +259,7 @@ describe( 'BalloonPanelView', () => { expect( view.position ).to.equal( 'nw' ); } ); - // #126 + // https://github.com/ckeditor/ckeditor5-ui-default/issues/126 it( 'works in a positioned ancestor (position: absolute)', () => { const positionedAncestor = document.createElement( 'div' ); @@ -291,7 +291,7 @@ describe( 'BalloonPanelView', () => { expect( view.left ).to.equal( -80 ); } ); - // #126 + // https://github.com/ckeditor/ckeditor5-ui-default/issues/126 it( 'works in a positioned ancestor (position: static)', () => { const positionedAncestor = document.createElement( 'div' ); From 5fbbe27be59b7a7b98a09bf321cf94965b64c087 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Tue, 4 Apr 2017 16:24:41 +0200 Subject: [PATCH 16/19] Simplfied the test. --- tests/panel/balloon/balloonpanelview.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/panel/balloon/balloonpanelview.js b/tests/panel/balloon/balloonpanelview.js index b2a4fb8b..51eba369 100644 --- a/tests/panel/balloon/balloonpanelview.js +++ b/tests/panel/balloon/balloonpanelview.js @@ -9,7 +9,6 @@ import global from '@ckeditor/ckeditor5-utils/src/dom/global'; import ViewCollection from '../../../src/viewcollection'; import BalloonPanelView from '../../../src/panel/balloon/balloonpanelview'; import ButtonView from '../../../src/button/buttonview'; -import Rect from '@ckeditor/ckeditor5-utils/src/dom/rect'; import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils'; import * as positionUtils from '@ckeditor/ckeditor5-utils/src/dom/position'; @@ -515,9 +514,9 @@ describe( 'BalloonPanelView', () => { sinon.assert.calledTwice( attachToSpy ); } ); - it( 'should work for Rect as a target', () => { + it( 'should work for rect as a target', () => { // Just check if this normally works without errors. - const rect = new Rect( {} ); + const rect = {}; view.keepAttachedTo( { target: rect, limiter } ); From b7d759e5c74330d23cdd19fe89003cf0da07ef04 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Wed, 5 Apr 2017 11:45:12 +0200 Subject: [PATCH 17/19] Optimized BalloonPanelView#keepAttachedTo by extracting some logic from DOM "scroll" listener. --- src/panel/balloon/balloonpanelview.js | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/panel/balloon/balloonpanelview.js b/src/panel/balloon/balloonpanelview.js index 10c88dcf..113d405d 100644 --- a/src/panel/balloon/balloonpanelview.js +++ b/src/panel/balloon/balloonpanelview.js @@ -180,16 +180,20 @@ export default class BalloonPanelView extends View { // First we need to attach the balloon panel to the target element. this.attachTo( options ); - const target = options.target; const limiter = options.limiter || defaultLimiterElement; + let target = null; + + // We need to take HTMLElement related to the target if it is possible. + if ( isElement( options.target ) ) { + target = target; + } else if ( isRange( options.target ) ) { + target = options.target.commonAncestorContainer; + } // Then we need to listen on scroll event of eny element in the document. this.listenTo( global.document, 'scroll', ( evt, domEvt ) => { - // We need to take HTMLElement related to the target if it is possible. - const targetNode = isElement( target ) ? target : isRange( target ) ? target.commonAncestorContainer : null; - // We need to update position if scrolled element contains related to the balloon elements. - if ( ( targetNode && domEvt.target.contains( targetNode ) ) || domEvt.target.contains( limiter ) ) { + if ( ( target && domEvt.target.contains( target ) ) || domEvt.target.contains( limiter ) ) { this.attachTo( options ); } }, { useCapture: true } ); From e369e410a0ce3d9eb4d36d1c2b9b7b2fa7542cf0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Wed, 5 Apr 2017 12:11:50 +0200 Subject: [PATCH 18/19] Improved test. --- tests/panel/balloon/balloonpanelview.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/panel/balloon/balloonpanelview.js b/tests/panel/balloon/balloonpanelview.js index 51eba369..89ebe225 100644 --- a/tests/panel/balloon/balloonpanelview.js +++ b/tests/panel/balloon/balloonpanelview.js @@ -406,14 +406,17 @@ describe( 'BalloonPanelView', () => { } ); describe( 'keepAttachedTo()', () => { - let attachToSpy, target, limiter, notRelatedElement; + let attachToSpy, target, targetParent, limiter, notRelatedElement; beforeEach( () => { attachToSpy = testUtils.sinon.spy( view, 'attachTo' ); limiter = document.createElement( 'div' ); + targetParent = document.createElement( 'div' ); target = document.createElement( 'div' ); notRelatedElement = document.createElement( 'div' ); + targetParent.appendChild( target ); + document.body.appendChild( targetParent ); document.body.appendChild( limiter ); document.body.appendChild( notRelatedElement ); } ); @@ -430,7 +433,7 @@ describe( 'BalloonPanelView', () => { sinon.assert.calledOnce( attachToSpy ); sinon.assert.calledWith( attachToSpy.lastCall, { target, limiter } ); - document.dispatchEvent( new Event( 'scroll' ) ); + targetParent.dispatchEvent( new Event( 'scroll' ) ); sinon.assert.calledTwice( attachToSpy ); sinon.assert.calledWith( attachToSpy.lastCall, { target, limiter } ); From 3c559020e36e574bf797ab60c9d17e2e0be57d42 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Wed, 5 Apr 2017 12:12:23 +0200 Subject: [PATCH 19/19] Fixed failing test. --- src/panel/balloon/balloonpanelview.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/panel/balloon/balloonpanelview.js b/src/panel/balloon/balloonpanelview.js index 113d405d..21d62b68 100644 --- a/src/panel/balloon/balloonpanelview.js +++ b/src/panel/balloon/balloonpanelview.js @@ -185,7 +185,7 @@ export default class BalloonPanelView extends View { // We need to take HTMLElement related to the target if it is possible. if ( isElement( options.target ) ) { - target = target; + target = options.target; } else if ( isRange( options.target ) ) { target = options.target.commonAncestorContainer; }