diff --git a/src/toolbar/balloon/balloontoolbar.js b/src/toolbar/balloon/balloontoolbar.js index 827946bd..b6c1e86e 100644 --- a/src/toolbar/balloon/balloontoolbar.js +++ b/src/toolbar/balloon/balloontoolbar.js @@ -40,31 +40,21 @@ export default class BalloonToolbar extends Plugin { /** * @inheritDoc */ - init() { - const editor = this.editor; + constructor( editor ) { + super( editor ); /** * The toolbar view displayed in the balloon. * - * @member {module:ui/toolbar/toolbarview~ToolbarView} + * @type {module:ui/toolbar/toolbarview~ToolbarView} */ - this.toolbarView = new ToolbarView( editor.locale ); - - this.toolbarView.extendTemplate( { - attributes: { - class: [ - 'ck-toolbar_floating' - ] - } - } ); - - this.toolbarView.render(); + this.toolbarView = this._createToolbarView(); /** * The contextual balloon plugin instance. * * @private - * @member {module:ui/panel/balloon/contextualballoon~ContextualBalloon} + * @type {module:ui/panel/balloon/contextualballoon~ContextualBalloon} */ this._balloon = editor.plugins.get( ContextualBalloon ); @@ -75,19 +65,53 @@ export default class BalloonToolbar extends Plugin { * trailing debounced invocation on destroy. * * @private - * @member {Function} + * @type {Function} */ this._fireSelectionChangeDebounced = debounce( () => this.fire( '_selectionChangeDebounced' ), 200 ); - // Attach lifecycle actions. - this._handleSelectionChange(); - this._handleFocusChange(); - // The appearance of the BalloonToolbar method is event–driven. // It is possible to stop the #show event and this prevent the toolbar from showing up. this.decorate( 'show' ); } + /** + * @inheritDoc + */ + init() { + const editor = this.editor; + const selection = editor.model.document.selection; + + // Show/hide the toolbar on editable focus/blur. + this.listenTo( editor.editing.view.document, 'change:isFocused', ( evt, name, isEditableFocused ) => { + const isToolbarFocused = this.toolbarView.focusTracker.isFocused; + const isToolbarVisible = this._balloon.visibleView === this.toolbarView; + + if ( !isEditableFocused && !isToolbarFocused && isToolbarVisible ) { + this.hide(); + } else if ( isEditableFocused ) { + this.show(); + } + } ); + + // Hide the toolbar when the selection is changed by a direct change or has changed to collapsed. + this.listenTo( selection, 'change:range', ( evt, data ) => { + if ( data.directChange || selection.isCollapsed ) { + this.hide(); + } + + // Fire internal `_selectionChangeDebounced` event to use it for showing + // the toolbar after the selection stops changing. + this._fireSelectionChangeDebounced(); + } ); + + // Show the toolbar when the selection stops changing. + this.listenTo( this, '_selectionChangeDebounced', () => { + if ( this.editor.editing.view.document.isFocused ) { + this.show(); + } + } ); + } + /** * Creates toolbar components based on given configuration. * This needs to be done when all plugins are ready. @@ -102,52 +126,23 @@ export default class BalloonToolbar extends Plugin { } /** - * Handles the editor focus change and hides the toolbar if it's needed. + * Creates the toolbar view instance. * * @private + * @returns {module:ui/toolbar/toolbarview~ToolbarView} */ - _handleFocusChange() { - const editor = this.editor; + _createToolbarView() { + const toolbarView = new ToolbarView( this.editor.locale ); - // Hide the panel View when editor loses focus but no the other way around. - this.listenTo( editor.ui.focusTracker, 'change:isFocused', ( evt, name, isFocused ) => { - if ( this._balloon.visibleView === this.toolbarView && !isFocused ) { - this.hide(); + toolbarView.extendTemplate( { + attributes: { + class: [ 'ck-toolbar_floating' ] } } ); - } - - /** - * 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~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 - */ - _handleSelectionChange() { - const selection = this.editor.model.document.selection; - const viewDocument = this.editor.editing.view.document; - - 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.hide(); - } - // Fire internal `_selectionChangeDebounced` when the selection stops changing. - this._fireSelectionChangeDebounced(); - } ); + toolbarView.render(); - // Hide the toolbar when the selection stops changing. - this.listenTo( this, '_selectionChangeDebounced', () => { - // This implementation assumes that only non–collapsed selections gets the contextual toolbar. - if ( viewDocument.isFocused && !viewDocument.selection.isCollapsed ) { - this.show(); - } - } ); + return toolbarView; } /** @@ -156,11 +151,18 @@ export default class BalloonToolbar extends Plugin { * Fires {@link #event:show} event which can be stopped to prevent the toolbar from showing up. */ show() { + const editor = this.editor; + // Do not add the toolbar to the balloon stack twice. if ( this._balloon.hasView( this.toolbarView ) ) { return; } + // Do not show the toolbar when the selection is collapsed. + if ( editor.model.document.selection.isCollapsed ) { + return; + } + // Don not show the toolbar when all components inside are disabled // see https://github.com/ckeditor/ckeditor5-ui/issues/269. if ( Array.from( this.toolbarView.items ).every( item => item.isEnabled !== undefined && !item.isEnabled ) ) { diff --git a/tests/manual/tickets/418/1.html b/tests/manual/tickets/418/1.html new file mode 100644 index 00000000..cefb209d --- /dev/null +++ b/tests/manual/tickets/418/1.html @@ -0,0 +1,31 @@ +
+
+

The three greatest things you learn from traveling

+

+ Like all the great things on earth traveling teaches us by example. Here are some of the most precious lessons + I’ve learned over the years of traveling. +

+ +

Appreciation of diversity

+

+ Getting used to an entirely different culture can be challenging. While it’s also nice to learn about + cultures online or from books, nothing comes close to experiencing cultural diversity in person. + You learn to appreciate each and every single one of the differences while you become more culturally fluid. +

+
+
+ + diff --git a/tests/manual/tickets/418/1.js b/tests/manual/tickets/418/1.js new file mode 100644 index 00000000..8c6d618c --- /dev/null +++ b/tests/manual/tickets/418/1.js @@ -0,0 +1,45 @@ +/** + * @license Copyright (c) 2003-2018, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md. + */ + +/* globals window, document, console:false */ + +import BalloonEditor from '@ckeditor/ckeditor5-editor-balloon/src/ballooneditor'; +import Essentials from '@ckeditor/ckeditor5-essentials/src/essentials'; +import List from '@ckeditor/ckeditor5-list/src/list'; +import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph'; +import Heading from '@ckeditor/ckeditor5-heading/src/heading'; +import HeadingButtonsUI from '@ckeditor/ckeditor5-heading/src/headingbuttonsui'; +import ParagraphButtonUI from '@ckeditor/ckeditor5-paragraph/src/paragraphbuttonui'; +import Bold from '@ckeditor/ckeditor5-basic-styles/src/bold'; +import Italic from '@ckeditor/ckeditor5-basic-styles/src/italic'; +import Link from '@ckeditor/ckeditor5-link/src/link'; + +import BlockToolbar from '../../../../src/toolbar/block/blocktoolbar'; +import BalloonToolbar from '../../../../src/toolbar/balloon/balloontoolbar'; + +BalloonEditor + .create( document.querySelector( '#editor' ), { + plugins: [ + Essentials, + List, + Paragraph, + Heading, + HeadingButtonsUI, + ParagraphButtonUI, + Bold, + Italic, + Link, + BlockToolbar, + BalloonToolbar + ], + blockToolbar: [ 'paragraph', 'heading1', 'heading2', 'heading3', 'bulletedList', 'numberedList' ], + balloonToolbar: [ 'bold', 'italic', 'link' ] + } ) + .then( editor => { + window.editor = editor; + } ) + .catch( err => { + console.error( err.stack ); + } ); diff --git a/tests/manual/tickets/418/1.md b/tests/manual/tickets/418/1.md new file mode 100644 index 00000000..bae780ad --- /dev/null +++ b/tests/manual/tickets/418/1.md @@ -0,0 +1,5 @@ +## BlockToolbar and BalloonToolbar collision [#418](https://github.com/ckeditor/ckeditor5-ui/issues/418) + +- select some text, balloon toolbar should show up +- click block toolbar button, balloon toolbar should hide +- pres `Esc` to move focus back to editable, balloon toolbar should show up diff --git a/tests/toolbar/balloon/balloontoolbar.js b/tests/toolbar/balloon/balloontoolbar.js index 906ce89a..8eb91d10 100644 --- a/tests/toolbar/balloon/balloontoolbar.js +++ b/tests/toolbar/balloon/balloontoolbar.js @@ -275,7 +275,7 @@ describe( 'BalloonToolbar', () => { expect( targetRect ).to.deep.equal( backwardSelectionRect ); } ); - it( 'should update balloon position on ui#update event while balloon is added to the #_balloon', () => { + it( 'should update balloon position on ui#update event when #toolbarView is already added to the #_balloon', () => { setData( model, 'b[a]r' ); const spy = sandbox.spy( balloon, 'updatePosition' ); @@ -297,6 +297,13 @@ describe( 'BalloonToolbar', () => { sinon.assert.calledOnce( balloonAddSpy ); } ); + it( 'should not add #toolbarView to the #_balloon when the selection is collapsed', () => { + setData( model, 'b[]ar' ); + + balloonToolbar.show(); + sinon.assert.notCalled( balloonAddSpy ); + } ); + it( 'should not add #toolbarView to the #_balloon when all components inside #toolbarView are disabled', () => { Array.from( balloonToolbar.toolbarView.items ).forEach( item => { item.isEnabled = false; @@ -319,37 +326,6 @@ describe( 'BalloonToolbar', () => { balloonToolbar.show(); sinon.assert.calledOnce( balloonAddSpy ); } ); - - describe( 'on #_selectionChangeDebounced event', () => { - let showSpy; - - beforeEach( () => { - showSpy = sandbox.spy( balloonToolbar, 'show' ); - } ); - - it( 'should not be called when the editor is not focused', () => { - setData( model, 'b[a]r' ); - editingView.document.isFocused = false; - - balloonToolbar.fire( '_selectionChangeDebounced' ); - sinon.assert.notCalled( showSpy ); - } ); - - it( 'should not be called when the selection is collapsed', () => { - setData( model, 'b[]ar' ); - - balloonToolbar.fire( '_selectionChangeDebounced' ); - sinon.assert.notCalled( showSpy ); - } ); - - it( 'should be called when the selection is not collapsed and editor is focused', () => { - setData( model, 'b[a]r' ); - editingView.document.isFocused = true; - - balloonToolbar.fire( '_selectionChangeDebounced' ); - sinon.assert.calledOnce( showSpy ); - } ); - } ); } ); describe( 'hide()', () => { @@ -381,7 +357,7 @@ describe( 'BalloonToolbar', () => { sinon.assert.notCalled( spy ); } ); - it( 'should not remove #ttolbarView when is not added to the #_balloon', () => { + it( 'should not remove #toolbarView when is not added to the #_balloon', () => { balloonToolbar.hide(); sinon.assert.notCalled( removeBalloonSpy ); @@ -412,7 +388,7 @@ describe( 'BalloonToolbar', () => { } ); } ); - describe( 'showing and hiding', () => { + describe( 'show and hide triggers', () => { let showPanelSpy, hidePanelSpy; beforeEach( () => { @@ -422,7 +398,7 @@ describe( 'BalloonToolbar', () => { hidePanelSpy = sandbox.spy( balloonToolbar, 'hide' ); } ); - it( 'should open when selection stops changing', () => { + it( 'should show when selection stops changing', () => { sinon.assert.notCalled( showPanelSpy ); sinon.assert.notCalled( hidePanelSpy ); @@ -432,7 +408,18 @@ describe( 'BalloonToolbar', () => { sinon.assert.notCalled( hidePanelSpy ); } ); - it( 'should close when selection starts changing by a directChange', () => { + it( 'should not show when selection stops changing when editable is blurred', () => { + sinon.assert.notCalled( showPanelSpy ); + sinon.assert.notCalled( hidePanelSpy ); + + editingView.document.isFocused = false; + balloonToolbar.fire( '_selectionChangeDebounced' ); + + sinon.assert.notCalled( showPanelSpy ); + sinon.assert.notCalled( hidePanelSpy ); + } ); + + it( 'should hide when selection starts changing by a direct change', () => { balloonToolbar.fire( '_selectionChangeDebounced' ); sinon.assert.calledOnce( showPanelSpy ); @@ -444,7 +431,7 @@ describe( 'BalloonToolbar', () => { sinon.assert.calledOnce( hidePanelSpy ); } ); - it( 'should not close when selection starts changing by not a directChange', () => { + it( 'should not hide when selection starts changing by an indirect change', () => { balloonToolbar.fire( '_selectionChangeDebounced' ); sinon.assert.calledOnce( showPanelSpy ); @@ -456,7 +443,7 @@ describe( 'BalloonToolbar', () => { sinon.assert.notCalled( hidePanelSpy ); } ); - it( 'should close when selection starts changing by not a directChange but will become collapsed', () => { + it( 'should hide when selection starts changing by an indirect change but has changed to collapsed', () => { balloonToolbar.fire( '_selectionChangeDebounced' ); sinon.assert.calledOnce( showPanelSpy ); @@ -472,8 +459,8 @@ describe( 'BalloonToolbar', () => { sinon.assert.calledOnce( hidePanelSpy ); } ); - it( 'should hide if the editor loses focus', () => { - editor.ui.focusTracker.isFocused = true; + it( 'should hide on editable blur', () => { + editingView.document.isFocused = true; balloonToolbar.fire( '_selectionChangeDebounced' ); @@ -482,7 +469,7 @@ describe( 'BalloonToolbar', () => { sinon.assert.calledOnce( showPanelSpy ); sinon.assert.notCalled( hidePanelSpy ); - editor.ui.focusTracker.isFocused = false; + editingView.document.isFocused = false; sinon.assert.calledOnce( showPanelSpy ); sinon.assert.calledOnce( hidePanelSpy ); @@ -490,8 +477,27 @@ describe( 'BalloonToolbar', () => { stub.restore(); } ); - it( 'should not hide if the editor loses focus and #toolbarView is not visible', () => { - editor.ui.focusTracker.isFocused = true; + it( 'should not hide on editable blur when #toolbarView gets focus', () => { + editingView.document.isFocused = true; + + balloonToolbar.fire( '_selectionChangeDebounced' ); + + const stub = sandbox.stub( balloon, 'visibleView' ).get( () => balloonToolbar.toolbarView ); + + sinon.assert.calledOnce( showPanelSpy ); + sinon.assert.notCalled( hidePanelSpy ); + + balloonToolbar.toolbarView.focusTracker.isFocused = true; + editingView.document.isFocused = false; + + sinon.assert.calledOnce( showPanelSpy ); + sinon.assert.notCalled( hidePanelSpy ); + + stub.restore(); + } ); + + it( 'should not hide on editable blur when #toolbarView is not visible', () => { + editingView.document.isFocused = true; balloonToolbar.fire( '_selectionChangeDebounced' ); @@ -500,13 +506,25 @@ describe( 'BalloonToolbar', () => { sinon.assert.calledOnce( showPanelSpy ); sinon.assert.notCalled( hidePanelSpy ); - editor.ui.focusTracker.isFocused = false; + editingView.document.isFocused = false; sinon.assert.calledOnce( showPanelSpy ); sinon.assert.notCalled( hidePanelSpy ); stub.restore(); } ); + + it( 'should show when editable gets focus', () => { + editingView.document.isFocused = false; + + sinon.assert.notCalled( showPanelSpy ); + sinon.assert.notCalled( hidePanelSpy ); + + editingView.document.isFocused = true; + + sinon.assert.calledOnce( showPanelSpy ); + sinon.assert.notCalled( hidePanelSpy ); + } ); } ); describe( 'show event', () => {