From a9e0a4af548880f4b18ee76aea316baa7396c7c5 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Fri, 7 Apr 2017 13:05:23 +0200 Subject: [PATCH 01/12] Renamed and enhanced BalloonPanelView#keepAttachedTo->pin(). Added BalloonPanelView#unpin(). --- src/panel/balloon/balloonpanelview.js | 67 ++++++++++++++------ tests/panel/balloon/balloonpanelview.js | 84 +++++++++++++++++++++---- 2 files changed, 122 insertions(+), 29 deletions(-) diff --git a/src/panel/balloon/balloonpanelview.js b/src/panel/balloon/balloonpanelview.js index 21d62b68..f95aa08c 100644 --- a/src/panel/balloon/balloonpanelview.js +++ b/src/panel/balloon/balloonpanelview.js @@ -170,15 +170,16 @@ export default class BalloonPanelView extends View { * * 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. + * See: {@link #unpin}. * * @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}. */ - keepAttachedTo( options ) { - // First we need to attach the balloon panel to the target element. - this.attachTo( options ); + pin( options ) { + // See https://github.com/ckeditor/ckeditor5-ui/issues/170. + + this.unpin(); const limiter = options.limiter || defaultLimiterElement; let target = null; @@ -190,32 +191,62 @@ export default class BalloonPanelView extends View { 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 update position if scrolled element contains related to the balloon elements. - if ( ( target && domEvt.target.contains( target ) ) || domEvt.target.contains( limiter ) ) { + // Starts managing the pinned state of the panel. + // + // @private + const startPinning = () => { + this.attachTo( options ); + + // Then we need to listen on scroll event of eny element in the document. + this.listenTo( global.document, 'scroll', ( evt, domEvt ) => { + // We need to update position if scrolled element contains related to the balloon elements. + if ( ( target && domEvt.target.contains( target ) ) || domEvt.target.contains( limiter ) ) { + this.attachTo( options ); + } + }, { useCapture: true } ); + + // We need to listen on window resize event and update position. + this.listenTo( global.window, 'resize', () => { this.attachTo( options ); - } - }, { useCapture: true } ); + } ); + }; - // 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 listeners. - this.once( 'change:isVisible', () => { + // Stops managing the pinned state of the panel. + // + // @private + const stopPinning = () => { this.stopListening( global.document, 'scroll' ); this.stopListening( global.window, 'resize' ); + }; + + // If the panel is already visible, enable the listeners immediately. + if ( this.isVisible ) { + startPinning(); + } + + // Control the state of the listeners depending on whether the panel is visible + // or not. + // TODO: Use on() (https://github.com/ckeditor/ckeditor5-utils/issues/144). + this.listenTo( this, 'change:isVisible', () => { + if ( this.isVisible ) { + startPinning(); + } else { + stopPinning(); + } } ); } /** - * @inheritDoc + * Stops pinning the panel, as set up by {@link #pin}. */ - destroy() { + unpin() { + // Deactivate listeners attached by pin(). this.stopListening( global.document, 'scroll' ); this.stopListening( global.window, 'resize' ); - return super.destroy(); + // Deactivate the panel pin() control logic. + // TODO: Use off() (https://github.com/ckeditor/ckeditor5-utils/issues/144). + this.stopListening( this, 'change:isVisible' ); } } diff --git a/tests/panel/balloon/balloonpanelview.js b/tests/panel/balloon/balloonpanelview.js index 89ebe225..ef918954 100644 --- a/tests/panel/balloon/balloonpanelview.js +++ b/tests/panel/balloon/balloonpanelview.js @@ -405,7 +405,7 @@ describe( 'BalloonPanelView', () => { } ); } ); - describe( 'keepAttachedTo()', () => { + describe( 'pin()', () => { let attachToSpy, target, targetParent, limiter, notRelatedElement; beforeEach( () => { @@ -415,6 +415,8 @@ describe( 'BalloonPanelView', () => { target = document.createElement( 'div' ); notRelatedElement = document.createElement( 'div' ); + view.show(); + targetParent.appendChild( target ); document.body.appendChild( targetParent ); document.body.appendChild( limiter ); @@ -422,13 +424,35 @@ describe( 'BalloonPanelView', () => { } ); afterEach( () => { - attachToSpy.restore(); + targetParent.remove(); limiter.remove(); notRelatedElement.remove(); } ); - it( 'should keep the balloon attached to the target when any of the related elements is scrolled', () => { - view.keepAttachedTo( { target, limiter } ); + it( 'should not pin until the balloon gets visible', () => { + view.hide(); + + view.pin( { target, limiter } ); + sinon.assert.notCalled( attachToSpy ); + + view.show(); + sinon.assert.calledOnce( attachToSpy ); + } ); + + it( 'should stop pinning when the balloon becomes invisible', () => { + view.show(); + + view.pin( { target, limiter } ); + sinon.assert.calledOnce( attachToSpy ); + + view.hide(); + + targetParent.dispatchEvent( new Event( 'scroll' ) ); + sinon.assert.calledOnce( attachToSpy ); + } ); + + it( 'should keep the balloon pinned to the target when any of the related elements is scrolled', () => { + view.pin( { target, limiter } ); sinon.assert.calledOnce( attachToSpy ); sinon.assert.calledWith( attachToSpy.lastCall, { target, limiter } ); @@ -450,8 +474,8 @@ describe( 'BalloonPanelView', () => { 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 } ); + it( 'should keep the balloon pinned to the target when the browser window is being resized', () => { + view.pin( { target, limiter } ); sinon.assert.calledOnce( attachToSpy ); sinon.assert.calledWith( attachToSpy.lastCall, { target, limiter } ); @@ -463,7 +487,7 @@ describe( 'BalloonPanelView', () => { } ); it( 'should stop attaching when the balloon is hidden', () => { - view.keepAttachedTo( { target, limiter } ); + view.pin( { target, limiter } ); sinon.assert.calledOnce( attachToSpy ); @@ -477,7 +501,7 @@ describe( 'BalloonPanelView', () => { } ); it( 'should stop attaching once the view is destroyed', () => { - view.keepAttachedTo( { target, limiter } ); + view.pin( { target, limiter } ); sinon.assert.calledOnce( attachToSpy ); @@ -491,7 +515,7 @@ describe( 'BalloonPanelView', () => { } ); it( 'should set document.body as the default limiter', () => { - view.keepAttachedTo( { target } ); + view.pin( { target } ); sinon.assert.calledOnce( attachToSpy ); @@ -508,7 +532,7 @@ describe( 'BalloonPanelView', () => { document.body.appendChild( element ); range.selectNodeContents( element ); - view.keepAttachedTo( { target: range } ); + view.pin( { target: range } ); sinon.assert.calledOnce( attachToSpy ); @@ -521,7 +545,7 @@ describe( 'BalloonPanelView', () => { // Just check if this normally works without errors. const rect = {}; - view.keepAttachedTo( { target: rect, limiter } ); + view.pin( { target: rect, limiter } ); sinon.assert.calledOnce( attachToSpy ); @@ -530,6 +554,44 @@ describe( 'BalloonPanelView', () => { sinon.assert.calledTwice( attachToSpy ); } ); } ); + + describe( 'unpin()', () => { + let attachToSpy, target, targetParent, limiter; + + beforeEach( () => { + attachToSpy = testUtils.sinon.spy( view, 'attachTo' ); + limiter = document.createElement( 'div' ); + targetParent = document.createElement( 'div' ); + target = document.createElement( 'div' ); + + view.show(); + + targetParent.appendChild( target ); + document.body.appendChild( targetParent ); + document.body.appendChild( limiter ); + } ); + + afterEach( () => { + targetParent.remove(); + limiter.remove(); + } ); + + it( 'should stop attaching when the balloon is hidden', () => { + view.pin( { target, limiter } ); + sinon.assert.calledOnce( attachToSpy ); + + view.unpin(); + + view.hide(); + window.dispatchEvent( new Event( 'resize' ) ); + window.dispatchEvent( new Event( 'scroll' ) ); + view.show(); + window.dispatchEvent( new Event( 'resize' ) ); + window.dispatchEvent( new Event( 'scroll' ) ); + + sinon.assert.calledOnce( attachToSpy ); + } ); + } ); } ); function mockBoundingBox( element, data ) { From df7bb4aab7d054efbfa72bc905df58fabdc60b48 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Fri, 7 Apr 2017 14:29:36 +0200 Subject: [PATCH 02/12] Code refactoring in BalloonPanelView. --- src/panel/balloon/balloonpanelview.js | 104 ++++++++++++++------------ 1 file changed, 55 insertions(+), 49 deletions(-) diff --git a/src/panel/balloon/balloonpanelview.js b/src/panel/balloon/balloonpanelview.js index f95aa08c..7229b102 100644 --- a/src/panel/balloon/balloonpanelview.js +++ b/src/panel/balloon/balloonpanelview.js @@ -177,63 +177,23 @@ export default class BalloonPanelView extends View { * {@link module:ui/panel/balloon/balloonpanelview~BalloonPanelView.defaultPositions}. */ pin( options ) { - // See https://github.com/ckeditor/ckeditor5-ui/issues/170. - this.unpin(); - 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 = options.target; - } else if ( isRange( options.target ) ) { - target = options.target.commonAncestorContainer; - } - - // Starts managing the pinned state of the panel. - // - // @private - const startPinning = () => { - this.attachTo( options ); - - // Then we need to listen on scroll event of eny element in the document. - this.listenTo( global.document, 'scroll', ( evt, domEvt ) => { - // We need to update position if scrolled element contains related to the balloon elements. - if ( ( target && domEvt.target.contains( target ) ) || domEvt.target.contains( limiter ) ) { - this.attachTo( options ); - } - }, { useCapture: true } ); - - // We need to listen on window resize event and update position. - this.listenTo( global.window, 'resize', () => { - this.attachTo( options ); - } ); - }; - - // Stops managing the pinned state of the panel. - // - // @private - const stopPinning = () => { - this.stopListening( global.document, 'scroll' ); - this.stopListening( global.window, 'resize' ); + this._pinWhenVisible = () => { + if ( this.isVisible ) { + this._startPinning( options ); + } else { + this._stopPinning(); + } }; // If the panel is already visible, enable the listeners immediately. - if ( this.isVisible ) { - startPinning(); - } + this._pinWhenVisible(); // Control the state of the listeners depending on whether the panel is visible // or not. // TODO: Use on() (https://github.com/ckeditor/ckeditor5-utils/issues/144). - this.listenTo( this, 'change:isVisible', () => { - if ( this.isVisible ) { - startPinning(); - } else { - stopPinning(); - } - } ); + this.listenTo( this, 'change:isVisible', this._pinWhenVisible ); } /** @@ -246,7 +206,53 @@ export default class BalloonPanelView extends View { // Deactivate the panel pin() control logic. // TODO: Use off() (https://github.com/ckeditor/ckeditor5-utils/issues/144). - this.stopListening( this, 'change:isVisible' ); + if ( this._pinWhenVisible ) { + this.stopListening( this, 'change:isVisible', this._pinWhenVisible ); + } + } + + /** + * Starts managing the pinned state of the panel. See {@link #pin}. + * + * @private + * @param {module:utils/dom/position~Options} options Positioning options compatible with + * {@link module:utils/dom/position~getOptimalPosition}. + */ + _startPinning( options ) { + this.attachTo( options ); + + 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 = options.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 update position if scrolled element contains related to the balloon elements. + if ( ( target && domEvt.target.contains( target ) ) || domEvt.target.contains( limiter ) ) { + this.attachTo( options ); + } + }, { useCapture: true } ); + + // We need to listen on window resize event and update position. + this.listenTo( global.window, 'resize', () => { + this.attachTo( options ); + } ); + } + + /** + * Stops managing the pinned state of the panel. See {@link #pin}. + * + * @private + */ + _stopPinning() { + this.stopListening( global.document, 'scroll' ); + this.stopListening( global.window, 'resize' ); } } From 04a5a172b55f6e3a2f36cfaf1ac20060433395d9 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Fri, 7 Apr 2017 14:43:22 +0200 Subject: [PATCH 03/12] Implemented BalloonPanelView#withArrow. --- src/panel/balloon/balloonpanelview.js | 22 +++++++--- theme/components/panel/balloonpanel.scss | 52 ++++++++++++------------ 2 files changed, 43 insertions(+), 31 deletions(-) diff --git a/src/panel/balloon/balloonpanelview.js b/src/panel/balloon/balloonpanelview.js index 7229b102..46e05434 100644 --- a/src/panel/balloon/balloonpanelview.js +++ b/src/panel/balloon/balloonpanelview.js @@ -74,6 +74,15 @@ export default class BalloonPanelView extends View { */ this.set( 'isVisible', false ); + /** + * Controls whether the balloon panel has an arrow. + * + * @observable + * @default true + * @member {Boolean} #withArrow + */ + this.set( 'withArrow', true ); + /** * Max width of the balloon panel, as in CSS. * @@ -94,8 +103,9 @@ export default class BalloonPanelView extends View { attributes: { class: [ 'ck-balloon-panel', - bind.to( 'position', ( value ) => `ck-balloon-panel_arrow_${ value }` ), - bind.if( 'isVisible', 'ck-balloon-panel_visible' ) + bind.to( 'position', ( value ) => `ck-balloon-panel_${ value }` ), + bind.if( 'isVisible', 'ck-balloon-panel_visible' ), + bind.if( 'withArrow', 'ck-balloon-panel_arrow' ) ], style: { @@ -347,24 +357,24 @@ BalloonPanelView.defaultPositions = { se: ( targetRect ) => ( { top: targetRect.bottom + BalloonPanelView.arrowVerticalOffset, left: targetRect.left + targetRect.width / 2 - BalloonPanelView.arrowHorizontalOffset, - name: 'se' + name: 'arrow_se' } ), sw: ( targetRect, balloonRect ) => ( { top: targetRect.bottom + BalloonPanelView.arrowVerticalOffset, left: targetRect.left + targetRect.width / 2 - balloonRect.width + BalloonPanelView.arrowHorizontalOffset, - name: 'sw' + name: 'arrow_sw' } ), ne: ( targetRect, balloonRect ) => ( { top: targetRect.top - balloonRect.height - BalloonPanelView.arrowVerticalOffset, left: targetRect.left + targetRect.width / 2 - BalloonPanelView.arrowHorizontalOffset, - name: 'ne' + name: 'arrow_ne' } ), nw: ( targetRect, balloonRect ) => ( { top: targetRect.top - balloonRect.height - BalloonPanelView.arrowVerticalOffset, left: targetRect.left + targetRect.width / 2 - balloonRect.width + BalloonPanelView.arrowHorizontalOffset, - name: 'nw' + name: 'arrow_nw' } ) }; diff --git a/theme/components/panel/balloonpanel.scss b/theme/components/panel/balloonpanel.scss index 736bed48..4d572655 100644 --- a/theme/components/panel/balloonpanel.scss +++ b/theme/components/panel/balloonpanel.scss @@ -9,41 +9,43 @@ z-index: ck-z( 'modal' ); - &:before, - &:after { - content: ""; - position: absolute; - } - - &:before { - z-index: ck-z(); - } - - &:after { - z-index: ck-z( 'default', 1 ); - } + &.ck-balloon-panel_arrow { + &:before, + &:after { + content: ""; + position: absolute; + } - &_arrow_s, - &_arrow_se, - &_arrow_sw { &:before { - z-index: ck-z( 'default' ); + z-index: ck-z(); } &:after { z-index: ck-z( 'default', 1 ); } - } - &_arrow_n, - &_arrow_ne, - &_arrow_nw { - &:before { - z-index: ck-z( 'default' ); + &_s, + &_se, + &_sw { + &:before { + z-index: ck-z( 'default' ); + } + + &:after { + z-index: ck-z( 'default', 1 ); + } } - &:after { - z-index: ck-z( 'default', 1 ); + &_n, + &_ne, + &_nw { + &:before { + z-index: ck-z( 'default' ); + } + + &:after { + z-index: ck-z( 'default', 1 ); + } } } From fb982384f62208211b9fe7a2c40dd9e1312f9ab4 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Mon, 10 Apr 2017 11:27:30 +0200 Subject: [PATCH 04/12] Renamed default positions in BalloonPanelView.defaultPositions. Updated tests to the new pin() API. Updated documentation. --- src/panel/balloon/balloonpanelview.js | 30 +++++++---- tests/manual/tickets/170/1.js | 2 +- tests/manual/tickets/170/1.md | 2 +- tests/panel/balloon/balloonpanelview.js | 71 +++++++++++++++++-------- 4 files changed, 71 insertions(+), 34 deletions(-) diff --git a/src/panel/balloon/balloonpanelview.js b/src/panel/balloon/balloonpanelview.js index 46e05434..fd0a4777 100644 --- a/src/panel/balloon/balloonpanelview.js +++ b/src/panel/balloon/balloonpanelview.js @@ -61,9 +61,9 @@ export default class BalloonPanelView extends View { * * @observable * @default 'se' - * @member {'se'|'sw'|'ne'|'nw'} #position + * @member {'arrow_se'|'arrow_sw'|'arrow_ne'|'arrow_nw'} #position */ - this.set( 'position', 'se' ); + this.set( 'position', 'arrow_se' ); /** * Controls whether the balloon panel is visible or not. @@ -90,6 +90,14 @@ export default class BalloonPanelView extends View { * @member {Number} #maxWidth */ + /** + * A callback that starts pining the panel when {@link #isVisible} gets + * `true`. Used by {@link #pin}. + * + * @private + * @member {Function} #_pinWhenVisibleCallback + */ + /** * Collection of the child views which creates balloon panel contents. * @@ -189,7 +197,7 @@ export default class BalloonPanelView extends View { pin( options ) { this.unpin(); - this._pinWhenVisible = () => { + this._pinWhenVisibleCallback = () => { if ( this.isVisible ) { this._startPinning( options ); } else { @@ -198,12 +206,14 @@ export default class BalloonPanelView extends View { }; // If the panel is already visible, enable the listeners immediately. - this._pinWhenVisible(); + if ( this.isVisible ) { + this._startPinning( options ); + } // Control the state of the listeners depending on whether the panel is visible // or not. // TODO: Use on() (https://github.com/ckeditor/ckeditor5-utils/issues/144). - this.listenTo( this, 'change:isVisible', this._pinWhenVisible ); + this.listenTo( this, 'change:isVisible', this._pinWhenVisibleCallback ); } /** @@ -214,10 +224,12 @@ export default class BalloonPanelView extends View { this.stopListening( global.document, 'scroll' ); this.stopListening( global.window, 'resize' ); - // Deactivate the panel pin() control logic. - // TODO: Use off() (https://github.com/ckeditor/ckeditor5-utils/issues/144). - if ( this._pinWhenVisible ) { - this.stopListening( this, 'change:isVisible', this._pinWhenVisible ); + if ( this._pinWhenVisibleCallback ) { + // Deactivate the panel pin() control logic. + // TODO: Use off() (https://github.com/ckeditor/ckeditor5-utils/issues/144). + this.stopListening( this, 'change:isVisible', this._pinWhenVisibleCallback ); + + this._pinWhenVisibleCallback = null; } } diff --git a/tests/manual/tickets/170/1.js b/tests/manual/tickets/170/1.js index ccfa0362..ac00b1d7 100644 --- a/tests/manual/tickets/170/1.js +++ b/tests/manual/tickets/170/1.js @@ -52,7 +52,7 @@ ClassicEditor.create( document.querySelector( '#editor-stick' ), { editor.ui.view.element.querySelector( '.ck-editor__editable' ).scrollTop = 360; panel.init().then( () => { - panel.keepAttachedTo( { + panel.pin( { target: editor.ui.view.element.querySelector( '.ck-editor__editable p strong' ), limiter: editor.ui.view.editableElement } ); diff --git a/tests/manual/tickets/170/1.md b/tests/manual/tickets/170/1.md index 904f1286..bf692040 100644 --- a/tests/manual/tickets/170/1.md +++ b/tests/manual/tickets/170/1.md @@ -1,4 +1,4 @@ -## BalloonPanelView `attachTo` vs `keepAttachedTo` +## BalloonPanelView `attachTo` vs `pin` 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. diff --git a/tests/panel/balloon/balloonpanelview.js b/tests/panel/balloon/balloonpanelview.js index ef918954..16f103c0 100644 --- a/tests/panel/balloon/balloonpanelview.js +++ b/tests/panel/balloon/balloonpanelview.js @@ -25,6 +25,12 @@ describe( 'BalloonPanelView', () => { return view.init(); } ); + afterEach( () => { + if ( view ) { + return view.destroy(); + } + } ); + describe( 'constructor()', () => { it( 'should create element from template', () => { expect( view.element.tagName ).to.equal( 'DIV' ); @@ -35,7 +41,7 @@ describe( 'BalloonPanelView', () => { it( 'should set default values', () => { expect( view.top ).to.equal( 0 ); expect( view.left ).to.equal( 0 ); - expect( view.position ).to.equal( 'se' ); + expect( view.position ).to.equal( 'arrow_se' ); expect( view.isVisible ).to.equal( false ); } ); @@ -49,7 +55,7 @@ describe( 'BalloonPanelView', () => { it( 'should react on view#position', () => { expect( view.element.classList.contains( 'ck-balloon-panel_arrow_se' ) ).to.true; - view.position = 'sw'; + view.position = 'arrow_sw'; expect( view.element.classList.contains( 'ck-balloon-panel_arrow_sw' ) ).to.true; } ); @@ -96,9 +102,10 @@ describe( 'BalloonPanelView', () => { expect( view.element.childNodes.length ).to.equal( 0 ); const button = new ButtonView( { t() {} } ); - view.content.add( button ); - expect( view.element.childNodes.length ).to.equal( 1 ); + return view.content.add( button ).then( () => { + expect( view.element.childNodes.length ).to.equal( 1 ); + } ); } ); } ); } ); @@ -203,7 +210,7 @@ describe( 'BalloonPanelView', () => { view.attachTo( { target, limiter } ); - expect( view.position ).to.equal( 'se' ); + expect( view.position ).to.equal( 'arrow_se' ); } ); it( 'should put balloon on the `south east` side of the target element when target is on the top left side of the limiter', () => { @@ -216,7 +223,7 @@ describe( 'BalloonPanelView', () => { view.attachTo( { target, limiter } ); - expect( view.position ).to.equal( 'se' ); + expect( view.position ).to.equal( 'arrow_se' ); } ); it( 'should put balloon on the `south west` side of the target element when target is on the right side of the limiter', () => { @@ -229,7 +236,7 @@ describe( 'BalloonPanelView', () => { view.attachTo( { target, limiter } ); - expect( view.position ).to.equal( 'sw' ); + expect( view.position ).to.equal( 'arrow_sw' ); } ); it( 'should put balloon on the `north east` side of the target element when target is on the bottom of the limiter ', () => { @@ -242,7 +249,7 @@ describe( 'BalloonPanelView', () => { view.attachTo( { target, limiter } ); - expect( view.position ).to.equal( 'ne' ); + expect( view.position ).to.equal( 'arrow_ne' ); } ); it( 'should put balloon on the `north west` side of the target element when target is on the bottom right of the limiter', () => { @@ -255,7 +262,7 @@ describe( 'BalloonPanelView', () => { view.attachTo( { target, limiter } ); - expect( view.position ).to.equal( 'nw' ); + expect( view.position ).to.equal( 'arrow_nw' ); } ); // https://github.com/ckeditor/ckeditor5-ui-default/issues/126 @@ -336,7 +343,7 @@ describe( 'BalloonPanelView', () => { view.attachTo( { target, limiter } ); - expect( view.position ).to.equal( 'sw' ); + expect( view.position ).to.equal( 'arrow_sw' ); } ); it( 'should put balloon on the `south east` position when `south west` is limited', () => { @@ -356,7 +363,7 @@ describe( 'BalloonPanelView', () => { view.attachTo( { target, limiter } ); - expect( view.position ).to.equal( 'se' ); + expect( view.position ).to.equal( 'arrow_se' ); } ); it( 'should put balloon on the `north east` position when `south east` is limited', () => { @@ -380,7 +387,7 @@ describe( 'BalloonPanelView', () => { view.attachTo( { target, limiter } ); - expect( view.position ).to.equal( 'ne' ); + expect( view.position ).to.equal( 'arrow_ne' ); } ); it( 'should put balloon on the `south east` position when `north east` is limited', () => { @@ -400,7 +407,7 @@ describe( 'BalloonPanelView', () => { view.attachTo( { target, limiter } ); - expect( view.position ).to.equal( 'se' ); + expect( view.position ).to.equal( 'arrow_se' ); } ); } ); } ); @@ -409,7 +416,7 @@ describe( 'BalloonPanelView', () => { let attachToSpy, target, targetParent, limiter, notRelatedElement; beforeEach( () => { - attachToSpy = testUtils.sinon.spy( view, 'attachTo' ); + attachToSpy = sinon.spy( view, 'attachTo' ); limiter = document.createElement( 'div' ); targetParent = document.createElement( 'div' ); target = document.createElement( 'div' ); @@ -451,6 +458,22 @@ describe( 'BalloonPanelView', () => { sinon.assert.calledOnce( attachToSpy ); } ); + it( 'should unpin if already pinned', () => { + const unpinSpy = testUtils.sinon.spy( view, 'unpin' ); + + view.show(); + sinon.assert.notCalled( attachToSpy ); + + view.pin( { target, limiter } ); + sinon.assert.calledOnce( attachToSpy ); + + view.pin( { target, limiter } ); + sinon.assert.calledTwice( unpinSpy ); + + targetParent.dispatchEvent( new Event( 'scroll' ) ); + sinon.assert.calledThrice( attachToSpy ); + } ); + it( 'should keep the balloon pinned to the target when any of the related elements is scrolled', () => { view.pin( { target, limiter } ); @@ -505,13 +528,15 @@ describe( 'BalloonPanelView', () => { sinon.assert.calledOnce( attachToSpy ); - view.destroy(); + return view.destroy().then( () => { + view = null; - window.dispatchEvent( new Event( 'resize' ) ); - window.dispatchEvent( new Event( 'scroll' ) ); + window.dispatchEvent( new Event( 'resize' ) ); + window.dispatchEvent( new Event( 'scroll' ) ); - // Still once. - sinon.assert.calledOnce( attachToSpy ); + // Still once. + sinon.assert.calledOnce( attachToSpy ); + } ); } ); it( 'should set document.body as the default limiter', () => { @@ -559,7 +584,7 @@ describe( 'BalloonPanelView', () => { let attachToSpy, target, targetParent, limiter; beforeEach( () => { - attachToSpy = testUtils.sinon.spy( view, 'attachTo' ); + attachToSpy = sinon.spy( view, 'attachTo' ); limiter = document.createElement( 'div' ); targetParent = document.createElement( 'div' ); target = document.createElement( 'div' ); @@ -576,7 +601,7 @@ describe( 'BalloonPanelView', () => { limiter.remove(); } ); - it( 'should stop attaching when the balloon is hidden', () => { + it( 'should stop attaching', () => { view.pin( { target, limiter } ); sinon.assert.calledOnce( attachToSpy ); @@ -584,10 +609,10 @@ describe( 'BalloonPanelView', () => { view.hide(); window.dispatchEvent( new Event( 'resize' ) ); - window.dispatchEvent( new Event( 'scroll' ) ); + document.dispatchEvent( new Event( 'scroll' ) ); view.show(); window.dispatchEvent( new Event( 'resize' ) ); - window.dispatchEvent( new Event( 'scroll' ) ); + document.dispatchEvent( new Event( 'scroll' ) ); sinon.assert.calledOnce( attachToSpy ); } ); From 9e0a3a786864ca2bafba34ea4753e77282c96e36 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Mon, 10 Apr 2017 11:28:47 +0200 Subject: [PATCH 05/12] Tests: Refactoring in BalloonPanelView#pin|unpin tests. --- tests/panel/balloon/balloonpanelview.js | 226 +++++++++++------------- 1 file changed, 104 insertions(+), 122 deletions(-) diff --git a/tests/panel/balloon/balloonpanelview.js b/tests/panel/balloon/balloonpanelview.js index 16f103c0..963f83a1 100644 --- a/tests/panel/balloon/balloonpanelview.js +++ b/tests/panel/balloon/balloonpanelview.js @@ -412,7 +412,7 @@ describe( 'BalloonPanelView', () => { } ); } ); - describe( 'pin()', () => { + describe( 'pin() and unpin()', () => { let attachToSpy, target, targetParent, limiter, notRelatedElement; beforeEach( () => { @@ -436,100 +436,86 @@ describe( 'BalloonPanelView', () => { notRelatedElement.remove(); } ); - it( 'should not pin until the balloon gets visible', () => { - view.hide(); - - view.pin( { target, limiter } ); - sinon.assert.notCalled( attachToSpy ); - - view.show(); - sinon.assert.calledOnce( attachToSpy ); - } ); - - it( 'should stop pinning when the balloon becomes invisible', () => { - view.show(); - - view.pin( { target, limiter } ); - sinon.assert.calledOnce( attachToSpy ); + describe( 'pin()', () => { + it( 'should not pin until the balloon gets visible', () => { + view.hide(); - view.hide(); - - targetParent.dispatchEvent( new Event( 'scroll' ) ); - sinon.assert.calledOnce( attachToSpy ); - } ); + view.pin( { target, limiter } ); + sinon.assert.notCalled( attachToSpy ); - it( 'should unpin if already pinned', () => { - const unpinSpy = testUtils.sinon.spy( view, 'unpin' ); + view.show(); + sinon.assert.calledOnce( attachToSpy ); + } ); - view.show(); - sinon.assert.notCalled( attachToSpy ); + it( 'should stop pinning when the balloon becomes invisible', () => { + view.show(); - view.pin( { target, limiter } ); - sinon.assert.calledOnce( attachToSpy ); + view.pin( { target, limiter } ); + sinon.assert.calledOnce( attachToSpy ); - view.pin( { target, limiter } ); - sinon.assert.calledTwice( unpinSpy ); + view.hide(); - targetParent.dispatchEvent( new Event( 'scroll' ) ); - sinon.assert.calledThrice( attachToSpy ); - } ); + targetParent.dispatchEvent( new Event( 'scroll' ) ); + sinon.assert.calledOnce( attachToSpy ); + } ); - it( 'should keep the balloon pinned to the target when any of the related elements is scrolled', () => { - view.pin( { target, limiter } ); + it( 'should unpin if already pinned', () => { + const unpinSpy = testUtils.sinon.spy( view, 'unpin' ); - sinon.assert.calledOnce( attachToSpy ); - sinon.assert.calledWith( attachToSpy.lastCall, { target, limiter } ); + view.show(); + sinon.assert.notCalled( attachToSpy ); - targetParent.dispatchEvent( new Event( 'scroll' ) ); + view.pin( { target, limiter } ); + sinon.assert.calledOnce( attachToSpy ); - sinon.assert.calledTwice( attachToSpy ); - sinon.assert.calledWith( attachToSpy.lastCall, { target, limiter } ); + view.pin( { target, limiter } ); + sinon.assert.calledTwice( unpinSpy ); - limiter.dispatchEvent( new Event( 'scroll' ) ); + targetParent.dispatchEvent( new Event( 'scroll' ) ); + sinon.assert.calledThrice( attachToSpy ); + } ); - sinon.assert.calledThrice( attachToSpy ); - sinon.assert.calledWith( attachToSpy.lastCall, { target, limiter } ); + it( 'should keep the balloon pinned to the target when any of the related elements is scrolled', () => { + view.pin( { target, limiter } ); - notRelatedElement.dispatchEvent( new Event( 'scroll' ) ); + sinon.assert.calledOnce( attachToSpy ); + sinon.assert.calledWith( attachToSpy.lastCall, { target, limiter } ); - // Nothing's changed. - sinon.assert.calledThrice( attachToSpy ); - sinon.assert.calledWith( attachToSpy.lastCall, { target, limiter } ); - } ); + targetParent.dispatchEvent( new Event( 'scroll' ) ); - it( 'should keep the balloon pinned to the target when the browser window is being resized', () => { - view.pin( { target, limiter } ); + sinon.assert.calledTwice( attachToSpy ); + sinon.assert.calledWith( attachToSpy.lastCall, { target, limiter } ); - sinon.assert.calledOnce( attachToSpy ); - sinon.assert.calledWith( attachToSpy.lastCall, { target, limiter } ); + limiter.dispatchEvent( new Event( 'scroll' ) ); - window.dispatchEvent( new Event( 'resize' ) ); + sinon.assert.calledThrice( attachToSpy ); + sinon.assert.calledWith( attachToSpy.lastCall, { target, limiter } ); - sinon.assert.calledTwice( attachToSpy ); - sinon.assert.calledWith( attachToSpy.lastCall, { target, limiter } ); - } ); + notRelatedElement.dispatchEvent( new Event( 'scroll' ) ); - it( 'should stop attaching when the balloon is hidden', () => { - view.pin( { target, limiter } ); + // Nothing's changed. + sinon.assert.calledThrice( attachToSpy ); + sinon.assert.calledWith( attachToSpy.lastCall, { target, limiter } ); + } ); - sinon.assert.calledOnce( attachToSpy ); + it( 'should keep the balloon pinned to the target when the browser window is being resized', () => { + view.pin( { target, limiter } ); - view.hide(); + sinon.assert.calledOnce( attachToSpy ); + sinon.assert.calledWith( attachToSpy.lastCall, { target, limiter } ); - window.dispatchEvent( new Event( 'resize' ) ); - window.dispatchEvent( new Event( 'scroll' ) ); + window.dispatchEvent( new Event( 'resize' ) ); - // Still once. - sinon.assert.calledOnce( attachToSpy ); - } ); + sinon.assert.calledTwice( attachToSpy ); + sinon.assert.calledWith( attachToSpy.lastCall, { target, limiter } ); + } ); - it( 'should stop attaching once the view is destroyed', () => { - view.pin( { target, limiter } ); + it( 'should stop attaching when the balloon is hidden', () => { + view.pin( { target, limiter } ); - sinon.assert.calledOnce( attachToSpy ); + sinon.assert.calledOnce( attachToSpy ); - return view.destroy().then( () => { - view = null; + view.hide(); window.dispatchEvent( new Event( 'resize' ) ); window.dispatchEvent( new Event( 'scroll' ) ); @@ -537,84 +523,80 @@ describe( 'BalloonPanelView', () => { // Still once. sinon.assert.calledOnce( attachToSpy ); } ); - } ); - it( 'should set document.body as the default limiter', () => { - view.pin( { target } ); + it( 'should stop attaching once the view is destroyed', () => { + view.pin( { target, limiter } ); - sinon.assert.calledOnce( attachToSpy ); + sinon.assert.calledOnce( attachToSpy ); - document.body.dispatchEvent( new Event( 'scroll' ) ); + return view.destroy().then( () => { + view = null; - sinon.assert.calledTwice( attachToSpy ); - } ); + window.dispatchEvent( new Event( 'resize' ) ); + window.dispatchEvent( new Event( 'scroll' ) ); - it( 'should work for Range as a target', () => { - const element = document.createElement( 'div' ); - const range = document.createRange(); + // Still once. + sinon.assert.calledOnce( attachToSpy ); + } ); + } ); - element.appendChild( document.createTextNode( 'foo bar' ) ); - document.body.appendChild( element ); - range.selectNodeContents( element ); + it( 'should set document.body as the default limiter', () => { + view.pin( { target } ); - view.pin( { target: range } ); + sinon.assert.calledOnce( attachToSpy ); - sinon.assert.calledOnce( attachToSpy ); + document.body.dispatchEvent( new Event( 'scroll' ) ); - element.dispatchEvent( new Event( 'scroll' ) ); + sinon.assert.calledTwice( attachToSpy ); + } ); - sinon.assert.calledTwice( attachToSpy ); - } ); + it( 'should work for Range as a target', () => { + const element = document.createElement( 'div' ); + const range = document.createRange(); - it( 'should work for rect as a target', () => { - // Just check if this normally works without errors. - const rect = {}; + element.appendChild( document.createTextNode( 'foo bar' ) ); + document.body.appendChild( element ); + range.selectNodeContents( element ); - view.pin( { target: rect, limiter } ); + view.pin( { target: range } ); - sinon.assert.calledOnce( attachToSpy ); + sinon.assert.calledOnce( attachToSpy ); - limiter.dispatchEvent( new Event( 'scroll' ) ); + element.dispatchEvent( new Event( 'scroll' ) ); - sinon.assert.calledTwice( attachToSpy ); - } ); - } ); + sinon.assert.calledTwice( attachToSpy ); + } ); - describe( 'unpin()', () => { - let attachToSpy, target, targetParent, limiter; + it( 'should work for rect as a target', () => { + // Just check if this normally works without errors. + const rect = {}; - beforeEach( () => { - attachToSpy = sinon.spy( view, 'attachTo' ); - limiter = document.createElement( 'div' ); - targetParent = document.createElement( 'div' ); - target = document.createElement( 'div' ); + view.pin( { target: rect, limiter } ); - view.show(); + sinon.assert.calledOnce( attachToSpy ); - targetParent.appendChild( target ); - document.body.appendChild( targetParent ); - document.body.appendChild( limiter ); - } ); + limiter.dispatchEvent( new Event( 'scroll' ) ); - afterEach( () => { - targetParent.remove(); - limiter.remove(); + sinon.assert.calledTwice( attachToSpy ); + } ); } ); - it( 'should stop attaching', () => { - view.pin( { target, limiter } ); - sinon.assert.calledOnce( attachToSpy ); + describe( 'unpin()', () => { + it( 'should stop attaching', () => { + view.pin( { target, limiter } ); + sinon.assert.calledOnce( attachToSpy ); - view.unpin(); + view.unpin(); - view.hide(); - window.dispatchEvent( new Event( 'resize' ) ); - document.dispatchEvent( new Event( 'scroll' ) ); - view.show(); - window.dispatchEvent( new Event( 'resize' ) ); - document.dispatchEvent( new Event( 'scroll' ) ); + view.hide(); + window.dispatchEvent( new Event( 'resize' ) ); + document.dispatchEvent( new Event( 'scroll' ) ); + view.show(); + window.dispatchEvent( new Event( 'resize' ) ); + document.dispatchEvent( new Event( 'scroll' ) ); - sinon.assert.calledOnce( attachToSpy ); + sinon.assert.calledOnce( attachToSpy ); + } ); } ); } ); } ); From fdf3803fcee9227e828033dd54fe65d1b00f1c55 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Mon, 10 Apr 2017 11:29:50 +0200 Subject: [PATCH 06/12] Removed FloatingPanelView. Replaced by BalloonPanelView. --- src/panel/floating/floatingpanelview.js | 196 ------------------ tests/panel/floating/floatingpanelview.js | 229 ---------------------- 2 files changed, 425 deletions(-) delete mode 100644 src/panel/floating/floatingpanelview.js delete mode 100644 tests/panel/floating/floatingpanelview.js diff --git a/src/panel/floating/floatingpanelview.js b/src/panel/floating/floatingpanelview.js deleted file mode 100644 index 042be7cd..00000000 --- a/src/panel/floating/floatingpanelview.js +++ /dev/null @@ -1,196 +0,0 @@ -/** - * @license Copyright (c) 2003-2017, CKSource - Frederico Knabben. All rights reserved. - * For licensing, see LICENSE.md. - */ - -/** - * @module ui/panel/floating/floatingpanelview - */ - -import global from '@ckeditor/ckeditor5-utils/src/dom/global'; -import Template from '../../template'; -import View from '../../view'; -import toUnit from '@ckeditor/ckeditor5-utils/src/dom/tounit'; -import { getOptimalPosition } from '@ckeditor/ckeditor5-utils/src/dom/position'; - -const toPx = toUnit( 'px' ); - -/** - * The floating panel view class. It floats around the - * {@link module:ui/panel/floating/floatingpanelview~FloatingPanelView#targetElement} in DOM - * to remain visible in the browser viewport. - * - * See {@link module:ui/panel/floating/floatingpanelview~FloatingPanelView.defaultPositions} - * to learn about the positioning. - * - * @extends module:ui/view~View - */ -export default class FloatingPanelView extends View { - /** - * @inheritDoc - */ - constructor( locale ) { - super( locale ); - - const bind = this.bindTemplate; - - /** - * Controls whether the floating panel is active. When any editable - * is focused in the editor, panel becomes active. - * - * @readonly - * @observable - * @member {Boolean} #isActive - */ - this.set( 'isActive', false ); - - /** - * The absolute top position of the panel, in pixels. - * - * @observable - * @default 0 - * @member {Number} #top - */ - this.set( 'top', 0 ); - - /** - * The absolute left position of the panel, in pixels. - * - * @observable - * @default 0 - * @member {Number} #left - */ - this.set( 'left', 0 ); - - /** - * An element with respect to which the panel is positioned. - * - * @readonly - * @observable - * @member {HTMLElement} #targetElement - */ - this.set( 'targetElement', null ); - - /** - * Collection of the child views which creates balloon panel contents. - * - * @readonly - * @member {module:ui/viewcollection~ViewCollection} - */ - this.content = this.createCollection(); - - this.template = new Template( { - tag: 'div', - attributes: { - class: [ - 'ck-floating-panel', - bind.if( 'isActive', 'ck-floating-panel_active' ), - ], - style: { - top: bind.to( 'top', toPx ), - left: bind.to( 'left', toPx ), - } - }, - - children: this.content - } ); - } - - /** - * @inheritDoc - */ - init() { - this.listenTo( global.window, 'scroll', () => this._updatePosition() ); - this.listenTo( this, 'change:isActive', () => this._updatePosition() ); - - return super.init(); - } - - /** - * Analyzes the environment to decide where the panel should - * be positioned. - * - * @protected - */ - _updatePosition() { - if ( !this.isActive ) { - return; - } - - const { nw, sw, ne, se } = FloatingPanelView.defaultPositions; - const { top, left } = getOptimalPosition( { - element: this.element, - target: this.targetElement, - positions: [ nw, sw, ne, se ], - limiter: global.document.body, - fitInViewport: true - } ); - - Object.assign( this, { top, left } ); - } -} - -/** - * A default set of positioning functions used by the panel view to float - * around {@link module:ui/panel/floating/floatingpanelview~FloatingPanelView#targetElement}. - * - * The available positioning functions are as follows: - * - * * South east: - * - * +----------------+ - * | #targetElement | - * +----------------+ - * [ Panel ] - * - * * South west: - * - * +----------------+ - * | #targetElement | - * +----------------+ - * [ Panel ] - * - * * North east: - * - * [ Panel ] - * +----------------+ - * | #targetElement | - * +----------------+ - * - * - * * North west: - * - * [ Panel ] - * +----------------+ - * | #targetElement | - * +----------------+ - * - * Positioning functions must be compatible with {@link module:utils/dom/position~Position}. - * - * @member {Object} module:ui/panel/floating/floatingpanelview~FloatingPanelView.defaultPositions - */ -FloatingPanelView.defaultPositions = { - nw: ( targetRect, panelRect ) => ( { - top: targetRect.top - panelRect.height, - left: targetRect.left, - name: 'nw' - } ), - - sw: ( targetRect ) => ( { - top: targetRect.bottom, - left: targetRect.left, - name: 'sw' - } ), - - ne: ( targetRect, panelRect ) => ( { - top: targetRect.top - panelRect.height, - left: targetRect.left + targetRect.width - panelRect.width, - name: 'ne' - } ), - - se: ( targetRect, panelRect ) => ( { - top: targetRect.bottom, - left: targetRect.left + targetRect.width - panelRect.width, - name: 'se' - } ) -}; diff --git a/tests/panel/floating/floatingpanelview.js b/tests/panel/floating/floatingpanelview.js deleted file mode 100644 index 5c3f02e0..00000000 --- a/tests/panel/floating/floatingpanelview.js +++ /dev/null @@ -1,229 +0,0 @@ -/** - * @license Copyright (c) 2003-2017, CKSource - Frederico Knabben. All rights reserved. - * For licensing, see LICENSE.md. - */ - -/* global document, Event */ - -import global from '@ckeditor/ckeditor5-utils/src/dom/global'; -import FloatingPanelView from '../../../src/panel/floating/floatingpanelview'; -import View from '../../../src/view'; -import ViewCollection from '../../../src/viewcollection'; -import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils'; -import * as positionUtils from '@ckeditor/ckeditor5-utils/src/dom/position'; - -testUtils.createSinonSandbox(); - -describe( 'FloatingPanelView', () => { - let locale, view, target; - - beforeEach( () => { - locale = {}; - view = new FloatingPanelView( locale ); - - target = global.document.createElement( 'a' ); - - global.document.body.appendChild( view.element ); - global.document.body.appendChild( target ); - - view.targetElement = target; - - return view.init(); - } ); - - afterEach( () => { - view.element.remove(); - - return view.destroy(); - } ); - - describe( 'constructor()', () => { - it( 'should inherit from View', () => { - expect( view ).to.be.instanceOf( View ); - } ); - - it( 'should set view#locale', () => { - expect( view.locale ).to.equal( locale ); - } ); - - it( 'should set #isActive', () => { - expect( view.isActive ).to.be.false; - } ); - - it( 'should set #top', () => { - expect( view.top ).to.equal( 0 ); - } ); - - it( 'should set #left', () => { - expect( view.left ).to.equal( 0 ); - } ); - - it( 'should set #targetElement', () => { - view = new FloatingPanelView( locale ); - - expect( view.targetElement ).to.be.null; - } ); - - it( 'creates view#content collection', () => { - expect( view.content ).to.be.instanceOf( ViewCollection ); - } ); - } ); - - describe( 'template', () => { - it( 'should create element from template', () => { - expect( view.element.classList.contains( 'ck-floating-panel' ) ).to.be.true; - } ); - - describe( 'bindings', () => { - describe( 'class', () => { - it( 'reacts on #isActive', () => { - view.isActive = false; - expect( view.element.classList.contains( 'ck-floating-panel_active' ) ).to.be.false; - - view.isActive = true; - expect( view.element.classList.contains( 'ck-floating-panel_active' ) ).to.be.true; - } ); - } ); - - describe( 'style', () => { - it( 'reacts on #top', () => { - view.top = 30; - expect( view.element.style.top ).to.equal( '30px' ); - } ); - - it( 'reacts on #left', () => { - view.left = 20; - expect( view.element.style.left ).to.equal( '20px' ); - } ); - } ); - - describe( 'children', () => { - it( 'should react on view#content', () => { - expect( view.element.childNodes.length ).to.equal( 0 ); - - const item = new View(); - item.element = document.createElement( 'div' ); - view.content.add( item ); - - expect( view.element.childNodes.length ).to.equal( 1 ); - } ); - } ); - } ); - } ); - - describe( 'init()', () => { - it( 'calls #_updatePosition on window.scroll', () => { - const spy = sinon.spy( view, '_updatePosition' ); - - global.window.dispatchEvent( new Event( 'scroll', { bubbles: true } ) ); - sinon.assert.calledOnce( spy ); - } ); - - it( 'calls #_updatePosition on #change:isActive', () => { - view.isActive = false; - - const spy = sinon.spy( view, '_updatePosition' ); - - view.isActive = true; - sinon.assert.calledOnce( spy ); - - view.isActive = false; - sinon.assert.calledTwice( spy ); - } ); - } ); - - describe( '_updatePosition()', () => { - it( 'does not update when not #isActive', () => { - const spy = testUtils.sinon.spy( positionUtils, 'getOptimalPosition' ); - - view.isActive = false; - - view._updatePosition(); - sinon.assert.notCalled( spy ); - - view.isActive = true; - - view._updatePosition(); - - // Note: #_updatePosition() is called on #change:isActive. - sinon.assert.calledTwice( spy ); - } ); - - it( 'uses getOptimalPosition() utility', () => { - const { nw, sw, ne, se } = FloatingPanelView.defaultPositions; - - view.isActive = true; - - const spy = testUtils.sinon.stub( positionUtils, 'getOptimalPosition' ).returns( { - top: 5, - left: 10 - } ); - - view._updatePosition(); - - sinon.assert.calledWithExactly( spy, { - element: view.element, - target: target, - positions: [ nw, sw, ne, se ], - limiter: global.document.body, - fitInViewport: true - } ); - - expect( view.top ).to.equal( 5 ); - expect( view.left ).to.equal( 10 ); - } ); - } ); - - describe( 'defaultPositions', () => { - let targetRect, panelRect, defaultPositions; - - beforeEach( () => { - defaultPositions = FloatingPanelView.defaultPositions; - - targetRect = { - top: 10, - width: 100, - left: 10, - height: 10, - bottom: 20 - }; - - panelRect = { - width: 20, - height: 10 - }; - } ); - - it( 'should provide "nw" position', () => { - expect( defaultPositions.nw( targetRect, panelRect ) ).to.deep.equal( { - top: 0, - left: 10, - name: 'nw' - } ); - } ); - - it( 'should provide "sw" position', () => { - expect( defaultPositions.sw( targetRect, panelRect ) ).to.deep.equal( { - top: 20, - left: 10, - name: 'sw' - } ); - } ); - - it( 'should provide "ne" position', () => { - expect( defaultPositions.ne( targetRect, panelRect ) ).to.deep.equal( { - top: 0, - left: 90, - name: 'ne' - } ); - } ); - - it( 'should provide "se" position', () => { - expect( defaultPositions.se( targetRect, panelRect ) ).to.deep.equal( { - top: 20, - left: 90, - name: 'se' - } ); - } ); - } ); -} ); From 060afa3a982920ad3aa0d15ecf6ad75b4ca1b722 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Mon, 10 Apr 2017 11:45:52 +0200 Subject: [PATCH 07/12] Fixed: BalloonPanelView#pin and BalloonPanelView#unpin should not control the visibility of the panel. --- src/panel/balloon/balloonpanelview.js | 20 +++++++++--------- tests/manual/tickets/170/1.js | 1 + tests/panel/balloon/balloonpanelview.js | 27 ++++++++++++++++++++++--- 3 files changed, 35 insertions(+), 13 deletions(-) diff --git a/src/panel/balloon/balloonpanelview.js b/src/panel/balloon/balloonpanelview.js index fd0a4777..569fc892 100644 --- a/src/panel/balloon/balloonpanelview.js +++ b/src/panel/balloon/balloonpanelview.js @@ -95,7 +95,7 @@ export default class BalloonPanelView extends View { * `true`. Used by {@link #pin}. * * @private - * @member {Function} #_pinWhenVisibleCallback + * @member {Function} #_pinWhenIsVisibleCallback */ /** @@ -197,7 +197,7 @@ export default class BalloonPanelView extends View { pin( options ) { this.unpin(); - this._pinWhenVisibleCallback = () => { + this._pinWhenIsVisibleCallback = () => { if ( this.isVisible ) { this._startPinning( options ); } else { @@ -205,7 +205,7 @@ export default class BalloonPanelView extends View { } }; - // If the panel is already visible, enable the listeners immediately. + // If the panel is already visible, start pinning immediately. if ( this.isVisible ) { this._startPinning( options ); } @@ -213,23 +213,23 @@ export default class BalloonPanelView extends View { // Control the state of the listeners depending on whether the panel is visible // or not. // TODO: Use on() (https://github.com/ckeditor/ckeditor5-utils/issues/144). - this.listenTo( this, 'change:isVisible', this._pinWhenVisibleCallback ); + this.listenTo( this, 'change:isVisible', this._pinWhenIsVisibleCallback ); } /** * Stops pinning the panel, as set up by {@link #pin}. */ unpin() { - // Deactivate listeners attached by pin(). - this.stopListening( global.document, 'scroll' ); - this.stopListening( global.window, 'resize' ); + if ( this._pinWhenIsVisibleCallback ) { + // Deactivate listeners attached by pin(). + this.stopListening( global.document, 'scroll' ); + this.stopListening( global.window, 'resize' ); - if ( this._pinWhenVisibleCallback ) { // Deactivate the panel pin() control logic. // TODO: Use off() (https://github.com/ckeditor/ckeditor5-utils/issues/144). - this.stopListening( this, 'change:isVisible', this._pinWhenVisibleCallback ); + this.stopListening( this, 'change:isVisible', this._pinWhenIsVisibleCallback ); - this._pinWhenVisibleCallback = null; + this._pinWhenIsVisibleCallback = null; } } diff --git a/tests/manual/tickets/170/1.js b/tests/manual/tickets/170/1.js index ac00b1d7..f2b918a5 100644 --- a/tests/manual/tickets/170/1.js +++ b/tests/manual/tickets/170/1.js @@ -52,6 +52,7 @@ ClassicEditor.create( document.querySelector( '#editor-stick' ), { editor.ui.view.element.querySelector( '.ck-editor__editable' ).scrollTop = 360; panel.init().then( () => { + panel.show(); panel.pin( { target: editor.ui.view.element.querySelector( '.ck-editor__editable p strong' ), limiter: editor.ui.view.editableElement diff --git a/tests/panel/balloon/balloonpanelview.js b/tests/panel/balloon/balloonpanelview.js index 963f83a1..864cf75c 100644 --- a/tests/panel/balloon/balloonpanelview.js +++ b/tests/panel/balloon/balloonpanelview.js @@ -437,14 +437,26 @@ describe( 'BalloonPanelView', () => { } ); describe( 'pin()', () => { - it( 'should not pin until the balloon gets visible', () => { + it( 'should not show the balloon', () => { + const showSpy = sinon.spy( view, 'show' ); + view.hide(); view.pin( { target, limiter } ); - sinon.assert.notCalled( attachToSpy ); + sinon.assert.notCalled( showSpy ); + } ); + + it( 'should start pinning when the balloon is visible', () => { + view.hide(); + view.pin( { target, limiter } ); + + targetParent.dispatchEvent( new Event( 'scroll' ) ); view.show(); - sinon.assert.calledOnce( attachToSpy ); + + targetParent.dispatchEvent( new Event( 'scroll' ) ); + + sinon.assert.calledTwice( attachToSpy ); } ); it( 'should stop pinning when the balloon becomes invisible', () => { @@ -582,6 +594,15 @@ describe( 'BalloonPanelView', () => { } ); describe( 'unpin()', () => { + it( 'should not hide the balloon if pinned', () => { + const hideSpy = sinon.spy( view, 'hide' ); + + view.pin( { target, limiter } ); + view.unpin(); + + sinon.assert.notCalled( hideSpy ); + } ); + it( 'should stop attaching', () => { view.pin( { target, limiter } ); sinon.assert.calledOnce( attachToSpy ); From f3d1011a7e44c532702bf409f3811b1089385b5d Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Mon, 10 Apr 2017 12:12:33 +0200 Subject: [PATCH 08/12] Code refactoring. Tests for BalloonPanelView#withArrow. --- src/panel/balloon/balloonpanelview.js | 6 ++++-- tests/manual/contextualtoolbar/contextualtoolbar.js | 4 ++-- tests/panel/balloon/balloonpanelview.js | 9 +++++++++ theme/components/panel/balloonpanel.scss | 4 +++- 4 files changed, 18 insertions(+), 5 deletions(-) diff --git a/src/panel/balloon/balloonpanelview.js b/src/panel/balloon/balloonpanelview.js index 569fc892..f525fcc8 100644 --- a/src/panel/balloon/balloonpanelview.js +++ b/src/panel/balloon/balloonpanelview.js @@ -59,8 +59,10 @@ export default class BalloonPanelView extends View { * Default position names correspond with * {@link module:ui/panel/balloon/balloonpanelview~BalloonPanelView.defaultPositions}. * + * See {@link #withArrow}. + * * @observable - * @default 'se' + * @default 'arrow_se' * @member {'arrow_se'|'arrow_sw'|'arrow_ne'|'arrow_nw'} #position */ this.set( 'position', 'arrow_se' ); @@ -113,7 +115,7 @@ export default class BalloonPanelView extends View { 'ck-balloon-panel', bind.to( 'position', ( value ) => `ck-balloon-panel_${ value }` ), bind.if( 'isVisible', 'ck-balloon-panel_visible' ), - bind.if( 'withArrow', 'ck-balloon-panel_arrow' ) + bind.if( 'withArrow', 'ck-balloon-panel_with-arrow' ) ], style: { diff --git a/tests/manual/contextualtoolbar/contextualtoolbar.js b/tests/manual/contextualtoolbar/contextualtoolbar.js index ba6cbf54..414152f3 100644 --- a/tests/manual/contextualtoolbar/contextualtoolbar.js +++ b/tests/manual/contextualtoolbar/contextualtoolbar.js @@ -28,7 +28,7 @@ const positions = { forwardSelection: ( targetRect, balloonRect ) => ( { top: targetRect.bottom + arrowVOffset, left: targetRect.right - balloonRect.width / 2, - name: 's' + name: 'arrow_s' } ), // +-----------------+ @@ -39,7 +39,7 @@ const positions = { backwardSelection: ( targetRect, balloonRect ) => ( { top: targetRect.top - balloonRect.height - arrowVOffset, left: targetRect.left - balloonRect.width / 2, - name: 'n' + name: 'arrow_n' } ) }; diff --git a/tests/panel/balloon/balloonpanelview.js b/tests/panel/balloon/balloonpanelview.js index 864cf75c..f9cc0edb 100644 --- a/tests/panel/balloon/balloonpanelview.js +++ b/tests/panel/balloon/balloonpanelview.js @@ -43,6 +43,7 @@ describe( 'BalloonPanelView', () => { expect( view.left ).to.equal( 0 ); expect( view.position ).to.equal( 'arrow_se' ); expect( view.isVisible ).to.equal( false ); + expect( view.withArrow ).to.equal( true ); } ); it( 'creates view#content collection', () => { @@ -59,6 +60,14 @@ describe( 'BalloonPanelView', () => { expect( view.element.classList.contains( 'ck-balloon-panel_arrow_sw' ) ).to.true; } ); + + it( 'should react on view#withArrow', () => { + expect( view.element.classList.contains( 'ck-balloon-panel_with-arrow' ) ).to.be.true; + + view.withArrow = false; + + expect( view.element.classList.contains( 'ck-balloon-panel_with-arrow' ) ).to.be.false; + } ); } ); describe( 'isVisible', () => { diff --git a/theme/components/panel/balloonpanel.scss b/theme/components/panel/balloonpanel.scss index 4d572655..ca578498 100644 --- a/theme/components/panel/balloonpanel.scss +++ b/theme/components/panel/balloonpanel.scss @@ -9,7 +9,7 @@ z-index: ck-z( 'modal' ); - &.ck-balloon-panel_arrow { + &.ck-balloon-panel_with-arrow { &:before, &:after { content: ""; @@ -23,7 +23,9 @@ &:after { z-index: ck-z( 'default', 1 ); } + } + &.ck-balloon-panel_arrow { &_s, &_se, &_sw { From fab94700d74c9f13db831af51061c47e9907f1b9 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Mon, 10 Apr 2017 12:23:24 +0200 Subject: [PATCH 09/12] Docs: Extended BalloonPanelView docs. --- src/panel/balloon/balloonpanelview.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/panel/balloon/balloonpanelview.js b/src/panel/balloon/balloonpanelview.js index f525fcc8..4e8aa983 100644 --- a/src/panel/balloon/balloonpanelview.js +++ b/src/panel/balloon/balloonpanelview.js @@ -52,13 +52,15 @@ export default class BalloonPanelView extends View { /** * Balloon panel's current position. The position name is reflected in the CSS class set - * to the balloon, i.e. `.ck-balloon-panel_arrow_se` for "se" position. The class + * to the balloon, i.e. `.ck-balloon-panel_arrow_se` for "arrow_se" position. The class * controls the minor aspects of the balloon's visual appearance like placement * of the "arrow". To support a new position, an additional CSS must be created. * * Default position names correspond with * {@link module:ui/panel/balloon/balloonpanelview~BalloonPanelView.defaultPositions}. * + * See {@link #attachTo} to learn about custom balloon positions. + * * See {@link #withArrow}. * * @observable @@ -77,7 +79,8 @@ export default class BalloonPanelView extends View { this.set( 'isVisible', false ); /** - * Controls whether the balloon panel has an arrow. + * Controls whether the balloon panel has an arrow. The presence of the arrow + * is reflected in `ck-balloon-panel_with-arrow` CSS class. * * @observable * @default true From 6833a9e6706754872fc8109f1f0dc3238ae0ad1e Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Mon, 10 Apr 2017 12:43:25 +0200 Subject: [PATCH 10/12] Tests: Updated ContextualBalloon manual test after BalloonPanelView api changes. --- tests/manual/contextualballoon/contextualballoon.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/manual/contextualballoon/contextualballoon.js b/tests/manual/contextualballoon/contextualballoon.js index 7ecbc4e5..c3697aa8 100644 --- a/tests/manual/contextualballoon/contextualballoon.js +++ b/tests/manual/contextualballoon/contextualballoon.js @@ -230,13 +230,13 @@ function createContextualToolbar( editor ) { forwardSelection: ( targetRect, balloonRect ) => ( { top: targetRect.bottom + arrowVOffset, left: targetRect.right - balloonRect.width / 2, - name: 's' + name: 'arrow_s' } ), backwardSelection: ( targetRect, balloonRect ) => ( { top: targetRect.top - balloonRect.height - arrowVOffset, left: targetRect.left - balloonRect.width / 2, - name: 'n' + name: 'arrow_n' } ) }; From 6f93201998675e875adbe3a07ebaecdf1e8f290f Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Mon, 10 Apr 2017 12:57:37 +0200 Subject: [PATCH 11/12] Used #_stopPinning in #unpin. --- src/panel/balloon/balloonpanelview.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/panel/balloon/balloonpanelview.js b/src/panel/balloon/balloonpanelview.js index 4e8aa983..4f4b5a34 100644 --- a/src/panel/balloon/balloonpanelview.js +++ b/src/panel/balloon/balloonpanelview.js @@ -227,8 +227,7 @@ export default class BalloonPanelView extends View { unpin() { if ( this._pinWhenIsVisibleCallback ) { // Deactivate listeners attached by pin(). - this.stopListening( global.document, 'scroll' ); - this.stopListening( global.window, 'resize' ); + this._stopPinning(); // Deactivate the panel pin() control logic. // TODO: Use off() (https://github.com/ckeditor/ckeditor5-utils/issues/144). From 6fd3276d8c30ff2e1905a6e8518258bbc7e05373 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Mon, 10 Apr 2017 13:52:21 +0200 Subject: [PATCH 12/12] BalloonPanelView#pin should show the balloon and BalloonPanelView#unpin should hide it. --- src/panel/balloon/balloonpanelview.js | 7 +++---- tests/manual/tickets/170/1.js | 1 - tests/panel/balloon/balloonpanelview.js | 19 ++++++++++--------- 3 files changed, 13 insertions(+), 14 deletions(-) diff --git a/src/panel/balloon/balloonpanelview.js b/src/panel/balloon/balloonpanelview.js index 4f4b5a34..a2529457 100644 --- a/src/panel/balloon/balloonpanelview.js +++ b/src/panel/balloon/balloonpanelview.js @@ -210,10 +210,7 @@ export default class BalloonPanelView extends View { } }; - // If the panel is already visible, start pinning immediately. - if ( this.isVisible ) { - this._startPinning( options ); - } + this._startPinning( options ); // Control the state of the listeners depending on whether the panel is visible // or not. @@ -234,6 +231,8 @@ export default class BalloonPanelView extends View { this.stopListening( this, 'change:isVisible', this._pinWhenIsVisibleCallback ); this._pinWhenIsVisibleCallback = null; + + this.hide(); } } diff --git a/tests/manual/tickets/170/1.js b/tests/manual/tickets/170/1.js index f2b918a5..ac00b1d7 100644 --- a/tests/manual/tickets/170/1.js +++ b/tests/manual/tickets/170/1.js @@ -52,7 +52,6 @@ ClassicEditor.create( document.querySelector( '#editor-stick' ), { editor.ui.view.element.querySelector( '.ck-editor__editable' ).scrollTop = 360; panel.init().then( () => { - panel.show(); panel.pin( { target: editor.ui.view.element.querySelector( '.ck-editor__editable p strong' ), limiter: editor.ui.view.editableElement diff --git a/tests/panel/balloon/balloonpanelview.js b/tests/panel/balloon/balloonpanelview.js index f9cc0edb..00bfb396 100644 --- a/tests/panel/balloon/balloonpanelview.js +++ b/tests/panel/balloon/balloonpanelview.js @@ -446,26 +446,27 @@ describe( 'BalloonPanelView', () => { } ); describe( 'pin()', () => { - it( 'should not show the balloon', () => { - const showSpy = sinon.spy( view, 'show' ); + it( 'should show the balloon', () => { + const spy = sinon.spy( view, 'show' ); view.hide(); view.pin( { target, limiter } ); - sinon.assert.notCalled( showSpy ); + sinon.assert.calledOnce( spy ); } ); it( 'should start pinning when the balloon is visible', () => { - view.hide(); view.pin( { target, limiter } ); + sinon.assert.calledOnce( attachToSpy ); + view.hide(); targetParent.dispatchEvent( new Event( 'scroll' ) ); view.show(); + sinon.assert.calledTwice( attachToSpy ); targetParent.dispatchEvent( new Event( 'scroll' ) ); - - sinon.assert.calledTwice( attachToSpy ); + sinon.assert.calledThrice( attachToSpy ); } ); it( 'should stop pinning when the balloon becomes invisible', () => { @@ -603,13 +604,13 @@ describe( 'BalloonPanelView', () => { } ); describe( 'unpin()', () => { - it( 'should not hide the balloon if pinned', () => { - const hideSpy = sinon.spy( view, 'hide' ); + it( 'should hide the balloon if pinned', () => { + const spy = sinon.spy( view, 'hide' ); view.pin( { target, limiter } ); view.unpin(); - sinon.assert.notCalled( hideSpy ); + sinon.assert.calledOnce( spy ); } ); it( 'should stop attaching', () => {