8
0
فهرست منبع

Changed: MarkersCollection methods now can't take ranges as parameters.
Docs: changes in docs.

Szymon Cofalik 9 سال پیش
والد
کامیت
1301c1fef7

+ 18 - 54
packages/ckeditor5-engine/src/model/markerscollection.js

@@ -29,9 +29,12 @@ import mix from '../../utils/mix.js';
  * using {@link module:engine/conversion/buildmodelconverter~buildModelConverter model converter builder}.
  *
  * Markers are similar to adding and converting attributes on nodes. The difference is that attribute is connected to
- * a given node (i.e. a character is bold no matter if it gets moved or content around it changes). Markers on the
- * other hand are continuous ranges (i.e. if a character from inside of marker range is moved somewhere else, marker
- * range is shrunk and the character does not have any attribute or information that it was in the marked range).
+ * a given node (e.g. a character is bold no matter if it gets moved or content around it changes). Markers on the
+ * other hand are continuous ranges (e.g. if a character from inside of marker range is moved somewhere else, marker
+ * range is shrunk and the character does not have any attribute or information that it was in the marked range). Another
+ * upside of markers is that finding marked text is fast and easy. Using attributes to mark some nodes and then trying to
+ * find that part of document would require traversing whole document tree. For markers, only marker name is needed
+ * and a proper range can {@link module:engine/model/markerscollection~MarkersCollection#get be obtained} from the collection.
  */
 export default class MarkersCollection {
 	/**
@@ -102,27 +105,17 @@ export default class MarkersCollection {
 	}
 
 	/**
-	 * Removes a live range from markers collection.
+	 * Removes a live range having given `name` from markers collection.
 	 *
-	 * @param {String|module:engine/model/liverange~LiveRange} nameOrRange Name of live range to remove or live range instance.
+	 * @param {String} name Name of live range to remove.
 	 * @returns {Boolean} `true` is passed if range was found and removed from the markers collection, `false` otherwise.
 	 */
-	remove( nameOrRange ) {
-		if ( nameOrRange instanceof LiveRange ) {
-			const name = this._getMarkerName( nameOrRange );
-
-			if ( name ) {
-				return this.remove( name );
-			}
-
-			return false;
-		}
-
-		const range = this._nameToRange.get( nameOrRange );
+	remove( name ) {
+		const range = this._nameToRange.get( name );
 
 		if ( range ) {
-			this._nameToRange.delete( nameOrRange );
-			this.fire( 'remove', nameOrRange, Range.createFromRange( range ) );
+			this._nameToRange.delete( name );
+			this.fire( 'remove', name, Range.createFromRange( range ) );
 
 			return true;
 		}
@@ -131,32 +124,20 @@ export default class MarkersCollection {
 	}
 
 	/**
-	 * Substitutes given `oldLiveRange`, that was already added to the markers collection, with given `newLiveRange`.
+	 * Substitutes range having given `name`, that was already added to the markers collection, with given `newLiveRange`.
 	 *
 	 * This method is basically a wrapper for using {@link module:engine/model/markerscollection~MarkersCollection#removeRange removeRange}
 	 * followed by using {@link module:engine/model/markerscollection~MarkersCollection#addRange addRange}.
 	 *
-	 * **Note**: this method does not change properties of `oldLiveRange`.
-	 *
-	 * @param {String|module:engine/model/liverange~LiveRange} nameOrRange Name of range or range instance to be changed.
+	 * @param {String} name Name of a range to be changed.
 	 * @param {module:engine/model/liverange~LiveRange} newLiveRange Live range to be added.
-	 * @returns {Boolean} `true` if `oldLiveRange` was found and changed, `false` otherwise.
+	 * @returns {Boolean} `true` if range for given `name` was found and changed, `false` otherwise.
 	 */
-	update( nameOrRange, newLiveRange ) {
-		if ( nameOrRange instanceof LiveRange ) {
-			const name = this._getMarkerName( nameOrRange );
-
-			if ( name ) {
-				return this.update( name, newLiveRange );
-			}
-
-			return false;
-		}
-
-		const removed = this.remove( nameOrRange );
+	update( name, newLiveRange ) {
+		const removed = this.remove( name );
 
 		if ( removed ) {
-			this.add( nameOrRange, newLiveRange );
+			this.add( name, newLiveRange );
 		}
 
 		return removed;
@@ -168,23 +149,6 @@ export default class MarkersCollection {
 	destroy() {
 		this.stopListening();
 	}
-
-	/**
-	 * Returns range for given name.
-	 *
-	 * @private
-	 * @param {module:engine/model/liverange~LiveRange} range Range for which name should be obtained.
-	 * @returns {String|null} Name of the range or `null` if range has not been added to the collection.
-	 */
-	_getMarkerName( range ) {
-		for ( let [ markerName, markerRange ] of this ) {
-			if ( markerRange == range ) {
-				return markerName;
-			}
-		}
-
-		return null;
-	}
 }
 
 mix( MarkersCollection, EmitterMixin );

+ 1 - 1
packages/ckeditor5-engine/src/view/writer.js

@@ -275,7 +275,7 @@ export function mergeContainers( position ) {
 /**
  * Breaks given `range` on a set of {@link module:engine/view/range~Range ranges}, that each are contained within a
  * {@link module:engine/view/containerelement~ContainerElement container element}. After `range` is broken, it's "pieces" can
- * be used by {@link module:engine/view/writer~writer} (which expects that passed ranges are contained within on container element).
+ * be used by other {@link module:engine/view/writer~writer} (which expect that passed ranges are contained within one container element).
  *
  * @function module:engine/view/writer~writer.breakViewRangePerContainer
  * @param {module:engine/view/range~Range} range Range to break.

+ 3 - 51
packages/ckeditor5-engine/tests/model/markerscollection.js

@@ -75,7 +75,7 @@ describe( 'MarkersCollection', () => {
 	} );
 
 	describe( 'remove', () => {
-		it( 'should return true and fire remove event if range is removed - passed name', () => {
+		it( 'should return true and fire remove event if range is removed', () => {
 			markers.add( 'name', live );
 
 			sinon.spy( markers, 'fire' );
@@ -92,36 +92,10 @@ describe( 'MarkersCollection', () => {
 			expect( markers.fire.calledWith( 'remove' ) ).to.be.true;
 		} );
 
-		it( 'should return true and fire remove event if range is removed - passed range', () => {
-			markers.add( 'name', live );
-
-			sinon.spy( markers, 'fire' );
-
-			markers.on( 'remove', ( evt, name, range ) => {
-				expect( name ).to.equal( 'name' );
-				expect( range.isEqual( live ) ).to.be.true;
-				expect( range ).not.to.equal( live );
-			} );
-
-			const result = markers.remove( live );
-
-			expect( result ).to.be.true;
-			expect( markers.fire.calledWith( 'remove' ) ).to.be.true;
-		} );
-
 		it( 'should return false if name has not been found in collection', () => {
-			const result = markers.remove( 'name' );
-
-			expect( result ).to.be.false;
-		} );
-
-		it( 'should return false if range has not been found in collection', () => {
 			markers.add( 'name', live );
 
-			const other = LiveRange.createFromParentsAndOffsets( root, 0, root, 4 );
-			const result = markers.remove( other );
-
-			other.detach();
+			const result = markers.remove( 'other' );
 
 			expect( result ).to.be.false;
 		} );
@@ -138,7 +112,7 @@ describe( 'MarkersCollection', () => {
 			newLive.detach();
 		} );
 
-		it( 'should return true and use remove and add methods if range was found in collection - passed name', () => {
+		it( 'should return true and use remove and add methods if range was found in collection', () => {
 			const newLive = LiveRange.createFromParentsAndOffsets( root, 1, root, 5 );
 			markers.add( 'name', live );
 
@@ -154,33 +128,11 @@ describe( 'MarkersCollection', () => {
 			newLive.detach();
 		} );
 
-		it( 'should return true and use remove and add methods if range was found in collection - passed range', () => {
-			const newLive = LiveRange.createFromParentsAndOffsets( root, 1, root, 5 );
-			markers.add( 'name', live );
-
-			sinon.spy( markers, 'remove' );
-			sinon.spy( markers, 'add' );
-
-			const result = markers.update( live, newLive );
-
-			expect( markers.remove.calledWith( 'name' ) ).to.be.true;
-			expect( markers.add.calledWith( 'name', newLive ) ).to.be.true;
-			expect( result ).to.be.true;
-
-			newLive.detach();
-		} );
-
 		it( 'should return false if given name was not found in collection', () => {
 			const result = markers.update( 'name', newLive );
 
 			expect( result ).to.be.false;
 		} );
-
-		it( 'should return false if given range was not found in collection', () => {
-			const result = markers.update( live, newLive );
-
-			expect( result ).to.be.false;
-		} );
 	} );
 
 	describe( 'destroy', () => {