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 #66 from ckeditor/t/58
Browse files Browse the repository at this point in the history 
Fix: The editor no longer crashes when undoing or redoing changes should reshow temporarily invisible image caption. Closes #58.
  • Loading branch information
@Reinmar
Reinmar authored Mar 8, 2017
2 parents cc8f671 + ef82bf2 commit 8e36645
Show file tree
Hide file tree
Showing 4 changed files with 242 additions and 65 deletions.
174 changes: 112 additions & 62 deletions src/imagecaption/imagecaptionengine.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,18 @@ import ModelTreeWalker from '@ckeditor/ckeditor5-engine/src/model/treewalker';
import ModelElement from '@ckeditor/ckeditor5-engine/src/model/element';
import ViewContainerElement from '@ckeditor/ckeditor5-engine/src/view/containerelement';
import ViewElement from '@ckeditor/ckeditor5-engine/src/view/element';
import ViewPosition from '@ckeditor/ckeditor5-engine/src/view/position';
import ViewRange from '@ckeditor/ckeditor5-engine/src/view/range';
import viewWriter from '@ckeditor/ckeditor5-engine/src/view/writer';
import ModelPosition from '@ckeditor/ckeditor5-engine/src/model/position';
import ViewPosition from '@ckeditor/ckeditor5-engine/src/view/position';
import buildViewConverter from '@ckeditor/ckeditor5-engine/src/conversion/buildviewconverter';
import ViewMatcher from '@ckeditor/ckeditor5-engine/src/view/matcher';
import { isImage, isImageWidget } from '../image/utils';
import { captionElementCreator, isCaption, getCaptionFromImage } from './utils';
import {
captionElementCreator,
isCaption,
getCaptionFromImage,
matchImageCaption
} from './utils';

/**
* The image caption engine plugin.
Expand All @@ -39,74 +43,55 @@ export default class ImageCaptionEngine extends Plugin {
const schema = document.schema;
const data = editor.data;
const editing = editor.editing;
const mapper = editing.mapper;

/**
* Last selected caption editable.
* It is used for hiding editable when is empty and image widget is no longer selected.
*
* @member {module:image/imagecaption/imagecaptionengine~ImageCaptionEngine} #_lastSelectedEditable
* @private
* @member {module:engine/view/editableelement~EditableElement} #_lastSelectedEditable
*/

/**
* Function used to create editable caption element in the editing view.
*
* @private
* @member {Function}
*/
this._createCaption = captionElementCreator( viewDocument );

// Schema configuration.
schema.registerItem( 'caption' );
schema.allow( { name: '$inline', inside: 'caption' } );
schema.allow( { name: 'caption', inside: 'image' } );
schema.limits.add( 'caption' );

// Add caption element to each image inserted without it.
document.on( 'change', insertMissingCaptionElement );

// View to model converter for data pipeline.
const matcher = new ViewMatcher( ( element ) => {
const parent = element.parent;

// Convert only captions for images.
if ( element.name == 'figcaption' && parent && parent.name == 'figure' && parent.hasClass( 'image' ) ) {
return { name: true };
}

return null;
} );
document.on( 'change', insertMissingModelCaptionElement );

// View to model converter for the data pipeline.
buildViewConverter()
.for( data.viewToModel )
.from( matcher )
.from( matchImageCaption )
.toElement( 'caption' );

// Model to view converter for data pipeline.
data.modelToView.on(
'insert:caption',
captionModelToView( new ViewContainerElement( 'figcaption' ) )
);
// Model to view converter for the data pipeline.
data.modelToView.on( 'insert:caption', captionModelToView( new ViewContainerElement( 'figcaption' ) ) );

// Model to view converter for editing pipeline.
editing.modelToView.on(
'insert:caption',
captionModelToView( captionElementCreator( viewDocument ) )
);
// Model to view converter for the editing pipeline.
editing.modelToView.on( 'insert:caption', captionModelToView( this._createCaption ) );

// Adding / removing caption element when there is no text in the model.
const selection = viewDocument.selection;
// When inserting something to caption in the model - make sure that caption in the view is also present.
// See https://github.com/ckeditor/ckeditor5-image/issues/58.
editing.modelToView.on( 'insert', insertMissingViewCaptionElement( this._createCaption, mapper ), { priority: 'high' } );

// Update view before each rendering.
this.listenTo( viewDocument, 'render', () => {
// Check if there is an empty caption view element to remove.
this._removeEmptyCaption();

// Check if image widget is selected and caption view element needs to be added.
this._addCaption();

// If selection is currently inside caption editable - store it to hide when empty.
const editableElement = selection.editableElement;

if ( editableElement && isCaption( selection.editableElement ) ) {
this._lastSelectedEditable = selection.editableElement;
}
}, { priority: 'high' } );
this.listenTo( viewDocument, 'render', () => this._updateView(), { priority: 'high' } );
}

/**
* Checks if there is an empty caption element to remove from view.
* Checks if there is an empty caption element to remove from the view.
*
* @private
*/
Expand Down Expand Up @@ -148,36 +133,55 @@ export default class ImageCaptionEngine extends Plugin {
*
* @private
*/
_addCaption() {
_addCaptionWhenSelected() {
const editing = this.editor.editing;
const selection = editing.view.selection;
const imageFigure = selection.getSelectedElement();
const viewImage = selection.getSelectedElement();
const mapper = editing.mapper;
const editableCreator = captionElementCreator( editing.view );

if ( imageFigure && isImageWidget( imageFigure ) ) {
const modelImage = mapper.toModelElement( imageFigure );
if ( viewImage && isImageWidget( viewImage ) ) {
const modelImage = mapper.toModelElement( viewImage );
const modelCaption = getCaptionFromImage( modelImage );
let viewCaption = mapper.toViewElement( modelCaption );

if ( !viewCaption ) {
viewCaption = editableCreator();

const viewPosition = ViewPosition.createAt( imageFigure, 'end' );
mapper.bindElements( modelCaption, viewCaption );
viewWriter.insert( viewPosition, viewCaption );
viewCaption = this._createCaption();
insertViewCaptionAndBind( viewCaption, modelCaption, viewImage, mapper );
}

this._lastSelectedEditable = viewCaption;
}
}

/**
* Updates view before each rendering, making sure that empty captions (so unnecessary ones) are removed
* and then re-added when the image is selected.
*
* @private
*/
_updateView() {
const selection = this.editor.editing.view.selection;

// Check if there is an empty caption view element to remove.
this._removeEmptyCaption();

// Check if image widget is selected and caption view element needs to be added.
this._addCaptionWhenSelected();

// If selection is currently inside caption editable - store it to hide when empty.
const editableElement = selection.editableElement;

if ( editableElement && isCaption( selection.editableElement ) ) {
this._lastSelectedEditable = selection.editableElement;
}
}
}

// Checks whether data inserted to the model document have image element that has no caption element inside it.
// If there is none - adds it to the image element.
//
// @private
function insertMissingCaptionElement( evt, changeType, data, batch ) {
function insertMissingModelCaptionElement( evt, changeType, data, batch ) {
if ( changeType !== 'insert' ) {
return;
}
Expand All @@ -198,6 +202,31 @@ function insertMissingCaptionElement( evt, changeType, data, batch ) {
}
}

// Returns function that should be executed when model to view conversion is made. It checks if insertion is placed
// inside model caption and makes sure that corresponding view element exists.
//
// @private
// @param {function} creator Function that returns view caption element.
// @param {module:engine/conversion/mapper~Mapper} mapper
// @return {function}
function insertMissingViewCaptionElement( creator, mapper ) {
return ( evt, data ) => {
if ( isInsideCaption( data.item ) ) {
const modelCaption = data.item.parent;
const modelImage = modelCaption.parent;

const viewImage = mapper.toViewElement( modelImage );
let viewCaption = mapper.toViewElement( modelCaption );

// Image should be already converted to the view.
if ( viewImage && !viewCaption ) {
viewCaption = creator();
insertViewCaptionAndBind( viewCaption, modelCaption, viewImage, mapper );
}
}
};
}

// Creates a converter that converts image caption model element to view element.
//
// @private
Expand All @@ -212,14 +241,35 @@ function captionModelToView( elementCreator ) {
return;
}

const imageFigure = conversionApi.mapper.toViewElement( data.range.start.parent );
const viewElement = ( elementCreator instanceof ViewElement ) ?
const viewImage = conversionApi.mapper.toViewElement( data.range.start.parent );
const viewCaption = ( elementCreator instanceof ViewElement ) ?
elementCreator.clone( true ) :
elementCreator( data, consumable, conversionApi );
elementCreator();

const viewPosition = ViewPosition.createAt( imageFigure, 'end' );
conversionApi.mapper.bindElements( data.item, viewElement );
viewWriter.insert( viewPosition, viewElement );
insertViewCaptionAndBind( viewCaption, data.item, viewImage, conversionApi.mapper );
}
};
}

// Returns `true` if provided `node` is placed inside image's caption.
//
// @private
// @param {module:engine/model/node~Node} node
// @return {Boolean}
function isInsideCaption( node ) {
return !!( node.parent && node.parent.name == 'caption' && node.parent.parent && node.parent.parent.name == 'image' );
}

// Inserts `viewCaption` at the end of `viewImage` and binds it to `modelCaption`.
//
// @private
// @param {module:engine/view/containerelement~ContainerElement} viewCaption
// @param {module:engine/model/element~Element} modelCaption
// @param {module:engine/view/containerelement~ContainerElement} viewImage
// @param {module:engine/conversion/mapper~Mapper} mapper
function insertViewCaptionAndBind( viewCaption, modelCaption, viewImage, mapper ) {
const viewPosition = ViewPosition.createAt( viewImage, 'end' );

viewWriter.insert( viewPosition, viewCaption );
mapper.bindElements( modelCaption, viewCaption );
}
21 changes: 20 additions & 1 deletion src/imagecaption/utils.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ export function isCaption( viewElement ) {
}

/**
* Returns caption's model element from given image element. Returns `null` if no caption is found.
* Returns caption model element from given image element. Returns `null` if no caption is found.
*
* @param {module:engine/model/element~Element} imageModelElement
* @return {module:engine/model/element~Element|null}
Expand All @@ -61,3 +61,22 @@ export function getCaptionFromImage( imageModelElement ) {

return null;
}

/**
* {@link module:engine/view/matcher~Matcher} pattern. Checks if given element is `figcaption` element and is placed
* inside image `figure` element.
*
* @param {module:engine/view/element~Element} element
* @returns {Object|null} Returns object accepted by {@link module:engine/view/matcher~Matcher} or `null` if element
* cannot be matched.
*/
export function matchImageCaption( element ) {
const parent = element.parent;

// Convert only captions for images.
if ( element.name == 'figcaption' && parent && parent.name == 'figure' && parent.hasClass( 'image' ) ) {
return { name: true };
}

return null;
}
69 changes: 68 additions & 1 deletion tests/imagecaption/imagecaptionengine.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import ModelRange from '@ckeditor/ckeditor5-engine/src/model/range';
import ModelPosition from '@ckeditor/ckeditor5-engine/src/model/position';
import ImageCaptionEngine from '../../src/imagecaption/imagecaptionengine';
import ImageEngine from '../../src/image/imageengine';
import UndoEngine from '@ckeditor/ckeditor5-undo/src/undoengine';
import { getData as getModelData, setData as setModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model';
import { getData as getViewData } from '@ckeditor/ckeditor5-engine/src/dev-utils/view';
import buildViewConverter from '@ckeditor/ckeditor5-engine/src/conversion/buildviewconverter';
Expand All @@ -22,7 +23,7 @@ describe( 'ImageCaptionEngine', () => {

beforeEach( () => {
return VirtualTestEditor.create( {
plugins: [ ImageCaptionEngine, ImageEngine ]
plugins: [ ImageCaptionEngine, ImageEngine, UndoEngine ]
} )
.then( newEditor => {
editor = newEditor;
Expand Down Expand Up @@ -186,6 +187,39 @@ describe( 'ImageCaptionEngine', () => {
} );
} );

describe( 'inserting into image caption', () => {
it( 'should add view caption if insertion was made to model caption', () => {
setModelData( document, '<image src=""><caption></caption></image>' );
const image = document.getRoot().getChild( 0 );
const caption = image.getChild( 0 );

// Check if there is no caption in the view
expect( getViewData( viewDocument, { withoutSelection: true } ) ).to.equal(
'<figure class="image ck-widget" contenteditable="false"><img src=""></img></figure>'
);

document.enqueueChanges( () => {
const batch = document.batch();
const position = ModelPosition.createAt( caption );

batch.insert( position, 'foo bar baz' );
} );

// Check if data is inside model.
expect( getModelData( document, { withoutSelection: true } ) ).to.equal(
'<image src=""><caption>foo bar baz</caption></image>'
);

// Check if view has caption.
expect( getViewData( viewDocument, { withoutSelection: true } ) ).to.equal(
'<figure class="image ck-widget" contenteditable="false">' +
'<img src=""></img>' +
'<figcaption contenteditable="true">foo bar baz</figcaption>' +
'</figure>'
);
} );
} );

describe( 'editing view', () => {
it( 'image should have empty figcaption element when is selected', () => {
setModelData( document, '[<image src=""><caption></caption></image>]' );
Expand Down Expand Up @@ -284,5 +318,38 @@ describe( 'ImageCaptionEngine', () => {
'</figure>]'
);
} );

describe( 'undo/redo integration', () => {
it( 'should create view element after redo', () => {
setModelData( document, '<image src=""><caption>[foo bar baz]</caption></image>' );

const modelRoot = document.getRoot();
const modelImage = modelRoot.getChild( 0 );
const modelCaption = modelImage.getChild( 0 );

// Remove text and selection from caption.
document.enqueueChanges( () => {
const batch = document.batch();

batch.remove( ModelRange.createIn( modelCaption ) );
document.selection.removeAllRanges();
} );

// Check if there is no figcaption in the view.
expect( getViewData( viewDocument ) ).to.equal(
'[]<figure class="image ck-widget" contenteditable="false"><img src=""></img></figure>'
);

editor.execute( 'undo' );

// Check if figcaption is back with contents.
expect( getViewData( viewDocument ) ).to.equal(
'<figure class="image ck-widget" contenteditable="false">' +
'<img src=""></img>' +
'<figcaption contenteditable="true">{foo bar baz}</figcaption>' +
'</figure>'
);
} );
} );
} );
} );
Loading

0 comments on commit 8e36645

Please sign in to comment.