Browse Source

Added fake selection options to `Selection#setTo()`.

Maciej Bukowski 8 years ago
parent
commit
d0500d6bbf

+ 32 - 41
packages/ckeditor5-engine/src/view/selection.js

@@ -67,14 +67,14 @@ export default class Selection {
 	 *		const paragraph = writer.createElement( 'paragraph' );
 	 *		selection.setTo( paragraph, 'on', { backward } );
 	 *
-	* @param {module:engine/view/selection~Selection|module:engine/view/position~Position|
-	 * Iterable.<module:engine/view/range~Range>|module:engine/view/range~Range|module:engine/view/item~Item|null} selectable
+	 * @param {module:engine/view/selection~Selection|module:engine/view/position~Position|
+	 * Iterable.<module:engine/view/range~Range>|module:engine/view/range~Range|module:engine/view/item~Item|null} [selectable=null]
 	 * @param {Object|Number|'before'|'end'|'after'|'on'|'in'} [optionsOrPlaceOrOffset]
 	 * @param {Boolean} [optionsOrPlaceOrOffset.backward]
 	 * @param {Object} [options]
 	 * @param {Boolean} [options.backward]
 	 */
-	constructor( selectable, optionsOrPlaceOrOffset, options ) {
+	constructor( selectable = null, optionsOrPlaceOrOffset, options ) {
 		/**
 		 * Stores all ranges that are selected.
 		 *
@@ -107,15 +107,13 @@ export default class Selection {
 		 */
 		this._fakeSelectionLabel = '';
 
-		if ( selectable ) {
-			this._setTo( selectable, optionsOrPlaceOrOffset, options );
-		}
+		this._setTo( selectable, optionsOrPlaceOrOffset, options );
 	}
 
 	/**
 	 * Returns true if selection instance is marked as `fake`.
 	 *
-	 * @see #_setFake
+	 * @see #_setTo
 	 * @returns {Boolean}
 	 */
 	get isFake() {
@@ -125,7 +123,7 @@ export default class Selection {
 	/**
 	 * Returns fake selection label.
 	 *
-	 * @see #_setFake
+	 * @see #_setTo
 	 * @returns {String}
 	 */
 	get fakeSelectionLabel() {
@@ -399,18 +397,6 @@ export default class Selection {
 		return ( nodeAfterStart instanceof Element && nodeAfterStart == nodeBeforeEnd ) ? nodeAfterStart : null;
 	}
 
-	/**
-	 * Removes all ranges that were added to the selection.
-	 *
-	 * @fires change
-	 */
-	_removeAllRanges() {
-		if ( this._ranges.length ) {
-			this._ranges = [];
-			this.fire( 'change' );
-		}
-	}
-
 	/**
 	 * Sets this selection's ranges and direction to the specified location based on the given
 	 * {@link module:engine/view/selection~Selection selection}, {@link module:engine/view/position~Position position},
@@ -452,21 +438,27 @@ export default class Selection {
 	 * @param {module:engine/view/selection~Selection|module:engine/view/position~Position|
 	 * Iterable.<module:engine/view/range~Range>|module:engine/view/range~Range|module:engine/view/item~Item|null} selectable
 	 * @param {Object|Number|'before'|'end'|'after'|'on'|'in'} [optionsOrPlaceOrOffset]
-	 * @param {Boolean} [optionsOrPlaceOrOffset.backward]
+	 * @param {Boolean} [optionsOrPlaceOrOffset.backward] Sets this selection as backward.
+	 * @param {Boolean} [optionsOrPlaceOrOffset.fake] Sets this selection instance to be marked as `fake`.
+	 * @param {String} [optionsOrPlaceOrOffset.label] Label for the fake selection.
 	 * @param {Object} [options]
-	 * @param {Boolean} [options.backward]
+	 * @param {Boolean} [options.backward] Sets this selection as backward.
+	 * @param {Boolean} [options.fake] Sets this selection instance to be marked as `fake`.
+	 * @param {String} [options.label] Label for the fake selection.
 	 */
 	_setTo( selectable, optionsOrPlaceOrOffset, options ) {
 		if ( selectable === null ) {
-			this._removeAllRanges();
+			this._setRanges( [] );
+			this._setFakeOptions( optionsOrPlaceOrOffset );
 		} else if ( selectable instanceof Selection ) {
-			this._isFake = selectable.isFake;
-			this._fakeSelectionLabel = selectable.fakeSelectionLabel;
 			this._setRanges( selectable.getRanges(), selectable.isBackward );
+			this._setFakeOptions( { fake: selectable.isFake, label: selectable.fakeSelectionLabel } );
 		} else if ( selectable instanceof Range ) {
-			this._setRanges( [ selectable ], !!optionsOrPlaceOrOffset && !!optionsOrPlaceOrOffset.backward );
+			this._setRanges( [ selectable ], optionsOrPlaceOrOffset && optionsOrPlaceOrOffset.backward );
+			this._setFakeOptions( optionsOrPlaceOrOffset );
 		} else if ( selectable instanceof Position ) {
 			this._setRanges( [ new Range( selectable ) ] );
+			this._setFakeOptions( optionsOrPlaceOrOffset );
 		} else if ( selectable instanceof Node ) {
 			const backward = !!options && !!options.backward;
 			let range;
@@ -488,9 +480,11 @@ export default class Selection {
 			}
 
 			this._setRanges( [ range ], backward );
+			this._setFakeOptions( options );
 		} else if ( isIterable( selectable ) ) {
 			// We assume that the selectable is an iterable of ranges.
-			this._setRanges( selectable, optionsOrPlaceOrOffset && !!optionsOrPlaceOrOffset.backward );
+			this._setRanges( selectable, optionsOrPlaceOrOffset && optionsOrPlaceOrOffset.backward );
+			this._setFakeOptions( optionsOrPlaceOrOffset );
 		} else {
 			/**
 			 * Cannot set selection to given place.
@@ -499,6 +493,8 @@ export default class Selection {
 			 */
 			throw new CKEditorError( 'view-selection-setTo-not-selectable: Cannot set selection to given place.' );
 		}
+
+		this.fire( 'change' );
 	}
 
 	/**
@@ -506,13 +502,12 @@ export default class Selection {
 	 * is treated like the last added range and is used to set {@link #anchor anchor} and {@link #focus focus}.
 	 * Accepts a flag describing in which way the selection is made.
 	 *
-	 * @protected
-	 * @fires change
+	 * @private
 	 * @param {Iterable.<module:engine/view/range~Range>} newRanges Iterable object of ranges to set.
-	 * @param {Boolean} [isLastBackward] Flag describing if last added range was selected forward - from start to end
+	 * @param {Boolean} [isLastBackward=false] Flag describing if last added range was selected forward - from start to end
 	 * (`false`) or backward - from end to start (`true`). Defaults to `false`.
 	 */
-	_setRanges( newRanges, isLastBackward ) {
+	_setRanges( newRanges, isLastBackward = false ) {
 		this._ranges = [];
 
 		for ( const range of newRanges ) {
@@ -520,7 +515,6 @@ export default class Selection {
 		}
 
 		this._lastRangeBackward = !!isLastBackward;
-		this.fire( 'change' );
 	}
 
 	/**
@@ -573,17 +567,14 @@ export default class Selection {
 	 * Additionally fake's selection label can be provided. It will be used to describe fake selection in DOM (and be
 	 * properly handled by screen readers).
 	 *
-	 * @protected
-	 * @fires change
-	 * @param {Boolean} [value=true] If set to true selection will be marked as `fake`.
-	 * @param {Object} [options] Additional options.
+	 * @private
+	 * @param {Object} [options] Options.
+	 * @param {Boolean} [options.fake] If set to true selection will be marked as `fake`.
 	 * @param {String} [options.label=''] Fake selection label.
 	 */
-	_setFake( value = true, options = {} ) {
-		this._isFake = value;
-		this._fakeSelectionLabel = value ? options.label || '' : '';
-
-		this.fire( 'change' );
+	_setFakeOptions( options = {} ) {
+		this._isFake = !!options.fake;
+		this._fakeSelectionLabel = options.fake ? options.label || '' : '';
 	}
 
 	/**

+ 75 - 102
packages/ckeditor5-engine/tests/view/selection.js

@@ -93,8 +93,7 @@ describe( 'Selection', () => {
 		} );
 
 		it( 'should be able to create a fake selection from the other fake selection', () => {
-			const otherSelection = new Selection( [ range2, range3 ], true );
-			otherSelection._setFake( true, { label: 'foo bar baz' } );
+			const otherSelection = new Selection( [ range2, range3 ], { fake: true, label: 'foo bar baz' } );
 			const selection = new Selection( otherSelection );
 
 			expect( selection.isFake ).to.be.true;
@@ -510,26 +509,21 @@ describe( 'Selection', () => {
 		} );
 
 		it( 'should return false if one selection is fake', () => {
-			const otherSelection = new Selection();
-			otherSelection._setFake( true );
+			const otherSelection = new Selection( null, { fake: true } );
 
 			expect( selection.isEqual( otherSelection ) ).to.be.false;
 		} );
 
 		it( 'should return true if both selection are fake', () => {
-			const otherSelection = new Selection( [ range1 ] );
-			otherSelection._setFake( true );
-			selection._setFake( true );
-			selection._setTo( range1 );
+			const otherSelection = new Selection( range1, { fake: true } );
+			selection._setTo( range1, { fake: true } );
 
 			expect( selection.isEqual( otherSelection ) ).to.be.true;
 		} );
 
 		it( 'should return false if both selection are fake but have different label', () => {
-			const otherSelection = new Selection( [ range1 ] );
-			otherSelection._setFake( true, { label: 'foo bar baz' } );
-			selection._setFake( true );
-			selection._setTo( range1 );
+			const otherSelection = new Selection( [ range1 ], { fake: true, label: 'foo bar baz' } );
+			selection._setTo( range1, { fake: true, label: 'foo' } );
 
 			expect( selection.isEqual( otherSelection ) ).to.be.false;
 		} );
@@ -609,38 +603,6 @@ describe( 'Selection', () => {
 		} );
 	} );
 
-	describe( '_setRanges()', () => {
-		it( 'should throw an error when range is invalid', () => {
-			expect( () => {
-				selection._setRanges( [ { invalid: 'range' } ] );
-			} ).to.throw( CKEditorError, 'view-selection-invalid-range: Invalid Range.' );
-		} );
-
-		it( 'should add ranges and fire change event', done => {
-			selection._setTo( range1 );
-
-			selection.once( 'change', () => {
-				expect( selection.rangeCount ).to.equal( 2 );
-				expect( selection._ranges[ 0 ].isEqual( range2 ) ).to.be.true;
-				expect( selection._ranges[ 0 ] ).is.not.equal( range2 );
-				expect( selection._ranges[ 1 ].isEqual( range3 ) ).to.be.true;
-				expect( selection._ranges[ 1 ] ).is.not.equal( range3 );
-				done();
-			} );
-
-			selection._setRanges( [ range2, range3 ] );
-		} );
-
-		it( 'should throw when range is intersecting with already added range', () => {
-			const text = el.getChild( 0 );
-			const range2 = Range.createFromParentsAndOffsets( text, 7, text, 15 );
-
-			expect( () => {
-				selection._setRanges( [ range1, range2 ] );
-			} ).to.throw( CKEditorError, 'view-selection-range-intersects' );
-		} );
-	} );
-
 	describe( '_setTo()', () => {
 		describe( 'simple scenarios', () => {
 			it( 'should set selection ranges from the given selection', () => {
@@ -659,39 +621,27 @@ describe( 'Selection', () => {
 				expect( selection.anchor.isEqual( range3.end ) ).to.be.true;
 			} );
 
-			it( 'should set selection on the given Range using _setRanges method', () => {
-				const spy = sinon.spy( selection, '_setRanges' );
-
+			it( 'should set selection on the given Range', () => {
 				selection._setTo( range1 );
 
 				expect( Array.from( selection.getRanges() ) ).to.deep.equal( [ range1 ] );
 				expect( selection.isBackward ).to.be.false;
-				expect( selection._setRanges.calledOnce ).to.be.true;
-				spy.restore();
 			} );
 
-			it( 'should set selection on the given iterable of Ranges using _setRanges method', () => {
-				const spy = sinon.spy( selection, '_setRanges' );
-
+			it( 'should set selection on the given iterable of Ranges', () => {
 				selection._setTo( new Set( [ range1, range2 ] ) );
 
 				expect( Array.from( selection.getRanges() ) ).to.deep.equal( [ range1, range2 ] );
 				expect( selection.isBackward ).to.be.false;
-				expect( selection._setRanges.calledOnce ).to.be.true;
-				spy.restore();
 			} );
 
-			it( 'should set collapsed selection on the given Position using _setRanges method', () => {
-				const spy = sinon.spy( selection, '_setRanges' );
-
+			it( 'should set collapsed selection on the given Position', () => {
 				selection._setTo( range1.start );
 
 				expect( Array.from( selection.getRanges() ).length ).to.equal( 1 );
 				expect( Array.from( selection.getRanges() )[ 0 ].start ).to.deep.equal( range1.start );
 				expect( selection.isBackward ).to.be.false;
 				expect( selection.isCollapsed ).to.be.true;
-				expect( selection._setRanges.calledOnce ).to.be.true;
-				spy.restore();
 			} );
 
 			it( 'should fire change event', done => {
@@ -707,9 +657,8 @@ describe( 'Selection', () => {
 			} );
 
 			it( 'should set fake state and label', () => {
-				const otherSelection = new Selection();
 				const label = 'foo bar baz';
-				otherSelection._setFake( true, { label } );
+				const otherSelection = new Selection( null, { fake: true, label } );
 				selection._setTo( otherSelection );
 
 				expect( selection.isFake ).to.be.true;
@@ -869,6 +818,71 @@ describe( 'Selection', () => {
 				expect( fireSpy.notCalled ).to.be.true;
 			} );
 		} );
+
+		describe( 'setting fake selection', () => {
+			it( 'should allow to set selection to fake', () => {
+				selection._setTo( range1, { fake: true } );
+
+				expect( selection.isFake ).to.be.true;
+			} );
+
+			it( 'should allow to set fake selection label', () => {
+				const label = 'foo bar baz';
+				selection._setTo( range1, { fake: true, label } );
+
+				expect( selection.fakeSelectionLabel ).to.equal( label );
+			} );
+
+			it( 'should not set label when set to false', () => {
+				const label = 'foo bar baz';
+				selection._setTo( range1, { fake: false, label } );
+
+				expect( selection.fakeSelectionLabel ).to.equal( '' );
+			} );
+
+			it( 'should reset label when set to false', () => {
+				const label = 'foo bar baz';
+				selection._setTo( range1, { fake: true, label } );
+				selection._setTo( range1 );
+
+				expect( selection.fakeSelectionLabel ).to.equal( '' );
+			} );
+
+			it( 'should fire change event', done => {
+				selection.once( 'change', () => {
+					expect( selection.isFake ).to.be.true;
+					expect( selection.fakeSelectionLabel ).to.equal( 'foo bar baz' );
+
+					done();
+				} );
+
+				selection._setTo( range1, { fake: true, label: 'foo bar baz' } );
+			} );
+
+			it( 'should be possible to create an empty fake selection', () => {
+				selection._setTo( null, { fake: true, label: 'foo bar baz' } );
+
+				expect( selection.fakeSelectionLabel ).to.equal( 'foo bar baz' );
+				expect( selection.isFake ).to.be.true;
+			} );
+		} );
+
+		describe( 'throwing errors', () => {
+			it( 'should throw an error when range is invalid', () => {
+				expect( () => {
+					selection._setTo( [ { invalid: 'range' } ] );
+				} ).to.throw( CKEditorError, 'view-selection-invalid-range: Invalid Range.' );
+			} );
+
+			it( 'should throw when range is intersecting with already added range', () => {
+				const text = el.getChild( 0 );
+				const range2 = Range.createFromParentsAndOffsets( text, 7, text, 15 );
+
+				expect( () => {
+					selection._setTo( [ range1, range2 ] );
+				} ).to.throw( CKEditorError, 'view-selection-range-intersects' );
+			} );
+		} );
 	} );
 
 	describe( 'getEditableElement()', () => {
@@ -901,47 +915,6 @@ describe( 'Selection', () => {
 		} );
 	} );
 
-	describe( '_setFake()', () => {
-		it( 'should allow to set selection to fake', () => {
-			selection._setFake( true );
-
-			expect( selection.isFake ).to.be.true;
-		} );
-
-		it( 'should allow to set fake selection label', () => {
-			const label = 'foo bar baz';
-			selection._setFake( true, { label } );
-
-			expect( selection.fakeSelectionLabel ).to.equal( label );
-		} );
-
-		it( 'should not set label when set to false', () => {
-			const label = 'foo bar baz';
-			selection._setFake( false, { label } );
-
-			expect( selection.fakeSelectionLabel ).to.equal( '' );
-		} );
-
-		it( 'should reset label when set to false', () => {
-			const label = 'foo bar baz';
-			selection._setFake( true, { label } );
-			selection._setFake( false );
-
-			expect( selection.fakeSelectionLabel ).to.equal( '' );
-		} );
-
-		it( 'should fire change event', done => {
-			selection.once( 'change', () => {
-				expect( selection.isFake ).to.be.true;
-				expect( selection.fakeSelectionLabel ).to.equal( 'foo bar baz' );
-
-				done();
-			} );
-
-			selection._setFake( true, { label: 'foo bar baz' } );
-		} );
-	} );
-
 	describe( 'getSelectedElement()', () => {
 		it( 'should return selected element', () => {
 			const { selection, view } = parse( 'foo [<b>bar</b>] baz' );