Parcourir la source

Selection improvements.

Maciej Bukowski il y a 8 ans
Parent
commit
edca03b7dd

+ 0 - 8
packages/ckeditor5-engine/src/model/liveselection.js

@@ -155,14 +155,6 @@ export default class LiveSelection extends Selection {
 		return super.getLastRange() || this._document._getDefaultRange();
 	}
 
-	/**
-	 * @inheritDoc
-	 */
-	_addRange( range, isBackward = false ) {
-		super._addRange( range, isBackward );
-		this.refreshAttributes();
-	}
-
 	/**
 	 * @inheritDoc
 	 */

+ 15 - 80
packages/ckeditor5-engine/src/model/selection.js

@@ -21,6 +21,8 @@ import isIterable from '@ckeditor/ckeditor5-utils/src/isiterable';
  * `Selection` is a group of {@link module:engine/model/range~Range ranges} which has a direction specified by
  * {@link module:engine/model/selection~Selection#anchor anchor} and {@link module:engine/model/selection~Selection#focus focus}.
  * Additionally, `Selection` may have it's own attributes.
+ *
+ * @mixes {module:utils/emittermixin~EmitterMixin}
  */
 export default class Selection {
 	/**
@@ -35,7 +37,7 @@ export default class Selection {
 		 * Specifies whether the last added range was added as a backward or forward range.
 		 *
 		 * @private
-		 * @member {Boolean}
+		 * @type {Boolean}
 		 */
 		this._lastRangeBackward = false;
 
@@ -43,7 +45,7 @@ export default class Selection {
 		 * Stores selection ranges.
 		 *
 		 * @protected
-		 * @member {Array.<module:engine/model/range~Range>}
+		 * @type {Array.<module:engine/model/range~Range>}
 		 */
 		this._ranges = [];
 
@@ -51,7 +53,7 @@ export default class Selection {
 		 * List of attributes set on current selection.
 		 *
 		 * @protected
-		 * @member {Map} module:engine/model/selection~Selection#_attrs
+		 * @type {Map}
 		 */
 		this._attrs = new Map();
 
@@ -174,7 +176,7 @@ export default class Selection {
 	}
 
 	/**
-	 * Returns an iterator that iterates over copies of selection ranges.
+	 * Returns an iterable that iterates over copies of selection ranges.
 	 *
 	 * @returns {Iterable.<module:engine/model/range~Range>}
 	 */
@@ -258,30 +260,6 @@ export default class Selection {
 		return lastRange ? Position.createFromPosition( lastRange.end ) : null;
 	}
 
-	/**
-	 * Adds a range to this selection. Added range is copied. This means that passed range is not saved in `Selection`
-	 * instance and operating on it will not change `Selection` state.
-	 *
-	 * Accepts a flag describing in which way the selection is made - passed range might be selected from
-	 * {@link module:engine/model/range~Range#start start} to {@link module:engine/model/range~Range#end end}
-	 * or from {@link module:engine/model/range~Range#end end}
-	 * to {@link module:engine/model/range~Range#start start}.
-	 * The flag is used to set {@link #anchor} and
-	 * {@link #focus} properties.
-	 *
-	 * @protected
-	 * @fires change:range
-	 * @param {module:engine/model/range~Range} range Range to add.
-	 * @param {Boolean} [isBackward=false] Flag describing if added range was selected forward - from start to end (`false`)
-	 * or backward - from end to start (`true`).
-	 */
-	_addRange( range, isBackward = false ) {
-		this._pushRange( range );
-		this._lastRangeBackward = !!isBackward;
-
-		this.fire( 'change:range', { directChange: true } );
-	}
-
 	/**
 	 * Sets this selection's ranges and direction to the specified location based on the given
 	 * {@link module:engine/model/selection~Selection selection}, {@link module:engine/model/position~Position position},
@@ -290,7 +268,7 @@ export default class Selection {
 	 * @protected
 	 * @param {module:engine/model/selection~Selection|module:engine/model/position~Position|
 	 * Iterable.<module:engine/model/range~Range>|module:engine/model/range~Range|null} selectable
-	 * @param {string|boolean|number} [backwardSelectionOrOffset]
+	 * @param {Boolean|Number|'before'|'end'|'after'} [backwardSelectionOrOffset]
 	 */
 	setTo( selectable, backwardSelectionOrOffset ) {
 		if ( !selectable ) {
@@ -364,53 +342,6 @@ export default class Selection {
 		}
 	}
 
-	/**
-	 * Sets collapsed selection at the specified location.
-	 *
-	 * The location can be specified in the same form as {@link module:engine/model/position~Position.createAt} parameters.
-	 *
-	 * @fires change:range
-	 * @param {module:engine/model/item~Item|module:engine/model/position~Position} itemOrPosition
-	 * @param {Number|'end'|'before'|'after'} [offset=0] Offset or one of the flags. Used only when
-	 * first parameter is a {@link module:engine/model/item~Item model item}.
-	 */
-	setCollapsedAt( itemOrPosition, offset ) {
-		const pos = Position.createAt( itemOrPosition, offset );
-		const range = new Range( pos, pos );
-
-		this._setRanges( [ range ] );
-	}
-
-	/**
-	 * Collapses selection to the selection's {@link module:engine/model/selection~Selection#getFirstPosition first position}.
-	 * All ranges, besides the collapsed one, will be removed. Nothing will change if there are no ranges stored
-	 * inside selection.
-	 *
-	 * @fires change
-	 */
-	collapseToStart() {
-		const startPosition = this.getFirstPosition();
-
-		if ( startPosition !== null ) {
-			this._setRanges( [ new Range( startPosition, startPosition ) ] );
-		}
-	}
-
-	/**
-	 * Collapses selection to the selection's {@link module:engine/model/selection~Selection#getLastPosition last position}.
-	 * All ranges, besides the collapsed one, will be removed. Nothing will change if there are no ranges stored
-	 * inside selection.
-	 *
-	 * @fires change
-	 */
-	collapseToEnd() {
-		const endPosition = this.getLastPosition();
-
-		if ( endPosition !== null ) {
-			this._setRanges( [ new Range( endPosition, endPosition ) ] );
-		}
-	}
-
 	/**
 	 * Moves {@link module:engine/model/selection~Selection#focus} to the specified location.
 	 *
@@ -446,10 +377,14 @@ export default class Selection {
 		}
 
 		if ( newFocus.compareWith( anchor ) == 'before' ) {
-			this._addRange( new Range( newFocus, anchor ), true );
+			this._pushRange( new Range( newFocus, anchor ) );
+			this._lastRangeBackward = true;
 		} else {
-			this._addRange( new Range( anchor, newFocus ) );
+			this._pushRange( new Range( anchor, newFocus ) );
+			this._lastRangeBackward = false;
 		}
+
+		this.fire( 'change:range', { directChange: true } );
 	}
 
 	/**
@@ -463,7 +398,7 @@ export default class Selection {
 	}
 
 	/**
-	 * Returns iterator that iterates over this selection's attributes.
+	 * Returns iterable that iterates over this selection's attributes.
 	 *
 	 * Attributes are returned as arrays containing two items. First one is attribute key and second is attribute value.
 	 * This format is accepted by native `Map` object and also can be passed in `Node` constructor.
@@ -475,7 +410,7 @@ export default class Selection {
 	}
 
 	/**
-	 * Returns iterator that iterates over this selection's attribute keys.
+	 * Returns iterable that iterates over this selection's attribute keys.
 	 *
 	 * @returns {Iterable.<String>}
 	 */

+ 14 - 92
packages/ckeditor5-engine/tests/model/selection.js

@@ -161,84 +161,6 @@ describe( 'Selection', () => {
 		} );
 	} );
 
-	describe.skip( 'addRange()', () => {
-		it( 'should copy added ranges and store multiple ranges', () => {
-			selection.addRange( liveRange );
-			selection.addRange( range );
-
-			const ranges = selection._ranges;
-
-			expect( ranges.length ).to.equal( 2 );
-			expect( ranges[ 0 ].isEqual( liveRange ) ).to.be.true;
-			expect( ranges[ 1 ].isEqual( range ) ).to.be.true;
-			expect( ranges[ 0 ] ).not.to.be.equal( liveRange );
-			expect( ranges[ 1 ] ).not.to.be.equal( range );
-		} );
-
-		it( 'should set anchor and focus to the start and end of the most recently added range', () => {
-			selection.addRange( liveRange );
-
-			expect( selection.anchor.path ).to.deep.equal( [ 0 ] );
-			expect( selection.focus.path ).to.deep.equal( [ 1 ] );
-
-			selection.addRange( range );
-
-			expect( selection.anchor.path ).to.deep.equal( [ 2 ] );
-			expect( selection.focus.path ).to.deep.equal( [ 2, 2 ] );
-		} );
-
-		it( 'should set anchor and focus to the end and start of the most recently added range if backward flag was used', () => {
-			selection.addRange( liveRange, true );
-
-			expect( selection.anchor.path ).to.deep.equal( [ 1 ] );
-			expect( selection.focus.path ).to.deep.equal( [ 0 ] );
-
-			selection.addRange( range, true );
-
-			expect( selection.anchor.path ).to.deep.equal( [ 2, 2 ] );
-			expect( selection.focus.path ).to.deep.equal( [ 2 ] );
-		} );
-
-		it( 'should return a copy of (not a reference to) array of stored ranges', () => {
-			selection.addRange( liveRange );
-
-			const ranges = Array.from( selection.getRanges() );
-
-			selection.addRange( range );
-
-			expect( ranges.length ).to.equal( 1 );
-			expect( ranges[ 0 ].isEqual( liveRange ) ).to.be.true;
-		} );
-
-		it( 'should fire change:range event when adding a range', () => {
-			const spy = sinon.spy();
-			selection.on( 'change:range', spy );
-
-			selection.addRange( range );
-
-			expect( spy.called ).to.be.true;
-		} );
-
-		it( 'should throw an error when range is invalid', () => {
-			expect( () => {
-				selection.addRange( { invalid: 'range' } );
-			} ).to.throw( CKEditorError, /model-selection-added-not-range/ );
-		} );
-
-		it( 'should throw an error if added range intersects with already stored range', () => {
-			selection.addRange( liveRange );
-
-			expect( () => {
-				selection.addRange(
-					new Range(
-						new Position( root, [ 0, 4 ] ),
-						new Position( root, [ 1, 2 ] )
-					)
-				);
-			} ).to.throw( CKEditorError, /model-selection-range-intersects/ );
-		} );
-	} );
-
 	describe( 'setTo()', () => {
 		it( 'should set selection inside an element', () => {
 			const element = new Element( 'p', null, [ new Text( 'foo' ), new Text( 'bar' ) ] );
@@ -382,7 +304,7 @@ describe( 'Selection', () => {
 			const startPos = Position.createAt( root, 1 );
 			const endPos = Position.createAt( root, 2 );
 
-			selection.setCollapsedAt( startPos );
+			selection.setTo( startPos );
 
 			selection.moveFocusTo( endPos );
 
@@ -394,7 +316,7 @@ describe( 'Selection', () => {
 			const startPos = Position.createAt( root, 1 );
 			const endPos = Position.createAt( root, 0 );
 
-			selection.setCollapsedAt( startPos );
+			selection.setTo( startPos );
 
 			selection.moveFocusTo( endPos );
 
@@ -780,14 +702,14 @@ describe( 'Selection', () => {
 		} );
 	} );
 
-	describe.skip( 'collapseToStart()', () => {
+	describe( 'setTo - used to collapse at start', () => {
 		it( 'should collapse to start position and fire change event', () => {
-			selection._setRanges( [ range2, range1, range3 ] );
+			selection.setTo( [ range2, range1, range3 ] );
 
 			const spy = sinon.spy();
 			selection.on( 'change:range', spy );
 
-			selection.collapseToStart();
+			selection.setTo( selection.getFirstPosition() );
 
 			expect( selection.rangeCount ).to.equal( 1 );
 			expect( selection.isCollapsed ).to.be.true;
@@ -796,11 +718,11 @@ describe( 'Selection', () => {
 		} );
 
 		it( 'should do nothing if selection was already collapsed', () => {
-			selection.setCollapsedAt( range1.start );
+			selection.setTo( range1.start );
 
 			const spy = sinon.spy( selection, 'fire' );
 
-			selection.collapseToStart();
+			selection.setTo( selection.getFirstPosition() );
 
 			expect( spy.notCalled ).to.be.true;
 			spy.restore();
@@ -809,21 +731,21 @@ describe( 'Selection', () => {
 		it( 'should do nothing if no ranges present', () => {
 			const spy = sinon.spy( selection, 'fire' );
 
-			selection.collapseToStart();
+			selection.setTo( selection.getFirstPosition() );
 
 			spy.restore();
 			expect( spy.notCalled ).to.be.true;
 		} );
 	} );
 
-	describe.skip( 'collapseToEnd()', () => {
+	describe( 'setTo - used to collapse at end', () => {
 		it( 'should collapse to start position and fire change:range event', () => {
-			selection._setRanges( [ range2, range3, range1 ] );
+			selection.setTo( [ range2, range3, range1 ] );
 
 			const spy = sinon.spy();
 			selection.on( 'change:range', spy );
 
-			selection.collapseToEnd();
+			selection.setTo( selection.getLastPosition() );
 
 			expect( selection.rangeCount ).to.equal( 1 );
 			expect( selection.isCollapsed ).to.be.true;
@@ -832,11 +754,11 @@ describe( 'Selection', () => {
 		} );
 
 		it( 'should do nothing if selection was already collapsed', () => {
-			selection.setCollapsedAt( range1.start );
+			selection.setTo( range1.start );
 
 			const spy = sinon.spy( selection, 'fire' );
 
-			selection.collapseToEnd();
+			selection.setTo( selection.getLastPosition() );
 
 			expect( spy.notCalled ).to.be.true;
 			spy.restore();
@@ -845,7 +767,7 @@ describe( 'Selection', () => {
 		it( 'should do nothing if selection has no ranges', () => {
 			const spy = sinon.spy( selection, 'fire' );
 
-			selection.collapseToEnd();
+			selection.setTo( selection.getLastPosition() );
 
 			expect( spy.notCalled ).to.be.true;
 			spy.restore();