Преглед изворни кода

Merge pull request #276 from ckeditor/t/269

Other: The `ContextualToolbar` should not show up when all child items are disabled. The `ContextualToolbar#beforeShow` event has been replaced by `ContextualToolbar#show`. Closes #269. Closes #232.

BREAKING CHANGE: `ContextualToolbar#beforeShow` is no longer available. Please refer to `ContextualToolbar#show` instead.
Aleksander Nowodzinski пре 8 година
родитељ
комит
1b7f3c4611

+ 23 - 32
packages/ckeditor5-ui/src/toolbar/contextual/contextualtoolbar.js

@@ -79,6 +79,10 @@ export default class ContextualToolbar extends Plugin {
 		// Attach lifecycle actions.
 		this._handleSelectionChange();
 		this._handleFocusChange();
+
+		// The appearance of the ContextualToolbar method is event–driven.
+		// It is possible to stop the #show event and this prevent the toolbar from showing up.
+		this.decorate( 'show' );
 	}
 
 	/**
@@ -95,7 +99,7 @@ export default class ContextualToolbar extends Plugin {
 	}
 
 	/**
-	 * Handles editor focus change and hides panel if it's needed.
+	 * Handles the editor focus change and hides the toolbar if it's needed.
 	 *
 	 * @private
 	 */
@@ -146,41 +150,31 @@ export default class ContextualToolbar extends Plugin {
 	/**
 	 * Shows the toolbar and attaches it to the selection.
 	 *
-	 * Fires {@link #event:beforeShow} event just before displaying the panel.
+	 * Fires {@link #event:show} event which can be stopped to prevent the toolbar from showing up.
 	 */
 	show() {
-		const editingView = this.editor.editing.view;
-		let isStopped = false;
-
-		// Do not add toolbar to the balloon stack twice.
+		// Do not add the toolbar to the balloon stack twice.
 		if ( this._balloon.hasView( this.toolbarView ) ) {
 			return;
 		}
 
-		// If `beforeShow` event is not stopped by any external code then panel will be displayed.
-		this.once( 'beforeShow', () => {
-			if ( isStopped ) {
-				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 ) ) {
+			return;
+		}
 
-			// Update panel position when selection changes while balloon will be opened
-			// (by an external document changes).
-			this.listenTo( editingView, 'render', () => {
-				this._balloon.updatePosition( this._getBalloonPositionData() );
-			} );
-
-			// Add panel to the common editor contextual balloon.
-			this._balloon.add( {
-				view: this.toolbarView,
-				position: this._getBalloonPositionData(),
-				balloonClassName: 'ck-toolbar-container ck-editor-toolbar-container'
-			} );
+		// Update the toolbar position upon #render (e.g. external document changes)
+		// while it's visible.
+		this.listenTo( this.editor.editing.view, 'render', () => {
+			this._balloon.updatePosition( this._getBalloonPositionData() );
 		} );
 
-		// Fire this event to inform that `ContextualToolbar` is going to be shown.
-		// Helper function for preventing the panel from being displayed is passed along with the event.
-		this.fire( 'beforeShow', () => {
-			isStopped = true;
+		// Add the toolbar to the common editor contextual balloon.
+		this._balloon.add( {
+			view: this.toolbarView,
+			position: this._getBalloonPositionData(),
+			balloonClassName: 'ck-toolbar-container ck-editor-toolbar-container'
 		} );
 	}
 
@@ -234,12 +228,9 @@ export default class ContextualToolbar extends Plugin {
 	}
 
 	/**
-	 * This event is fired just before the toolbar shows.
-	 * Using this event, an external code can prevent ContextualToolbar
-	 * from being displayed by calling a `stop` function which is passed along with this event.
+	 * This event is fired just before the toolbar shows up. Stopping this event will prevent this.
 	 *
-	 * @event beforeShow
-	 * @param {Function} stop Calling this function prevents panel from being displayed.
+	 * @event show
 	 */
 
 	/**

+ 2 - 1
packages/ckeditor5-ui/tests/manual/contextualtoolbar/contextualtoolbar.html

@@ -1,5 +1,6 @@
 <div id="editor">
-	<p><i>This</i> is a <strong>first line</strong> of text.</p>
+	<p><em>ContextualToolbar</em> won't show for the first block element.</p>
+	<p><em>This</em> is a <strong>first line</strong> of text.</p>
 	<p><em>This</em> is a <strong>second line</strong> of text.</p>
 	<p><em>This</em> is the <strong>end</strong> of text.</p>
 </div>

+ 12 - 0
packages/ckeditor5-ui/tests/manual/contextualtoolbar/contextualtoolbar.js

@@ -8,6 +8,7 @@
 import ClassicEditor from '@ckeditor/ckeditor5-editor-classic/src/classiceditor';
 import ArticlePresets from '@ckeditor/ckeditor5-presets/src/article';
 import ContextualToolbar from '../../../src/toolbar/contextual/contextualtoolbar';
+import Range from '@ckeditor/ckeditor5-engine/src/model/range';
 
 ClassicEditor.create( document.querySelector( '#editor' ), {
 	plugins: [ ArticlePresets, ContextualToolbar ],
@@ -16,6 +17,17 @@ ClassicEditor.create( document.querySelector( '#editor' ), {
 } )
 .then( editor => {
 	window.editor = editor;
+
+	const contextualToolbar = editor.plugins.get( 'ContextualToolbar' );
+
+	contextualToolbar.on( 'show', evt => {
+		const selectionRange = editor.document.selection.getFirstRange();
+		const blockRange = Range.createOn( editor.document.getRoot().getChild( 0 ) );
+
+		if ( selectionRange.containsRange( blockRange ) || selectionRange.isIntersecting( blockRange ) ) {
+			evt.stop();
+		}
+	}, { priority: 'high' } );
 } )
 .catch( err => {
 	console.error( err.stack );

+ 1 - 0
packages/ckeditor5-ui/tests/manual/contextualtoolbar/contextualtoolbar.md

@@ -3,3 +3,4 @@
 1. Create a non–collapsed selection.
 2. Create another non–collapsed selection but in another direction.
 3. For each selection, a contextual toolbar should appear and the beginning/end of the selection, duplicating main editor toolbar.
+4. Toolbar should not display when selection is placed in the first block element.

+ 28 - 7
packages/ckeditor5-ui/tests/toolbar/contextual/contextualtoolbar.js

@@ -217,6 +217,29 @@ describe( 'ContextualToolbar', () => {
 			sinon.assert.calledOnce( balloonAddSpy );
 		} );
 
+		it( 'should not add #toolbarView to the #_balloon when all components inside #toolbarView are disabled', () => {
+			Array.from( contextualToolbar.toolbarView.items ).forEach( item => {
+				item.isEnabled = false;
+			} );
+			setData( editor.document, '<paragraph>b[a]r</paragraph>' );
+
+			contextualToolbar.show();
+			sinon.assert.notCalled( balloonAddSpy );
+		} );
+
+		it( 'should add #toolbarView to the #_balloon when at least one component inside does not have #isEnabled interface', () => {
+			Array.from( contextualToolbar.toolbarView.items ).forEach( item => {
+				item.isEnabled = false;
+			} );
+
+			delete contextualToolbar.toolbarView.items.get( 0 ).isEnabled;
+
+			setData( editor.document, '<paragraph>b[a]r</paragraph>' );
+
+			contextualToolbar.show();
+			sinon.assert.calledOnce( balloonAddSpy );
+		} );
+
 		describe( 'on #_selectionChangeDebounced event', () => {
 			let showSpy;
 
@@ -408,25 +431,23 @@ describe( 'ContextualToolbar', () => {
 		} );
 	} );
 
-	describe( 'beforeShow event', () => {
-		it( 'should fire `beforeShow` event just before panel shows', () => {
+	describe( 'show event', () => {
+		it( 'should fire `show` event just before panel shows', () => {
 			const spy = sinon.spy();
 
-			contextualToolbar.on( 'beforeShow', spy );
+			contextualToolbar.on( 'show', spy );
 			setData( editor.document, '<paragraph>b[a]r</paragraph>' );
 
 			contextualToolbar.show();
 			sinon.assert.calledOnce( spy );
 		} );
 
-		it( 'should not show the panel when `beforeShow` event is stopped', () => {
+		it( 'should not show the panel when `show` event is stopped', () => {
 			const balloonAddSpy = sandbox.spy( balloon, 'add' );
 
 			setData( editor.document, '<paragraph>b[a]r</paragraph>' );
 
-			contextualToolbar.on( 'beforeShow', ( evt, stop ) => {
-				stop();
-			} );
+			contextualToolbar.on( 'show', evt => evt.stop(), { priority: 'high' } );
 
 			contextualToolbar.show();
 			sinon.assert.notCalled( balloonAddSpy );