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

T/77 Image captions in the view are hidden instead of being removed #81

Merged
merged 9 commits into from
Mar 30, 2017
Merged

T/77 Image captions in the view are hidden instead of being removed #81

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
5af04c8
Added test checking undo with inserting image with caption.
szymonkups Mar 22, 2017
34b3970
Merge branch 't/57' into t/77
szymonkups Mar 22, 2017
71017fa
Image captions in view are hidden instead of being removed.
szymonkups Mar 23, 2017
4f5a65f
Removed custom ck-editable_hidden class and used generic ck-hidden cl…
szymonkups Mar 28, 2017
58d33d6
Merge branch 'master' into t/77
Reinmar Mar 28, 2017
cd5da0e
Showing image caption when content is inserted inside model.
szymonkups Mar 29, 2017
649ff24
Update caption visibility in the view when something is inserted to i…
szymonkups Mar 29, 2017
f757c0b
Added one more test to ImageCaptionEngine.
szymonkups Mar 29, 2017
93d60f1
Tests: reverted incorrectly renamed test.
scofalik Mar 30, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
151 changes: 35 additions & 116 deletions src/imagecaption/imagecaptionengine.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ 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 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';
Expand Down Expand Up @@ -43,14 +42,13 @@ 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.
*
* @private
* @member {module:engine/view/editableelement~EditableElement} #_lastSelectedEditable
* @member {module:engine/view/editableelement~EditableElement} #_lastSelectedCaption
*/

/**
Expand All @@ -77,102 +75,47 @@ export default class ImageCaptionEngine extends Plugin {
.toElement( 'caption' );

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

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

// 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', () => this._updateView(), { priority: 'high' } );
this.listenTo( viewDocument, 'render', () => this._updateCaptionVisibility(), { priority: 'high' } );
}

/**
* Checks if there is an empty caption element to remove from the view.
* Updates view before each rendering, making sure that empty captions (so unnecessary ones) are hidden
* and then visible when the image is selected.
*
* @private
*/
_removeEmptyCaption() {
_updateCaptionVisibility() {
const mapper = this.editor.editing.mapper;
const viewSelection = this.editor.editing.view.selection;
const viewCaptionElement = this._lastSelectedEditable;

// No caption to hide.
if ( !viewCaptionElement ) {
return;
}

// If selection is placed inside caption - do not remove it.
if ( viewSelection.editableElement === viewCaptionElement ) {
return;
}

// Do not remove caption if selection is placed on image that contains that caption.
const selectedElement = viewSelection.getSelectedElement();
let viewCaption;

if ( selectedElement && isImageWidget( selectedElement ) ) {
const viewImage = viewCaptionElement.findAncestor( element => element == selectedElement );

if ( viewImage ) {
return;
}
// Hide last selected caption if have no child elements.
if ( this._lastSelectedCaption && !this._lastSelectedCaption.childCount ) {
this._lastSelectedCaption.addClass( 'ck-editable_hidden' );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have .ck-hidden class for this kind of things.

}

// Remove image caption if its empty.
if ( viewCaptionElement.childCount === 0 ) {
const mapper = this.editor.editing.mapper;
viewWriter.remove( ViewRange.createOn( viewCaptionElement ) );
mapper.unbindViewElement( viewCaptionElement );
}
}

/**
* Checks if selected image needs a new caption element inside.
*
* @private
*/
_addCaptionWhenSelected() {
const editing = this.editor.editing;
const selection = editing.view.selection;
const viewImage = selection.getSelectedElement();
const mapper = editing.mapper;

if ( viewImage && isImageWidget( viewImage ) ) {
const modelImage = mapper.toModelElement( viewImage );
// If whole image widget is selected.
if ( selectedElement && isImageWidget( selectedElement ) ) {
const modelImage = mapper.toModelElement( selectedElement );
const modelCaption = getCaptionFromImage( modelImage );
let viewCaption = mapper.toViewElement( modelCaption );

if ( !viewCaption ) {
viewCaption = this._createCaption();
insertViewCaptionAndBind( viewCaption, modelCaption, viewImage, mapper );
}

this._lastSelectedEditable = viewCaption;
viewCaption = mapper.toViewElement( modelCaption );
}
}

/**
* 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 selection is placed inside caption.
if ( isCaption( viewSelection.editableElement ) ) {
viewCaption = viewSelection.editableElement;
}

if ( editableElement && isCaption( selection.editableElement ) ) {
this._lastSelectedEditable = selection.editableElement;
if ( viewCaption ) {
viewCaption.removeClass( 'ck-editable_hidden' );
this._lastSelectedCaption = viewCaption;
}
}
}
Expand Down Expand Up @@ -201,42 +144,22 @@ function insertMissingModelCaptionElement( 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
// @param {Function|module:engine/view/element~Element} elementCreator
// @param {Boolean} [hide=true] When set to `false` view element will not be inserted when it's empty.
// @return {Function}
function captionModelToView( elementCreator ) {
function captionModelToView( elementCreator, hide = true ) {
return ( evt, data, consumable, conversionApi ) => {
const captionElement = data.item;

if ( isImage( captionElement.parent ) && ( captionElement.childCount > 0 ) ) {
// Return if element shouldn't be present when empty.
if ( !captionElement.childCount && !hide ) {
return;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be better if this part of code is in if ( isImage( ... ) ) { ... } so it won't ever affect other converters if we have different type of captions.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the other hand it won't affect other converters because it does not consume anything, but I was also thinking whether the caption should not be consumed in this case 🤔 . Since we are not consuming, the code could stay here.

}

if ( isImage( captionElement.parent ) ) {
if ( !consumable.consume( data.item, 'insert' ) ) {
return;
}
Expand All @@ -246,20 +169,16 @@ function captionModelToView( elementCreator ) {
elementCreator.clone( true ) :
elementCreator();

// Hide if empty.
if ( !captionElement.childCount ) {
viewCaption.addClass( 'ck-editable_hidden' );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have .ck-hidden class for this kind of things.

}

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
Expand Down
Loading