T/77 Image captions in the view are hidden instead of being removed #81
Conversation
| } | ||
| // Hide last selected caption if have no child elements. | ||
| if ( this._lastSelectedCaption && !this._lastSelectedCaption.childCount ) { | ||
| this._lastSelectedCaption.addClass( 'ck-editable_hidden' ); |
There was a problem hiding this comment.
We have .ck-hidden class for this kind of things.
| @@ -246,20 +169,16 @@ function captionModelToView( elementCreator ) { | |||
| elementCreator.clone( true ) : | |||
| elementCreator(); | |||
|
|
|||
| // Hide if empty. | |||
| if ( !captionElement.childCount ) { | |||
| viewCaption.addClass( 'ck-editable_hidden' ); | |||
There was a problem hiding this comment.
We have .ck-hidden class for this kind of things.
…ass in image caption styles.
|
I asked @scofalik to check this PR. |
| if ( isImage( captionElement.parent ) && ( captionElement.childCount > 0 ) ) { | ||
| // Return if element shouldn't be present when empty. | ||
| if ( !captionElement.childCount && !hide ) { | ||
| return; |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
|
I'd like to see one improvement introduced here (unless there is already ticket for this and you plan to work on it). Right now, when content is inserted into the caption, the caption does not show. The content could come from OT, for example. |
Docs fixes.
Suggested merge commit message (convention)
Fix: Image captions in the view are hidden instead of being removed. Closes ckeditor/ckeditor5#5080 .
Additional information
Please close #74 before.