Skip to content
This repository was archived by the owner on Jun 26, 2020. It is now read-only.

Commit d83d07d

Browse files
authored
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.
2 parents 111a728 + 18e1c6b commit d83d07d

File tree

5 files changed

+66
-40
lines changed

5 files changed

+66
-40
lines changed

src/toolbar/contextual/contextualtoolbar.js

Lines changed: 23 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,10 @@ export default class ContextualToolbar extends Plugin {
7979
// Attach lifecycle actions.
8080
this._handleSelectionChange();
8181
this._handleFocusChange();
82+
83+
// The appearance of the ContextualToolbar method is event–driven.
84+
// It is possible to stop the #show event and this prevent the toolbar from showing up.
85+
this.decorate( 'show' );
8286
}
8387

8488
/**
@@ -95,7 +99,7 @@ export default class ContextualToolbar extends Plugin {
9599
}
96100

97101
/**
98-
* Handles editor focus change and hides panel if it's needed.
102+
* Handles the editor focus change and hides the toolbar if it's needed.
99103
*
100104
* @private
101105
*/
@@ -146,41 +150,31 @@ export default class ContextualToolbar extends Plugin {
146150
/**
147151
* Shows the toolbar and attaches it to the selection.
148152
*
149-
* Fires {@link #event:beforeShow} event just before displaying the panel.
153+
* Fires {@link #event:show} event which can be stopped to prevent the toolbar from showing up.
150154
*/
151155
show() {
152-
const editingView = this.editor.editing.view;
153-
let isStopped = false;
154-
155-
// Do not add toolbar to the balloon stack twice.
156+
// Do not add the toolbar to the balloon stack twice.
156157
if ( this._balloon.hasView( this.toolbarView ) ) {
157158
return;
158159
}
159160

160-
// If `beforeShow` event is not stopped by any external code then panel will be displayed.
161-
this.once( 'beforeShow', () => {
162-
if ( isStopped ) {
163-
return;
164-
}
161+
// Don not show the toolbar when all components inside are disabled
162+
// see https://github.com/ckeditor/ckeditor5-ui/issues/269.
163+
if ( Array.from( this.toolbarView.items ).every( item => item.isEnabled !== undefined && !item.isEnabled ) ) {
164+
return;
165+
}
165166

166-
// Update panel position when selection changes while balloon will be opened
167-
// (by an external document changes).
168-
this.listenTo( editingView, 'render', () => {
169-
this._balloon.updatePosition( this._getBalloonPositionData() );
170-
} );
171-
172-
// Add panel to the common editor contextual balloon.
173-
this._balloon.add( {
174-
view: this.toolbarView,
175-
position: this._getBalloonPositionData(),
176-
balloonClassName: 'ck-toolbar-container ck-editor-toolbar-container'
177-
} );
167+
// Update the toolbar position upon #render (e.g. external document changes)
168+
// while it's visible.
169+
this.listenTo( this.editor.editing.view, 'render', () => {
170+
this._balloon.updatePosition( this._getBalloonPositionData() );
178171
} );
179172

180-
// Fire this event to inform that `ContextualToolbar` is going to be shown.
181-
// Helper function for preventing the panel from being displayed is passed along with the event.
182-
this.fire( 'beforeShow', () => {
183-
isStopped = true;
173+
// Add the toolbar to the common editor contextual balloon.
174+
this._balloon.add( {
175+
view: this.toolbarView,
176+
position: this._getBalloonPositionData(),
177+
balloonClassName: 'ck-toolbar-container ck-editor-toolbar-container'
184178
} );
185179
}
186180

@@ -234,12 +228,9 @@ export default class ContextualToolbar extends Plugin {
234228
}
235229

236230
/**
237-
* This event is fired just before the toolbar shows.
238-
* Using this event, an external code can prevent ContextualToolbar
239-
* from being displayed by calling a `stop` function which is passed along with this event.
231+
* This event is fired just before the toolbar shows up. Stopping this event will prevent this.
240232
*
241-
* @event beforeShow
242-
* @param {Function} stop Calling this function prevents panel from being displayed.
233+
* @event show
243234
*/
244235

245236
/**

tests/manual/contextualtoolbar/contextualtoolbar.html

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
<div id="editor">
2-
<p><i>This</i> is a <strong>first line</strong> of text.</p>
2+
<p><em>ContextualToolbar</em> won't show for the first block element.</p>
3+
<p><em>This</em> is a <strong>first line</strong> of text.</p>
34
<p><em>This</em> is a <strong>second line</strong> of text.</p>
45
<p><em>This</em> is the <strong>end</strong> of text.</p>
56
</div>

tests/manual/contextualtoolbar/contextualtoolbar.js

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import ClassicEditor from '@ckeditor/ckeditor5-editor-classic/src/classiceditor';
99
import ArticlePresets from '@ckeditor/ckeditor5-presets/src/article';
1010
import ContextualToolbar from '../../../src/toolbar/contextual/contextualtoolbar';
11+
import Range from '@ckeditor/ckeditor5-engine/src/model/range';
1112

1213
ClassicEditor.create( document.querySelector( '#editor' ), {
1314
plugins: [ ArticlePresets, ContextualToolbar ],
@@ -16,6 +17,17 @@ ClassicEditor.create( document.querySelector( '#editor' ), {
1617
} )
1718
.then( editor => {
1819
window.editor = editor;
20+
21+
const contextualToolbar = editor.plugins.get( 'ContextualToolbar' );
22+
23+
contextualToolbar.on( 'show', evt => {
24+
const selectionRange = editor.document.selection.getFirstRange();
25+
const blockRange = Range.createOn( editor.document.getRoot().getChild( 0 ) );
26+
27+
if ( selectionRange.containsRange( blockRange ) || selectionRange.isIntersecting( blockRange ) ) {
28+
evt.stop();
29+
}
30+
}, { priority: 'high' } );
1931
} )
2032
.catch( err => {
2133
console.error( err.stack );

tests/manual/contextualtoolbar/contextualtoolbar.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,3 +3,4 @@
33
1. Create a non–collapsed selection.
44
2. Create another non–collapsed selection but in another direction.
55
3. For each selection, a contextual toolbar should appear and the beginning/end of the selection, duplicating main editor toolbar.
6+
4. Toolbar should not display when selection is placed in the first block element.

tests/toolbar/contextual/contextualtoolbar.js

Lines changed: 28 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,29 @@ describe( 'ContextualToolbar', () => {
217217
sinon.assert.calledOnce( balloonAddSpy );
218218
} );
219219

220+
it( 'should not add #toolbarView to the #_balloon when all components inside #toolbarView are disabled', () => {
221+
Array.from( contextualToolbar.toolbarView.items ).forEach( item => {
222+
item.isEnabled = false;
223+
} );
224+
setData( editor.document, '<paragraph>b[a]r</paragraph>' );
225+
226+
contextualToolbar.show();
227+
sinon.assert.notCalled( balloonAddSpy );
228+
} );
229+
230+
it( 'should add #toolbarView to the #_balloon when at least one component inside does not have #isEnabled interface', () => {
231+
Array.from( contextualToolbar.toolbarView.items ).forEach( item => {
232+
item.isEnabled = false;
233+
} );
234+
235+
delete contextualToolbar.toolbarView.items.get( 0 ).isEnabled;
236+
237+
setData( editor.document, '<paragraph>b[a]r</paragraph>' );
238+
239+
contextualToolbar.show();
240+
sinon.assert.calledOnce( balloonAddSpy );
241+
} );
242+
220243
describe( 'on #_selectionChangeDebounced event', () => {
221244
let showSpy;
222245

@@ -408,25 +431,23 @@ describe( 'ContextualToolbar', () => {
408431
} );
409432
} );
410433

411-
describe( 'beforeShow event', () => {
412-
it( 'should fire `beforeShow` event just before panel shows', () => {
434+
describe( 'show event', () => {
435+
it( 'should fire `show` event just before panel shows', () => {
413436
const spy = sinon.spy();
414437

415-
contextualToolbar.on( 'beforeShow', spy );
438+
contextualToolbar.on( 'show', spy );
416439
setData( editor.document, '<paragraph>b[a]r</paragraph>' );
417440

418441
contextualToolbar.show();
419442
sinon.assert.calledOnce( spy );
420443
} );
421444

422-
it( 'should not show the panel when `beforeShow` event is stopped', () => {
445+
it( 'should not show the panel when `show` event is stopped', () => {
423446
const balloonAddSpy = sandbox.spy( balloon, 'add' );
424447

425448
setData( editor.document, '<paragraph>b[a]r</paragraph>' );
426449

427-
contextualToolbar.on( 'beforeShow', ( evt, stop ) => {
428-
stop();
429-
} );
450+
contextualToolbar.on( 'show', evt => evt.stop(), { priority: 'high' } );
430451

431452
contextualToolbar.show();
432453
sinon.assert.notCalled( balloonAddSpy );

0 commit comments

Comments
 (0)