From 6c797b943f8407bd10ae3aedeb647120dd328f82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maksymilian=20Barna=C5=9B?= Date: Thu, 29 Apr 2021 12:16:06 +0200 Subject: [PATCH] Store a caption in TableCaptionEditing caption registry. --- .../src/tablecaption/tablecaptionediting.js | 59 ++++++++++++++++- .../tablecaption/toggletablecaptioncommand.js | 27 +++----- .../tablecaption/toggletablecaptioncommand.js | 63 ++++++++++++++++++- 3 files changed, 127 insertions(+), 22 deletions(-) diff --git a/packages/ckeditor5-table/src/tablecaption/tablecaptionediting.js b/packages/ckeditor5-table/src/tablecaption/tablecaptionediting.js index 043b0f21302..212e2589cf3 100644 --- a/packages/ckeditor5-table/src/tablecaption/tablecaptionediting.js +++ b/packages/ckeditor5-table/src/tablecaption/tablecaptionediting.js @@ -8,7 +8,7 @@ */ import { Plugin } from 'ckeditor5/src/core'; -import { enablePlaceholder } from 'ckeditor5/src/engine'; +import { Element, enablePlaceholder } from 'ckeditor5/src/engine'; import { toWidgetEditable } from 'ckeditor5/src/widget'; import injectTableCaptionPostFixer from '../converters/table-caption-post-fixer'; @@ -28,6 +28,23 @@ export default class TableCaptionEditing extends Plugin { return 'TableCaptionEditing'; } + /** + * @inheritDoc + */ + constructor( editor ) { + super( editor ); + + /** + * A map that keeps saved JSONified table captions and table model elements they are + * associated with. + * + * To learn more about this system, see {@link #_saveCaption}. + * + * @member {WeakMap.} + */ + this._savedCaptionsMap = new WeakMap(); + } + /** * @inheritDoc */ @@ -93,5 +110,45 @@ export default class TableCaptionEditing extends Plugin { injectTableCaptionPostFixer( editor.model ); } + + /** + * Returns the saved {@link module:engine/model/element~Element#toJSON JSONified} caption + * of an table model element. + * + * See {@link #_saveCaption}. + * + * @protected + * @param {module:engine/model/element~Element} tableModelElement The model element the + * caption should be returned for. + * @returns {module:engine/model/element~Element|null} The model caption element or `null` if there is none. + */ + _getSavedCaption( tableModelElement ) { + const jsonObject = this._savedCaptionsMap.get( tableModelElement ); + + return jsonObject ? Element.fromJSON( jsonObject ) : null; + } + + /** + * Saves a {@link module:engine/model/element~Element#toJSON JSONified} caption for + * an table element to allow restoring it in the future. + * + * A caption is saved every time it gets hidden and/or the type of an table changes. The + * user should be able to restore it on demand. + * + * **Note**: The caption cannot be stored in the table model element attribute because, + * for instance, when the model state propagates to collaborators, the attribute would get + * lost (mainly because it does not convert to anything when the caption is hidden) and + * the states of collaborators' models would de-synchronize causing numerous issues. + * + * See {@link #_getSavedCaption}. + * + * @protected + * @param {module:engine/model/element~Element} tableModelElement The model element the + * caption is saved for. + * @param {module:engine/model/element~Element} caption The caption model element to be saved. + */ + _saveCaption( tableModelElement, caption ) { + this._savedCaptionsMap.set( tableModelElement, caption.toJSON() ); + } } diff --git a/packages/ckeditor5-table/src/tablecaption/toggletablecaptioncommand.js b/packages/ckeditor5-table/src/tablecaption/toggletablecaptioncommand.js index 683416969a7..bb0606b4c95 100644 --- a/packages/ckeditor5-table/src/tablecaption/toggletablecaptioncommand.js +++ b/packages/ckeditor5-table/src/tablecaption/toggletablecaptioncommand.js @@ -8,7 +8,6 @@ */ import { Command } from 'ckeditor5/src/core'; -import { Element } from 'ckeditor5/src/engine'; import { getCaptionFromTableModelElement, getSelectionAffectedTable } from './utils'; @@ -74,7 +73,7 @@ export default class ToggleTableCaptionCommand extends Command { /** * Shows the table caption. Also: * - * * it attempts to restore the caption content from the `caption` attribute, + * * it attempts to restore the caption content from the `TableCaptionEditing` caption registry, * * it moves the selection to the caption right away, it the `focusCaptionOnShow` option was set. * * @private @@ -84,18 +83,11 @@ export default class ToggleTableCaptionCommand extends Command { _showTableCaption( writer, focusCaptionOnShow ) { const model = this.editor.model; const tableElement = getSelectionAffectedTable( model.document.selection ); + const tableCaptionEditing = this.editor.plugins.get( 'TableCaptionEditing' ); + const savedCaptionElement = tableCaptionEditing._getSavedCaption( tableElement ); - let newCaptionElement; - - // Try restoring the caption from the attribute. - if ( tableElement.hasAttribute( 'caption' ) ) { - newCaptionElement = Element.fromJSON( tableElement.getAttribute( 'caption' ) ); - - // The model attribute is no longer needed if the caption was created out of it. - writer.removeAttribute( 'caption', tableElement ); - } else { - newCaptionElement = writer.createElement( 'caption' ); - } + // Try restoring the caption from the TableCaptionEditing plugin storage. + const newCaptionElement = savedCaptionElement || writer.createElement( 'caption' ); writer.append( newCaptionElement, tableElement ); @@ -107,8 +99,8 @@ export default class ToggleTableCaptionCommand extends Command { /** * Hides the caption of a selected table (or an table caption the selection is anchored to). * - * The content of the caption is stored in the `caption` model attribute of the table - * to make this a reversible action. + * The content of the caption is stored in the `TableCaptionEditing` caption registry to make this + * a reversible action. * * @private * @param {module:engine/model/writer~Writer} writer @@ -116,12 +108,11 @@ export default class ToggleTableCaptionCommand extends Command { _hideTableCaption( writer ) { const model = this.editor.model; const tableElement = getSelectionAffectedTable( model.document.selection ); + const tableCaptionEditing = this.editor.plugins.get( 'TableCaptionEditing' ); const captionElement = getCaptionFromTableModelElement( tableElement ); // Store the caption content so it can be restored quickly if the user changes their mind. - if ( captionElement.childCount ) { - writer.setAttribute( 'caption', captionElement.toJSON(), tableElement ); - } + tableCaptionEditing._saveCaption( tableElement, captionElement ); writer.setSelection( writer.createRangeIn( tableElement.getChild( 0 ).getChild( 0 ) ) ); writer.remove( captionElement ); diff --git a/packages/ckeditor5-table/tests/tablecaption/toggletablecaptioncommand.js b/packages/ckeditor5-table/tests/tablecaption/toggletablecaptioncommand.js index ce77af749a6..1c13d97552d 100644 --- a/packages/ckeditor5-table/tests/tablecaption/toggletablecaptioncommand.js +++ b/packages/ckeditor5-table/tests/tablecaption/toggletablecaptioncommand.js @@ -221,14 +221,53 @@ describe( 'ToggleTableCaptionCommand', () => { '' + '' + '' + - 'Foo caption[]' + + 'Foo<$text bold="true">bar' + '' ); command.execute(); assertEqualMarkup( getData( model ), - '' + + '
' + + '' + + '' + + '[]' + + '' + + '' + + '
' + ); + + command.execute(); + + assertEqualMarkup( getData( model ), + '' + + '' + + '' + + '[]' + + '' + + '' + + '' + + '
Foo<$text bold="true">bar
' + ); + } ); + + it( 'should overwrite caption with an empty one', () => { + setData( model, + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '
Foo
' + ); + + // Hide the caption. + command.execute(); + + assertEqualMarkup( getData( model ), + '' + '' + '' + '[]' + @@ -237,6 +276,22 @@ describe( 'ToggleTableCaptionCommand', () => { '
' ); + // Show the caption. + command.execute(); + + setData( model, + '' + + '' + + '' + + '' + + '' + + '' + + '' + + '
' + ); + + // Hide and then show the caption. + command.execute(); command.execute(); assertEqualMarkup( getData( model ), @@ -246,7 +301,9 @@ describe( 'ToggleTableCaptionCommand', () => { '[]' + '' + '' + - 'Foo caption' + + + // Caption should be empty. + '' + '' ); } );