diff --git a/src/controller/editingcontroller.js b/src/controller/editingcontroller.js index 816447aec..03f4a6eaa 100644 --- a/src/controller/editingcontroller.js +++ b/src/controller/editingcontroller.js @@ -87,7 +87,7 @@ export default class EditingController { // Also convert model selection. this.listenTo( doc, 'change', () => { this.view.change( writer => { - this.downcastDispatcher.convertChanges( doc.differ, writer ); + this.downcastDispatcher.convertChanges( doc.differ, markers, writer ); this.downcastDispatcher.convertSelection( selection, markers, writer ); } ); }, { priority: 'low' } ); diff --git a/src/conversion/downcastdispatcher.js b/src/conversion/downcastdispatcher.js index ae2f63b52..ee45d46b3 100644 --- a/src/conversion/downcastdispatcher.js +++ b/src/conversion/downcastdispatcher.js @@ -106,10 +106,10 @@ export default class DowncastDispatcher { * Creates a `DowncastDispatcher` instance. * * @see module:engine/conversion/downcastdispatcher~DowncastConversionApi - * @param {Object} [conversionApi] Additional properties for interface that will be passed to events fired + * @param {Object} conversionApi Additional properties for interface that will be passed to events fired * by `DowncastDispatcher`. */ - constructor( conversionApi = {} ) { + constructor( conversionApi ) { /** * Interface passed by dispatcher to the events callbacks. * @@ -122,9 +122,10 @@ export default class DowncastDispatcher { * Takes {@link module:engine/model/differ~Differ model differ} object with buffered changes and fires conversion basing on it. * * @param {module:engine/model/differ~Differ} differ Differ object with buffered changes. + * @param {module:engine/model/markercollection~MarkerCollection} markers Markers connected with converted model. * @param {module:engine/view/downcastwriter~DowncastWriter} writer View writer that should be used to modify view document. */ - convertChanges( differ, writer ) { + convertChanges( differ, markers, writer ) { // Before the view is updated, remove markers which have changed. for ( const change of differ.getMarkersToRemove() ) { this.convertMarkerRemove( change.name, change.range, writer ); @@ -142,6 +143,13 @@ export default class DowncastDispatcher { } } + for ( const markerName of this.conversionApi.mapper.flushUnboundMarkerNames() ) { + const markerRange = markers.get( markerName ).getRange(); + + this.convertMarkerRemove( markerName, markerRange, writer ); + this.convertMarkerAdd( markerName, markerRange, writer ); + } + // After the view is updated, convert markers which have changed. for ( const change of differ.getMarkersToAdd() ) { this.convertMarkerAdd( change.name, change.range, writer ); @@ -252,7 +260,7 @@ export default class DowncastDispatcher { * @fires addMarker * @fires attribute * @param {module:engine/model/selection~Selection} selection Selection to convert. - * @param {Array.} markers Array of markers containing model markers. + * @param {module:engine/model/markercollection~MarkerCollection} markers Markers connected with converted model. * @param {module:engine/view/downcastwriter~DowncastWriter} writer View writer that should be used to modify view document. */ convertSelection( selection, markers, writer ) { diff --git a/src/conversion/mapper.js b/src/conversion/mapper.js index d0734594e..7dba3bf94 100644 --- a/src/conversion/mapper.js +++ b/src/conversion/mapper.js @@ -77,6 +77,25 @@ export default class Mapper { */ this._markerNameToElements = new Map(); + /** + * View element to model marker names mapping. + * + * This is reverse to {@link ~Mapper#_markerNameToElements} map. + * + * @private + * @member {Map} + */ + this._elementToMarkerNames = new Map(); + + /** + * Stores marker names of markers which has changed due to unbinding a view element (so it is assumed that the view element + * has been removed, moved or renamed). + * + * @private + * @member {Set.} + */ + this._unboundMarkerNames = new Set(); + // Default mapper algorithm for mapping model position to view position. this.on( 'modelToViewPosition', ( evt, data ) => { if ( data.viewPosition ) { @@ -132,6 +151,12 @@ export default class Mapper { this._viewToModelMapping.delete( viewElement ); + if ( this._elementToMarkerNames.has( viewElement ) ) { + for ( const markerName of this._elementToMarkerNames.get( viewElement ) ) { + this._unboundMarkerNames.add( markerName ); + } + } + if ( this._modelToViewMapping.get( modelElement ) == viewElement ) { this._modelToViewMapping.delete( modelElement ); } @@ -167,10 +192,13 @@ export default class Mapper { */ bindElementToMarker( element, name ) { const elements = this._markerNameToElements.get( name ) || new Set(); - elements.add( element ); + const names = this._elementToMarkerNames.get( element ) || new Set(); + names.add( name ); + this._markerNameToElements.set( name, elements ); + this._elementToMarkerNames.set( element, names ); } /** @@ -182,15 +210,41 @@ export default class Mapper { unbindElementFromMarkerName( element, name ) { const elements = this._markerNameToElements.get( name ); - if ( elements ) { - elements.delete( element ); + if ( !elements ) { + return; + } + + elements.delete( element ); + + if ( elements.size == 0 ) { + this._markerNameToElements.delete( name ); + } + + const names = this._elementToMarkerNames.get( element ); + + if ( names ) { + names.delete( name ); - if ( elements.size == 0 ) { - this._markerNameToElements.delete( name ); + if ( names.size == 0 ) { + this._elementToMarkerNames.delete( element ); } } } + /** + * Returns all marker names of markers which has changed due to unbinding a view element (so it is assumed that the view element + * has been removed, moved or renamed) since the last flush. After returning, the marker names list is cleared. + * + * @returns {Array.} + */ + flushUnboundMarkerNames() { + const markerNames = Array.from( this._unboundMarkerNames ); + + this._unboundMarkerNames.clear(); + + return markerNames; + } + /** * Removes all model to view and view to model bindings. */ @@ -198,6 +252,8 @@ export default class Mapper { this._modelToViewMapping = new WeakMap(); this._viewToModelMapping = new WeakMap(); this._markerNameToElements = new Map(); + this._elementToMarkerNames = new Map(); + this._unboundMarkerNames = new Set(); } /** diff --git a/src/view/downcastwriter.js b/src/view/downcastwriter.js index 6abdf3316..a930e2dda 100644 --- a/src/view/downcastwriter.js +++ b/src/view/downcastwriter.js @@ -69,7 +69,7 @@ export default class DowncastWriter { * writer.setSelection( range ); * * // Sets selection to given ranges. - * const ranges = [ writer.createRange( start1, end2 ), writer.createRange( star2, end2 ) ]; + * const ranges = [ writer.createRange( start1, end2 ), writer.createRange( start2, end2 ) ]; * writer.setSelection( range ); * * // Sets selection to the other selection. diff --git a/tests/conversion/downcastdispatcher.js b/tests/conversion/downcastdispatcher.js index 890aacffb..5646be95e 100644 --- a/tests/conversion/downcastdispatcher.js +++ b/tests/conversion/downcastdispatcher.js @@ -4,6 +4,8 @@ */ import DowncastDispatcher from '../../src/conversion/downcastdispatcher'; +import Mapper from '../../src/conversion/mapper'; + import Model from '../../src/model/model'; import ModelText from '../../src/model/text'; import ModelElement from '../../src/model/element'; @@ -13,13 +15,14 @@ import View from '../../src/view/view'; import ViewContainerElement from '../../src/view/containerelement'; describe( 'DowncastDispatcher', () => { - let dispatcher, doc, root, differStub, model, view; + let dispatcher, doc, root, differStub, model, view, mapper; beforeEach( () => { model = new Model(); view = new View(); doc = model.document; - dispatcher = new DowncastDispatcher(); + mapper = new Mapper(); + dispatcher = new DowncastDispatcher( { mapper } ); root = doc.createRoot(); differStub = { @@ -48,7 +51,7 @@ describe( 'DowncastDispatcher', () => { differStub.getChanges = () => [ { type: 'insert', position, length: 1 } ]; view.change( writer => { - dispatcher.convertChanges( differStub, writer ); + dispatcher.convertChanges( differStub, model.markers, writer ); } ); expect( dispatcher.convertInsert.calledOnce ).to.be.true; @@ -63,7 +66,7 @@ describe( 'DowncastDispatcher', () => { differStub.getChanges = () => [ { type: 'remove', position, length: 2, name: '$text' } ]; view.change( writer => { - dispatcher.convertChanges( differStub, writer ); + dispatcher.convertChanges( differStub, model.markers, writer ); } ); expect( dispatcher.convertRemove.calledWith( position, 2, '$text' ) ).to.be.true; @@ -80,7 +83,7 @@ describe( 'DowncastDispatcher', () => { ]; view.change( writer => { - dispatcher.convertChanges( differStub, writer ); + dispatcher.convertChanges( differStub, model.markers, writer ); } ); expect( dispatcher.convertAttribute.calledWith( range, 'key', null, 'foo' ) ).to.be.true; @@ -102,7 +105,7 @@ describe( 'DowncastDispatcher', () => { ]; view.change( writer => { - dispatcher.convertChanges( differStub, writer ); + dispatcher.convertChanges( differStub, model.markers, writer ); } ); expect( dispatcher.convertInsert.calledTwice ).to.be.true; @@ -122,7 +125,7 @@ describe( 'DowncastDispatcher', () => { ]; view.change( writer => { - dispatcher.convertChanges( differStub, writer ); + dispatcher.convertChanges( differStub, model.markers, writer ); } ); expect( dispatcher.convertMarkerAdd.calledWith( 'foo', fooRange ) ); @@ -141,11 +144,34 @@ describe( 'DowncastDispatcher', () => { ]; view.change( writer => { - dispatcher.convertChanges( differStub, writer ); + dispatcher.convertChanges( differStub, model.markers, writer ); + } ); + + expect( dispatcher.convertMarkerRemove.calledWith( 'foo', fooRange ) ); + expect( dispatcher.convertMarkerRemove.calledWith( 'bar', barRange ) ); + } ); + + it( 'should re-render markers which view elements got unbound during conversion', () => { + sinon.stub( dispatcher, 'convertMarkerRemove' ); + sinon.stub( dispatcher, 'convertMarkerAdd' ); + + const fooRange = model.createRange( model.createPositionAt( root, 0 ), model.createPositionAt( root, 1 ) ); + const barRange = model.createRange( model.createPositionAt( root, 3 ), model.createPositionAt( root, 6 ) ); + + model.markers._set( 'foo', fooRange ); + model.markers._set( 'bar', barRange ); + + // Stub `_unboundMarkers`. + dispatcher.conversionApi.mapper._unboundMarkers = new Set( [ 'foo', 'bar' ] ); + + view.change( writer => { + dispatcher.convertChanges( differStub, model.markers, writer ); } ); expect( dispatcher.convertMarkerRemove.calledWith( 'foo', fooRange ) ); expect( dispatcher.convertMarkerRemove.calledWith( 'bar', barRange ) ); + expect( dispatcher.convertMarkerAdd.calledWith( 'foo', fooRange ) ); + expect( dispatcher.convertMarkerAdd.calledWith( 'bar', barRange ) ); } ); } ); diff --git a/tests/conversion/mapper.js b/tests/conversion/mapper.js index 1afea7c3f..4aa252619 100644 --- a/tests/conversion/mapper.js +++ b/tests/conversion/mapper.js @@ -23,15 +23,23 @@ describe( 'Mapper', () => { const viewA = new ViewElement( 'a' ); const viewB = new ViewElement( 'b' ); const viewC = new ViewElement( 'c' ); + const viewD = new ViewElement( 'd' ); const modelA = new ModelElement( 'a' ); const modelB = new ModelElement( 'b' ); const modelC = new ModelElement( 'c' ); + const modelD = new ModelElement( 'd' ); const mapper = new Mapper(); + mapper.bindElements( modelA, viewA ); mapper.bindElements( modelB, viewB ); mapper.bindElements( modelC, viewC ); + mapper.bindElements( modelD, viewD ); + + mapper.bindElementToMarker( viewA, 'foo' ); + mapper.bindElementToMarker( viewD, 'foo' ); + mapper.bindElementToMarker( viewD, 'bar' ); expect( mapper.toModelElement( viewA ) ).to.equal( modelA ); expect( mapper.toModelElement( viewB ) ).to.equal( modelB ); @@ -41,6 +49,9 @@ describe( 'Mapper', () => { expect( mapper.toViewElement( modelB ) ).to.equal( viewB ); expect( mapper.toViewElement( modelC ) ).to.equal( viewC ); + expect( Array.from( mapper.markerNameToElements( 'foo' ) ) ).to.deep.equal( [ viewA, viewD ] ); + + mapper.unbindViewElement( viewD ); mapper.clearBindings(); expect( mapper.toModelElement( viewA ) ).to.be.undefined; @@ -50,6 +61,9 @@ describe( 'Mapper', () => { expect( mapper.toViewElement( modelA ) ).to.be.undefined; expect( mapper.toViewElement( modelB ) ).to.be.undefined; expect( mapper.toViewElement( modelC ) ).to.be.undefined; + + expect( mapper.markerNameToElements( 'foo' ) ).to.be.null; + expect( mapper.flushUnboundMarkerNames() ).to.deep.equal( [] ); } ); } ); @@ -123,6 +137,21 @@ describe( 'Mapper', () => { expect( mapper.toModelElement( viewA ) ).to.be.undefined; expect( mapper.toViewElement( modelA ) ).to.equal( viewB ); } ); + + it( 'should add marker names to _unboundMarkers set', () => { + const viewA = new ViewElement( 'a' ); + const modelA = new ModelElement( 'a' ); + + const mapper = new Mapper(); + mapper.bindElements( modelA, viewA ); + + mapper.bindElementToMarker( viewA, 'foo' ); + mapper.bindElementToMarker( viewA, 'bar' ); + + mapper.unbindViewElement( viewA ); + + expect( Array.from( mapper._unboundMarkers ) ).to.deep.equal( [ 'foo', 'bar' ] ); + } ); } ); describe( 'Standard mapping', () => { @@ -648,11 +677,13 @@ describe( 'Mapper', () => { const viewB = new ViewElement( 'b' ); mapper.bindElementToMarker( viewA, 'marker' ); + mapper.bindElementToMarker( viewA, 'markerB' ); mapper.bindElementToMarker( viewB, 'marker' ); mapper.unbindElementFromMarkerName( viewA, 'marker' ); expect( Array.from( mapper.markerNameToElements( 'marker' ) ) ).to.deep.equal( [ viewB ] ); + expect( Array.from( mapper.markerNameToElements( 'markerB' ) ) ).to.deep.equal( [ viewA ] ); mapper.unbindElementFromMarkerName( viewB, 'marker' ); @@ -751,4 +782,25 @@ describe( 'Mapper', () => { expect( viewMappedAncestor ).to.equal( viewP ); } ); } ); + + describe( 'flushUnboundMarkerNames()', () => { + it( 'should return marker names of markers which elements has been unbound and clear that list', () => { + const viewA = new ViewElement( 'a' ); + const viewB = new ViewElement( 'b' ); + + const mapper = new Mapper(); + + mapper.bindElementToMarker( viewA, 'foo' ); + mapper.bindElementToMarker( viewA, 'bar' ); + mapper.bindElementToMarker( viewB, 'bar' ); + + expect( mapper.flushUnboundMarkerNames() ).to.deep.equal( [ 'foo', 'bar' ] ); + expect( mapper.flushUnboundMarkerNames() ).to.deep.equal( [] ); + + mapper.bindElementToMarker( viewA, 'foo' ); + + expect( mapper.flushUnboundMarkerNames() ).to.deep.equal( [ 'foo' ] ); + expect( mapper.flushUnboundMarkerNames() ).to.deep.equal( [] ); + } ); + } ); } );