Browse Source

Tests/Docs: Updated Writer and DocumentSelection docs and tests to the newest API.

Aleksander Nowodzinski 7 years ago
parent
commit
7fc7d346fe

+ 23 - 7
packages/ckeditor5-engine/src/model/documentselection.js

@@ -400,10 +400,14 @@ export default class DocumentSelection {
 	}
 
 	/**
-	 * Temporarily changes the gravity of the selection from left to right. The gravity defines from which direction
-	 * the selection inherits its attributes. If it's the default left gravity, the selection (after being moved by
-	 * the user) inherits attributes from its left hand side. This method allows to temporarily override this behavior
-	 * by forcing the gravity to the right.
+	 * Temporarily changes the gravity of the selection from the left to the right.
+	 *
+	 * The gravity defines from which direction the selection inherits its attributes. If it's the default left
+	 * gravity, the selection (after being moved by the the user) inherits attributes from its left hand side.
+	 * This method allows to temporarily override this behavior by forcing the gravity to the right.
+	 *
+	 * It returns an unique identifier which is required to restore the gravity. It guarantees the symmetry
+	 * of the process.
 	 *
 	 * @see module:engine/model/writer~Writer#overrideSelectionGravity
 	 * @protected
@@ -414,9 +418,11 @@ export default class DocumentSelection {
 	}
 
 	/**
-	 * Restores {@link ~DocumentSelection#_overrideGravity overridden gravity}.
+	 * Restores the {@link ~DocumentSelection#_overrideGravity overridden gravity}.
 	 *
-	 * Note that gravity remains overridden as long as won't be restored the same number of times as was overridden.
+	 * Restoring the gravity is only possible using the unique identifier returned by
+	 * {@link ~DocumentSelection#_overrideGravity}. Note that the gravity remains overridden as long as won't be restored
+	 * the same number of times it was overridden.
 	 *
 	 * @see module:engine/model/writer~Writer#restoreSelectionGravity
 	 * @protected
@@ -683,7 +689,17 @@ class LiveSelection extends Selection {
 
 	restoreGravity( uid ) {
 		if ( !this._overriddenGravityRegister.has( uid ) ) {
-			throw 'Restoring gravity for unknown id ' + uid;
+			/**
+			 * Restoring gravity for an unknown UID is not possible. Make sure you are using a correct
+			 * UID obtained from the {@link module:engine/model/writer~Writer#overrideSelectionGravity} to restore.
+			 *
+			 * @error document-selection-gravity-wrong-restore
+			 * @param {String} uid The unique identifier returned by {@link #overrideGravity}.
+			 */
+			throw new CKEditorError(
+				'document-selection-gravity-wrong-restore: Attempting to restore the selection gravity for an unknown UID.',
+				{ uid }
+			);
 		}
 
 		this._overriddenGravityRegister.delete( uid );

+ 7 - 2
packages/ckeditor5-engine/src/model/writer.js

@@ -1112,6 +1112,9 @@ export default class Writer {
 	 * * Default gravity: selection will have the `bold` and `linkHref` attributes.
 	 * * Overridden gravity: selection will have `bold` attribute.
 	 *
+	 * **Note**: It returns an unique identifier which is required to restore the gravity. It guarantees the symmetry
+	 * of the process.
+	 *
 	 * @returns {String} The unique id which allows restoring the gravity.
 	 */
 	overrideSelectionGravity() {
@@ -1121,9 +1124,11 @@ export default class Writer {
 	/**
 	 * Restores {@link ~Writer#overrideSelectionGravity} gravity to default.
 	 *
-	 * Note that the gravity remains overridden as long as will not be restored the same number of times as it was overridden.
+	 * Restoring the gravity is only possible using the unique identifier returned by
+	 * {@link ~Writer#overrideSelectionGravity}. Note that the gravity remains overridden as long as won't be restored
+	 * the same number of times it was overridden.
 	 *
-	 * @param {String} uid The unique id returned by {@link #overrideSelectionGravity}.
+	 * @param {String} uid The unique id returned by {@link ~Writer#overrideSelectionGravity}.
 	 */
 	restoreSelectionGravity( uid ) {
 		this.model.document.selection._restoreGravity( uid );

+ 37 - 18
packages/ckeditor5-engine/tests/model/documentselection.js

@@ -772,6 +772,15 @@ describe( 'DocumentSelection', () => {
 			} );
 		} );
 
+		it( 'should return the UID', () => {
+			const overrideUidA = selection._overrideGravity();
+			const overrideUidB = selection._overrideGravity();
+
+			expect( overrideUidA ).to.be.a( 'string' );
+			expect( overrideUidB ).to.be.a( 'string' );
+			expect( overrideUidA ).to.not.equal( overrideUidB );
+		} );
+
 		it( 'should mark gravity as overridden', () => {
 			expect( selection.isGravityOverridden ).to.false;
 
@@ -800,7 +809,7 @@ describe( 'DocumentSelection', () => {
 			expect( Array.from( selection.getAttributeKeys() ) ).to.have.members( [ 'bold', 'italic' ] );
 		} );
 
-		it( 'should retain attributes that are set explicit', () => {
+		it( 'should not retain attributes that are set explicitly', () => {
 			setData( model, '<$text italic="true">foo[]</$text>' );
 
 			selection._setAttribute( 'bold', true );
@@ -809,7 +818,7 @@ describe( 'DocumentSelection', () => {
 
 			selection._overrideGravity();
 
-			expect( Array.from( selection.getAttributeKeys() ) ).to.have.members( [ 'bold' ] );
+			expect( Array.from( selection.getAttributeKeys() ) ).to.have.members( [] );
 		} );
 
 		it( 'should retain overridden until selection will not change range by a direct change', () => {
@@ -836,43 +845,53 @@ describe( 'DocumentSelection', () => {
 			} );
 		} );
 
+		it( 'should throw when a wrong, already restored, or no UID provided', () => {
+			const overrideUid = selection._overrideGravity();
+			selection._restoreGravity( overrideUid );
+
+			// Wrong UID
+			expect( () => {
+				selection._restoreGravity( 'foo' );
+			} ).to.throw( CKEditorError, /^document-selection-gravity-wrong-restore/ );
+
+			// Already restored
+			expect( () => {
+				selection._restoreGravity( overrideUid );
+			} ).to.throw( CKEditorError, /^document-selection-gravity-wrong-restore/ );
+
+			// No UID
+			expect( () => {
+				selection._restoreGravity();
+			} ).to.throw( CKEditorError, /^document-selection-gravity-wrong-restore/ );
+		} );
+
 		it( 'should revert default gravity when is overridden', () => {
 			setData( model, '<$text bold="true" italic="true">foo[]</$text>' );
 
-			selection._overrideGravity();
+			const overrideUid = selection._overrideGravity();
 
 			expect( Array.from( selection.getAttributeKeys() ) ).to.length( 0 );
 			expect( selection.isGravityOverridden ).to.true;
 
-			selection._restoreGravity();
+			selection._restoreGravity( overrideUid );
 
 			expect( Array.from( selection.getAttributeKeys() ) ).to.have.members( [ 'bold', 'italic' ] );
 			expect( selection.isGravityOverridden ).to.false;
 		} );
 
-		it( 'should do nothing when gravity is not overridden', () => {
-			setData( model, '<$text bold="true" italic="true">foo[]</$text>' );
-
-			expect( () => {
-				selection._restoreGravity();
-			} ).to.not.throw();
-
-			expect( selection.isGravityOverridden ).to.false;
-		} );
-
 		it( 'should be called the same number of times as gravity is overridden to restore it', () => {
 			setData( model, '<$text bold="true" italic="true">foo[]</$text>' );
 
-			selection._overrideGravity();
-			selection._overrideGravity();
+			const overrideUidA = selection._overrideGravity();
+			const overrideUidB = selection._overrideGravity();
 
 			expect( selection.isGravityOverridden ).to.true;
 
-			selection._restoreGravity();
+			selection._restoreGravity( overrideUidA );
 
 			expect( selection.isGravityOverridden ).to.true;
 
-			selection._restoreGravity();
+			selection._restoreGravity( overrideUidB );
 
 			expect( selection.isGravityOverridden ).to.false;
 		} );

+ 14 - 27
packages/ckeditor5-engine/tests/model/writer.js

@@ -2455,6 +2455,10 @@ describe( 'Writer', () => {
 			overrideGravitySpy.restore();
 		} );
 
+		it( 'should return the unique id', () => {
+			expect( overrideSelectionGravity() ).to.be.a( 'string' );
+		} );
+
 		it( 'should not get attributes from the node before the caret when gravity is overridden', () => {
 			const root = doc.createRoot();
 			root._appendChild( [
@@ -2472,37 +2476,20 @@ describe( 'Writer', () => {
 			expect( Array.from( model.document.selection.getAttributeKeys() ) ).to.deep.equal( [ 'foo' ] );
 			expect( model.document.selection.isGravityOverridden ).to.true;
 
-			// Disable override by moving selection.
+			// Moving selection should not restore the gravity.
 			setSelection( new Position( root, [ 5 ] ) );
 
 			expect( Array.from( model.document.selection.getAttributeKeys() ) ).to.deep.equal( [ 'foo', 'bar' ] );
-			expect( model.document.selection.isGravityOverridden ).to.false;
-		} );
-
-		it( 'should allow to restorer gravity in a custom way', () => {
-			const root = doc.createRoot();
-			root._appendChild( [ new Text( 'foobar', { foo: true } ) ] );
-
-			setSelection( new Position( root, [ 1 ] ) );
-
-			overrideSelectionGravity( true );
-
-			// Moving selection does not restore gravity.
-			setSelection( new Position( root, [ 2 ] ) );
 			expect( model.document.selection.isGravityOverridden ).to.true;
-
-			// We need to do it manually.
-			restoreSelectionGravity();
-
-			expect( model.document.selection.isGravityOverridden ).to.false;
 		} );
 	} );
 
 	describe( 'restoreSelectionGravity()', () => {
 		it( 'should use DocumentSelection#_restoreGravity', () => {
+			const overrideUid = overrideSelectionGravity();
 			const restoreGravitySpy = sinon.spy( DocumentSelection.prototype, '_restoreGravity' );
 
-			restoreSelectionGravity();
+			restoreSelectionGravity( overrideUid );
 
 			sinon.assert.calledOnce( restoreGravitySpy );
 			restoreGravitySpy.restore();
@@ -2518,11 +2505,11 @@ describe( 'Writer', () => {
 
 			setSelection( new Position( root, [ 6 ] ) );
 
-			overrideSelectionGravity();
+			const overrideUid = overrideSelectionGravity();
 
 			expect( Array.from( model.document.selection.getAttributeKeys() ) ).to.deep.equal( [ 'foo' ] );
 
-			restoreSelectionGravity();
+			restoreSelectionGravity( overrideUid );
 
 			expect( Array.from( model.document.selection.getAttributeKeys() ) ).to.deep.equal( [ 'foo', 'bar' ] );
 		} );
@@ -2694,15 +2681,15 @@ describe( 'Writer', () => {
 		} );
 	}
 
-	function overrideSelectionGravity( customRestore ) {
-		model.change( writer => {
-			writer.overrideSelectionGravity( customRestore );
+	function overrideSelectionGravity() {
+		return model.change( writer => {
+			return writer.overrideSelectionGravity();
 		} );
 	}
 
-	function restoreSelectionGravity() {
+	function restoreSelectionGravity( overrideUid ) {
 		model.change( writer => {
-			writer.restoreSelectionGravity();
+			writer.restoreSelectionGravity( overrideUid );
 		} );
 	}
 } );