From c51b3c05e600c61988179c18c83f6b2b9c27d9e3 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Wed, 24 May 2017 11:27:09 +0200 Subject: [PATCH 1/5] Improved the naming of BalloonPanelView.defaultPositions, added new positions. --- src/panel/balloon/balloonpanelview.js | 315 +++++++++++++----- .../panel/balloon/balloonpanelview.html | 22 ++ .../manual/panel/balloon/balloonpanelview.js | 29 ++ .../manual/panel/balloon/balloonpanelview.md | 5 + tests/panel/balloon/balloonpanelview.js | 156 ++++++--- 5 files changed, 400 insertions(+), 127 deletions(-) create mode 100644 tests/manual/panel/balloon/balloonpanelview.html create mode 100644 tests/manual/panel/balloon/balloonpanelview.js create mode 100644 tests/manual/panel/balloon/balloonpanelview.md diff --git a/src/panel/balloon/balloonpanelview.js b/src/panel/balloon/balloonpanelview.js index bfb67d47..08de1351 100644 --- a/src/panel/balloon/balloonpanelview.js +++ b/src/panel/balloon/balloonpanelview.js @@ -175,10 +175,10 @@ export default class BalloonPanelView extends View { const positionOptions = Object.assign( {}, { element: this.element, positions: [ - defaultPositions.southEastArrowNorthEast, - defaultPositions.southWestArrowNorthEast, - defaultPositions.northEastArrowSouthWest, - defaultPositions.northWestArrowSouthEast + defaultPositions.southArrowNorthWest, + defaultPositions.southArrowNorthEast, + defaultPositions.northArrowSouthWest, + defaultPositions.northArrowSouthEast ], limiter: defaultLimiterElement, fitInViewport: true @@ -334,94 +334,163 @@ BalloonPanelView.arrowVerticalOffset = 15; * A default set of positioning functions used by the balloon panel view * when attaching using {@link module:ui/panel/balloon/balloonpanelview~BalloonPanelView#attachTo} method. * - * The available positioning functions are as follows: + * The available positioning functions are as follow: * - * * South east (arrow north west): + * **North** * - * [ Target ] - * ^ - * +-----------------+ - * | Balloon | - * +-----------------+ - * - * - * * South west (arrow north east): - * - * [ Target ] - * ^ - * +-----------------+ - * | Balloon | - * +-----------------+ - * - * - * * North east (arrow south west): - * - * +-----------------+ - * | Balloon | - * +-----------------+ - * V - * [ Target ] - * - * - * * North west (arrow south east): - * - * +-----------------+ - * | Balloon | - * +-----------------+ - * V - * [ Target ] + * * `northArrowSouth` * + * +-----------------+ + * | Balloon | + * +-----------------+ + * V + * [ Target ] * - * * South east (arrow north): - * - * [ Target ] - * ^ - * +-----------------+ - * | Balloon | - * +-----------------+ + * * `northArrowSouthEast` * + * +-----------------+ + * | Balloon | + * +-----------------+ + * V + * [ Target ] * - * * North east (arrow south): + * * `northArrowSouthWest` * * +-----------------+ * | Balloon | * +-----------------+ - * V - * [ Target ] + * V + * [ Target ] * + * **North west** * - * * North west (arrow south): + * * `northWestArrowSouth` * * +-----------------+ * | Balloon | * +-----------------+ * V - * [ Target ] + * [ Target ] * + * * `northWestArrowSouthWest` * - * * South west (arrow north): + * +-----------------+ + * | Balloon | + * +-----------------+ + * V + * [ Target ] + * + * * `northWestArrowSouthEast` * - * [ Target ] - * ^ * +-----------------+ * | Balloon | * +-----------------+ + * V + * [ Target ] + * + * **North east** * - * * South (arrow north): + * * `northEastArrowSouth` * - * [ Target ] - * ^ * +-----------------+ * | Balloon | * +-----------------+ + * V + * [ Target ] * - * * North (arrow south): + * * `northEastArrowSouthEast` * * +-----------------+ * | Balloon | * +-----------------+ - * V - * [ Target ] + * V + * [ Target ] + * + * * `northEastArrowSouthWest` + * + * +-----------------+ + * | Balloon | + * +-----------------+ + * V + * [ Target ] + * + * **South** + * + * * `southArrowNorth` + * + * [ Target ] + * ^ + * +-----------------+ + * | Balloon | + * +-----------------+ + * + * * `southArrowNorthEast` + * + * [ Target ] + * ^ + * +-----------------+ + * | Balloon | + * +-----------------+ + * + * * `southArrowNorthWest` + * + * [ Target ] + * ^ + * +-----------------+ + * | Balloon | + * +-----------------+ + * + * **South west** + * + * * `southWestArrowNorth` + * + * [ Target ] + * ^ + * +-----------------+ + * | Balloon | + * +-----------------+ + * + * * `southWestArrowNorthWest` + * + * [ Target ] + * ^ + * +-----------------+ + * | Balloon | + * +-----------------+ + * + * * `southWestArrowNorthEast` + * + * [ Target ] + * ^ + * +-----------------+ + * | Balloon | + * +-----------------+ + * + * **South east** + * + * * `southEastArrowNorth` + * + * [ Target ] + * ^ + * +-----------------+ + * | Balloon | + * +-----------------+ + * + * * `southEastArrowNorthEast` + * + * [ Target ] + * ^ + * +-----------------+ + * | Balloon | + * +-----------------+ + * + * * `southEastArrowNorthWest` + * + * [ Target ] + * ^ + * +-----------------+ + * | Balloon | + * +-----------------+ * * See {@link module:ui/panel/balloon/balloonpanelview~BalloonPanelView#attachTo}. * @@ -433,63 +502,131 @@ BalloonPanelView.arrowVerticalOffset = 15; * @member {Object} module:ui/panel/balloon/balloonpanelview~BalloonPanelView.defaultPositions */ BalloonPanelView.defaultPositions = { - southEastArrowNorthEast: targetRect => ( { - top: targetRect.bottom + BalloonPanelView.arrowVerticalOffset, - left: targetRect.left + targetRect.width / 2 - BalloonPanelView.arrowHorizontalOffset, - name: 'arrow_ne' + // ------- North + + northArrowSouth: ( targetRect, balloonRect ) => ( { + top: getNorthTop( targetRect, balloonRect ), + left: targetRect.left + targetRect.width / 2 - balloonRect.width / 2, + name: 'arrow_s' } ), - southWestArrowNorthEast: ( targetRect, balloonRect ) => ( { - top: targetRect.bottom + BalloonPanelView.arrowVerticalOffset, + northArrowSouthEast: ( targetRect, balloonRect ) => ( { + top: getNorthTop( targetRect, balloonRect ), left: targetRect.left + targetRect.width / 2 - balloonRect.width + BalloonPanelView.arrowHorizontalOffset, - name: 'arrow_nw' + name: 'arrow_sw' } ), - northEastArrowSouthWest: ( targetRect, balloonRect ) => ( { - top: targetRect.top - balloonRect.height - BalloonPanelView.arrowVerticalOffset, + northArrowSouthWest: ( targetRect, balloonRect ) => ( { + top: getNorthTop( targetRect, balloonRect ), left: targetRect.left + targetRect.width / 2 - BalloonPanelView.arrowHorizontalOffset, name: 'arrow_se' } ), + // ------- North west + + northWestArrowSouth: ( targetRect, balloonRect ) => ( { + top: getNorthTop( targetRect, balloonRect ), + left: targetRect.left - balloonRect.width / 2, + name: 'arrow_s' + } ), + + northWestArrowSouthWest: ( targetRect, balloonRect ) => ( { + top: getNorthTop( targetRect, balloonRect ), + left: targetRect.left - BalloonPanelView.arrowHorizontalOffset, + name: 'arrow_se' + } ), + northWestArrowSouthEast: ( targetRect, balloonRect ) => ( { - top: targetRect.top - balloonRect.height - BalloonPanelView.arrowVerticalOffset, - left: targetRect.left + targetRect.width / 2 - balloonRect.width + BalloonPanelView.arrowHorizontalOffset, + top: getNorthTop( targetRect, balloonRect ), + left: targetRect.left - balloonRect.width + BalloonPanelView.arrowHorizontalOffset, name: 'arrow_sw' } ), - southEastArrowNorth: ( targetRect, balloonRect ) => ( { - top: targetRect.bottom + BalloonPanelView.arrowVerticalOffset, - left: targetRect.right - balloonRect.width / 2, - name: 'arrow_n' - } ), + // ------- North east northEastArrowSouth: ( targetRect, balloonRect ) => ( { - top: targetRect.top - balloonRect.height - BalloonPanelView.arrowVerticalOffset, + top: getNorthTop( targetRect, balloonRect ), left: targetRect.right - balloonRect.width / 2, name: 'arrow_s' } ), - northWestArrowSouth: ( targetRect, balloonRect ) => ( { - top: targetRect.top - balloonRect.height - BalloonPanelView.arrowVerticalOffset, - left: targetRect.left - balloonRect.width / 2, - name: 'arrow_s' + northEastArrowSouthEast: ( targetRect, balloonRect ) => ( { + top: getNorthTop( targetRect, balloonRect ), + left: targetRect.right - balloonRect.width + BalloonPanelView.arrowHorizontalOffset, + name: 'arrow_sw' + } ), + + northEastArrowSouthWest: ( targetRect, balloonRect ) => ( { + top: getNorthTop( targetRect, balloonRect ), + left: targetRect.right - BalloonPanelView.arrowHorizontalOffset, + name: 'arrow_se' } ), + // ------- South + + southArrowNorth: ( targetRect, balloonRect ) => ( { + top: getSouthTop( targetRect, balloonRect ), + left: targetRect.left + targetRect.width / 2 - balloonRect.width / 2, + name: 'arrow_n' + } ), + + southArrowNorthEast: ( targetRect, balloonRect ) => ( { + top: getSouthTop( targetRect, balloonRect ), + left: targetRect.left + targetRect.width / 2 - balloonRect.width + BalloonPanelView.arrowHorizontalOffset, + name: 'arrow_nw' + } ), + + southArrowNorthWest: ( targetRect, balloonRect ) => ( { + top: getSouthTop( targetRect, balloonRect ), + left: targetRect.left + targetRect.width / 2 - BalloonPanelView.arrowHorizontalOffset, + name: 'arrow_ne' + } ), + + // ------- South west + southWestArrowNorth: ( targetRect, balloonRect ) => ( { - top: targetRect.bottom + BalloonPanelView.arrowVerticalOffset, + top: getSouthTop( targetRect, balloonRect ), left: targetRect.left - balloonRect.width / 2, name: 'arrow_n' } ), - southArrowNorth: ( targetRect, balloonRect ) => ( { - top: targetRect.bottom + BalloonPanelView.arrowVerticalOffset, - left: targetRect.left + targetRect.width / 2 - balloonRect.width / 2, + southWestArrowNorthWest: ( targetRect, balloonRect ) => ( { + top: getSouthTop( targetRect, balloonRect ), + left: targetRect.left - BalloonPanelView.arrowHorizontalOffset, + name: 'arrow_ne' + } ), + + southWestArrowNorthEast: ( targetRect, balloonRect ) => ( { + top: getSouthTop( targetRect, balloonRect ), + left: targetRect.left - balloonRect.width + BalloonPanelView.arrowHorizontalOffset, + name: 'arrow_nw' + } ), + + // ------- South east + + southEastArrowNorth: ( targetRect, balloonRect ) => ( { + top: getSouthTop( targetRect, balloonRect ), + left: targetRect.right - balloonRect.width / 2, name: 'arrow_n' } ), - northArrowSouth: ( targetRect, balloonRect ) => ( { - top: targetRect.top - balloonRect.height - BalloonPanelView.arrowVerticalOffset, - left: targetRect.left + targetRect.width / 2 - balloonRect.width / 2, - name: 'arrow_s' - } ) + southEastArrowNorthEast: ( targetRect, balloonRect ) => ( { + top: getSouthTop( targetRect, balloonRect ), + left: targetRect.right - balloonRect.width + BalloonPanelView.arrowHorizontalOffset, + name: 'arrow_nw' + } ), + + southEastArrowNorthWest: ( targetRect, balloonRect ) => ( { + top: getSouthTop( targetRect, balloonRect ), + left: targetRect.right - BalloonPanelView.arrowHorizontalOffset, + name: 'arrow_ne' + } ), }; + +function getNorthTop( targetRect, balloonRect ) { + return targetRect.top - balloonRect.height - BalloonPanelView.arrowVerticalOffset; +} + +function getSouthTop( targetRect ) { + return targetRect.bottom + BalloonPanelView.arrowVerticalOffset; +} diff --git a/tests/manual/panel/balloon/balloonpanelview.html b/tests/manual/panel/balloon/balloonpanelview.html new file mode 100644 index 00000000..eb7bca89 --- /dev/null +++ b/tests/manual/panel/balloon/balloonpanelview.html @@ -0,0 +1,22 @@ +
+ + diff --git a/tests/manual/panel/balloon/balloonpanelview.js b/tests/manual/panel/balloon/balloonpanelview.js new file mode 100644 index 00000000..d2c299b4 --- /dev/null +++ b/tests/manual/panel/balloon/balloonpanelview.js @@ -0,0 +1,29 @@ +/** + * @license Copyright (c) 2003-2017, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md. + */ + +/* globals document */ + +import BalloonPanelView from '../../../../src/panel/balloon/balloonpanelview'; +import '@ckeditor/ckeditor5-theme-lark/theme/theme.scss'; + +const defaultPositions = BalloonPanelView.defaultPositions; +const container = document.querySelector( '#container' ); + +for ( const i in defaultPositions ) { + const target = document.createElement( 'div' ); + target.classList.add( 'target' ); + container.appendChild( target ); + + const balloon = new BalloonPanelView(); + balloon.element.textContent = i; + document.body.appendChild( balloon.element ); + + balloon.attachTo( { + target, + positions: [ + defaultPositions[ i ] + ] + } ); +} diff --git a/tests/manual/panel/balloon/balloonpanelview.md b/tests/manual/panel/balloon/balloonpanelview.md new file mode 100644 index 00000000..980f663e --- /dev/null +++ b/tests/manual/panel/balloon/balloonpanelview.md @@ -0,0 +1,5 @@ +## `BalloonPanelView` and `defaultPositions` + +1. A number of green rectangles should be displayed in the page. +2. Each rectangle should have a panel attached. +3. Make sure the description in each panel matches the location of the panel. diff --git a/tests/panel/balloon/balloonpanelview.js b/tests/panel/balloon/balloonpanelview.js index 417acd53..27750cb9 100644 --- a/tests/panel/balloon/balloonpanelview.js +++ b/tests/panel/balloon/balloonpanelview.js @@ -188,10 +188,10 @@ describe( 'BalloonPanelView', () => { element: view.element, target, positions: [ - BalloonPanelView.defaultPositions.southEastArrowNorthEast, - BalloonPanelView.defaultPositions.southWestArrowNorthEast, - BalloonPanelView.defaultPositions.northEastArrowSouthWest, - BalloonPanelView.defaultPositions.northWestArrowSouthEast + BalloonPanelView.defaultPositions.southArrowNorthWest, + BalloonPanelView.defaultPositions.southArrowNorthEast, + BalloonPanelView.defaultPositions.northArrowSouthWest, + BalloonPanelView.defaultPositions.northArrowSouthEast ], limiter: document.body, fitInViewport: true @@ -670,47 +670,65 @@ describe( 'BalloonPanelView', () => { }; } ); - it( 'southEastArrowNorthEast', () => { - expect( positions.southEastArrowNorthEast( targetRect ) ).to.deep.equal( { - top: 215, - left: 120, - name: 'arrow_ne' + it( 'should have a proper length', () => { + expect( Object.keys( positions ) ).to.have.length( 18 ); + } ); + + // ------- North + + it( 'should define the "northArrowSouth" position', () => { + expect( positions.northArrowSouth( targetRect, balloonRect ) ).to.deep.equal( { + top: 35, + left: 125, + name: 'arrow_s' } ); } ); - it( 'southWestArrowNorthEast', () => { - expect( positions.southWestArrowNorthEast( targetRect, balloonRect ) ).to.deep.equal( { - top: 215, + it( 'should define the "northArrowSouthEast" position', () => { + expect( positions.northArrowSouthEast( targetRect, balloonRect ) ).to.deep.equal( { + top: 35, left: 130, - name: 'arrow_nw' + name: 'arrow_sw' } ); } ); - it( 'northEastArrowSouthWest', () => { - expect( positions.northEastArrowSouthWest( targetRect, balloonRect ) ).to.deep.equal( { + it( 'should define the "northArrowSouthWest" position', () => { + expect( positions.northArrowSouthWest( targetRect, balloonRect ) ).to.deep.equal( { top: 35, left: 120, name: 'arrow_se' } ); } ); - it( 'northWestArrowSouthEast', () => { - expect( positions.northWestArrowSouthEast( targetRect, balloonRect ) ).to.deep.equal( { + // ------- North west + + it( 'should define the "northWestArrowSouth" position', () => { + expect( positions.northWestArrowSouth( targetRect, balloonRect ) ).to.deep.equal( { top: 35, - left: 130, - name: 'arrow_sw' + left: 75, + name: 'arrow_s' } ); } ); - it( 'southEastArrowNorth', () => { - expect( positions.southEastArrowNorth( targetRect, balloonRect ) ).to.deep.equal( { - top: 215, - left: 175, - name: 'arrow_n' + it( 'should define the "northWestArrowSouthWest" position', () => { + expect( positions.northWestArrowSouthWest( targetRect, balloonRect ) ).to.deep.equal( { + top: 35, + left: 70, + name: 'arrow_se' + } ); + } ); + + it( 'should define the "northWestArrowSouthEast" position', () => { + expect( positions.northWestArrowSouthEast( targetRect, balloonRect ) ).to.deep.equal( { + top: 35, + left: 80, + name: 'arrow_sw' } ); } ); - it( 'northEastArrowSouth', () => { + // ------- North east + + it( 'should define the "northEastArrowSouth" position', () => { expect( positions.northEastArrowSouth( targetRect, balloonRect ) ).to.deep.equal( { top: 35, left: 175, @@ -718,15 +736,51 @@ describe( 'BalloonPanelView', () => { } ); } ); - it( 'northWestArrowSouth', () => { - expect( positions.northWestArrowSouth( targetRect, balloonRect ) ).to.deep.equal( { + it( 'should define the "northEastArrowSouthEast" position', () => { + expect( positions.northEastArrowSouthEast( targetRect, balloonRect ) ).to.deep.equal( { top: 35, - left: 75, - name: 'arrow_s' + left: 180, + name: 'arrow_sw' + } ); + } ); + + it( 'should define the "northEastArrowSouthWest" position', () => { + expect( positions.northEastArrowSouthWest( targetRect, balloonRect ) ).to.deep.equal( { + top: 35, + left: 170, + name: 'arrow_se' + } ); + } ); + + // ------- South + + it( 'should define the "southArrowNorth" position', () => { + expect( positions.southArrowNorth( targetRect, balloonRect ) ).to.deep.equal( { + top: 215, + left: 125, + name: 'arrow_n' + } ); + } ); + + it( 'should define the "southArrowNorthEast" position', () => { + expect( positions.southArrowNorthEast( targetRect, balloonRect ) ).to.deep.equal( { + top: 215, + left: 130, + name: 'arrow_nw' + } ); + } ); + + it( 'should define the "southArrowNorthWest" position', () => { + expect( positions.southArrowNorthWest( targetRect, balloonRect ) ).to.deep.equal( { + top: 215, + left: 120, + name: 'arrow_ne' } ); } ); - it( 'southWestArrowNorth', () => { + // ------- South west + + it( 'should define the "southWestArrowNorth" position', () => { expect( positions.southWestArrowNorth( targetRect, balloonRect ) ).to.deep.equal( { top: 215, left: 75, @@ -734,19 +788,45 @@ describe( 'BalloonPanelView', () => { } ); } ); - it( 'southArrowNorth', () => { - expect( positions.southArrowNorth( targetRect, balloonRect ) ).to.deep.equal( { + it( 'should define the "southWestArrowNorthWest" position', () => { + expect( positions.southWestArrowNorthWest( targetRect, balloonRect ) ).to.deep.equal( { top: 215, - left: 125, + left: 70, + name: 'arrow_ne' + } ); + } ); + + it( 'should define the "southWestArrowNorthEast" position', () => { + expect( positions.southWestArrowNorthEast( targetRect, balloonRect ) ).to.deep.equal( { + top: 215, + left: 80, + name: 'arrow_nw' + } ); + } ); + + // ------- South east + + it( 'should define the "southEastArrowNorth" position', () => { + expect( positions.southEastArrowNorth( targetRect, balloonRect ) ).to.deep.equal( { + top: 215, + left: 175, name: 'arrow_n' } ); } ); - it( 'northArrowSouth', () => { - expect( positions.northArrowSouth( targetRect, balloonRect ) ).to.deep.equal( { - top: 35, - left: 125, - name: 'arrow_s' + it( 'should define the "southEastArrowNorthEast" position', () => { + expect( positions.southEastArrowNorthEast( targetRect, balloonRect ) ).to.deep.equal( { + top: 215, + left: 180, + name: 'arrow_nw' + } ); + } ); + + it( 'should define the "southEastArrowNorthWest" position', () => { + expect( positions.southEastArrowNorthWest( targetRect, balloonRect ) ).to.deep.equal( { + top: 215, + left: 170, + name: 'arrow_ne' } ); } ); } ); From 0757d520f3afb99adb23a732e35bf44e292f979e Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Wed, 24 May 2017 11:29:48 +0200 Subject: [PATCH 2/5] Docs: Added missing documentation for private helpers. --- src/panel/balloon/balloonpanelview.js | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/panel/balloon/balloonpanelview.js b/src/panel/balloon/balloonpanelview.js index 08de1351..0fef639b 100644 --- a/src/panel/balloon/balloonpanelview.js +++ b/src/panel/balloon/balloonpanelview.js @@ -623,10 +623,22 @@ BalloonPanelView.defaultPositions = { } ), }; +// Returns the top coordinate for positions starting with `north*`. +// +// @private +// @param {utils/dom/rect~Rect} targetRect A rect of the target. +// @param {utils/dom/rect~Rect} elementRect A rect of the balloon. +// @returns {Number} function getNorthTop( targetRect, balloonRect ) { return targetRect.top - balloonRect.height - BalloonPanelView.arrowVerticalOffset; } +// Returns the top coordinate for positions starting with `south*`. +// +// @private +// @param {utils/dom/rect~Rect} targetRect A rect of the target. +// @param {utils/dom/rect~Rect} elementRect A rect of the balloon. +// @returns {Number} function getSouthTop( targetRect ) { return targetRect.bottom + BalloonPanelView.arrowVerticalOffset; } From 0bdec0912354cc46ec8f7acd4d3598d2fda9501d Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Wed, 24 May 2017 11:38:02 +0200 Subject: [PATCH 3/5] Extended positions used by the ContextualToolbar. Defined the position limiter for the ContextualToolbar. Fixed ContextualToolbar content collapsing in narrow spaces. --- src/toolbar/contextual/contextualtoolbar.js | 37 ++++++++++++++++--- tests/toolbar/contextual/contextualtoolbar.js | 21 ++++++++++- 2 files changed, 50 insertions(+), 8 deletions(-) diff --git a/src/toolbar/contextual/contextualtoolbar.js b/src/toolbar/contextual/contextualtoolbar.js index 729f6801..f27757df 100644 --- a/src/toolbar/contextual/contextualtoolbar.js +++ b/src/toolbar/contextual/contextualtoolbar.js @@ -14,8 +14,6 @@ import ToolbarView from '../toolbarview'; import BalloonPanelView from '../../panel/balloon/balloonpanelview.js'; import debounce from '@ckeditor/ckeditor5-utils/src/lib/lodash/debounce'; -const defaultPositions = BalloonPanelView.defaultPositions; - /** * The contextual toolbar. * @@ -51,7 +49,10 @@ export default class ContextualToolbar extends Plugin { Template.extend( this.toolbarView.template, { attributes: { - class: 'ck-editor-toolbar' + class: [ + 'ck-editor-toolbar', + 'ck-toolbar_floating' + ] } } ); @@ -231,9 +232,8 @@ export default class ContextualToolbar extends Plugin { // Select the proper range rect depending on the direction of the selection. return isBackward ? rangeRects.item( 0 ) : rangeRects.item( rangeRects.length - 1 ); }, - positions: isBackward ? - [ defaultPositions.northWestArrowSouth, defaultPositions.southWestArrowNorth ] : - [ defaultPositions.southEastArrowNorth, defaultPositions.northEastArrowSouth ] + limiter: this.editor.ui.view.editable.element, + positions: getBalloonPositions( isBackward ) }; } @@ -263,3 +263,28 @@ export default class ContextualToolbar extends Plugin { * @event _selectionChangeDebounced */ } + +// Returns toolbar positions for the given direction of the selection. +// +// @private +// @param {Boolean} isBackward +// @returns {Array.} +function getBalloonPositions( isBackward ) { + const defaultPositions = BalloonPanelView.defaultPositions; + + return isBackward ? [ + defaultPositions.northWestArrowSouth, + defaultPositions.northWestArrowSouthWest, + defaultPositions.northWestArrowSouthEast, + defaultPositions.southWestArrowNorth, + defaultPositions.southWestArrowNorthWest, + defaultPositions.southWestArrowNorthEast, + ] : [ + defaultPositions.southEastArrowNorth, + defaultPositions.southEastArrowNorthEast, + defaultPositions.southEastArrowNorthWest, + defaultPositions.northEastArrowSouth, + defaultPositions.northEastArrowSouthEast, + defaultPositions.northEastArrowSouthWest, + ]; +} diff --git a/tests/toolbar/contextual/contextualtoolbar.js b/tests/toolbar/contextual/contextualtoolbar.js index bfdbe823..1e380e16 100644 --- a/tests/toolbar/contextual/contextualtoolbar.js +++ b/tests/toolbar/contextual/contextualtoolbar.js @@ -59,6 +59,7 @@ describe( 'ContextualToolbar', () => { expect( contextualToolbar ).to.instanceOf( ContextualToolbar ); expect( contextualToolbar.toolbarView ).to.instanceof( ToolbarView ); expect( contextualToolbar.toolbarView.element.classList.contains( 'ck-editor-toolbar' ) ).to.be.true; + expect( contextualToolbar.toolbarView.element.classList.contains( 'ck-toolbar_floating' ) ).to.be.true; } ); it( 'should load ContextualBalloon', () => { @@ -161,7 +162,15 @@ describe( 'ContextualToolbar', () => { balloonClassName: 'ck-toolbar-container ck-editor-toolbar-container', position: { target: sinon.match( value => value() == backwardSelectionRect ), - positions: [ defaultPositions.southEastArrowNorth, defaultPositions.northEastArrowSouth ] + limiter: editor.ui.view.editable.element, + positions: [ + defaultPositions.southEastArrowNorth, + defaultPositions.southEastArrowNorthEast, + defaultPositions.southEastArrowNorthWest, + defaultPositions.northEastArrowSouth, + defaultPositions.northEastArrowSouthEast, + defaultPositions.northEastArrowSouthWest, + ] } } ); } ); @@ -179,7 +188,15 @@ describe( 'ContextualToolbar', () => { balloonClassName: 'ck-toolbar-container ck-editor-toolbar-container', position: { target: sinon.match( value => value() == forwardSelectionRect ), - positions: [ defaultPositions.northWestArrowSouth, defaultPositions.southWestArrowNorth ] + limiter: editor.ui.view.editable.element, + positions: [ + defaultPositions.northWestArrowSouth, + defaultPositions.northWestArrowSouthWest, + defaultPositions.northWestArrowSouthEast, + defaultPositions.southWestArrowNorth, + defaultPositions.southWestArrowNorthWest, + defaultPositions.southWestArrowNorthEast, + ] } } ); } ); From ae6768f42ce70d0b6848f257924a43999e18491d Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Thu, 25 May 2017 11:04:42 +0200 Subject: [PATCH 4/5] Renamed BalloonPanelView's arrow classes to reflect the actual position of the arrow. --- src/panel/balloon/balloonpanelview.js | 32 ++++++++-------- tests/panel/balloon/balloonpanelview.js | 50 ++++++++++++------------- 2 files changed, 41 insertions(+), 41 deletions(-) diff --git a/src/panel/balloon/balloonpanelview.js b/src/panel/balloon/balloonpanelview.js index 0fef639b..30bdde26 100644 --- a/src/panel/balloon/balloonpanelview.js +++ b/src/panel/balloon/balloonpanelview.js @@ -53,7 +53,7 @@ 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_ne` for "arrow_ne" position. The class + * to the balloon, i.e. `.ck-balloon-panel_arrow_nw` for "arrow_nw" 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. * @@ -65,10 +65,10 @@ export default class BalloonPanelView extends View { * See {@link #withArrow}. * * @observable - * @default 'arrow_ne' - * @member {'arrow_ne'|'arrow_nw'|'arrow_se'|'arrow_sw'} #position + * @default 'arrow_nw' + * @member {'arrow_nw'|'arrow_ne'|'arrow_sw'|'arrow_se'} #position */ - this.set( 'position', 'arrow_ne' ); + this.set( 'position', 'arrow_nw' ); /** * Controls whether the balloon panel is visible or not. @@ -513,13 +513,13 @@ BalloonPanelView.defaultPositions = { northArrowSouthEast: ( targetRect, balloonRect ) => ( { top: getNorthTop( targetRect, balloonRect ), left: targetRect.left + targetRect.width / 2 - balloonRect.width + BalloonPanelView.arrowHorizontalOffset, - name: 'arrow_sw' + name: 'arrow_se' } ), northArrowSouthWest: ( targetRect, balloonRect ) => ( { top: getNorthTop( targetRect, balloonRect ), left: targetRect.left + targetRect.width / 2 - BalloonPanelView.arrowHorizontalOffset, - name: 'arrow_se' + name: 'arrow_sw' } ), // ------- North west @@ -533,13 +533,13 @@ BalloonPanelView.defaultPositions = { northWestArrowSouthWest: ( targetRect, balloonRect ) => ( { top: getNorthTop( targetRect, balloonRect ), left: targetRect.left - BalloonPanelView.arrowHorizontalOffset, - name: 'arrow_se' + name: 'arrow_sw' } ), northWestArrowSouthEast: ( targetRect, balloonRect ) => ( { top: getNorthTop( targetRect, balloonRect ), left: targetRect.left - balloonRect.width + BalloonPanelView.arrowHorizontalOffset, - name: 'arrow_sw' + name: 'arrow_se' } ), // ------- North east @@ -553,13 +553,13 @@ BalloonPanelView.defaultPositions = { northEastArrowSouthEast: ( targetRect, balloonRect ) => ( { top: getNorthTop( targetRect, balloonRect ), left: targetRect.right - balloonRect.width + BalloonPanelView.arrowHorizontalOffset, - name: 'arrow_sw' + name: 'arrow_se' } ), northEastArrowSouthWest: ( targetRect, balloonRect ) => ( { top: getNorthTop( targetRect, balloonRect ), left: targetRect.right - BalloonPanelView.arrowHorizontalOffset, - name: 'arrow_se' + name: 'arrow_sw' } ), // ------- South @@ -573,13 +573,13 @@ BalloonPanelView.defaultPositions = { southArrowNorthEast: ( targetRect, balloonRect ) => ( { top: getSouthTop( targetRect, balloonRect ), left: targetRect.left + targetRect.width / 2 - balloonRect.width + BalloonPanelView.arrowHorizontalOffset, - name: 'arrow_nw' + name: 'arrow_ne' } ), southArrowNorthWest: ( targetRect, balloonRect ) => ( { top: getSouthTop( targetRect, balloonRect ), left: targetRect.left + targetRect.width / 2 - BalloonPanelView.arrowHorizontalOffset, - name: 'arrow_ne' + name: 'arrow_nw' } ), // ------- South west @@ -593,13 +593,13 @@ BalloonPanelView.defaultPositions = { southWestArrowNorthWest: ( targetRect, balloonRect ) => ( { top: getSouthTop( targetRect, balloonRect ), left: targetRect.left - BalloonPanelView.arrowHorizontalOffset, - name: 'arrow_ne' + name: 'arrow_nw' } ), southWestArrowNorthEast: ( targetRect, balloonRect ) => ( { top: getSouthTop( targetRect, balloonRect ), left: targetRect.left - balloonRect.width + BalloonPanelView.arrowHorizontalOffset, - name: 'arrow_nw' + name: 'arrow_ne' } ), // ------- South east @@ -613,13 +613,13 @@ BalloonPanelView.defaultPositions = { southEastArrowNorthEast: ( targetRect, balloonRect ) => ( { top: getSouthTop( targetRect, balloonRect ), left: targetRect.right - balloonRect.width + BalloonPanelView.arrowHorizontalOffset, - name: 'arrow_nw' + name: 'arrow_ne' } ), southEastArrowNorthWest: ( targetRect, balloonRect ) => ( { top: getSouthTop( targetRect, balloonRect ), left: targetRect.right - BalloonPanelView.arrowHorizontalOffset, - name: 'arrow_ne' + name: 'arrow_nw' } ), }; diff --git a/tests/panel/balloon/balloonpanelview.js b/tests/panel/balloon/balloonpanelview.js index 27750cb9..068278d2 100644 --- a/tests/panel/balloon/balloonpanelview.js +++ b/tests/panel/balloon/balloonpanelview.js @@ -38,7 +38,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( 'arrow_ne' ); + expect( view.position ).to.equal( 'arrow_nw' ); expect( view.isVisible ).to.equal( false ); expect( view.withArrow ).to.equal( true ); } ); @@ -51,11 +51,11 @@ describe( 'BalloonPanelView', () => { describe( 'DOM bindings', () => { describe( 'arrow', () => { it( 'should react on view#position', () => { - expect( view.element.classList.contains( 'ck-balloon-panel_arrow_ne' ) ).to.true; + expect( view.element.classList.contains( 'ck-balloon-panel_arrow_nw' ) ).to.true; - view.position = 'arrow_nw'; + view.position = 'arrow_ne'; - expect( view.element.classList.contains( 'ck-balloon-panel_arrow_nw' ) ).to.true; + expect( view.element.classList.contains( 'ck-balloon-panel_arrow_ne' ) ).to.true; } ); it( 'should react on view#withArrow', () => { @@ -220,7 +220,7 @@ describe( 'BalloonPanelView', () => { view.attachTo( { target, limiter } ); - expect( view.position ).to.equal( 'arrow_ne' ); + expect( view.position ).to.equal( 'arrow_nw' ); } ); it( 'should put balloon on the `south east` side of the target element when ' + @@ -234,7 +234,7 @@ describe( 'BalloonPanelView', () => { view.attachTo( { target, limiter } ); - expect( view.position ).to.equal( 'arrow_ne' ); + expect( view.position ).to.equal( 'arrow_nw' ); } ); it( 'should put balloon on the `south west` side of the target element when target is on the right side of the limiter', () => { @@ -247,7 +247,7 @@ describe( 'BalloonPanelView', () => { view.attachTo( { target, limiter } ); - expect( view.position ).to.equal( 'arrow_nw' ); + expect( view.position ).to.equal( 'arrow_ne' ); } ); it( 'should put balloon on the `north east` side of the target element when target is on the bottom of the limiter ', () => { @@ -260,7 +260,7 @@ describe( 'BalloonPanelView', () => { view.attachTo( { target, limiter } ); - expect( view.position ).to.equal( 'arrow_se' ); + expect( view.position ).to.equal( 'arrow_sw' ); } ); it( 'should put balloon on the `north west` side of the target element when ' + @@ -274,7 +274,7 @@ describe( 'BalloonPanelView', () => { view.attachTo( { target, limiter } ); - expect( view.position ).to.equal( 'arrow_sw' ); + expect( view.position ).to.equal( 'arrow_se' ); } ); // https://github.com/ckeditor/ckeditor5-ui-default/issues/126 @@ -355,7 +355,7 @@ describe( 'BalloonPanelView', () => { view.attachTo( { target, limiter } ); - expect( view.position ).to.equal( 'arrow_nw' ); + expect( view.position ).to.equal( 'arrow_ne' ); } ); it( 'should put balloon on the `south east` position when `south west` is limited', () => { @@ -375,7 +375,7 @@ describe( 'BalloonPanelView', () => { view.attachTo( { target, limiter } ); - expect( view.position ).to.equal( 'arrow_ne' ); + expect( view.position ).to.equal( 'arrow_nw' ); } ); it( 'should put balloon on the `north east` position when `south east` is limited', () => { @@ -399,7 +399,7 @@ describe( 'BalloonPanelView', () => { view.attachTo( { target, limiter } ); - expect( view.position ).to.equal( 'arrow_se' ); + expect( view.position ).to.equal( 'arrow_sw' ); } ); it( 'should put balloon on the `south east` position when `north east` is limited', () => { @@ -419,7 +419,7 @@ describe( 'BalloonPanelView', () => { view.attachTo( { target, limiter } ); - expect( view.position ).to.equal( 'arrow_ne' ); + expect( view.position ).to.equal( 'arrow_nw' ); } ); } ); } ); @@ -688,7 +688,7 @@ describe( 'BalloonPanelView', () => { expect( positions.northArrowSouthEast( targetRect, balloonRect ) ).to.deep.equal( { top: 35, left: 130, - name: 'arrow_sw' + name: 'arrow_se' } ); } ); @@ -696,7 +696,7 @@ describe( 'BalloonPanelView', () => { expect( positions.northArrowSouthWest( targetRect, balloonRect ) ).to.deep.equal( { top: 35, left: 120, - name: 'arrow_se' + name: 'arrow_sw' } ); } ); @@ -714,7 +714,7 @@ describe( 'BalloonPanelView', () => { expect( positions.northWestArrowSouthWest( targetRect, balloonRect ) ).to.deep.equal( { top: 35, left: 70, - name: 'arrow_se' + name: 'arrow_sw' } ); } ); @@ -722,7 +722,7 @@ describe( 'BalloonPanelView', () => { expect( positions.northWestArrowSouthEast( targetRect, balloonRect ) ).to.deep.equal( { top: 35, left: 80, - name: 'arrow_sw' + name: 'arrow_se' } ); } ); @@ -740,7 +740,7 @@ describe( 'BalloonPanelView', () => { expect( positions.northEastArrowSouthEast( targetRect, balloonRect ) ).to.deep.equal( { top: 35, left: 180, - name: 'arrow_sw' + name: 'arrow_se' } ); } ); @@ -748,7 +748,7 @@ describe( 'BalloonPanelView', () => { expect( positions.northEastArrowSouthWest( targetRect, balloonRect ) ).to.deep.equal( { top: 35, left: 170, - name: 'arrow_se' + name: 'arrow_sw' } ); } ); @@ -766,7 +766,7 @@ describe( 'BalloonPanelView', () => { expect( positions.southArrowNorthEast( targetRect, balloonRect ) ).to.deep.equal( { top: 215, left: 130, - name: 'arrow_nw' + name: 'arrow_ne' } ); } ); @@ -774,7 +774,7 @@ describe( 'BalloonPanelView', () => { expect( positions.southArrowNorthWest( targetRect, balloonRect ) ).to.deep.equal( { top: 215, left: 120, - name: 'arrow_ne' + name: 'arrow_nw' } ); } ); @@ -792,7 +792,7 @@ describe( 'BalloonPanelView', () => { expect( positions.southWestArrowNorthWest( targetRect, balloonRect ) ).to.deep.equal( { top: 215, left: 70, - name: 'arrow_ne' + name: 'arrow_nw' } ); } ); @@ -800,7 +800,7 @@ describe( 'BalloonPanelView', () => { expect( positions.southWestArrowNorthEast( targetRect, balloonRect ) ).to.deep.equal( { top: 215, left: 80, - name: 'arrow_nw' + name: 'arrow_ne' } ); } ); @@ -818,7 +818,7 @@ describe( 'BalloonPanelView', () => { expect( positions.southEastArrowNorthEast( targetRect, balloonRect ) ).to.deep.equal( { top: 215, left: 180, - name: 'arrow_nw' + name: 'arrow_ne' } ); } ); @@ -826,7 +826,7 @@ describe( 'BalloonPanelView', () => { expect( positions.southEastArrowNorthWest( targetRect, balloonRect ) ).to.deep.equal( { top: 215, left: 170, - name: 'arrow_ne' + name: 'arrow_nw' } ); } ); } ); From 55a727bd517442b43d84f742aad85210e455ea40 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szymon=20Kup=C5=9B?= Date: Tue, 30 May 2017 12:33:41 +0200 Subject: [PATCH 5/5] Removed redundant commas. --- src/toolbar/contextual/contextualtoolbar.js | 4 ++-- tests/toolbar/contextual/contextualtoolbar.js | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/toolbar/contextual/contextualtoolbar.js b/src/toolbar/contextual/contextualtoolbar.js index f27757df..60d72d7e 100644 --- a/src/toolbar/contextual/contextualtoolbar.js +++ b/src/toolbar/contextual/contextualtoolbar.js @@ -278,13 +278,13 @@ function getBalloonPositions( isBackward ) { defaultPositions.northWestArrowSouthEast, defaultPositions.southWestArrowNorth, defaultPositions.southWestArrowNorthWest, - defaultPositions.southWestArrowNorthEast, + defaultPositions.southWestArrowNorthEast ] : [ defaultPositions.southEastArrowNorth, defaultPositions.southEastArrowNorthEast, defaultPositions.southEastArrowNorthWest, defaultPositions.northEastArrowSouth, defaultPositions.northEastArrowSouthEast, - defaultPositions.northEastArrowSouthWest, + defaultPositions.northEastArrowSouthWest ]; } diff --git a/tests/toolbar/contextual/contextualtoolbar.js b/tests/toolbar/contextual/contextualtoolbar.js index 1e380e16..77ee9fba 100644 --- a/tests/toolbar/contextual/contextualtoolbar.js +++ b/tests/toolbar/contextual/contextualtoolbar.js @@ -169,7 +169,7 @@ describe( 'ContextualToolbar', () => { defaultPositions.southEastArrowNorthWest, defaultPositions.northEastArrowSouth, defaultPositions.northEastArrowSouthEast, - defaultPositions.northEastArrowSouthWest, + defaultPositions.northEastArrowSouthWest ] } } ); @@ -195,7 +195,7 @@ describe( 'ContextualToolbar', () => { defaultPositions.northWestArrowSouthEast, defaultPositions.southWestArrowNorth, defaultPositions.southWestArrowNorthWest, - defaultPositions.southWestArrowNorthEast, + defaultPositions.southWestArrowNorthEast ] } } );