From 5a12de0ecbcd121dc09f0e715dec1391ce2c2488 Mon Sep 17 00:00:00 2001 From: Kamil Piechaczek Date: Tue, 9 Jun 2020 12:33:33 +0200 Subject: [PATCH 1/2] Do not display Widget Toolbar if its child collection is empty. --- .../tests/_utils-tests/articlepluginset.js | 7 ++++++- packages/ckeditor5-font/tests/integration.js | 5 ++++- .../ckeditor5-image/tests/imagetoolbar.js | 5 ++++- packages/ckeditor5-image/tests/integration.js | 6 +++++- .../tests/mediaembedtoolbar.js | 2 +- .../src/widgettoolbarrepository.js | 19 ++++++++++++++++++- .../ckeditor5-widget/tests/widgetresize.js | 5 ++++- .../tests/widgetresize/resizer.js | 5 ++++- .../tests/widgettoolbarrepository.js | 16 +++++++++++++++- .../widgettypearound/widgettypearound.js | 5 ++++- 10 files changed, 65 insertions(+), 10 deletions(-) diff --git a/packages/ckeditor5-core/tests/_utils-tests/articlepluginset.js b/packages/ckeditor5-core/tests/_utils-tests/articlepluginset.js index fb84874db0f..4999172a1f8 100644 --- a/packages/ckeditor5-core/tests/_utils-tests/articlepluginset.js +++ b/packages/ckeditor5-core/tests/_utils-tests/articlepluginset.js @@ -33,7 +33,12 @@ describe( 'ArticlePluginSet', () => { editorElement = document.createElement( 'div' ); document.body.appendChild( editorElement ); - editor = await ClassicTestEditor.create( editorElement, { plugins: [ ArticlePluginSet ] } ); + editor = await ClassicTestEditor.create( editorElement, { + plugins: [ ArticlePluginSet ], + image: { + toolbar: [ 'imageStyle:full', 'imageStyle:side' ] + } + } ); } ); afterEach( async () => { diff --git a/packages/ckeditor5-font/tests/integration.js b/packages/ckeditor5-font/tests/integration.js index 6230c9e6dde..774243e830a 100644 --- a/packages/ckeditor5-font/tests/integration.js +++ b/packages/ckeditor5-font/tests/integration.js @@ -19,7 +19,10 @@ describe( 'Integration test Font', () => { return ClassicTestEditor .create( element, { - plugins: [ Font, ArticlePluginSet ] + plugins: [ Font, ArticlePluginSet ], + image: { + toolbar: [ 'imageStyle:full', 'imageStyle:side' ] + } } ) .then( newEditor => { editor = newEditor; diff --git a/packages/ckeditor5-image/tests/imagetoolbar.js b/packages/ckeditor5-image/tests/imagetoolbar.js index 54209000b4c..d787e321fba 100644 --- a/packages/ckeditor5-image/tests/imagetoolbar.js +++ b/packages/ckeditor5-image/tests/imagetoolbar.js @@ -3,7 +3,7 @@ * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license */ -/* global document */ +/* global document, console */ import ClassicEditor from '@ckeditor/ckeditor5-editor-classic/src/classiceditor'; import ImageToolbar from '../src/imagetoolbar'; @@ -53,6 +53,7 @@ describe( 'ImageToolbar', () => { } ); it( 'should not initialize if there is no configuration', () => { + const consoleWarnStub = sinon.stub( console, 'warn' ); const editorElement = global.document.createElement( 'div' ); global.document.body.appendChild( editorElement ); @@ -61,6 +62,8 @@ describe( 'ImageToolbar', () => { } ) .then( editor => { expect( editor.plugins.get( ImageToolbar )._toolbar ).to.be.undefined; + expect( consoleWarnStub.calledOnce ).to.equal( true ); + expect( consoleWarnStub.firstCall.args[ 0 ] ).to.match( /^widget-toolbar-no-items:/ ); editorElement.remove(); return editor.destroy(); diff --git a/packages/ckeditor5-image/tests/integration.js b/packages/ckeditor5-image/tests/integration.js index 6c5825d4139..61be0b788be 100644 --- a/packages/ckeditor5-image/tests/integration.js +++ b/packages/ckeditor5-image/tests/integration.js @@ -12,6 +12,7 @@ import Image from '../src/image'; import ImageToolbar from '../src/imagetoolbar'; import View from '@ckeditor/ckeditor5-ui/src/view'; import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils'; +import ImageTextAlternative from '../src/imagetextalternative'; describe( 'ImageToolbar integration', () => { describe( 'with the BalloonToolbar', () => { @@ -25,7 +26,10 @@ describe( 'ImageToolbar integration', () => { return ClassicTestEditor .create( editorElement, { - plugins: [ Image, ImageToolbar, BalloonToolbar, Paragraph ] + plugins: [ Image, ImageTextAlternative, ImageToolbar, BalloonToolbar, Paragraph ], + image: { + toolbar: [ 'imageTextAlternative' ] + } } ) .then( editor => { newEditor = editor; diff --git a/packages/ckeditor5-media-embed/tests/mediaembedtoolbar.js b/packages/ckeditor5-media-embed/tests/mediaembedtoolbar.js index 532db5e9618..4e5204d6447 100644 --- a/packages/ckeditor5-media-embed/tests/mediaembedtoolbar.js +++ b/packages/ckeditor5-media-embed/tests/mediaembedtoolbar.js @@ -186,7 +186,7 @@ describe( 'MediaEmbedToolbar - integration with BalloonEditor', () => { return BalloonEditor.create( element, { plugins: [ Paragraph, MediaEmbed, MediaEmbedToolbar, FakeButton, Bold ], balloonToolbar: [ 'bold' ], - media: { + mediaEmbed: { toolbar: [ 'fake_button' ] } } ).then( _editor => { diff --git a/packages/ckeditor5-widget/src/widgettoolbarrepository.js b/packages/ckeditor5-widget/src/widgettoolbarrepository.js index 60f673f00c3..68816b71e5b 100644 --- a/packages/ckeditor5-widget/src/widgettoolbarrepository.js +++ b/packages/ckeditor5-widget/src/widgettoolbarrepository.js @@ -7,6 +7,8 @@ * @module widget/widgettoolbarrepository */ +/* global console */ + import Plugin from '@ckeditor/ckeditor5-core/src/plugin'; import ContextualBalloon from '@ckeditor/ckeditor5-ui/src/panel/balloon/contextualballoon'; import ToolbarView from '@ckeditor/ckeditor5-ui/src/toolbar/toolbarview'; @@ -15,7 +17,7 @@ import { isWidget, centeredBalloonPositionForLongWidgets } from './utils'; -import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror'; +import CKEditorError, { attachLinkToDocumentation } from '@ckeditor/ckeditor5-utils/src/ckeditorerror'; /** * Widget toolbar repository plugin. A central point for registering widget toolbars. This plugin handles the whole @@ -124,6 +126,21 @@ export default class WidgetToolbarRepository extends Plugin { * @param {String} [options.balloonClassName='ck-toolbar-container'] CSS class for the widget balloon. */ register( toolbarId, { ariaLabel, items, getRelatedElement, balloonClassName = 'ck-toolbar-container' } ) { + // Trying to register a toolbar without any item. + if ( !items.length ) { + /** + * When {@link #register} a new toolbar, you need to provide a non-empty array with + * the items that will be inserted into the toolbar. + * + * @error widget-toolbar-no-items + */ + console.warn( + attachLinkToDocumentation( 'widget-toolbar-no-items: Trying to register a toolbar without items.' ), { toolbarId } + ); + + return; + } + const editor = this.editor; const t = editor.t; const toolbarView = new ToolbarView( editor.locale ); diff --git a/packages/ckeditor5-widget/tests/widgetresize.js b/packages/ckeditor5-widget/tests/widgetresize.js index 3379c1ae9d6..cf039a8bb12 100644 --- a/packages/ckeditor5-widget/tests/widgetresize.js +++ b/packages/ckeditor5-widget/tests/widgetresize.js @@ -561,7 +561,10 @@ describe( 'WidgetResize', () => { .create( element, { plugins: [ ArticlePluginSet, WidgetResize, simpleWidgetPlugin - ] + ], + image: { + toolbar: [ 'imageStyle:full', 'imageStyle:side' ] + } } ); } diff --git a/packages/ckeditor5-widget/tests/widgetresize/resizer.js b/packages/ckeditor5-widget/tests/widgetresize/resizer.js index bc4ca65147f..37fe124aaff 100644 --- a/packages/ckeditor5-widget/tests/widgetresize/resizer.js +++ b/packages/ckeditor5-widget/tests/widgetresize/resizer.js @@ -24,7 +24,10 @@ describe( 'Resizer', () => { .create( editorElement, { plugins: [ ArticlePluginSet - ] + ], + image: { + toolbar: [ 'imageStyle:full', 'imageStyle:side' ] + } } ) .then( newEditor => { editor = newEditor; diff --git a/packages/ckeditor5-widget/tests/widgettoolbarrepository.js b/packages/ckeditor5-widget/tests/widgettoolbarrepository.js index 4cad02baeb8..dc9628c7eb8 100644 --- a/packages/ckeditor5-widget/tests/widgettoolbarrepository.js +++ b/packages/ckeditor5-widget/tests/widgettoolbarrepository.js @@ -3,7 +3,7 @@ * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license */ -/* global document */ +/* global document, console */ import ClassicTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/classictesteditor'; import BalloonEditor from '@ckeditor/ckeditor5-editor-balloon/src/ballooneditor'; @@ -126,6 +126,20 @@ describe( 'WidgetToolbarRepository', () => { toolbarView.destroy(); } ); + + it( 'should not register a toolbar when passed an empty collection of the items', () => { + const consoleWarnStub = sinon.stub( console, 'warn' ); + + widgetToolbarRepository.register( 'fake', { + items: [], + getRelatedElement: () => null + } ); + + expect( widgetToolbarRepository._toolbarDefinitions.get( 'fake' ) ).to.be.undefined; + + expect( consoleWarnStub.calledOnce ).to.equal( true ); + expect( consoleWarnStub.firstCall.args[ 0 ] ).to.match( /^widget-toolbar-no-items:/ ); + } ); } ); describe( 'integration tests', () => { diff --git a/packages/ckeditor5-widget/tests/widgettypearound/widgettypearound.js b/packages/ckeditor5-widget/tests/widgettypearound/widgettypearound.js index 84d52070cad..06a50eac5b7 100644 --- a/packages/ckeditor5-widget/tests/widgettypearound/widgettypearound.js +++ b/packages/ckeditor5-widget/tests/widgettypearound/widgettypearound.js @@ -28,7 +28,10 @@ describe( 'WidgetTypeAround', () => { ArticlePluginSet, Widget, blockWidgetPlugin, inlineWidgetPlugin - ] + ], + image: { + toolbar: [ 'imageStyle:full', 'imageStyle:side' ] + } } ); editingView = editor.editing.view; From c1e4a23d174563799d857c8b875151abf6ad04f4 Mon Sep 17 00:00:00 2001 From: Kamil Piechaczek Date: Tue, 9 Jun 2020 14:55:29 +0200 Subject: [PATCH 2/2] Fixed CI errors. --- packages/ckeditor5-font/tests/integration.js | 6 ++++++ .../tests/mediaembedtoolbar.js | 20 ++++++++++++++++++- 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/packages/ckeditor5-font/tests/integration.js b/packages/ckeditor5-font/tests/integration.js index 774243e830a..4ed95b67789 100644 --- a/packages/ckeditor5-font/tests/integration.js +++ b/packages/ckeditor5-font/tests/integration.js @@ -69,6 +69,9 @@ describe( 'Integration test Font', () => { fontSize: { options: [ 10, 12, 14 ], supportAllValues: true + }, + image: { + toolbar: [ 'imageStyle:full', 'imageStyle:side' ] } } ) .then( editor => { @@ -132,6 +135,9 @@ describe( 'Integration test Font', () => { fontSize: { options: [ 10, 12, 14 ], supportAllValues: true + }, + image: { + toolbar: [ 'imageStyle:full', 'imageStyle:side' ] } } ) .then( editor => { diff --git a/packages/ckeditor5-media-embed/tests/mediaembedtoolbar.js b/packages/ckeditor5-media-embed/tests/mediaembedtoolbar.js index 4e5204d6447..1bef6159c8f 100644 --- a/packages/ckeditor5-media-embed/tests/mediaembedtoolbar.js +++ b/packages/ckeditor5-media-embed/tests/mediaembedtoolbar.js @@ -3,7 +3,7 @@ * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license */ -/* global document */ +/* global document, console */ import ClassicTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/classictesteditor'; import BalloonEditor from '@ckeditor/ckeditor5-editor-balloon/src/ballooneditor'; @@ -239,6 +239,24 @@ describe( 'MediaEmbedToolbar - integration with BalloonEditor', () => { expect( balloon.visibleView ).to.equal( balloonToolbar.toolbarView ); } ); + + it( 'does not create the toolbar if its items are not specified', () => { + const consoleWarnStub = sinon.stub( console, 'warn' ); + const element = document.createElement( 'div' ); + + return BalloonEditor.create( element, { + plugins: [ Paragraph, MediaEmbed, MediaEmbedToolbar, Bold ] + } ).then( editor => { + widgetToolbarRepository = editor.plugins.get( 'WidgetToolbarRepository' ); + + expect( widgetToolbarRepository._toolbarDefinitions.get( 'mediaEmbed' ) ).to.be.undefined; + expect( consoleWarnStub.calledOnce ).to.equal( true ); + expect( consoleWarnStub.firstCall.args[ 0 ] ).to.match( /^widget-toolbar-no-items:/ ); + + element.remove(); + return editor.destroy(); + } ); + } ); } ); // Plugin that adds fake_button to editor's component factory.