Parcourir la source

Added test for undoing table multi cell merge.

Kuba Niegowski il y a 5 ans
Parent
commit
df81626bbc

+ 19 - 5
packages/ckeditor5-undo/src/basecommand.js

@@ -99,10 +99,14 @@ export default class BaseCommand extends Command {
 			.filter( range => range.root != document.graveyard )
 			.sort( ( a, b ) => a.start.isBefore( b.start ) ? -1 : 1 );
 
+		// Join ranges if they intersect. This is because of moving parts of document between different original ranges.
 		normalizeRanges( selectionRanges );
+
+		// Join ranges if they are touching one another but if element is not excluded from joining (i.e. tableCell).
+		// This is to avoid changing single-range selection into multi-range selection if it's not expected.
 		normalizeRanges( selectionRanges, elementsExpectingSeparateSelectionRange );
 
-		// @if CK_DEBUG_ENGINE // console.log( `Restored selection from undo: ${ selectionRanges.join( ', ' ) }` );
+		// @if CK_DEBUG_ENGINE // console.log( `Restored selection by undo: ${ selectionRanges.join( ', ' ) }` );
 
 		// `selectionRanges` may be empty if all ranges ended up in graveyard. If that is the case, do not restore selection.
 		if ( selectionRanges.length ) {
@@ -161,18 +165,28 @@ export default class BaseCommand extends Command {
 	}
 }
 
-function normalizeRanges( ranges, dontSumElements = null ) {
+// Normalizes list of ranges by joining intersecting ranges. It expects input ranges to be sorted.
+//
+// Performs 'loose' joining if second argument is provided (ranges are joined if they are 'touching' one another)
+// and additionally avoids merging ranges that fully-contain any of the elements provided in the exclusion list.
+//
+// @param {Array.<module:engine/model/range~Range>} ranges
+// @param {Array.<String>} [elementExclusions]
+//
+function normalizeRanges( ranges, elementExclusions = null ) {
 	for ( let i = 1; i < ranges.length; i++ ) {
 		const previousRange = ranges[ i - 1 ];
-		const containedElement = dontSumElements && previousRange.getContainedElement();
+		const containedElement = elementExclusions && previousRange.getContainedElement();
 
-		if ( containedElement && dontSumElements.includes( containedElement.name ) ) {
+		// Skip joining a range if the previous range fully contained an element from exclusion list.
+		if ( containedElement && elementExclusions.includes( containedElement.name ) ) {
 			continue;
 		}
 
-		const summedRange = previousRange.getSum( ranges[ i ], !!dontSumElements );
+		const summedRange = previousRange.getSum( ranges[ i ], !!elementExclusions );
 
 		if ( summedRange ) {
+			// Replace the ranges on the list with the new joined range.
 			ranges.splice( --i, 2, summedRange );
 		}
 	}

+ 86 - 7
packages/ckeditor5-undo/tests/undoediting-integration.js

@@ -15,6 +15,7 @@ import Typing from '@ckeditor/ckeditor5-typing/src/typing';
 import Enter from '@ckeditor/ckeditor5-enter/src/enter';
 import Clipboard from '@ckeditor/ckeditor5-clipboard/src/clipboard';
 import BoldEditing from '@ckeditor/ckeditor5-basic-styles/src/bold/boldediting';
+import TableEditing from '@ckeditor/ckeditor5-table/src/tableediting';
 
 import { setData, getData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model';
 
@@ -25,15 +26,16 @@ describe( 'UndoEditing integration', () => {
 		div = document.createElement( 'div' );
 		document.body.appendChild( div );
 
-		return ClassicEditor.create( div, { plugins: [ Paragraph, HeadingEditing, Typing, Enter, Clipboard, BoldEditing, UndoEditing ] } )
-			.then( newEditor => {
-				editor = newEditor;
+		return ClassicEditor.create( div, {
+			plugins: [ Paragraph, HeadingEditing, Typing, Enter, Clipboard, BoldEditing, UndoEditing, TableEditing ]
+		} ).then( newEditor => {
+			editor = newEditor;
 
-				model = editor.model;
-				doc = model.document;
+			model = editor.model;
+			doc = model.document;
 
-				root = doc.getRoot();
-			} );
+			root = doc.getRoot();
+		} );
 	} );
 
 	afterEach( () => {
@@ -1070,6 +1072,83 @@ describe( 'UndoEditing integration', () => {
 			expect( p.root ).to.equal( gy );
 			expect( p.getAttribute( 'bold' ) ).to.be.true;
 		} );
+
+		it( 'undo table cells merge', () => {
+			input(
+				'<table>' +
+					'<tableRow>' +
+						'<tableCell><paragraph>00</paragraph></tableCell>' +
+						'[<tableCell><paragraph>01</paragraph></tableCell>]' +
+						'[<tableCell><paragraph>02</paragraph></tableCell>]' +
+						'<tableCell><paragraph>03</paragraph></tableCell>' +
+					'</tableRow>' +
+					'<tableRow>' +
+						'<tableCell><paragraph>10</paragraph></tableCell>' +
+						'[<tableCell><paragraph>11</paragraph></tableCell>]' +
+						'[<tableCell><paragraph>12</paragraph></tableCell>]' +
+						'<tableCell><paragraph>13</paragraph></tableCell>' +
+					'</tableRow>' +
+					'<tableRow>' +
+						'<tableCell><paragraph>20</paragraph></tableCell>' +
+						'<tableCell><paragraph>21</paragraph></tableCell>' +
+						'<tableCell><paragraph>22</paragraph></tableCell>' +
+						'<tableCell><paragraph>23</paragraph></tableCell>' +
+					'</tableRow>' +
+				'</table>'
+			);
+
+			editor.execute( 'mergeTableCells' );
+
+			output(
+				'<table>' +
+					'<tableRow>' +
+						'<tableCell><paragraph>00</paragraph></tableCell>' +
+						'<tableCell colspan="2" rowspan="2">' +
+							'<paragraph>[01</paragraph>' +
+							'<paragraph>02</paragraph>' +
+							'<paragraph>11</paragraph>' +
+							'<paragraph>12]</paragraph>' +
+						'</tableCell>' +
+						'<tableCell><paragraph>03</paragraph></tableCell>' +
+					'</tableRow>' +
+					'<tableRow>' +
+						'<tableCell><paragraph>10</paragraph></tableCell>' +
+						'<tableCell><paragraph>13</paragraph></tableCell>' +
+					'</tableRow>' +
+					'<tableRow>' +
+						'<tableCell><paragraph>20</paragraph></tableCell>' +
+						'<tableCell><paragraph>21</paragraph></tableCell>' +
+						'<tableCell><paragraph>22</paragraph></tableCell>' +
+						'<tableCell><paragraph>23</paragraph></tableCell>' +
+					'</tableRow>' +
+				'</table>'
+			);
+
+			editor.execute( 'undo' );
+
+			output(
+				'<table>' +
+					'<tableRow>' +
+						'<tableCell><paragraph>00</paragraph></tableCell>' +
+						'[<tableCell><paragraph>01</paragraph></tableCell>]' +
+						'[<tableCell><paragraph>02</paragraph></tableCell>]' +
+						'<tableCell><paragraph>03</paragraph></tableCell>' +
+					'</tableRow>' +
+					'<tableRow>' +
+						'<tableCell><paragraph>10</paragraph></tableCell>' +
+						'[<tableCell><paragraph>11</paragraph></tableCell>]' +
+						'[<tableCell><paragraph>12</paragraph></tableCell>]' +
+						'<tableCell><paragraph>13</paragraph></tableCell>' +
+					'</tableRow>' +
+					'<tableRow>' +
+						'<tableCell><paragraph>20</paragraph></tableCell>' +
+						'<tableCell><paragraph>21</paragraph></tableCell>' +
+						'<tableCell><paragraph>22</paragraph></tableCell>' +
+						'<tableCell><paragraph>23</paragraph></tableCell>' +
+					'</tableRow>' +
+				'</table>'
+			);
+		} );
 	} );
 
 	it( 'postfixers should not add another undo step when fixing undo changes', () => {