Merge pull request #1694 from ckeditor/t/1649

Feature: Added possibility to refresh the marker with no changes through `Writer#updateMarker()` method. Closes #1649.

src/model/markercollection.js

@@ -153,6 +153,28 @@ export default class MarkerCollection {
 		return false;
 	}
 
+	/**
+	 * Fires an {@link module:engine/model/markercollection~MarkerCollection#event:update} event for the given {@link ~Marker marker}
+	 * but does not change the marker. Useful to force {@link module:engine/conversion/downcastdispatcher~DowncastDispatcher downcast
+	 * conversion} for the marker.
+	 *
+	 * @protected
+	 * @fires module:engine/model/markercollection~MarkerCollection#event:update
+	 * @param {String} markerOrName Marker or name of a marker to refresh.
+	 */
+	_refresh( markerOrName ) {
+		const markerName = markerOrName instanceof Marker ? markerOrName.name : markerOrName;
+		const marker = this._markers.get( markerName );
+
+		if ( !marker ) {
+			throw new CKEditorError( 'markercollection-refresh-marker-not-exists: Marker with provided name does not exists.' );
+		}
+
+		const range = marker.getRange();
+
+		this.fire( 'update:' + markerName, marker, range, range, marker.managedUsingOperations, marker.affectsData );
+	}
+
 	/**
 	 * Returns iterator that iterates over all markers, which ranges contain given {@link module:engine/model/position~Position position}.
 	 *

src/model/writer.js

@@ -939,13 +939,38 @@ export default class Writer {
 	}
 
 	/**
-	 * Adds or updates a {@link module:engine/model/markercollection~Marker marker}. Marker is a named range, which tracks
+	 * Adds, updates or refreshes a {@link module:engine/model/markercollection~Marker marker}. Marker is a named range, which tracks
 	 * changes in the document and updates its range automatically, when model tree changes. Still, it is possible to change the
 	 * marker's range directly using this method.
 	 *
 	 * As the first parameter you can set marker name or instance. If none of them is provided, new marker, with a unique
 	 * name is created and returned.
 	 *
+	 * As the second parameter you can set the new marker data or leave this parameter as empty which will just refresh
+	 * the marker by triggering downcast conversion for it. Refreshing the marker is useful when you want to change
+	 * the marker {@link module:engine/view/element~Element view element} without changing any marker data.
+	 *
+	 * 		let isCommentActive = false;
+	 *
+	 * 		model.conversion.markerToHighlight( {
+	 * 			model: 'comment',
+	 *			view: data => {
+	 *				const classes = [ 'comment-marker' ];
+	 *
+	 *				if ( isCommentActive ) {
+	 *					classes.push( 'comment-marker--active' );
+	 *				}
+	 *
+	 *				return { classes };
+	 *			}
+	 * 		} );
+	 *
+	 * 		// Change the property that indicates if marker is displayed as active or not.
+	 * 		isCommentActive = true;
+	 *
+	 * 		// And refresh the marker to convert it with additional class.
+	 * 		model.change( writer => writer.updateMarker( 'comment' ) );
+	 *
 	 * The `options.usingOperation` parameter lets you change if the marker should be managed by operations or not. See
 	 * {@link module:engine/model/markercollection~Marker marker class description} to learn about the difference between
 	 * markers managed by operations and not-managed by operations. It is possible to change this option for an existing marker.
@@ -975,13 +1000,14 @@ export default class Writer {
 	 *
 	 * @see module:engine/model/markercollection~Marker
 	 * @param {String} markerOrName Name of a marker to update, or a marker instance.
-	 * @param {Object} options
+	 * @param {Object} [options] If options object is not defined then marker will be refreshed by triggering
+	 * downcast conversion for this marker with the same data.
 	 * @param {module:engine/model/range~Range} [options.range] Marker range to update.
 	 * @param {Boolean} [options.usingOperation] Flag indicated whether the marker should be added by MarkerOperation.
 	 * See {@link module:engine/model/markercollection~Marker#managedUsingOperations}.
 	 * @param {Boolean} [options.affectsData] Flag indicating that the marker changes the editor data.
 	 */
-	updateMarker( markerOrName, options = {} ) {
+	updateMarker( markerOrName, options ) {
 		this._assertWriterUsedCorrectly();
 
 		const markerName = typeof markerOrName == 'string' ? markerOrName : markerOrName.name;
@@ -996,6 +1022,12 @@ export default class Writer {
 			throw new CKEditorError( 'writer-updateMarker-marker-not-exists: Marker with provided name does not exists.' );
 		}
 
+		if ( !options ) {
+			this.model.markers._refresh( currentMarker );
+
+			return;
+		}
+
 		const hasUsingOperationDefined = typeof options.usingOperation == 'boolean';
 		const affectsDataDefined = typeof options.affectsData == 'boolean';
 

tests/model/markercollection.js

@@ -195,6 +195,24 @@ describe( 'MarkerCollection', () => {
 		} );
 	} );
 
+	describe( '_refresh()', () => {
+		it( 'should fire update:<markerName> event', () => {
+			const marker = markers._set( 'name', range );
+
+			sinon.spy( markers, 'fire' );
+
+			markers._refresh( 'name' );
+
+			sinon.assert.calledWithExactly( markers.fire, 'update:name', marker, range, range, false, false );
+		} );
+
+		it( 'should throw if marker does not exist', () => {
+			expect( () => {
+				markers._refresh( 'name' );
+			} ).to.throw( CKEditorError, 'markercollection-refresh-marker-not-exists: Marker with provided name does not exists.' );
+		} );
+	} );
+
 	describe( 'getMarkersGroup', () => {
 		it( 'returns all markers which names start on given prefix', () => {
 			const markerFooA = markers._set( 'foo:a', range );

tests/model/writer.js

@@ -2402,7 +2402,7 @@ describe( 'Writer', () => {
 		it( 'should throw when range and usingOperations were not provided', () => {
 			expect( () => {
 				addMarker( 'name', { range, usingOperation: false } );
-				updateMarker( 'name' );
+				updateMarker( 'name', {} );
 			} ).to.throw( CKEditorError, /^writer-updateMarker-wrong-options/ );
 		} );
 
@@ -2412,6 +2412,21 @@ describe( 'Writer', () => {
 			} ).to.throw( CKEditorError, /^writer-updateMarker-marker-not-exists/ );
 		} );
 
+		it( 'should only refresh the marker when there is no provided options to update', () => {
+			const marker = addMarker( 'name', { range, usingOperation: true } );
+			const spy = sinon.spy( model.markers, '_refresh' );
+
+			updateMarker( marker );
+
+			sinon.assert.calledOnce( spy );
+			sinon.assert.calledWithExactly( spy, marker );
+
+			updateMarker( 'name' );
+
+			sinon.assert.calledTwice( spy );
+			sinon.assert.calledWithExactly( spy.secondCall, marker );
+		} );
+
 		it( 'should throw when trying to use detached writer', () => {
 			const marker = addMarker( 'name', { range, usingOperation: false } );
 			const writer = new Writer( model, batch );