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

Commit

Permalink
Merge pull request #138 from ckeditor/t/137
Browse files Browse the repository at this point in the history
Fix: The `ImageTextAlternative` UI should hide when the edited image element has been removed by an external change. Closes #137.
  • Loading branch information
oleq authored Sep 13, 2017
2 parents 060583e + 12be7a9 commit 6ab8c40
Show file tree
Hide file tree
Showing 9 changed files with 86 additions and 19 deletions.
6 changes: 2 additions & 4 deletions src/image.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +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 { isImageWidgetSelected } from './image/utils';

import '../theme/theme.scss';

Expand Down Expand Up @@ -49,9 +49,7 @@ export default class Image extends Plugin {
// https://github.com/ckeditor/ckeditor5-image/issues/110
if ( contextualToolbar ) {
this.listenTo( contextualToolbar, 'show', evt => {
const selectedElement = editor.editing.view.selection.getSelectedElement();

if ( selectedElement && isImageWidget( selectedElement ) ) {
if ( isImageWidgetSelected( editor.editing.view.selection ) ) {
evt.stop();
}
}, { priority: 'high' } );
Expand Down
6 changes: 2 additions & 4 deletions src/image/ui/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
*/

import BalloonPanelView from '@ckeditor/ckeditor5-ui/src/panel/balloon/balloonpanelview';
import { isImageWidget } from '../utils';
import { isImageWidgetSelected } from '../utils';

/**
* A helper utility which positions the
Expand All @@ -18,11 +18,9 @@ import { isImageWidget } from '../utils';
* @param {module:core/editor/editor~Editor} editor The editor instance.
*/
export function repositionContextualBalloon( editor ) {
const editingView = editor.editing.view;
const balloon = editor.plugins.get( 'ContextualBalloon' );
const selectedElement = editingView.selection.getSelectedElement();

if ( selectedElement && isImageWidget( selectedElement ) ) {
if ( isImageWidgetSelected( editor.editing.view.selection ) ) {
const position = getBalloonPositionData( editor );

balloon.updatePosition( position );
Expand Down
12 changes: 12 additions & 0 deletions src/image/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,18 @@ export function isImageWidget( viewElement ) {
return !!viewElement.getCustomProperty( imageSymbol ) && isWidget( viewElement );
}

/**
* Checks if an image widget is the only selected element.
*
* @param {module:engine/view/selection~Selection} viewSelection
* @returns {Boolean}
*/
export function isImageWidgetSelected( viewSelection ) {
const viewElement = viewSelection.getSelectedElement();

return !!( viewElement && isImageWidget( viewElement ) );
}

/**
* Checks if the provided model element is an instance of {@link module:engine/model/element~Element Element} and its name
* is `image`.
Expand Down
9 changes: 6 additions & 3 deletions src/imagetextalternative.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import TextAlternativeFormView from './imagetextalternative/ui/textalternativefo
import ContextualBalloon from '@ckeditor/ckeditor5-ui/src/panel/balloon/contextualballoon';
import textAlternativeIcon from '@ckeditor/ckeditor5-core/theme/icons/low-vision.svg';
import { repositionContextualBalloon, getBalloonPositionData } from './image/ui/utils';
import { isImageWidgetSelected } from './image/utils';

import '../theme/imagetextalternative/theme.scss';

Expand Down Expand Up @@ -119,9 +120,11 @@ export default class ImageTextAlternative extends Plugin {
cancel();
} );

// Reposition the balloon upon #render.
// Reposition the balloon or hide the form if an image widget is no longer selected.
this.listenTo( editingView, 'render', () => {
if ( this._isVisible ) {
if ( !isImageWidgetSelected( editingView.selection ) ) {
this._hideForm( true );
} else if ( this._isVisible ) {
repositionContextualBalloon( editor );
}
}, { priority: 'low' } );
Expand Down Expand Up @@ -176,7 +179,7 @@ export default class ImageTextAlternative extends Plugin {
/**
* Removes the {@link #_form} from the {@link #_balloon}.
*
* @param {Boolean} focusEditable Controls whether the editing view is focused afterwards.
* @param {Boolean} [focusEditable=false] Controls whether the editing view is focused afterwards.
* @private
*/
_hideForm( focusEditable ) {
Expand Down
7 changes: 2 additions & 5 deletions src/imagetoolbar.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import Template from '@ckeditor/ckeditor5-ui/src/template';
import Plugin from '@ckeditor/ckeditor5-core/src/plugin';
import ToolbarView from '@ckeditor/ckeditor5-ui/src/toolbar/toolbarview';
import ContextualBalloon from '@ckeditor/ckeditor5-ui/src/panel/balloon/contextualballoon';
import { isImageWidget } from './image/utils';
import { isImageWidgetSelected } from './image/utils';
import { repositionContextualBalloon, getBalloonPositionData } from './image/ui/utils';

const balloonClassName = 'ck-toolbar-container ck-editor-toolbar-container';
Expand Down Expand Up @@ -103,10 +103,7 @@ export default class ImageToolbar extends Plugin {
if ( !editor.ui.focusTracker.isFocused ) {
this._hideToolbar();
} else {
const editingView = editor.editing.view;
const selectedElement = editingView.selection.getSelectedElement();

if ( selectedElement && isImageWidget( selectedElement ) ) {
if ( isImageWidgetSelected( editor.editing.view.selection ) ) {
this._showToolbar();
} else {
this._hideToolbar();
Expand Down
39 changes: 38 additions & 1 deletion tests/image/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,11 @@
*/

import ViewElement from '@ckeditor/ckeditor5-engine/src/view/element';
import ViewSelection from '@ckeditor/ckeditor5-engine/src/view/selection';
import ViewDocumentFragment from '@ckeditor/ckeditor5-engine/src/view/documentfragment';
import ViewRange from '@ckeditor/ckeditor5-engine/src/view/range';
import ModelElement from '@ckeditor/ckeditor5-engine/src/model/element';
import { toImageWidget, isImageWidget, isImage } from '../../src/image/utils';
import { toImageWidget, isImageWidget, isImageWidgetSelected, isImage } from '../../src/image/utils';
import { isWidget, getLabel } from '@ckeditor/ckeditor5-widget/src/utils';

describe( 'image widget utils', () => {
Expand Down Expand Up @@ -49,6 +52,40 @@ describe( 'image widget utils', () => {
} );
} );

describe( 'isImageWidgetSelected()', () => {
let frag;

it( 'should return true when image widget is the only element in the selection', () => {
// We need to create a container for the element to be able to create a Range on this element.
frag = new ViewDocumentFragment( [ element ] );

const selection = new ViewSelection( [ ViewRange.createOn( element ) ] );

expect( isImageWidgetSelected( selection ) ).to.be.true;
} );

it( 'should return false when non-widgetized elements is the only element in the selection', () => {
const notWidgetizedElement = new ViewElement( 'p' );

// We need to create a container for the element to be able to create a Range on this element.
frag = new ViewDocumentFragment( [ notWidgetizedElement ] );

const selection = new ViewSelection( [ ViewRange.createOn( notWidgetizedElement ) ] );

expect( isImageWidgetSelected( selection ) ).to.be.false;
} );

it( 'should return false when widget element is not the only element in the selection', () => {
const notWidgetizedElement = new ViewElement( 'p' );

frag = new ViewDocumentFragment( [ element, notWidgetizedElement ] );

const selection = new ViewSelection( [ ViewRange.createIn( frag ) ] );

expect( isImageWidgetSelected( selection ) ).to.be.false;
} );
} );

describe( 'isImage', () => {
it( 'should return true for image element', () => {
const image = new ModelElement( 'image' );
Expand Down
16 changes: 16 additions & 0 deletions tests/imagetextalternative.js
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,22 @@ describe( 'ImageTextAlternative', () => {
editingView.fire( 'render' );
sinon.assert.calledOnce( spy );
} );

it( 'should hide the form and focus editable when image widget has been removed by external change', () => {
setData( doc, '[<image src=""></image>]' );
button.fire( 'execute' );

const remveSpy = sinon.spy( balloon, 'remove' );
const focusSpy = sinon.spy( editor.editing.view, 'focus' );

// EnqueueChanges automatically fires #render event.
doc.enqueueChanges( () => {
doc.batch( 'transparent' ).remove( doc.selection.getFirstRange() );
} );

sinon.assert.calledWithExactly( remveSpy, form );
sinon.assert.calledOnce( focusSpy );
} );
} );

describe( 'integration with the editor focus', () => {
Expand Down
9 changes: 7 additions & 2 deletions tests/manual/tickets/106/1.js
Original file line number Diff line number Diff line change
Expand Up @@ -108,9 +108,14 @@ function startExternalDelete( editor ) {
const document = editor.document;
const bath = document.batch( 'transparent' );

wait( 3000 ).then( () => {
function removeSecondBlock() {
document.enqueueChanges( () => {
bath.remove( Range.createFromPositionAndShift( new Position( document.getRoot(), [ 1 ] ), 1 ) );
} );
} );
}

wait( 3000 )
.then( () => removeSecondBlock() )
.then( () => wait( 3000 ) )
.then( () => removeSecondBlock() );
}
1 change: 1 addition & 0 deletions tests/manual/tickets/106/1.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,4 @@

1. Click **Start external changes** then quickly select the image in the content, then from the toolbar open the **text alternative** editing balloon.
2. Observe if the balloon remains attached to the image as the content is deleted.
3. Wait a bit more and observe if the balloon is removed together with an image.

0 comments on commit 6ab8c40

Please sign in to comment.