From 79ccae5bd9b8f2c98a8cd06519b63f713023dbec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Tue, 4 Apr 2017 12:46:29 +0200 Subject: [PATCH 01/41] Introduced contextual toolbar component. --- src/toolbar/contextualtoolbar.js | 214 +++++++++++++++++++++ tests/toolbar/contextualtoolbar.js | 288 +++++++++++++++++++++++++++++ 2 files changed, 502 insertions(+) create mode 100644 src/toolbar/contextualtoolbar.js create mode 100644 tests/toolbar/contextualtoolbar.js diff --git a/src/toolbar/contextualtoolbar.js b/src/toolbar/contextualtoolbar.js new file mode 100644 index 00000000..75fc970d --- /dev/null +++ b/src/toolbar/contextualtoolbar.js @@ -0,0 +1,214 @@ +/** + * @license Copyright (c) 2003-2017, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md. + */ + +/** + * @module ui/toolbar/contextualtoolbar + */ + +import BalloonPanelView from '../panel/balloon/balloonpanelview'; +import ToolbarView from './toolbarview'; + +/** + * The contextual toolbar. This class displays given editor components + * inside a panel attached to the selection. + * + * Panel is displayed using {@link module:core/editor/editorui~EditorUI#balloon}. + */ +export default class ContextualToolbar { + /** + * Creates an instance of the contextual toolbar class. + * + * @param {module:core/editor/editor~Editor} editor The editor instance. + */ + constructor( editor ) { + /** + * Editor that the UI belongs to. + * + * @readonly + * @member {module:core/editor/editor~Editor} #editor + */ + this.editor = editor; + + /** + * Panel content. + * + * @private + * @member {module:ui/toolbar/toolbarview~ToolbarView} #_toolbarView + */ + this._toolbarView = new ToolbarView( this.editor.locale ); + + // Handle editor action and show or hide toolbar. + this._handleSelectionChange(); + this._handleFocusChange(); + } + + /** + * Adds editor component to the contextual toolbar. + * + * @param {Array} components List of the toolbar components. + * @param {module:ui/componentfactory~ComponentFactory} [factory=core/editor/editorui#componentFactory] + * A factory producing toolbar items. + */ + addComponents( components, factory = this.editor.ui.componentFactory ) { + this._toolbarView.fillFromConfig( components, factory ); + } + + /** + * Returns true when contextual toolbar panel is currently visible + * in {@link module:core/editor/editorui~EditorUI#balloon}. + * + * @private + * @returns {Boolean} + */ + get _isVisible() { + const balloon = this.editor.ui.balloon; + + return balloon.visible && balloon.visible.view === this._toolbarView; + } + + /** + * Handles editor focus change and hides panel if it's needed. + * + * @protected + */ + _handleFocusChange() { + const editor = this.editor; + + // Hide the panel View when editor loses focus but no the other way around. + this._toolbarView.listenTo( editor.ui.focusTracker, 'change:isFocused', ( evt, name, isFocused ) => { + if ( this._isVisible && !isFocused ) { + this._hidePanel(); + } + } ); + } + + /** + * Handles {@link module:core/editor/editor~Editor#editing} selection change + * and show or hide toolbar. + * + * @private + */ + _handleSelectionChange() { + const toolbarView = this._toolbarView; + const editingView = this.editor.editing.view; + + // Hide panel when selection is changing. + toolbarView.listenTo( editingView, 'selectionChange', () => this._hidePanel() ); + + // Display panel attached to the selection when selection stops changing. + toolbarView.listenTo( editingView, 'selectionChangeDone', () => this._showPanel() ); + } + + /** + * Adds panel view to the {@link: core/editor/editorui~EditorUI#balloon} and attaches panel to the selection. + * + * @private + */ + _showPanel() { + const editingView = this.editor.editing.view; + const balloon = this.editor.ui.balloon; + + // Do not add panel to the balloon stack twice. + if ( balloon.isPanelInStack( this._toolbarView ) ) { + return; + } + + // This implementation assumes that only non–collapsed selections gets the contextual toolbar. + if ( !editingView.isFocused || editingView.selection.isCollapsed ) { + return; + } + + // Get direction of the selection. + const isBackward = editingView.selection.isBackward; + + // getBoundingClientRect() makes no sense when the selection spans across number + // of lines of text. Using getClientRects() allows us to browse micro–ranges + // that would normally make up the bounding client rect. + const rangeRects = editingView.domConverter.viewRangeToDom( editingView.selection.getFirstRange() ).getClientRects(); + + // Select the proper range rect depending on the direction of the selection. + const rangeRect = isBackward ? rangeRects.item( 0 ) : rangeRects.item( rangeRects.length - 1 ); + + // Add panel to the common editor contextual balloon. + balloon.add( { + view: this._toolbarView, + position: { + target: rangeRect, + positions: isBackward ? + [ positions.backwardSelection, positions.backwardSelectionAlternative ] : + [ positions.forwardSelection, positions.forwardSelectionAlternative ], + } + } ); + + // Update panel position when editor content has changed. + this._toolbarView.listenTo( editingView, 'render', () => { + if ( this._isVisible ) { + balloon.updatePosition(); + } + } ); + } + + /** + * Removes panel from the {@link: core/editor/editorui~EditorUI#balloon}. + * + * @private + */ + _hidePanel() { + const balloon = this.editor.ui.balloon; + + if ( balloon.isPanelInStack( this._toolbarView ) ) { + balloon.remove( this._toolbarView ); + this._toolbarView.stopListening( this.editor.editing.view, 'render' ); + } + } +} + +// List of available toolbar positions. +const arrowVOffset = BalloonPanelView.arrowVerticalOffset; +const positions = { + // [text range] + // ^ + // +-----------------+ + // | Balloon | + // +-----------------+ + forwardSelection: ( targetRect, balloonRect ) => ( { + top: targetRect.bottom + arrowVOffset, + left: targetRect.right - balloonRect.width / 2, + name: 's' + } ), + + // +-----------------+ + // | Balloon | + // +-----------------+ + // V + // [text range] + forwardSelectionAlternative: ( targetRect, balloonRect ) => ( { + top: targetRect.top - balloonRect.height - arrowVOffset, + left: targetRect.right - balloonRect.width / 2, + name: 'n' + } ), + + // +-----------------+ + // | Balloon | + // +-----------------+ + // V + // [text range] + backwardSelection: ( targetRect, balloonRect ) => ( { + top: targetRect.top - balloonRect.height - arrowVOffset, + left: targetRect.left - balloonRect.width / 2, + name: 'n' + } ), + + // [text range] + // ^ + // +-----------------+ + // | Balloon | + // +-----------------+ + backwardSelectionAlternative: ( targetRect, balloonRect ) => ( { + top: targetRect.bottom + arrowVOffset, + left: targetRect.left - balloonRect.width / 2, + name: 's' + } ) +}; diff --git a/tests/toolbar/contextualtoolbar.js b/tests/toolbar/contextualtoolbar.js new file mode 100644 index 00000000..9d96c0a4 --- /dev/null +++ b/tests/toolbar/contextualtoolbar.js @@ -0,0 +1,288 @@ +/** + * Copyright (c) 2016, CKSource - Frederico Knabben. All rights reserved. + */ + +import ClassicTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/classictesteditor'; +import ContextualToolbar from '../../src/toolbar/contextualtoolbar'; +import ViewSelection from '@ckeditor/ckeditor5-engine/src/view/selection'; +import ToolbarView from '@ckeditor/ckeditor5-ui/src/toolbar/toolbarview'; +import Bold from '@ckeditor/ckeditor5-basic-styles/src/bold'; +import Italic from '@ckeditor/ckeditor5-basic-styles/src/italic'; +import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph'; +import global from '@ckeditor/ckeditor5-utils/src/dom/global'; + +import { setData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model.js'; + +/* global document, window */ + +describe( 'ContextualToolbar', () => { + let sandbox, editor, contextualToolbar, balloon, editorElement; + + beforeEach( () => { + editorElement = document.createElement( 'div' ); + document.body.appendChild( editorElement ); + sandbox = sinon.sandbox.create(); + + return ClassicTestEditor.create( editorElement, { + plugins: [ Paragraph, Bold, Italic ] + } ).then( newEditor => { + editor = newEditor; + editor.editing.view.attachDomRoot( editorElement ); + balloon = editor.ui.balloon; + contextualToolbar = new ContextualToolbar( editor ); + editor.editing.view.isFocused = true; + } ); + } ); + + afterEach( () => { + editor.destroy(); + sandbox.restore(); + } ); + + it( 'should open below if the selection is forward', () => { + setData( editor.document, '[bar]' ); + + stubClientRects(); + + editor.editing.view.fire( 'selectionChangeDone' ); + + expect( balloon.visible.view ).to.equal( contextualToolbar._toolbarView ); + + expect( balloon.view.top ).to.be.above( 310 ); + } ); + + it( 'should open above if the selection is forward but panel stick out of the limiter element', () => { + setData( editor.document, '[bar]' ); + + stubClientRects(); + + // Mock limiter rect. + mockBoundingBox( document.body, { + left: 0, + width: 1000, + top: 0, + height: 310 + } ); + + editor.editing.view.fire( 'selectionChangeDone' ); + + expect( balloon.visible.view ).to.equal( contextualToolbar._toolbarView ); + + expect( balloon.view.top ).to.be.below( 310 ); + } ); + + it( 'should open above if the selection is backward', () => { + setData( editor.document, '[bar]', { lastRangeBackward: true } ); + + stubClientRects(); + + editor.editing.view.fire( 'selectionChangeDone' ); + + expect( balloon.visible.view ).to.equal( contextualToolbar._toolbarView ); + expect( balloon.view.top ).to.be.below( 100 ); + } ); + + it( 'should open below if the selection is backward but panel stick out of the limiter element', () => { + setData( editor.document, '[bar]', { lastRangeBackward: true } ); + + stubClientRects(); + + // Mock limiter rect. + mockBoundingBox( document.body, { + left: 0, + width: 1000, + top: 95, + height: 905 + } ); + + editor.editing.view.fire( 'selectionChangeDone' ); + + expect( balloon.visible.view ).to.equal( contextualToolbar._toolbarView ); + expect( balloon.view.top ).to.be.above( 100 ); + } ); + + it( 'should not open if selection is collapsed and is moving', () => { + setData( editor.document, 'ba[]r' ); + + const oldSelection = editor.editing.view.selection; + const newSelection = new ViewSelection(); + + editor.editing.view.fire( 'selectionChange', { oldSelection, newSelection } ); + editor.editing.view.fire( 'selectionChangeDone' ); + + setData( editor.document, 'b[]ar' ); + + editor.editing.view.fire( 'selectionChange', { oldSelection, newSelection } ); + editor.editing.view.fire( 'selectionChangeDone' ); + + expect( balloon.visible ).to.null; + } ); + + it( 'should hide if editor loses focus', () => { + setData( editor.document, '[bar]' ); + editor.ui.focusTracker.isFocused = true; + + stubClientRects(); + + editor.editing.view.fire( 'selectionChangeDone' ); + + expect( balloon.visible.view ).to.equal( contextualToolbar._toolbarView ); + + editor.ui.focusTracker.isFocused = false; + + expect( balloon.visible ).to.null; + } ); + + it( 'should hide if selection is changing', () => { + setData( editor.document, '[bar]' ); + + stubClientRects(); + + editor.editing.view.fire( 'selectionChangeDone' ); + + expect( balloon.visible.view ).to.equal( contextualToolbar._toolbarView ); + + const oldSelection = editor.editing.view.selection; + const newSelection = new ViewSelection(); + + editor.editing.view.fire( 'selectionChange', { oldSelection, newSelection } ); + + expect( balloon.visible ).to.null; + } ); + + it( 'should do nothing when panel is being added to balloon stack twice', () => { + setData( editor.document, '[bar]' ); + + editor.editing.view.fire( 'selectionChangeDone' ); + + expect( balloon.visible.view ).to.equal( contextualToolbar._toolbarView ); + + expect( () => { + editor.editing.view.fire( 'selectionChangeDone' ); + } ).to.not.throw(); + } ); + + it( 'should update toolbar position when is visible and editor content has changed', () => { + const spy = sandbox.spy( balloon, 'updatePosition' ); + + setData( editor.document, 'ba[r]' ); + + editor.editing.view.fire( 'selectionChangeDone' ); + + sinon.assert.notCalled( spy ); + + editor.editing.view.fire( 'render' ); + + sinon.assert.calledOnce( spy ); + } ); + + it( 'should not update toolbar position when is added to the balloon stack but is not visible and editor content has changed', () => { + const spy = sandbox.spy( balloon, 'updatePosition' ); + + setData( editor.document, 'ba[r]' ); + + editor.editing.view.fire( 'selectionChangeDone' ); + + const viewMock = {}; + + balloon.add( { view: viewMock } ); + + editor.editing.view.fire( 'render' ); + + sinon.assert.notCalled( spy ); + } ); + + it( 'should not update toolbar position when is added to the balloon stack but is not visible and editor content has changed', () => { + const spy = sandbox.spy( balloon, 'updatePosition' ); + + setData( editor.document, 'ba[r]' ); + + editor.editing.view.fire( 'selectionChangeDone' ); + + const viewMock = {}; + + balloon.add( { view: viewMock } ); + + editor.editing.view.fire( 'render' ); + + sinon.assert.notCalled( spy ); + } ); + + describe( 'addComponents()', () => { + it( 'should adds given components as a toolbar content', () => { + contextualToolbar = new ContextualToolbar( editor ); + + expect( contextualToolbar._toolbarView ).to.instanceof( ToolbarView ); + expect( contextualToolbar._toolbarView.items.length ).to.equal( 0 ); + + contextualToolbar.addComponents( [ 'bold', 'italic' ] ); + + expect( contextualToolbar._toolbarView.items.length ).to.equal( 2 ); + } ); + } ); + + function stubClientRects() { + const editingView = editor.editing.view; + const originalViewRangeToDom = editingView.domConverter.viewRangeToDom; + + // Mock selection rect. + sandbox.stub( editingView.domConverter, 'viewRangeToDom', ( ...args ) => { + const domRange = originalViewRangeToDom.apply( editingView.domConverter, args ); + + sandbox.stub( domRange, 'getClientRects', () => { + return { + length: 2, + item: id => { + if ( id === 0 ) { + return { + top: 100, + height: 10, + bottom: 110, + left: 200, + width: 50, + right: 250 + }; + } + + return { + top: 300, + height: 10, + bottom: 310, + left: 400, + width: 50, + right: 450 + }; + } + }; + } ); + + return domRange; + } ); + + // Mock window rect. + sandbox.stub( global, 'window', { + innerWidth: 1000, + innerHeight: 1000, + scrollX: 0, + scrollY: 0, + getComputedStyle: el => { + return window.getComputedStyle( el ); + } + } ); + + // Mock balloon rect. + mockBoundingBox( balloon.view.element, { + width: 150, + height: 50 + } ); + } + + function mockBoundingBox( element, data ) { + const boundingBox = Object.assign( {}, data ); + + boundingBox.right = boundingBox.left + boundingBox.width; + boundingBox.bottom = boundingBox.top + boundingBox.height; + + sandbox.stub( element, 'getBoundingClientRect' ).returns( boundingBox ); + } +} ); From 48f86cc86f6e5c558d913c30e8ae31ea9b8f48b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Tue, 4 Apr 2017 12:47:46 +0200 Subject: [PATCH 02/41] Replace contextual toolbar implementation by ContextualToolbar component in manual test. --- .../contextualtoolbar/contextualtoolbar.js | 119 ++---------------- 1 file changed, 8 insertions(+), 111 deletions(-) diff --git a/tests/manual/contextualtoolbar/contextualtoolbar.js b/tests/manual/contextualtoolbar/contextualtoolbar.js index ba6cbf54..f3ce8cec 100644 --- a/tests/manual/contextualtoolbar/contextualtoolbar.js +++ b/tests/manual/contextualtoolbar/contextualtoolbar.js @@ -6,124 +6,21 @@ /* globals window, document, console:false */ import ClassicEditor from '@ckeditor/ckeditor5-editor-classic/src/classic'; -import DomEventObserver from '@ckeditor/ckeditor5-engine/src/view/observer/domeventobserver'; -import Enter from '@ckeditor/ckeditor5-enter/src/enter'; -import Typing from '@ckeditor/ckeditor5-typing/src/typing'; -import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph'; -import Undo from '@ckeditor/ckeditor5-undo/src/undo'; -import Bold from '@ckeditor/ckeditor5-basic-styles/src/bold'; -import Italic from '@ckeditor/ckeditor5-basic-styles/src/italic'; +import ArticlePresets from '@ckeditor/ckeditor5-presets/src/article'; -import Template from '../../../src/template'; -import ToolbarView from '../../../src/toolbar/toolbarview'; -import BalloonPanelView from '../../../src/panel/balloon/balloonpanelview'; - -const arrowVOffset = BalloonPanelView.arrowVerticalOffset; -const positions = { - // [text range] - // ^ - // +-----------------+ - // | Balloon | - // +-----------------+ - forwardSelection: ( targetRect, balloonRect ) => ( { - top: targetRect.bottom + arrowVOffset, - left: targetRect.right - balloonRect.width / 2, - name: 's' - } ), - - // +-----------------+ - // | Balloon | - // +-----------------+ - // V - // [text range] - backwardSelection: ( targetRect, balloonRect ) => ( { - top: targetRect.top - balloonRect.height - arrowVOffset, - left: targetRect.left - balloonRect.width / 2, - name: 'n' - } ) -}; +import ContextualToolbar from '../../../src/toolbar/contextualtoolbar'; ClassicEditor.create( document.querySelector( '#editor' ), { - plugins: [ Enter, Typing, Paragraph, Undo, Bold, Italic ], - toolbar: [ 'bold', 'italic', 'undo', 'redo' ] + plugins: [ ArticlePresets ], + toolbar: [ 'bold', 'italic', 'undo', 'redo', 'link' ] } ) .then( editor => { - createContextualToolbar( editor ); + const toolbar = new ContextualToolbar( editor ); + + toolbar.addComponents( [ 'bold', 'italic' ] ); + window.editor = editor; } ) .catch( err => { console.error( err.stack ); } ); - -function createContextualToolbar( editor ) { - // Create a plain toolbar instance. - const toolbar = new ToolbarView(); - - // Create a BalloonPanelView instance. - const panel = new BalloonPanelView( editor.locale ); - - Template.extend( panel.template, { - attributes: { - class: [ - 'ck-toolbar__container', - ] - } - } ); - - // Putting the toolbar inside of the balloon panel. - panel.content.add( toolbar ); - - editor.ui.view.body.add( panel ).then( () => { - const editingView = editor.editing.view; - - // Fill the toolbar with some buttons. Simply copy default editor toolbar. - toolbar.fillFromConfig( editor.config.get( 'toolbar' ), editor.ui.componentFactory ); - - // Let the focusTracker know about new focusable UI element. - editor.ui.focusTracker.add( panel.element ); - - // Hide the panel when editor loses focus but no the other way around. - panel.listenTo( editor.ui.focusTracker, 'change:isFocused', ( evt, name, is, was ) => { - if ( was && !is ) { - panel.hide(); - } - } ); - - // Add "mouseup" event observer. It's enought to use ClickObserver in Chrome - // but Firefox requires "mouseup" to work properly. - editingView.addObserver( class extends DomEventObserver { - get domEventType() { - return [ 'mouseup' ]; - } - - onDomEvent( domEvent ) { - this.fire( domEvent.type, domEvent ); - } - } ); - - // Position the panel each time the user clicked in editable. - editor.listenTo( editingView, 'mouseup', () => { - // This implementation assumes that only non–collapsed selections gets the contextual toolbar. - if ( !editingView.selection.isCollapsed ) { - const isBackward = editingView.selection.isBackward; - - // getBoundingClientRect() makes no sense when the selection spans across number - // of lines of text. Using getClientRects() allows us to browse micro–ranges - // that would normally make up the bounding client rect. - const rangeRects = editingView.domConverter.viewRangeToDom( editingView.selection.getFirstRange() ).getClientRects(); - - // Select the proper range rect depending on the direction of the selection. - const rangeRect = isBackward ? rangeRects.item( 0 ) : rangeRects.item( rangeRects.length - 1 ); - - panel.attachTo( { - target: rangeRect, - positions: [ - positions[ isBackward ? 'backwardSelection' : 'forwardSelection' ] - ] - } ); - } else { - panel.hide(); - } - } ); - } ); -} From 13399cb727ffb1f28edde1ecb24ff3624e5b2187 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Tue, 4 Apr 2017 12:52:35 +0200 Subject: [PATCH 03/41] Updated docs. --- src/toolbar/contextualtoolbar.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/toolbar/contextualtoolbar.js b/src/toolbar/contextualtoolbar.js index 75fc970d..e350feb2 100644 --- a/src/toolbar/contextualtoolbar.js +++ b/src/toolbar/contextualtoolbar.js @@ -24,7 +24,7 @@ export default class ContextualToolbar { */ constructor( editor ) { /** - * Editor that the UI belongs to. + * Editor instance. * * @readonly * @member {module:core/editor/editor~Editor} #editor @@ -32,7 +32,7 @@ export default class ContextualToolbar { this.editor = editor; /** - * Panel content. + * Content of the panel. * * @private * @member {module:ui/toolbar/toolbarview~ToolbarView} #_toolbarView @@ -45,7 +45,7 @@ export default class ContextualToolbar { } /** - * Adds editor component to the contextual toolbar. + * Adds editor components to the contextual toolbar. * * @param {Array} components List of the toolbar components. * @param {module:ui/componentfactory~ComponentFactory} [factory=core/editor/editorui#componentFactory] From 1c36b61bf26927e8972d1a0b5b6fdc1ea5d92565 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Wed, 5 Apr 2017 22:39:14 +0200 Subject: [PATCH 04/41] Aligned ContextualToolbar to the latest ContextualPanel API. --- src/toolbar/contextualtoolbar.js | 99 +++++++--------- .../contextualtoolbar/contextualtoolbar.js | 10 +- tests/toolbar/contextualtoolbar.js | 112 ++++++++---------- 3 files changed, 96 insertions(+), 125 deletions(-) diff --git a/src/toolbar/contextualtoolbar.js b/src/toolbar/contextualtoolbar.js index e350feb2..d1a68657 100644 --- a/src/toolbar/contextualtoolbar.js +++ b/src/toolbar/contextualtoolbar.js @@ -7,78 +7,74 @@ * @module ui/toolbar/contextualtoolbar */ -import BalloonPanelView from '../panel/balloon/balloonpanelview'; +import Plugin from '@ckeditor/ckeditor5-core/src/plugin'; +import ContextualBalloon from '@ckeditor/ckeditor5-ui/src/contextualballoon'; import ToolbarView from './toolbarview'; +import BalloonPanelView from '../panel/balloon/balloonpanelview.js'; /** - * The contextual toolbar. This class displays given editor components - * inside a panel attached to the selection. + * The contextual toolbar. * - * Panel is displayed using {@link module:core/editor/editorui~EditorUI#balloon}. + * It uses the {@link module:ui/contextualballoon~ContextualBalloon contextual balloon plugin}. + * + * @extends module:core/plugin~Plugin */ -export default class ContextualToolbar { +export default class ContextualToolbar extends Plugin { /** - * Creates an instance of the contextual toolbar class. - * - * @param {module:core/editor/editor~Editor} editor The editor instance. + * @inheritDoc */ - constructor( editor ) { + static get requires() { + return [ ContextualBalloon ]; + } + + /** + * @inheritDoc + */ + init() { /** - * Editor instance. + * The toolbar view displayed in the balloon. * - * @readonly - * @member {module:core/editor/editor~Editor} #editor + * @member {module:ui/toolbar/toolbarview~ToolbarView} */ - this.editor = editor; + this.toolbarView = new ToolbarView( this.editor.locale ); /** - * Content of the panel. + * The contextual balloon plugin instance. * * @private - * @member {module:ui/toolbar/toolbarview~ToolbarView} #_toolbarView + * @member {module:ui/contextualballoon~ContextualBalloon} */ - this._toolbarView = new ToolbarView( this.editor.locale ); + this._balloon = this.editor.plugins.get( ContextualBalloon ); - // Handle editor action and show or hide toolbar. + // Attach lifecycle actions. this._handleSelectionChange(); this._handleFocusChange(); } /** - * Adds editor components to the contextual toolbar. - * - * @param {Array} components List of the toolbar components. - * @param {module:ui/componentfactory~ComponentFactory} [factory=core/editor/editorui#componentFactory] - * A factory producing toolbar items. - */ - addComponents( components, factory = this.editor.ui.componentFactory ) { - this._toolbarView.fillFromConfig( components, factory ); - } - - /** - * Returns true when contextual toolbar panel is currently visible - * in {@link module:core/editor/editorui~EditorUI#balloon}. + * Creates toolbar components based on given configuration. + * This need to be done when all plugins will be ready. * - * @private - * @returns {Boolean} + * @inheritDoc */ - get _isVisible() { - const balloon = this.editor.ui.balloon; + afterInit() { + const config = this.editor.config.get( 'contextualToolbar' ); + const factory = this.editor.ui.componentFactory; - return balloon.visible && balloon.visible.view === this._toolbarView; + return this.toolbarView.fillFromConfig( config, factory ); } /** * Handles editor focus change and hides panel if it's needed. * - * @protected + * @private */ _handleFocusChange() { const editor = this.editor; // Hide the panel View when editor loses focus but no the other way around. - this._toolbarView.listenTo( editor.ui.focusTracker, 'change:isFocused', ( evt, name, isFocused ) => { - if ( this._isVisible && !isFocused ) { + this.toolbarView.listenTo( editor.ui.focusTracker, 'change:isFocused', ( evt, name, isFocused ) => { + if ( this._balloon.visibleView === this.toolbarView && !isFocused ) { this._hidePanel(); } } ); @@ -91,7 +87,7 @@ export default class ContextualToolbar { * @private */ _handleSelectionChange() { - const toolbarView = this._toolbarView; + const toolbarView = this.toolbarView; const editingView = this.editor.editing.view; // Hide panel when selection is changing. @@ -102,16 +98,15 @@ export default class ContextualToolbar { } /** - * Adds panel view to the {@link: core/editor/editorui~EditorUI#balloon} and attaches panel to the selection. + * Adds panel view to the {@link: #_balloon} and attaches panel to the selection. * * @private */ _showPanel() { const editingView = this.editor.editing.view; - const balloon = this.editor.ui.balloon; // Do not add panel to the balloon stack twice. - if ( balloon.isPanelInStack( this._toolbarView ) ) { + if ( this._balloon.hasView( this.toolbarView ) ) { return; } @@ -132,8 +127,8 @@ export default class ContextualToolbar { const rangeRect = isBackward ? rangeRects.item( 0 ) : rangeRects.item( rangeRects.length - 1 ); // Add panel to the common editor contextual balloon. - balloon.add( { - view: this._toolbarView, + this._balloon.add( { + view: this.toolbarView, position: { target: rangeRect, positions: isBackward ? @@ -143,24 +138,22 @@ export default class ContextualToolbar { } ); // Update panel position when editor content has changed. - this._toolbarView.listenTo( editingView, 'render', () => { - if ( this._isVisible ) { - balloon.updatePosition(); + this.toolbarView.listenTo( editingView, 'render', () => { + if ( this._balloon.visibleView === this.toolbarView ) { + this._balloon.updatePosition(); } } ); } /** - * Removes panel from the {@link: core/editor/editorui~EditorUI#balloon}. + * Removes panel from the {@link: #_balloon}. * * @private */ _hidePanel() { - const balloon = this.editor.ui.balloon; - - if ( balloon.isPanelInStack( this._toolbarView ) ) { - balloon.remove( this._toolbarView ); - this._toolbarView.stopListening( this.editor.editing.view, 'render' ); + if ( this._balloon.hasView( this.toolbarView ) ) { + this._balloon.remove( this.toolbarView ); + this.toolbarView.stopListening( this.editor.editing.view, 'render' ); } } } diff --git a/tests/manual/contextualtoolbar/contextualtoolbar.js b/tests/manual/contextualtoolbar/contextualtoolbar.js index f3ce8cec..a5f7ef39 100644 --- a/tests/manual/contextualtoolbar/contextualtoolbar.js +++ b/tests/manual/contextualtoolbar/contextualtoolbar.js @@ -7,18 +7,14 @@ import ClassicEditor from '@ckeditor/ckeditor5-editor-classic/src/classic'; import ArticlePresets from '@ckeditor/ckeditor5-presets/src/article'; - import ContextualToolbar from '../../../src/toolbar/contextualtoolbar'; ClassicEditor.create( document.querySelector( '#editor' ), { - plugins: [ ArticlePresets ], - toolbar: [ 'bold', 'italic', 'undo', 'redo', 'link' ] + plugins: [ ArticlePresets, ContextualToolbar ], + toolbar: [ 'bold', 'italic', 'link', 'undo', 'redo' ], + contextualToolbar: [ 'bold', 'italic', 'link' ] } ) .then( editor => { - const toolbar = new ContextualToolbar( editor ); - - toolbar.addComponents( [ 'bold', 'italic' ] ); - window.editor = editor; } ) .catch( err => { diff --git a/tests/toolbar/contextualtoolbar.js b/tests/toolbar/contextualtoolbar.js index 9d96c0a4..1c60eb37 100644 --- a/tests/toolbar/contextualtoolbar.js +++ b/tests/toolbar/contextualtoolbar.js @@ -4,8 +4,9 @@ import ClassicTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/classictesteditor'; import ContextualToolbar from '../../src/toolbar/contextualtoolbar'; +import ContextualBalloon from '../../src/contextualballoon'; +import ToolbarView from '../../src/toolbar/toolbarview'; import ViewSelection from '@ckeditor/ckeditor5-engine/src/view/selection'; -import ToolbarView from '@ckeditor/ckeditor5-ui/src/toolbar/toolbarview'; import Bold from '@ckeditor/ckeditor5-basic-styles/src/bold'; import Italic from '@ckeditor/ckeditor5-basic-styles/src/italic'; import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph'; @@ -19,43 +20,62 @@ describe( 'ContextualToolbar', () => { let sandbox, editor, contextualToolbar, balloon, editorElement; beforeEach( () => { + sandbox = sinon.sandbox.create(); + editorElement = document.createElement( 'div' ); document.body.appendChild( editorElement ); - sandbox = sinon.sandbox.create(); return ClassicTestEditor.create( editorElement, { - plugins: [ Paragraph, Bold, Italic ] - } ).then( newEditor => { + plugins: [ Paragraph, Bold, Italic, ContextualToolbar ], + contextualToolbar: [ 'bold', 'italic' ] + } ) + .then( newEditor => { + newEditor.editing.view.attachDomRoot( editorElement ); + editor = newEditor; - editor.editing.view.attachDomRoot( editorElement ); - balloon = editor.ui.balloon; - contextualToolbar = new ContextualToolbar( editor ); + contextualToolbar = editor.plugins.get( ContextualToolbar ); + balloon = editor.plugins.get( ContextualBalloon ); + + // Focus the engine. editor.editing.view.isFocused = true; + + stubClientRects(); } ); } ); afterEach( () => { - editor.destroy(); sandbox.restore(); + editor.destroy(); + } ); + + it( 'should be loaded', () => { + expect( contextualToolbar ).to.instanceOf( ContextualToolbar ); + } ); + + it( 'should load ContextualBalloon', () => { + expect( balloon ).to.instanceof( ContextualBalloon ); + } ); + + it( 'should create plugin instance with properties', () => { + expect( contextualToolbar.toolbarView ).to.instanceof( ToolbarView ); + } ); + + it( 'should create components from config', () => { + expect( contextualToolbar.toolbarView.items ).to.length( 2 ); } ); it( 'should open below if the selection is forward', () => { setData( editor.document, '[bar]' ); - stubClientRects(); - editor.editing.view.fire( 'selectionChangeDone' ); - expect( balloon.visible.view ).to.equal( contextualToolbar._toolbarView ); - + expect( balloon.visibleView ).to.equal( contextualToolbar.toolbarView ); expect( balloon.view.top ).to.be.above( 310 ); } ); it( 'should open above if the selection is forward but panel stick out of the limiter element', () => { setData( editor.document, '[bar]' ); - stubClientRects(); - // Mock limiter rect. mockBoundingBox( document.body, { left: 0, @@ -66,27 +86,22 @@ describe( 'ContextualToolbar', () => { editor.editing.view.fire( 'selectionChangeDone' ); - expect( balloon.visible.view ).to.equal( contextualToolbar._toolbarView ); - + expect( balloon.visibleView ).to.equal( contextualToolbar.toolbarView ); expect( balloon.view.top ).to.be.below( 310 ); } ); it( 'should open above if the selection is backward', () => { setData( editor.document, '[bar]', { lastRangeBackward: true } ); - stubClientRects(); - editor.editing.view.fire( 'selectionChangeDone' ); - expect( balloon.visible.view ).to.equal( contextualToolbar._toolbarView ); + expect( balloon.visibleView ).to.equal( contextualToolbar.toolbarView ); expect( balloon.view.top ).to.be.below( 100 ); } ); it( 'should open below if the selection is backward but panel stick out of the limiter element', () => { setData( editor.document, '[bar]', { lastRangeBackward: true } ); - stubClientRects(); - // Mock limiter rect. mockBoundingBox( document.body, { left: 0, @@ -97,11 +112,11 @@ describe( 'ContextualToolbar', () => { editor.editing.view.fire( 'selectionChangeDone' ); - expect( balloon.visible.view ).to.equal( contextualToolbar._toolbarView ); + expect( balloon.visibleView ).to.equal( contextualToolbar.toolbarView ); expect( balloon.view.top ).to.be.above( 100 ); } ); - it( 'should not open if selection is collapsed and is moving', () => { + it( 'should not open if the collapsed selection is moving', () => { setData( editor.document, 'ba[]r' ); const oldSelection = editor.editing.view.selection; @@ -115,39 +130,35 @@ describe( 'ContextualToolbar', () => { editor.editing.view.fire( 'selectionChange', { oldSelection, newSelection } ); editor.editing.view.fire( 'selectionChangeDone' ); - expect( balloon.visible ).to.null; + expect( balloon.visibleView ).to.null; } ); - it( 'should hide if editor loses focus', () => { + it( 'should hide if the editor loses focus', () => { setData( editor.document, '[bar]' ); editor.ui.focusTracker.isFocused = true; - stubClientRects(); - editor.editing.view.fire( 'selectionChangeDone' ); - expect( balloon.visible.view ).to.equal( contextualToolbar._toolbarView ); + expect( balloon.visibleView ).to.equal( contextualToolbar.toolbarView ); editor.ui.focusTracker.isFocused = false; - expect( balloon.visible ).to.null; + expect( balloon.visibleView ).to.null; } ); - it( 'should hide if selection is changing', () => { + it( 'should hide if the selection is changing', () => { setData( editor.document, '[bar]' ); - stubClientRects(); - editor.editing.view.fire( 'selectionChangeDone' ); - expect( balloon.visible.view ).to.equal( contextualToolbar._toolbarView ); + expect( balloon.visibleView ).to.equal( contextualToolbar.toolbarView ); const oldSelection = editor.editing.view.selection; const newSelection = new ViewSelection(); editor.editing.view.fire( 'selectionChange', { oldSelection, newSelection } ); - expect( balloon.visible ).to.null; + expect( balloon.visibleView ).to.null; } ); it( 'should do nothing when panel is being added to balloon stack twice', () => { @@ -155,7 +166,7 @@ describe( 'ContextualToolbar', () => { editor.editing.view.fire( 'selectionChangeDone' ); - expect( balloon.visible.view ).to.equal( contextualToolbar._toolbarView ); + expect( balloon.visibleView ).to.equal( contextualToolbar.toolbarView ); expect( () => { editor.editing.view.fire( 'selectionChangeDone' ); @@ -183,23 +194,7 @@ describe( 'ContextualToolbar', () => { editor.editing.view.fire( 'selectionChangeDone' ); - const viewMock = {}; - - balloon.add( { view: viewMock } ); - - editor.editing.view.fire( 'render' ); - - sinon.assert.notCalled( spy ); - } ); - - it( 'should not update toolbar position when is added to the balloon stack but is not visible and editor content has changed', () => { - const spy = sandbox.spy( balloon, 'updatePosition' ); - - setData( editor.document, 'ba[r]' ); - - editor.editing.view.fire( 'selectionChangeDone' ); - - const viewMock = {}; + const viewMock = { destroy: () => {} }; balloon.add( { view: viewMock } ); @@ -208,19 +203,6 @@ describe( 'ContextualToolbar', () => { sinon.assert.notCalled( spy ); } ); - describe( 'addComponents()', () => { - it( 'should adds given components as a toolbar content', () => { - contextualToolbar = new ContextualToolbar( editor ); - - expect( contextualToolbar._toolbarView ).to.instanceof( ToolbarView ); - expect( contextualToolbar._toolbarView.items.length ).to.equal( 0 ); - - contextualToolbar.addComponents( [ 'bold', 'italic' ] ); - - expect( contextualToolbar._toolbarView.items.length ).to.equal( 2 ); - } ); - } ); - function stubClientRects() { const editingView = editor.editing.view; const originalViewRangeToDom = editingView.domConverter.viewRangeToDom; From 99deee2be408016f0036580a6ebfa543e2781120 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Thu, 6 Apr 2017 13:09:32 +0200 Subject: [PATCH 05/41] Refactored to listen model selection change:range event. --- src/toolbar/contextualtoolbar.js | 41 +++++---- tests/toolbar/contextualtoolbar.js | 131 +++++++++++++++-------------- 2 files changed, 93 insertions(+), 79 deletions(-) diff --git a/src/toolbar/contextualtoolbar.js b/src/toolbar/contextualtoolbar.js index d1a68657..e4f9a09f 100644 --- a/src/toolbar/contextualtoolbar.js +++ b/src/toolbar/contextualtoolbar.js @@ -11,6 +11,7 @@ import Plugin from '@ckeditor/ckeditor5-core/src/plugin'; import ContextualBalloon from '@ckeditor/ckeditor5-ui/src/contextualballoon'; import ToolbarView from './toolbarview'; import BalloonPanelView from '../panel/balloon/balloonpanelview.js'; +import debounce from '@ckeditor/ckeditor5-utils/src/lib/lodash/debounce'; /** * The contextual toolbar. @@ -73,7 +74,7 @@ export default class ContextualToolbar extends Plugin { const editor = this.editor; // Hide the panel View when editor loses focus but no the other way around. - this.toolbarView.listenTo( editor.ui.focusTracker, 'change:isFocused', ( evt, name, isFocused ) => { + this.listenTo( editor.ui.focusTracker, 'change:isFocused', ( evt, name, isFocused ) => { if ( this._balloon.visibleView === this.toolbarView && !isFocused ) { this._hidePanel(); } @@ -81,20 +82,32 @@ export default class ContextualToolbar extends Plugin { } /** - * Handles {@link module:core/editor/editor~Editor#editing} selection change - * and show or hide toolbar. + * Handles {@link modules:engine/model/document#selection} change and show or hide toolbar. + * + * Note that in this case it's better to listen to {@link modules:engine/model/document modelDocument} + * selection instead of {@link modules:engine/view/document viewDocument} selection because the first one + * is not fired when text changes styles like bold or italic and toolbar doesn't blink. * * @private */ _handleSelectionChange() { - const toolbarView = this.toolbarView; - const editingView = this.editor.editing.view; + const selection = this.editor.document.selection; + + // This is internal plugin event which is fired 200 ms after selection last change (lodash#debounce). + // This is to makes easy test debounced action without need to use `setTimeout`. + // Because lodash keeps time related stuff in a closure it's not possible to override it + // by sinon fake timers. + const fireChangeDoneDebounced = debounce( () => this.fire( '_selectionChangeDone' ), 200 ); + + this.listenTo( selection, 'change:range', () => { + // Hide the toolbar when the selection starts changing. + this._hidePanel(); - // Hide panel when selection is changing. - toolbarView.listenTo( editingView, 'selectionChange', () => this._hidePanel() ); + // Show the toolbar attached to the selection when the selection stops changing. + fireChangeDoneDebounced(); + } ); - // Display panel attached to the selection when selection stops changing. - toolbarView.listenTo( editingView, 'selectionChangeDone', () => this._showPanel() ); + this.on( '_selectionChangeDone', () => this._showPanel() ); } /** @@ -105,7 +118,7 @@ export default class ContextualToolbar extends Plugin { _showPanel() { const editingView = this.editor.editing.view; - // Do not add panel to the balloon stack twice. + // Do not add toolbar to the balloon stack twice. if ( this._balloon.hasView( this.toolbarView ) ) { return; } @@ -136,13 +149,6 @@ export default class ContextualToolbar extends Plugin { [ positions.forwardSelection, positions.forwardSelectionAlternative ], } } ); - - // Update panel position when editor content has changed. - this.toolbarView.listenTo( editingView, 'render', () => { - if ( this._balloon.visibleView === this.toolbarView ) { - this._balloon.updatePosition(); - } - } ); } /** @@ -153,7 +159,6 @@ export default class ContextualToolbar extends Plugin { _hidePanel() { if ( this._balloon.hasView( this.toolbarView ) ) { this._balloon.remove( this.toolbarView ); - this.toolbarView.stopListening( this.editor.editing.view, 'render' ); } } } diff --git a/tests/toolbar/contextualtoolbar.js b/tests/toolbar/contextualtoolbar.js index 1c60eb37..8d67bed8 100644 --- a/tests/toolbar/contextualtoolbar.js +++ b/tests/toolbar/contextualtoolbar.js @@ -6,7 +6,6 @@ import ClassicTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/classictest import ContextualToolbar from '../../src/toolbar/contextualtoolbar'; import ContextualBalloon from '../../src/contextualballoon'; import ToolbarView from '../../src/toolbar/toolbarview'; -import ViewSelection from '@ckeditor/ckeditor5-engine/src/view/selection'; import Bold from '@ckeditor/ckeditor5-basic-styles/src/bold'; import Italic from '@ckeditor/ckeditor5-basic-styles/src/italic'; import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph'; @@ -14,7 +13,7 @@ import global from '@ckeditor/ckeditor5-utils/src/dom/global'; import { setData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model.js'; -/* global document, window */ +/* global document, window, setTimeout */ describe( 'ContextualToolbar', () => { let sandbox, editor, contextualToolbar, balloon, editorElement; @@ -64,10 +63,68 @@ describe( 'ContextualToolbar', () => { expect( contextualToolbar.toolbarView.items ).to.length( 2 ); } ); + it( 'should fire internal `_selectionChangeDone` event 200 ms after last selection change', () => { + // This test uses setTimeout to test lodash#debounce because sinon fake timers + // doesn't work with lodash. Lodash keeps time related stuff in a closure + // and sinon is not able to override it. + + const spy = sandbox.spy(); + setData( editor.document, '[bar]' ); + contextualToolbar.on( '_selectionChangeDone', spy ); + + editor.document.selection.fire( 'change:range' ); + + // Not yet. + sinon.assert.notCalled( spy ); + + // Lets wait 100 ms. + setTimeout( () => { + // Still not yet. + sinon.assert.notCalled( spy ); + + // Fire event one more time. + editor.document.selection.fire( 'change:range' ); + + // Another 100 ms waiting. + setTimeout( () => { + // Still not yet. + sinon.assert.notCalled( spy ); + + // Another 100 ms waiting. + setTimeout( () => { + // And here it is. + sinon.assert.calledOnce( spy ); + }, 100 ); + }, 100 ); + }, 100 ); + } ); + + it( 'should open when selection stops changing', () => { + setData( editor.document, '[bar]' ); + + expect( balloon.visibleView ).to.null; + + contextualToolbar.fire( '_selectionChangeDone' ); + + expect( balloon.visibleView ).to.equal( contextualToolbar.toolbarView ); + } ); + + it( 'should close when selection starts changing', () => { + setData( editor.document, '[bar]' ); + + contextualToolbar.fire( '_selectionChangeDone' ); + + expect( balloon.visibleView ).to.equal( contextualToolbar.toolbarView ); + + editor.document.selection.fire( 'change:range' ); + + expect( balloon.visibleView ).to.null; + } ); + it( 'should open below if the selection is forward', () => { setData( editor.document, '[bar]' ); - editor.editing.view.fire( 'selectionChangeDone' ); + contextualToolbar.fire( '_selectionChangeDone' ); expect( balloon.visibleView ).to.equal( contextualToolbar.toolbarView ); expect( balloon.view.top ).to.be.above( 310 ); @@ -84,7 +141,7 @@ describe( 'ContextualToolbar', () => { height: 310 } ); - editor.editing.view.fire( 'selectionChangeDone' ); + contextualToolbar.fire( '_selectionChangeDone' ); expect( balloon.visibleView ).to.equal( contextualToolbar.toolbarView ); expect( balloon.view.top ).to.be.below( 310 ); @@ -93,7 +150,7 @@ describe( 'ContextualToolbar', () => { it( 'should open above if the selection is backward', () => { setData( editor.document, '[bar]', { lastRangeBackward: true } ); - editor.editing.view.fire( 'selectionChangeDone' ); + contextualToolbar.fire( '_selectionChangeDone' ); expect( balloon.visibleView ).to.equal( contextualToolbar.toolbarView ); expect( balloon.view.top ).to.be.below( 100 ); @@ -110,7 +167,7 @@ describe( 'ContextualToolbar', () => { height: 905 } ); - editor.editing.view.fire( 'selectionChangeDone' ); + contextualToolbar.fire( '_selectionChangeDone' ); expect( balloon.visibleView ).to.equal( contextualToolbar.toolbarView ); expect( balloon.view.top ).to.be.above( 100 ); @@ -119,16 +176,13 @@ describe( 'ContextualToolbar', () => { it( 'should not open if the collapsed selection is moving', () => { setData( editor.document, 'ba[]r' ); - const oldSelection = editor.editing.view.selection; - const newSelection = new ViewSelection(); - - editor.editing.view.fire( 'selectionChange', { oldSelection, newSelection } ); - editor.editing.view.fire( 'selectionChangeDone' ); + editor.document.selection.fire( 'change:range' ); + contextualToolbar.fire( '_selectionChangeDone' ); setData( editor.document, 'b[]ar' ); - editor.editing.view.fire( 'selectionChange', { oldSelection, newSelection } ); - editor.editing.view.fire( 'selectionChangeDone' ); + editor.document.selection.fire( 'change:range' ); + contextualToolbar.fire( '_selectionChangeDone' ); expect( balloon.visibleView ).to.null; } ); @@ -137,7 +191,7 @@ describe( 'ContextualToolbar', () => { setData( editor.document, '[bar]' ); editor.ui.focusTracker.isFocused = true; - editor.editing.view.fire( 'selectionChangeDone' ); + contextualToolbar.fire( '_selectionChangeDone' ); expect( balloon.visibleView ).to.equal( contextualToolbar.toolbarView ); @@ -146,63 +200,18 @@ describe( 'ContextualToolbar', () => { expect( balloon.visibleView ).to.null; } ); - it( 'should hide if the selection is changing', () => { - setData( editor.document, '[bar]' ); - - editor.editing.view.fire( 'selectionChangeDone' ); - - expect( balloon.visibleView ).to.equal( contextualToolbar.toolbarView ); - - const oldSelection = editor.editing.view.selection; - const newSelection = new ViewSelection(); - - editor.editing.view.fire( 'selectionChange', { oldSelection, newSelection } ); - - expect( balloon.visibleView ).to.null; - } ); - it( 'should do nothing when panel is being added to balloon stack twice', () => { setData( editor.document, '[bar]' ); - editor.editing.view.fire( 'selectionChangeDone' ); + contextualToolbar.fire( '_selectionChangeDone' ); expect( balloon.visibleView ).to.equal( contextualToolbar.toolbarView ); expect( () => { - editor.editing.view.fire( 'selectionChangeDone' ); + contextualToolbar.fire( '_selectionChangeDone' ); } ).to.not.throw(); } ); - it( 'should update toolbar position when is visible and editor content has changed', () => { - const spy = sandbox.spy( balloon, 'updatePosition' ); - - setData( editor.document, 'ba[r]' ); - - editor.editing.view.fire( 'selectionChangeDone' ); - - sinon.assert.notCalled( spy ); - - editor.editing.view.fire( 'render' ); - - sinon.assert.calledOnce( spy ); - } ); - - it( 'should not update toolbar position when is added to the balloon stack but is not visible and editor content has changed', () => { - const spy = sandbox.spy( balloon, 'updatePosition' ); - - setData( editor.document, 'ba[r]' ); - - editor.editing.view.fire( 'selectionChangeDone' ); - - const viewMock = { destroy: () => {} }; - - balloon.add( { view: viewMock } ); - - editor.editing.view.fire( 'render' ); - - sinon.assert.notCalled( spy ); - } ); - function stubClientRects() { const editingView = editor.editing.view; const originalViewRangeToDom = editingView.domConverter.viewRangeToDom; From b2c7f3dd650a7c2c1895c9a5a7a0e980bf662983 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Fri, 7 Apr 2017 09:11:17 +0200 Subject: [PATCH 06/41] Improved ContextualBalloon#updatePosition(). --- src/contextualballoon.js | 15 +++++++++++---- tests/contextualballoon.js | 19 +++++++++++++++++++ 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/src/contextualballoon.js b/src/contextualballoon.js index 9586821c..10d9906d 100644 --- a/src/contextualballoon.js +++ b/src/contextualballoon.js @@ -10,6 +10,7 @@ import Plugin from '@ckeditor/ckeditor5-core/src/plugin'; import BalloonPanelView from './panel/balloon/balloonpanelview'; import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror'; +import nth from '@ckeditor/ckeditor5-utils/src/nth'; /** * Provides the common contextual balloon panel for the editor. @@ -151,10 +152,16 @@ export default class ContextualBalloon extends Plugin { } /** - * Updates the position of the balloon panel according to position data - * of the first view in the stack. + * Updates the position of the balloon panel according to the given position data + * or position data of the first view in the stack. + * + * @param {module:utils/dom/position~Options} [position] position options. */ - updatePosition() { + updatePosition( position ) { + if ( position ) { + nth( 0, this._stack )[ 1 ].position = position; + } + this.view.attachTo( this._getBalloonPosition() ); } @@ -178,7 +185,7 @@ export default class ContextualBalloon extends Plugin { * @returns {module:utils/dom/position~Options} */ _getBalloonPosition() { - return Array.from( this._stack.values() )[ 0 ].position; + return nth( 0, this._stack )[ 1 ].position; } /** diff --git a/tests/contextualballoon.js b/tests/contextualballoon.js index 3b339638..917a880a 100644 --- a/tests/contextualballoon.js +++ b/tests/contextualballoon.js @@ -251,6 +251,25 @@ describe( 'ContextualBalloon', () => { expect( balloon.view.attachTo.firstCall.args[ 0 ] ).to.deep.equal( { target: 'fake' } ); } ); + it( 'should attach balloon to the target using new position options', () => { + balloon.add( { + view: viewA, + position: { target: 'fake' } + } ); + + balloon.view.attachTo.reset(); + + balloon.updatePosition( { target: 'new' } ); + + expect( balloon.view.attachTo.calledOnce ); + expect( balloon.view.attachTo.firstCall.args[ 0 ] ).to.deep.equal( { target: 'new' } ); + + balloon.updatePosition(); + + expect( balloon.view.attachTo.calledTwice ); + expect( balloon.view.attachTo.firstCall.args[ 0 ] ).to.deep.equal( { target: 'new' } ); + } ); + it( 'should attach balloon to the target using the same position options as currently set when there is more than one view', () => { balloon.add( { view: viewA, From 2c1821fe6846455341aa9cd4a8e9043a45dc94b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Fri, 7 Apr 2017 10:42:59 +0200 Subject: [PATCH 07/41] Improved toolbar to work with collaborative editing. --- src/toolbar/contextualtoolbar.js | 97 ++++++++++++++++++-------- tests/toolbar/contextualtoolbar.js | 106 ++++++++++++++++++++++++++--- 2 files changed, 166 insertions(+), 37 deletions(-) diff --git a/src/toolbar/contextualtoolbar.js b/src/toolbar/contextualtoolbar.js index e4f9a09f..7f5ca0d9 100644 --- a/src/toolbar/contextualtoolbar.js +++ b/src/toolbar/contextualtoolbar.js @@ -47,6 +47,19 @@ export default class ContextualToolbar extends Plugin { */ this._balloon = this.editor.plugins.get( ContextualBalloon ); + /** + * This is internal plugin event which is fired 200 ms after selection last change (lodash#debounce). + * This is to makes easy test debounced action without need to use `setTimeout`. Lodash keeps time related + * stuff in a closure and it's not possible to override it by sinon fake timers. + * + * This debounced function is stored as a plugin property to make possible to cancel + * trailing debounced invocation on destroy. + * + * @private + * @member {Function} + */ + this._fireChangeDoneDebounced = debounce( () => this.fire( '_selectionChangeDone' ), 200 ); + // Attach lifecycle actions. this._handleSelectionChange(); this._handleFocusChange(); @@ -93,21 +106,19 @@ export default class ContextualToolbar extends Plugin { _handleSelectionChange() { const selection = this.editor.document.selection; - // This is internal plugin event which is fired 200 ms after selection last change (lodash#debounce). - // This is to makes easy test debounced action without need to use `setTimeout`. - // Because lodash keeps time related stuff in a closure it's not possible to override it - // by sinon fake timers. - const fireChangeDoneDebounced = debounce( () => this.fire( '_selectionChangeDone' ), 200 ); - - this.listenTo( selection, 'change:range', () => { - // Hide the toolbar when the selection starts changing. - this._hidePanel(); + this.listenTo( selection, 'change:range', ( evt, data ) => { + // When the selection is not changed by a collaboration and when is not collapsed. + if ( data.directChange || selection.isCollapsed ) { + // Hide the toolbar when the selection starts changing. + this._hidePanel(); + } - // Show the toolbar attached to the selection when the selection stops changing. - fireChangeDoneDebounced(); + // Fire internal `_selectionChangeDone` when the selection stops changing. + this._fireChangeDoneDebounced(); } ); - this.on( '_selectionChangeDone', () => this._showPanel() ); + // Hide the toolbar when the selection stops changing. + this.listenTo( this, '_selectionChangeDone', () => this._showPanel() ); } /** @@ -128,26 +139,15 @@ export default class ContextualToolbar extends Plugin { return; } - // Get direction of the selection. - const isBackward = editingView.selection.isBackward; - - // getBoundingClientRect() makes no sense when the selection spans across number - // of lines of text. Using getClientRects() allows us to browse micro–ranges - // that would normally make up the bounding client rect. - const rangeRects = editingView.domConverter.viewRangeToDom( editingView.selection.getFirstRange() ).getClientRects(); - - // Select the proper range rect depending on the direction of the selection. - const rangeRect = isBackward ? rangeRects.item( 0 ) : rangeRects.item( rangeRects.length - 1 ); - // Add panel to the common editor contextual balloon. this._balloon.add( { view: this.toolbarView, - position: { - target: rangeRect, - positions: isBackward ? - [ positions.backwardSelection, positions.backwardSelectionAlternative ] : - [ positions.forwardSelection, positions.forwardSelectionAlternative ], - } + position: this._getBalloonPositionData() + } ); + + // Update panel position when selection changes while balloon will be opened (by a collaboration). + this.listenTo( this.editor.editing.view, 'render', () => { + this._balloon.updatePosition( this._getBalloonPositionData() ); } ); } @@ -158,9 +158,48 @@ export default class ContextualToolbar extends Plugin { */ _hidePanel() { if ( this._balloon.hasView( this.toolbarView ) ) { + this.stopListening( this.editor.editing.view, 'render' ); this._balloon.remove( this.toolbarView ); } } + + /** + * Returns positioning options for the {@link #_balloon}. They control the way balloon is attached + * to the selection. + * + * @private + * @returns {module:utils/dom/position~Options} + */ + _getBalloonPositionData() { + const editingView = this.editor.editing.view; + + // Get direction of the selection. + const isBackward = editingView.selection.isBackward; + + // getBoundingClientRect() makes no sense when the selection spans across number + // of lines of text. Using getClientRects() allows us to browse micro–ranges + // that would normally make up the bounding client rect. + const rangeRects = editingView.domConverter.viewRangeToDom( editingView.selection.getFirstRange() ).getClientRects(); + + // Select the proper range rect depending on the direction of the selection. + const rangeRect = isBackward ? rangeRects.item( 0 ) : rangeRects.item( rangeRects.length - 1 ); + + return { + target: rangeRect, + positions: isBackward ? + [ positions.backwardSelection, positions.backwardSelectionAlternative ] : + [ positions.forwardSelection, positions.forwardSelectionAlternative ], + }; + } + + /** + * @inheritDoc + */ + destroy() { + this._fireChangeDoneDebounced.cancel(); + this.stopListening(); + super.destroy(); + } } // List of available toolbar positions. diff --git a/tests/toolbar/contextualtoolbar.js b/tests/toolbar/contextualtoolbar.js index 8d67bed8..6d95bfe6 100644 --- a/tests/toolbar/contextualtoolbar.js +++ b/tests/toolbar/contextualtoolbar.js @@ -44,7 +44,8 @@ describe( 'ContextualToolbar', () => { afterEach( () => { sandbox.restore(); - editor.destroy(); + + return editor.destroy(); } ); it( 'should be loaded', () => { @@ -63,7 +64,7 @@ describe( 'ContextualToolbar', () => { expect( contextualToolbar.toolbarView.items ).to.length( 2 ); } ); - it( 'should fire internal `_selectionChangeDone` event 200 ms after last selection change', () => { + it( 'should fire internal `_selectionChangeDone` event 200 ms after last selection change', ( done ) => { // This test uses setTimeout to test lodash#debounce because sinon fake timers // doesn't work with lodash. Lodash keeps time related stuff in a closure // and sinon is not able to override it. @@ -72,7 +73,7 @@ describe( 'ContextualToolbar', () => { setData( editor.document, '[bar]' ); contextualToolbar.on( '_selectionChangeDone', spy ); - editor.document.selection.fire( 'change:range' ); + editor.document.selection.fire( 'change:range', {} ); // Not yet. sinon.assert.notCalled( spy ); @@ -83,7 +84,7 @@ describe( 'ContextualToolbar', () => { sinon.assert.notCalled( spy ); // Fire event one more time. - editor.document.selection.fire( 'change:range' ); + editor.document.selection.fire( 'change:range', {} ); // Another 100 ms waiting. setTimeout( () => { @@ -94,6 +95,7 @@ describe( 'ContextualToolbar', () => { setTimeout( () => { // And here it is. sinon.assert.calledOnce( spy ); + done(); }, 100 ); }, 100 ); }, 100 ); @@ -109,14 +111,40 @@ describe( 'ContextualToolbar', () => { expect( balloon.visibleView ).to.equal( contextualToolbar.toolbarView ); } ); - it( 'should close when selection starts changing', () => { + it( 'should close when selection starts changing by a directChange', () => { setData( editor.document, '[bar]' ); contextualToolbar.fire( '_selectionChangeDone' ); expect( balloon.visibleView ).to.equal( contextualToolbar.toolbarView ); - editor.document.selection.fire( 'change:range' ); + editor.document.selection.fire( 'change:range', { directChange: true } ); + + expect( balloon.visibleView ).to.null; + } ); + + it( 'should not close when selection starts changing by not a directChange', () => { + setData( editor.document, '[bar]' ); + + contextualToolbar.fire( '_selectionChangeDone' ); + + expect( balloon.visibleView ).to.equal( contextualToolbar.toolbarView ); + + editor.document.selection.fire( 'change:range', { directChange: false } ); + + expect( balloon.visibleView ).to.equal( contextualToolbar.toolbarView ); + } ); + + it( 'should close when selection starts changing by not a directChange but will become collapsed', () => { + setData( editor.document, '[bar]' ); + + contextualToolbar.fire( '_selectionChangeDone' ); + + // Collapse range silently (without firing `change:range` { directChange: true } event). + const range = editor.document.selection._ranges[ 0 ]; + range.end = range.start; + + editor.document.selection.fire( 'change:range', { directChange: false } ); expect( balloon.visibleView ).to.null; } ); @@ -124,6 +152,14 @@ describe( 'ContextualToolbar', () => { it( 'should open below if the selection is forward', () => { setData( editor.document, '[bar]' ); + // Mock limiter. + mockBoundingBox( document.body, { + left: 0, + width: 1000, + top: 0, + height: 1000 + } ); + contextualToolbar.fire( '_selectionChangeDone' ); expect( balloon.visibleView ).to.equal( contextualToolbar.toolbarView ); @@ -150,6 +186,14 @@ describe( 'ContextualToolbar', () => { it( 'should open above if the selection is backward', () => { setData( editor.document, '[bar]', { lastRangeBackward: true } ); + // Mock limiter. + mockBoundingBox( document.body, { + left: 0, + width: 1000, + top: 0, + height: 1000 + } ); + contextualToolbar.fire( '_selectionChangeDone' ); expect( balloon.visibleView ).to.equal( contextualToolbar.toolbarView ); @@ -176,12 +220,12 @@ describe( 'ContextualToolbar', () => { it( 'should not open if the collapsed selection is moving', () => { setData( editor.document, 'ba[]r' ); - editor.document.selection.fire( 'change:range' ); + editor.document.selection.fire( 'change:range', {} ); contextualToolbar.fire( '_selectionChangeDone' ); setData( editor.document, 'b[]ar' ); - editor.document.selection.fire( 'change:range' ); + editor.document.selection.fire( 'change:range', {} ); contextualToolbar.fire( '_selectionChangeDone' ); expect( balloon.visibleView ).to.null; @@ -212,6 +256,52 @@ describe( 'ContextualToolbar', () => { } ).to.not.throw(); } ); + it( 'should update balloon position when toolbar is opened and editor content has changed', () => { + const spy = sandbox.spy( balloon, 'updatePosition' ); + + setData( editor.document, '[bar]' ); + + contextualToolbar.fire( '_selectionChangeDone' ); + + sinon.assert.notCalled( spy ); + + editor.editing.view.fire( 'render' ); + + sinon.assert.calledOnce( spy ); + } ); + + it( 'should update balloon position when toolbar is closed', () => { + const spy = sandbox.spy( balloon, 'updatePosition' ); + + setData( editor.document, '[bar]' ); + + contextualToolbar.fire( '_selectionChangeDone' ); + + // Hide toolbar. + editor.document.selection.fire( 'change:range', { directChange: true } ); + + editor.editing.view.fire( 'render' ); + + sinon.assert.notCalled( spy ); + } ); + + describe( 'destroy()', () => { + it( 'should not fire `_selectionChangeDone` after plugin destroy', ( done ) => { + const spy = sandbox.spy(); + + contextualToolbar.on( '_selectionChangeDone', spy ); + + editor.document.selection.fire( 'change:range', { directChange: true } ); + + contextualToolbar.destroy(); + + setTimeout( () => { + sinon.assert.notCalled( spy ); + done(); + }, 200 ); + } ); + } ); + function stubClientRects() { const editingView = editor.editing.view; const originalViewRangeToDom = editingView.domConverter.viewRangeToDom; From 7c0544c6bbc20896af8768d3115dfccf2d58af01 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Fri, 7 Apr 2017 11:15:23 +0200 Subject: [PATCH 08/41] Improved docs. --- src/toolbar/contextualtoolbar.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/toolbar/contextualtoolbar.js b/src/toolbar/contextualtoolbar.js index 7f5ca0d9..4f407abb 100644 --- a/src/toolbar/contextualtoolbar.js +++ b/src/toolbar/contextualtoolbar.js @@ -99,7 +99,7 @@ export default class ContextualToolbar extends Plugin { * * Note that in this case it's better to listen to {@link modules:engine/model/document modelDocument} * selection instead of {@link modules:engine/view/document viewDocument} selection because the first one - * is not fired when text changes styles like bold or italic and toolbar doesn't blink. + * doesn't fire `change` event after text styles changes (like bold or italic) and toolbar doesn't blink. * * @private */ From 7befb56d13af4b5a576babf66107f078574be250 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Fri, 7 Apr 2017 11:15:48 +0200 Subject: [PATCH 09/41] Changed absolute import path to relative. --- src/toolbar/contextualtoolbar.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/toolbar/contextualtoolbar.js b/src/toolbar/contextualtoolbar.js index 4f407abb..fe63b5f0 100644 --- a/src/toolbar/contextualtoolbar.js +++ b/src/toolbar/contextualtoolbar.js @@ -8,7 +8,7 @@ */ import Plugin from '@ckeditor/ckeditor5-core/src/plugin'; -import ContextualBalloon from '@ckeditor/ckeditor5-ui/src/contextualballoon'; +import ContextualBalloon from '../contextualballoon'; import ToolbarView from './toolbarview'; import BalloonPanelView from '../panel/balloon/balloonpanelview.js'; import debounce from '@ckeditor/ckeditor5-utils/src/lib/lodash/debounce'; From 3d09f5f2c62ad3cb9161dea72ef07f812626de2d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Fri, 7 Apr 2017 13:09:22 +0200 Subject: [PATCH 10/41] Improved code style. --- src/contextualballoon.js | 11 +++++++---- tests/contextualballoon.js | 5 +++-- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/src/contextualballoon.js b/src/contextualballoon.js index 10d9906d..7d0cd72a 100644 --- a/src/contextualballoon.js +++ b/src/contextualballoon.js @@ -29,6 +29,13 @@ import nth from '@ckeditor/ckeditor5-utils/src/nth'; * @extends module:core/plugin~Plugin */ export default class ContextualBalloon extends Plugin { + /** + * @inheritDoc + */ + static get pluginName() { + return 'contextualballoon'; + } + /** * @inheritDoc */ @@ -54,10 +61,6 @@ export default class ContextualBalloon extends Plugin { this.editor.ui.view.body.add( this.view ); } - static get pluginName() { - return 'contextualballoon'; - } - /** * Returns the currently visible view or `null` when there are no * views in the stack. diff --git a/tests/contextualballoon.js b/tests/contextualballoon.js index 917a880a..e2e3e3bc 100644 --- a/tests/contextualballoon.js +++ b/tests/contextualballoon.js @@ -39,13 +39,14 @@ describe( 'ContextualBalloon', () => { editor.destroy(); } ); - it( 'should be a plugin instance', () => { + it( 'should create a plugin instance', () => { expect( balloon ).to.instanceof( Plugin ); + expect( balloon ).to.instanceof( ContextualBalloon ); } ); describe( 'pluginName', () => { it( 'should return plugin by name', () => { - expect( editor.plugins.get( 'contextualballoon' ) ).to.instanceof( ContextualBalloon ); + expect( editor.plugins.get( 'contextualballoon' ) ).to.equal( balloon ); } ); } ); From 1b736f59041f110e2ed55a36f20fea39e0410daa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Fri, 7 Apr 2017 13:14:26 +0200 Subject: [PATCH 11/41] Added plugin name to the ContextualToolbar plugin. --- src/toolbar/contextualtoolbar.js | 7 +++++++ tests/toolbar/contextualtoolbar.js | 10 +++++++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/src/toolbar/contextualtoolbar.js b/src/toolbar/contextualtoolbar.js index fe63b5f0..aefac6c5 100644 --- a/src/toolbar/contextualtoolbar.js +++ b/src/toolbar/contextualtoolbar.js @@ -21,6 +21,13 @@ import debounce from '@ckeditor/ckeditor5-utils/src/lib/lodash/debounce'; * @extends module:core/plugin~Plugin */ export default class ContextualToolbar extends Plugin { + /** + * @inheritDoc + */ + static get pluginName() { + return 'contextualtoolbar'; + } + /** * @inheritDoc */ diff --git a/tests/toolbar/contextualtoolbar.js b/tests/toolbar/contextualtoolbar.js index 6d95bfe6..307946a4 100644 --- a/tests/toolbar/contextualtoolbar.js +++ b/tests/toolbar/contextualtoolbar.js @@ -6,6 +6,7 @@ import ClassicTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/classictest import ContextualToolbar from '../../src/toolbar/contextualtoolbar'; import ContextualBalloon from '../../src/contextualballoon'; import ToolbarView from '../../src/toolbar/toolbarview'; +import Plugin from '@ckeditor/ckeditor5-core/src/plugin'; import Bold from '@ckeditor/ckeditor5-basic-styles/src/bold'; import Italic from '@ckeditor/ckeditor5-basic-styles/src/italic'; import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph'; @@ -48,7 +49,8 @@ describe( 'ContextualToolbar', () => { return editor.destroy(); } ); - it( 'should be loaded', () => { + it( 'should create a plugin instance', () => { + expect( contextualToolbar ).to.instanceOf( Plugin ); expect( contextualToolbar ).to.instanceOf( ContextualToolbar ); } ); @@ -285,6 +287,12 @@ describe( 'ContextualToolbar', () => { sinon.assert.notCalled( spy ); } ); + describe( 'pluginName', () => { + it( 'should return plugin by name', () => { + expect( editor.plugins.get( 'contextualballoon' ) ).to.equal( balloon ); + } ); + } ); + describe( 'destroy()', () => { it( 'should not fire `_selectionChangeDone` after plugin destroy', ( done ) => { const spy = sandbox.spy(); From ed637b90468b66eba53605b4f25d3db396d77943 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Fri, 7 Apr 2017 13:15:47 +0200 Subject: [PATCH 12/41] Changed setTimeout value of failing test on travis. --- tests/toolbar/contextualtoolbar.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/toolbar/contextualtoolbar.js b/tests/toolbar/contextualtoolbar.js index 307946a4..94ea90d1 100644 --- a/tests/toolbar/contextualtoolbar.js +++ b/tests/toolbar/contextualtoolbar.js @@ -93,13 +93,13 @@ describe( 'ContextualToolbar', () => { // Still not yet. sinon.assert.notCalled( spy ); - // Another 100 ms waiting. + // Another 101 ms waiting. setTimeout( () => { // And here it is. sinon.assert.calledOnce( spy ); done(); }, 100 ); - }, 100 ); + }, 101 ); }, 100 ); } ); From 32a467366605138f5e2c5188ed9485860eb11d35 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Fri, 7 Apr 2017 14:42:28 +0200 Subject: [PATCH 13/41] Improved adding views to the balloon stack. --- src/contextualballoon.js | 8 +++++++- tests/contextualballoon.js | 30 ++++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/src/contextualballoon.js b/src/contextualballoon.js index 7d0cd72a..76f9a4bf 100644 --- a/src/contextualballoon.js +++ b/src/contextualballoon.js @@ -177,7 +177,13 @@ export default class ContextualBalloon extends Plugin { */ _show( view ) { this.view.content.add( view ); - this.view.attachTo( this._getBalloonPosition() ); + + // When view is not rendered we need to wait for it. See: https://github.com/ckeditor/ckeditor5-ui/issues/187. + if ( !view.ready ) { + view.once( 'change:ready', () => this.view.attachTo( this._getBalloonPosition() ) ); + } else { + this.view.attachTo( this._getBalloonPosition() ); + } } /** diff --git a/tests/contextualballoon.js b/tests/contextualballoon.js index e2e3e3bc..89b5867c 100644 --- a/tests/contextualballoon.js +++ b/tests/contextualballoon.js @@ -97,12 +97,38 @@ describe( 'ContextualBalloon', () => { position: { target: 'fake' } } ); + viewA.ready = true; + expect( balloon.view.content.length ).to.equal( 1 ); expect( balloon.view.content.get( 0 ) ).to.deep.equal( viewA ); expect( balloon.view.attachTo.calledOnce ).to.true; expect( balloon.view.attachTo.firstCall.args[ 0 ] ).to.deep.equal( { target: 'fake' } ); } ); + it( 'should wait for view init', () => { + balloon.add( { + view: viewA, + position: { target: 'fake' } + } ); + + sinon.assert.notCalled( balloon.view.attachTo ); + + viewA.ready = true; + + sinon.assert.calledOnce( balloon.view.attachTo ); + } ); + + it( 'should not wait when view is ready', () => { + viewA.ready = true; + + balloon.add( { + view: viewA, + position: { target: 'fake' } + } ); + + sinon.assert.calledOnce( balloon.view.attachTo ); + } ); + it( 'should throw an error when try to add the same view more than once', () => { balloon.add( { view: viewA, @@ -138,11 +164,15 @@ describe( 'ContextualBalloon', () => { position: { target: 'fake', foo: 'bar' } } ); + viewA.ready = true; + balloon.add( { view: viewB, position: { target: 'fake', bar: 'biz' } } ); + viewB.ready = true; + expect( balloon.view.attachTo.calledTwice ).to.true; expect( balloon.view.attachTo.firstCall.args[ 0 ] ).to.deep.equal( { From 9d5862033e7e46533cc154b8ef113c75297ec021 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Fri, 7 Apr 2017 14:52:13 +0200 Subject: [PATCH 14/41] Fixed failing tests. --- tests/toolbar/contextualtoolbar.js | 21 ++++----------------- 1 file changed, 4 insertions(+), 17 deletions(-) diff --git a/tests/toolbar/contextualtoolbar.js b/tests/toolbar/contextualtoolbar.js index 94ea90d1..d38d47a3 100644 --- a/tests/toolbar/contextualtoolbar.js +++ b/tests/toolbar/contextualtoolbar.js @@ -36,10 +36,13 @@ describe( 'ContextualToolbar', () => { contextualToolbar = editor.plugins.get( ContextualToolbar ); balloon = editor.plugins.get( ContextualBalloon ); + stubClientRects(); + // Focus the engine. editor.editing.view.isFocused = true; - stubClientRects(); + // Init child view. + return contextualToolbar.toolbarView.init(); } ); } ); @@ -154,14 +157,6 @@ describe( 'ContextualToolbar', () => { it( 'should open below if the selection is forward', () => { setData( editor.document, '[bar]' ); - // Mock limiter. - mockBoundingBox( document.body, { - left: 0, - width: 1000, - top: 0, - height: 1000 - } ); - contextualToolbar.fire( '_selectionChangeDone' ); expect( balloon.visibleView ).to.equal( contextualToolbar.toolbarView ); @@ -188,14 +183,6 @@ describe( 'ContextualToolbar', () => { it( 'should open above if the selection is backward', () => { setData( editor.document, '[bar]', { lastRangeBackward: true } ); - // Mock limiter. - mockBoundingBox( document.body, { - left: 0, - width: 1000, - top: 0, - height: 1000 - } ); - contextualToolbar.fire( '_selectionChangeDone' ); expect( balloon.visibleView ).to.equal( contextualToolbar.toolbarView ); From f773f233785bac25e6cfc625e98bc40ebc33e55b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Sun, 9 Apr 2017 23:52:15 +0200 Subject: [PATCH 15/41] Improved docs. --- src/toolbar/contextualtoolbar.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/toolbar/contextualtoolbar.js b/src/toolbar/contextualtoolbar.js index aefac6c5..5a3b657e 100644 --- a/src/toolbar/contextualtoolbar.js +++ b/src/toolbar/contextualtoolbar.js @@ -74,7 +74,7 @@ export default class ContextualToolbar extends Plugin { /** * Creates toolbar components based on given configuration. - * This need to be done when all plugins will be ready. + * This needs to be done when all plugins will be ready. * * @inheritDoc */ @@ -106,7 +106,7 @@ export default class ContextualToolbar extends Plugin { * * Note that in this case it's better to listen to {@link modules:engine/model/document modelDocument} * selection instead of {@link modules:engine/view/document viewDocument} selection because the first one - * doesn't fire `change` event after text styles changes (like bold or italic) and toolbar doesn't blink. + * doesn't fire `change` event after text style change (like bold or italic) and toolbar doesn't blink. * * @private */ From 2226fe0b5c9cc8220e4abb60d74189bcf7b43c1a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Sun, 9 Apr 2017 23:52:29 +0200 Subject: [PATCH 16/41] Improved tests. --- tests/toolbar/contextualtoolbar.js | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/tests/toolbar/contextualtoolbar.js b/tests/toolbar/contextualtoolbar.js index d38d47a3..17003b5a 100644 --- a/tests/toolbar/contextualtoolbar.js +++ b/tests/toolbar/contextualtoolbar.js @@ -55,16 +55,13 @@ describe( 'ContextualToolbar', () => { it( 'should create a plugin instance', () => { expect( contextualToolbar ).to.instanceOf( Plugin ); expect( contextualToolbar ).to.instanceOf( ContextualToolbar ); + expect( contextualToolbar.toolbarView ).to.instanceof( ToolbarView ); } ); it( 'should load ContextualBalloon', () => { expect( balloon ).to.instanceof( ContextualBalloon ); } ); - it( 'should create plugin instance with properties', () => { - expect( contextualToolbar.toolbarView ).to.instanceof( ToolbarView ); - } ); - it( 'should create components from config', () => { expect( contextualToolbar.toolbarView.items ).to.length( 2 ); } ); From 335e0e335956d2ef498101228a614706c32d44d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Tue, 11 Apr 2017 11:03:17 +0200 Subject: [PATCH 17/41] Fixed failing test. --- tests/toolbar/contextualtoolbar.js | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/toolbar/contextualtoolbar.js b/tests/toolbar/contextualtoolbar.js index 17003b5a..5221142a 100644 --- a/tests/toolbar/contextualtoolbar.js +++ b/tests/toolbar/contextualtoolbar.js @@ -154,6 +154,14 @@ describe( 'ContextualToolbar', () => { it( 'should open below if the selection is forward', () => { setData( editor.document, '[bar]' ); + // Mock limiter rect. + mockBoundingBox( document.body, { + left: 0, + width: 1000, + top: 0, + height: 1000 + } ); + contextualToolbar.fire( '_selectionChangeDone' ); expect( balloon.visibleView ).to.equal( contextualToolbar.toolbarView ); @@ -180,6 +188,14 @@ describe( 'ContextualToolbar', () => { it( 'should open above if the selection is backward', () => { setData( editor.document, '[bar]', { lastRangeBackward: true } ); + // Mock limiter rect. + mockBoundingBox( document.body, { + left: 0, + width: 1000, + top: 0, + height: 1000 + } ); + contextualToolbar.fire( '_selectionChangeDone' ); expect( balloon.visibleView ).to.equal( contextualToolbar.toolbarView ); From b74702f7e139f87508eabf4cd97224a1cbaff4a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Tue, 11 Apr 2017 15:36:42 +0200 Subject: [PATCH 18/41] Moved ContextualToolbar positions to the BalloonPanelView#defaultPositions. --- src/panel/balloon/balloonpanelview.js | 60 +++++++++++++++++ src/toolbar/contextualtoolbar.js | 54 ++------------- tests/panel/balloon/balloonpanelview.js | 90 +++++++++++++++++++++++++ 3 files changed, 154 insertions(+), 50 deletions(-) diff --git a/src/panel/balloon/balloonpanelview.js b/src/panel/balloon/balloonpanelview.js index a2529457..41aff86f 100644 --- a/src/panel/balloon/balloonpanelview.js +++ b/src/panel/balloon/balloonpanelview.js @@ -359,6 +359,42 @@ BalloonPanelView.arrowVerticalOffset = 15; * V * [ Target ] * + * + * * Forward selection: + * + * [text range] + * ^ + * +-----------------+ + * | Balloon | + * +-----------------+ + * + * + * * Forward selection alternative: + * + * +-----------------+ + * | Balloon | + * +-----------------+ + * V + * [text range] + * + * + * * Backward selection: + * + * +-----------------+ + * | Balloon | + * +-----------------+ + * V + * [text range] + * + * + * * Backward selection alternative: + * + * [text range] + * ^ + * +-----------------+ + * | Balloon | + * +-----------------+ + * * See {@link module:ui/panel/balloon/balloonpanelview~BalloonPanelView#attachTo}. * * Positioning functions must be compatible with {@link module:utils/dom/position~Position}. @@ -391,5 +427,29 @@ BalloonPanelView.defaultPositions = { top: targetRect.top - balloonRect.height - BalloonPanelView.arrowVerticalOffset, left: targetRect.left + targetRect.width / 2 - balloonRect.width + BalloonPanelView.arrowHorizontalOffset, name: 'arrow_nw' + } ), + + forwardSelection: ( targetRect, balloonRect ) => ( { + top: targetRect.bottom + BalloonPanelView.arrowVerticalOffset, + left: targetRect.right - balloonRect.width / 2, + name: 'arrow_s' + } ), + + forwardSelectionAlternative: ( targetRect, balloonRect ) => ( { + top: targetRect.top - balloonRect.height - BalloonPanelView.arrowVerticalOffset, + left: targetRect.right - balloonRect.width / 2, + name: 'arrow_n' + } ), + + backwardSelection: ( targetRect, balloonRect ) => ( { + top: targetRect.top - balloonRect.height - BalloonPanelView.arrowVerticalOffset, + left: targetRect.left - balloonRect.width / 2, + name: 'arrow_n' + } ), + + backwardSelectionAlternative: ( targetRect, balloonRect ) => ( { + top: targetRect.bottom + BalloonPanelView.arrowVerticalOffset, + left: targetRect.left - balloonRect.width / 2, + name: 'arrow_s' } ) }; diff --git a/src/toolbar/contextualtoolbar.js b/src/toolbar/contextualtoolbar.js index 5a3b657e..62db4ca3 100644 --- a/src/toolbar/contextualtoolbar.js +++ b/src/toolbar/contextualtoolbar.js @@ -13,6 +13,8 @@ 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. * @@ -194,8 +196,8 @@ export default class ContextualToolbar extends Plugin { return { target: rangeRect, positions: isBackward ? - [ positions.backwardSelection, positions.backwardSelectionAlternative ] : - [ positions.forwardSelection, positions.forwardSelectionAlternative ], + [ defaultPositions.backwardSelection, defaultPositions.backwardSelectionAlternative ] : + [ defaultPositions.forwardSelection, defaultPositions.forwardSelectionAlternative ], }; } @@ -208,51 +210,3 @@ export default class ContextualToolbar extends Plugin { super.destroy(); } } - -// List of available toolbar positions. -const arrowVOffset = BalloonPanelView.arrowVerticalOffset; -const positions = { - // [text range] - // ^ - // +-----------------+ - // | Balloon | - // +-----------------+ - forwardSelection: ( targetRect, balloonRect ) => ( { - top: targetRect.bottom + arrowVOffset, - left: targetRect.right - balloonRect.width / 2, - name: 's' - } ), - - // +-----------------+ - // | Balloon | - // +-----------------+ - // V - // [text range] - forwardSelectionAlternative: ( targetRect, balloonRect ) => ( { - top: targetRect.top - balloonRect.height - arrowVOffset, - left: targetRect.right - balloonRect.width / 2, - name: 'n' - } ), - - // +-----------------+ - // | Balloon | - // +-----------------+ - // V - // [text range] - backwardSelection: ( targetRect, balloonRect ) => ( { - top: targetRect.top - balloonRect.height - arrowVOffset, - left: targetRect.left - balloonRect.width / 2, - name: 'n' - } ), - - // [text range] - // ^ - // +-----------------+ - // | Balloon | - // +-----------------+ - backwardSelectionAlternative: ( targetRect, balloonRect ) => ( { - top: targetRect.bottom + arrowVOffset, - left: targetRect.left - balloonRect.width / 2, - name: 's' - } ) -}; diff --git a/tests/panel/balloon/balloonpanelview.js b/tests/panel/balloon/balloonpanelview.js index 00bfb396..01e93c39 100644 --- a/tests/panel/balloon/balloonpanelview.js +++ b/tests/panel/balloon/balloonpanelview.js @@ -630,6 +630,96 @@ describe( 'BalloonPanelView', () => { } ); } ); } ); + + describe( 'defaultPositions', () => { + let positions, balloonRect, targetRect; + + beforeEach( () => { + positions = BalloonPanelView.defaultPositions; + + targetRect = { + top: 100, + bottom: 200, + left: 100, + right: 200, + width: 100, + height: 100 + }; + + balloonRect = { + top: 0, + bottom: 0, + left: 0, + right: 0, + width: 50, + height: 50 + }; + } ); + + it( 'se', () => { + expect( positions.se( targetRect ) ).to.deep.equal( { + top: 215, + left: 120, + name: 'arrow_se' + } ); + } ); + + it( 'sw', () => { + expect( positions.sw( targetRect, balloonRect ) ).to.deep.equal( { + top: 215, + left: 130, + name: 'arrow_sw' + } ); + } ); + + it( 'ne', () => { + expect( positions.ne( targetRect, balloonRect ) ).to.deep.equal( { + top: 35, + left: 120, + name: 'arrow_ne' + } ); + } ); + + it( 'nw', () => { + expect( positions.nw( targetRect, balloonRect ) ).to.deep.equal( { + top: 35, + left: 130, + name: 'arrow_nw' + } ); + } ); + + it( 'forwardSelection', () => { + expect( positions.forwardSelection( targetRect, balloonRect ) ).to.deep.equal( { + top: 215, + left: 175, + name: 'arrow_s' + } ); + } ); + + it( 'forwardSelectionAlternative', () => { + expect( positions.forwardSelectionAlternative( targetRect, balloonRect ) ).to.deep.equal( { + top: 35, + left: 175, + name: 'arrow_n' + } ); + } ); + + it( 'backwardSelection', () => { + expect( positions.backwardSelection( targetRect, balloonRect ) ).to.deep.equal( { + top: 35, + left: 75, + name: 'arrow_n' + } ); + } ); + + it( 'backwardSelectionAlternative', () => { + expect( positions.backwardSelectionAlternative( targetRect, balloonRect ) ).to.deep.equal( { + top: 215, + left: 75, + name: 'arrow_s' + } ); + } ); + } ); } ); function mockBoundingBox( element, data ) { From 52966dc6bc5ddee794f22f5820417cbbbb9eba1a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Tue, 11 Apr 2017 15:48:13 +0200 Subject: [PATCH 19/41] Renamed `_selectionChangeDone` event to `selectionChangeDebounced`. --- src/toolbar/contextualtoolbar.js | 25 +++++++++++++------- tests/toolbar/contextualtoolbar.js | 38 +++++++++++++++--------------- 2 files changed, 35 insertions(+), 28 deletions(-) diff --git a/src/toolbar/contextualtoolbar.js b/src/toolbar/contextualtoolbar.js index 62db4ca3..68f5f001 100644 --- a/src/toolbar/contextualtoolbar.js +++ b/src/toolbar/contextualtoolbar.js @@ -57,17 +57,15 @@ export default class ContextualToolbar extends Plugin { this._balloon = this.editor.plugins.get( ContextualBalloon ); /** - * This is internal plugin event which is fired 200 ms after selection last change (lodash#debounce). - * This is to makes easy test debounced action without need to use `setTimeout`. Lodash keeps time related - * stuff in a closure and it's not possible to override it by sinon fake timers. + * Fires {@link #event:_selectionChangeDebounced} event using `lodash#debounce`. * - * This debounced function is stored as a plugin property to make possible to cancel + * This function is stored as a plugin property to make possible to cancel * trailing debounced invocation on destroy. * * @private * @member {Function} */ - this._fireChangeDoneDebounced = debounce( () => this.fire( '_selectionChangeDone' ), 200 ); + this._fireSelectionChangeDebounced = debounce( () => this.fire( '_selectionChangeDebounced' ), 200 ); // Attach lifecycle actions. this._handleSelectionChange(); @@ -122,12 +120,12 @@ export default class ContextualToolbar extends Plugin { this._hidePanel(); } - // Fire internal `_selectionChangeDone` when the selection stops changing. - this._fireChangeDoneDebounced(); + // Fire internal `_selectionChangeDebounced` when the selection stops changing. + this._fireSelectionChangeDebounced(); } ); // Hide the toolbar when the selection stops changing. - this.listenTo( this, '_selectionChangeDone', () => this._showPanel() ); + this.listenTo( this, '_selectionChangeDebounced', () => this._showPanel() ); } /** @@ -205,8 +203,17 @@ export default class ContextualToolbar extends Plugin { * @inheritDoc */ destroy() { - this._fireChangeDoneDebounced.cancel(); + this._fireSelectionChangeDebounced.cancel(); this.stopListening(); super.destroy(); } + + /** + * This is internal plugin event which is fired 200 ms after model selection last change (lodash#debounce). + * This is to makes easy test debounced action without need to use `setTimeout`. Lodash keeps time related + * stuff in a closure and it's not possible to override it by sinon fake timers. + * + * @private + * @event _selectionChangeDebounced + */ } diff --git a/tests/toolbar/contextualtoolbar.js b/tests/toolbar/contextualtoolbar.js index 5221142a..8d5f0495 100644 --- a/tests/toolbar/contextualtoolbar.js +++ b/tests/toolbar/contextualtoolbar.js @@ -66,14 +66,14 @@ describe( 'ContextualToolbar', () => { expect( contextualToolbar.toolbarView.items ).to.length( 2 ); } ); - it( 'should fire internal `_selectionChangeDone` event 200 ms after last selection change', ( done ) => { + it( 'should fire internal `_selectionChangeDebounced` event 200 ms after last selection change', ( done ) => { // This test uses setTimeout to test lodash#debounce because sinon fake timers // doesn't work with lodash. Lodash keeps time related stuff in a closure // and sinon is not able to override it. const spy = sandbox.spy(); setData( editor.document, '[bar]' ); - contextualToolbar.on( '_selectionChangeDone', spy ); + contextualToolbar.on( '_selectionChangeDebounced', spy ); editor.document.selection.fire( 'change:range', {} ); @@ -108,7 +108,7 @@ describe( 'ContextualToolbar', () => { expect( balloon.visibleView ).to.null; - contextualToolbar.fire( '_selectionChangeDone' ); + contextualToolbar.fire( '_selectionChangeDebounced' ); expect( balloon.visibleView ).to.equal( contextualToolbar.toolbarView ); } ); @@ -116,7 +116,7 @@ describe( 'ContextualToolbar', () => { it( 'should close when selection starts changing by a directChange', () => { setData( editor.document, '[bar]' ); - contextualToolbar.fire( '_selectionChangeDone' ); + contextualToolbar.fire( '_selectionChangeDebounced' ); expect( balloon.visibleView ).to.equal( contextualToolbar.toolbarView ); @@ -128,7 +128,7 @@ describe( 'ContextualToolbar', () => { it( 'should not close when selection starts changing by not a directChange', () => { setData( editor.document, '[bar]' ); - contextualToolbar.fire( '_selectionChangeDone' ); + contextualToolbar.fire( '_selectionChangeDebounced' ); expect( balloon.visibleView ).to.equal( contextualToolbar.toolbarView ); @@ -140,7 +140,7 @@ describe( 'ContextualToolbar', () => { it( 'should close when selection starts changing by not a directChange but will become collapsed', () => { setData( editor.document, '[bar]' ); - contextualToolbar.fire( '_selectionChangeDone' ); + contextualToolbar.fire( '_selectionChangeDebounced' ); // Collapse range silently (without firing `change:range` { directChange: true } event). const range = editor.document.selection._ranges[ 0 ]; @@ -162,7 +162,7 @@ describe( 'ContextualToolbar', () => { height: 1000 } ); - contextualToolbar.fire( '_selectionChangeDone' ); + contextualToolbar.fire( '_selectionChangeDebounced' ); expect( balloon.visibleView ).to.equal( contextualToolbar.toolbarView ); expect( balloon.view.top ).to.be.above( 310 ); @@ -179,7 +179,7 @@ describe( 'ContextualToolbar', () => { height: 310 } ); - contextualToolbar.fire( '_selectionChangeDone' ); + contextualToolbar.fire( '_selectionChangeDebounced' ); expect( balloon.visibleView ).to.equal( contextualToolbar.toolbarView ); expect( balloon.view.top ).to.be.below( 310 ); @@ -196,7 +196,7 @@ describe( 'ContextualToolbar', () => { height: 1000 } ); - contextualToolbar.fire( '_selectionChangeDone' ); + contextualToolbar.fire( '_selectionChangeDebounced' ); expect( balloon.visibleView ).to.equal( contextualToolbar.toolbarView ); expect( balloon.view.top ).to.be.below( 100 ); @@ -213,7 +213,7 @@ describe( 'ContextualToolbar', () => { height: 905 } ); - contextualToolbar.fire( '_selectionChangeDone' ); + contextualToolbar.fire( '_selectionChangeDebounced' ); expect( balloon.visibleView ).to.equal( contextualToolbar.toolbarView ); expect( balloon.view.top ).to.be.above( 100 ); @@ -223,12 +223,12 @@ describe( 'ContextualToolbar', () => { setData( editor.document, 'ba[]r' ); editor.document.selection.fire( 'change:range', {} ); - contextualToolbar.fire( '_selectionChangeDone' ); + contextualToolbar.fire( '_selectionChangeDebounced' ); setData( editor.document, 'b[]ar' ); editor.document.selection.fire( 'change:range', {} ); - contextualToolbar.fire( '_selectionChangeDone' ); + contextualToolbar.fire( '_selectionChangeDebounced' ); expect( balloon.visibleView ).to.null; } ); @@ -237,7 +237,7 @@ describe( 'ContextualToolbar', () => { setData( editor.document, '[bar]' ); editor.ui.focusTracker.isFocused = true; - contextualToolbar.fire( '_selectionChangeDone' ); + contextualToolbar.fire( '_selectionChangeDebounced' ); expect( balloon.visibleView ).to.equal( contextualToolbar.toolbarView ); @@ -249,12 +249,12 @@ describe( 'ContextualToolbar', () => { it( 'should do nothing when panel is being added to balloon stack twice', () => { setData( editor.document, '[bar]' ); - contextualToolbar.fire( '_selectionChangeDone' ); + contextualToolbar.fire( '_selectionChangeDebounced' ); expect( balloon.visibleView ).to.equal( contextualToolbar.toolbarView ); expect( () => { - contextualToolbar.fire( '_selectionChangeDone' ); + contextualToolbar.fire( '_selectionChangeDebounced' ); } ).to.not.throw(); } ); @@ -263,7 +263,7 @@ describe( 'ContextualToolbar', () => { setData( editor.document, '[bar]' ); - contextualToolbar.fire( '_selectionChangeDone' ); + contextualToolbar.fire( '_selectionChangeDebounced' ); sinon.assert.notCalled( spy ); @@ -277,7 +277,7 @@ describe( 'ContextualToolbar', () => { setData( editor.document, '[bar]' ); - contextualToolbar.fire( '_selectionChangeDone' ); + contextualToolbar.fire( '_selectionChangeDebounced' ); // Hide toolbar. editor.document.selection.fire( 'change:range', { directChange: true } ); @@ -294,10 +294,10 @@ describe( 'ContextualToolbar', () => { } ); describe( 'destroy()', () => { - it( 'should not fire `_selectionChangeDone` after plugin destroy', ( done ) => { + it( 'should not fire `_selectionChangeDebounced` after plugin destroy', ( done ) => { const spy = sandbox.spy(); - contextualToolbar.on( '_selectionChangeDone', spy ); + contextualToolbar.on( '_selectionChangeDebounced', spy ); editor.document.selection.fire( 'change:range', { directChange: true } ); From 4cb81d43af1be9406b51cc571d3aef50a82d006f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Tue, 11 Apr 2017 15:56:05 +0200 Subject: [PATCH 20/41] Used position names instead of magic numbers in test. --- tests/toolbar/contextualtoolbar.js | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/tests/toolbar/contextualtoolbar.js b/tests/toolbar/contextualtoolbar.js index 8d5f0495..33622623 100644 --- a/tests/toolbar/contextualtoolbar.js +++ b/tests/toolbar/contextualtoolbar.js @@ -151,7 +151,7 @@ describe( 'ContextualToolbar', () => { expect( balloon.visibleView ).to.null; } ); - it( 'should open below if the selection is forward', () => { + it( 'should put balloon on the `south` if the selection is forward', () => { setData( editor.document, '[bar]' ); // Mock limiter rect. @@ -165,10 +165,10 @@ describe( 'ContextualToolbar', () => { contextualToolbar.fire( '_selectionChangeDebounced' ); expect( balloon.visibleView ).to.equal( contextualToolbar.toolbarView ); - expect( balloon.view.top ).to.be.above( 310 ); + expect( balloon.view.position ).to.equal( 'arrow_s' ); } ); - it( 'should open above if the selection is forward but panel stick out of the limiter element', () => { + it( 'should put balloon on the `north` if the selection is forward `south` is limited', () => { setData( editor.document, '[bar]' ); // Mock limiter rect. @@ -182,10 +182,10 @@ describe( 'ContextualToolbar', () => { contextualToolbar.fire( '_selectionChangeDebounced' ); expect( balloon.visibleView ).to.equal( contextualToolbar.toolbarView ); - expect( balloon.view.top ).to.be.below( 310 ); + expect( balloon.view.position ).to.equal( 'arrow_n' ); } ); - it( 'should open above if the selection is backward', () => { + it( 'should put balloon on the `north` if the selection is backward', () => { setData( editor.document, '[bar]', { lastRangeBackward: true } ); // Mock limiter rect. @@ -199,10 +199,10 @@ describe( 'ContextualToolbar', () => { contextualToolbar.fire( '_selectionChangeDebounced' ); expect( balloon.visibleView ).to.equal( contextualToolbar.toolbarView ); - expect( balloon.view.top ).to.be.below( 100 ); + expect( balloon.view.position ).to.equal( 'arrow_n' ); } ); - it( 'should open below if the selection is backward but panel stick out of the limiter element', () => { + it( 'should put balloon on the `south` if the selection is backward `north` is limited', () => { setData( editor.document, '[bar]', { lastRangeBackward: true } ); // Mock limiter rect. @@ -216,7 +216,7 @@ describe( 'ContextualToolbar', () => { contextualToolbar.fire( '_selectionChangeDebounced' ); expect( balloon.visibleView ).to.equal( contextualToolbar.toolbarView ); - expect( balloon.view.top ).to.be.above( 100 ); + expect( balloon.view.position ).to.be.equal( 'arrow_s' ); } ); it( 'should not open if the collapsed selection is moving', () => { From 515d0c6aa9ec56821b16a600c1ae583b8cefdb3a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Tue, 11 Apr 2017 15:58:07 +0200 Subject: [PATCH 21/41] Fixed typo in docs. --- src/toolbar/contextualtoolbar.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/toolbar/contextualtoolbar.js b/src/toolbar/contextualtoolbar.js index 68f5f001..a1412cb3 100644 --- a/src/toolbar/contextualtoolbar.js +++ b/src/toolbar/contextualtoolbar.js @@ -102,10 +102,10 @@ export default class ContextualToolbar extends Plugin { } /** - * Handles {@link modules:engine/model/document#selection} change and show or hide toolbar. + * Handles {@link module:engine/model/document#selection} change and show or hide toolbar. * - * Note that in this case it's better to listen to {@link modules:engine/model/document modelDocument} - * selection instead of {@link modules:engine/view/document viewDocument} selection because the first one + * Note that in this case it's better to listen to {@link module:engine/model/document modelDocument} + * selection instead of {@link module:engine/view/document viewDocument} selection because the first one * doesn't fire `change` event after text style change (like bold or italic) and toolbar doesn't blink. * * @private From 8de5a284511457b280749339855123016d45f47d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Tue, 11 Apr 2017 16:06:19 +0200 Subject: [PATCH 22/41] Moved ContextualToolbar to the `contextual` subdiredtory. --- src/toolbar/{ => contextual}/contextualtoolbar.js | 8 ++++---- tests/manual/contextualtoolbar/contextualtoolbar.js | 2 +- tests/toolbar/{ => contextual}/contextualtoolbar.js | 6 +++--- 3 files changed, 8 insertions(+), 8 deletions(-) rename src/toolbar/{ => contextual}/contextualtoolbar.js (96%) rename tests/toolbar/{ => contextual}/contextualtoolbar.js (98%) diff --git a/src/toolbar/contextualtoolbar.js b/src/toolbar/contextual/contextualtoolbar.js similarity index 96% rename from src/toolbar/contextualtoolbar.js rename to src/toolbar/contextual/contextualtoolbar.js index a1412cb3..44b6adf1 100644 --- a/src/toolbar/contextualtoolbar.js +++ b/src/toolbar/contextual/contextualtoolbar.js @@ -4,13 +4,13 @@ */ /** - * @module ui/toolbar/contextualtoolbar + * @module ui/toolbar/contextual/contextualtoolbar */ import Plugin from '@ckeditor/ckeditor5-core/src/plugin'; -import ContextualBalloon from '../contextualballoon'; -import ToolbarView from './toolbarview'; -import BalloonPanelView from '../panel/balloon/balloonpanelview.js'; +import ContextualBalloon from '../../contextualballoon'; +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; diff --git a/tests/manual/contextualtoolbar/contextualtoolbar.js b/tests/manual/contextualtoolbar/contextualtoolbar.js index a5f7ef39..627fae20 100644 --- a/tests/manual/contextualtoolbar/contextualtoolbar.js +++ b/tests/manual/contextualtoolbar/contextualtoolbar.js @@ -7,7 +7,7 @@ import ClassicEditor from '@ckeditor/ckeditor5-editor-classic/src/classic'; import ArticlePresets from '@ckeditor/ckeditor5-presets/src/article'; -import ContextualToolbar from '../../../src/toolbar/contextualtoolbar'; +import ContextualToolbar from '../../../src/toolbar/contextual/contextualtoolbar'; ClassicEditor.create( document.querySelector( '#editor' ), { plugins: [ ArticlePresets, ContextualToolbar ], diff --git a/tests/toolbar/contextualtoolbar.js b/tests/toolbar/contextual/contextualtoolbar.js similarity index 98% rename from tests/toolbar/contextualtoolbar.js rename to tests/toolbar/contextual/contextualtoolbar.js index 33622623..41a0ca5e 100644 --- a/tests/toolbar/contextualtoolbar.js +++ b/tests/toolbar/contextual/contextualtoolbar.js @@ -3,9 +3,9 @@ */ import ClassicTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/classictesteditor'; -import ContextualToolbar from '../../src/toolbar/contextualtoolbar'; -import ContextualBalloon from '../../src/contextualballoon'; -import ToolbarView from '../../src/toolbar/toolbarview'; +import ContextualToolbar from '../../../src/toolbar/contextual/contextualtoolbar'; +import ContextualBalloon from '../../../src/contextualballoon'; +import ToolbarView from '../../../src/toolbar/toolbarview'; import Plugin from '@ckeditor/ckeditor5-core/src/plugin'; import Bold from '@ckeditor/ckeditor5-basic-styles/src/bold'; import Italic from '@ckeditor/ckeditor5-basic-styles/src/italic'; From 851c2132a0254a0c31fdce5474d609ac21dc23d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Tue, 11 Apr 2017 17:07:08 +0200 Subject: [PATCH 23/41] Added className property to the BalloonPanelView interface. --- src/panel/balloon/balloonpanelview.js | 12 +++++++++++- tests/panel/balloon/balloonpanelview.js | 12 ++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/src/panel/balloon/balloonpanelview.js b/src/panel/balloon/balloonpanelview.js index 41aff86f..b3c65398 100644 --- a/src/panel/balloon/balloonpanelview.js +++ b/src/panel/balloon/balloonpanelview.js @@ -88,6 +88,15 @@ export default class BalloonPanelView extends View { */ this.set( 'withArrow', true ); + /** + * Additional css class added to the {#element}. + * + * @observable + * @default '' + * @member {String} #className + */ + this.set( 'className', '' ); + /** * Max width of the balloon panel, as in CSS. * @@ -118,7 +127,8 @@ 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_with-arrow' ) + bind.if( 'withArrow', 'ck-balloon-panel_with-arrow' ), + bind.to( 'className' ) ], style: { diff --git a/tests/panel/balloon/balloonpanelview.js b/tests/panel/balloon/balloonpanelview.js index 01e93c39..8ce8ee95 100644 --- a/tests/panel/balloon/balloonpanelview.js +++ b/tests/panel/balloon/balloonpanelview.js @@ -106,6 +106,18 @@ describe( 'BalloonPanelView', () => { } ); } ); + describe( 'className', () => { + it( 'should set additional class to the view#element', () => { + view.className = 'foo'; + + expect( view.element.classList.contains( 'foo' ) ).to.true; + + view.className = ''; + + expect( view.element.classList.contains( 'foo' ) ).to.false; + } ); + } ); + describe( 'children', () => { it( 'should react on view#content', () => { expect( view.element.childNodes.length ).to.equal( 0 ); From 03302cb5aca5cc178a0e52d779fd4ac246447017 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Tue, 11 Apr 2017 17:09:03 +0200 Subject: [PATCH 24/41] Removed useless test. --- tests/panel/balloon/balloonpanelview.js | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/tests/panel/balloon/balloonpanelview.js b/tests/panel/balloon/balloonpanelview.js index 8ce8ee95..257d5516 100644 --- a/tests/panel/balloon/balloonpanelview.js +++ b/tests/panel/balloon/balloonpanelview.js @@ -131,16 +131,6 @@ describe( 'BalloonPanelView', () => { } ); } ); - describe( 'isVisible', () => { - it( 'should return view#isVisible value', () => { - expect( view.isVisible ).to.false; - - view.isVisible = true; - - expect( view.isVisible ).to.true; - } ); - } ); - describe( 'show()', () => { it( 'should set view#isVisible as true', () => { view.isVisible = false; From b1f58183ccb03b3922c17c97ce24684a6a167318 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Tue, 11 Apr 2017 17:17:45 +0200 Subject: [PATCH 25/41] Added balloonClassName option to ContextualBalloon#add method. --- src/contextualballoon.js | 16 ++++++++++------ tests/contextualballoon.js | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 6 deletions(-) diff --git a/src/contextualballoon.js b/src/contextualballoon.js index 76f9a4bf..1f39cb38 100644 --- a/src/contextualballoon.js +++ b/src/contextualballoon.js @@ -87,8 +87,9 @@ export default class ContextualBalloon extends Plugin { * Adds a new view to the stack and makes it visible. * * @param {Object} data Configuration of the view. - * @param {module:ui/view~View} view Content of the balloon. - * @param {module:utils/dom/position~Options} position Positioning options. + * @param {module:ui/view~View} [data.view] Content of the balloon. + * @param {module:utils/dom/position~Options} [data.position] Positioning options. + * @param {String} [data.balloonClassName] Additional css class for {@link #view} added when given view is visible. */ add( data ) { if ( this.hasView( data.view ) ) { @@ -109,7 +110,7 @@ export default class ContextualBalloon extends Plugin { // Add new view to the stack. this._stack.set( data.view, data ); // And display it. - this._show( data.view ); + this._show( data ); } /** @@ -143,7 +144,7 @@ export default class ContextualBalloon extends Plugin { // If it is some other view. if ( last ) { // Just show it. - this._show( last.view ); + this._show( last ); } else { // Hide the balloon panel. this.view.hide(); @@ -173,10 +174,13 @@ export default class ContextualBalloon extends Plugin { * options of the first view. * * @private - * @param {module:ui/view~View} view View to show in the balloon. + * @param {Object} data Configuration. + * @param {module:ui/view~View} [data.view] View to show in the balloon. + * @param {String} [data.balloonClassName=''] View to show in the balloon. */ - _show( view ) { + _show( { view, balloonClassName = '' } ) { this.view.content.add( view ); + this.view.className = balloonClassName; // When view is not rendered we need to wait for it. See: https://github.com/ckeditor/ckeditor5-ui/issues/187. if ( !view.ready ) { diff --git a/tests/contextualballoon.js b/tests/contextualballoon.js index 89b5867c..50c3ae7d 100644 --- a/tests/contextualballoon.js +++ b/tests/contextualballoon.js @@ -185,6 +185,24 @@ describe( 'ContextualBalloon', () => { foo: 'bar' } ); } ); + + it( 'should set additional css class of visible view to BalloonPanelView', () => { + balloon.add( { + view: viewA, + position: { target: 'fake' }, + balloonClassName: 'foo' + } ); + + expect( balloon.view.className ).to.equal( 'foo' ); + + balloon.add( { + view: viewB, + position: { target: 'fake' }, + balloonClassName: 'bar' + } ); + + expect( balloon.view.className ).to.equal( 'bar' ); + } ); } ); describe( 'visibleView', () => { @@ -265,6 +283,24 @@ describe( 'ContextualBalloon', () => { balloon.remove( viewA ); } ).to.throw( CKEditorError, /^contextualballoon-remove-view-not-exist/ ); } ); + + it( 'should set additional css class of visible view to BalloonPanelView', () => { + balloon.add( { + view: viewA, + position: { target: 'fake' }, + balloonClassName: 'foo' + } ); + + balloon.add( { + view: viewB, + position: { target: 'fake' }, + balloonClassName: 'bar' + } ); + + balloon.remove( viewB ); + + expect( balloon.view.className ).to.equal( 'foo' ); + } ); } ); describe( 'updatePosition()', () => { From 760771be6dd563910782921204882338c313b908 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Tue, 11 Apr 2017 17:21:20 +0200 Subject: [PATCH 26/41] Added additional class to the ContextualBalloon#view element when Contextualtoolbar is opened. --- src/toolbar/contextual/contextualtoolbar.js | 3 ++- tests/toolbar/contextual/contextualtoolbar.js | 8 ++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/src/toolbar/contextual/contextualtoolbar.js b/src/toolbar/contextual/contextualtoolbar.js index 44b6adf1..606eef09 100644 --- a/src/toolbar/contextual/contextualtoolbar.js +++ b/src/toolbar/contextual/contextualtoolbar.js @@ -149,7 +149,8 @@ export default class ContextualToolbar extends Plugin { // Add panel to the common editor contextual balloon. this._balloon.add( { view: this.toolbarView, - position: this._getBalloonPositionData() + position: this._getBalloonPositionData(), + balloonClassName: 'ck-toolbar__container' } ); // Update panel position when selection changes while balloon will be opened (by a collaboration). diff --git a/tests/toolbar/contextual/contextualtoolbar.js b/tests/toolbar/contextual/contextualtoolbar.js index 41a0ca5e..205817bf 100644 --- a/tests/toolbar/contextual/contextualtoolbar.js +++ b/tests/toolbar/contextual/contextualtoolbar.js @@ -113,6 +113,14 @@ describe( 'ContextualToolbar', () => { expect( balloon.visibleView ).to.equal( contextualToolbar.toolbarView ); } ); + it( 'should add additional class to the ContextualBalloon#view', () => { + setData( editor.document, '[bar]' ); + + contextualToolbar.fire( '_selectionChangeDebounced' ); + + expect( balloon.view.className ).to.equal( 'ck-toolbar__container' ); + } ); + it( 'should close when selection starts changing by a directChange', () => { setData( editor.document, '[bar]' ); From 6f004a9d75ec1dc8204689957e25a3c032f463ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Wed, 12 Apr 2017 15:18:44 +0200 Subject: [PATCH 27/41] Get rid of nth helper. --- src/contextualballoon.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/contextualballoon.js b/src/contextualballoon.js index 1f39cb38..48376f6d 100644 --- a/src/contextualballoon.js +++ b/src/contextualballoon.js @@ -10,7 +10,6 @@ import Plugin from '@ckeditor/ckeditor5-core/src/plugin'; import BalloonPanelView from './panel/balloon/balloonpanelview'; import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror'; -import nth from '@ckeditor/ckeditor5-utils/src/nth'; /** * Provides the common contextual balloon panel for the editor. @@ -163,7 +162,7 @@ export default class ContextualBalloon extends Plugin { */ updatePosition( position ) { if ( position ) { - nth( 0, this._stack )[ 1 ].position = position; + this._stack.values().next().value.position = position; } this.view.attachTo( this._getBalloonPosition() ); @@ -198,7 +197,7 @@ export default class ContextualBalloon extends Plugin { * @returns {module:utils/dom/position~Options} */ _getBalloonPosition() { - return nth( 0, this._stack )[ 1 ].position; + return this._stack.values().next().value.position; } /** From f6b8d43ce1468e098cf0e729bf49bb30399dc5a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Fri, 14 Apr 2017 00:31:06 +0200 Subject: [PATCH 28/41] Improved docs. --- src/toolbar/contextual/contextualtoolbar.js | 7 +-- tests/toolbar/contextual/contextualtoolbar.js | 61 +++++++++++++------ 2 files changed, 47 insertions(+), 21 deletions(-) diff --git a/src/toolbar/contextual/contextualtoolbar.js b/src/toolbar/contextual/contextualtoolbar.js index 8bb0df87..55945163 100644 --- a/src/toolbar/contextual/contextualtoolbar.js +++ b/src/toolbar/contextual/contextualtoolbar.js @@ -210,11 +210,10 @@ export default class ContextualToolbar extends Plugin { } /** - * This is internal plugin event which is fired 200 ms after model selection last change (lodash#debounce). - * This is to makes easy test debounced action without need to use `setTimeout`. Lodash keeps time related - * stuff in a closure and it's not possible to override it by sinon fake timers. + * This is internal plugin event which is fired 200 ms after model selection last change. + * This is to makes easy test debounced action without need to use `setTimeout`. * - * @private + * @protected * @event _selectionChangeDebounced */ } diff --git a/tests/toolbar/contextual/contextualtoolbar.js b/tests/toolbar/contextual/contextualtoolbar.js index b4b272b1..0fa2143b 100644 --- a/tests/toolbar/contextual/contextualtoolbar.js +++ b/tests/toolbar/contextual/contextualtoolbar.js @@ -14,7 +14,7 @@ import global from '@ckeditor/ckeditor5-utils/src/dom/global'; import { setData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model.js'; -/* global document, window, setTimeout */ +/* global document, setTimeout */ describe( 'ContextualToolbar', () => { let sandbox, editor, contextualToolbar, balloon, editorElement; @@ -36,8 +36,6 @@ describe( 'ContextualToolbar', () => { contextualToolbar = editor.plugins.get( ContextualToolbar ); balloon = editor.plugins.get( ContextualBalloon ); - stubClientRects(); - // Focus the engine. editor.editing.view.isFocused = true; @@ -162,6 +160,8 @@ describe( 'ContextualToolbar', () => { it( 'should put balloon on the `south` if the selection is forward', () => { setData( editor.document, '[bar]' ); + stubClientRects(); + // Mock limiter rect. mockBoundingBox( document.body, { left: 0, @@ -173,12 +173,19 @@ describe( 'ContextualToolbar', () => { contextualToolbar.fire( '_selectionChangeDebounced' ); expect( balloon.visibleView ).to.equal( contextualToolbar.toolbarView ); - expect( balloon.view.position ).to.equal( 'arrow_s' ); + + // Balloon is attached after internal promise resolve and we have + // no access to this promise so we need to wait for it. + return wait().then( () => { + expect( balloon.view.position ).to.equal( 'arrow_s' ); + } ); } ); it( 'should put balloon on the `north` if the selection is forward `south` is limited', () => { setData( editor.document, '[bar]' ); + stubClientRects(); + // Mock limiter rect. mockBoundingBox( document.body, { left: 0, @@ -190,12 +197,19 @@ describe( 'ContextualToolbar', () => { contextualToolbar.fire( '_selectionChangeDebounced' ); expect( balloon.visibleView ).to.equal( contextualToolbar.toolbarView ); - expect( balloon.view.position ).to.equal( 'arrow_n' ); + + // Balloon is attached after internal promise resolve and we have + // no access to this promise so we need to wait for it. + return wait().then( () => { + expect( balloon.view.position ).to.equal( 'arrow_n' ); + } ); } ); it( 'should put balloon on the `north` if the selection is backward', () => { setData( editor.document, '[bar]', { lastRangeBackward: true } ); + stubClientRects(); + // Mock limiter rect. mockBoundingBox( document.body, { left: 0, @@ -207,12 +221,19 @@ describe( 'ContextualToolbar', () => { contextualToolbar.fire( '_selectionChangeDebounced' ); expect( balloon.visibleView ).to.equal( contextualToolbar.toolbarView ); - expect( balloon.view.position ).to.equal( 'arrow_n' ); + + // Balloon is attached after internal promise resolve and we have + // no access to this promise so we need to wait for it. + return wait().then( () => { + expect( balloon.view.position ).to.equal( 'arrow_n' ); + } ); } ); it( 'should put balloon on the `south` if the selection is backward `north` is limited', () => { setData( editor.document, '[bar]', { lastRangeBackward: true } ); + stubClientRects(); + // Mock limiter rect. mockBoundingBox( document.body, { left: 0, @@ -224,7 +245,12 @@ describe( 'ContextualToolbar', () => { contextualToolbar.fire( '_selectionChangeDebounced' ); expect( balloon.visibleView ).to.equal( contextualToolbar.toolbarView ); - expect( balloon.view.position ).to.be.equal( 'arrow_s' ); + + // Balloon is attached after internal promise resolve and we have + // no access to this promise so we need to wait for it. + return wait().then( () => { + expect( balloon.view.position ).to.equal( 'arrow_s' ); + } ); } ); it( 'should not open if the collapsed selection is moving', () => { @@ -267,7 +293,7 @@ describe( 'ContextualToolbar', () => { } ); it( 'should update balloon position when toolbar is opened and editor content has changed', () => { - const spy = sandbox.spy( balloon, 'updatePosition' ); + const spy = sandbox.stub( balloon, 'updatePosition' ); setData( editor.document, '[bar]' ); @@ -357,15 +383,10 @@ describe( 'ContextualToolbar', () => { } ); // Mock window rect. - sandbox.stub( global, 'window', { - innerWidth: 1000, - innerHeight: 1000, - scrollX: 0, - scrollY: 0, - getComputedStyle: el => { - return window.getComputedStyle( el ); - } - } ); + sandbox.stub( global.window, 'innerWidth', 1000 ); + sandbox.stub( global.window, 'innerHeight', 1000 ); + sandbox.stub( global.window, 'scrollX', 0 ); + sandbox.stub( global.window, 'scrollY', 0 ); // Mock balloon rect. mockBoundingBox( balloon.view.element, { @@ -383,3 +404,9 @@ describe( 'ContextualToolbar', () => { sandbox.stub( element, 'getBoundingClientRect' ).returns( boundingBox ); } } ); + +function wait( delay = 1 ) { + return new Promise( ( resolve ) => { + setTimeout( () => resolve(), delay ); + } ); +} From 18168e27885d883172d82c3c7317af61a68aeb1e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Fri, 14 Apr 2017 09:50:41 +0200 Subject: [PATCH 29/41] Improved tests. --- tests/toolbar/contextual/contextualtoolbar.js | 162 ++---------------- 1 file changed, 13 insertions(+), 149 deletions(-) diff --git a/tests/toolbar/contextual/contextualtoolbar.js b/tests/toolbar/contextual/contextualtoolbar.js index 0fa2143b..cd9b6775 100644 --- a/tests/toolbar/contextual/contextualtoolbar.js +++ b/tests/toolbar/contextual/contextualtoolbar.js @@ -5,12 +5,12 @@ import ClassicTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/classictesteditor'; import ContextualToolbar from '../../../src/toolbar/contextual/contextualtoolbar'; import ContextualBalloon from '../../../src/panel/balloon/contextualballoon'; +import BalloonPanelView from '../../../src/panel/balloon/balloonpanelview'; import ToolbarView from '../../../src/toolbar/toolbarview'; import Plugin from '@ckeditor/ckeditor5-core/src/plugin'; import Bold from '@ckeditor/ckeditor5-basic-styles/src/bold'; import Italic from '@ckeditor/ckeditor5-basic-styles/src/italic'; import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph'; -import global from '@ckeditor/ckeditor5-utils/src/dom/global'; import { setData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model.js'; @@ -157,100 +157,30 @@ describe( 'ContextualToolbar', () => { expect( balloon.visibleView ).to.null; } ); - it( 'should put balloon on the `south` if the selection is forward', () => { - setData( editor.document, '[bar]' ); - - stubClientRects(); - - // Mock limiter rect. - mockBoundingBox( document.body, { - left: 0, - width: 1000, - top: 0, - height: 1000 - } ); - - contextualToolbar.fire( '_selectionChangeDebounced' ); - - expect( balloon.visibleView ).to.equal( contextualToolbar.toolbarView ); - - // Balloon is attached after internal promise resolve and we have - // no access to this promise so we need to wait for it. - return wait().then( () => { - expect( balloon.view.position ).to.equal( 'arrow_s' ); - } ); - } ); + it( 'should open with specified positions configuration for the forward selection', () => { + const spy = sandbox.stub( balloon, 'add', () => {} ); + const defaultPositions = BalloonPanelView.defaultPositions; - it( 'should put balloon on the `north` if the selection is forward `south` is limited', () => { setData( editor.document, '[bar]' ); - stubClientRects(); - - // Mock limiter rect. - mockBoundingBox( document.body, { - left: 0, - width: 1000, - top: 0, - height: 310 - } ); - contextualToolbar.fire( '_selectionChangeDebounced' ); - expect( balloon.visibleView ).to.equal( contextualToolbar.toolbarView ); - - // Balloon is attached after internal promise resolve and we have - // no access to this promise so we need to wait for it. - return wait().then( () => { - expect( balloon.view.position ).to.equal( 'arrow_n' ); - } ); + expect( spy.firstCall.args[ 0 ].position.positions ).to.deep.equal( + [ defaultPositions.forwardSelection, defaultPositions.forwardSelectionAlternative ] + ); } ); - it( 'should put balloon on the `north` if the selection is backward', () => { - setData( editor.document, '[bar]', { lastRangeBackward: true } ); - - stubClientRects(); - - // Mock limiter rect. - mockBoundingBox( document.body, { - left: 0, - width: 1000, - top: 0, - height: 1000 - } ); - - contextualToolbar.fire( '_selectionChangeDebounced' ); - - expect( balloon.visibleView ).to.equal( contextualToolbar.toolbarView ); - - // Balloon is attached after internal promise resolve and we have - // no access to this promise so we need to wait for it. - return wait().then( () => { - expect( balloon.view.position ).to.equal( 'arrow_n' ); - } ); - } ); + it( 'should open with specified positions configuration for the backward selection', () => { + const spy = sandbox.stub( balloon, 'add', () => {} ); + const defaultPositions = BalloonPanelView.defaultPositions; - it( 'should put balloon on the `south` if the selection is backward `north` is limited', () => { setData( editor.document, '[bar]', { lastRangeBackward: true } ); - stubClientRects(); - - // Mock limiter rect. - mockBoundingBox( document.body, { - left: 0, - width: 1000, - top: 95, - height: 905 - } ); - contextualToolbar.fire( '_selectionChangeDebounced' ); - expect( balloon.visibleView ).to.equal( contextualToolbar.toolbarView ); - - // Balloon is attached after internal promise resolve and we have - // no access to this promise so we need to wait for it. - return wait().then( () => { - expect( balloon.view.position ).to.equal( 'arrow_s' ); - } ); + expect( spy.firstCall.args[ 0 ].position.positions ).to.deep.equal( + [ defaultPositions.backwardSelection, defaultPositions.backwardSelectionAlternative ] + ); } ); it( 'should not open if the collapsed selection is moving', () => { @@ -343,70 +273,4 @@ describe( 'ContextualToolbar', () => { }, 200 ); } ); } ); - - function stubClientRects() { - const editingView = editor.editing.view; - const originalViewRangeToDom = editingView.domConverter.viewRangeToDom; - - // Mock selection rect. - sandbox.stub( editingView.domConverter, 'viewRangeToDom', ( ...args ) => { - const domRange = originalViewRangeToDom.apply( editingView.domConverter, args ); - - sandbox.stub( domRange, 'getClientRects', () => { - return { - length: 2, - item: id => { - if ( id === 0 ) { - return { - top: 100, - height: 10, - bottom: 110, - left: 200, - width: 50, - right: 250 - }; - } - - return { - top: 300, - height: 10, - bottom: 310, - left: 400, - width: 50, - right: 450 - }; - } - }; - } ); - - return domRange; - } ); - - // Mock window rect. - sandbox.stub( global.window, 'innerWidth', 1000 ); - sandbox.stub( global.window, 'innerHeight', 1000 ); - sandbox.stub( global.window, 'scrollX', 0 ); - sandbox.stub( global.window, 'scrollY', 0 ); - - // Mock balloon rect. - mockBoundingBox( balloon.view.element, { - width: 150, - height: 50 - } ); - } - - function mockBoundingBox( element, data ) { - const boundingBox = Object.assign( {}, data ); - - boundingBox.right = boundingBox.left + boundingBox.width; - boundingBox.bottom = boundingBox.top + boundingBox.height; - - sandbox.stub( element, 'getBoundingClientRect' ).returns( boundingBox ); - } } ); - -function wait( delay = 1 ) { - return new Promise( ( resolve ) => { - setTimeout( () => resolve(), delay ); - } ); -} From a82b22426fa51ae98b87b5f20d1238c162ce3efb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Fri, 14 Apr 2017 11:42:45 +0200 Subject: [PATCH 30/41] Renamed default positions of BalloonPanelView. --- src/panel/balloon/balloonpanelview.js | 40 ++++++++++++------------- tests/panel/balloon/balloonpanelview.js | 40 ++++++++++++------------- 2 files changed, 40 insertions(+), 40 deletions(-) diff --git a/src/panel/balloon/balloonpanelview.js b/src/panel/balloon/balloonpanelview.js index b3c65398..5a503604 100644 --- a/src/panel/balloon/balloonpanelview.js +++ b/src/panel/balloon/balloonpanelview.js @@ -181,10 +181,10 @@ export default class BalloonPanelView extends View { const positionOptions = Object.assign( {}, { element: this.element, positions: [ - defaultPositions.se, - defaultPositions.sw, - defaultPositions.ne, - defaultPositions.nw + defaultPositions.southEastArrowNorthEast, + defaultPositions.southWestArrowNorthEast, + defaultPositions.northEastArrowSouthWest, + defaultPositions.northWestArrowSouthEast ], limiter: defaultLimiterElement, fitInViewport: true @@ -334,7 +334,7 @@ BalloonPanelView.arrowVerticalOffset = 15; * * The available positioning functions are as follows: * - * * South east: + * * South east arrow north west: * * [ Target ] * ^ @@ -343,7 +343,7 @@ BalloonPanelView.arrowVerticalOffset = 15; * +-----------------+ * * - * * South west: + * * South west arrow north east: * * [ Target ] * ^ @@ -352,7 +352,7 @@ BalloonPanelView.arrowVerticalOffset = 15; * +-----------------+ * * - * * North east: + * * North east arrow south west: * * +-----------------+ * | Balloon | @@ -361,7 +361,7 @@ BalloonPanelView.arrowVerticalOffset = 15; * [ Target ] * * - * * North west: + * * North west arrow south east: * * +-----------------+ * | Balloon | @@ -370,7 +370,7 @@ BalloonPanelView.arrowVerticalOffset = 15; * [ Target ] * * - * * Forward selection: + * * South east arrow north: * * [text range] * ^ @@ -379,7 +379,7 @@ BalloonPanelView.arrowVerticalOffset = 15; * +-----------------+ * * - * * Forward selection alternative: + * * North east arrow south: * * +-----------------+ * | Balloon | @@ -388,7 +388,7 @@ BalloonPanelView.arrowVerticalOffset = 15; * [text range] * * - * * Backward selection: + * * North west arrow south: * * +-----------------+ * | Balloon | @@ -397,7 +397,7 @@ BalloonPanelView.arrowVerticalOffset = 15; * [text range] * * - * * Backward selection alternative: + * * South west arrow north: * * [text range] * ^ @@ -415,49 +415,49 @@ BalloonPanelView.arrowVerticalOffset = 15; * @member {Object} module:ui/panel/balloon/balloonpanelview~BalloonPanelView.defaultPositions */ BalloonPanelView.defaultPositions = { - se: ( targetRect ) => ( { + southEastArrowNorthEast: ( targetRect ) => ( { top: targetRect.bottom + BalloonPanelView.arrowVerticalOffset, left: targetRect.left + targetRect.width / 2 - BalloonPanelView.arrowHorizontalOffset, name: 'arrow_se' } ), - sw: ( targetRect, balloonRect ) => ( { + southWestArrowNorthEast: ( targetRect, balloonRect ) => ( { top: targetRect.bottom + BalloonPanelView.arrowVerticalOffset, left: targetRect.left + targetRect.width / 2 - balloonRect.width + BalloonPanelView.arrowHorizontalOffset, name: 'arrow_sw' } ), - ne: ( targetRect, balloonRect ) => ( { + northEastArrowSouthWest: ( targetRect, balloonRect ) => ( { top: targetRect.top - balloonRect.height - BalloonPanelView.arrowVerticalOffset, left: targetRect.left + targetRect.width / 2 - BalloonPanelView.arrowHorizontalOffset, name: 'arrow_ne' } ), - nw: ( targetRect, balloonRect ) => ( { + northWestArrowSouthEast: ( targetRect, balloonRect ) => ( { top: targetRect.top - balloonRect.height - BalloonPanelView.arrowVerticalOffset, left: targetRect.left + targetRect.width / 2 - balloonRect.width + BalloonPanelView.arrowHorizontalOffset, name: 'arrow_nw' } ), - forwardSelection: ( targetRect, balloonRect ) => ( { + southEastArrowNorth: ( targetRect, balloonRect ) => ( { top: targetRect.bottom + BalloonPanelView.arrowVerticalOffset, left: targetRect.right - balloonRect.width / 2, name: 'arrow_s' } ), - forwardSelectionAlternative: ( targetRect, balloonRect ) => ( { + northEastArrowSouth: ( targetRect, balloonRect ) => ( { top: targetRect.top - balloonRect.height - BalloonPanelView.arrowVerticalOffset, left: targetRect.right - balloonRect.width / 2, name: 'arrow_n' } ), - backwardSelection: ( targetRect, balloonRect ) => ( { + northWestArrowSouth: ( targetRect, balloonRect ) => ( { top: targetRect.top - balloonRect.height - BalloonPanelView.arrowVerticalOffset, left: targetRect.left - balloonRect.width / 2, name: 'arrow_n' } ), - backwardSelectionAlternative: ( targetRect, balloonRect ) => ( { + southWestArrowNorth: ( targetRect, balloonRect ) => ( { top: targetRect.bottom + BalloonPanelView.arrowVerticalOffset, left: targetRect.left - balloonRect.width / 2, name: 'arrow_s' diff --git a/tests/panel/balloon/balloonpanelview.js b/tests/panel/balloon/balloonpanelview.js index 257d5516..2166717f 100644 --- a/tests/panel/balloon/balloonpanelview.js +++ b/tests/panel/balloon/balloonpanelview.js @@ -189,10 +189,10 @@ describe( 'BalloonPanelView', () => { element: view.element, target: target, positions: [ - BalloonPanelView.defaultPositions.se, - BalloonPanelView.defaultPositions.sw, - BalloonPanelView.defaultPositions.ne, - BalloonPanelView.defaultPositions.nw + BalloonPanelView.defaultPositions.southEastArrowNorthEast, + BalloonPanelView.defaultPositions.southWestArrowNorthEast, + BalloonPanelView.defaultPositions.northEastArrowSouthWest, + BalloonPanelView.defaultPositions.northWestArrowSouthEast ], limiter: document.body, fitInViewport: true @@ -658,64 +658,64 @@ describe( 'BalloonPanelView', () => { }; } ); - it( 'se', () => { - expect( positions.se( targetRect ) ).to.deep.equal( { + it( 'southEastArrowNorthEast', () => { + expect( positions.southEastArrowNorthEast( targetRect ) ).to.deep.equal( { top: 215, left: 120, name: 'arrow_se' } ); } ); - it( 'sw', () => { - expect( positions.sw( targetRect, balloonRect ) ).to.deep.equal( { + it( 'southWestArrowNorthEast', () => { + expect( positions.southWestArrowNorthEast( targetRect, balloonRect ) ).to.deep.equal( { top: 215, left: 130, name: 'arrow_sw' } ); } ); - it( 'ne', () => { - expect( positions.ne( targetRect, balloonRect ) ).to.deep.equal( { + it( 'northEastArrowSouthWest', () => { + expect( positions.northEastArrowSouthWest( targetRect, balloonRect ) ).to.deep.equal( { top: 35, left: 120, name: 'arrow_ne' } ); } ); - it( 'nw', () => { - expect( positions.nw( targetRect, balloonRect ) ).to.deep.equal( { + it( 'northWestArrowSouthEast', () => { + expect( positions.northWestArrowSouthEast( targetRect, balloonRect ) ).to.deep.equal( { top: 35, left: 130, name: 'arrow_nw' } ); } ); - it( 'forwardSelection', () => { - expect( positions.forwardSelection( targetRect, balloonRect ) ).to.deep.equal( { + it( 'southEastArrowNorth', () => { + expect( positions.southEastArrowNorth( targetRect, balloonRect ) ).to.deep.equal( { top: 215, left: 175, name: 'arrow_s' } ); } ); - it( 'forwardSelectionAlternative', () => { - expect( positions.forwardSelectionAlternative( targetRect, balloonRect ) ).to.deep.equal( { + it( 'northEastArrowSouth', () => { + expect( positions.northEastArrowSouth( targetRect, balloonRect ) ).to.deep.equal( { top: 35, left: 175, name: 'arrow_n' } ); } ); - it( 'backwardSelection', () => { - expect( positions.backwardSelection( targetRect, balloonRect ) ).to.deep.equal( { + it( 'northWestArrowSouth', () => { + expect( positions.northWestArrowSouth( targetRect, balloonRect ) ).to.deep.equal( { top: 35, left: 75, name: 'arrow_n' } ); } ); - it( 'backwardSelectionAlternative', () => { - expect( positions.backwardSelectionAlternative( targetRect, balloonRect ) ).to.deep.equal( { + it( 'southWestArrowNorth', () => { + expect( positions.southWestArrowNorth( targetRect, balloonRect ) ).to.deep.equal( { top: 215, left: 75, name: 'arrow_s' From ccf31b525fb365f1f7438b3639b757e2f6ce0d9d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Fri, 14 Apr 2017 11:46:06 +0200 Subject: [PATCH 31/41] Changed ContextualToolbar position names. --- 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 55945163..8abfc572 100644 --- a/src/toolbar/contextual/contextualtoolbar.js +++ b/src/toolbar/contextual/contextualtoolbar.js @@ -195,8 +195,8 @@ export default class ContextualToolbar extends Plugin { return { target: rangeRect, positions: isBackward ? - [ defaultPositions.backwardSelection, defaultPositions.backwardSelectionAlternative ] : - [ defaultPositions.forwardSelection, defaultPositions.forwardSelectionAlternative ], + [ defaultPositions.northWestArrowSouth, defaultPositions.southWestArrowNorth ] : + [ defaultPositions.southEastArrowNorth, defaultPositions.northEastArrowSouth ] }; } diff --git a/tests/toolbar/contextual/contextualtoolbar.js b/tests/toolbar/contextual/contextualtoolbar.js index cd9b6775..1eb0dd24 100644 --- a/tests/toolbar/contextual/contextualtoolbar.js +++ b/tests/toolbar/contextual/contextualtoolbar.js @@ -166,7 +166,7 @@ describe( 'ContextualToolbar', () => { contextualToolbar.fire( '_selectionChangeDebounced' ); expect( spy.firstCall.args[ 0 ].position.positions ).to.deep.equal( - [ defaultPositions.forwardSelection, defaultPositions.forwardSelectionAlternative ] + [ defaultPositions.southEastArrowNorth, defaultPositions.northEastArrowSouth ] ); } ); @@ -179,7 +179,7 @@ describe( 'ContextualToolbar', () => { contextualToolbar.fire( '_selectionChangeDebounced' ); expect( spy.firstCall.args[ 0 ].position.positions ).to.deep.equal( - [ defaultPositions.backwardSelection, defaultPositions.backwardSelectionAlternative ] + [ defaultPositions.northWestArrowSouth, defaultPositions.southWestArrowNorth ] ); } ); From d3e0c54df85ecf44c5ae9c462620f54546fd5a5e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Fri, 14 Apr 2017 12:01:17 +0200 Subject: [PATCH 32/41] Removed unnecessary default value from BalloonPanelView#className. --- src/panel/balloon/balloonpanelview.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/panel/balloon/balloonpanelview.js b/src/panel/balloon/balloonpanelview.js index 5a503604..0ea9a1e1 100644 --- a/src/panel/balloon/balloonpanelview.js +++ b/src/panel/balloon/balloonpanelview.js @@ -95,7 +95,7 @@ export default class BalloonPanelView extends View { * @default '' * @member {String} #className */ - this.set( 'className', '' ); + this.set( 'className' ); /** * Max width of the balloon panel, as in CSS. From a3e53964b94188a9a45a75b7cafbedd253433270 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Fri, 14 Apr 2017 14:52:45 +0200 Subject: [PATCH 33/41] Updated docs. --- src/panel/balloon/balloonpanelview.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/panel/balloon/balloonpanelview.js b/src/panel/balloon/balloonpanelview.js index 0ea9a1e1..6f25f3b2 100644 --- a/src/panel/balloon/balloonpanelview.js +++ b/src/panel/balloon/balloonpanelview.js @@ -92,7 +92,6 @@ export default class BalloonPanelView extends View { * Additional css class added to the {#element}. * * @observable - * @default '' * @member {String} #className */ this.set( 'className' ); From 6c2fe1b0a625f9ad586b237d04be19685d314bf1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Tue, 18 Apr 2017 18:06:06 +0200 Subject: [PATCH 34/41] Made _showPanel and _hidePanel protected and refactored tests to not show errors on the browser console. --- src/toolbar/contextual/contextualtoolbar.js | 19 +- tests/toolbar/contextual/contextualtoolbar.js | 341 ++++++++++++------ 2 files changed, 243 insertions(+), 117 deletions(-) diff --git a/src/toolbar/contextual/contextualtoolbar.js b/src/toolbar/contextual/contextualtoolbar.js index 8abfc572..0f02f536 100644 --- a/src/toolbar/contextual/contextualtoolbar.js +++ b/src/toolbar/contextual/contextualtoolbar.js @@ -131,32 +131,33 @@ export default class ContextualToolbar extends Plugin { /** * Adds panel view to the {@link: #_balloon} and attaches panel to the selection. * - * @private + * @protected + * @return {Promise} A promise resolved when the {@link #toolbarView} {@link module:ui/view~View#init} is done. */ _showPanel() { const editingView = this.editor.editing.view; // Do not add toolbar to the balloon stack twice. if ( this._balloon.hasView( this.toolbarView ) ) { - return; + return Promise.resolve(); } // This implementation assumes that only non–collapsed selections gets the contextual toolbar. if ( !editingView.isFocused || editingView.selection.isCollapsed ) { - return; + return Promise.resolve(); } + // Update panel position when selection changes while balloon will be opened (by a collaboration). + this.listenTo( this.editor.editing.view, 'render', () => { + this._balloon.updatePosition( this._getBalloonPositionData() ); + } ); + // Add panel to the common editor contextual balloon. - this._balloon.add( { + return this._balloon.add( { view: this.toolbarView, position: this._getBalloonPositionData(), balloonClassName: 'ck-toolbar__container' } ); - - // Update panel position when selection changes while balloon will be opened (by a collaboration). - this.listenTo( this.editor.editing.view, 'render', () => { - this._balloon.updatePosition( this._getBalloonPositionData() ); - } ); } /** diff --git a/tests/toolbar/contextual/contextualtoolbar.js b/tests/toolbar/contextual/contextualtoolbar.js index 1eb0dd24..5fc2d384 100644 --- a/tests/toolbar/contextual/contextualtoolbar.js +++ b/tests/toolbar/contextual/contextualtoolbar.js @@ -36,11 +36,12 @@ describe( 'ContextualToolbar', () => { contextualToolbar = editor.plugins.get( ContextualToolbar ); balloon = editor.plugins.get( ContextualBalloon ); + // There is no point to execute BalloonPanelView attachTo and pin methods so lets override it. + sandbox.stub( balloon.view, 'attachTo', () => {} ); + sandbox.stub( balloon.view, 'pin', () => {} ); + // Focus the engine. editor.editing.view.isFocused = true; - - // Init child view. - return contextualToolbar.toolbarView.init(); } ); } ); @@ -101,176 +102,300 @@ describe( 'ContextualToolbar', () => { }, 100 ); } ); - it( 'should open when selection stops changing', () => { - setData( editor.document, '[bar]' ); + describe( 'pluginName', () => { + it( 'should return plugin by its name', () => { + expect( editor.plugins.get( 'contextualtoolbar' ) ).to.equal( contextualToolbar ); + } ); + } ); - expect( balloon.visibleView ).to.null; + describe( '_showPanel()', () => { + let balloonAddSpy, forwardSelectionRect, backwardSelectionRect; + + beforeEach( () => { + forwardSelectionRect = { + top: 100, + height: 10, + bottom: 110, + left: 200, + width: 50, + right: 250 + }; + + backwardSelectionRect = { + top: 100, + height: 10, + bottom: 110, + left: 200, + width: 50, + right: 250 + }; + + stubSelectionRect( forwardSelectionRect, backwardSelectionRect ); + + balloonAddSpy = sandbox.spy( balloon, 'add' ); + editor.editing.view.isFocused = true; + } ); - contextualToolbar.fire( '_selectionChangeDebounced' ); + it( 'should return promise', () => { + setData( editor.document, 'b[a]r' ); - expect( balloon.visibleView ).to.equal( contextualToolbar.toolbarView ); - } ); + const returned = contextualToolbar._showPanel(); - it( 'should add additional class to the ContextualBalloon#view', () => { - setData( editor.document, '[bar]' ); + expect( returned ).to.instanceof( Promise ); - contextualToolbar.fire( '_selectionChangeDebounced' ); + return returned; + } ); - expect( balloon.view.className ).to.equal( 'ck-toolbar__container' ); - } ); + it( 'should add #toolbarView to the #_balloon and attach the #_balloon to the selection for the forward selection', () => { + setData( editor.document, 'b[a]r' ); + + const defaultPositions = BalloonPanelView.defaultPositions; + + return contextualToolbar._showPanel().then( () => { + sinon.assert.calledWithExactly( balloonAddSpy, { + view: contextualToolbar.toolbarView, + balloonClassName: 'ck-toolbar__container', + position: { + target: forwardSelectionRect, + positions: [ defaultPositions.southEastArrowNorth, defaultPositions.northEastArrowSouth ] + } + } ); + } ); + } ); - it( 'should close when selection starts changing by a directChange', () => { - setData( editor.document, '[bar]' ); + it( 'should add #toolbarView to the #_balloon and attach the #_balloon to the selection for the backward selection', () => { + setData( editor.document, 'b[a]r', { lastRangeBackward: true } ); + + const defaultPositions = BalloonPanelView.defaultPositions; + + return contextualToolbar._showPanel() + .then( () => { + sinon.assert.calledWithExactly( balloonAddSpy, { + view: contextualToolbar.toolbarView, + balloonClassName: 'ck-toolbar__container', + position: { + target: backwardSelectionRect, + positions: [ defaultPositions.northWestArrowSouth, defaultPositions.southWestArrowNorth ] + } + } ); + } ); + } ); - contextualToolbar.fire( '_selectionChangeDebounced' ); + it( 'should update balloon position on ViewDocument#render event while balloon is added to the #_balloon', () => { + setData( editor.document, 'b[a]r' ); - expect( balloon.visibleView ).to.equal( contextualToolbar.toolbarView ); + const spy = sinon.spy( balloon, 'updatePosition' ); - editor.document.selection.fire( 'change:range', { directChange: true } ); + editor.editing.view.fire( 'render' ); - expect( balloon.visibleView ).to.null; - } ); + return contextualToolbar._showPanel() + .then( () => { + sinon.assert.notCalled( spy ); - it( 'should not close when selection starts changing by not a directChange', () => { - setData( editor.document, '[bar]' ); + editor.editing.view.fire( 'render' ); - contextualToolbar.fire( '_selectionChangeDebounced' ); + sinon.assert.calledOnce( spy ); + } ); + } ); - expect( balloon.visibleView ).to.equal( contextualToolbar.toolbarView ); + it( 'should not add #toolbarView to the #_balloon more than once', () => { + setData( editor.document, 'b[a]r' ); - editor.document.selection.fire( 'change:range', { directChange: false } ); + return contextualToolbar._showPanel() + .then( () => contextualToolbar._showPanel() ) + .then( () => { + sinon.assert.calledOnce( balloonAddSpy ); + } ); + } ); - expect( balloon.visibleView ).to.equal( contextualToolbar.toolbarView ); - } ); + it( 'should not add #toolbarView to the #_balloon when editor is not focused', () => { + setData( editor.document, 'b[a]r' ); + editor.editing.view.isFocused = false; - it( 'should close when selection starts changing by not a directChange but will become collapsed', () => { - setData( editor.document, '[bar]' ); + return contextualToolbar._showPanel() + .then( () => { + sinon.assert.notCalled( balloonAddSpy ); + } ); + } ); - contextualToolbar.fire( '_selectionChangeDebounced' ); + it( 'should not add #toolbarView to the #_balloon when selection is collapsed', () => { + setData( editor.document, 'b[]ar' ); - // Collapse range silently (without firing `change:range` { directChange: true } event). - const range = editor.document.selection._ranges[ 0 ]; - range.end = range.start; + return contextualToolbar._showPanel() + .then( () => { + sinon.assert.notCalled( balloonAddSpy ); + } ); + } ); + } ); - editor.document.selection.fire( 'change:range', { directChange: false } ); + describe( '_hidePanel()', () => { + let removeBalloonSpy; - expect( balloon.visibleView ).to.null; - } ); + beforeEach( () => { + removeBalloonSpy = sandbox.spy( balloon, 'remove' ); + editor.editing.view.isFocused = true; + } ); - it( 'should open with specified positions configuration for the forward selection', () => { - const spy = sandbox.stub( balloon, 'add', () => {} ); - const defaultPositions = BalloonPanelView.defaultPositions; + it( 'should remove #toolbarView from the #_balloon', () => { + setData( editor.document, 'b[a]r' ); - setData( editor.document, '[bar]' ); + return contextualToolbar._showPanel() + .then( () => { + contextualToolbar._hidePanel(); - contextualToolbar.fire( '_selectionChangeDebounced' ); + sinon.assert.calledWithExactly( removeBalloonSpy, contextualToolbar.toolbarView ); + } ); + } ); - expect( spy.firstCall.args[ 0 ].position.positions ).to.deep.equal( - [ defaultPositions.southEastArrowNorth, defaultPositions.northEastArrowSouth ] - ); - } ); + it( 'should stop update balloon position on ViewDocument#render event', () => { + setData( editor.document, 'b[a]r' ); + + const spy = sinon.spy( balloon, 'updatePosition' ); - it( 'should open with specified positions configuration for the backward selection', () => { - const spy = sandbox.stub( balloon, 'add', () => {} ); - const defaultPositions = BalloonPanelView.defaultPositions; + return contextualToolbar._showPanel() + .then( () => { + contextualToolbar._hidePanel(); - setData( editor.document, '[bar]', { lastRangeBackward: true } ); + editor.editing.view.fire( 'render' ); + + sinon.assert.notCalled( spy ); + } ); + } ); - contextualToolbar.fire( '_selectionChangeDebounced' ); + it( 'should not remove #ttolbarView when is not added to the #_balloon', () => { + contextualToolbar._hidePanel(); - expect( spy.firstCall.args[ 0 ].position.positions ).to.deep.equal( - [ defaultPositions.northWestArrowSouth, defaultPositions.southWestArrowNorth ] - ); + sinon.assert.notCalled( removeBalloonSpy ); + } ); } ); - it( 'should not open if the collapsed selection is moving', () => { - setData( editor.document, 'ba[]r' ); + describe( 'destroy()', () => { + it( 'should not fire `_selectionChangeDebounced` after plugin destroy', ( done ) => { + const spy = sandbox.spy(); - editor.document.selection.fire( 'change:range', {} ); - contextualToolbar.fire( '_selectionChangeDebounced' ); + contextualToolbar.on( '_selectionChangeDebounced', spy ); - setData( editor.document, 'b[]ar' ); + editor.document.selection.fire( 'change:range', { directChange: true } ); - editor.document.selection.fire( 'change:range', {} ); - contextualToolbar.fire( '_selectionChangeDebounced' ); + contextualToolbar.destroy(); - expect( balloon.visibleView ).to.null; + setTimeout( () => { + sinon.assert.notCalled( spy ); + done(); + }, 200 ); + } ); } ); - it( 'should hide if the editor loses focus', () => { - setData( editor.document, '[bar]' ); - editor.ui.focusTracker.isFocused = true; + describe( 'showing and hiding', () => { + let showPanelSpy, hidePanelSpy; - contextualToolbar.fire( '_selectionChangeDebounced' ); + beforeEach( () => { + setData( editor.document, '[bar]' ); - expect( balloon.visibleView ).to.equal( contextualToolbar.toolbarView ); + // Methods are stubbed because return internal promise which can't be returned in test. + showPanelSpy = sandbox.stub( contextualToolbar, '_showPanel', () => {} ); + hidePanelSpy = sandbox.stub( contextualToolbar, '_hidePanel', () => {} ); + } ); - editor.ui.focusTracker.isFocused = false; + it( 'should open when selection stops changing', () => { + sinon.assert.notCalled( showPanelSpy ); + sinon.assert.notCalled( hidePanelSpy ); - expect( balloon.visibleView ).to.null; - } ); + contextualToolbar.fire( '_selectionChangeDebounced' ); - it( 'should do nothing when panel is being added to balloon stack twice', () => { - setData( editor.document, '[bar]' ); + sinon.assert.calledOnce( showPanelSpy ); + sinon.assert.notCalled( hidePanelSpy ); + } ); - contextualToolbar.fire( '_selectionChangeDebounced' ); + it( 'should close when selection starts changing by a directChange', () => { + contextualToolbar.fire( '_selectionChangeDebounced' ); + + sinon.assert.calledOnce( showPanelSpy ); + sinon.assert.notCalled( hidePanelSpy ); - expect( balloon.visibleView ).to.equal( contextualToolbar.toolbarView ); + editor.document.selection.fire( 'change:range', { directChange: true } ); - expect( () => { + sinon.assert.calledOnce( showPanelSpy ); + sinon.assert.calledOnce( hidePanelSpy ); + } ); + + it( 'should not close when selection starts changing by not a directChange', () => { contextualToolbar.fire( '_selectionChangeDebounced' ); - } ).to.not.throw(); - } ); - it( 'should update balloon position when toolbar is opened and editor content has changed', () => { - const spy = sandbox.stub( balloon, 'updatePosition' ); + sinon.assert.calledOnce( showPanelSpy ); + sinon.assert.notCalled( hidePanelSpy ); - setData( editor.document, '[bar]' ); + editor.document.selection.fire( 'change:range', { directChange: false } ); - contextualToolbar.fire( '_selectionChangeDebounced' ); + sinon.assert.calledOnce( showPanelSpy ); + sinon.assert.notCalled( hidePanelSpy ); + } ); - sinon.assert.notCalled( spy ); + it( 'should close when selection starts changing by not a directChange but will become collapsed', () => { + contextualToolbar.fire( '_selectionChangeDebounced' ); - editor.editing.view.fire( 'render' ); + sinon.assert.calledOnce( showPanelSpy ); + sinon.assert.notCalled( hidePanelSpy ); - sinon.assert.calledOnce( spy ); - } ); + // Collapse range silently (without firing `change:range` { directChange: true } event). + const range = editor.document.selection._ranges[ 0 ]; + range.end = range.start; - it( 'should update balloon position when toolbar is closed', () => { - const spy = sandbox.spy( balloon, 'updatePosition' ); + editor.document.selection.fire( 'change:range', { directChange: false } ); - setData( editor.document, '[bar]' ); + sinon.assert.calledOnce( showPanelSpy ); + sinon.assert.calledOnce( hidePanelSpy ); + } ); - contextualToolbar.fire( '_selectionChangeDebounced' ); + it( 'should hide if the editor loses focus', () => { + editor.ui.focusTracker.isFocused = true; - // Hide toolbar. - editor.document.selection.fire( 'change:range', { directChange: true } ); + contextualToolbar.fire( '_selectionChangeDebounced' ); - editor.editing.view.fire( 'render' ); + sinon.stub( balloon, 'visibleView', { get: () => contextualToolbar.toolbarView } ); - sinon.assert.notCalled( spy ); - } ); + sinon.assert.calledOnce( showPanelSpy ); + sinon.assert.notCalled( hidePanelSpy ); - describe( 'pluginName', () => { - it( 'should return plugin by name', () => { - expect( editor.plugins.get( 'contextualballoon' ) ).to.equal( balloon ); + editor.ui.focusTracker.isFocused = false; + + sinon.assert.calledOnce( showPanelSpy ); + sinon.assert.calledOnce( hidePanelSpy ); } ); - } ); - describe( 'destroy()', () => { - it( 'should not fire `_selectionChangeDebounced` after plugin destroy', ( done ) => { - const spy = sandbox.spy(); + it( 'should not hide if the editor loses focus and #toolbarView is not visible', () => { + editor.ui.focusTracker.isFocused = true; - contextualToolbar.on( '_selectionChangeDebounced', spy ); + contextualToolbar.fire( '_selectionChangeDebounced' ); - editor.document.selection.fire( 'change:range', { directChange: true } ); + sinon.stub( balloon, 'visibleView', { get: () => null } ); - contextualToolbar.destroy(); + sinon.assert.calledOnce( showPanelSpy ); + sinon.assert.notCalled( hidePanelSpy ); - setTimeout( () => { - sinon.assert.notCalled( spy ); - done(); - }, 200 ); + editor.ui.focusTracker.isFocused = false; + + sinon.assert.calledOnce( showPanelSpy ); + sinon.assert.notCalled( hidePanelSpy ); } ); } ); + + function stubSelectionRect( forwardSelectionRect, backwardSelectionRect ) { + const editingView = editor.editing.view; + const originalViewRangeToDom = editingView.domConverter.viewRangeToDom; + + // Mock selection rect. + sandbox.stub( editingView.domConverter, 'viewRangeToDom', ( ...args ) => { + const domRange = originalViewRangeToDom.apply( editingView.domConverter, args ); + + sandbox.stub( domRange, 'getClientRects', () => { + return { + length: 2, + item: id => id === 0 ? forwardSelectionRect : backwardSelectionRect + }; + } ); + + return domRange; + } ); + } } ); From 094c4e828393b2b9087839dab7bb61d079c5c2e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Tue, 18 Apr 2017 18:06:28 +0200 Subject: [PATCH 35/41] Changed plugin name. --- src/toolbar/contextual/contextualtoolbar.js | 2 +- tests/toolbar/contextual/contextualtoolbar.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/toolbar/contextual/contextualtoolbar.js b/src/toolbar/contextual/contextualtoolbar.js index 0f02f536..69855095 100644 --- a/src/toolbar/contextual/contextualtoolbar.js +++ b/src/toolbar/contextual/contextualtoolbar.js @@ -27,7 +27,7 @@ export default class ContextualToolbar extends Plugin { * @inheritDoc */ static get pluginName() { - return 'contextualtoolbar'; + return 'ui/contextualtoolbar'; } /** diff --git a/tests/toolbar/contextual/contextualtoolbar.js b/tests/toolbar/contextual/contextualtoolbar.js index 5fc2d384..fc9acfbf 100644 --- a/tests/toolbar/contextual/contextualtoolbar.js +++ b/tests/toolbar/contextual/contextualtoolbar.js @@ -104,7 +104,7 @@ describe( 'ContextualToolbar', () => { describe( 'pluginName', () => { it( 'should return plugin by its name', () => { - expect( editor.plugins.get( 'contextualtoolbar' ) ).to.equal( contextualToolbar ); + expect( editor.plugins.get( 'ui/contextualtoolbar' ) ).to.equal( contextualToolbar ); } ); } ); From 0fbf00907aa747b72acb118283731c1d1e0b7034 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Tue, 18 Apr 2017 18:08:42 +0200 Subject: [PATCH 36/41] Changed ContextualBalloon plugin name. --- src/panel/balloon/contextualballoon.js | 2 +- tests/panel/balloon/contextualballoon.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/panel/balloon/contextualballoon.js b/src/panel/balloon/contextualballoon.js index eea317bf..aed4a67f 100644 --- a/src/panel/balloon/contextualballoon.js +++ b/src/panel/balloon/contextualballoon.js @@ -32,7 +32,7 @@ export default class ContextualBalloon extends Plugin { * @inheritDoc */ static get pluginName() { - return 'contextualballoon'; + return 'ui/contextualballoon'; } /** diff --git a/tests/panel/balloon/contextualballoon.js b/tests/panel/balloon/contextualballoon.js index 9cad6166..ebb3dfa5 100644 --- a/tests/panel/balloon/contextualballoon.js +++ b/tests/panel/balloon/contextualballoon.js @@ -56,7 +56,7 @@ describe( 'ContextualBalloon', () => { describe( 'pluginName', () => { it( 'should return plugin by name', () => { - expect( editor.plugins.get( 'contextualballoon' ) ).to.equal( balloon ); + expect( editor.plugins.get( 'ui/contextualballoon' ) ).to.equal( balloon ); } ); } ); From b9b4b9105894f57bf51f8143c07904a87c025755 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Wed, 19 Apr 2017 08:16:24 +0200 Subject: [PATCH 37/41] Improved tests tear down. --- tests/toolbar/contextual/contextualtoolbar.js | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/tests/toolbar/contextual/contextualtoolbar.js b/tests/toolbar/contextual/contextualtoolbar.js index fc9acfbf..59d59332 100644 --- a/tests/toolbar/contextual/contextualtoolbar.js +++ b/tests/toolbar/contextual/contextualtoolbar.js @@ -42,6 +42,8 @@ describe( 'ContextualToolbar', () => { // Focus the engine. editor.editing.view.isFocused = true; + + return contextualToolbar.toolbarView.init(); } ); } ); @@ -184,7 +186,7 @@ describe( 'ContextualToolbar', () => { it( 'should update balloon position on ViewDocument#render event while balloon is added to the #_balloon', () => { setData( editor.document, 'b[a]r' ); - const spy = sinon.spy( balloon, 'updatePosition' ); + const spy = sandbox.spy( balloon, 'updatePosition' ); editor.editing.view.fire( 'render' ); @@ -250,7 +252,7 @@ describe( 'ContextualToolbar', () => { it( 'should stop update balloon position on ViewDocument#render event', () => { setData( editor.document, 'b[a]r' ); - const spy = sinon.spy( balloon, 'updatePosition' ); + const spy = sandbox.spy( balloon, 'updatePosition' ); return contextualToolbar._showPanel() .then( () => { @@ -352,7 +354,8 @@ describe( 'ContextualToolbar', () => { contextualToolbar.fire( '_selectionChangeDebounced' ); - sinon.stub( balloon, 'visibleView', { get: () => contextualToolbar.toolbarView } ); + // Stubbing getters doesn't wor for sandbox. + const stub = sinon.stub( balloon, 'visibleView', { get: () => contextualToolbar.toolbarView } ); sinon.assert.calledOnce( showPanelSpy ); sinon.assert.notCalled( hidePanelSpy ); @@ -361,6 +364,8 @@ describe( 'ContextualToolbar', () => { sinon.assert.calledOnce( showPanelSpy ); sinon.assert.calledOnce( hidePanelSpy ); + + stub.restore(); } ); it( 'should not hide if the editor loses focus and #toolbarView is not visible', () => { @@ -368,7 +373,8 @@ describe( 'ContextualToolbar', () => { contextualToolbar.fire( '_selectionChangeDebounced' ); - sinon.stub( balloon, 'visibleView', { get: () => null } ); + // Stubbing getters doesn't wor for sandbox. + const stub = sinon.stub( balloon, 'visibleView', { get: () => null } ); sinon.assert.calledOnce( showPanelSpy ); sinon.assert.notCalled( hidePanelSpy ); @@ -377,6 +383,8 @@ describe( 'ContextualToolbar', () => { sinon.assert.calledOnce( showPanelSpy ); sinon.assert.notCalled( hidePanelSpy ); + + stub.restore(); } ); } ); From 23398121e8ccfda305a7aa38dcfcd29f4f098bae Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Wed, 19 Apr 2017 15:05:11 +0200 Subject: [PATCH 38/41] Imported 2 positions from MT to BalloonPanelView.defaultPositions. Fixed broken MT. --- src/panel/balloon/balloonpanelview.js | 12 ++++++++++ tests/manual/imagetoolbar/imagetoolbar.js | 29 +++-------------------- tests/panel/balloon/balloonpanelview.js | 16 +++++++++++++ 3 files changed, 31 insertions(+), 26 deletions(-) diff --git a/src/panel/balloon/balloonpanelview.js b/src/panel/balloon/balloonpanelview.js index 6f25f3b2..824d724c 100644 --- a/src/panel/balloon/balloonpanelview.js +++ b/src/panel/balloon/balloonpanelview.js @@ -460,5 +460,17 @@ BalloonPanelView.defaultPositions = { top: targetRect.bottom + BalloonPanelView.arrowVerticalOffset, left: targetRect.left - balloonRect.width / 2, name: 'arrow_s' + } ), + + southArrowNorth: ( targetRect, balloonRect ) => ( { + top: targetRect.bottom + BalloonPanelView.arrowVerticalOffset, + left: targetRect.left + targetRect.width / 2 - balloonRect.width / 2, + name: 'arrow_s' + } ), + + northArrowSouth: ( targetRect, balloonRect ) => ( { + top: targetRect.top - balloonRect.height - BalloonPanelView.arrowVerticalOffset, + left: targetRect.left + targetRect.width / 2 - balloonRect.width / 2, + name: 'arrow_n' } ) }; diff --git a/tests/manual/imagetoolbar/imagetoolbar.js b/tests/manual/imagetoolbar/imagetoolbar.js index 0dcf56f4..ae5fab8e 100644 --- a/tests/manual/imagetoolbar/imagetoolbar.js +++ b/tests/manual/imagetoolbar/imagetoolbar.js @@ -19,31 +19,6 @@ import Template from '../../../src/template'; import ToolbarView from '../../../src/toolbar/toolbarview'; import BalloonPanelView from '../../../src/panel/balloon/balloonpanelview'; -const arrowVOffset = BalloonPanelView.arrowVerticalOffset; -const positions = { - // [text range] - // ^ - // +-----------------+ - // | Balloon | - // +-----------------+ - south: ( targetRect, balloonRect ) => ( { - top: targetRect.bottom + arrowVOffset, - left: targetRect.left + targetRect.width / 2 - balloonRect.width / 2, - name: 's' - } ), - - // +-----------------+ - // | Balloon | - // +-----------------+ - // V - // [text range] - north: ( targetRect, balloonRect ) => ( { - top: targetRect.top - balloonRect.height - arrowVOffset, - left: targetRect.left + targetRect.width / 2 - balloonRect.width / 2, - name: 'n' - } ) -}; - ClassicEditor.create( document.querySelector( '#editor' ), { plugins: [ Enter, Typing, Paragraph, Undo, Bold, Italic, Image ], toolbar: [ 'bold', 'italic', 'undo', 'redo' ] @@ -111,9 +86,11 @@ function createImageToolbar( editor ) { } ); function attachToolbar() { + const defaultPositions = BalloonPanelView.defaultPositions; + panel.attachTo( { target: editingView.domConverter.viewRangeToDom( editingView.selection.getFirstRange() ), - positions: [ positions.north, positions.south ] + positions: [ defaultPositions.northArrowSouth, defaultPositions.southArrowNorth ] } ); } } ); diff --git a/tests/panel/balloon/balloonpanelview.js b/tests/panel/balloon/balloonpanelview.js index 2166717f..20681c7f 100644 --- a/tests/panel/balloon/balloonpanelview.js +++ b/tests/panel/balloon/balloonpanelview.js @@ -721,6 +721,22 @@ describe( 'BalloonPanelView', () => { name: 'arrow_s' } ); } ); + + it( 'southArrowNorth', () => { + expect( positions.southArrowNorth( targetRect, balloonRect ) ).to.deep.equal( { + top: 215, + left: 125, + name: 'arrow_s' + } ); + } ); + + it( 'northArrowSouth', () => { + expect( positions.northArrowSouth( targetRect, balloonRect ) ).to.deep.equal( { + top: 35, + left: 125, + name: 'arrow_n' + } ); + } ); } ); } ); From 105a544f7000fbd180bddad6fc4fbf68ad8375cd Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Wed, 19 Apr 2017 15:16:23 +0200 Subject: [PATCH 39/41] Docs: Updated BalloonPanelView#defaultPositions docs. --- src/panel/balloon/balloonpanelview.js | 40 +++++++++++++++++++-------- 1 file changed, 28 insertions(+), 12 deletions(-) diff --git a/src/panel/balloon/balloonpanelview.js b/src/panel/balloon/balloonpanelview.js index 824d724c..f6202791 100644 --- a/src/panel/balloon/balloonpanelview.js +++ b/src/panel/balloon/balloonpanelview.js @@ -333,7 +333,7 @@ BalloonPanelView.arrowVerticalOffset = 15; * * The available positioning functions are as follows: * - * * South east arrow north west: + * * South east (arrow north west): * * [ Target ] * ^ @@ -342,7 +342,7 @@ BalloonPanelView.arrowVerticalOffset = 15; * +-----------------+ * * - * * South west arrow north east: + * * South west (arrow north east): * * [ Target ] * ^ @@ -351,7 +351,7 @@ BalloonPanelView.arrowVerticalOffset = 15; * +-----------------+ * * - * * North east arrow south west: + * * North east (arrow south west): * * +-----------------+ * | Balloon | @@ -360,7 +360,7 @@ BalloonPanelView.arrowVerticalOffset = 15; * [ Target ] * * - * * North west arrow south east: + * * North west (arrow south east): * * +-----------------+ * | Balloon | @@ -369,41 +369,57 @@ BalloonPanelView.arrowVerticalOffset = 15; * [ Target ] * * - * * South east arrow north: + * * South east (arrow north): * - * [text range] + * [ Target ] * ^ * +-----------------+ * | Balloon | * +-----------------+ * * - * * North east arrow south: + * * North east (arrow south): * * +-----------------+ * | Balloon | * +-----------------+ * V - * [text range] + * [ Target ] * * - * * North west arrow south: + * * North west (arrow south): * * +-----------------+ * | Balloon | * +-----------------+ * V - * [text range] + * [ Target ] * * - * * South west arrow north: + * * South west (arrow north): * - * [text range] + * [ Target ] * ^ * +-----------------+ * | Balloon | * +-----------------+ * + * * South (arrow north): + * + * [ Target ] + * ^ + * +-----------------+ + * | Balloon | + * +-----------------+ + * + * * North (arrow south): + * + * +-----------------+ + * | Balloon | + * +-----------------+ + * V + * [ Target ] + * * See {@link module:ui/panel/balloon/balloonpanelview~BalloonPanelView#attachTo}. * * Positioning functions must be compatible with {@link module:utils/dom/position~Position}. From 6f6e55086bea0f2877d90429bab77281994502b7 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Wed, 19 Apr 2017 15:30:25 +0200 Subject: [PATCH 40/41] Code refactoring: Simplified BalloonPanelView.defaultPositions' names which were ambiguous. --- src/panel/balloon/balloonpanelview.js | 28 +++++------ .../contextualballoon/contextualballoon.js | 19 ++------ tests/panel/balloon/balloonpanelview.js | 46 +++++++++---------- theme/components/panel/balloonpanel.scss | 12 ++--- 4 files changed, 46 insertions(+), 59 deletions(-) diff --git a/src/panel/balloon/balloonpanelview.js b/src/panel/balloon/balloonpanelview.js index f6202791..ef4d0a88 100644 --- a/src/panel/balloon/balloonpanelview.js +++ b/src/panel/balloon/balloonpanelview.js @@ -52,7 +52,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_se` for "arrow_se" position. The class + * to the balloon, i.e. `.ck-balloon-panel_arrow_ne` for "arrow_ne" 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. * @@ -64,10 +64,10 @@ export default class BalloonPanelView extends View { * See {@link #withArrow}. * * @observable - * @default 'arrow_se' - * @member {'arrow_se'|'arrow_sw'|'arrow_ne'|'arrow_nw'} #position + * @default 'arrow_ne' + * @member {'arrow_ne'|'arrow_nw'|'arrow_se'|'arrow_sw'} #position */ - this.set( 'position', 'arrow_se' ); + this.set( 'position', 'arrow_ne' ); /** * Controls whether the balloon panel is visible or not. @@ -433,60 +433,60 @@ BalloonPanelView.defaultPositions = { southEastArrowNorthEast: ( targetRect ) => ( { top: targetRect.bottom + BalloonPanelView.arrowVerticalOffset, left: targetRect.left + targetRect.width / 2 - BalloonPanelView.arrowHorizontalOffset, - name: 'arrow_se' + name: 'arrow_ne' } ), southWestArrowNorthEast: ( targetRect, balloonRect ) => ( { top: targetRect.bottom + BalloonPanelView.arrowVerticalOffset, left: targetRect.left + targetRect.width / 2 - balloonRect.width + BalloonPanelView.arrowHorizontalOffset, - name: 'arrow_sw' + name: 'arrow_nw' } ), northEastArrowSouthWest: ( targetRect, balloonRect ) => ( { top: targetRect.top - balloonRect.height - BalloonPanelView.arrowVerticalOffset, left: targetRect.left + targetRect.width / 2 - BalloonPanelView.arrowHorizontalOffset, - name: 'arrow_ne' + name: 'arrow_se' } ), northWestArrowSouthEast: ( targetRect, balloonRect ) => ( { top: targetRect.top - balloonRect.height - BalloonPanelView.arrowVerticalOffset, left: targetRect.left + targetRect.width / 2 - balloonRect.width + BalloonPanelView.arrowHorizontalOffset, - name: 'arrow_nw' + name: 'arrow_sw' } ), southEastArrowNorth: ( targetRect, balloonRect ) => ( { top: targetRect.bottom + BalloonPanelView.arrowVerticalOffset, left: targetRect.right - balloonRect.width / 2, - name: 'arrow_s' + name: 'arrow_n' } ), northEastArrowSouth: ( targetRect, balloonRect ) => ( { top: targetRect.top - balloonRect.height - BalloonPanelView.arrowVerticalOffset, left: targetRect.right - balloonRect.width / 2, - name: 'arrow_n' + name: 'arrow_s' } ), northWestArrowSouth: ( targetRect, balloonRect ) => ( { top: targetRect.top - balloonRect.height - BalloonPanelView.arrowVerticalOffset, left: targetRect.left - balloonRect.width / 2, - name: 'arrow_n' + name: 'arrow_s' } ), southWestArrowNorth: ( targetRect, balloonRect ) => ( { top: targetRect.bottom + BalloonPanelView.arrowVerticalOffset, left: targetRect.left - balloonRect.width / 2, - name: 'arrow_s' + name: 'arrow_n' } ), southArrowNorth: ( targetRect, balloonRect ) => ( { top: targetRect.bottom + BalloonPanelView.arrowVerticalOffset, left: targetRect.left + targetRect.width / 2 - balloonRect.width / 2, - name: 'arrow_s' + name: 'arrow_n' } ), northArrowSouth: ( targetRect, balloonRect ) => ( { top: targetRect.top - balloonRect.height - BalloonPanelView.arrowVerticalOffset, left: targetRect.left + targetRect.width / 2 - balloonRect.width / 2, - name: 'arrow_n' + name: 'arrow_s' } ) }; diff --git a/tests/manual/contextualballoon/contextualballoon.js b/tests/manual/contextualballoon/contextualballoon.js index 5f16b477..1fd15e1a 100644 --- a/tests/manual/contextualballoon/contextualballoon.js +++ b/tests/manual/contextualballoon/contextualballoon.js @@ -16,6 +16,7 @@ import EssentialsPresets from '@ckeditor/ckeditor5-presets/src/essentials'; import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph'; import Plugin from '@ckeditor/ckeditor5-core/src/plugin'; +import BalloonPanelView from '../../../src/panel/balloon/balloonpanelview'; import ContextualBalloon from '../../../src/panel/balloon/contextualballoon'; import ToolbarView from '../../../src/toolbar/toolbarview'; import ButtonView from '../../../src/button/buttonview'; @@ -224,21 +225,7 @@ function createContextualToolbar( editor ) { const balloon = editor.plugins.get( ContextualBalloon ); const toolbar = new ToolbarView(); const editingView = editor.editing.view; - const arrowVOffset = 20; - - const positions = { - forwardSelection: ( targetRect, balloonRect ) => ( { - top: targetRect.bottom + arrowVOffset, - left: targetRect.right - balloonRect.width / 2, - name: 'arrow_s' - } ), - - backwardSelection: ( targetRect, balloonRect ) => ( { - top: targetRect.top - balloonRect.height - arrowVOffset, - left: targetRect.left - balloonRect.width / 2, - name: 'arrow_n' - } ) - }; + const defaultPositions = BalloonPanelView.defaultPositions; // Add plugins to the toolbar. toolbar.fillFromConfig( [ 'PluginA', 'PluginB' ], editor.ui.componentFactory ); @@ -257,7 +244,7 @@ function createContextualToolbar( editor ) { view: toolbar, position: { target: isBackward ? rangeRects.item( 0 ) : rangeRects.item( rangeRects.length - 1 ), - positions: [ positions[ isBackward ? 'backwardSelection' : 'forwardSelection' ] ] + positions: [ defaultPositions[ isBackward ? 'northArrowSouth' : 'southArrowNorth' ] ] } } ); } diff --git a/tests/panel/balloon/balloonpanelview.js b/tests/panel/balloon/balloonpanelview.js index 20681c7f..35ee09a6 100644 --- a/tests/panel/balloon/balloonpanelview.js +++ b/tests/panel/balloon/balloonpanelview.js @@ -41,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( 'arrow_se' ); + expect( view.position ).to.equal( 'arrow_ne' ); expect( view.isVisible ).to.equal( false ); expect( view.withArrow ).to.equal( true ); } ); @@ -54,11 +54,11 @@ describe( 'BalloonPanelView', () => { describe( 'DOM bindings', () => { describe( 'arrow', () => { it( 'should react on view#position', () => { - expect( view.element.classList.contains( 'ck-balloon-panel_arrow_se' ) ).to.true; + expect( view.element.classList.contains( 'ck-balloon-panel_arrow_ne' ) ).to.true; - view.position = 'arrow_sw'; + view.position = 'arrow_nw'; - expect( view.element.classList.contains( 'ck-balloon-panel_arrow_sw' ) ).to.true; + expect( view.element.classList.contains( 'ck-balloon-panel_arrow_nw' ) ).to.true; } ); it( 'should react on view#withArrow', () => { @@ -221,7 +221,7 @@ describe( 'BalloonPanelView', () => { view.attachTo( { target, limiter } ); - expect( view.position ).to.equal( 'arrow_se' ); + expect( view.position ).to.equal( 'arrow_ne' ); } ); it( 'should put balloon on the `south east` side of the target element when target is on the top left side of the limiter', () => { @@ -234,7 +234,7 @@ describe( 'BalloonPanelView', () => { view.attachTo( { target, limiter } ); - expect( view.position ).to.equal( 'arrow_se' ); + expect( view.position ).to.equal( 'arrow_ne' ); } ); 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_sw' ); + expect( view.position ).to.equal( 'arrow_nw' ); } ); 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_ne' ); + expect( view.position ).to.equal( 'arrow_se' ); } ); it( 'should put balloon on the `north west` side of the target element when target is on the bottom right of the limiter', () => { @@ -273,7 +273,7 @@ describe( 'BalloonPanelView', () => { view.attachTo( { target, limiter } ); - expect( view.position ).to.equal( 'arrow_nw' ); + expect( view.position ).to.equal( 'arrow_sw' ); } ); // https://github.com/ckeditor/ckeditor5-ui-default/issues/126 @@ -354,7 +354,7 @@ describe( 'BalloonPanelView', () => { view.attachTo( { target, limiter } ); - expect( view.position ).to.equal( 'arrow_sw' ); + expect( view.position ).to.equal( 'arrow_nw' ); } ); it( 'should put balloon on the `south east` position when `south west` is limited', () => { @@ -374,7 +374,7 @@ describe( 'BalloonPanelView', () => { view.attachTo( { target, limiter } ); - expect( view.position ).to.equal( 'arrow_se' ); + expect( view.position ).to.equal( 'arrow_ne' ); } ); it( 'should put balloon on the `north east` position when `south east` is limited', () => { @@ -398,7 +398,7 @@ describe( 'BalloonPanelView', () => { view.attachTo( { target, limiter } ); - expect( view.position ).to.equal( 'arrow_ne' ); + expect( view.position ).to.equal( 'arrow_se' ); } ); it( 'should put balloon on the `south east` position when `north east` is limited', () => { @@ -418,7 +418,7 @@ describe( 'BalloonPanelView', () => { view.attachTo( { target, limiter } ); - expect( view.position ).to.equal( 'arrow_se' ); + expect( view.position ).to.equal( 'arrow_ne' ); } ); } ); } ); @@ -662,7 +662,7 @@ describe( 'BalloonPanelView', () => { expect( positions.southEastArrowNorthEast( targetRect ) ).to.deep.equal( { top: 215, left: 120, - name: 'arrow_se' + name: 'arrow_ne' } ); } ); @@ -670,7 +670,7 @@ describe( 'BalloonPanelView', () => { expect( positions.southWestArrowNorthEast( targetRect, balloonRect ) ).to.deep.equal( { top: 215, left: 130, - name: 'arrow_sw' + name: 'arrow_nw' } ); } ); @@ -678,7 +678,7 @@ describe( 'BalloonPanelView', () => { expect( positions.northEastArrowSouthWest( targetRect, balloonRect ) ).to.deep.equal( { top: 35, left: 120, - name: 'arrow_ne' + name: 'arrow_se' } ); } ); @@ -686,7 +686,7 @@ describe( 'BalloonPanelView', () => { expect( positions.northWestArrowSouthEast( targetRect, balloonRect ) ).to.deep.equal( { top: 35, left: 130, - name: 'arrow_nw' + name: 'arrow_sw' } ); } ); @@ -694,7 +694,7 @@ describe( 'BalloonPanelView', () => { expect( positions.southEastArrowNorth( targetRect, balloonRect ) ).to.deep.equal( { top: 215, left: 175, - name: 'arrow_s' + name: 'arrow_n' } ); } ); @@ -702,7 +702,7 @@ describe( 'BalloonPanelView', () => { expect( positions.northEastArrowSouth( targetRect, balloonRect ) ).to.deep.equal( { top: 35, left: 175, - name: 'arrow_n' + name: 'arrow_s' } ); } ); @@ -710,7 +710,7 @@ describe( 'BalloonPanelView', () => { expect( positions.northWestArrowSouth( targetRect, balloonRect ) ).to.deep.equal( { top: 35, left: 75, - name: 'arrow_n' + name: 'arrow_s' } ); } ); @@ -718,7 +718,7 @@ describe( 'BalloonPanelView', () => { expect( positions.southWestArrowNorth( targetRect, balloonRect ) ).to.deep.equal( { top: 215, left: 75, - name: 'arrow_s' + name: 'arrow_n' } ); } ); @@ -726,7 +726,7 @@ describe( 'BalloonPanelView', () => { expect( positions.southArrowNorth( targetRect, balloonRect ) ).to.deep.equal( { top: 215, left: 125, - name: 'arrow_s' + name: 'arrow_n' } ); } ); @@ -734,7 +734,7 @@ describe( 'BalloonPanelView', () => { expect( positions.northArrowSouth( targetRect, balloonRect ) ).to.deep.equal( { top: 35, left: 125, - name: 'arrow_n' + name: 'arrow_s' } ); } ); } ); diff --git a/theme/components/panel/balloonpanel.scss b/theme/components/panel/balloonpanel.scss index ca578498..da68b3aa 100644 --- a/theme/components/panel/balloonpanel.scss +++ b/theme/components/panel/balloonpanel.scss @@ -26,9 +26,9 @@ } &.ck-balloon-panel_arrow { - &_s, - &_se, - &_sw { + &_n, + &_ne, + &_nw { &:before { z-index: ck-z( 'default' ); } @@ -38,9 +38,9 @@ } } - &_n, - &_ne, - &_nw { + &_s, + &_se, + &_sw { &:before { z-index: ck-z( 'default' ); } From 94f6a15dd05b9b3ca66f5249cc317e70b554d044 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Wed, 19 Apr 2017 17:01:01 +0200 Subject: [PATCH 41/41] Docs: Improved docs in the ContextualToolbar class. --- src/toolbar/contextual/contextualtoolbar.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/toolbar/contextual/contextualtoolbar.js b/src/toolbar/contextual/contextualtoolbar.js index 69855095..fb8c02cf 100644 --- a/src/toolbar/contextual/contextualtoolbar.js +++ b/src/toolbar/contextual/contextualtoolbar.js @@ -102,10 +102,10 @@ export default class ContextualToolbar extends Plugin { } /** - * Handles {@link module:engine/model/document#selection} change and show or hide toolbar. + * Handles {@link module:engine/model/document~Document#selection} change and show or hide toolbar. * - * Note that in this case it's better to listen to {@link module:engine/model/document modelDocument} - * selection instead of {@link module:engine/view/document viewDocument} selection because the first one + * Note that in this case it's better to listen to {@link module:engine/model/document~Document model document} + * selection instead of {@link module:engine/view/document~Document view document} selection because the first one * doesn't fire `change` event after text style change (like bold or italic) and toolbar doesn't blink. * * @private