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

Restoring selection ranges on undo without the need for schema customization.

Kuba Niegowski 5 лет назад
Родитель
Сommit
c71aea2c05

+ 3 - 3
packages/ckeditor5-engine/src/model/range.js

@@ -290,13 +290,13 @@ export default class Range {
 	 *			model.createPositionFromPath( root, [ 1 ] ),
 	 *			model.createPositionFromPath( root, [ 2 ] )
  	 *		);
-	 *		let transformed = range.getSum( otherRange ); // null - ranges have no common part
+	 *		let transformed = range.getJoined( otherRange ); // null - ranges have no common part
 	 *
 	 *		otherRange = model.createRange(
 	 *			model.createPositionFromPath( root, [ 3 ] ),
 	 *			model.createPositionFromPath( root, [ 5 ] )
 	 *		);
-	 *		transformed = range.getSum( otherRange ); // range from [ 2, 7 ] to [ 5 ]
+	 *		transformed = range.getJoined( otherRange ); // range from [ 2, 7 ] to [ 5 ]
 	 *
 	 * @param {module:engine/model/range~Range} otherRange Range to sum with.
 	 * @param {Boolean} [loose=false] Whether the sum intersection check is loose or strict. If the check is strict (`false`),
@@ -304,7 +304,7 @@ export default class Range {
 	 * current range.
 	 * @returns {module:engine/model/range~Range|null} A sum of given ranges or `null` if ranges have no common part.
 	 */
-	getSum( otherRange, loose = false ) {
+	getJoined( otherRange, loose = false ) {
 		let shouldSum = this.isIntersecting( otherRange );
 
 		if ( loose && !shouldSum ) {

+ 3 - 3
packages/ckeditor5-engine/src/model/schema.js

@@ -1490,7 +1490,7 @@ function compileBaseItemRule( sourceItemRules, itemName ) {
 		inheritTypesFrom: []
 	};
 
-	copyTypesAndFlags( sourceItemRules, itemRule );
+	copyTypes( sourceItemRules, itemRule );
 
 	copyProperty( sourceItemRules, itemRule, 'allowIn' );
 	copyProperty( sourceItemRules, itemRule, 'allowContentOf' );
@@ -1585,9 +1585,9 @@ function cleanUpAllowAttributes( compiledDefinitions, itemName ) {
 	itemRule.allowAttributes = Array.from( new Set( itemRule.allowAttributes ) );
 }
 
-function copyTypesAndFlags( sourceItemRules, itemRule ) {
+function copyTypes( sourceItemRules, itemRule ) {
 	for ( const sourceItemRule of sourceItemRules ) {
-		const typeNames = Object.keys( sourceItemRule ).filter( name => name.startsWith( 'is' ) || name == 'allowMultiRangeSelection' );
+		const typeNames = Object.keys( sourceItemRule ).filter( name => name.startsWith( 'is' ) );
 
 		for ( const name of typeNames ) {
 			itemRule[ name ] = sourceItemRule[ name ];

+ 11 - 11
packages/ckeditor5-engine/tests/model/range.js

@@ -789,7 +789,7 @@ describe( 'Range', () => {
 		} );
 	} );
 
-	describe( 'getSum()', () => {
+	describe( 'getJoined()', () => {
 		let range;
 
 		beforeEach( () => {
@@ -798,28 +798,28 @@ describe( 'Range', () => {
 
 		it( 'should return null if ranges do not intersect', () => {
 			const otherRange = new Range( new Position( root, [ 5, 4 ] ), new Position( root, [ 7 ] ) );
-			const sum = range.getSum( otherRange );
+			const sum = range.getJoined( otherRange );
 
 			expect( sum ).to.be.null;
 		} );
 
 		it( 'should return a range spanning both of the ranges - original range contains the other range', () => {
 			const otherRange = new Range( new Position( root, [ 4 ] ), new Position( root, [ 5 ] ) );
-			const sum = range.getSum( otherRange );
+			const sum = range.getJoined( otherRange );
 
 			expect( sum.isEqual( range ) ).to.be.true;
 		} );
 
 		it( 'should return a range spanning both of the ranges - original range is contained by the other range', () => {
 			const otherRange = new Range( new Position( root, [ 3 ] ), new Position( root, [ 6 ] ) );
-			const sum = range.getSum( otherRange );
+			const sum = range.getJoined( otherRange );
 
 			expect( sum.isEqual( otherRange ) ).to.be.true;
 		} );
 
 		it( 'should return a range spanning both of the ranges - original range intersects with the other range', () => {
 			const otherRange = new Range( new Position( root, [ 3 ] ), new Position( root, [ 4, 7 ] ) );
-			const sum = range.getSum( otherRange );
+			const sum = range.getJoined( otherRange );
 
 			expect( sum.start.path ).to.deep.equal( [ 3 ] );
 			expect( sum.end.path ).to.deep.equal( [ 5, 4 ] );
@@ -827,7 +827,7 @@ describe( 'Range', () => {
 
 		it( 'should return a range spanning both of the ranges if both ranges are equal', () => {
 			const otherRange = range.clone();
-			const sum = range.getSum( otherRange );
+			const sum = range.getJoined( otherRange );
 
 			expect( sum.isEqual( range ) ).to.be.true;
 		} );
@@ -840,7 +840,7 @@ describe( 'Range', () => {
 			it( 'should return null if ranges are not intersecting nor touching', () => {
 				const range = new Range( new Position( root, [ 0, 1 ] ), new Position( root, [ 3 ] ) );
 				const otherRange = new Range( new Position( root, [ 3, 1 ] ), new Position( root, [ 3, 2 ] ) );
-				const sum = range.getSum( otherRange, true );
+				const sum = range.getJoined( otherRange, true );
 
 				expect( sum ).to.be.null;
 			} );
@@ -848,7 +848,7 @@ describe( 'Range', () => {
 			it( 'should return a range spanning both of the ranges - original range end is equal to other range start position', () => {
 				const range = new Range( new Position( root, [ 0, 1 ] ), new Position( root, [ 3 ] ) );
 				const otherRange = new Range( new Position( root, [ 3 ] ), new Position( root, [ 3, 2 ] ) );
-				const sum = range.getSum( otherRange, true );
+				const sum = range.getJoined( otherRange, true );
 
 				expect( sum.start.path ).to.deep.equal( [ 0, 1 ] );
 				expect( sum.end.path ).to.deep.equal( [ 3, 2 ] );
@@ -857,7 +857,7 @@ describe( 'Range', () => {
 			it( 'should return a range spanning both of the ranges - original range start is equal to other range end position', () => {
 				const range = new Range( new Position( root, [ 3 ] ), new Position( root, [ 3, 2 ] ) );
 				const otherRange = new Range( new Position( root, [ 0, 1 ] ), new Position( root, [ 3 ] ) );
-				const sum = range.getSum( otherRange, true );
+				const sum = range.getJoined( otherRange, true );
 
 				expect( sum.start.path ).to.deep.equal( [ 0, 1 ] );
 				expect( sum.end.path ).to.deep.equal( [ 3, 2 ] );
@@ -866,7 +866,7 @@ describe( 'Range', () => {
 			it( 'should return a range spanning both of the ranges - original range is touching other range on the right side', () => {
 				const range = new Range( new Position( root, [ 0, 1 ] ), new Position( root, [ 3 ] ) );
 				const otherRange = new Range( new Position( root, [ 3, 0 ] ), new Position( root, [ 3, 2 ] ) );
-				const sum = range.getSum( otherRange, true );
+				const sum = range.getJoined( otherRange, true );
 
 				expect( sum.start.path ).to.deep.equal( [ 0, 1 ] );
 				expect( sum.end.path ).to.deep.equal( [ 3, 2 ] );
@@ -875,7 +875,7 @@ describe( 'Range', () => {
 			it( 'should return a range spanning both of the ranges - original range is touching other range on the left side', () => {
 				const range = new Range( new Position( root, [ 1, 0 ] ), new Position( root, [ 3, 2 ] ) );
 				const otherRange = new Range( new Position( root, [ 0, 1 ] ), new Position( root, [ 0, 2 ] ) );
-				const sum = range.getSum( otherRange, true );
+				const sum = range.getJoined( otherRange, true );
 
 				expect( sum.start.path ).to.deep.equal( [ 0, 1 ] );
 				expect( sum.end.path ).to.deep.equal( [ 3, 2 ] );

+ 0 - 12
packages/ckeditor5-engine/tests/model/schema.js

@@ -259,18 +259,6 @@ describe( 'Schema', () => {
 				allowAttributes: [ 'foo' ]
 			} );
 		} );
-
-		it( 'handles allowMultiRangeSelection flag', () => {
-			schema.register( 'foo', {
-				isBlock: true,
-				allowMultiRangeSelection: true
-			} );
-
-			const definitions = schema.getDefinitions();
-
-			expect( definitions.foo ).to.have.property( 'isBlock', true );
-			expect( definitions.foo ).to.have.property( 'allowMultiRangeSelection', true );
-		} );
 	} );
 
 	describe( 'getDefinition()', () => {

+ 0 - 1
packages/ckeditor5-table/tests/tableediting.js

@@ -68,7 +68,6 @@ describe( 'TableEditing', () => {
 		// Table cell:
 		expect( model.schema.isRegistered( 'tableCell' ) ).to.be.true;
 		expect( model.schema.isLimit( 'tableCell' ) ).to.be.true;
-		expect( model.schema.getDefinition( 'tableCell' ) ).to.have.property( 'allowMultiRangeSelection', true );
 
 		expect( model.schema.checkChild( [ '$root' ], 'tableCell' ) ).to.be.false;
 		expect( model.schema.checkChild( [ 'table' ], 'tableCell' ) ).to.be.false;

+ 32 - 27
packages/ckeditor5-undo/src/basecommand.js

@@ -89,22 +89,32 @@ export default class BaseCommand extends Command {
 		const model = this.editor.model;
 		const document = model.document;
 
-		const elementsExpectingSeparateSelectionRange = Object.values( model.schema.getDefinitions() )
-			.filter( ( { allowMultiRangeSelection } ) => allowMultiRangeSelection )
-			.map( ( { name } ) => name );
-
-		// Transform all ranges from the restored selection.
-		const selectionRanges = ranges
-			.flatMap( range => range.getTransformedByOperations( operations ) )
-			.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 );
+		// This will keep the transformed selection ranges.
+		const selectionRanges = [];
+
+		const transformedRangeGroups = ranges.map( range => range.getTransformedByOperations( operations ) );
+		const allRanges = transformedRangeGroups.flat();
+
+		for ( const rangeGroup of transformedRangeGroups ) {
+			// While transforming there could appear ranges that are contained by other ranges, we shall ignore them.
+			const newRanges = rangeGroup.filter( range => !allRanges.some( otherRange => (
+				otherRange !== range && otherRange.containsRange( range, true )
+			) ) );
+
+			// After the range got transformed, we have an array of ranges. Some of those
+			// ranges may be "touching" -- they can be next to each other and could be merged.
+			normalizeRanges( newRanges );
+
+			// For each `range` from `ranges`, we take only one transformed range.
+			// This is because we want to prevent situation where single-range selection
+			// got transformed to multi-range selection. We will take the first range that
+			// is not in the graveyard.
+			const newRange = newRanges.find( range => range.root != document.graveyard );
+
+			if ( newRange ) {
+				selectionRanges.push( newRange );
+			}
+		}
 
 		// @if CK_DEBUG_ENGINE // console.log( `Restored selection by undo: ${ selectionRanges.join( ', ' ) }` );
 
@@ -171,23 +181,18 @@ export default class BaseCommand extends Command {
 // 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 ) {
+function normalizeRanges( ranges ) {
+	ranges.sort( ( a, b ) => a.start.isBefore( b.start ) ? -1 : 1 );
+
 	for ( let i = 1; i < ranges.length; i++ ) {
 		const previousRange = ranges[ i - 1 ];
-		const containedElement = elementExclusions && previousRange.getContainedElement();
-
-		// 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 ], !!elementExclusions );
+		const summedRange = previousRange.getJoined( ranges[ i ], true );
 
 		if ( summedRange ) {
 			// Replace the ranges on the list with the new joined range.
-			ranges.splice( --i, 2, summedRange );
+			i--;
+			ranges.splice( i, 2, summedRange );
 		}
 	}
 }