Browse Source

Changed: Extract table-cell-content-post-fixer from table-post-fixer. Rename table-post-fixer to table-layout-post-fixer.

Maciej Gołaszewski 7 years ago
parent
commit
af414fa2a7

+ 112 - 0
packages/ckeditor5-table/src/converters/table-cell-content-post-fixer.js

@@ -0,0 +1,112 @@
+/**
+ * @license Copyright (c) 2003-2018, CKSource - Frederico Knabben. All rights reserved.
+ * For licensing, see LICENSE.md.
+ */
+
+/**
+ * @module table/converters/table-cell-content-post-fixer
+ */
+
+/**
+ * Injects a table cell post-fixer into the model.
+ *
+ * The role of the table post-fixer is to ensure that the table cells have the correct content
+ * after a {@link module:engine/model/model~Model#change `change()`} block was executed.
+ *
+ * A table cells must contains at least one block as a child. The empty table cell will have empty `<paragraph>` as a child.
+ *
+ *        <table>
+ *            <tableRow>
+ *                <tableCell></tableCell>
+ *            </tableRow>
+ *        </table>
+ *
+ * Will be fixed to:
+ *
+ *        <table>
+ *            <tableRow>
+ *                <tableCell><paragraph></paragraph></tableCell>
+ *            </tableRow>
+ *        </table>
+ *
+ * @param {module:engine/model/model~Model} model
+ */
+export default function injectTableCellContentPostFixer( model ) {
+	model.document.registerPostFixer( writer => tableCellContentsPostFixer( writer, model ) );
+}
+
+// The table cell contents post-fixer.
+//
+// @param {module:engine/model/writer~Writer} writer
+// @param {module:engine/model/model~Model} model
+function tableCellContentsPostFixer( writer, model ) {
+	const changes = model.document.differ.getChanges();
+
+	let wasFixed = false;
+
+	for ( const entry of changes ) {
+		// Enforce paragraph in tableCell even after other feature remove its contents.
+		if ( entry.type == 'remove' && entry.position.parent.is( 'tableCell' ) ) {
+			wasFixed = fixTableCellContent( entry.position.parent, writer ) || wasFixed;
+		}
+
+		// Analyze table cells on insertion.
+		if ( entry.type == 'insert' ) {
+			if ( entry.name == 'table' ) {
+				wasFixed = fixTable( entry.position.nodeAfter, writer ) || wasFixed;
+			}
+
+			if ( entry.name == 'tableRow' ) {
+				wasFixed = fixTableRow( entry.position.nodeAfter, writer ) || wasFixed;
+			}
+
+			if ( entry.name == 'tableCell' ) {
+				wasFixed = fixTableCellContent( entry.position.nodeAfter, writer ) || wasFixed;
+			}
+		}
+	}
+
+	return wasFixed;
+}
+
+// Fixes all table cells in a table.
+//
+// @param {module:engine/model/element~Element} table
+// @param {module:engine/model/writer~Writer} writer
+function fixTable( table, writer ) {
+	let wasFixed = false;
+
+	for ( const row of table.getChildren() ) {
+		wasFixed = fixTableRow( row, writer ) || wasFixed;
+	}
+
+	return wasFixed;
+}
+
+// Fixes all table cells in a table row.
+//
+// @param {module:engine/model/element~Element} tableRow
+// @param {module:engine/model/writer~Writer} writer
+function fixTableRow( tableRow, writer ) {
+	let wasFixed = false;
+
+	for ( const tableCell of tableRow.getChildren() ) {
+		wasFixed = fixTableCellContent( tableCell, writer ) || wasFixed;
+	}
+
+	return wasFixed;
+}
+
+// Fixes all table cell content by adding paragraph to a table cell without any child.
+//
+// @param {module:engine/model/element~Element} table
+// @param {module:engine/model/writer~Writer} writer
+function fixTableCellContent( tableCell, writer ) {
+	if ( tableCell.childCount == 0 ) {
+		writer.insertElement( 'paragraph', tableCell );
+
+		return true;
+	}
+
+	return false;
+}

+ 7 - 33
packages/ckeditor5-table/src/converters/table-post-fixer.js

@@ -4,7 +4,7 @@
  */
 
 /**
- * @module table/converters/table-post-fixer
+ * @module table/converters/table-layout-post-fixer
  */
 
 import Position from '@ckeditor/ckeditor5-engine/src/model/position';
@@ -12,9 +12,9 @@ import { createEmptyTableCell, findAncestor, updateNumericAttribute } from './..
 import TableWalker from './../tablewalker';
 
 /**
- * Injects a table post-fixer into the model.
+ * Injects a table layout post-fixer into the model.
  *
- * The role of the table post-fixer is to ensure that the table rows have the correct structure
+ * The role of the table layout post-fixer is to ensure that the table rows have the correct structure
  * after a {@link module:engine/model/model~Model#change `change()`} block was executed.
  *
  * The correct structure means that:
@@ -212,36 +212,17 @@ import TableWalker from './../tablewalker';
  *				</tr>
  *			</tbody>
  *		</table>
- *
- * ## Ensuring proper table structure
- *
- * A table cells must contains at least one block as a child. The empty table cell will have empty `<paragraph>` as a child.
- *
- *		<table>
- *			<tableRow>
- *				<tableCell></tableCell>
- *			</tableRow>
- *		</table>
- *
- * Will be fixed to:
- *
- *		<table>
- *			<tableRow>
- *				<tableCell><paragraph></paragraph></tableCell>
- *			</tableRow>
- *		</table>
- *
  * @param {module:engine/model/model~Model} model
  */
-export default function injectTablePostFixer( model ) {
-	model.document.registerPostFixer( writer => tablePostFixer( writer, model ) );
+export default function injectTableLayoutPostFixer( model ) {
+	model.document.registerPostFixer( writer => tableLayoutPostFixer( writer, model ) );
 }
 
-// The table post-fixer.
+// The table layout post-fixer.
 //
 // @param {module:engine/model/writer~Writer} writer
 // @param {module:engine/model/model~Model} model
-function tablePostFixer( writer, model ) {
+function tableLayoutPostFixer( writer, model ) {
 	const changes = model.document.differ.getChanges();
 
 	let wasFixed = false;
@@ -252,13 +233,6 @@ function tablePostFixer( writer, model ) {
 	for ( const entry of changes ) {
 		let table;
 
-		// Enforce paragraph in tableCell even after other feature remove its contents.
-		if ( entry.type == 'remove' && entry.position.parent.is( 'tableCell' ) && entry.position.parent.childCount == 0 ) {
-			writer.insertElement( 'paragraph', entry.position.parent );
-			wasFixed = true;
-		}
-
-		// Fix table on table insert.
 		if ( entry.name == 'table' && entry.type == 'insert' ) {
 			table = entry.position.nodeAfter;
 		}

+ 4 - 2
packages/ckeditor5-table/src/tableediting.js

@@ -33,7 +33,8 @@ import SetHeaderColumnCommand from './commands/setheadercolumncommand';
 import { findAncestor } from './commands/utils';
 import TableUtils from '../src/tableutils';
 
-import injectTablePostFixer from './converters/table-post-fixer';
+import injectTableLayoutPostFixer from './converters/table-layout-post-fixer';
+import injectTableCellContentsPostFixer from './converters/table-cell-content-post-fixer';
 import injectTableCellPostFixer from './converters/tablecell-post-fixer';
 
 import '../theme/tableediting.css';
@@ -145,7 +146,8 @@ export default class TableEditing extends Plugin {
 		editor.commands.add( 'setTableColumnHeader', new SetHeaderColumnCommand( editor ) );
 		editor.commands.add( 'setTableRowHeader', new SetHeaderRowCommand( editor ) );
 
-		injectTablePostFixer( model );
+		injectTableLayoutPostFixer( model );
+		injectTableCellContentsPostFixer( model );
 
 		// Handle tab key navigation.
 		this.editor.keystrokes.set( 'Tab', ( ...args ) => this._handleTabOnSelectedTable( ...args ), { priority: 'low' } );

+ 126 - 0
packages/ckeditor5-table/tests/converters/table-cell-content-post-fixer.js

@@ -0,0 +1,126 @@
+/**
+ * @license Copyright (c) 2003-2018, CKSource - Frederico Knabben. All rights reserved.
+ * For licensing, see LICENSE.md.
+ */
+
+import Range from '@ckeditor/ckeditor5-engine/src/model/range';
+import Position from '@ckeditor/ckeditor5-engine/src/model/position';
+import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph';
+import VirtualTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/virtualtesteditor';
+import { getData as getModelData, setData as setModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model';
+
+import TableEditing from '../../src/tableediting';
+import { formatTable } from './../_utils/utils';
+import UndoEditing from '@ckeditor/ckeditor5-undo/src/undoediting';
+
+describe( 'Table cell content post-fixer', () => {
+	let editor, model, root;
+
+	beforeEach( () => {
+		return VirtualTestEditor
+			.create( {
+				plugins: [ TableEditing, Paragraph, UndoEditing ]
+			} )
+			.then( newEditor => {
+				editor = newEditor;
+				model = editor.model;
+				root = model.document.getRoot();
+			} );
+	} );
+
+	afterEach( () => {
+		editor.destroy();
+	} );
+
+	it( 'should add a paragraph to an empty table cell (on table insert)', () => {
+		setModelData( model,
+			'<table>' +
+				'<tableRow>' +
+					'<tableCell></tableCell>' +
+				'</tableRow>' +
+			'</table>'
+		);
+
+		expect( formatTable( getModelData( model, { withoutSelection: true } ) ) ).to.equal( formatTable(
+			'<table>' +
+				'<tableRow>' +
+					'<tableCell><paragraph></paragraph></tableCell>' +
+				'</tableRow>' +
+			'</table>'
+		) );
+	} );
+
+	it( 'should add a paragraph to an empty table cell (on row insert)', () => {
+		setModelData( model,
+			'<table>' +
+				'<tableRow>' +
+					'<tableCell><paragraph></paragraph></tableCell>' +
+				'</tableRow>' +
+			'</table>'
+		);
+
+		// Insert table row with one table cell
+		model.change( writer => {
+			writer.insertElement( 'tableRow', Position.createAfter( root.getNodeByPath( [ 0, 0 ] ) ) );
+			writer.insertElement( 'tableCell', Position.createAt( root.getNodeByPath( [ 0, 1 ] ) ) );
+		} );
+
+		expect( formatTable( getModelData( model, { withoutSelection: true } ) ) ).to.equal( formatTable(
+			'<table>' +
+				'<tableRow>' +
+					'<tableCell><paragraph></paragraph></tableCell>' +
+				'</tableRow>' +
+				'<tableRow>' +
+					'<tableCell><paragraph></paragraph></tableCell>' +
+				'</tableRow>' +
+			'</table>'
+		) );
+	} );
+
+	it( 'should add a paragraph to an empty table cell (on table cell insert)', () => {
+		setModelData( model,
+			'<table>' +
+				'<tableRow>' +
+					'<tableCell><paragraph></paragraph></tableCell>' +
+				'</tableRow>' +
+			'</table>'
+		);
+
+		// Insert table row with one table cell
+		model.change( writer => {
+			writer.insertElement( 'tableCell', Position.createAt( root.getNodeByPath( [ 0, 0 ], 'end' ) ) );
+		} );
+
+		expect( formatTable( getModelData( model, { withoutSelection: true } ) ) ).to.equal( formatTable(
+			'<table>' +
+				'<tableRow>' +
+					'<tableCell><paragraph></paragraph></tableCell>' +
+					'<tableCell><paragraph></paragraph></tableCell>' +
+				'</tableRow>' +
+			'</table>'
+		) );
+	} );
+
+	it( 'should add a paragraph to an empty table cell (after remove)', () => {
+		setModelData( model,
+			'<table>' +
+				'<tableRow>' +
+					'<tableCell><paragraph>foo</paragraph></tableCell>' +
+				'</tableRow>' +
+			'</table>'
+		);
+
+		// Remove paragraph from table cell.
+		model.change( writer => {
+			writer.remove( Range.createIn( root.getNodeByPath( [ 0, 0, 0 ] ) ) );
+		} );
+
+		expect( formatTable( getModelData( model, { withoutSelection: true } ) ) ).to.equal( formatTable(
+			'<table>' +
+				'<tableRow>' +
+					'<tableCell><paragraph></paragraph></tableCell>' +
+				'</tableRow>' +
+			'</table>'
+		) );
+	} );
+} );

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

@@ -12,7 +12,7 @@ import TableEditing from '../../src/tableediting';
 import { formatTable, formattedModelTable, modelTable } from './../_utils/utils';
 import UndoEditing from '@ckeditor/ckeditor5-undo/src/undoediting';
 
-describe( 'Table post-fixer', () => {
+describe( 'Table layout post-fixer', () => {
 	let editor, model, root;
 
 	beforeEach( () => {
@@ -181,19 +181,6 @@ describe( 'Table post-fixer', () => {
 		} );
 	} );
 
-	describe( 'on removing tableCell contents', () => {
-		it( 'should fix empty table cells', () => {
-			setModelData( model, modelTable( [ [ '11[]' ] ] ) );
-
-			model.change( writer => {
-				// Remove paragraph from table cell.
-				writer.remove( Range.createIn( root.getNodeByPath( [ 0, 0, 0 ] ) ) );
-			} );
-
-			expect( formatTable( getModelData( model, { withoutSelection: true } ) ) ).to.equal( formattedModelTable( [ [ '' ] ] ) );
-		} );
-	} );
-
 	describe( 'on collaboration', () => {
 		it( 'should add missing cells to columns (remove column vs insert row)', () => {
 			_testExternal(