From 273a5cff3dfdda81c190564b925b40729c936bfc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Wed, 10 May 2017 20:42:58 +0200 Subject: [PATCH 1/5] Fixed double image toolbar when ContextualToolbar plugin is loaded. --- src/image.js | 21 +++++++++ src/imagetextalternative.js | 2 +- .../ui/textalternativeformview.js | 2 +- tests/image.js | 45 ++++++++++++++++++- tests/manual/tickets/110/1.html | 8 ++++ tests/manual/tickets/110/1.js | 30 +++++++++++++ tests/manual/tickets/110/1.md | 6 +++ 7 files changed, 110 insertions(+), 4 deletions(-) create mode 100644 tests/manual/tickets/110/1.html create mode 100644 tests/manual/tickets/110/1.js create mode 100644 tests/manual/tickets/110/1.md diff --git a/src/image.js b/src/image.js index 777af10a..5c8b81c9 100644 --- a/src/image.js +++ b/src/image.js @@ -11,6 +11,7 @@ import Plugin from '@ckeditor/ckeditor5-core/src/plugin'; import ImageEngine from './image/imageengine'; import Widget from '@ckeditor/ckeditor5-widget/src/widget'; import ImageTextAlternative from './imagetextalternative'; +import { isImageWidget } from './image/utils'; import '../theme/theme.scss'; @@ -35,4 +36,24 @@ export default class Image extends Plugin { static get pluginName() { return 'image/image'; } + + /** + * @inheritDoc + */ + init() { + const editor = this.editor; + const contextualToolbar = editor.plugins.get( 'ui/contextualtoolbar' ); + + // If `ContextualToolbar` plugin is loaded we need to disable it for `Image` + // because `Image` has its own toolbar. See: ckeditor/ckeditor5-image#110. + if ( contextualToolbar ) { + this.listenTo( contextualToolbar, 'beforeShow', ( evt, stop ) => { + const selectedElement = editor.editing.view.selection.getSelectedElement(); + + if ( selectedElement && isImageWidget( selectedElement ) ) { + stop(); + } + } ); + } + } } diff --git a/src/imagetextalternative.js b/src/imagetextalternative.js index 0773e259..2605efd4 100644 --- a/src/imagetextalternative.js +++ b/src/imagetextalternative.js @@ -49,7 +49,7 @@ export default class ImageTextAlternative extends Plugin { /** * Balloon panel containing text alternative change form. * - * @member {module:image/ui/imageballoonpanel~ImageBalloonPanelView} #baloonPanel + * @member {module:image/image/ui/imageballoonpanel~ImageBalloonPanelView} #baloonPanel */ this.balloonPanel = panel; diff --git a/src/imagetextalternative/ui/textalternativeformview.js b/src/imagetextalternative/ui/textalternativeformview.js index 7def38bc..7e965430 100644 --- a/src/imagetextalternative/ui/textalternativeformview.js +++ b/src/imagetextalternative/ui/textalternativeformview.js @@ -4,7 +4,7 @@ */ /** - * @module image/imagetextalternative/ui/imagetextalternativeformview + * @module image/imagetextalternative/ui/textalternativeformview */ import View from '@ckeditor/ckeditor5-ui/src/view'; diff --git a/tests/image.js b/tests/image.js index ff5f01e5..6dfd78c8 100644 --- a/tests/image.js +++ b/tests/image.js @@ -13,12 +13,14 @@ import ImageTextAlternative from '../src/imagetextalternative'; import { setData as setModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model'; import { getData as getViewData } from '@ckeditor/ckeditor5-engine/src/dev-utils/view'; import ModelRange from '@ckeditor/ckeditor5-engine/src/model/range'; +import ContextualToolbar from '@ckeditor/ckeditor5-ui/src/toolbar/contextual/contextualtoolbar'; +import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph'; describe( 'Image', () => { - let editor, document, viewDocument; + let editorElement, editor, document, viewDocument; beforeEach( () => { - const editorElement = window.document.createElement( 'div' ); + editorElement = window.document.createElement( 'div' ); window.document.body.appendChild( editorElement ); return ClassicTestEditor.create( editorElement, { @@ -31,6 +33,12 @@ describe( 'Image', () => { } ); } ); + afterEach( () => { + editorElement.remove(); + + return editor.destroy(); + } ); + it( 'should be loaded', () => { expect( editor.plugins.get( Image ) ).to.instanceOf( Image ); } ); @@ -47,6 +55,39 @@ describe( 'Image', () => { expect( editor.plugins.get( ImageTextAlternative ) ).to.instanceOf( ImageTextAlternative ); } ); + it( 'should prevent ContextualToolbar of being displayed when Image is selected', () => { + return ClassicTestEditor.create( editorElement, { + plugins: [ Image, ContextualToolbar, Paragraph ] + } ) + .then( newEditor => { + const balloon = newEditor.plugins.get( 'ui/contextualballoon' ); + const contextualToolbar = newEditor.plugins.get( 'ui/contextualtoolbar' ); + + newEditor.editing.view.isFocused = true; + + // When image is selected along with text. + setModelData( newEditor.document, 'fo[oalt text]' ); + + contextualToolbar._showPanel(); + + // ContextualToolbar should be visible. + expect( balloon.visibleView ).to.equal( contextualToolbar.toolbarView ); + + // When only image is selected. + setModelData( newEditor.document, 'foo[alt text]' ); + + contextualToolbar._showPanel(); + + // ContextualToolbar should not be visible. + expect( balloon.visibleView ).to.null; + + // Cleaning up. + editorElement.remove(); + + return newEditor.destroy(); + } ); + } ); + describe( 'selection', () => { it( 'should create fake selection', () => { setModelData( document, '[alt text]' ); diff --git a/tests/manual/tickets/110/1.html b/tests/manual/tickets/110/1.html new file mode 100644 index 00000000..eedc75a9 --- /dev/null +++ b/tests/manual/tickets/110/1.html @@ -0,0 +1,8 @@ +
+

Lorem ipsum dolor sit amet, consectetur adipiscing elit. Nulla finibus consequat placerat. Vestibulum id tellus et mauris sagittis tincidunt quis id mauris. Curabitur consectetur lectus sit amet tellus mattis, non lobortis leo interdum.

+
+ CKEditor logo +
+

Lorem ipsum dolor sit amet, consectetur adipiscing elit. Nulla finibus consequat placerat. Vestibulum id tellus et mauris sagittis tincidunt quis id mauris. Curabitur consectetur lectus sit amet tellus mattis, non lobortis leo interdum. Lorem ipsum dolor sit amet, consectetur adipiscing elit. Nulla finibus consequat placerat. Vestibulum id tellus et mauris sagittis tincidunt quis id mauris. Curabitur consectetur lectus sit amet tellus mattis, non lobortis leo interdum. Lorem ipsum dolor sit amet, consectetur adipiscing elit. Nulla finibus consequat placerat. Vestibulum id tellus et mauris sagittis tincidunt quis id mauris. Curabitur consectetur lectus sit amet tellus mattis, non lobortis leo interdum. Lorem ipsum dolor sit amet, consectetur adipiscing elit. Nulla finibus consequat placerat. Vestibulum id tellus et mauris sagittis tincidunt quis id mauris. Curabitur consectetur lectus sit amet tellus mattis, non lobortis leo interdum.

+

Lorem ipsum dolor sit amet, consectetur adipiscing elit. Nulla finibus consequat placerat. Vestibulum id tellus et mauris sagittis tincidunt quis id mauris. Curabitur consectetur lectus sit amet tellus mattis, non lobortis leo interdum.

+
diff --git a/tests/manual/tickets/110/1.js b/tests/manual/tickets/110/1.js new file mode 100644 index 00000000..ee957d15 --- /dev/null +++ b/tests/manual/tickets/110/1.js @@ -0,0 +1,30 @@ +/** + * @license Copyright (c) 2003-2017, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md. + */ + +/* global document, console, window */ + +import ClassicEditor from '@ckeditor/ckeditor5-editor-classic/src/classic'; +import EnterPlugin from '@ckeditor/ckeditor5-enter/src/enter'; +import TypingPlugin from '@ckeditor/ckeditor5-typing/src/typing'; +import ParagraphPlugin from '@ckeditor/ckeditor5-paragraph/src/paragraph'; +import HeadingPlugin from '@ckeditor/ckeditor5-heading/src/heading'; +import ImagePlugin from '../../../../src/image'; +import UndoPlugin from '@ckeditor/ckeditor5-undo/src/undo'; +import ContextualToolbar from '@ckeditor/ckeditor5-ui/src/toolbar/contextual/contextualtoolbar'; +import ImageToolbar from '../../../../src/imagetoolbar'; + +ClassicEditor.create( document.querySelector( '#editor' ), { + plugins: [ EnterPlugin, TypingPlugin, ParagraphPlugin, HeadingPlugin, ImagePlugin, UndoPlugin, ImageToolbar, ContextualToolbar ], + contextualToolbar: [ 'headings', 'undo', 'redo' ], + image: { + toolbar: [ 'imageTextAlternative' ] + } +} ) +.then( editor => { + window.editor = editor; +} ) +.catch( err => { + console.error( err.stack ); +} ); diff --git a/tests/manual/tickets/110/1.md b/tests/manual/tickets/110/1.md new file mode 100644 index 00000000..340c9578 --- /dev/null +++ b/tests/manual/tickets/110/1.md @@ -0,0 +1,6 @@ +### Double image toolbar when ContextualToolbar plugin is loaded [#110](https://github.com/ckeditor/ckeditor5-image/issues/110) + +Click on image: +- balloon toolbar should appear, +- ContextualToolbar should not appear. + From 6f3fda2e27a6a514d58f5a3db285c75478f908f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Wed, 10 May 2017 20:45:55 +0200 Subject: [PATCH 2/5] Removed empty line at the end of the file. --- tests/manual/tickets/110/1.md | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/manual/tickets/110/1.md b/tests/manual/tickets/110/1.md index 340c9578..6faba04b 100644 --- a/tests/manual/tickets/110/1.md +++ b/tests/manual/tickets/110/1.md @@ -3,4 +3,3 @@ Click on image: - balloon toolbar should appear, - ContextualToolbar should not appear. - From bce60ac37c65cb84b53d21c1082f65201e48dad1 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Thu, 11 May 2017 15:43:40 +0200 Subject: [PATCH 3/5] Docs: Imprved docs in the Image class. --- src/image.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/image.js b/src/image.js index 5c8b81c9..a5d15035 100644 --- a/src/image.js +++ b/src/image.js @@ -44,8 +44,9 @@ export default class Image extends Plugin { const editor = this.editor; const contextualToolbar = editor.plugins.get( 'ui/contextualtoolbar' ); - // If `ContextualToolbar` plugin is loaded we need to disable it for `Image` - // because `Image` has its own toolbar. See: ckeditor/ckeditor5-image#110. + // If `ContextualToolbar` plugin is loaded, it should be disabled for images + // which have their own toolbar to avoid duplication. + // https://github.com/ckeditor/ckeditor5-image/issues/110 if ( contextualToolbar ) { this.listenTo( contextualToolbar, 'beforeShow', ( evt, stop ) => { const selectedElement = editor.editing.view.selection.getSelectedElement(); From 51f3a152b60b23c07b6ef74a64db9b7a44a99379 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Thu, 11 May 2017 15:44:09 +0200 Subject: [PATCH 4/5] Tests: Secured Image test by using Promises. --- tests/image.js | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/tests/image.js b/tests/image.js index 6dfd78c8..52eb6ec2 100644 --- a/tests/image.js +++ b/tests/image.js @@ -55,7 +55,7 @@ describe( 'Image', () => { expect( editor.plugins.get( ImageTextAlternative ) ).to.instanceOf( ImageTextAlternative ); } ); - it( 'should prevent ContextualToolbar of being displayed when Image is selected', () => { + it( 'should prevent the ContextualToolbar from being displayed when an image is selected', () => { return ClassicTestEditor.create( editorElement, { plugins: [ Image, ContextualToolbar, Paragraph ] } ) @@ -68,23 +68,25 @@ describe( 'Image', () => { // When image is selected along with text. setModelData( newEditor.document, 'fo[oalt text]' ); - contextualToolbar._showPanel(); + return contextualToolbar._showPanel() + .then( () => { + // ContextualToolbar should be visible. + expect( balloon.visibleView ).to.equal( contextualToolbar.toolbarView ); - // ContextualToolbar should be visible. - expect( balloon.visibleView ).to.equal( contextualToolbar.toolbarView ); + // When only image is selected. + setModelData( newEditor.document, 'foo[alt text]' ); - // When only image is selected. - setModelData( newEditor.document, 'foo[alt text]' ); + return contextualToolbar._showPanel() + .then( () => { + // ContextualToolbar should not be visible. + expect( balloon.visibleView ).to.be.null; - contextualToolbar._showPanel(); + // Cleaning up. + editorElement.remove(); - // ContextualToolbar should not be visible. - expect( balloon.visibleView ).to.null; - - // Cleaning up. - editorElement.remove(); - - return newEditor.destroy(); + return newEditor.destroy(); + } ); + } ); } ); } ); From da334c8bf4ab7759c0d22c5a42f7d954f63067c3 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Thu, 11 May 2017 16:00:34 +0200 Subject: [PATCH 5/5] Tests: Improved #110 manual test description. --- tests/manual/tickets/110/1.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/manual/tickets/110/1.md b/tests/manual/tickets/110/1.md index 6faba04b..b5ea72ff 100644 --- a/tests/manual/tickets/110/1.md +++ b/tests/manual/tickets/110/1.md @@ -1,5 +1,6 @@ ### Double image toolbar when ContextualToolbar plugin is loaded [#110](https://github.com/ckeditor/ckeditor5-image/issues/110) -Click on image: -- balloon toolbar should appear, -- ContextualToolbar should not appear. +1. When selecting a text, the `ContextualToolbar` should appear. +2. Click on image: + 1. The image toolbar in a balloon should show up, + 2. The ContextualToolbar should not be visible.