8
0
Просмотр исходного кода

Other: Refactor table cell post fixer and add docs.

Maciej Gołaszewski 7 лет назад
Родитель
Сommit
bedf2f039b

+ 94 - 58
packages/ckeditor5-table/src/converters/tablecell-post-fixer.js

@@ -10,6 +10,63 @@
 /**
  * Injects a table cell post-fixer into the editing controller.
  *
+ * The role of the table cell post-fixer is to ensure that the table cell contents in the editing view are properly converted.
+ *
+ * This post-fixer will ensure that after model changes in the editing view:
+ * * single paragraphs are rendered as `<span>
+ * * single paragraphs with one or more attributes are rendered as `<p>`
+ * * single paragraphs in table cell with other block elements are rendered as `<p>`
+ * * paragraphs in table cells with other block elements (including other paragraphs) are rendered as `<p>`.
+ *
+ * In the model each table cell has always at least one block element inside. If no other block was defined (empty table cell) the table
+ * feature will insert empty `<paragraph>`. Similarly text nodes will be wrapped in paragraphs. Rendering in the data pipeline differs
+ * from rendering in the editing pipeline - text nodes in single `<paragraph>` are rendered in the data pipeline as direct children
+ * of the `<td>` or `<th>` elements. In other cases `<paragraph>` elements are rendered as `<p>` blocks.
+ *
+ * To ensure proper mappings between model and view elements and positions in the editing pipeline the table feature will always render
+ * an element in the view: `<span>` for single or empty `<paragraph>` and `<p>` otherwise.
+ *
+ * Example:
+ *
+ *		<table>
+ *			<tableRow>
+ *				<tableCell><paragraph></paragraph></tableCell>
+ *				<tableCell><paragraph>foo</paragraph></tableCell>
+ *				<tableCell><paragraph baz="bar">foo</paragraph></tableCell>
+ *				<tableCell><heading2>bar</heading2><paragraph>baz</paragraph></tableCell>
+ *			</tableRow>
+ *		</table>
+ *
+ * The editor will render in the data pipeline:
+ *
+ *		<figure>
+ *			<table>
+ *				<tbody>
+ *					<tr>
+ *						<td></td>
+ *						<td>foo</td>
+ *						<td><p baz="bar">foo</p></td>
+ *						<td><h3>bar</h3><p>baz</p></td>
+ *					</tr>
+ *				</tbody>
+ *			</table>
+ *		</figure>
+ *
+ * and in the editing view (without widget markup):
+ *
+ *		<figure>
+ *			<table>
+ *				<tbody>
+ *					<tr>
+ *						<td><span></span></td>
+ *						<td><span>foo</span></td>
+ *						<td><p baz="bar">foo</p></td>
+ *						<td><h3>bar</h3><p>baz</p></td>
+ *					</tr>
+ *				</tbody>
+ *			</table>
+ *		</figure>
+ *
  * @param {module:engine/model/model~Model} model
  * @param {module:engine/controller/editingcontroller~EditingController} editing
  */
@@ -25,78 +82,57 @@ export default function injectTableCellPostFixer( model, editing ) {
 function tableCellPostFixer( writer, model, mapper ) {
 	const changes = model.document.differ.getChanges();
 
+	// While this is view post fixer only nodes that changed are worth investigating.
 	for ( const entry of changes ) {
-		const tableCell = entry.position && entry.position.parent;
-
-		if ( !tableCell && entry.type == 'attribute' && entry.range.start.parent.name == 'tableCell' ) {
+		// Attribute change - check if it is single paragraph inside table cell that has attributes changed.
+		if ( entry.type == 'attribute' && entry.range.start.parent.name == 'tableCell' ) {
 			const tableCell = entry.range.start.parent;
 
 			if ( tableCell.childCount === 1 ) {
 				const singleChild = tableCell.getChild( 0 );
+				const renameTo = Array.from( singleChild.getAttributes() ).length ? 'p' : 'span';
 
-				if ( !singleChild || !singleChild.is( 'paragraph' ) ) {
-					return;
-				}
-
-				const viewElement = mapper.toViewElement( singleChild );
-
-				let renameTo = 'p';
-
-				if ( viewElement.name === 'p' ) {
-					if ( [ ...singleChild.getAttributes() ].length ) {
-						return;
-					} else {
-						renameTo = 'span';
-					}
-				}
-
-				const renamedViewElement = writer.rename( viewElement, renameTo );
-
-				// Re-bind table cell to renamed view element.
-				mapper.bindElements( singleChild, renamedViewElement );
+				renameParagraphIfDifferent( singleChild, renameTo, writer, mapper );
 			}
-		}
+		} else {
+			// Check all nodes inside table cell on insert/remove operations (also other blocks).
+			const tableCell = entry.position && entry.position.parent;
 
-		if ( !tableCell ) {
-			continue;
-		}
+			if ( tableCell && tableCell.is( 'tableCell' ) ) {
+				const renameTo = tableCell.childCount > 1 ? 'p' : 'span';
 
-		if ( tableCell.is( 'tableCell' ) ) {
-			if ( tableCell.childCount > 1 ) {
 				for ( const child of tableCell.getChildren() ) {
-					if ( child.name != 'paragraph' ) {
-						continue;
-					}
-
-					const viewElement = mapper.toViewElement( child );
-
-					if ( viewElement && viewElement.name === 'span' ) {
-						// Unbind table cell as <span> will be renamed to <p>.
-						// mapper.unbindModelElement( tableCell );
-
-						const renamedViewElement = writer.rename( viewElement, 'p' );
-
-						// Re-bind table cell to renamed view element.
-						mapper.bindElements( child, renamedViewElement );
-					}
+					renameParagraphIfDifferent( child, renameTo, writer, mapper );
 				}
-			} else {
-				const singleChild = tableCell.getChild( 0 );
+			}
+		}
+	}
+}
 
-				if ( !singleChild || !singleChild.is( 'paragraph' ) ) {
-					return;
-				}
+// Renames associated view element to a desired one. It will only rename if:
+// - model elemenet is a paragraph
+// - view element is converted (mapped)
+// - view element has different name then requested.
+//
+// @param modelElement
+// @param desiredElementName
+// @param {module:engine/view/writer~Writer} writer
+// @param {module:engine/conversion/mapper~Mapper} mapper
+function renameParagraphIfDifferent( modelElement, desiredElementName, writer, mapper ) {
+	// Only rename paragraph elements.
+	if ( !modelElement.is( 'paragraph' ) ) {
+		return;
+	}
 
-				const viewElement = mapper.toViewElement( singleChild );
+	const viewElement = mapper.toViewElement( modelElement );
 
-				// Unbind table cell as <span> will be renamed to <p>.
-				// mapper.unbindModelElement( tableCell );
+	// Only rename converted elements which aren't desired ones.
+	if ( !viewElement || viewElement.name === desiredElementName ) {
+		return;
+	}
 
-				const renamedViewElement = writer.rename( viewElement, 'span' );
+	const renamedViewElement = writer.rename( viewElement, desiredElementName );
 
-				// Re-bind table cell to renamed view element.
-				mapper.bindElements( singleChild, renamedViewElement );
-			}
-		}
-	}
+	// Bind table cell to renamed view element.
+	mapper.bindElements( modelElement, renamedViewElement );
 }

+ 27 - 1
packages/ckeditor5-table/tests/converters/tablecell-post-fixer.js

@@ -45,7 +45,7 @@ describe( 'TableCell post-fixer', () => {
 			} );
 	} );
 
-	it( 'should rename <span> to <p> when more then one block content inside table cell', () => {
+	it( 'should rename <span> to <p> when adding more <paragraph> elements to the same table cell', () => {
 		setModelData( model, modelTable( [ [ '00[]' ] ] ) );
 
 		const table = root.getChild( 0 );
@@ -85,6 +85,32 @@ describe( 'TableCell post-fixer', () => {
 		], { asWidget: true } ) );
 	} );
 
+	it( 'should properly rename the same element on consecutive changes', () => {
+		setModelData( model, modelTable( [ [ '00[]' ] ] ) );
+
+		const table = root.getChild( 0 );
+
+		model.change( writer => {
+			const nodeByPath = table.getNodeByPath( [ 0, 0, 0 ] );
+
+			writer.insertElement( 'paragraph', nodeByPath, 'after' );
+
+			writer.setSelection( nodeByPath.nextSibling, 0 );
+		} );
+
+		expect( formatTable( getViewData( viewDocument, { withoutSelection: true } ) ) ).to.equal( formattedViewTable( [
+			[ '<p>00</p><p></p>' ]
+		], { asWidget: true } ) );
+
+		model.change( writer => {
+			writer.remove( table.getNodeByPath( [ 0, 0, 1 ] ) );
+		} );
+
+		expect( formatTable( getViewData( viewDocument, { withoutSelection: true } ) ) ).to.equal( formattedViewTable( [
+			[ '00' ]
+		], { asWidget: true } ) );
+	} );
+
 	it( 'should rename <span> to <p> when setting attribute on <paragraph>', () => {
 		setModelData( model, modelTable( [ [ '<paragraph>00[]</paragraph>' ] ] ) );