Bläddra i källkod

Changed: Improve table post-fixer after removing a table cell (for undo).

Maciej Gołaszewski 7 år sedan
förälder
incheckning
06e932a2af

+ 112 - 26
packages/ckeditor5-table/src/tableediting.js

@@ -247,53 +247,139 @@ export default class TableEditing extends Plugin {
 }
 }
 
 
 function injectTablePostFixer( model, tableUtils ) {
 function injectTablePostFixer( model, tableUtils ) {
-	model.document.registerPostFixer( writer => fixTableLayout( writer, model, tableUtils ) );
+	model.document.registerPostFixer( writer => tablePostFixer( writer, model, tableUtils ) );
 }
 }
 
 
-function fixTableLayout( writer, model, tableUtils ) {
+function tablePostFixer( writer, model, tableUtils ) {
 	const changes = model.document.differ.getChanges();
 	const changes = model.document.differ.getChanges();
 
 
 	let wasFixed = false;
 	let wasFixed = false;
 
 
+	const tableRowsOfRemovedCells = [];
+
 	for ( const entry of changes ) {
 	for ( const entry of changes ) {
-		if ( entry.type == 'insert' && entry.name == 'table' ) {
-			const table = entry.position.nodeAfter;
+		if ( entry.type == 'insert' ) {
+			let table;
+
+			if ( entry.name == 'table' ) {
+				table = entry.position.nodeAfter;
+			}
+
+			if ( entry.name == 'tableRow' ) {
+				const tableRow = entry.position.nodeAfter;
+				table = tableRow.parent;
+			}
+
+			if ( table ) {
+				wasFixed = fixTableOnInsert( tableUtils, table, writer, wasFixed );
+			}
+		}
+
+		if ( entry.type == 'remove' && entry.name == 'tableCell' ) {
+			const tableRow = entry.position.parent;
+
+			tableRowsOfRemovedCells.push( tableRow );
+		}
+	}
+
+	wasFixed = fixTableOnRemoveCells( tableRowsOfRemovedCells, writer ) || wasFixed;
+
+	return wasFixed;
+}
+
+function getRowsLengths( table ) {
+	const lengths = {};
+
+	for ( const data of new TableWalker( table ) ) {
+		const row = data.row;
+		const column = data.column;
+		const colspan = data.colspan;
+
+		if ( !lengths[ row ] ) {
+			// Le first row - the first column will be current width including rowspanned cells.
+			lengths[ row ] = column;
+		}
+
+		lengths[ row ] += colspan;
+	}
+
+	return lengths;
+}
 
 
-			const lengths = {};
+function fixTableOnInsert( tableUtils, table, writer, wasFixed ) {
+	const tableSize = tableUtils.getColumns( table );
 
 
-			const tableSize = tableUtils.getColumns( table );
+	const lengths = getRowsLengths( table );
 
 
-			for ( const data of new TableWalker( table ) ) {
-				const row = data.row;
-				const column = data.column;
-				const colspan = data.colspan;
+	const isValid = Object.values( lengths ).every( length => length === tableSize );
 
 
-				if ( !lengths[ row ] ) {
-					// Le first row - the first column will be current width including rowspanned cells.
-					lengths[ row ] = column;
+	if ( !isValid ) {
+		const maxColumns = Object.values( lengths ).reduce( ( prev, current ) => current > prev ? current : prev, 0 );
+
+		for ( const [ rowIndex, size ] of Object.entries( lengths ) ) {
+			const columnsToInsert = maxColumns - size;
+
+			if ( columnsToInsert ) {
+				for ( let i = 0; i < columnsToInsert; i++ ) {
+					writer.insertElement( 'tableCell', Position.createAt( table.getChild( rowIndex ), 'end' ) );
 				}
 				}
 
 
-				lengths[ row ] += colspan;
+				wasFixed = true;
 			}
 			}
+		}
+	}
+
+	return wasFixed;
+}
+
+function fixTableOnRemoveCells( tableRowsToCheck, writer ) {
+	let wasFixed = false;
+
+	if ( !tableRowsToCheck.length ) {
+		return wasFixed;
+	}
+
+	const indexes = tableRowsToCheck.map( tableRow => tableRow.index );
 
 
-			const isValid = Object.values( lengths ).every( length => length === tableSize );
+	const table = tableRowsToCheck[ 0 ].parent;
+	const allIndexes = [ ...table.getChildren() ].map( tableRow => tableRow.index );
 
 
-			if ( !isValid ) {
-				const maxColumns = Object.values( lengths ).reduce( ( prev, current ) => current > prev ? current : prev, 0 );
+	const other = allIndexes.filter( index => !indexes.includes( index ) );
 
 
-				for ( const [ rowIndex, size ] of Object.entries( lengths ) ) {
-					const columnsToInsert = maxColumns - size;
+	const lengths = getRowsLengths( table );
 
 
-					if ( columnsToInsert ) {
-						for ( let i = 0; i < columnsToInsert; i++ ) {
-							writer.insertElement( 'tableCell', Position.createAt( table.getChild( rowIndex ), 'end' ) );
-						}
+	const areSame = indexes.every( index => lengths[ index ] === lengths[ 0 ] );
 
 
-						wasFixed = true;
-					}
+	if ( areSame ) {
+		const properLength = lengths[ 0 ];
+
+		other.map( index => {
+			let length = lengths[ index ];
+
+			const tableRow = table.getChild( index );
+
+			while ( length > properLength ) {
+				const tableCell = tableRow.getChild( tableRow.childCount - 1 );
+
+				const howMany = length - properLength;
+
+				const colspan = parseInt( tableCell.getAttribute( 'colspan' ) || 1 );
+
+				if ( colspan > howMany ) {
+					writer.setAttribute( 'colspan', colspan - howMany, tableCell );
+
+					length = properLength;
+				} else {
+					writer.remove( tableCell );
+
+					length -= colspan;
 				}
 				}
+
+				length = properLength - 10;
+
+				wasFixed = true;
 			}
 			}
-		}
+		} );
 	}
 	}
 
 
 	return wasFixed;
 	return wasFixed;

+ 1 - 1
packages/ckeditor5-table/tests/_utils/utils.js

@@ -25,7 +25,7 @@
  *			contents: 'foo' // text contents of a cell
  *			contents: 'foo' // text contents of a cell
  *		};
  *		};
  *
  *
- * @param {Array.<String>} tableData
+ * @param {Array.<Array.<String>|Object>} tableData
  * @param {Object} [attributes] Optional table attributes: `headingRows` and `headingColumns`.
  * @param {Object} [attributes] Optional table attributes: `headingRows` and `headingColumns`.
  *
  *
  * @returns {String}
  * @returns {String}

+ 118 - 4
packages/ckeditor5-table/tests/tableediting.js

@@ -361,6 +361,12 @@ describe( 'TableEditing', () => {
 	} );
 	} );
 
 
 	describe( 'post-fixer', () => {
 	describe( 'post-fixer', () => {
+		let root;
+
+		beforeEach( () => {
+			root = model.document.getRoot();
+		} );
+
 		it( 'should add missing columns to a tableRows that are shorter then longest table row', () => {
 		it( 'should add missing columns to a tableRows that are shorter then longest table row', () => {
 			const parsed = parse( modelTable( [
 			const parsed = parse( modelTable( [
 				[ '00' ],
 				[ '00' ],
@@ -368,8 +374,6 @@ describe( 'TableEditing', () => {
 				[ '20', '21' ]
 				[ '20', '21' ]
 			] ), model.schema );
 			] ), model.schema );
 
 
-			const root = model.document.getRoot();
-
 			model.change( writer => {
 			model.change( writer => {
 				writer.remove( Range.createIn( root ) );
 				writer.remove( Range.createIn( root ) );
 				writer.insert( parsed, root );
 				writer.insert( parsed, root );
@@ -389,8 +393,6 @@ describe( 'TableEditing', () => {
 				[ '21', '22' ]
 				[ '21', '22' ]
 			] ), model.schema );
 			] ), model.schema );
 
 
-			const root = model.document.getRoot();
-
 			model.change( writer => {
 			model.change( writer => {
 				writer.remove( Range.createIn( root ) );
 				writer.remove( Range.createIn( root ) );
 				writer.insert( parsed, root );
 				writer.insert( parsed, root );
@@ -402,5 +404,117 @@ describe( 'TableEditing', () => {
 				[ '21', '22', '', '', '' ]
 				[ '21', '22', '', '', '' ]
 			] ) );
 			] ) );
 		} );
 		} );
+
+		// Client A: insert row. Client B: insert column.
+		it( 'should fix missing insert column before insert row', () => {
+			setModelData( model, modelTable( [
+				[ '00[]', '01', '02' ],
+				[ '10', '11', '12' ],
+				[ '20', '21', '22' ]
+			] ) );
+
+			const table = root.getChild( 0 );
+
+			// Insert column:
+			model.change( writer => {
+				for ( const tableRow of table.getChildren() ) {
+					const tableCell = writer.createElement( 'tableCell' );
+					writer.insert( tableCell, tableRow, 1 );
+					writer.insertText( 'x', tableCell );
+				}
+			} );
+
+			// Insert table row
+			model.change( writer => {
+				const parsedTable = parse(
+					modelTable( [ [ 'a', 'b', 'c' ] ] ),
+					model.schema
+				);
+
+				writer.insert( parsedTable.getChild( 0 ), table, 2 );
+			} );
+
+			expect( formatTable( getModelData( model, { withoutSelection: true } ) ) ).to.equal( formattedModelTable( [
+				[ '00', 'x', '01', '02' ],
+				[ '10', 'x', '11', '12' ],
+				[ 'a', 'b', 'c', '' ],
+				[ '20', 'x', '21', '22' ]
+			] ) );
+		} );
+
+		// Client A: insert row. Client B: insert column.
+		it( 'should fix (undo of add tableRow & add tableCell)', () => {
+			setModelData( model, modelTable( [
+				[ '00', 'x', '01', '02' ],
+				[ '10', 'x', '11', '12' ],
+				[ 'a', 'b', 'c', '' ],
+				[ '20', 'x', '21', '22' ]
+			] ) );
+
+			const table = root.getChild( 0 );
+
+			// Insert column:
+			model.change( writer => {
+				[ 0, 1, 3 ].map( index => writer.remove( table.getChild( index ).getChild( 1 ) ) );
+			} );
+
+			expect( formatTable( getModelData( model, { withoutSelection: true } ) ) ).to.equal( formattedModelTable( [
+				[ '00', '01', '02' ],
+				[ '10', '11', '12' ],
+				[ 'a', 'b', 'c' ],
+				[ '20', '21', '22' ]
+			] ) );
+		} );
+
+		// Client A: insert row. Client B: insert column.
+		it( 'should fix (undo of add tableRow & add tableCell) fff', () => {
+			setModelData( model, modelTable( [
+				[ '00', 'x', '01', '02' ],
+				[ '10', 'x', '11', '12' ],
+				[ 'a', { colspan: 3, contents: 'b' } ],
+				[ '20', 'x', '21', '22' ]
+			] ) );
+
+			const table = root.getChild( 0 );
+
+			// Insert column:
+			model.change( writer => {
+				[ 0, 1, 3 ].map( index => writer.remove( table.getChild( index ).getChild( 1 ) ) );
+			} );
+
+			expect( formatTable( getModelData( model, { withoutSelection: true } ) ) ).to.equal( formattedModelTable( [
+				[ '00', '01', '02' ],
+				[ '10', '11', '12' ],
+				[ 'a', { colspan: 2, contents: 'b' } ],
+				[ '20', '21', '22' ]
+			] ) );
+		} );
+
+		// Client A: insert row. Client B: insert column.
+		it( 'should not fix (undo of add tableRow & add tableCell) - OK', () => {
+			setModelData( model, modelTable( [
+				[ '00', 'x', '01', '02' ],
+				[ '10', 'x', '11', '12' ],
+				[ 'a', 'b', 'c', '' ],
+				[ '20', 'x', '21', '22' ]
+			] ) );
+
+			const table = root.getChild( 0 );
+
+			// Insert column:
+			model.change( writer => {
+				[ 0, 1, 3 ].map( index => writer.remove( table.getChild( index ).getChild( 1 ) ) );
+
+				// Remove one more cell... (shouldn't occur??)
+				writer.remove( table.getChild( 0 ).getChild( 1 ) );
+			} );
+
+			expect( formatTable( getModelData( model, { withoutSelection: true } ) ) ).to.equal( formattedModelTable( [
+				[ '00', '02' ],
+				[ '10', '11', '12' ],
+				[ 'a', 'b', 'c', '' ],
+				[ '20', '21', '22' ]
+			] ) );
+		} );
 	} );
 	} );
 } );
 } );