Store a caption in TableCaptionEditing caption registry.

ckeditor · Apr 29, 2021 · 6c797b9 · 6c797b9
1 parent fb8c775
commit 6c797b9
Show file tree

Hide file tree

Showing 3 changed files with 127 additions and 22 deletions.
diff --git 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.<module:engine/model/element~Element,Object>}
+		 */
+		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
@@ -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,21 +99,20 @@ 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
 	 */
 	_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
@@ -221,14 +221,53 @@ describe( 'ToggleTableCaptionCommand', () => {
 							'<paragraph></paragraph>' +
 						'</tableCell>' +
 					'</tableRow>' +
-					'<caption>Foo caption[]</caption>' +
+					'<caption>Foo<$text bold="true">bar</$text></caption>' +
 				'</table>'
 			);
 
 			command.execute();
 
 			assertEqualMarkup( getData( model ),
-				'<table caption="{"name":"caption","children":[{"data":"Foo caption"}]}">' +
+				'<table>' +
+					'<tableRow>' +
+						'<tableCell>' +
+							'<paragraph>[]</paragraph>' +
+						'</tableCell>' +
+					'</tableRow>' +
+				'</table>'
+			);
+
+			command.execute();
+
+			assertEqualMarkup( getData( model ),
+				'<table>' +
+					'<tableRow>' +
+						'<tableCell>' +
+							'<paragraph>[]</paragraph>' +
+						'</tableCell>' +
+					'</tableRow>' +
+					'<caption>Foo<$text bold="true">bar</$text></caption>' +
+				'</table>'
+			);
+		} );
+
+		it( 'should overwrite caption with an empty one', () => {
+			setData( model,
+				'<table>' +
+					'<tableRow>' +
+						'<tableCell>' +
+							'<paragraph></paragraph>' +
+						'</tableCell>' +
+					'</tableRow>' +
+					'<caption>Foo</caption>' +
+				'</table>'
+			);
+
+			// Hide the caption.
+			command.execute();
+
+			assertEqualMarkup( getData( model ),
+				'<table>' +
 					'<tableRow>' +
 						'<tableCell>' +
 							'<paragraph>[]</paragraph>' +
@@ -237,6 +276,22 @@ describe( 'ToggleTableCaptionCommand', () => {
 				'</table>'
 			);
 
+			// Show the caption.
+			command.execute();
+
+			setData( model,
+				'<table>' +
+					'<tableRow>' +
+						'<tableCell>' +
+							'<paragraph></paragraph>' +
+						'</tableCell>' +
+					'</tableRow>' +
+					'<caption></caption>' +
+				'</table>'
+			);
+
+			// Hide and then show the caption.
+			command.execute();
 			command.execute();
 
 			assertEqualMarkup( getData( model ),
@@ -246,7 +301,9 @@ describe( 'ToggleTableCaptionCommand', () => {
 							'<paragraph>[]</paragraph>' +
 						'</tableCell>' +
 					'</tableRow>' +
-					'<caption>Foo caption</caption>' +
+
+					// Caption should be empty.
+					'<caption></caption>' +
 				'</table>'
 			);
 		} );