Ver código fonte

Merge pull request #417 from ckeditor/t/416

t/416: Selection isCollapsed should return true only if there is single collapsed range.
Szymon Kupś 9 anos atrás
pai
commit
280bb53cda

+ 19 - 6
packages/ckeditor5-engine/src/treemodel/selection.js

@@ -93,18 +93,31 @@ export default class Selection {
 	}
 
 	/**
-	 * Returns whether the selection is collapsed. Selection is collapsed when all it's ranges are collapsed.
+	 * Returns whether the selection is collapsed. Selection is collapsed when there is exactly one range which is
+	 * collapsed.
 	 *
 	 * @type {Boolean}
 	 */
 	get isCollapsed() {
-		for ( let i = 0; i < this._ranges.length; i++ ) {
-			if ( !this._ranges[ i ].isCollapsed ) {
-				return false;
-			}
+		const length = this._ranges.length;
+
+		if ( length === 0 ) {
+			// Default range is collapsed.
+			return true;
+		} else if ( length === 1 ) {
+			return this._ranges[ 0 ].isCollapsed;
+		} else {
+			return false;
 		}
+	}
 
-		return true;
+	/**
+	 * Returns number of ranges in selection.
+	 *
+	 * @type {Number}
+     */
+	get rangeCount() {
+		return this._ranges.length ? this._ranges.length : 1;
 	}
 
 	/**

+ 3 - 8
packages/ckeditor5-engine/src/treeview/selection.js

@@ -85,18 +85,13 @@ export default class Selection {
 	}
 
 	/**
-	 * Returns whether the selection is collapsed. Selection is collapsed when all it's ranges are collapsed.
+	 * Returns whether the selection is collapsed. Selection is collapsed when there is exactly one range which is
+	 * collapsed.
 	 *
 	 * @type {Boolean}
 	 */
 	get isCollapsed() {
-		for ( let range of this._ranges ) {
-			if ( !range.isCollapsed ) {
-				return false;
-			}
-		}
-
-		return true;
+		return this.rangeCount === 1 && this._ranges[ 0 ].isCollapsed;
 	}
 
 	/**

+ 24 - 3
packages/ckeditor5-engine/tests/treemodel/selection.js

@@ -66,20 +66,41 @@ describe( 'Selection', () => {
 	} );
 
 	describe( 'isCollapsed', () => {
-		it( 'should be true if all ranges are collapsed', () => {
+		it( 'should return true for default range', () => {
+			expect( selection.isCollapsed ).to.be.true;
+		} );
+
+		it( 'should return true when there is single collapsed ranges', () => {
 			selection.addRange( new Range( new Position( root, [ 0 ] ), new Position( root, [ 0 ] ) ) );
 
 			expect( selection.isCollapsed ).to.be.true;
 		} );
 
-		it( 'should be false when it has a range that is not collapsed', () => {
+		it( 'should return false when there are multiple ranges', () => {
+			selection.addRange( new Range( new Position( root, [ 0 ] ), new Position( root, [ 0 ] ) ) );
+			selection.addRange( new Range( new Position( root, [ 2 ] ), new Position( root, [ 2 ] ) ) );
+
+			expect( selection.isCollapsed ).to.be.false;
+		} );
+
+		it( 'should return false when there is not collapsed range', () => {
 			selection.addRange( range );
 
 			expect( selection.isCollapsed ).to.be.false;
+		} );
+	} );
+
+	describe( 'rangeCount', () => {
+		it( 'should return proper range count', () => {
+			expect( selection.rangeCount ).to.equal( 1 );
 
 			selection.addRange( new Range( new Position( root, [ 0 ] ), new Position( root, [ 0 ] ) ) );
 
-			expect( selection.isCollapsed ).to.be.false;
+			expect( selection.rangeCount ).to.equal( 1 );
+
+			selection.addRange( new Range( new Position( root, [ 2 ] ), new Position( root, [ 2 ] ) ) );
+
+			expect( selection.rangeCount ).to.equal( 2 );
 		} );
 	} );
 

+ 17 - 8
packages/ckeditor5-engine/tests/treeview/selection.js

@@ -83,31 +83,40 @@ describe( 'Selection', () => {
 	} );
 
 	describe( 'isCollapsed', () => {
-		it( 'should return true when all ranges are collapsed', () => {
-			const range1 = Range.createFromParentsAndOffsets( el, 5, el, 5 );
-			const range2 = Range.createFromParentsAndOffsets( el, 15, el, 15 );
-			selection.addRange( range1 );
-			selection.addRange( range2 );
+		it( 'should return true when there is single collapsed range', () => {
+			const range = Range.createFromParentsAndOffsets( el, 5, el, 5 );
+			selection.addRange( range );
 
 			expect( selection.isCollapsed ).to.be.true;
 		} );
 
-		it( 'should return false when not all ranges are collapsed', () => {
+		it( 'should return false when there are multiple ranges', () => {
 			const range1 = Range.createFromParentsAndOffsets( el, 5, el, 5 );
-			const range2 = Range.createFromParentsAndOffsets( el, 15, el, 16 );
+			const range2 = Range.createFromParentsAndOffsets( el, 15, el, 15 );
 			selection.addRange( range1 );
 			selection.addRange( range2 );
 
 			expect( selection.isCollapsed ).to.be.false;
 		} );
+
+		it( 'should return false when there is not collapsed range', () => {
+			const range = Range.createFromParentsAndOffsets( el, 15, el, 16 );
+			selection.addRange( range );
+
+			expect( selection.isCollapsed ).to.be.false;
+		} );
 	} );
 
 	describe( 'rangeCount', () => {
 		it( 'should return proper range count', () => {
 			expect( selection.rangeCount ).to.equal( 0 );
+
 			selection.addRange( range1 );
+
 			expect( selection.rangeCount ).to.equal( 1 );
+
 			selection.addRange( range2 );
+
 			expect( selection.rangeCount ).to.equal( 2 );
 		} );
 	} );
@@ -378,4 +387,4 @@ describe( 'Selection', () => {
 			expect( fireSpy.notCalled ).to.be.true;
 		} );
 	} );
-} );
+} );