diff --git a/packages/ckeditor5-engine/src/controller/datacontroller.js b/packages/ckeditor5-engine/src/controller/datacontroller.js index f89d9567687..d63d71af341 100644 --- a/packages/ckeditor5-engine/src/controller/datacontroller.js +++ b/packages/ckeditor5-engine/src/controller/datacontroller.js @@ -241,27 +241,16 @@ export default class DataController { this.mapper.bindElements( modelElementOrFragment, viewDocumentFragment ); - // Make additional options available during conversion process through `conversionApi`. - this.downcastDispatcher.conversionApi.options = options; - - // We have no view controller and rendering to DOM in DataController so view.change() block is not used here. - this.downcastDispatcher.convertInsert( modelRange, viewWriter ); - - // Convert markers. + // Prepare list of markers. // For document fragment, simply take the markers assigned to this document fragment. // For model root, all markers in that root will be taken. // For model element, we need to check which markers are intersecting with this element and relatively modify the markers' ranges. // Collapsed markers at element boundary, although considered as not intersecting with the element, will also be returned. const markers = modelElementOrFragment.is( 'documentFragment' ) ? - Array.from( modelElementOrFragment.markers ) : + modelElementOrFragment.markers : _getMarkersRelativeToElement( modelElementOrFragment ); - for ( const [ name, range ] of markers ) { - this.downcastDispatcher.convertMarkerAdd( name, range, viewWriter ); - } - - // Clean `conversionApi`. - delete this.downcastDispatcher.conversionApi.options; + this.downcastDispatcher.convert( modelRange, markers, viewWriter, options ); return viewDocumentFragment; } @@ -533,11 +522,11 @@ mix( DataController, ObservableMixin ); // at element boundary, it is considered as contained inside the element and marker range is returned. Otherwise, if the marker is // intersecting with the element, the intersection is returned. function _getMarkersRelativeToElement( element ) { - const result = []; + const result = new Map(); const doc = element.root.document; if ( !doc ) { - return []; + return result; } const elementRange = ModelRange._createIn( element ); @@ -549,12 +538,12 @@ function _getMarkersRelativeToElement( element ) { const isMarkerAtElementBoundary = markerRange.start.isEqual( elementRange.start ) || markerRange.end.isEqual( elementRange.end ); if ( isMarkerCollapsed && isMarkerAtElementBoundary ) { - result.push( [ marker.name, markerRange ] ); + result.set( marker.name, markerRange ); } else { const updatedMarkerRange = elementRange.getIntersection( markerRange ); if ( updatedMarkerRange ) { - result.push( [ marker.name, updatedMarkerRange ] ); + result.set( marker.name, updatedMarkerRange ); } } } diff --git a/packages/ckeditor5-engine/src/conversion/conversion.js b/packages/ckeditor5-engine/src/conversion/conversion.js index a75acb6556f..d812576d2fb 100644 --- a/packages/ckeditor5-engine/src/conversion/conversion.js +++ b/packages/ckeditor5-engine/src/conversion/conversion.js @@ -140,6 +140,7 @@ export default class Conversion { * * downcast (model-to-view) conversion helpers: * * * {@link module:engine/conversion/downcasthelpers~DowncastHelpers#elementToElement `elementToElement()`}, + * * {@link module:engine/conversion/downcasthelpers~DowncastHelpers#elementToStructure `elementToStructure()`}, * * {@link module:engine/conversion/downcasthelpers~DowncastHelpers#attributeToElement `attributeToElement()`}, * * {@link module:engine/conversion/downcasthelpers~DowncastHelpers#attributeToAttribute `attributeToAttribute()`}. * * {@link module:engine/conversion/downcasthelpers~DowncastHelpers#markerToElement `markerToElement()`}. diff --git a/packages/ckeditor5-engine/src/conversion/downcastdispatcher.js b/packages/ckeditor5-engine/src/conversion/downcastdispatcher.js index 54923ce26a5..b3aee26998c 100644 --- a/packages/ckeditor5-engine/src/conversion/downcastdispatcher.js +++ b/packages/ckeditor5-engine/src/conversion/downcastdispatcher.js @@ -9,7 +9,6 @@ import Consumable from './modelconsumable'; import Range from '../model/range'; -import Position, { getNodeAfterPosition, getTextNodeAtPosition } from '../model/position'; import EmitterMixin from '@ckeditor/ckeditor5-utils/src/emittermixin'; import mix from '@ckeditor/ckeditor5-utils/src/mix'; @@ -49,8 +48,9 @@ import mix from '@ckeditor/ckeditor5-utils/src/mix'; * * Additionally, downcast dispatcher fires events for {@link module:engine/model/markercollection~Marker marker} changes: * - * * {@link module:engine/conversion/downcastdispatcher~DowncastDispatcher#event:addMarker} – If a marker was added. - * * {@link module:engine/conversion/downcastdispatcher~DowncastDispatcher#event:removeMarker} – If a marker was removed. + * * {@link module:engine/conversion/downcastdispatcher~DowncastDispatcher#event:addMarker `addMarker`} – If a marker was added. + * * {@link module:engine/conversion/downcastdispatcher~DowncastDispatcher#event:removeMarker `removeMarker`} – If a marker was + * removed. * * Note that changing a marker is done through removing the marker from the old range and adding it to the new range, * so both events are fired. @@ -58,11 +58,11 @@ import mix from '@ckeditor/ckeditor5-utils/src/mix'; * Finally, downcast dispatcher also handles firing events for the {@link module:engine/model/selection model selection} * conversion: * - * * {@link module:engine/conversion/downcastdispatcher~DowncastDispatcher#event:selection} + * * {@link module:engine/conversion/downcastdispatcher~DowncastDispatcher#event:selection `selection`} * – Converts the selection from the model to the view. - * * {@link module:engine/conversion/downcastdispatcher~DowncastDispatcher#event:attribute} + * * {@link module:engine/conversion/downcastdispatcher~DowncastDispatcher#event:attribute `attribute`} * – Fired for every selection attribute. - * * {@link module:engine/conversion/downcastdispatcher~DowncastDispatcher#event:addMarker} + * * {@link module:engine/conversion/downcastdispatcher~DowncastDispatcher#event:addMarker `addMarker`} * – Fired for every marker that contains a selection. * * Unlike the model tree and the markers, the events for selection are not fired for changes but for a selection state. @@ -118,206 +118,88 @@ export default class DowncastDispatcher { */ constructor( conversionApi ) { /** - * An interface passed by the dispatcher to the event callbacks. + * A template for an interface passed by the dispatcher to the event callbacks. * + * @protected * @member {module:engine/conversion/downcastdispatcher~DowncastConversionApi} */ - this.conversionApi = Object.assign( { dispatcher: this }, conversionApi ); - - /** - * Maps conversion event names that will trigger element reconversion for a given element name. - * - * @type {Map} - * @private - */ - this._reconversionEventsMapping = new Map(); + this._conversionApi = { dispatcher: this, ...conversionApi }; } /** - * Takes a {@link module:engine/model/differ~Differ model differ} object with buffered changes and fires conversion basing on it. + * Converts changes buffered in the given {@link module:engine/model/differ~Differ model differ} + * and fires conversion events based on it. * + * @fires insert + * @fires remove + * @fires attribute + * @fires addMarker + * @fires removeMarker + * @fires reducedChanges * @param {module:engine/model/differ~Differ} differ The differ object with buffered changes. - * @param {module:engine/model/markercollection~MarkerCollection} markers Markers connected with the converted model. + * @param {module:engine/model/markercollection~MarkerCollection} markers Markers related to the model fragment to convert. * @param {module:engine/view/downcastwriter~DowncastWriter} writer The view writer that should be used to modify the view document. */ convertChanges( differ, markers, writer ) { + const conversionApi = this._createConversionApi( writer ); + // Before the view is updated, remove markers which have changed. for ( const change of differ.getMarkersToRemove() ) { - this.convertMarkerRemove( change.name, change.range, writer ); + this._convertMarkerRemove( change.name, change.range, conversionApi ); } - const changes = this._mapChangesWithAutomaticReconversion( differ ); + // Let features modify the change list (for example to allow reconversion). + const changes = this._reduceChanges( differ.getChanges() ); // Convert changes that happened on model tree. for ( const entry of changes ) { if ( entry.type === 'insert' ) { - this.convertInsert( Range._createFromPositionAndShift( entry.position, entry.length ), writer ); + this._convertInsert( Range._createFromPositionAndShift( entry.position, entry.length ), conversionApi ); + } else if ( entry.type === 'reinsert' ) { + this._convertReinsert( Range._createFromPositionAndShift( entry.position, entry.length ), conversionApi ); } else if ( entry.type === 'remove' ) { - this.convertRemove( entry.position, entry.length, entry.name, writer ); - } else if ( entry.type === 'reconvert' ) { - this.reconvertElement( entry.element, writer ); + this._convertRemove( entry.position, entry.length, entry.name, conversionApi ); } else { // Defaults to 'attribute' change. - this.convertAttribute( entry.range, entry.attributeKey, entry.attributeOldValue, entry.attributeNewValue, writer ); + this._convertAttribute( entry.range, entry.attributeKey, entry.attributeOldValue, entry.attributeNewValue, conversionApi ); } } - for ( const markerName of this.conversionApi.mapper.flushUnboundMarkerNames() ) { + for ( const markerName of conversionApi.mapper.flushUnboundMarkerNames() ) { const markerRange = markers.get( markerName ).getRange(); - this.convertMarkerRemove( markerName, markerRange, writer ); - this.convertMarkerAdd( markerName, markerRange, writer ); + this._convertMarkerRemove( markerName, markerRange, conversionApi ); + this._convertMarkerAdd( markerName, markerRange, conversionApi ); } // After the view is updated, convert markers which have changed. for ( const change of differ.getMarkersToAdd() ) { - this.convertMarkerAdd( change.name, change.range, writer ); - } - } - - /** - * Starts a conversion of a range insertion. - * - * For each node in the range, {@link #event:insert `insert` event is fired}. For each attribute on each node, - * {@link #event:attribute `attribute` event is fired}. - * - * @fires insert - * @fires attribute - * @param {module:engine/model/range~Range} range The inserted range. - * @param {module:engine/view/downcastwriter~DowncastWriter} writer The view writer that should be used to modify the view document. - */ - convertInsert( range, writer ) { - this.conversionApi.writer = writer; - - // Create a list of things that can be consumed, consisting of nodes and their attributes. - this.conversionApi.consumable = this._createInsertConsumable( range ); - - // Fire a separate insert event for each node and text fragment contained in the range. - for ( const data of Array.from( range ).map( walkerValueToEventData ) ) { - this._convertInsertWithAttributes( data ); + this._convertMarkerAdd( change.name, change.range, conversionApi ); } - this._clearConversionApi(); - } - - /** - * Fires conversion of a single node removal. Fires {@link #event:remove remove event} with provided data. - * - * @param {module:engine/model/position~Position} position Position from which node was removed. - * @param {Number} length Offset size of removed node. - * @param {String} name Name of removed node. - * @param {module:engine/view/downcastwriter~DowncastWriter} writer View writer that should be used to modify view document. - */ - convertRemove( position, length, name, writer ) { - this.conversionApi.writer = writer; - - this.fire( 'remove:' + name, { position, length }, this.conversionApi ); - - this._clearConversionApi(); + // Remove mappings for all removed view elements. + conversionApi.mapper.flushDeferredBindings(); } /** - * Starts a conversion of an attribute change on a given `range`. - * - * For each node in the given `range`, {@link #event:attribute attribute event} is fired with the passed data. - * - * @fires attribute - * @param {module:engine/model/range~Range} range Changed range. - * @param {String} key Key of the attribute that has changed. - * @param {*} oldValue Attribute value before the change or `null` if the attribute has not been set before. - * @param {*} newValue New attribute value or `null` if the attribute has been removed. - * @param {module:engine/view/downcastwriter~DowncastWriter} writer View writer that should be used to modify view document. - */ - convertAttribute( range, key, oldValue, newValue, writer ) { - this.conversionApi.writer = writer; - - // Create a list with attributes to consume. - this.conversionApi.consumable = this._createConsumableForRange( range, `attribute:${ key }` ); - - // Create a separate attribute event for each node in the range. - for ( const value of range ) { - const item = value.item; - const itemRange = Range._createFromPositionAndShift( value.previousPosition, value.length ); - const data = { - item, - range: itemRange, - attributeKey: key, - attributeOldValue: oldValue, - attributeNewValue: newValue - }; - - this._testAndFire( `attribute:${ key }`, data ); - } - - this._clearConversionApi(); - } - - /** - * Starts the reconversion of an element. It will: - * - * * Fire an {@link #event:insert `insert` event} for the element to reconvert. - * * Fire an {@link #event:attribute `attribute` event} for element attributes. - * - * This will not reconvert children of the element if they have existing (already converted) views. For newly inserted child elements - * it will behave the same as {@link #convertInsert}. - * - * Element reconversion is defined by the `triggerBy` configuration for the - * {@link module:engine/conversion/downcasthelpers~DowncastHelpers#elementToElement `elementToElement()`} conversion helper. + * Starts a conversion of a model range and the provided markers. * * @fires insert * @fires attribute - * @param {module:engine/model/element~Element} element The element to be reconverted. + * @fires addMarker + * @param {module:engine/model/range~Range} range The inserted range. + * @param {Map} markers The map of markers that should be down-casted. * @param {module:engine/view/downcastwriter~DowncastWriter} writer The view writer that should be used to modify the view document. + * @param {Object} [options] Optional options object passed to `convertionApi.options`. */ - reconvertElement( element, writer ) { - const elementRange = Range._createOn( element ); - - this.conversionApi.writer = writer; - - // Create a list of things that can be consumed, consisting of nodes and their attributes. - this.conversionApi.consumable = this._createInsertConsumable( elementRange ); - - const mapper = this.conversionApi.mapper; - const currentView = mapper.toViewElement( element ); - - // Remove the old view but do not remove mapper mappings - those will be used to revive existing elements. - writer.remove( currentView ); - - // Convert the element - without converting children. - this._convertInsertWithAttributes( { - item: element, - range: elementRange - } ); - - const convertedViewElement = mapper.toViewElement( element ); + convert( range, markers, writer, options = {} ) { + const conversionApi = this._createConversionApi( writer, options ); - // Iterate over children of reconverted element in order to... - for ( const value of Range._createIn( element ) ) { - const { item } = value; + this._convertInsert( range, conversionApi ); - const view = elementOrTextProxyToView( item, mapper ); - - // ...either bring back previously converted view... - if ( view ) { - // Do not move views that are already in converted element - those might be created by the main element converter in case - // when main element converts also its direct children. - if ( view.root !== convertedViewElement.root ) { - writer.move( - writer.createRangeOn( view ), - mapper.toViewPosition( Position._createBefore( item ) ) - ); - } - } - // ... or by converting newly inserted elements. - else { - this._convertInsertWithAttributes( walkerValueToEventData( value ) ); - } + for ( const [ name, range ] of markers ) { + this._convertMarkerAdd( name, range, conversionApi ); } - - // After reconversion is done we can unbind the old view. - mapper.unbindViewElement( currentView ); - - this._clearConversionApi(); } /** @@ -335,21 +217,20 @@ export default class DowncastDispatcher { convertSelection( selection, markers, writer ) { const markersAtSelection = Array.from( markers.getMarkersAtPosition( selection.getFirstPosition() ) ); - this.conversionApi.writer = writer; - this.conversionApi.consumable = this._createSelectionConsumable( selection, markersAtSelection ); + const conversionApi = this._createConversionApi( writer ); - this.fire( 'selection', { selection }, this.conversionApi ); + this._addConsumablesForSelection( conversionApi.consumable, selection, markersAtSelection ); - if ( !selection.isCollapsed ) { - this._clearConversionApi(); + this.fire( 'selection', { selection }, conversionApi ); + if ( !selection.isCollapsed ) { return; } for ( const marker of markersAtSelection ) { const markerRange = marker.getRange(); - if ( !shouldMarkerChangeBeConverted( selection.getFirstPosition(), marker, this.conversionApi.mapper ) ) { + if ( !shouldMarkerChangeBeConverted( selection.getFirstPosition(), marker, conversionApi.mapper ) ) { continue; } @@ -359,8 +240,8 @@ export default class DowncastDispatcher { markerRange }; - if ( this.conversionApi.consumable.test( selection, 'addMarker:' + marker.name ) ) { - this.fire( 'addMarker:' + marker.name, data, this.conversionApi ); + if ( conversionApi.consumable.test( selection, 'addMarker:' + marker.name ) ) { + this.fire( 'addMarker:' + marker.name, data, conversionApi ); } } @@ -374,130 +255,217 @@ export default class DowncastDispatcher { }; // Do not fire event if the attribute has been consumed. - if ( this.conversionApi.consumable.test( selection, 'attribute:' + data.attributeKey ) ) { - this.fire( 'attribute:' + data.attributeKey + ':$text', data, this.conversionApi ); + if ( conversionApi.consumable.test( selection, 'attribute:' + data.attributeKey ) ) { + this.fire( 'attribute:' + data.attributeKey + ':$text', data, conversionApi ); } } + } + + /** + * Fires insertion conversion of a range of nodes. + * + * For each node in the range, {@link #event:insert `insert` event is fired}. For each attribute on each node, + * {@link #event:attribute `attribute` event is fired}. + * + * @protected + * @fires insert + * @fires attribute + * @param {module:engine/model/range~Range} range The inserted range. + * @param {module:engine/conversion/downcastdispatcher~DowncastConversionApi} conversionApi The conversion API object. + */ + _convertInsert( range, conversionApi ) { + const walkerValues = Array.from( range ); + + // Collect a list of things that can be consumed, consisting of nodes and their attributes. + this._addConsumablesForInsert( conversionApi.consumable, walkerValues ); - this._clearConversionApi(); + // Fire a separate insert event for each node and text fragment contained in the range. + for ( const data of walkerValues.map( walkerValueToEventData ) ) { + this._convertInsertWithAttributes( data, conversionApi ); + } + } + + /** + * Fires conversion of a single node removal. Fires {@link #event:remove remove event} with provided data. + * + * @protected + * @param {module:engine/model/position~Position} position Position from which node was removed. + * @param {Number} length Offset size of removed node. + * @param {String} name Name of removed node. + * @param {module:engine/conversion/downcastdispatcher~DowncastConversionApi} conversionApi The conversion API object. + */ + _convertRemove( position, length, name, conversionApi ) { + this.fire( 'remove:' + name, { position, length }, conversionApi ); + } + + /** + * Starts a conversion of an attribute change on a given `range`. + * + * For each node in the given `range`, {@link #event:attribute attribute event} is fired with the passed data. + * + * @protected + * @fires attribute + * @param {module:engine/model/range~Range} range Changed range. + * @param {String} key Key of the attribute that has changed. + * @param {*} oldValue Attribute value before the change or `null` if the attribute has not been set before. + * @param {*} newValue New attribute value or `null` if the attribute has been removed. + * @param {module:engine/conversion/downcastdispatcher~DowncastConversionApi} conversionApi The conversion API object. + */ + _convertAttribute( range, key, oldValue, newValue, conversionApi ) { + // Create a list with attributes to consume. + this._addConsumablesForRange( conversionApi.consumable, range, `attribute:${ key }` ); + + // Create a separate attribute event for each node in the range. + for ( const value of range ) { + const item = value.item; + const itemRange = Range._createFromPositionAndShift( value.previousPosition, value.length ); + const data = { + item, + range: itemRange, + attributeKey: key, + attributeOldValue: oldValue, + attributeNewValue: newValue + }; + + this._testAndFire( `attribute:${ key }`, data, conversionApi ); + } + } + + /** + * Fires re-insertion conversion (with a `reconversion` flag passed to `insert` events) + * of a range of elements (only elements on the range depth, without children). + * + * For each node in the range on its depth (without children), {@link #event:insert `insert` event} is fired. + * For each attribute on each node, {@link #event:attribute `attribute` event} is fired. + * + * @protected + * @fires insert + * @fires attribute + * @param {module:engine/model/range~Range} range The range to reinsert. + * @param {module:engine/conversion/downcastdispatcher~DowncastConversionApi} conversionApi The conversion API object. + */ + _convertReinsert( range, conversionApi ) { + // Convert the elements - without converting children. + const walkerValues = Array.from( range.getWalker( { shallow: true } ) ); + + // Collect a list of things that can be consumed, consisting of nodes and their attributes. + this._addConsumablesForInsert( conversionApi.consumable, walkerValues ); + + // Fire a separate insert event for each node and text fragment contained shallowly in the range. + for ( const data of walkerValues.map( walkerValueToEventData ) ) { + this._convertInsertWithAttributes( { ...data, reconversion: true }, conversionApi ); + } } /** * Converts the added marker. Fires the {@link #event:addMarker `addMarker`} event for each item * in the marker's range. If the range is collapsed, a single event is dispatched. See the event description for more details. * + * @protected * @fires addMarker * @param {String} markerName Marker name. * @param {module:engine/model/range~Range} markerRange The marker range. - * @param {module:engine/view/downcastwriter~DowncastWriter} writer View writer that should be used to modify the view document. + * @param {module:engine/conversion/downcastdispatcher~DowncastConversionApi} conversionApi The conversion API object. */ - convertMarkerAdd( markerName, markerRange, writer ) { + _convertMarkerAdd( markerName, markerRange, conversionApi ) { // Do not convert if range is in graveyard. if ( markerRange.root.rootName == '$graveyard' ) { return; } - this.conversionApi.writer = writer; - // In markers' case, event name == consumable name. const eventName = 'addMarker:' + markerName; // // First, fire an event for the whole marker. // - const consumable = new Consumable(); - consumable.add( markerRange, eventName ); - - this.conversionApi.consumable = consumable; + conversionApi.consumable.add( markerRange, eventName ); - this.fire( eventName, { markerName, markerRange }, this.conversionApi ); + this.fire( eventName, { markerName, markerRange }, conversionApi ); // // Do not fire events for each item inside the range if the range got consumed. + // Also consume the whole marker consumable if it wasn't consumed. // - if ( !consumable.test( markerRange, eventName ) ) { - this._clearConversionApi(); - + if ( !conversionApi.consumable.consume( markerRange, eventName ) ) { return; } // // Then, fire an event for each item inside the marker range. // - this.conversionApi.consumable = this._createConsumableForRange( markerRange, eventName ); + this._addConsumablesForRange( conversionApi.consumable, markerRange, eventName ); for ( const item of markerRange.getItems() ) { // Do not fire event for already consumed items. - if ( !this.conversionApi.consumable.test( item, eventName ) ) { + if ( !conversionApi.consumable.test( item, eventName ) ) { continue; } const data = { item, range: Range._createOn( item ), markerName, markerRange }; - this.fire( eventName, data, this.conversionApi ); + this.fire( eventName, data, conversionApi ); } - - this._clearConversionApi(); } /** * Fires the conversion of the marker removal. Fires the {@link #event:removeMarker `removeMarker`} event with the provided data. * + * @protected * @fires removeMarker * @param {String} markerName Marker name. * @param {module:engine/model/range~Range} markerRange The marker range. - * @param {module:engine/view/downcastwriter~DowncastWriter} writer View writer that should be used to modify the view document. + * @param {module:engine/conversion/downcastdispatcher~DowncastConversionApi} conversionApi The conversion API object. */ - convertMarkerRemove( markerName, markerRange, writer ) { + _convertMarkerRemove( markerName, markerRange, conversionApi ) { // Do not convert if range is in graveyard. if ( markerRange.root.rootName == '$graveyard' ) { return; } - this.conversionApi.writer = writer; - - this.fire( 'removeMarker:' + markerName, { markerName, markerRange }, this.conversionApi ); - - this._clearConversionApi(); + this.fire( 'removeMarker:' + markerName, { markerName, markerRange }, conversionApi ); } /** - * Maps the model element "insert" reconversion for given event names. The event names must be fully specified: - * - * * For "attribute" change event, it should include the main element name, i.e: `'attribute:attributeName:elementName'`. - * * For child node change events, these should use the child event name as well, i.e: - * * For adding a node: `'insert:childElementName'`. - * * For removing a node: `'remove:childElementName'`. + * Fires the reduction of changes buffered in the {@link module:engine/model/differ~Differ `Differ`}. * - * **Note**: This method should not be used directly. The reconversion is defined by the `triggerBy()` configuration of the - * `elementToElement()` conversion helper. + * Features can replace selected {@link module:engine/model/differ~DiffItem `DiffItem`}s with `reinsert` entries to trigger + * reconversion. The {@link module:engine/conversion/downcasthelpers~DowncastHelpers#elementToStructure + * `DowncastHelpers.elementToStructure()`} is using this event to trigger reconversion. * - * @protected - * @param {String} modelName The name of the main model element for which the events will trigger the reconversion. - * @param {String} eventName The name of an event that would trigger conversion for a given model element. + * @private + * @fires reduceChanges + * @param {Iterable.} changes + * @returns {Iterable.} */ - _mapReconversionTriggerEvent( modelName, eventName ) { - this._reconversionEventsMapping.set( eventName, modelName ); + _reduceChanges( changes ) { + const data = { changes }; + + this.fire( 'reduceChanges', data ); + + return data.changes; } /** - * Creates {@link module:engine/conversion/modelconsumable~ModelConsumable} with values to consume from a given range, + * Populates provided {@link module:engine/conversion/modelconsumable~ModelConsumable} with values to consume from a given range, * assuming that the range has just been inserted to the model. * * @private - * @param {module:engine/model/range~Range} range The inserted range. + * @param {module:engine/conversion/modelconsumable~ModelConsumable} consumable The consumable. + * @param {Iterable.} walkerValues The walker values for the inserted range. * @returns {module:engine/conversion/modelconsumable~ModelConsumable} The values to consume. */ - _createInsertConsumable( range ) { - const consumable = new Consumable(); - - for ( const value of range ) { + _addConsumablesForInsert( consumable, walkerValues ) { + for ( const value of walkerValues ) { const item = value.item; - consumable.add( item, 'insert' ); + // Add consumable if it wasn't there yet. + if ( consumable.test( item, 'insert' ) === null ) { + consumable.add( item, 'insert' ); - for ( const key of item.getAttributeKeys() ) { - consumable.add( item, 'attribute:' + key ); + for ( const key of item.getAttributeKeys() ) { + consumable.add( item, 'attribute:' + key ); + } } } @@ -505,16 +473,15 @@ export default class DowncastDispatcher { } /** - * Creates {@link module:engine/conversion/modelconsumable~ModelConsumable} with values to consume for a given range. + * Populates provided {@link module:engine/conversion/modelconsumable~ModelConsumable} with values to consume for a given range. * * @private + * @param {module:engine/conversion/modelconsumable~ModelConsumable} consumable The consumable. * @param {module:engine/model/range~Range} range The affected range. * @param {String} type Consumable type. * @returns {module:engine/conversion/modelconsumable~ModelConsumable} The values to consume. */ - _createConsumableForRange( range, type ) { - const consumable = new Consumable(); - + _addConsumablesForRange( consumable, range, type ) { for ( const item of range.getItems() ) { consumable.add( item, type ); } @@ -523,16 +490,15 @@ export default class DowncastDispatcher { } /** - * Creates {@link module:engine/conversion/modelconsumable~ModelConsumable} with selection consumable values. + * Populates provided {@link module:engine/conversion/modelconsumable~ModelConsumable} with selection consumable values. * * @private + * @param {module:engine/conversion/modelconsumable~ModelConsumable} consumable The consumable. * @param {module:engine/model/selection~Selection} selection The selection to create the consumable from. * @param {Iterable.} markers Markers that contain the selection. * @returns {module:engine/conversion/modelconsumable~ModelConsumable} The values to consume. */ - _createSelectionConsumable( selection, markers ) { - const consumable = new Consumable(); - + _addConsumablesForSelection( consumable, selection, markers ) { consumable.add( selection, 'selection' ); for ( const marker of markers ) { @@ -554,24 +520,36 @@ export default class DowncastDispatcher { * @fires attribute * @param {String} type Event type. * @param {Object} data Event data. + * @param {module:engine/conversion/downcastdispatcher~DowncastConversionApi} conversionApi The conversion API object. */ - _testAndFire( type, data ) { - if ( !this.conversionApi.consumable.test( data.item, type ) ) { + _testAndFire( type, data, conversionApi ) { + if ( !conversionApi.consumable.test( data.item, type ) ) { // Do not fire event if the item was consumed. return; } - this.fire( getEventName( type, data ), data, this.conversionApi ); + this.fire( getEventName( type, data ), data, conversionApi ); } /** - * Clears the conversion API object. + * Builds an instance of the {@link module:engine/conversion/downcastdispatcher~DowncastConversionApi} from a template and a given + * {@link module:engine/view/downcastwriter~DowncastWriter `DowncastWriter`} and options object. * * @private + * @param {module:engine/view/downcastwriter~DowncastWriter} writer View writer that should be used to modify the view document. + * @param {Object} [options] Optional options passed to `convertionApi.options`. + * @return {module:engine/conversion/downcastdispatcher~DowncastConversionApi} The conversion API object. */ - _clearConversionApi() { - delete this.conversionApi.writer; - delete this.conversionApi.consumable; + _createConversionApi( writer, options = {} ) { + const conversionApi = { + ...this._conversionApi, + consumable: new Consumable(), + writer, + options, + convertInsert: range => this._convertInsert( range, conversionApi ) + }; + + return conversionApi; } /** @@ -581,9 +559,10 @@ export default class DowncastDispatcher { * @fires insert * @fires attribute * @param {Object} data Event data. + * @param {module:engine/conversion/downcastdispatcher~DowncastConversionApi} conversionApi The conversion API object. */ - _convertInsertWithAttributes( data ) { - this._testAndFire( 'insert', data ); + _convertInsertWithAttributes( data, conversionApi ) { + this._testAndFire( 'insert', data, conversionApi ); // Fire a separate addAttribute event for each attribute that was set on inserted items. // This is important because most attributes converters will listen only to add/change/removeAttribute events. @@ -593,106 +572,24 @@ export default class DowncastDispatcher { data.attributeOldValue = null; data.attributeNewValue = data.item.getAttribute( key ); - this._testAndFire( `attribute:${ key }`, data ); + this._testAndFire( `attribute:${ key }`, data, conversionApi ); } } /** - * Returns differ changes together with added "reconvert" type changes for {@link #reconvertElement}. These are defined by - * a the `triggerBy()` configuration for the - * {@link module:engine/conversion/downcasthelpers~DowncastHelpers#elementToElement `elementToElement()`} conversion helper. - * - * This method will remove every mapped insert or remove change with a single "reconvert" change. - * - * For instance: Having a `triggerBy()` configuration defined for the `` element that issues this element reconversion on - * `foo` and `bar` attributes change, and a set of changes for this element: - * - * const differChanges = [ - * { type: 'attribute', attributeKey: 'foo', ... }, - * { type: 'attribute', attributeKey: 'bar', ... }, - * { type: 'attribute', attributeKey: 'baz', ... } - * ]; - * - * This method will return: - * - * const updatedChanges = [ - * { type: 'reconvert', element: complexElementInstance }, - * { type: 'attribute', attributeKey: 'baz', ... } - * ]; - * - * In the example above, the `'baz'` attribute change will fire an {@link #event:attribute attribute event} - * - * @param {module:engine/model/differ~Differ} differ The differ object with buffered changes. - * @returns {Array.} Updated set of changes. - * @private + * Fired to enable reducing (transforming) changes buffered in the {@link module:engine/model/differ~Differ `Differ`} before + * {@link #convertChanges `convertChanges()`} will fire any conversion events. + * + * For instance, a feature can replace selected {@link module:engine/model/differ~DiffItem `DiffItem`}s with a `reinsert` entry + * to trigger reconversion of an element when e.g. its attribute has changes. + * The {@link module:engine/conversion/downcasthelpers~DowncastHelpers#elementToStructure + * `DowncastHelpers.elementToStructure()`} helper is using this event to trigger reconversion of an element when the element, + * its attributes or direct children changed. + * + * @param {Object} data + * @param {Iterable.} data.changes A buffered changes to get reduced. + * @event reduceChanges */ - _mapChangesWithAutomaticReconversion( differ ) { - const itemsToReconvert = new Set(); - const updated = []; - - for ( const entry of differ.getChanges() ) { - const position = entry.position || entry.range.start; - // Cached parent - just in case. See https://github.com/ckeditor/ckeditor5/issues/6579. - const positionParent = position.parent; - const textNode = getTextNodeAtPosition( position, positionParent ); - - // Reconversion is done only on elements so skip text changes. - if ( textNode ) { - updated.push( entry ); - - continue; - } - - const element = entry.type === 'attribute' ? getNodeAfterPosition( position, positionParent, null ) : positionParent; - - // Case of text node set directly in root. For now used only in tests but can be possible when enabled in paragraph-like roots. - // See: https://github.com/ckeditor/ckeditor5/issues/762. - if ( element.is( '$text' ) ) { - updated.push( entry ); - - continue; - } - - let eventName; - - if ( entry.type === 'attribute' ) { - eventName = `attribute:${ entry.attributeKey }:${ element.name }`; - } else { - eventName = `${ entry.type }:${ entry.name }`; - } - - if ( this._isReconvertTriggerEvent( eventName, element.name ) ) { - if ( itemsToReconvert.has( element ) ) { - // Element is already reconverted, so skip this change. - continue; - } - - itemsToReconvert.add( element ); - - // Add special "reconvert" change. - updated.push( { type: 'reconvert', element } ); - } else { - updated.push( entry ); - } - } - - return updated; - } - - /** - * Checks if the resulting change should trigger element reconversion. - * - * These are defined by a `triggerBy()` configuration for the - * {@link module:engine/conversion/downcasthelpers~DowncastHelpers#elementToElement `elementToElement()`} conversion helper. - * - * @private - * @param {String} eventName The event name to check. - * @param {String} elementName The element name to check. - * @returns {Boolean} - */ - _isReconvertTriggerEvent( eventName, elementName ) { - return this._reconversionEventsMapping.get( eventName ) === elementName; - } /** * Fired for inserted nodes. @@ -857,17 +754,6 @@ function walkerValueToEventData( value ) { }; } -function elementOrTextProxyToView( item, mapper ) { - if ( item.is( 'textProxy' ) ) { - const mappedPosition = mapper.toViewPosition( Position._createBefore( item ) ); - const positionParent = mappedPosition.parent; - - return positionParent.is( '$text' ) ? positionParent : null; - } - - return mapper.toViewElement( item ); -} - /** * Conversion interface that is registered for given {@link module:engine/conversion/downcastdispatcher~DowncastDispatcher} * and is passed as one of parameters when {@link module:engine/conversion/downcastdispatcher~DowncastDispatcher dispatcher} @@ -907,6 +793,14 @@ function elementOrTextProxyToView( item, mapper ) { * @member {module:engine/view/downcastwriter~DowncastWriter} #writer */ +/** + * Triggers conversion of a specified range. + * This conversion is triggered within (as a separate process of) the parent conversion. + * + * @method #convertInsert + * @param {module:engine/model/range~Range} range The range to trigger nested insert conversion on. + */ + /** * An object with an additional configuration which can be used during the conversion process. Available only for data downcast conversion. * diff --git a/packages/ckeditor5-engine/src/conversion/downcasthelpers.js b/packages/ckeditor5-engine/src/conversion/downcasthelpers.js index 229622749af..9f87d46b1b7 100644 --- a/packages/ckeditor5-engine/src/conversion/downcasthelpers.js +++ b/packages/ckeditor5-engine/src/conversion/downcasthelpers.js @@ -12,6 +12,7 @@ import ModelRange from '../model/range'; import ModelSelection from '../model/selection'; import ModelElement from '../model/element'; +import ModelPosition from '../model/position'; import ViewAttributeElement from '../view/attributeelement'; import DocumentSelection from '../model/documentselection'; @@ -96,6 +97,148 @@ export default class DowncastHelpers extends ConversionHelpers { return this.add( downcastElementToElement( config ) ); } + /** + * Model element to view structure (several elements) conversion helper. + * + * This conversion results in creating a view structure with defined one or more slots for the child nodes. + * For example, a model `` may become this structure in the view: + * + *
+ *
+ * ${ slot for table rows } + *
+ * + * + * The children of the model's `` element will be inserted into the `` element. + * If a `elementToElement()` helper was used, the children would be inserted into the `
`. + * + * An example converter that converts the following model structure: + * + * Some text. + * + * into this sturcture in the view: + * + *
+ *

Some text.

+ *
+ * + * would look like this: + * + * editor.conversion.for( 'downcast' ).elementToStructure( { + * model: 'wrappedParagraph', + * view: ( modelElement, conversionApi ) => { + * const { writer, slotFor } = conversionApi; + * + * const wrapperViewElement = writer.createContainerElement( 'div', { class: 'wrapper' } ); + * const paragraphViewElement = writer.createContainerElement( 'p' ); + * + * writer.insert( writer.createPositionAt( wrapperViewElement, 0 ), paragraphViewElement ); + * writer.insert( writer.createPositionAt( paragraphViewElement, 0 ), slotFor( 'children' ) ); + * + * return wrapperViewElement; + * } + * } ); + * + * The `slorFor()` function can also take a callback that allows filtering which children of the model element + * should be converted into this slot. + * + * Imagine a table feature where for this model structure: + * + *
+ * ... table cells 1 ... + * ... table cells 2 ... + * ... table cells 3 ... + * + *
Caption text
+ * + * We want to generate this view structure: + * + *
+ * + * + * ... table cells 1 ... + * + * + * ... table cells 2 ... + * ... table cells 3 ... + * + *
+ *
Caption text
+ *
+ * + * The converter has to take `headingRows` attribute into consideration when allocating `` elements + * into the `` and `` elements. Hence, we need two slots and define proper filter callbacks for them. + * + * Additionally, all other elements than `` should be placed outside ``. In the example above, this will + * handle the table caption. + * + * Such a converter would look like this: + * + * editor.conversion.for( 'downcast' ).elementToStructure( { + * model: { + * name: 'table', + * attributes: [ 'headingRows' ], + * children: true + * }, + * view: ( modelElement, conversionApi ) => { + * const { writer, slotFor } = conversionApi; + * + * const figureElement = writer.createContainerElement( 'figure', { class: 'table' } ); + * const tableElement = writer.createContainerElement( 'table' ); + * + * writer.insert( writer.createPositionAt( figureElement, 0 ), tableElement ); + * + * const headingRows = modelElement.getAttribute( 'headingRows' ) || 0; + * + * if ( headingRows > 0 ) { + * const tableHead = writer.createContainerElement( 'thead' ); + * + * const headSlot = slotFor( element => element.is( 'element', 'tableRow' ) && element.index < headingRows ); + * + * writer.insert( writer.createPositionAt( tableElement, 'end' ), tableHead ); + * writer.insert( writer.createPositionAt( tableHead, 0 ), headSlot ); + * } + * + * if ( headingRows < tableUtils.getRows( table ) ) { + * const tableBody = writer.createContainerElement( 'tbody' ); + * + * const bodySlot = slotFor( element => element.is( 'element', 'tableRow' ) && element.index >= headingRows ); + * + * writer.insert( writer.createPositionAt( tableElement, 'end' ), tableBody ); + * writer.insert( writer.createPositionAt( tableBody, 0 ), bodySlot ); + * } + * + * const restSlot = slotFor( element => !element.is( 'element', 'tableRow' ) ); + * + * writer.insert( writer.createPositionAt( figureElement, 'end' ), restSlot ); + * + * return figureElement; + * } + * } ); + * + * Note: The children of a model element that's being converted must be allocated in the same order in the view + * in which they are placed in the model. + * + * See {@link module:engine/conversion/conversion~Conversion#for `conversion.for()`} to learn how to add a converter + * to the conversion process. + * + * @method #elementToStructure + * @param {Object} config Conversion configuration. + * @param {String|Object} config.model The description or a name of the model element to convert. + * @param {String} [config.model.name] The name of the model element to convert. + * @param {Array.} [config.model.attributes] The list of attribute names that should be consumed while creating + * the view structure. Note that the view will be reconverted if any of the listed attributes will change. + * @param {Boolean} [config.model.children] Specifies whether the view structure requires reconversion if the list + * of model child nodes changed. + * @param {module:engine/conversion/downcasthelpers~StructureCreatorFunction} config.view A function + * that takes the model element and {@link module:engine/conversion/downcasthelpers~DowncastConversionWithSlotsApi downcast + * conversion API} as parameters and returns a view container element with slots for model child nodes to be converted into. + * @returns {module:engine/conversion/downcasthelpers~DowncastHelpers} + */ + elementToStructure( config ) { + return this.add( downcastElementToStructure( config ) ); + } + /** * Model attribute to view element conversion helper. * @@ -464,7 +607,7 @@ export default class DowncastHelpers extends ConversionHelpers { * // Model: *
[]Foo
* - * // View: + * // View: *

Foo

* * Similarly, when a marker is collapsed after the last element: @@ -565,7 +708,7 @@ export function remove() { // After the range is removed, unbind all view elements from the model. // Range inside view document fragment is used to unbind deeply. for ( const child of conversionApi.writer.createRangeIn( removed ).getItems() ) { - conversionApi.mapper.unbindViewElement( child ); + conversionApi.mapper.unbindViewElement( child, { defer: true } ); } }; } @@ -789,8 +932,7 @@ export function wrap( elementCreator ) { * It is expected that the function returns an {@link module:engine/view/element~Element}. * The result of the function will be inserted into the view. * - * The converter automatically consumes the corresponding value from the consumables list, stops the event (see - * {@link module:engine/conversion/downcastdispatcher~DowncastDispatcher}) and binds the model and view elements. + * The converter automatically consumes the corresponding value from the consumables list and binds the model and view elements. * * downcastDispatcher.on( * 'insert:myElem', @@ -827,6 +969,53 @@ export function insertElement( elementCreator ) { }; } +/** + * Function factory that creates a converter which converts a single model node insertion to a view structure. + * + * It is expected that the passed element creator function returns an {@link module:engine/view/element~Element} with attached slots + * created with `conversionApi.slotFor()` to indicate where child nodes should be converted. + * + * @see module:engine/conversion/downcasthelpers~DowncastHelpers#elementToStructure + * + * @protected + * @param {module:engine/conversion/downcasthelpers~StructureCreatorFunction} elementCreator Function returning a view structure, + * which will be inserted. + * @param {module:engine/conversion/downcasthelpers~ConsumerFunction} consumer A callback that is expected to consume all the consumables + * that were used by the element creator. + * @returns {Function} Insert element event converter. +*/ +export function insertStructure( elementCreator, consumer ) { + return ( evt, data, conversionApi ) => { + const slotsMap = new Map(); + + // View creation. + const viewElement = elementCreator( data.item, { + ...conversionApi, + slotFor: createSlotFactory( data.item, slotsMap, conversionApi ) + } ); + + if ( !viewElement ) { + return; + } + + // Check if all children are covered by slots and there is no child that landed in multiple slots. + validateSlotsChildren( data.item, slotsMap, conversionApi ); + + // Consume an element insertion and all present attributes that are specified as a reconversion triggers. + if ( !consumer( data.item, conversionApi.consumable ) ) { + return; + } + + const viewPosition = conversionApi.mapper.toViewPosition( data.range.start ); + + conversionApi.mapper.bindElements( data.item, viewElement ); + conversionApi.writer.insert( viewPosition, viewElement ); + + // Fill view slots with previous view elements or create new ones. + fillSlots( viewElement, slotsMap, conversionApi, { reconversion: data.reconversion } ); + }; +} + /** * Function factory that creates a converter which converts marker adding change to the * {@link module:engine/view/uielement~UIElement view UI element}. @@ -1138,10 +1327,7 @@ function changeAttribute( attributeCreator ) { * * @error conversion-attribute-to-attribute-on-text */ - throw new CKEditorError( - 'conversion-attribute-to-attribute-on-text', - [ data, conversionApi ] - ); + throw new CKEditorError( 'conversion-attribute-to-attribute-on-text', conversionApi.dispatcher, data ); } // First remove the old attribute if there was one. @@ -1379,20 +1565,35 @@ function downcastElementToElement( config ) { return dispatcher => { dispatcher.on( 'insert:' + config.model, insertElement( config.view ), { priority: config.converterPriority || 'normal' } ); + }; +} - if ( config.triggerBy ) { - if ( config.triggerBy.attributes ) { - for ( const attributeKey of config.triggerBy.attributes ) { - dispatcher._mapReconversionTriggerEvent( config.model, `attribute:${ attributeKey }:${ config.model }` ); - } - } +// Model element to view structure conversion helper. +// +// See {@link ~DowncastHelpers#elementToStructure `.elementToStructure()` downcast helper} for examples and config params description. +// +// @param {Object} config Conversion configuration. +// @param {String|Object} config.model +// @param {String} [config.model.name] +// @param {Array.} [config.model.attributes] +// @param {Boolean} [config.model.children] +// @param {module:engine/conversion/downcasthelpers~StructureCreatorFunction} config.view +// @returns {Function} Conversion helper. +function downcastElementToStructure( config ) { + config = cloneDeep( config ); - if ( config.triggerBy.children ) { - for ( const childName of config.triggerBy.children ) { - dispatcher._mapReconversionTriggerEvent( config.model, `insert:${ childName }` ); - dispatcher._mapReconversionTriggerEvent( config.model, `remove:${ childName }` ); - } - } + config.model = normalizeModelElementConfig( config.model ); + config.view = normalizeToElementConfig( config.view, 'container' ); + + return dispatcher => { + dispatcher.on( + 'insert:' + config.model.name, + insertStructure( config.view, createConsumer( config.model ) ), + { priority: config.converterPriority || 'normal' } + ); + + if ( config.model.children || config.model.attributes.length ) { + dispatcher.on( 'reduceChanges', createChangeReducer( config.model ), { priority: 'low' } ); } }; } @@ -1541,6 +1742,31 @@ function downcastMarkerToHighlight( config ) { }; } +// Takes `config.model`, and converts it to an object with normalized structure. +// +// @param {String|Object} model Model configuration or element name. +// @param {String} model.name +// @param {Array.} [model.attributes] +// @param {Boolean} [model.children] +// @returns {Object} +function normalizeModelElementConfig( model ) { + if ( typeof model == 'string' ) { + model = { name: model }; + } + + // List of attributes that should trigger reconversion. + if ( !model.attributes ) { + model.attributes = []; + } else if ( !Array.isArray( model.attributes ) ) { + model.attributes = [ model.attributes ]; + } + + // Whether a children insertion/deletion should trigger reconversion. + model.children = !!model.children; + + return model; +} + // Takes `config.view`, and if it is an {@link module:engine/view/elementdefinition~ElementDefinition}, converts it // to a function (because lower level converters accept only element creator functions). // @@ -1670,6 +1896,247 @@ function prepareDescriptor( highlightDescriptor, data, conversionApi ) { return descriptor; } +// Creates a function that checks a single differ diff item whether it should trigger reconversion. +// +// @param {Object} model A normalized `config.model` converter configuration. +// @param {String} model.name The name of element. +// @param {Array.} model.attributes The list of attribute names that should trigger reconversion. +// @param {Boolean} [model.children] Whether the child list change should trigger reconversion. +// @returns {Function} +function createChangeReducerCallback( model ) { + return ( node, change ) => { + if ( !node.is( 'element', model.name ) ) { + return null; + } + + if ( change.type == 'attribute' ) { + if ( !model.attributes.includes( change.attributeKey ) ) { + return null; + } + } else { + if ( !model.children ) { + return null; + } + } + + return [ node ]; + }; +} + +// Creates a `reduceChanges` event handler for reconversion. +// +// @param {Object} model A normalized `config.model` converter configuration. +// @param {String} model.name The name of element. +// @param {Array.} model.attributes The list of attribute names that should trigger reconversion. +// @param {Boolean} [model.children] Whether the child list change should trigger reconversion. +// @param {Function} The callback for checking a single diff item whether it should trigger reconversion. +// @returns {Function} +function createChangeReducer( model, callback = createChangeReducerCallback( model ) ) { + return ( evt, data ) => { + const reducedChanges = []; + + if ( !data.reconvertedElements ) { + data.reconvertedElements = new Set(); + } + + for ( const change of data.changes ) { + // For attribute use node affected by the change. + // For insert or remove use parent element because we need to check if it's added/removed child. + const node = change.position ? change.position.parent : change.range.start.nodeAfter; + + const elements = callback( node, change ); + + if ( !elements ) { + reducedChanges.push( change ); + + continue; + } + + for ( const element of elements ) { + // If it's already marked for reconversion, so skip this change. + if ( data.reconvertedElements.has( element ) ) { + continue; + } + + data.reconvertedElements.add( element ); + + const position = ModelPosition._createBefore( element ); + + reducedChanges.push( { + type: 'remove', + name: element.name, + position, + length: 1 + }, { + type: 'reinsert', + name: element.name, + position, + length: 1 + } ); + } + } + + data.changes = reducedChanges; + }; +} + +// Creates a function that checks if an element and it's watched attributes can be consumed and consumes them. +// +// @param {Object} model A normalized `config.model` converter configuration. +// @param {String} model.name The name of element. +// @param {Array.} model.attributes The list of attribute names that should trigger reconversion. +// @param {Boolean} [model.children] Whether the child list change should trigger reconversion. +// @returns {module:engine/conversion/downcasthelpers~ConsumerFunction} +function createConsumer( model ) { + return ( node, consumable ) => { + const events = [ 'insert' ]; + + // Collect all set attributes that are triggering conversion. + for ( const attributeName of model.attributes ) { + if ( node.hasAttribute( attributeName ) ) { + events.push( `attribute:${ attributeName }` ); + } + } + + if ( !events.every( event => consumable.test( node, event ) ) ) { + return false; + } + + return events.every( event => consumable.consume( node, event ) ); + }; +} + +// Creates a function that create view slots. +// +// @param {module:engine/model/element~Element} element +// @param {Map.>} slotsMap +// @param {module:engine/conversion/downcastdispatcher~DowncastConversionApi} conversionApi +// @returns {Function} +function createSlotFactory( element, slotsMap, conversionApi ) { + return modeOrFilter => { + const slot = conversionApi.writer.createContainerElement( '$slot' ); + + let children = null; + + if ( modeOrFilter === 'children' ) { + children = Array.from( element.getChildren() ); + } else if ( typeof modeOrFilter == 'function' ) { + children = Array.from( element.getChildren() ).filter( element => modeOrFilter( element ) ); + } else { + /** + * Unknown slot mode was provided to `conversionApi.slotFor()` in downcast converter. + * + * @error conversion-slot-mode-unknown + */ + throw new CKEditorError( 'conversion-slot-mode-unknown', conversionApi.dispatcher, { modeOrFilter } ); + } + + slotsMap.set( slot, children ); + + return slot; + }; +} + +// Checks if all children are covered by slots and there is no child that landed in multiple slots. +// +// @param {module:engine/model/element~Element} +// @param {Map.>} slotsMap +// @param {module:engine/conversion/downcastdispatcher~DowncastConversionApi} conversionApi +function validateSlotsChildren( element, slotsMap, conversionApi ) { + const childrenInSlots = Array.from( slotsMap.values() ).flat(); + const uniqueChildrenInSlots = new Set( childrenInSlots ); + + if ( uniqueChildrenInSlots.size != childrenInSlots.length ) { + /** + * Filters provided to `conversionApi.slotFor()` overlap (at least two filters accept the same child element). + * + * @error conversion-slot-filter-overlap + * @param {module:engine/model/element~Element} element The element of which children would not be properly + * allocated to multiple slots. + */ + throw new CKEditorError( 'conversion-slot-filter-overlap', conversionApi.dispatcher, { element } ); + } + + if ( uniqueChildrenInSlots.size != element.childCount ) { + /** + * Filters provided to `conversionApi.slotFor()` are incomplete and exclude at least one children element (one of + * the children elements would not be assigned to any of the slots). + * + * @error conversion-slot-filter-incomplete + * @param {module:engine/model/element~Element} element The element of which children would not be properly + * allocated to multiple slots. + */ + throw new CKEditorError( 'conversion-slot-filter-incomplete', conversionApi.dispatcher, { element } ); + } +} + +// Fill slots with appropriate view elements. +// +// @param {module:engine/view/element~Element} viewElement +// @param {Map.>} slotsMap +// @param {module:engine/conversion/downcastdispatcher~DowncastConversionApi} conversionApi +// @param {Object} options +// @param {Boolean} [options.reconversion] +function fillSlots( viewElement, slotsMap, conversionApi, options ) { + // Set temporary position mapping to redirect child view elements into a proper slots. + conversionApi.mapper.on( 'modelToViewPosition', toViewPositionMapping, { priority: 'highest' } ); + + let currentSlot = null; + let currentSlotNodes = null; + + // Fill slots with nested view nodes. + for ( [ currentSlot, currentSlotNodes ] of slotsMap ) { + reinsertNodes( viewElement, currentSlotNodes, conversionApi, options ); + + conversionApi.writer.move( + conversionApi.writer.createRangeIn( currentSlot ), + conversionApi.writer.createPositionBefore( currentSlot ) + ); + conversionApi.writer.remove( currentSlot ); + } + + conversionApi.mapper.off( 'modelToViewPosition', toViewPositionMapping ); + + function toViewPositionMapping( evt, data ) { + const element = data.modelPosition.nodeAfter; + + // Find the proper offset within the slot. + const index = currentSlotNodes.indexOf( element ); + + if ( index < 0 ) { + return; + } + + data.viewPosition = data.mapper.findPositionIn( currentSlot, index ); + } +} + +// Inserts view representation of `nodes` into the `viewElement` either by bringing back just removed view nodes +// or by triggering conversion for them. +// +// @param {module:engine/view/element~Element} viewElement +// @param {Iterable.} modelNodes +// @param {module:engine/conversion/downcastdispatcher~DowncastConversionApi} conversionApi +// @param {Object} options +// @param {Boolean} [options.reconversion] +function reinsertNodes( viewElement, modelNodes, conversionApi, options ) { + const { writer, mapper } = conversionApi; + + // Fill with nested view nodes. + for ( const modelChildNode of modelNodes ) { + const viewChildNode = mapper.toViewElement( modelChildNode ); + + if ( options.reconversion && viewChildNode && viewChildNode.root != viewElement.root ) { + writer.move( + writer.createRangeOn( viewChildNode ), + mapper.toViewPosition( ModelPosition._createBefore( modelChildNode ) ) + ); + } else { + conversionApi.convertInsert( ModelRange._createOn( modelChildNode ) ); + } + } +} + /** * An object describing how the marker highlight should be represented in the view. * @@ -1704,3 +2171,82 @@ function prepareDescriptor( highlightDescriptor, data, conversionApi ) { * attribute element. If the descriptor is applied to an element, usually these attributes will be set on that element, however, * this depends on how the element converts the descriptor. */ + +/** + * A filtering function used to choose model child nodes to be downcasted into the specific view + * {@link module:engine/conversion/downcasthelpers~DowncastConversionWithSlotsApi#slotFor "slot"} while executing the + * {@link module:engine/conversion/downcasthelpers~DowncastHelpers#elementToStructure `elementToStructure()`} converter. + * + * @callback module:engine/conversion/downcasthelpers~SlotFilter + * + * @param {module:engine/model/node~Node} node A model node. + * @returns {Boolean} Whether provided model node should be downcasted into this slot. + * + * @see module:engine/conversion/downcasthelpers~DowncastConversionWithSlotsApi#slotFor + * @see module:engine/conversion/downcasthelpers~DowncastHelpers#elementToStructure + * @see module:engine/conversion/downcasthelpers~insertStructure + */ + +/** + * An extended {@link module:engine/conversion/downcastdispatcher~DowncastConversionApi `DowncastConversionApi`} that adds a + * {@link module:engine/conversion/downcasthelpers~DowncastConversionWithSlotsApi#slotFor `slotFor()`} helper to create placeholders for + * child elements converion. + * + * @interface module:engine/conversion/downcasthelpers~DowncastConversionWithSlotsApi + * @extends module:engine/conversion/downcastdispatcher~DowncastConversionApi + * + * @see module:engine/conversion/downcasthelpers~DowncastHelpers#elementToStructure + * @see module:engine/conversion/downcasthelpers~insertStructure + */ + +/** + * A helper that creates placeholders for child elements. + * + * const viewSlot = conversionApi.slotFor( 'children' ); + * const viewPosition = conversionApi.writer.createPositionAt( viewElement, 0 ); + * + * conversionApi.writer.insert( viewPosition, viewSlot ); + * + * It could be filtered to a specific subset of children (only `` model elements in this case): + * + * const viewSlot = conversionApi.slotFor( node => node.is( 'element', 'foo' ) ); + * const viewPosition = conversionApi.writer.createPositionAt( viewElement, 0 ); + * + * conversionApi.writer.insert( viewPosition, viewSlot ); + * + * While providing a filtered slot make sure to provide slots for all child nodes. A single node can not be downcasted into + * multiple slots. + * + * **Note**: You should not change the order of nodes. View elements should be in the same order as model nodes. + * + * @method #slotFor + * @param {'children'|module:engine/conversion/downcasthelpers~SlotFilter} modeOrFilter The filter for child nodes. + * @returns {module:engine/view/element~Element} The slot element to be placed in to the view structure while processing + * {@link module:engine/conversion/downcasthelpers~DowncastHelpers#elementToStructure `elementToStructure()`}. + */ + +/** + * A function that takes the model element and {@link module:engine/conversion/downcasthelpers~DowncastConversionWithSlotsApi downcast + * conversion API} as parameters and returns a view container element with slots for model child nodes to be converted into. + * + * @callback module:engine/conversion/downcasthelpers~StructureCreatorFunction + * @param {module:engine/model/element~Element} element The model element to be converted to the view structure. + * @param {module:engine/conversion/downcasthelpers~DowncastConversionWithSlotsApi} conversionApi The conversion interface with + * {@link module:engine/conversion/downcasthelpers~DowncastConversionWithSlotsApi#slotFor `slotFor()`} factory. + * @returns {module:engine/view/element~Element} The view structure with slots for model child nodes. + * + * @see module:engine/conversion/downcasthelpers~DowncastHelpers#elementToStructure + * @see module:engine/conversion/downcasthelpers~insertStructure + */ + +/** + * A function that is expected to consume all the consumables that were used by the element creator. + * + * @callback module:engine/conversion/downcasthelpers~ConsumerFunction + * @param {module:engine/model/element~Element} element The model element to be converted to the view structure. + * @param {module:engine/conversion/modelconsumable~ModelConsumable} consumable The `ModelConsumable` same as in + * {@link module:engine/conversion/downcastdispatcher~DowncastConversionApi#consumable `DowncastConversionApi.consumable`}. + * @returns {Boolean} `true` if all consumable values were available and were consumed, `false` otherwise. + * + * @see module:engine/conversion/downcasthelpers~insertStructure + */ diff --git a/packages/ckeditor5-engine/src/conversion/mapper.js b/packages/ckeditor5-engine/src/conversion/mapper.js index 1c9fed36963..04c304b1ab5 100644 --- a/packages/ckeditor5-engine/src/conversion/mapper.js +++ b/packages/ckeditor5-engine/src/conversion/mapper.js @@ -88,6 +88,14 @@ export default class Mapper { */ this._elementToMarkerNames = new Map(); + /** + * The map of removed view elements with their current root (used for deferred unbinding). + * + * @private + * @member {Map.} + */ + this._deferredBindingRemovals = 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). @@ -137,29 +145,36 @@ export default class Mapper { } /** - * Unbinds given {@link module:engine/view/element~Element view element} from the map. + * Unbinds the given {@link module:engine/view/element~Element view element} from the map. * * **Note:** view-to-model binding will be removed, if it existed. However, corresponding model-to-view binding - * will be removed only if model element is still bound to passed `viewElement`. + * will be removed only if model element is still bound to the passed `viewElement`. * * This behavior lets for re-binding model element to another view element without fear of losing the new binding * when the previously bound view element is unbound. * * @param {module:engine/view/element~Element} viewElement View element to unbind. + * @param {Object} [options={}] The options object. + * @param {Boolean} [options.defer=false] Controls whether the binding should be removed immediately or deferred until a + * {@link #flushDeferredBindings `flushDeferredBindings()`} call. */ - unbindViewElement( viewElement ) { + unbindViewElement( viewElement, options = {} ) { const modelElement = this.toModelElement( viewElement ); - 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 ); + if ( options.defer ) { + this._deferredBindingRemovals.set( viewElement, viewElement.root ); + } else { + this._viewToModelMapping.delete( viewElement ); + + if ( this._modelToViewMapping.get( modelElement ) == viewElement ) { + this._modelToViewMapping.delete( modelElement ); + } } } @@ -244,6 +259,22 @@ export default class Mapper { return markerNames; } + /** + * Unbinds all deferred binding removals of view elements that were not re-attached in the meantime to some root or document fragment. + * + * See: {@link #unbindViewElement `unbindViewElement()`}. + */ + flushDeferredBindings() { + for ( const [ viewElement, root ] of this._deferredBindingRemovals ) { + // Unbind it only if it wasn't re-attached to some root or document fragment. + if ( viewElement.root == root ) { + this.unbindViewElement( viewElement ); + } + } + + this._deferredBindingRemovals = new Map(); + } + /** * Removes all model to view and view to model bindings. */ @@ -253,6 +284,7 @@ export default class Mapper { this._markerNameToElements = new Map(); this._elementToMarkerNames = new Map(); this._unboundMarkerNames = new Set(); + this._deferredBindingRemovals = new Map(); } /** diff --git a/packages/ckeditor5-engine/src/conversion/modelconsumable.js b/packages/ckeditor5-engine/src/conversion/modelconsumable.js index 6a35c0a4f64..20b0175b694 100644 --- a/packages/ckeditor5-engine/src/conversion/modelconsumable.js +++ b/packages/ckeditor5-engine/src/conversion/modelconsumable.js @@ -270,24 +270,26 @@ export default class ModelConsumable { } if ( !symbol ) { - symbol = this._addSymbolForTextProxy( textProxy.startOffset, textProxy.endOffset, textProxy.parent ); + symbol = this._addSymbolForTextProxy( textProxy ); } return symbol; } /** - * Adds a symbol for given properties that characterizes a {@link module:engine/model/textproxy~TextProxy} instance. + * Adds a symbol for given {@link module:engine/model/textproxy~TextProxy} instance. * * Used internally to correctly consume `TextProxy` instances. * * @private - * @param {Number} startIndex Text proxy start index in it's parent. - * @param {Number} endIndex Text proxy end index in it's parent. - * @param {module:engine/model/element~Element} parent Text proxy parent. - * @returns {Symbol} Symbol generated for given properties. + * @param {module:engine/model/textproxy~TextProxy} textProxy Text proxy instance. + * @returns {Symbol} Symbol generated for given `TextProxy`. */ - _addSymbolForTextProxy( start, end, parent ) { + _addSymbolForTextProxy( textProxy ) { + const start = textProxy.startOffset; + const end = textProxy.endOffset; + const parent = textProxy.parent; + const symbol = Symbol( 'textProxySymbol' ); let startMap, endMap; @@ -320,6 +322,11 @@ export default class ModelConsumable { function _normalizeConsumableType( type ) { const parts = type.split( ':' ); + // For inserts allow passing event name, it's stored in the context of a specified element so the element name is not needed. + if ( parts[ 0 ] == 'insert' ) { + return parts[ 0 ]; + } + // Markers are identified by the whole name (otherwise we would consume the whole markers group). if ( parts[ 0 ] == 'addMarker' || parts[ 0 ] == 'removeMarker' ) { return type; diff --git a/packages/ckeditor5-engine/src/dev-utils/model.js b/packages/ckeditor5-engine/src/dev-utils/model.js index bd4aa51879d..91f076c190c 100644 --- a/packages/ckeditor5-engine/src/dev-utils/model.js +++ b/packages/ckeditor5-engine/src/dev-utils/model.js @@ -260,28 +260,28 @@ export function stringify( node, selectionOrPositionOrRange = null, markers = nu return writer.createUIElement( name ); } ) ); + const markersMap = new Map(); + + if ( markers ) { + // To provide stable results, sort markers by name. + for ( const marker of Array.from( markers ).sort( ( a, b ) => a.name < b.name ? 1 : -1 ) ) { + markersMap.set( marker.name, marker.getRange() ); + } + } + // Convert model to view. const writer = view._writer; - downcastDispatcher.convertInsert( range, writer ); + downcastDispatcher.convert( range, markersMap, writer ); // Convert model selection to view selection. if ( selection ) { downcastDispatcher.convertSelection( selection, markers || model.markers, writer ); } - if ( markers ) { - // To provide stable results, sort markers by name. - markers = Array.from( markers ).sort( ( a, b ) => a.name < b.name ? 1 : -1 ); - - for ( const marker of markers ) { - downcastDispatcher.convertMarkerAdd( marker.name, marker.getRange(), writer ); - } - } - // Parse view to data string. let data = viewStringify( viewRoot, viewDocument.selection, { sameSelectionCharacters: true } ); - // Removing unneccessary
and
added because `viewRoot` was also stringified alongside input data. + // Removing unnecessary
and
added because `viewRoot` was also stringified alongside input data. data = data.substr( 5, data.length - 11 ); view.destroy(); diff --git a/packages/ckeditor5-engine/tests/conversion/downcastdispatcher.js b/packages/ckeditor5-engine/tests/conversion/downcastdispatcher.js index 422eed0a7db..f8b799e3b91 100644 --- a/packages/ckeditor5-engine/tests/conversion/downcastdispatcher.js +++ b/packages/ckeditor5-engine/tests/conversion/downcastdispatcher.js @@ -11,20 +11,23 @@ import ModelText from '../../src/model/text'; import ModelElement from '../../src/model/element'; import ModelDocumentFragment from '../../src/model/documentfragment'; import ModelRange from '../../src/model/range'; +import ModelConsumable from '../../src/conversion/modelconsumable'; import View from '../../src/view/view'; import ViewContainerElement from '../../src/view/containerelement'; +import DowncastWriter from '../../src/view/downcastwriter'; import { StylesProcessor } from '../../src/view/stylesmap'; describe( 'DowncastDispatcher', () => { - let dispatcher, doc, root, differStub, model, view, mapper; + let dispatcher, doc, root, differStub, model, view, mapper, apiObj; beforeEach( () => { model = new Model(); view = new View( new StylesProcessor() ); doc = model.document; mapper = new Mapper(); - dispatcher = new DowncastDispatcher( { mapper } ); + apiObj = {}; + dispatcher = new DowncastDispatcher( { mapper, apiObj } ); root = doc.createRoot(); differStub = { @@ -35,17 +38,18 @@ describe( 'DowncastDispatcher', () => { } ); describe( 'constructor()', () => { - it( 'should create DowncastDispatcher with given api', () => { + it( 'should create DowncastDispatcher with given api template', () => { const apiObj = {}; const dispatcher = new DowncastDispatcher( { apiObj } ); - expect( dispatcher.conversionApi.apiObj ).to.equal( apiObj ); + expect( dispatcher._conversionApi.apiObj ).to.equal( apiObj ); } ); } ); describe( 'convertChanges', () => { - it( 'should call convertInsert for insert change', () => { - sinon.stub( dispatcher, 'convertInsert' ); + it( 'should call _convertInsert for insert change', () => { + sinon.stub( dispatcher, '_convertInsert' ); + sinon.stub( mapper, 'flushDeferredBindings' ); const position = model.createPositionFromPath( root, [ 0 ] ); const range = ModelRange._createFromPositionAndShift( position, 1 ); @@ -56,15 +60,42 @@ describe( 'DowncastDispatcher', () => { dispatcher.convertChanges( differStub, model.markers, writer ); } ); - expect( dispatcher.convertInsert.calledOnce ).to.be.true; - expect( dispatcher.convertInsert.firstCall.args[ 0 ].isEqual( range ) ).to.be.true; + expect( dispatcher._convertInsert.calledOnce ).to.be.true; + expect( dispatcher._convertInsert.firstCall.args[ 0 ].isEqual( range ) ).to.be.true; - expect( dispatcher.conversionApi.writer ).to.be.undefined; - expect( dispatcher.conversionApi.consumable ).to.be.undefined; + assertConversionApi( dispatcher._convertInsert.firstCall.args[ 1 ] ); + + expect( mapper.flushDeferredBindings.calledOnce ).to.be.true; + expect( dispatcher._conversionApi.writer ).to.be.undefined; + expect( dispatcher._conversionApi.consumable ).to.be.undefined; + } ); + + it( 'should call _convertReinsert for reinsert change', () => { + sinon.stub( dispatcher, '_convertReinsert' ); + sinon.stub( mapper, 'flushDeferredBindings' ); + + const position = model.createPositionFromPath( root, [ 0 ] ); + const range = ModelRange._createFromPositionAndShift( position, 1 ); + + differStub.getChanges = () => [ { type: 'reinsert', position, length: 1 } ]; + + view.change( writer => { + dispatcher.convertChanges( differStub, model.markers, writer ); + } ); + + expect( dispatcher._convertReinsert.calledOnce ).to.be.true; + expect( dispatcher._convertReinsert.firstCall.args[ 0 ].isEqual( range ) ).to.be.true; + + assertConversionApi( dispatcher._convertReinsert.firstCall.args[ 1 ] ); + + expect( mapper.flushDeferredBindings.calledOnce ).to.be.true; + expect( dispatcher._conversionApi.writer ).to.be.undefined; + expect( dispatcher._conversionApi.consumable ).to.be.undefined; } ); - it( 'should call convertRemove for remove change', () => { - sinon.stub( dispatcher, 'convertRemove' ); + it( 'should call _convertRemove for remove change', () => { + sinon.stub( dispatcher, '_convertRemove' ); + sinon.stub( mapper, 'flushDeferredBindings' ); const position = model.createPositionFromPath( root, [ 0 ] ); @@ -74,15 +105,18 @@ describe( 'DowncastDispatcher', () => { dispatcher.convertChanges( differStub, model.markers, writer ); } ); - expect( dispatcher.convertRemove.calledWith( position, 2, '$text' ) ).to.be.true; + expect( dispatcher._convertRemove.calledWith( position, 2, '$text' ) ).to.be.true; - expect( dispatcher.conversionApi.writer ).to.be.undefined; - expect( dispatcher.conversionApi.consumable ).to.be.undefined; + assertConversionApi( dispatcher._convertRemove.firstCall.args[ 3 ] ); + + expect( mapper.flushDeferredBindings.calledOnce ).to.be.true; + expect( dispatcher._conversionApi.writer ).to.be.undefined; + expect( dispatcher._conversionApi.consumable ).to.be.undefined; } ); - it( 'should call convertAttribute for attribute change', () => { - sinon.stub( dispatcher, 'convertAttribute' ); - sinon.stub( dispatcher, '_mapChangesWithAutomaticReconversion' ).callsFake( differ => differ.getChanges() ); + it( 'should call _convertAttribute for attribute change', () => { + sinon.stub( dispatcher, '_convertAttribute' ); + sinon.stub( mapper, 'flushDeferredBindings' ); const position = model.createPositionFromPath( root, [ 0 ] ); const range = ModelRange._createFromPositionAndShift( position, 1 ); @@ -95,23 +129,28 @@ describe( 'DowncastDispatcher', () => { dispatcher.convertChanges( differStub, model.markers, writer ); } ); - expect( dispatcher.convertAttribute.calledWith( range, 'key', null, 'foo' ) ).to.be.true; + expect( dispatcher._convertAttribute.calledWith( range, 'key', null, 'foo' ) ).to.be.true; + + assertConversionApi( dispatcher._convertAttribute.firstCall.args[ 4 ] ); - expect( dispatcher.conversionApi.writer ).to.be.undefined; - expect( dispatcher.conversionApi.consumable ).to.be.undefined; + expect( mapper.flushDeferredBindings.calledOnce ).to.be.true; + expect( dispatcher._conversionApi.writer ).to.be.undefined; + expect( dispatcher._conversionApi.consumable ).to.be.undefined; } ); it( 'should handle multiple changes', () => { - sinon.stub( dispatcher, 'convertInsert' ); - sinon.stub( dispatcher, 'convertRemove' ); - sinon.stub( dispatcher, 'convertAttribute' ); - sinon.stub( dispatcher, '_mapChangesWithAutomaticReconversion' ).callsFake( differ => differ.getChanges() ); + sinon.stub( dispatcher, '_convertInsert' ); + sinon.stub( dispatcher, '_convertReinsert' ); + sinon.stub( dispatcher, '_convertRemove' ); + sinon.stub( dispatcher, '_convertAttribute' ); + sinon.stub( mapper, 'flushDeferredBindings' ); const position = model.createPositionFromPath( root, [ 0 ] ); const range = ModelRange._createFromPositionAndShift( position, 1 ); differStub.getChanges = () => [ { type: 'insert', position, length: 1 }, + { type: 'reinsert', position, length: 1 }, { type: 'attribute', position, range, attributeKey: 'key', attributeOldValue: null, attributeNewValue: 'foo' }, { type: 'remove', position, length: 1, name: 'paragraph' }, { type: 'insert', position, length: 3 } @@ -121,16 +160,56 @@ describe( 'DowncastDispatcher', () => { dispatcher.convertChanges( differStub, model.markers, writer ); } ); - expect( dispatcher.convertInsert.calledTwice ).to.be.true; - expect( dispatcher.convertRemove.calledOnce ).to.be.true; - expect( dispatcher.convertAttribute.calledOnce ).to.be.true; + expect( dispatcher._convertInsert.calledTwice ).to.be.true; + expect( dispatcher._convertReinsert.calledOnce ).to.be.true; + expect( dispatcher._convertRemove.calledOnce ).to.be.true; + expect( dispatcher._convertAttribute.calledOnce ).to.be.true; + + expect( mapper.flushDeferredBindings.calledOnce ).to.be.true; + expect( dispatcher._conversionApi.writer ).to.be.undefined; + expect( dispatcher._conversionApi.consumable ).to.be.undefined; + } ); + + it( 'should fire "reduceChanges" event and use replaced changes', () => { + sinon.stub( dispatcher, '_convertInsert' ); + sinon.stub( dispatcher, '_convertReinsert' ); + sinon.stub( dispatcher, '_convertRemove' ); + sinon.stub( dispatcher, '_convertAttribute' ); + sinon.stub( mapper, 'flushDeferredBindings' ); + + const position = model.createPositionFromPath( root, [ 0 ] ); + const range = ModelRange._createFromPositionAndShift( position, 1 ); + + differStub.getChanges = () => [ + { type: 'insert', position, length: 1 }, + { type: 'attribute', position, range, attributeKey: 'key', attributeOldValue: null, attributeNewValue: 'foo' } + ]; + + dispatcher.on( 'reduceChanges', ( evt, data ) => { + data.changes = [ + { type: 'insert', position, length: 1 }, + { type: 'remove', position, length: 1 }, + { type: 'reinsert', position, length: 1 } + ]; + } ); + + view.change( writer => { + dispatcher.convertChanges( differStub, model.markers, writer ); + } ); + + expect( dispatcher._convertInsert.calledOnce ).to.be.true; + expect( dispatcher._convertReinsert.calledOnce ).to.be.true; + expect( dispatcher._convertRemove.calledOnce ).to.be.true; + expect( dispatcher._convertAttribute.notCalled ).to.be.true; - expect( dispatcher.conversionApi.writer ).to.be.undefined; - expect( dispatcher.conversionApi.consumable ).to.be.undefined; + expect( mapper.flushDeferredBindings.calledOnce ).to.be.true; + expect( dispatcher._conversionApi.writer ).to.be.undefined; + expect( dispatcher._conversionApi.consumable ).to.be.undefined; } ); - it( 'should call convertMarkerAdd when markers are added', () => { - sinon.stub( dispatcher, 'convertMarkerAdd' ); + it( 'should call _convertMarkerAdd when markers are added', () => { + sinon.stub( dispatcher, '_convertMarkerAdd' ); + sinon.stub( mapper, 'flushDeferredBindings' ); const fooRange = model.createRange( model.createPositionAt( root, 0 ), model.createPositionAt( root, 1 ) ); const barRange = model.createRange( model.createPositionAt( root, 3 ), model.createPositionAt( root, 6 ) ); @@ -144,15 +223,20 @@ describe( 'DowncastDispatcher', () => { dispatcher.convertChanges( differStub, model.markers, writer ); } ); - expect( dispatcher.convertMarkerAdd.calledWith( 'foo', fooRange ) ); - expect( dispatcher.convertMarkerAdd.calledWith( 'bar', barRange ) ); + expect( dispatcher._convertMarkerAdd.calledWith( 'foo', fooRange ) ); + expect( dispatcher._convertMarkerAdd.calledWith( 'bar', barRange ) ); + + assertConversionApi( dispatcher._convertMarkerAdd.firstCall.args[ 2 ] ); + assertConversionApi( dispatcher._convertMarkerAdd.secondCall.args[ 2 ] ); - expect( dispatcher.conversionApi.writer ).to.be.undefined; - expect( dispatcher.conversionApi.consumable ).to.be.undefined; + expect( mapper.flushDeferredBindings.calledOnce ).to.be.true; + expect( dispatcher._conversionApi.writer ).to.be.undefined; + expect( dispatcher._conversionApi.consumable ).to.be.undefined; } ); - it( 'should call convertMarkerRemove when markers are removed', () => { - sinon.stub( dispatcher, 'convertMarkerRemove' ); + it( 'should call _convertMarkerRemove when markers are removed', () => { + sinon.stub( dispatcher, '_convertMarkerRemove' ); + sinon.stub( mapper, 'flushDeferredBindings' ); const fooRange = model.createRange( model.createPositionAt( root, 0 ), model.createPositionAt( root, 1 ) ); const barRange = model.createRange( model.createPositionAt( root, 3 ), model.createPositionAt( root, 6 ) ); @@ -166,16 +250,21 @@ describe( 'DowncastDispatcher', () => { dispatcher.convertChanges( differStub, model.markers, writer ); } ); - expect( dispatcher.convertMarkerRemove.calledWith( 'foo', fooRange ) ); - expect( dispatcher.convertMarkerRemove.calledWith( 'bar', barRange ) ); + expect( dispatcher._convertMarkerRemove.calledWith( 'foo', fooRange ) ); + expect( dispatcher._convertMarkerRemove.calledWith( 'bar', barRange ) ); - expect( dispatcher.conversionApi.writer ).to.be.undefined; - expect( dispatcher.conversionApi.consumable ).to.be.undefined; + assertConversionApi( dispatcher._convertMarkerRemove.firstCall.args[ 2 ] ); + assertConversionApi( dispatcher._convertMarkerRemove.secondCall.args[ 2 ] ); + + expect( mapper.flushDeferredBindings.calledOnce ).to.be.true; + expect( dispatcher._conversionApi.writer ).to.be.undefined; + expect( dispatcher._conversionApi.consumable ).to.be.undefined; } ); it( 'should re-render markers which view elements got unbound during conversion', () => { - sinon.stub( dispatcher, 'convertMarkerRemove' ); - sinon.stub( dispatcher, 'convertMarkerAdd' ); + sinon.stub( dispatcher, '_convertMarkerRemove' ); + sinon.stub( dispatcher, '_convertMarkerAdd' ); + sinon.stub( mapper, 'flushDeferredBindings' ); const fooRange = model.createRange( model.createPositionAt( root, 0 ), model.createPositionAt( root, 1 ) ); const barRange = model.createRange( model.createPositionAt( root, 3 ), model.createPositionAt( root, 6 ) ); @@ -184,23 +273,29 @@ describe( 'DowncastDispatcher', () => { model.markers._set( 'bar', barRange ); // Stub `Mapper#flushUnboundMarkerNames`. - dispatcher.conversionApi.mapper.flushUnboundMarkerNames = () => [ 'foo', 'bar' ]; + dispatcher._conversionApi.mapper.flushUnboundMarkerNames = () => [ '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 ) ); + expect( dispatcher._convertMarkerRemove.calledWith( 'foo', fooRange ) ); + expect( dispatcher._convertMarkerRemove.calledWith( 'bar', barRange ) ); + expect( dispatcher._convertMarkerAdd.calledWith( 'foo', fooRange ) ); + expect( dispatcher._convertMarkerAdd.calledWith( 'bar', barRange ) ); + + assertConversionApi( dispatcher._convertMarkerRemove.firstCall.args[ 2 ] ); + assertConversionApi( dispatcher._convertMarkerRemove.secondCall.args[ 2 ] ); + assertConversionApi( dispatcher._convertMarkerAdd.firstCall.args[ 2 ] ); + assertConversionApi( dispatcher._convertMarkerAdd.secondCall.args[ 2 ] ); - expect( dispatcher.conversionApi.writer ).to.be.undefined; - expect( dispatcher.conversionApi.consumable ).to.be.undefined; + expect( mapper.flushDeferredBindings.calledOnce ).to.be.true; + expect( dispatcher._conversionApi.writer ).to.be.undefined; + expect( dispatcher._conversionApi.consumable ).to.be.undefined; } ); } ); - describe( 'convertInsert', () => { + describe( 'convert', () => { it( 'should fire event with correct parameters for every item in passed range', () => { root._appendChild( [ new ModelText( 'foo', { bold: true } ), @@ -240,7 +335,7 @@ describe( 'DowncastDispatcher', () => { expect( conversionApi.consumable.consume( data.item, 'attribute:' + key ) ).to.be.true; } ); - dispatcher.convertInsert( range ); + dispatcher.convert( range, [] ); // Check the data passed to called events and the order of them. expect( loggedEvents ).to.deep.equal( [ @@ -254,8 +349,8 @@ describe( 'DowncastDispatcher', () => { 'attribute:italic:true:$text:xx:7,0:7,2' ] ); - expect( dispatcher.conversionApi.writer ).to.be.undefined; - expect( dispatcher.conversionApi.consumable ).to.be.undefined; + expect( dispatcher._conversionApi.writer ).to.be.undefined; + expect( dispatcher._conversionApi.consumable ).to.be.undefined; } ); it( 'should not fire events for already consumed parts of model', () => { @@ -274,7 +369,7 @@ describe( 'DowncastDispatcher', () => { const range = model.createRangeIn( root ); - dispatcher.convertInsert( range ); + dispatcher.convert( range, [] ); expect( dispatcher.fire.calledWith( 'insert:imageBlock' ) ).to.be.true; expect( dispatcher.fire.calledWith( 'attribute:src:imageBlock' ) ).to.be.true; @@ -284,12 +379,285 @@ describe( 'DowncastDispatcher', () => { expect( dispatcher.fire.calledWith( 'attribute:bold:imageBlock' ) ).to.be.false; expect( dispatcher.fire.calledWith( 'insert:caption' ) ).to.be.false; - expect( dispatcher.conversionApi.writer ).to.be.undefined; - expect( dispatcher.conversionApi.consumable ).to.be.undefined; + expect( dispatcher._conversionApi.writer ).to.be.undefined; + expect( dispatcher._conversionApi.consumable ).to.be.undefined; + } ); + + it( 'should call _convertMarkerAdd for all provided markers', () => { + 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 ) ); + + const markers = new Map( [ + [ 'foo', fooRange ], + [ 'bar', barRange ] + ] ); + + const range = model.createRangeIn( root ); + + view.change( writer => { + dispatcher.convert( range, markers, writer ); + } ); + + expect( dispatcher._convertMarkerAdd.calledWith( 'foo', fooRange ) ); + expect( dispatcher._convertMarkerAdd.calledWith( 'bar', barRange ) ); + + assertConversionApi( dispatcher._convertMarkerAdd.firstCall.args[ 2 ] ); + assertConversionApi( dispatcher._convertMarkerAdd.secondCall.args[ 2 ] ); + + expect( dispatcher._conversionApi.writer ).to.be.undefined; + expect( dispatcher._conversionApi.consumable ).to.be.undefined; + } ); + + it( 'should pass options object to conversionApi', () => { + sinon.stub( dispatcher, '_convertInsert' ); + sinon.stub( dispatcher, '_convertMarkerAdd' ); + + const position = model.createPositionFromPath( root, [ 0 ] ); + const range = ModelRange._createFromPositionAndShift( position, 1 ); + + const fooRange = model.createRange( model.createPositionAt( root, 0 ), model.createPositionAt( root, 1 ) ); + const barRange = model.createRange( model.createPositionAt( root, 3 ), model.createPositionAt( root, 6 ) ); + + const markers = new Map( [ + [ 'foo', fooRange ], + [ 'bar', barRange ] + ] ); + + const options = {}; + + view.change( writer => { + dispatcher.convert( range, markers, writer, options ); + } ); + + expect( dispatcher._convertInsert.calledOnce ).to.be.true; + expect( dispatcher._convertInsert.firstCall.args[ 0 ].isEqual( range ) ).to.be.true; + + expect( dispatcher._convertMarkerAdd.calledWith( 'foo', fooRange ) ); + expect( dispatcher._convertMarkerAdd.calledWith( 'bar', barRange ) ); + + assertConversionApi( dispatcher._convertInsert.firstCall.args[ 1 ] ); + assertConversionApi( dispatcher._convertMarkerAdd.firstCall.args[ 2 ] ); + assertConversionApi( dispatcher._convertMarkerAdd.secondCall.args[ 2 ] ); + + expect( dispatcher._convertInsert.firstCall.args[ 1 ].options ).to.equal( options ); + expect( dispatcher._convertMarkerAdd.firstCall.args[ 2 ].options ).to.equal( options ); + expect( dispatcher._convertMarkerAdd.firstCall.args[ 2 ].options ).to.equal( options ); + + expect( dispatcher._conversionApi.writer ).to.be.undefined; + expect( dispatcher._conversionApi.consumable ).to.be.undefined; } ); } ); - describe( 'convertRemove', () => { + describe( '_convertInsert', () => { + it( 'should fire event with correct parameters for every item in passed range', () => { + root._appendChild( [ + new ModelText( 'foo', { bold: true } ), + new ModelElement( 'imageBlock', null, new ModelElement( 'caption' ) ), + new ModelText( 'bar' ), + new ModelElement( 'paragraph', { class: 'nice' }, new ModelText( 'xx', { italic: true } ) ) + ] ); + + const range = model.createRangeIn( root ); + const loggedEvents = []; + + // We will check everything connected with insert event: + dispatcher.on( 'insert', ( evt, data, conversionApi ) => { + // Check if the item is correct. + const itemId = data.item.name ? data.item.name : '$text:' + data.item.data; + // Check if the range is correct. + const log = 'insert:' + itemId + ':' + data.range.start.path + ':' + data.range.end.path; + + loggedEvents.push( log ); + + // Check if the event name is correct. + expect( evt.name ).to.equal( 'insert:' + ( data.item.name || '$text' ) ); + // Check if model consumable is correct. + expect( conversionApi.consumable.consume( data.item, 'insert' ) ).to.be.true; + expect( data ).to.not.have.property( 'reconversion' ); + } ); + + // Same here. + dispatcher.on( 'attribute', ( evt, data, conversionApi ) => { + const itemId = data.item.name ? data.item.name : '$text:' + data.item.data; + const key = data.attributeKey; + const value = data.attributeNewValue; + const log = 'attribute:' + key + ':' + value + ':' + itemId + ':' + data.range.start.path + ':' + data.range.end.path; + + loggedEvents.push( log ); + + expect( evt.name ).to.equal( 'attribute:' + key + ':' + ( data.item.name || '$text' ) ); + expect( conversionApi.consumable.consume( data.item, 'attribute:' + key ) ).to.be.true; + } ); + + view.change( writer => { + dispatcher._convertInsert( range, dispatcher._createConversionApi( writer ) ); + } ); + + // Check the data passed to called events and the order of them. + expect( loggedEvents ).to.deep.equal( [ + 'insert:$text:foo:0:3', + 'attribute:bold:true:$text:foo:0:3', + 'insert:imageBlock:3:4', + 'insert:caption:3,0:3,1', + 'insert:$text:bar:4:7', + 'insert:paragraph:7:8', + 'attribute:class:nice:paragraph:7:8', + 'insert:$text:xx:7,0:7,2', + 'attribute:italic:true:$text:xx:7,0:7,2' + ] ); + + expect( dispatcher._conversionApi.writer ).to.be.undefined; + expect( dispatcher._conversionApi.consumable ).to.be.undefined; + } ); + + it( 'should not fire events for already consumed parts of model', () => { + root._appendChild( [ + new ModelElement( 'imageBlock', { src: 'foo.jpg', title: 'bar', bold: true }, [ + new ModelElement( 'caption', {}, new ModelText( 'title' ) ) + ] ) + ] ); + + sinon.spy( dispatcher, 'fire' ); + + dispatcher.on( 'insert:imageBlock', ( evt, data, conversionApi ) => { + conversionApi.consumable.consume( data.item.getChild( 0 ), 'insert' ); + conversionApi.consumable.consume( data.item, 'attribute:bold' ); + } ); + + const range = model.createRangeIn( root ); + + view.change( writer => { + dispatcher._convertInsert( range, dispatcher._createConversionApi( writer ) ); + } ); + + expect( dispatcher.fire.calledWith( 'insert:imageBlock' ) ).to.be.true; + expect( dispatcher.fire.calledWith( 'attribute:src:imageBlock' ) ).to.be.true; + expect( dispatcher.fire.calledWith( 'attribute:title:imageBlock' ) ).to.be.true; + expect( dispatcher.fire.calledWith( 'insert:$text' ) ).to.be.true; + + expect( dispatcher.fire.calledWith( 'attribute:bold:imageBlock' ) ).to.be.false; + expect( dispatcher.fire.calledWith( 'insert:caption' ) ).to.be.false; + + expect( dispatcher._conversionApi.writer ).to.be.undefined; + expect( dispatcher._conversionApi.consumable ).to.be.undefined; + } ); + + it( 'should not add consumable item if it was added already in consumable', () => { + root._appendChild( [ + new ModelElement( 'imageBlock', {}, [ + new ModelElement( 'caption', {}, new ModelText( 'title' ) ) + ] ) + ] ); + + const loggedEvents = []; + + // We will check everything connected with insert event: + dispatcher.on( 'insert', ( evt, data, conversionApi ) => { + // Check if the item is correct. + const itemId = data.item.name ? data.item.name : '$text:' + data.item.data; + // Check if the range is correct. + const log = 'insert:' + itemId + ':' + data.range.start.path + ':' + data.range.end.path; + + loggedEvents.push( log ); + + // Check if the event name is correct. + expect( evt.name ).to.equal( 'insert:' + ( data.item.name || '$text' ) ); + // Check if model consumable is correct. + expect( conversionApi.consumable.test( data.item, 'insert' ) ).to.be.true; + expect( data ).to.not.have.property( 'reconversion' ); + } ); + + dispatcher.on( 'insert:imageBlock', ( evt, data, conversionApi ) => { + if ( conversionApi.consumable.consume( data.item, 'insert' ) ) { + conversionApi.convertInsert( data.range ); + } + } ); + + dispatcher.on( 'insert:caption', ( evt, data, conversionApi ) => { + conversionApi.consumable.consume( data.item, 'insert' ); + } ); + + dispatcher.on( 'insert:$text', ( evt, data, conversionApi ) => { + conversionApi.consumable.consume( data.item, 'insert' ); + } ); + + const range = model.createRangeIn( root ); + + view.change( writer => { + dispatcher._convertInsert( range, dispatcher._createConversionApi( writer ) ); + } ); + + expect( loggedEvents ).to.deep.equal( [ + 'insert:imageBlock:0:1', + 'insert:caption:0,0:0,1', + 'insert:$text:title:0,0,0:0,0,5' + ] ); + } ); + } ); + + describe( '_convertReinsert', () => { + it( 'should fire event with correct parameters for every item in passed range (shallow)', () => { + root._appendChild( [ + new ModelText( 'foo', { bold: true } ), + new ModelElement( 'imageBlock', null, new ModelElement( 'caption' ) ), + new ModelText( 'bar' ), + new ModelElement( 'paragraph', { class: 'nice' }, new ModelText( 'xx', { italic: true } ) ) + ] ); + + const range = model.createRangeIn( root ); + const loggedEvents = []; + + // We will check everything connected with insert event: + dispatcher.on( 'insert', ( evt, data, conversionApi ) => { + // Check if the item is correct. + const itemId = data.item.name ? data.item.name : '$text:' + data.item.data; + // Check if the range is correct. + const log = 'insert:' + itemId + ':' + data.range.start.path + ':' + data.range.end.path; + + loggedEvents.push( log ); + + // Check if the event name is correct. + expect( evt.name ).to.equal( 'insert:' + ( data.item.name || '$text' ) ); + // Check if model consumable is correct. + expect( conversionApi.consumable.consume( data.item, 'insert' ) ).to.be.true; + expect( data ).to.have.property( 'reconversion' ).to.be.true; + } ); + + // Same here. + dispatcher.on( 'attribute', ( evt, data, conversionApi ) => { + const itemId = data.item.name ? data.item.name : '$text:' + data.item.data; + const key = data.attributeKey; + const value = data.attributeNewValue; + const log = 'attribute:' + key + ':' + value + ':' + itemId + ':' + data.range.start.path + ':' + data.range.end.path; + + loggedEvents.push( log ); + + expect( evt.name ).to.equal( 'attribute:' + key + ':' + ( data.item.name || '$text' ) ); + expect( conversionApi.consumable.consume( data.item, 'attribute:' + key ) ).to.be.true; + } ); + + view.change( writer => { + dispatcher._convertReinsert( range, dispatcher._createConversionApi( writer ) ); + } ); + + // Check the data passed to called events and the order of them. + expect( loggedEvents ).to.deep.equal( [ + 'insert:$text:foo:0:3', + 'attribute:bold:true:$text:foo:0:3', + 'insert:imageBlock:3:4', + 'insert:$text:bar:4:7', + 'insert:paragraph:7:8', + 'attribute:class:nice:paragraph:7:8' + ] ); + + expect( dispatcher._conversionApi.writer ).to.be.undefined; + expect( dispatcher._conversionApi.consumable ).to.be.undefined; + } ); + } ); + + describe( '_convertRemove', () => { it( 'should fire event for removed range', () => { const loggedEvents = []; @@ -298,12 +666,12 @@ describe( 'DowncastDispatcher', () => { loggedEvents.push( log ); } ); - dispatcher.convertRemove( model.createPositionAt( root, 3 ), 3, '$text' ); + dispatcher._convertRemove( model.createPositionAt( root, 3 ), 3, '$text' ); expect( loggedEvents ).to.deep.equal( [ 'remove:3:3' ] ); - expect( dispatcher.conversionApi.writer ).to.be.undefined; - expect( dispatcher.conversionApi.consumable ).to.be.undefined; + expect( dispatcher._conversionApi.writer ).to.be.undefined; + expect( dispatcher._conversionApi.consumable ).to.be.undefined; } ); } ); @@ -330,8 +698,8 @@ describe( 'DowncastDispatcher', () => { { selection: sinon.match.instanceOf( doc.selection.constructor ) } ) ).to.be.true; - expect( dispatcher.conversionApi.writer ).to.be.undefined; - expect( dispatcher.conversionApi.consumable ).to.be.undefined; + expect( dispatcher._conversionApi.writer ).to.be.undefined; + expect( dispatcher._conversionApi.consumable ).to.be.undefined; } ); it( 'should prepare correct list of consumable values', () => { @@ -350,8 +718,8 @@ describe( 'DowncastDispatcher', () => { dispatcher.convertSelection( doc.selection, model.markers, [] ); - expect( dispatcher.conversionApi.writer ).to.be.undefined; - expect( dispatcher.conversionApi.consumable ).to.be.undefined; + expect( dispatcher._conversionApi.writer ).to.be.undefined; + expect( dispatcher._conversionApi.consumable ).to.be.undefined; } ); it( 'should not fire attributes events for non-collapsed selection', () => { @@ -369,8 +737,8 @@ describe( 'DowncastDispatcher', () => { expect( dispatcher.fire.calledWith( 'attribute:bold' ) ).to.be.false; expect( dispatcher.fire.calledWith( 'attribute:italic' ) ).to.be.false; - expect( dispatcher.conversionApi.writer ).to.be.undefined; - expect( dispatcher.conversionApi.consumable ).to.be.undefined; + expect( dispatcher._conversionApi.writer ).to.be.undefined; + expect( dispatcher._conversionApi.consumable ).to.be.undefined; } ); it( 'should fire attributes events for collapsed selection', () => { @@ -390,8 +758,8 @@ describe( 'DowncastDispatcher', () => { expect( dispatcher.fire.calledWith( 'attribute:bold:$text' ) ).to.be.true; - expect( dispatcher.conversionApi.writer ).to.be.undefined; - expect( dispatcher.conversionApi.consumable ).to.be.undefined; + expect( dispatcher._conversionApi.writer ).to.be.undefined; + expect( dispatcher._conversionApi.consumable ).to.be.undefined; } ); it( 'should not fire attributes events if attribute has been consumed', () => { @@ -418,8 +786,8 @@ describe( 'DowncastDispatcher', () => { expect( dispatcher.fire.calledWith( 'attribute:bold' ) ).to.be.false; - expect( dispatcher.conversionApi.writer ).to.be.undefined; - expect( dispatcher.conversionApi.consumable ).to.be.undefined; + expect( dispatcher._conversionApi.writer ).to.be.undefined; + expect( dispatcher._conversionApi.consumable ).to.be.undefined; } ); it( 'should fire events for markers for collapsed selection', () => { @@ -438,8 +806,8 @@ describe( 'DowncastDispatcher', () => { expect( dispatcher.fire.calledWith( 'addMarker:name' ) ).to.be.true; - expect( dispatcher.conversionApi.writer ).to.be.undefined; - expect( dispatcher.conversionApi.consumable ).to.be.undefined; + expect( dispatcher._conversionApi.writer ).to.be.undefined; + expect( dispatcher._conversionApi.consumable ).to.be.undefined; } ); it( 'should fire events for all markers of the same group for collapsed selection', () => { @@ -467,8 +835,8 @@ describe( 'DowncastDispatcher', () => { expect( dispatcher.fire.calledWith( 'addMarker:name:1' ) ).to.be.true; expect( dispatcher.fire.calledWith( 'addMarker:name:2' ) ).to.be.true; - expect( dispatcher.conversionApi.writer ).to.be.undefined; - expect( dispatcher.conversionApi.consumable ).to.be.undefined; + expect( dispatcher._conversionApi.writer ).to.be.undefined; + expect( dispatcher._conversionApi.consumable ).to.be.undefined; } ); it( 'should not fire events for markers for non-collapsed selection', () => { @@ -484,8 +852,8 @@ describe( 'DowncastDispatcher', () => { expect( dispatcher.fire.calledWith( 'addMarker:name' ) ).to.be.false; - expect( dispatcher.conversionApi.writer ).to.be.undefined; - expect( dispatcher.conversionApi.consumable ).to.be.undefined; + expect( dispatcher._conversionApi.writer ).to.be.undefined; + expect( dispatcher._conversionApi.consumable ).to.be.undefined; } ); it( 'should not fire event for marker if selection is in a element with custom highlight handling', () => { @@ -506,7 +874,7 @@ describe( 'DowncastDispatcher', () => { viewFigure._setCustomProperty( 'removeHighlight', () => {} ); // Create mapper mock. - dispatcher.conversionApi.mapper = { + dispatcher._conversionApi.mapper = { toViewElement( modelElement ) { if ( modelElement == image ) { return viewFigure; @@ -529,8 +897,8 @@ describe( 'DowncastDispatcher', () => { expect( dispatcher.fire.calledWith( 'addMarker:name' ) ).to.be.false; - expect( dispatcher.conversionApi.writer ).to.be.undefined; - expect( dispatcher.conversionApi.consumable ).to.be.undefined; + expect( dispatcher._conversionApi.writer ).to.be.undefined; + expect( dispatcher._conversionApi.consumable ).to.be.undefined; } ); it( 'should not fire events if information about marker has been consumed', () => { @@ -556,12 +924,12 @@ describe( 'DowncastDispatcher', () => { expect( dispatcher.fire.calledWith( 'addMarker:foo' ) ).to.be.true; expect( dispatcher.fire.calledWith( 'addMarker:bar' ) ).to.be.false; - expect( dispatcher.conversionApi.writer ).to.be.undefined; - expect( dispatcher.conversionApi.consumable ).to.be.undefined; + expect( dispatcher._conversionApi.writer ).to.be.undefined; + expect( dispatcher._conversionApi.consumable ).to.be.undefined; } ); } ); - describe( 'convertMarkerAdd', () => { + describe( '_convertMarkerAdd', () => { let element, text; beforeEach( () => { @@ -582,12 +950,12 @@ describe( 'DowncastDispatcher', () => { expect( data.markerRange.isEqual( range ) ).to.be.true; } ); - dispatcher.convertMarkerAdd( 'name', range ); + dispatcher._convertMarkerAdd( 'name', range, dispatcher._createConversionApi() ); expect( spy.calledOnce ).to.be.true; - expect( dispatcher.conversionApi.writer ).to.be.undefined; - expect( dispatcher.conversionApi.consumable ).to.be.undefined; + expect( dispatcher._conversionApi.writer ).to.be.undefined; + expect( dispatcher._conversionApi.consumable ).to.be.undefined; } ); it( 'should convert marker in document fragment', () => { @@ -596,24 +964,24 @@ describe( 'DowncastDispatcher', () => { const eleRange = model.createRange( model.createPositionAt( docFrag, 1 ), model.createPositionAt( docFrag, 2 ) ); sinon.spy( dispatcher, 'fire' ); - dispatcher.convertMarkerAdd( 'name', eleRange ); + dispatcher._convertMarkerAdd( 'name', eleRange, dispatcher._createConversionApi() ); expect( dispatcher.fire.called ).to.be.true; - expect( dispatcher.conversionApi.writer ).to.be.undefined; - expect( dispatcher.conversionApi.consumable ).to.be.undefined; + expect( dispatcher._conversionApi.writer ).to.be.undefined; + expect( dispatcher._conversionApi.consumable ).to.be.undefined; } ); it( 'should not convert marker if it is in graveyard', () => { const gyRange = model.createRange( model.createPositionAt( doc.graveyard, 0 ), model.createPositionAt( doc.graveyard, 0 ) ); sinon.spy( dispatcher, 'fire' ); - dispatcher.convertMarkerAdd( 'name', gyRange ); + dispatcher._convertMarkerAdd( 'name', gyRange, dispatcher._createConversionApi() ); expect( dispatcher.fire.called ).to.be.false; - expect( dispatcher.conversionApi.writer ).to.be.undefined; - expect( dispatcher.conversionApi.consumable ).to.be.undefined; + expect( dispatcher._conversionApi.writer ).to.be.undefined; + expect( dispatcher._conversionApi.consumable ).to.be.undefined; } ); it( 'should fire addMarker event for whole non-collapsed marker and for each item in the range', () => { @@ -645,7 +1013,7 @@ describe( 'DowncastDispatcher', () => { } } ); - dispatcher.convertMarkerAdd( 'name', range ); + dispatcher._convertMarkerAdd( 'name', range, dispatcher._createConversionApi() ); expect( spyWholeRange.calledOnce ).to.be.true; expect( spyItems.calledTwice ).to.be.true; @@ -653,8 +1021,8 @@ describe( 'DowncastDispatcher', () => { expect( items[ 0 ] ).to.equal( element ); expect( items[ 1 ].data ).to.equal( text.data ); - expect( dispatcher.conversionApi.writer ).to.be.undefined; - expect( dispatcher.conversionApi.consumable ).to.be.undefined; + expect( dispatcher._conversionApi.writer ).to.be.undefined; + expect( dispatcher._conversionApi.consumable ).to.be.undefined; } ); it( 'should not fire conversion for non-collapsed marker items if marker was consumed in earlier event', () => { @@ -674,12 +1042,12 @@ describe( 'DowncastDispatcher', () => { } } ); - dispatcher.convertMarkerAdd( 'name', range ); + dispatcher._convertMarkerAdd( 'name', range, dispatcher._createConversionApi() ); expect( spyItems.called ).to.be.false; - expect( dispatcher.conversionApi.writer ).to.be.undefined; - expect( dispatcher.conversionApi.consumable ).to.be.undefined; + expect( dispatcher._conversionApi.writer ).to.be.undefined; + expect( dispatcher._conversionApi.consumable ).to.be.undefined; } ); it( 'should be possible to override #1', () => { @@ -702,13 +1070,13 @@ describe( 'DowncastDispatcher', () => { } }, { priority: 'high' } ); - dispatcher.convertMarkerAdd( 'marker', range ); + dispatcher._convertMarkerAdd( 'marker', range, dispatcher._createConversionApi() ); expect( addMarkerSpy.called ).to.be.false; expect( highAddMarkerSpy.calledOnce ).to.be.true; - expect( dispatcher.conversionApi.writer ).to.be.undefined; - expect( dispatcher.conversionApi.consumable ).to.be.undefined; + expect( dispatcher._conversionApi.writer ).to.be.undefined; + expect( dispatcher._conversionApi.consumable ).to.be.undefined; } ); it( 'should be possible to override #2', () => { @@ -731,19 +1099,19 @@ describe( 'DowncastDispatcher', () => { } }, { priority: 'high' } ); - dispatcher.convertMarkerAdd( 'marker', range ); + dispatcher._convertMarkerAdd( 'marker', range, dispatcher._createConversionApi() ); expect( addMarkerSpy.called ).to.be.false; // Called once for each item, twice total. expect( highAddMarkerSpy.calledTwice ).to.be.true; - expect( dispatcher.conversionApi.writer ).to.be.undefined; - expect( dispatcher.conversionApi.consumable ).to.be.undefined; + expect( dispatcher._conversionApi.writer ).to.be.undefined; + expect( dispatcher._conversionApi.consumable ).to.be.undefined; } ); } ); - describe( 'convertMarkerRemove', () => { + describe( '_convertMarkerRemove', () => { let range, element, text; beforeEach( () => { @@ -757,24 +1125,24 @@ describe( 'DowncastDispatcher', () => { it( 'should fire removeMarker event', () => { sinon.spy( dispatcher, 'fire' ); - dispatcher.convertMarkerRemove( 'name', range ); + dispatcher._convertMarkerRemove( 'name', range, dispatcher._createConversionApi() ); expect( dispatcher.fire.calledWith( 'removeMarker:name' ) ).to.be.true; - expect( dispatcher.conversionApi.writer ).to.be.undefined; - expect( dispatcher.conversionApi.consumable ).to.be.undefined; + expect( dispatcher._conversionApi.writer ).to.be.undefined; + expect( dispatcher._conversionApi.consumable ).to.be.undefined; } ); it( 'should not convert marker if it is in graveyard', () => { const gyRange = model.createRange( model.createPositionAt( doc.graveyard, 0 ), model.createPositionAt( doc.graveyard, 0 ) ); sinon.spy( dispatcher, 'fire' ); - dispatcher.convertMarkerRemove( 'name', gyRange ); + dispatcher._convertMarkerRemove( 'name', gyRange, dispatcher._createConversionApi() ); expect( dispatcher.fire.called ).to.be.false; - expect( dispatcher.conversionApi.writer ).to.be.undefined; - expect( dispatcher.conversionApi.consumable ).to.be.undefined; + expect( dispatcher._conversionApi.writer ).to.be.undefined; + expect( dispatcher._conversionApi.consumable ).to.be.undefined; } ); it( 'should convert marker in document fragment', () => { @@ -783,12 +1151,12 @@ describe( 'DowncastDispatcher', () => { const eleRange = model.createRange( model.createPositionAt( docFrag, 1 ), model.createPositionAt( docFrag, 2 ) ); sinon.spy( dispatcher, 'fire' ); - dispatcher.convertMarkerRemove( 'name', eleRange ); + dispatcher._convertMarkerRemove( 'name', eleRange, dispatcher._createConversionApi() ); expect( dispatcher.fire.called ).to.be.true; - expect( dispatcher.conversionApi.writer ).to.be.undefined; - expect( dispatcher.conversionApi.consumable ).to.be.undefined; + expect( dispatcher._conversionApi.writer ).to.be.undefined; + expect( dispatcher._conversionApi.consumable ).to.be.undefined; } ); it( 'should fire conversion for the range', () => { @@ -799,10 +1167,10 @@ describe( 'DowncastDispatcher', () => { expect( data.markerRange.isEqual( range ) ).to.be.true; } ); - dispatcher.convertMarkerRemove( 'name', range ); + dispatcher._convertMarkerRemove( 'name', range, dispatcher._createConversionApi() ); - expect( dispatcher.conversionApi.writer ).to.be.undefined; - expect( dispatcher.conversionApi.consumable ).to.be.undefined; + expect( dispatcher._conversionApi.writer ).to.be.undefined; + expect( dispatcher._conversionApi.consumable ).to.be.undefined; } ); it( 'should be possible to override', () => { @@ -819,13 +1187,20 @@ describe( 'DowncastDispatcher', () => { evt.stop(); }, { priority: 'high' } ); - dispatcher.convertMarkerRemove( 'marker', range ); + dispatcher._convertMarkerRemove( 'marker', range, dispatcher._createConversionApi() ); expect( removeMarkerSpy.called ).to.be.false; expect( highRemoveMarkerSpy.calledOnce ).to.be.true; - expect( dispatcher.conversionApi.writer ).to.be.undefined; - expect( dispatcher.conversionApi.consumable ).to.be.undefined; + expect( dispatcher._conversionApi.writer ).to.be.undefined; + expect( dispatcher._conversionApi.consumable ).to.be.undefined; } ); } ); + + function assertConversionApi( conversionApi ) { + expect( conversionApi ).to.have.property( 'writer' ).that.is.instanceof( DowncastWriter ); + expect( conversionApi ).to.have.property( 'consumable' ).that.is.instanceof( ModelConsumable ); + expect( conversionApi ).to.have.property( 'mapper' ).that.is.equal( mapper ); + expect( conversionApi ).to.have.property( 'apiObj' ).that.is.equal( apiObj ); + } } ); diff --git a/packages/ckeditor5-engine/tests/conversion/downcasthelpers.js b/packages/ckeditor5-engine/tests/conversion/downcasthelpers.js index ce3b175014a..297dd40d4de 100644 --- a/packages/ckeditor5-engine/tests/conversion/downcasthelpers.js +++ b/packages/ckeditor5-engine/tests/conversion/downcasthelpers.js @@ -44,6 +44,8 @@ import { toWidget } from '@ckeditor/ckeditor5-widget/src/utils'; describe( 'DowncastHelpers', () => { let model, modelRoot, viewRoot, downcastHelpers, controller, modelRootStart; + testUtils.createSinonSandbox(); + beforeEach( () => { model = new Model(); const modelDoc = model.document; @@ -116,1193 +118,1423 @@ describe( 'DowncastHelpers', () => { expectResult( '

' ); } ); + } ); - describe( 'config.triggerBy', () => { - describe( 'with simple block view structure (without children)', () => { - beforeEach( () => { - model.schema.register( 'simpleBlock', { - allowIn: '$root', - allowAttributes: [ 'toStyle', 'toClass' ] - } ); - downcastHelpers.elementToElement( { - model: 'simpleBlock', - view: ( modelElement, { writer } ) => { - return writer.createContainerElement( 'div', getViewAttributes( modelElement ) ); - }, - triggerBy: { - attributes: [ 'toStyle', 'toClass' ] - } - } ); + describe( 'elementToStructure()', () => { + it( 'should be chainable', () => { + expect( downcastHelpers.elementToStructure( { model: 'paragraph', view: 'p' } ) ).to.equal( downcastHelpers ); + } ); + + it( 'config.view is a string', () => { + downcastHelpers.elementToStructure( { model: 'paragraph', view: 'p' } ); + + model.change( writer => { + writer.insertElement( 'paragraph', modelRoot, 0 ); + } ); + + expectResult( '

' ); + } ); + + it( 'can be overwritten using converterPriority', () => { + downcastHelpers.elementToStructure( { model: 'paragraph', view: 'p' } ); + downcastHelpers.elementToStructure( { model: 'paragraph', view: 'foo', converterPriority: 'high' } ); + + model.change( writer => { + writer.insertElement( 'paragraph', modelRoot, 0 ); + } ); + + expectResult( '' ); + } ); + + it( 'config.view is a view element definition', () => { + downcastHelpers.elementToStructure( { + model: 'fancyParagraph', + view: { + name: 'p', + classes: 'fancy' + } + } ); + + model.change( writer => { + writer.insertElement( 'fancyParagraph', modelRoot, 0 ); + } ); + + expectResult( '

' ); + } ); + + it( 'config.view is a function', () => { + downcastHelpers.elementToStructure( { + model: 'heading', + view: ( modelElement, { writer } ) => writer.createContainerElement( 'h' + modelElement.getAttribute( 'level' ) ) + } ); + + model.change( writer => { + writer.insertElement( 'heading', { level: 2 }, modelRoot, 0 ); + } ); + + expectResult( '

' ); + } ); + + it( 'config.view is a function that does not return view element', () => { + downcastHelpers.elementToStructure( { + model: 'heading', + view: () => null + } ); + + model.change( writer => { + writer.insertElement( 'heading', {}, modelRoot, 0 ); + } ); + + expectResult( '' ); + } ); + + describe( 'converting element together with selected attributes', () => { + it( 'should allow passing a list of attributes to convert and consume', () => { + model.schema.register( 'simpleBlock', { + allowIn: '$root', + allowAttributes: [ 'toStyle', 'toClass' ] } ); - it( 'should convert on insert', () => { - model.change( writer => { - writer.insertElement( 'simpleBlock', modelRoot, 0 ); - } ); + downcastHelpers.elementToStructure( { + model: { + name: 'simpleBlock', + attributes: [ 'toStyle', 'toClass' ] + }, + view: ( modelElement, { writer } ) => { + return writer.createContainerElement( 'div', getViewAttributes( modelElement ) ); + } + } ); + + let consumable; + + controller.downcastDispatcher.on( 'insert:simpleBlock', ( evt, data, conversionApi ) => { + consumable = conversionApi.consumable; + }, { priority: 'low' } ); - expectResult( '
' ); + setModelData( model, '' ); + + expectResult( '
' ); + + expect( consumable.test( modelRoot.getChild( 0 ), 'attribute:toStyle' ) ).to.be.false; + expect( consumable.test( modelRoot.getChild( 0 ), 'attribute:toClass' ) ).to.be.null; + } ); + + it( 'should allow passing a single attribute name to convert and consume', () => { + model.schema.register( 'simpleBlock', { + allowIn: '$root', + allowAttributes: [ 'toStyle', 'toClass' ] } ); - it( 'should convert on attribute set', () => { - setModelData( model, '' ); + downcastHelpers.elementToStructure( { + model: { + name: 'simpleBlock', + attributes: 'toStyle' + }, + view: ( modelElement, { writer } ) => { + return writer.createContainerElement( 'div', getViewAttributes( modelElement ) ); + } + } ); - const [ viewBefore ] = getNodes(); + let consumable; - model.change( writer => { - writer.setAttribute( 'toStyle', 'display:block', modelRoot.getChild( 0 ) ); - } ); + controller.downcastDispatcher.on( 'insert:simpleBlock', ( evt, data, conversionApi ) => { + consumable = conversionApi.consumable; + }, { priority: 'low' } ); + + setModelData( model, '' ); + + expectResult( '
' ); - const [ viewAfter ] = getNodes(); + expect( consumable.test( modelRoot.getChild( 0 ), 'attribute:toStyle' ) ).to.be.false; + expect( consumable.test( modelRoot.getChild( 0 ), 'attribute:toClass' ) ).to.be.null; + } ); + } ); - expectResult( '
' ); - expect( viewAfter ).to.not.equal( viewBefore ); + describe( 'with simple block view structure (without slots, without reconvert on children list change)', () => { + beforeEach( () => { + model.schema.register( 'simpleBlock', { + allowIn: '$root', + allowAttributes: [ 'toStyle', 'toClass' ] } ); - it( 'should convert on attribute change', () => { - setModelData( model, '' ); + downcastHelpers.elementToStructure( { + model: { + name: 'simpleBlock', + attributes: [ 'toStyle', 'toClass' ] + }, + view: ( modelElement, { writer } ) => { + return writer.createContainerElement( 'div', getViewAttributes( modelElement ) ); + } + } ); + } ); - const [ viewBefore ] = getNodes(); + it( 'should convert on insert', () => { + controller.downcastDispatcher.on( 'insert', ( evt, data ) => { + expect( data ).to.not.have.property( 'reconversion' ); + } ); - model.change( writer => { - writer.setAttribute( 'toStyle', 'display:inline', modelRoot.getChild( 0 ) ); - } ); + model.change( writer => { + writer.insertElement( 'simpleBlock', modelRoot, 0 ); + } ); - const [ viewAfter ] = getNodes(); + expectResult( '
' ); + } ); - expectResult( '
' ); + it( 'should convert on attribute set', () => { + setModelData( model, '' ); - expect( viewAfter ).to.not.equal( viewBefore ); + controller.downcastDispatcher.on( 'insert', ( evt, data ) => { + expect( data ).to.have.property( 'reconversion' ).to.be.true; } ); - it( 'should convert on attribute remove', () => { - setModelData( model, '' ); + const [ viewBefore ] = getNodes(); - model.change( writer => { - writer.removeAttribute( 'toStyle', modelRoot.getChild( 0 ) ); - } ); + model.change( writer => { + writer.setAttribute( 'toStyle', 'display:block', modelRoot.getChild( 0 ) ); + } ); + + const [ viewAfter ] = getNodes(); + + expectResult( '
' ); + expect( viewAfter ).to.not.equal( viewBefore ); + } ); - expectResult( '
' ); + it( 'should convert on attribute change', () => { + setModelData( model, '' ); + + controller.downcastDispatcher.on( 'insert', ( evt, data ) => { + expect( data ).to.have.property( 'reconversion' ).to.be.true; } ); - it( 'should convert on one attribute add and other remove', () => { - setModelData( model, '' ); + const [ viewBefore ] = getNodes(); - model.change( writer => { - writer.removeAttribute( 'toStyle', modelRoot.getChild( 0 ) ); - writer.setAttribute( 'toClass', true, modelRoot.getChild( 0 ) ); - } ); + model.change( writer => { + writer.setAttribute( 'toStyle', 'display:inline', modelRoot.getChild( 0 ) ); + } ); + + const [ viewAfter ] = getNodes(); - expectResult( '
' ); + expectResult( '
' ); + + expect( viewAfter ).to.not.equal( viewBefore ); + } ); + + it( 'should convert on attribute remove', () => { + setModelData( model, '' ); + + controller.downcastDispatcher.on( 'insert', ( evt, data ) => { + expect( data ).to.have.property( 'reconversion' ).to.be.true; } ); - it( 'should properly re-bind mapper mappings and retain markers', () => { - downcastHelpers.elementToElement( { - model: 'simpleBlock', - view: ( modelElement, { writer } ) => { - const viewElement = writer.createContainerElement( 'div', getViewAttributes( modelElement ) ); + model.change( writer => { + writer.removeAttribute( 'toStyle', modelRoot.getChild( 0 ) ); + } ); - return toWidget( viewElement, writer ); - }, - triggerBy: { - attributes: [ 'toStyle', 'toClass' ] - }, - converterPriority: 'high' - } ); + expectResult( '
' ); + } ); - const mapper = controller.mapper; + it( 'should convert on one attribute add and other remove', () => { + setModelData( model, '' ); - downcastHelpers.markerToHighlight( { - model: 'myMarker', - view: { classes: 'foo' } - } ); + controller.downcastDispatcher.on( 'insert', ( evt, data ) => { + expect( data ).to.have.property( 'reconversion' ).to.be.true; + } ); - setModelData( model, '' ); + model.change( writer => { + writer.removeAttribute( 'toStyle', modelRoot.getChild( 0 ) ); + writer.setAttribute( 'toClass', true, modelRoot.getChild( 0 ) ); + } ); - const modelElement = modelRoot.getChild( 0 ); - const [ viewBefore ] = getNodes(); + expectResult( '
' ); + } ); - model.change( writer => { - writer.addMarker( 'myMarker', { range: writer.createRangeOn( modelElement ), usingOperation: false } ); - } ); + it( 'should properly re-bind mapper mappings and retain markers', () => { + downcastHelpers.elementToElement( { + model: 'simpleBlock', + view: ( modelElement, { writer } ) => { + const viewElement = writer.createContainerElement( 'div', getViewAttributes( modelElement ) ); - expect( mapper.toViewElement( modelElement ) ).to.equal( viewBefore ); - expect( mapper.toModelElement( viewBefore ) ).to.equal( modelElement ); - expect( mapper.markerNameToElements( 'myMarker' ).has( viewBefore ) ).to.be.true; + return toWidget( viewElement, writer ); + }, + triggerBy: { + attributes: [ 'toStyle', 'toClass' ] + }, + converterPriority: 'high' + } ); - model.change( writer => { - writer.setAttribute( 'toStyle', 'display:block', modelElement ); - } ); + const mapper = controller.mapper; + + downcastHelpers.markerToHighlight( { + model: 'myMarker', + view: { classes: 'foo' } + } ); + + setModelData( model, '' ); - const [ viewAfter ] = getNodes(); + const modelElement = modelRoot.getChild( 0 ); + const [ viewBefore ] = getNodes(); - expect( mapper.toViewElement( modelElement ) ).to.equal( viewAfter ); - expect( mapper.toModelElement( viewBefore ) ).to.be.undefined; - expect( mapper.toModelElement( viewAfter ) ).to.equal( modelElement ); - expect( mapper.markerNameToElements( 'myMarker' ).has( viewAfter ) ).to.be.true; - expect( mapper.markerNameToElements( 'myMarker' ).has( viewBefore ) ).to.be.false; + model.change( writer => { + writer.addMarker( 'myMarker', { range: writer.createRangeOn( modelElement ), usingOperation: false } ); } ); - it( 'should do nothing if non-triggerBy attribute has changed', () => { - setModelData( model, '' ); + expect( mapper.toViewElement( modelElement ) ).to.equal( viewBefore ); + expect( mapper.toModelElement( viewBefore ) ).to.equal( modelElement ); + expect( mapper.markerNameToElements( 'myMarker' ).has( viewBefore ) ).to.be.true; - const [ viewBefore ] = getNodes(); + model.change( writer => { + writer.setAttribute( 'toStyle', 'display:block', modelElement ); + } ); - model.change( writer => { - writer.setAttribute( 'notTriggered', true, modelRoot.getChild( 0 ) ); - } ); + const [ viewAfter ] = getNodes(); - const [ viewAfter ] = getNodes(); + expect( mapper.toViewElement( modelElement ) ).to.equal( viewAfter ); + expect( mapper.toModelElement( viewBefore ) ).to.be.undefined; + expect( mapper.toModelElement( viewAfter ) ).to.equal( modelElement ); + expect( mapper.markerNameToElements( 'myMarker' ).has( viewAfter ) ).to.be.true; + expect( mapper.markerNameToElements( 'myMarker' ).has( viewBefore ) ).to.be.false; + } ); - expectResult( '
' ); + it( 'should not reconvert if non watched attribute has changed', () => { + setModelData( model, '' ); - expect( viewAfter ).to.equal( viewBefore ); + controller.downcastDispatcher.on( 'insert', ( evt, data ) => { + expect( data ).to.not.have.property( 'reconversion' ); } ); + + const [ viewBefore ] = getNodes(); + + model.change( writer => { + writer.setAttribute( 'notTriggered', true, modelRoot.getChild( 0 ) ); + } ); + + const [ viewAfter ] = getNodes(); + + expectResult( '
' ); + + expect( viewAfter ).to.equal( viewBefore ); } ); - describe( 'with simple block view structure (with children)', () => { - beforeEach( () => { - model.schema.register( 'simpleBlock', { - allowIn: '$root', - allowAttributes: [ 'toStyle', 'toClass' ] - } ); - downcastHelpers.elementToElement( { - model: 'simpleBlock', - view: ( modelElement, { writer } ) => { - return writer.createContainerElement( 'div', getViewAttributes( modelElement ) ); - }, - triggerBy: { - attributes: [ 'toStyle', 'toClass' ] - } - } ); + it( 'should not reconvert on child element added', () => { + model.schema.register( 'paragraph', { + inheritAllFrom: '$block', + allowIn: 'simpleBlock' + } ); - model.schema.register( 'paragraph', { - inheritAllFrom: '$block', - allowIn: 'simpleBlock' - } ); - downcastHelpers.elementToElement( { model: 'paragraph', view: 'p' } ); + downcastHelpers.elementToElement( { + model: 'paragraph', + view: 'p' } ); - it( 'should convert on insert', () => { - model.change( writer => { - const simpleBlock = writer.createElement( 'simpleBlock' ); - const paragraph = writer.createElement( 'paragraph' ); + setModelData( model, '' ); - writer.insert( simpleBlock, modelRoot, 0 ); - writer.insert( paragraph, simpleBlock, 0 ); - writer.insertText( 'foo', paragraph, 0 ); - } ); + controller.downcastDispatcher.on( 'insert', ( evt, data ) => { + expect( data ).to.not.have.property( 'reconversion' ); + } ); + + const [ viewBefore ] = getNodes(); - expectResult( '

foo

' ); + model.change( writer => { + writer.insertElement( 'paragraph', modelRoot.getChild( 0 ), 0 ); } ); - it( 'should convert on attribute set', () => { - setModelData( model, 'foo' ); + const [ viewAfter ] = getNodes(); - const [ viewBefore, paraBefore, textBefore ] = getNodes(); + expectResult( '

' ); - model.change( writer => { - writer.setAttribute( 'toStyle', 'display:block', modelRoot.getChild( 0 ) ); - } ); + expect( viewAfter ).to.equal( viewBefore ); + } ); + } ); - const [ viewAfter, paraAfter, textAfter ] = getNodes(); + describe( 'with simple block view structure (with slots, without reconvert on children list change)', () => { + beforeEach( () => { + model.schema.register( 'simpleBlock', { + allowIn: '$root', + allowAttributes: [ 'toStyle', 'toClass' ] + } ); + + downcastHelpers.elementToStructure( { + model: { + name: 'simpleBlock', + attributes: [ 'toStyle', 'toClass' ] + }, + view: ( modelElement, { writer, slotFor } ) => { + const viewElement = writer.createContainerElement( 'div', getViewAttributes( modelElement ) ); - expectResult( '

foo

' ); + writer.insert( writer.createPositionAt( viewElement, 0 ), slotFor( 'children' ) ); - expect( viewAfter, 'simpleBlock' ).to.not.equal( viewBefore ); - expect( paraAfter, 'para' ).to.equal( paraBefore ); - expect( textAfter, 'text' ).to.equal( textBefore ); + return viewElement; + } } ); - it( 'should convert on attribute change', () => { - setModelData( model, 'foo' ); + model.schema.register( 'paragraph', { + inheritAllFrom: '$block', + allowIn: 'simpleBlock' + } ); - const [ viewBefore, paraBefore, textBefore ] = getNodes(); + downcastHelpers.elementToElement( { + model: 'paragraph', + view: 'p' + } ); + } ); - model.change( writer => { - writer.setAttribute( 'toStyle', 'display:inline', modelRoot.getChild( 0 ) ); - } ); + it( 'should convert on insert', () => { + controller.downcastDispatcher.on( 'insert', ( evt, data ) => { + expect( data ).to.not.have.property( 'reconversion' ); + } ); - const [ viewAfter, paraAfter, textAfter ] = getNodes(); + model.change( writer => { + const simpleBlock = writer.createElement( 'simpleBlock' ); + const paragraph = writer.createElement( 'paragraph' ); + + writer.insert( simpleBlock, modelRoot, 0 ); + writer.insert( paragraph, simpleBlock, 0 ); + writer.insertText( 'foo', paragraph, 0 ); + } ); - expectResult( '

foo

' ); + expectResult( '

foo

' ); + } ); + + it( 'should convert on attribute set', () => { + setModelData( model, 'foo' ); - expect( viewAfter, 'simpleBlock' ).to.not.equal( viewBefore ); - expect( paraAfter, 'para' ).to.equal( paraBefore ); - expect( textAfter, 'text' ).to.equal( textBefore ); + controller.downcastDispatcher.on( 'insert', ( evt, data ) => { + expect( data ).to.have.property( 'reconversion' ).to.be.true; } ); - it( 'should convert on attribute remove', () => { - setModelData( model, 'foo' ); + const [ viewBefore, paraBefore, textBefore ] = getNodes(); - model.change( writer => { - writer.removeAttribute( 'toStyle', modelRoot.getChild( 0 ) ); - } ); + model.change( writer => { + writer.setAttribute( 'toStyle', 'display:block', modelRoot.getChild( 0 ) ); + } ); - expectResult( '

foo

' ); + const [ viewAfter, paraAfter, textAfter ] = getNodes(); + + expectResult( '

foo

' ); + + expect( viewAfter, 'simpleBlock' ).to.not.equal( viewBefore ); + expect( paraAfter, 'para' ).to.equal( paraBefore ); + expect( textAfter, 'text' ).to.equal( textBefore ); + } ); + + it( 'should convert on attribute change', () => { + setModelData( model, 'foo' ); + + controller.downcastDispatcher.on( 'insert', ( evt, data ) => { + expect( data ).to.have.property( 'reconversion' ).to.be.true; } ); - it( 'should convert on one attribute add and other remove', () => { - setModelData( model, 'foo' ); + const [ viewBefore, paraBefore, textBefore ] = getNodes(); - model.change( writer => { - writer.removeAttribute( 'toStyle', modelRoot.getChild( 0 ) ); - writer.setAttribute( 'toClass', true, modelRoot.getChild( 0 ) ); - } ); + model.change( writer => { + writer.setAttribute( 'toStyle', 'display:inline', modelRoot.getChild( 0 ) ); + } ); + + const [ viewAfter, paraAfter, textAfter ] = getNodes(); + + expectResult( '

foo

' ); + + expect( viewAfter, 'simpleBlock' ).to.not.equal( viewBefore ); + expect( paraAfter, 'para' ).to.equal( paraBefore ); + expect( textAfter, 'text' ).to.equal( textBefore ); + } ); + + it( 'should convert on attribute remove', () => { + setModelData( model, 'foo' ); - expectResult( '

foo

' ); + controller.downcastDispatcher.on( 'insert', ( evt, data ) => { + expect( data ).to.have.property( 'reconversion' ).to.be.true; } ); - it( 'should do nothing if non-triggerBy attribute has changed', () => { - setModelData( model, 'foo' ); + model.change( writer => { + writer.removeAttribute( 'toStyle', modelRoot.getChild( 0 ) ); + } ); + + expectResult( '

foo

' ); + } ); - const [ viewBefore, paraBefore, textBefore ] = getNodes(); + it( 'should convert on one attribute add and other remove', () => { + setModelData( model, 'foo' ); - model.change( writer => { - writer.setAttribute( 'notTriggered', true, modelRoot.getChild( 0 ) ); - } ); + controller.downcastDispatcher.on( 'insert', ( evt, data ) => { + expect( data ).to.have.property( 'reconversion' ).to.be.true; + } ); + + model.change( writer => { + writer.removeAttribute( 'toStyle', modelRoot.getChild( 0 ) ); + writer.setAttribute( 'toClass', true, modelRoot.getChild( 0 ) ); + } ); - const [ viewAfter, paraAfter, textAfter ] = getNodes(); + expectResult( '

foo

' ); + } ); - expectResult( '

foo

' ); + it( 'should not reconvert if non watched attribute has changed', () => { + setModelData( model, 'foo' ); - expect( viewAfter, 'simpleBlock' ).to.equal( viewBefore ); - expect( paraAfter, 'para' ).to.equal( paraBefore ); - // TODO - is text always re-converted? - expect( textAfter, 'text' ).to.equal( textBefore ); + controller.downcastDispatcher.on( 'insert', ( evt, data ) => { + expect( data ).to.not.have.property( 'reconversion' ); } ); + + const [ viewBefore, paraBefore, textBefore ] = getNodes(); + + model.change( writer => { + writer.setAttribute( 'notTriggered', true, modelRoot.getChild( 0 ) ); + } ); + + const [ viewAfter, paraAfter, textAfter ] = getNodes(); + + expectResult( '

foo

' ); + + expect( viewAfter, 'simpleBlock' ).to.equal( viewBefore ); + expect( paraAfter, 'para' ).to.equal( paraBefore ); + expect( textAfter, 'text' ).to.equal( textBefore ); } ); - describe( 'with simple block view structure (with children - reconvert on child add)', () => { - beforeEach( () => { - model.schema.register( 'simpleBlock', { - allowIn: '$root' - } ); - downcastHelpers.elementToElement( { - model: 'simpleBlock', - view: 'div', - triggerBy: { - children: [ 'paragraph' ] - } - } ); + it( 'should not reconvert on child element added', () => { + setModelData( model, '' ); - model.schema.register( 'paragraph', { - inheritAllFrom: '$block', - allowIn: 'simpleBlock' - } ); - downcastHelpers.elementToElement( { model: 'paragraph', view: 'p' } ); + controller.downcastDispatcher.on( 'insert', ( evt, data ) => { + expect( data ).to.not.have.property( 'reconversion' ); } ); - it( 'should convert on insert', () => { - model.change( writer => { - const simpleBlock = writer.createElement( 'simpleBlock' ); - const paragraph = writer.createElement( 'paragraph' ); + const [ viewBefore ] = getNodes(); - writer.insert( simpleBlock, modelRoot, 0 ); - writer.insert( paragraph, simpleBlock, 0 ); - writer.insertText( 'foo', paragraph, 0 ); - } ); + model.change( writer => { + writer.insertElement( 'paragraph', modelRoot.getChild( 0 ), 0 ); + } ); - expectResult( '

foo

' ); + const [ viewAfter ] = getNodes(); + + expectResult( '

' ); + + expect( viewAfter ).to.equal( viewBefore ); + } ); + } ); + + describe( 'with simple block view structure (reconvert on child add)', () => { + beforeEach( () => { + model.schema.register( 'simpleBlock', { + allowIn: '$root' } ); - it( 'should convert on adding a child (at the beginning)', () => { - setModelData( model, 'foo' ); + downcastHelpers.elementToStructure( { + model: { + name: 'simpleBlock', + children: true + }, + view: ( modelElement, { writer, slotFor } ) => { + const viewElement = writer.createContainerElement( 'div' ); - const [ viewBefore, paraBefore, textBefore ] = getNodes(); + writer.insert( writer.createPositionAt( viewElement, 0 ), slotFor( 'children' ) ); - model.change( writer => { - const paragraph = writer.createElement( 'paragraph' ); - const text = writer.createText( 'bar' ); + return viewElement; + } + } ); - writer.insert( paragraph, modelRoot.getChild( 0 ), 0 ); - writer.insert( text, paragraph, 0 ); - } ); + model.schema.register( 'paragraph', { + inheritAllFrom: '$block', + allowIn: 'simpleBlock' + } ); - const [ viewAfter, /* insertedPara */, /* insertedText */, paraAfter, textAfter ] = getNodes(); + downcastHelpers.elementToElement( { + model: 'paragraph', + view: 'p' + } ); + } ); - expectResult( '

bar

foo

' ); + it( 'should convert on insert', () => { + controller.downcastDispatcher.on( 'insert', ( evt, data ) => { + expect( data ).to.not.have.property( 'reconversion' ); + } ); + + model.change( writer => { + const simpleBlock = writer.createElement( 'simpleBlock' ); + const paragraph = writer.createElement( 'paragraph' ); - expect( viewAfter, 'simpleBlock' ).to.not.equal( viewBefore ); - expect( paraAfter, 'para' ).to.equal( paraBefore ); - expect( textAfter, 'text' ).to.equal( textBefore ); + writer.insert( simpleBlock, modelRoot, 0 ); + writer.insert( paragraph, simpleBlock, 0 ); + writer.insertText( 'foo', paragraph, 0 ); } ); - it( 'should convert on adding a child (in the middle)', () => { - setModelData( model, - '' + - 'foobar' + - '' ); + expectResult( '

foo

' ); + } ); - const [ viewBefore, paraFooBefore, textFooBefore, paraBarBefore, textBarBefore ] = getNodes(); + it( 'should convert on adding a child (at the beginning)', () => { + setModelData( model, 'foo' ); - model.change( writer => { - const paragraph = writer.createElement( 'paragraph' ); - const text = writer.createText( 'baz' ); + const spy = sinon.spy(); - writer.insert( paragraph, modelRoot.getChild( 0 ), 1 ); - writer.insert( text, paragraph, 0 ); - } ); + controller.downcastDispatcher.on( 'insert:simpleBlock', ( evt, data ) => { + expect( data ).to.have.property( 'reconversion' ).to.be.true; + spy(); + } ); - const [ viewAfter, - paraFooAfter, textFooAfter, /* insertedPara */, /* insertedText */, paraBarAfter, textBarAfter - ] = getNodes(); + const [ viewBefore, paraBefore, textBefore ] = getNodes(); - expectResult( '

foo

baz

bar

' ); + model.change( writer => { + const paragraph = writer.createElement( 'paragraph' ); + const text = writer.createText( 'bar' ); - expect( viewAfter, 'simpleBlock' ).to.not.equal( viewBefore ); - expect( paraFooAfter, 'para foo' ).to.equal( paraFooBefore ); - expect( textFooAfter, 'text foo' ).to.equal( textFooBefore ); - expect( paraBarAfter, 'para bar' ).to.equal( paraBarBefore ); - expect( textBarAfter, 'text bar' ).to.equal( textBarBefore ); + writer.insert( paragraph, modelRoot.getChild( 0 ), 0 ); + writer.insert( text, paragraph, 0 ); } ); - it( 'should convert on adding a child (at the end)', () => { - setModelData( model, 'foo' ); + const [ viewAfter, /* insertedPara */, /* insertedText */, paraAfter, textAfter ] = getNodes(); - const [ viewBefore, paraBefore, textBefore ] = getNodes(); + expectResult( '

bar

foo

' ); - model.change( writer => { - const paragraph = writer.createElement( 'paragraph' ); - const text = writer.createText( 'bar' ); + expect( viewAfter, 'simpleBlock' ).to.not.equal( viewBefore ); + expect( paraAfter, 'para' ).to.equal( paraBefore ); + expect( textAfter, 'text' ).to.equal( textBefore ); + expect( spy.called ).to.be.true; + } ); - writer.insert( paragraph, modelRoot.getChild( 0 ), 1 ); - writer.insert( text, paragraph, 0 ); - } ); + it( 'should convert on adding a child (in the middle)', () => { + setModelData( model, + '' + + 'foo' + + 'bar' + + '' + ); - const [ viewAfter, paraAfter, textAfter ] = getNodes(); + const spy = sinon.spy(); + + controller.downcastDispatcher.on( 'insert:simpleBlock', ( evt, data ) => { + expect( data ).to.have.property( 'reconversion' ).to.be.true; + spy(); + } ); + + const [ viewBefore, paraFooBefore, textFooBefore, paraBarBefore, textBarBefore ] = getNodes(); - expectResult( '

foo

bar

' ); + model.change( writer => { + const paragraph = writer.createElement( 'paragraph' ); + const text = writer.createText( 'baz' ); - expect( viewAfter, 'simpleBlock' ).to.not.equal( viewBefore ); - expect( paraAfter, 'para' ).to.equal( paraBefore ); - expect( textAfter, 'text' ).to.equal( textBefore ); + writer.insert( paragraph, modelRoot.getChild( 0 ), 1 ); + writer.insert( text, paragraph, 0 ); } ); - it( 'should convert on removing a child', () => { - setModelData( model, - 'foobar' ); + const [ viewAfter, + paraFooAfter, textFooAfter, /* insertedPara */, /* insertedText */, paraBarAfter, textBarAfter + ] = getNodes(); - const [ viewBefore, paraBefore, textBefore ] = getNodes(); + expectResult( '

foo

baz

bar

' ); - model.change( writer => { - writer.remove( modelRoot.getNodeByPath( [ 0, 1 ] ) ); - } ); + expect( viewAfter, 'simpleBlock' ).to.not.equal( viewBefore ); + expect( paraFooAfter, 'para foo' ).to.equal( paraFooBefore ); + expect( textFooAfter, 'text foo' ).to.equal( textFooBefore ); + expect( paraBarAfter, 'para bar' ).to.equal( paraBarBefore ); + expect( textBarAfter, 'text bar' ).to.equal( textBarBefore ); + expect( spy.called ).to.be.true; + } ); - const [ viewAfter, paraAfter, textAfter ] = getNodes(); + it( 'should convert on adding a child (at the end)', () => { + setModelData( model, 'foo' ); - expectResult( '

foo

' ); + const spy = sinon.spy(); - expect( viewAfter, 'simpleBlock' ).to.not.equal( viewBefore ); - expect( paraAfter, 'para' ).to.equal( paraBefore ); - expect( textAfter, 'text' ).to.equal( textBefore ); + controller.downcastDispatcher.on( 'insert:simpleBlock', ( evt, data ) => { + expect( data ).to.have.property( 'reconversion' ).to.be.true; + spy(); } ); + + const [ viewBefore, paraBefore, textBefore ] = getNodes(); + + model.change( writer => { + const paragraph = writer.createElement( 'paragraph' ); + const text = writer.createText( 'bar' ); + + writer.insert( paragraph, modelRoot.getChild( 0 ), 1 ); + writer.insert( text, paragraph, 0 ); + } ); + + const [ viewAfter, paraAfter, textAfter ] = getNodes(); + + expectResult( '

foo

bar

' ); + + expect( viewAfter, 'simpleBlock' ).to.not.equal( viewBefore ); + expect( paraAfter, 'para' ).to.equal( paraBefore ); + expect( textAfter, 'text' ).to.equal( textBefore ); + expect( spy.called ).to.be.true; } ); - describe( 'with complex view structure - no children allowed', () => { - beforeEach( () => { - model.schema.register( 'complex', { - allowIn: '$root', - allowAttributes: [ 'toStyle', 'toClass' ] - } ); - downcastHelpers.elementToElement( { - model: 'complex', - view: ( modelElement, { writer } ) => { - const outer = writer.createContainerElement( 'div', { class: 'complex-outer' } ); - const inner = writer.createContainerElement( 'div', getViewAttributes( modelElement ) ); + it( 'should convert on removing a child', () => { + setModelData( model, + 'foobar' ); - writer.insert( writer.createPositionAt( outer, 0 ), inner ); + const spy = sinon.spy(); - return outer; - }, - triggerBy: { - attributes: [ 'toStyle', 'toClass' ] - } - } ); + controller.downcastDispatcher.on( 'insert', ( evt, data ) => { + expect( data ).to.have.property( 'reconversion' ).to.be.true; + spy(); } ); - it( 'should convert on insert', () => { - model.change( writer => { - writer.insertElement( 'complex', modelRoot, 0 ); - } ); + const [ viewBefore, paraBefore, textBefore ] = getNodes(); - expectResult( '
' ); + model.change( writer => { + writer.remove( modelRoot.getNodeByPath( [ 0, 1 ] ) ); } ); - it( 'should convert on attribute set', () => { - setModelData( model, '' ); + const [ viewAfter, paraAfter, textAfter ] = getNodes(); - const [ outerDivBefore, innerDivBefore ] = getNodes(); + expectResult( '

foo

' ); - model.change( writer => { - writer.setAttribute( 'toStyle', 'display:block', modelRoot.getChild( 0 ) ); - } ); + expect( viewAfter, 'simpleBlock' ).to.not.equal( viewBefore ); + expect( paraAfter, 'para' ).to.equal( paraBefore ); + expect( textAfter, 'text' ).to.equal( textBefore ); + expect( spy.called ).to.be.true; + } ); - const [ outerDivAfter, innerDivAfter ] = getNodes(); + // https://github.com/ckeditor/ckeditor5/issues/9641 + it( 'should convert on multiple similar child hooks', () => { + model.schema.register( 'simpleBlock2', { + allowIn: '$root', + allowChildren: 'paragraph' + } ); - expectResult( '
' ); - expect( outerDivAfter, 'outer div' ).to.not.equal( outerDivBefore ); - expect( innerDivAfter, 'inner div' ).to.not.equal( innerDivBefore ); + downcastHelpers.elementToStructure( { + model: { + name: 'simpleBlock2', + children: true + }, + view: ( modelElement, { writer, slotFor } ) => { + const viewElement = writer.createContainerElement( 'div', { class: 'second' } ); + + writer.insert( writer.createPositionAt( viewElement, 0 ), slotFor( 'children' ) ); + + return viewElement; + } } ); - it( 'should convert on attribute change', () => { - setModelData( model, '' ); + setModelData( model, + 'foo' + + 'bar' + ); - model.change( writer => { - writer.setAttribute( 'toStyle', 'display:inline', modelRoot.getChild( 0 ) ); - } ); + const [ viewBefore0, paraBefore0, textBefore0 ] = getNodes( 0 ); + const [ viewBefore1, paraBefore1, textBefore1 ] = getNodes( 1 ); + + model.change( writer => { + const paragraph = writer.createElement( 'paragraph' ); + const text = writer.createText( 'abc' ); - expectResult( '
' ); + writer.insert( paragraph, modelRoot.getChild( 0 ), 1 ); + writer.insert( text, paragraph, 0 ); } ); - it( 'should convert on attribute remove', () => { - setModelData( model, '' ); + const [ viewAfter0, paraAfter0, textAfter0 ] = getNodes( 0 ); + const [ viewAfter1, paraAfter1, textAfter1 ] = getNodes( 1 ); - model.change( writer => { - writer.removeAttribute( 'toStyle', modelRoot.getChild( 0 ) ); - } ); + expectResult( + '

foo

abc

' + + '

bar

' + ); + + expect( viewAfter0, 'simpleBlock' ).to.not.equal( viewBefore0 ); + expect( paraAfter0, 'para' ).to.equal( paraBefore0 ); + expect( textAfter0, 'text' ).to.equal( textBefore0 ); + + expect( viewAfter1, 'simpleBlock' ).to.equal( viewBefore1 ); + expect( paraAfter1, 'para' ).to.equal( paraBefore1 ); + expect( textAfter1, 'text' ).to.equal( textBefore1 ); - expectResult( '
' ); + model.change( writer => { + const paragraph = writer.createElement( 'paragraph' ); + const text = writer.createText( '123' ); + + writer.insert( paragraph, modelRoot.getChild( 1 ), 1 ); + writer.insert( text, paragraph, 0 ); } ); - it( 'should convert on one attribute add and other remove', () => { - setModelData( model, '' ); + const [ viewAfterAfter0, paraAfterAfter0, textAfterAfter0 ] = getNodes( 0 ); + const [ viewAfterAfter1, paraAfterAfter1, textAfterAfter1 ] = getNodes( 1 ); - model.change( writer => { - writer.removeAttribute( 'toStyle', modelRoot.getChild( 0 ) ); - writer.setAttribute( 'toClass', true, modelRoot.getChild( 0 ) ); - } ); + expectResult( + '

foo

abc

' + + '

bar

123

' + ); + + expect( viewAfter0, 'simpleBlock' ).to.not.equal( viewBefore0 ); + expect( paraAfter0, 'para' ).to.equal( paraBefore0 ); + expect( textAfter0, 'text' ).to.equal( textBefore0 ); + + expect( viewAfter1, 'simpleBlock' ).to.equal( viewBefore1 ); + expect( paraAfter1, 'para' ).to.equal( paraBefore1 ); + expect( textAfter1, 'text' ).to.equal( textBefore1 ); + + expect( viewAfterAfter0, 'simpleBlock' ).to.equal( viewAfter0 ); + expect( paraAfterAfter0, 'para' ).to.equal( paraAfter0 ); + expect( textAfterAfter0, 'text' ).to.equal( textAfter0 ); - expectResult( '
' ); + expect( viewAfterAfter1, 'simpleBlock' ).to.not.equal( viewAfter1 ); + expect( paraAfterAfter1, 'para' ).to.equal( paraAfter1 ); + expect( textAfterAfter1, 'text' ).to.equal( textAfter1 ); + } ); + } ); + + describe( 'with complex view structure - single slot for all children', () => { + beforeEach( () => { + model.schema.register( 'complex', { + allowIn: '$root', + allowAttributes: [ 'toStyle', 'toClass' ] } ); - it( 'should do nothing if non-triggerBy attribute has changed', () => { - setModelData( model, '' ); + downcastHelpers.elementToStructure( { + model: { + name: 'complex', + attributes: [ 'toStyle', 'toClass' ], + children: true + }, + view: ( modelElement, { writer, slotFor } ) => { + const outer = writer.createContainerElement( 'div', { class: 'complex-outer' } ); + const inner = writer.createContainerElement( 'div', getViewAttributes( modelElement ) ); - const [ outerDivBefore, innerDivBefore ] = getNodes(); + writer.insert( writer.createPositionAt( outer, 0 ), inner ); + writer.insert( writer.createPositionAt( inner, 0 ), slotFor( 'children' ) ); - model.change( writer => { - writer.setAttribute( 'notTriggered', true, modelRoot.getChild( 0 ) ); - } ); + return outer; + } + } ); - const [ outerDivAfter, innerDivAfter ] = getNodes(); + model.schema.register( 'paragraph', { + inheritAllFrom: '$block', + allowIn: 'complex' + } ); - expectResult( '
' ); + downcastHelpers.elementToElement( { + model: 'paragraph', + view: 'p' + } ); + } ); - expect( outerDivAfter, 'outer div' ).to.equal( outerDivBefore ); - expect( innerDivAfter, 'inner div' ).to.equal( innerDivBefore ); + it( 'should convert on insert', () => { + const spy = sinon.spy(); + + controller.downcastDispatcher.on( 'insert:complex', ( evt, data ) => { + expect( data ).to.not.have.property( 'reconversion' ); + spy(); + } ); + + model.change( writer => { + writer.insertElement( 'complex', modelRoot, 0 ); } ); + + expectResult( '
' ); + expect( spy.called ).to.be.true; } ); - describe( 'with complex view structure (without slots)', () => { - beforeEach( () => { - model.schema.register( 'complex', { - allowIn: '$root', - allowAttributes: [ 'toStyle', 'toClass' ] - } ); - downcastHelpers.elementToElement( { - model: 'complex', - view: ( modelElement, { writer, mapper } ) => { - const outer = writer.createContainerElement( 'c-outer' ); - const inner = writer.createContainerElement( 'c-inner', getViewAttributes( modelElement ) ); + it( 'should convert on attribute set', () => { + setModelData( model, '' ); - writer.insert( writer.createPositionAt( outer, 0 ), inner ); - mapper.bindElements( modelElement, outer ); // Need for nested mapping - mapper.bindElements( modelElement, inner ); + const spy = sinon.spy(); - return outer; - }, - triggerBy: { - attributes: [ 'toStyle', 'toClass' ] - } - } ); + controller.downcastDispatcher.on( 'insert:complex', ( evt, data ) => { + expect( data ).to.have.property( 'reconversion' ).to.be.true; + spy(); + } ); - model.schema.register( 'paragraph', { - inheritAllFrom: '$block', - allowIn: 'complex' - } ); - downcastHelpers.elementToElement( { model: 'paragraph', view: 'p' } ); + const [ outerDivBefore, innerDivBefore ] = getNodes(); + + model.change( writer => { + writer.setAttribute( 'toStyle', 'display:block', modelRoot.getChild( 0 ) ); } ); - it( 'should convert on insert', () => { - model.change( writer => { - writer.insertElement( 'complex', modelRoot, 0 ); - } ); + const [ outerDivAfter, innerDivAfter ] = getNodes(); - expectResult( '' ); + expectResult( '
' ); + expect( outerDivAfter, 'outer div' ).to.not.equal( outerDivBefore ); + expect( innerDivAfter, 'inner div' ).to.not.equal( innerDivBefore ); + expect( spy.called ).to.be.true; + } ); + + it( 'should convert on attribute change', () => { + setModelData( model, '' ); + + const spy = sinon.spy(); + + controller.downcastDispatcher.on( 'insert:complex', ( evt, data ) => { + expect( data ).to.have.property( 'reconversion' ).to.be.true; + spy(); } ); - it( 'should convert on attribute set', () => { - setModelData( model, '' ); + model.change( writer => { + writer.setAttribute( 'toStyle', 'display:inline', modelRoot.getChild( 0 ) ); + } ); + + expectResult( '
' ); + expect( spy.called ).to.be.true; + } ); + + it( 'should convert on attribute remove', () => { + setModelData( model, '' ); - model.change( writer => { - writer.setAttribute( 'toStyle', 'display:block', modelRoot.getChild( 0 ) ); - } ); + const spy = sinon.spy(); - expectResult( '' ); + controller.downcastDispatcher.on( 'insert:complex', ( evt, data ) => { + expect( data ).to.have.property( 'reconversion' ).to.be.true; + spy(); } ); - it( 'should convert on attribute remove', () => { - setModelData( model, '' ); + model.change( writer => { + writer.removeAttribute( 'toStyle', modelRoot.getChild( 0 ) ); + } ); - model.change( writer => { - writer.removeAttribute( 'toStyle', modelRoot.getChild( 0 ) ); - } ); + expectResult( '
' ); + expect( spy.called ).to.be.true; + } ); - expectResult( '' ); - } ); + it( 'should convert on one attribute add and other remove', () => { + setModelData( model, '' ); - it( 'should convert on one attribute add and other remove', () => { - setModelData( model, '' ); + const spy = sinon.spy(); - model.change( writer => { - writer.removeAttribute( 'toStyle', modelRoot.getChild( 0 ) ); - writer.setAttribute( 'toClass', true, modelRoot.getChild( 0 ) ); - } ); + controller.downcastDispatcher.on( 'insert:complex', ( evt, data ) => { + expect( data ).to.have.property( 'reconversion' ).to.be.true; + spy(); + } ); - expectResult( '' ); + model.change( writer => { + writer.removeAttribute( 'toStyle', modelRoot.getChild( 0 ) ); + writer.setAttribute( 'toClass', true, modelRoot.getChild( 0 ) ); } ); - it( 'should do nothing if non-triggerBy attribute has changed', () => { - setModelData( model, '' ); + expectResult( '
' ); + expect( spy.called ).to.be.true; + } ); - model.change( writer => { - writer.setAttribute( 'notTriggered', true, modelRoot.getChild( 0 ) ); - } ); + it( 'should convert on adding a child (at the beginning)', () => { + setModelData( model, 'foo' ); - expectResult( '' ); - } ); + const spy = sinon.spy(); - describe( 'memoization', () => { - it( 'should create new element on re-converting element', () => { - setModelData( model, '' ); + controller.downcastDispatcher.on( 'insert:complex', ( evt, data ) => { + expect( data ).to.have.property( 'reconversion' ).to.be.true; + spy(); + } ); - const [ outerBefore, innerBefore ] = getNodes(); + const [ outerBefore, viewBefore, paraBefore, textBefore ] = getNodes(); - model.change( writer => { - writer.setAttribute( 'toStyle', 'display:block', modelRoot.getChild( 0 ) ); - } ); + model.change( writer => { + const paragraph = writer.createElement( 'paragraph' ); + const text = writer.createText( 'bar' ); - const [ outerAfter, innerAfter ] = getNodes(); + writer.insert( paragraph, modelRoot.getChild( 0 ), 0 ); + writer.insert( text, paragraph, 0 ); + } ); - expect( outerAfter, 'outer' ).to.not.equal( outerBefore ); - expect( innerAfter, 'inner' ).to.not.equal( innerBefore ); - } ); + const [ outerAfter, viewAfter, /* insertedPara */, /* insertedText */, paraAfter, textAfter ] = getNodes(); - // Skipped, as it would require two-level mapping. See https://github.com/ckeditor/ckeditor5/issues/1589. - // Doable as a similar case works in table scenario for table cells (table is refreshed). - it.skip( 'should not re-create child elements on re-converting element', () => { - setModelData( model, 'Foo bar baz' ); + expectResult( '

bar

foo

' ); - expectResult( '

Foo bar baz

' ); - const renderedViewView = viewRoot.getChild( 0 ).getChild( 0 ); + expect( outerAfter, 'outer' ).to.not.equal( outerBefore ); + expect( viewAfter, 'inner' ).to.not.equal( viewBefore ); + expect( paraAfter, 'para' ).to.equal( paraBefore ); + expect( textAfter, 'text' ).to.equal( textBefore ); + expect( spy.called ).to.be.true; + } ); - model.change( writer => { - writer.setAttribute( 'toStyle', 'display:block', modelRoot.getChild( 0 ) ); - } ); + it( 'should convert on adding a child (in the middle)', () => { + setModelData( model, + '' + + 'foo' + + 'bar' + + '' + ); - const viewAfterReRender = viewRoot.getChild( 0 ).getChild( 0 ); + const spy = sinon.spy(); - expect( viewAfterReRender ).to.equal( renderedViewView ); - } ); + controller.downcastDispatcher.on( 'insert:complex', ( evt, data ) => { + expect( data ).to.have.property( 'reconversion' ).to.be.true; + spy(); } ); - } ); - describe( 'with complex view structure (slot conversion)', () => { - beforeEach( () => { - model.schema.register( 'complex', { - allowIn: '$root', - allowAttributes: [ 'classForMain', 'classForWrap', 'attributeToElement' ] - } ); - downcastHelpers.elementToElement( { - model: 'complex', - view: ( modelElement, { writer, mapper } ) => { - const classForMain = !!modelElement.getAttribute( 'classForMain' ); - const classForWrap = !!modelElement.getAttribute( 'classForWrap' ); - const attributeToElement = !!modelElement.getAttribute( 'attributeToElement' ); - - const outer = writer.createContainerElement( 'div', { - class: `complex-slots${ classForMain ? ' with-class' : '' }` - } ); - const inner = writer.createContainerElement( 'div', { - class: `slots${ classForWrap ? ' with-class' : '' }` - } ); - - if ( attributeToElement ) { - const optional = writer.createEmptyElement( 'div', { class: 'optional' } ); - writer.insert( writer.createPositionAt( outer, 0 ), optional ); - } + const [ outerBefore, viewBefore, paraFooBefore, textFooBefore, paraBarBefore, textBarBefore ] = getNodes(); - writer.insert( writer.createPositionAt( outer, 'end' ), inner ); - mapper.bindElements( modelElement, inner ); + model.change( writer => { + const paragraph = writer.createElement( 'paragraph' ); + const text = writer.createText( 'baz' ); - for ( const slot of modelElement.getChildren() ) { - const viewSlot = writer.createContainerElement( 'div', { class: 'slot' } ); + writer.insert( paragraph, modelRoot.getChild( 0 ), 1 ); + writer.insert( text, paragraph, 0 ); + } ); - writer.insert( writer.createPositionAt( inner, slot.index ), viewSlot ); - mapper.bindElements( slot, viewSlot ); - } + const [ outerAfter, viewAfter, + paraFooAfter, textFooAfter, /* insertedPara */, /* insertedText */, paraBarAfter, textBarAfter + ] = getNodes(); - return outer; - }, - triggerBy: { - attributes: [ 'classForMain', 'classForWrap', 'attributeToElement' ], - children: [ 'slot' ] - } - } ); + expectResult( '

foo

baz

bar

' ); - model.schema.register( 'slot', { - allowIn: 'complex' - } ); + expect( outerAfter, 'outer' ).to.not.equal( outerBefore ); + expect( viewAfter, 'inner' ).to.not.equal( viewBefore ); + expect( paraFooAfter, 'para foo' ).to.equal( paraFooBefore ); + expect( textFooAfter, 'text foo' ).to.equal( textFooBefore ); + expect( paraBarAfter, 'para bar' ).to.equal( paraBarBefore ); + expect( textBarAfter, 'text bar' ).to.equal( textBarBefore ); + expect( spy.called ).to.be.true; + } ); - model.schema.register( 'paragraph', { - inheritAllFrom: '$block', - allowIn: 'slot' - } ); - downcastHelpers.elementToElement( { model: 'paragraph', view: 'p' } ); - } ); + it( 'should convert on adding a child (at the end)', () => { + setModelData( model, 'foo' ); - it( 'should convert on insert', () => { - model.change( writer => { - writer.insertElement( 'complex', modelRoot, 0 ); - } ); + const spy = sinon.spy(); - expectResult( '
' ); + controller.downcastDispatcher.on( 'insert:complex', ( evt, data ) => { + expect( data ).to.have.property( 'reconversion' ).to.be.true; + spy(); } ); - it( 'should convert on attribute set (main element)', () => { - setModelData( model, '' ); + const [ outerBefore, viewBefore, paraBefore, textBefore ] = getNodes(); - model.change( writer => { - writer.setAttribute( 'classForMain', true, modelRoot.getChild( 0 ) ); - } ); + model.change( writer => { + const paragraph = writer.createElement( 'paragraph' ); + const text = writer.createText( 'bar' ); - expectResult( '
' ); + writer.insert( paragraph, modelRoot.getChild( 0 ), 1 ); + writer.insert( text, paragraph, 0 ); } ); - it( 'should convert on attribute set (other element)', () => { - setModelData( model, '' ); + const [ outerAfter, viewAfter, paraAfter, textAfter ] = getNodes(); - model.change( writer => { - writer.setAttribute( 'classForWrap', true, modelRoot.getChild( 0 ) ); - } ); + expectResult( '

foo

bar

' ); - expectResult( '
' ); - } ); + expect( outerAfter, 'outer' ).to.not.equal( outerBefore ); + expect( viewAfter, 'inner' ).to.not.equal( viewBefore ); + expect( paraAfter, 'para' ).to.equal( paraBefore ); + expect( textAfter, 'text' ).to.equal( textBefore ); + expect( spy.called ).to.be.true; + } ); - it( 'should convert on attribute set (insert new view element)', () => { - setModelData( model, '' ); + it( 'should convert on removing a child', () => { + setModelData( model, 'foobar' ); - model.change( writer => { - writer.setAttribute( 'attributeToElement', true, modelRoot.getChild( 0 ) ); - } ); + const spy = sinon.spy(); - expectResult( '
' ); + controller.downcastDispatcher.on( 'insert:complex', ( evt, data ) => { + expect( data ).to.have.property( 'reconversion' ).to.be.true; + spy(); } ); - it( 'should convert element with slots', () => { - setModelData( model, - '' + - 'foo' + - 'bar' + - '' ); + const [ outerBefore, viewBefore, paraBefore, textBefore ] = getNodes(); - expectResult( - '
' + - '
' + - '

foo

' + - '

bar

' + - '
' + - '
' - ); + model.change( writer => { + writer.remove( modelRoot.getNodeByPath( [ 0, 1 ] ) ); } ); - it( 'should convert element on adding slot', () => { - setModelData( model, - '' + - 'foo' + - 'bar' + - '' ); + const [ outerAfter, viewAfter, paraAfter, textAfter ] = getNodes(); - model.change( writer => { - insertBazSlot( writer, modelRoot ); - } ); + expectResult( '

foo

' ); - expectResult( - '
' + - '
' + - '

foo

' + - '

bar

' + - '

baz

' + - '
' + - '
' - ); - } ); + expect( outerAfter, 'outer' ).to.not.equal( outerBefore ); + expect( viewAfter, 'inner' ).to.not.equal( viewBefore ); + expect( paraAfter, 'para' ).to.equal( paraBefore ); + expect( textAfter, 'text' ).to.equal( textBefore ); + expect( spy.called ).to.be.true; + } ); - it( 'should convert element on removing slot', () => { - setModelData( model, - '' + - 'foo' + - 'bar' + - '' ); + it( 'should not reconvert if non watched attribute has changed', () => { + setModelData( model, '' ); - model.change( writer => { - writer.remove( modelRoot.getChild( 0 ).getChild( 0 ) ); - } ); + const spy = sinon.spy(); - expectResult( - '
' + - '
' + - '

bar

' + - '
' + - '
' - ); + controller.downcastDispatcher.on( 'insert:complex', () => { + spy(); } ); - it( 'should convert element on multiple triggers (remove + insert)', () => { - setModelData( model, - '' + - 'foo' + - 'bar' + - '' ); - - model.change( writer => { - writer.remove( modelRoot.getChild( 0 ).getChild( 0 ) ); - insertBazSlot( writer, modelRoot ); - } ); + const [ outerDivBefore, innerDivBefore ] = getNodes(); - expectResult( - '
' + - '
' + - '

bar

' + - '

baz

' + - '
' + - '
' - ); + model.change( writer => { + writer.setAttribute( 'notTriggered', true, modelRoot.getChild( 0 ) ); } ); - it( 'should convert element on multiple triggers (remove + attribute)', () => { - setModelData( model, - '' + - 'foo' + - 'bar' + - '' ); + const [ outerDivAfter, innerDivAfter ] = getNodes(); - model.change( writer => { - writer.remove( modelRoot.getChild( 0 ).getChild( 0 ) ); - writer.setAttribute( 'classForMain', true, modelRoot.getChild( 0 ) ); - } ); + expectResult( '
' ); - expectResult( - '
' + - '
' + - '

bar

' + - '
' + - '
' - ); + expect( outerDivAfter, 'outer div' ).to.equal( outerDivBefore ); + expect( innerDivAfter, 'inner div' ).to.equal( innerDivBefore ); + expect( spy.notCalled ).to.be.true; + } ); + } ); + + describe( 'with complex view structure - multiple slots', () => { + beforeEach( () => { + model.schema.register( 'complex', { + allowIn: '$root' } ); - it( 'should convert element on multiple triggers (insert + attribute)', () => { - setModelData( model, - '' + - 'foo' + - 'bar' + - '' ); + downcastHelpers.elementToStructure( { + model: { + name: 'complex', + children: true + }, + view: ( modelElement, { writer, slotFor } ) => { + const outer = writer.createContainerElement( 'div', { class: 'complex-outer' } ); + const inner1 = writer.createContainerElement( 'div', { class: 'inner-first' } ); + const inner2 = writer.createContainerElement( 'div', { class: 'inner-second' } ); - model.change( writer => { - insertBazSlot( writer, modelRoot ); - writer.setAttribute( 'classForMain', true, modelRoot.getChild( 0 ) ); - } ); + writer.insert( writer.createPositionAt( outer, 'end' ), inner1 ); + writer.insert( writer.createPositionAt( outer, 'end' ), inner2 ); - expectResult( - '
' + - '
' + - '

foo

' + - '

bar

' + - '

baz

' + - '
' + - '
' - ); + writer.insert( writer.createPositionAt( inner1, 0 ), slotFor( element => element.index < 2 ) ); + writer.insert( writer.createPositionAt( inner2, 0 ), slotFor( element => element.index >= 2 ) ); + + return outer; + } } ); - it( 'should not trigger refresh on adding a slot to an element without triggerBy conversion', () => { - model.schema.register( 'other', { - allowIn: '$root' - } ); - model.schema.extend( 'slot', { - allowIn: 'other' - } ); - downcastHelpers.elementToElement( { - model: 'other', - view: { - name: 'div', - classes: 'other' - } - } ); - downcastHelpers.elementToElement( { - model: 'slot', - view: { - name: 'div', - classes: 'slot' - } - } ); + model.schema.register( 'paragraph', { + inheritAllFrom: '$block', + allowIn: 'complex' + } ); - setModelData( model, - '' + - 'foo' + - 'bar' + - '' - ); - const otherView = viewRoot.getChild( 0 ); + downcastHelpers.elementToElement( { + model: 'paragraph', + view: 'p' + } ); + } ); - model.change( writer => { - insertBazSlot( writer, modelRoot ); - } ); + it( 'should convert on insert', () => { + const spy = sinon.spy(); - expectResult( - '
' + - '

foo

' + - '

bar

' + - '

baz

' + - '
' - ); - const otherViewAfter = viewRoot.getChild( 0 ); + controller.downcastDispatcher.on( 'insert:complex', ( evt, data ) => { + expect( data ).to.not.have.property( 'reconversion' ); + spy(); + } ); - expect( otherView, 'the view should not be refreshed' ).to.equal( otherViewAfter ); + model.change( writer => { + writer.insertElement( 'complex', modelRoot, 0 ); } ); - describe( 'memoization', () => { - it( 'should create new element on re-converting element', () => { - setModelData( model, '' + - 'foo' + - 'bar' + - '' - ); + expectResult( '
' ); + expect( spy.called ).to.be.true; + } ); - const [ complexView ] = getNodes(); + it( 'should convert on adding a child (at the beginning)', () => { + setModelData( model, 'foo' ); - model.change( writer => { - writer.setAttribute( 'classForMain', true, modelRoot.getChild( 0 ) ); - } ); + const spy = sinon.spy(); - const [ viewAfterReRender ] = getNodes(); + controller.downcastDispatcher.on( 'insert:complex', ( evt, data ) => { + expect( data ).to.have.property( 'reconversion' ).to.be.true; + spy(); + } ); - expect( viewAfterReRender, 'the view should be refreshed' ).to.not.equal( complexView ); - } ); + const [ outerBefore, firstBefore, paraBefore, textBefore, secondBefore ] = getNodes(); - it( 'should not re-create slot\'s child elements on re-converting main element (attribute changed)', () => { - setModelData( model, '' + - 'foo' + - 'bar' + - '' - ); - - const [ main, /* unused */, - slotOne, paraOne, textNodeOne, - slotTwo, paraTwo, textNodeTwo ] = getNodes(); - - model.change( writer => { - writer.setAttribute( 'classForMain', true, modelRoot.getChild( 0 ) ); - } ); - - const [ mainAfter, /* unused */, - slotOneAfter, paraOneAfter, textNodeOneAfter, - slotTwoAfter, paraTwoAfter, textNodeTwoAfter ] = getNodes(); - - expect( mainAfter, 'main view' ).to.not.equal( main ); - expect( slotOneAfter, 'first slot view' ).to.not.equal( slotOne ); - expect( slotTwoAfter, 'second slot view' ).to.not.equal( slotTwo ); - expect( paraOneAfter, 'first slot paragraph view' ).to.equal( paraOne ); - expect( textNodeOneAfter, 'first slot text node view' ).to.equal( textNodeOne ); - expect( paraTwoAfter, 'second slot paragraph view' ).to.equal( paraTwo ); - expect( textNodeTwoAfter, 'second slot text node view' ).to.equal( textNodeTwo ); - } ); + model.change( writer => { + const paragraph = writer.createElement( 'paragraph' ); + const text = writer.createText( 'bar' ); - it( 'should not re-create slot\'s child elements on re-converting main element (slot added)', () => { - setModelData( model, '' + - 'foo' + - 'bar' + - '' - ); - - const [ main, /* unused */, - slotOne, paraOne, textNodeOne, - slotTwo, paraTwo, textNodeTwo ] = getNodes(); - - model.change( writer => { - const slot = writer.createElement( 'slot' ); - const paragraph = writer.createElement( 'paragraph' ); - writer.insertText( 'baz', paragraph, 0 ); - writer.insert( paragraph, slot, 0 ); - writer.insert( slot, modelRoot.getChild( 0 ), 'end' ); - } ); - - const [ mainAfter, /* unused */, - slotOneAfter, paraOneAfter, textNodeOneAfter, - slotTwoAfter, paraTwoAfter, textNodeTwoAfter, - slotThreeAfter, paraThreeAfter, textNodeThreeAfter - ] = getNodes(); - - expect( mainAfter, 'main view' ).to.not.equal( main ); - expect( slotOneAfter, 'first slot view' ).to.not.equal( slotOne ); - expect( slotTwoAfter, 'second slot view' ).to.not.equal( slotTwo ); - expect( paraOneAfter, 'first slot paragraph view' ).to.equal( paraOne ); - expect( textNodeOneAfter, 'first slot text node view' ).to.equal( textNodeOne ); - expect( paraTwoAfter, 'second slot paragraph view' ).to.equal( paraTwo ); - expect( textNodeTwoAfter, 'second slot text node view' ).to.equal( textNodeTwo ); - expect( slotThreeAfter, 'third slot view' ).to.not.be.undefined; - expect( paraThreeAfter, 'third slot paragraph view' ).to.not.be.undefined; - expect( textNodeThreeAfter, 'third slot text node view' ).to.not.be.undefined; - } ); + writer.insert( paragraph, modelRoot.getChild( 0 ), 0 ); + writer.insert( text, paragraph, 0 ); } ); - } ); - // Skipped, as it would require two-level mapping. See https://github.com/ckeditor/ckeditor5/issues/1589. - describe.skip( 'with complex view structure (slot conversion atomic converters for some changes)', () => { - beforeEach( () => { - model.schema.register( 'complex', { - allowIn: '$root', - allowAttributes: [ 'classForMain', 'classForWrap', 'attributeToElement' ] - } ); + const [ outerAfter, firstAfter, /* insertedPara */, /* insertedText */, paraAfter, textAfter, secondAfter ] = getNodes(); - function createViewSlot( slot, { writer, mapper } ) { - const viewSlot = writer.createContainerElement( 'div', { class: 'slot' } ); + expectResult( + '
' + + '

bar

foo

' + + '
' + + '
' + ); - mapper.bindElements( slot, viewSlot ); + expect( outerAfter, 'outer' ).to.not.equal( outerBefore ); + expect( firstAfter, 'inner first' ).to.not.equal( firstBefore ); + expect( secondAfter, 'inner second' ).to.not.equal( secondBefore ); + expect( paraAfter, 'para' ).to.equal( paraBefore ); + expect( textAfter, 'text' ).to.equal( textBefore ); + expect( spy.called ).to.be.true; + } ); - return viewSlot; - } + it( 'should convert on adding a child (in the middle)', () => { + setModelData( model, + '' + + 'foo' + + 'bar' + + '' + ); - downcastHelpers.elementToElement( { - model: 'complex', - view: ( modelElement, { writer, mapper, consumable } ) => { - const classForMain = !!modelElement.getAttribute( 'classForMain' ); - const classForWrap = !!modelElement.getAttribute( 'classForWrap' ); - const attributeToElement = !!modelElement.getAttribute( 'attributeToElement' ); - - const outer = writer.createContainerElement( 'div', { - class: `complex-slots${ classForMain ? ' with-class' : '' }` - } ); - const inner = writer.createContainerElement( 'div', { - class: `slots${ classForWrap ? ' with-class' : '' }` - } ); + const spy = sinon.spy(); - if ( attributeToElement ) { - const optional = writer.createEmptyElement( 'div', { class: 'optional' } ); - writer.insert( writer.createPositionAt( outer, 0 ), optional ); - } + controller.downcastDispatcher.on( 'insert:complex', ( evt, data ) => { + expect( data ).to.have.property( 'reconversion' ).to.be.true; + spy(); + } ); - writer.insert( writer.createPositionAt( outer, 'end' ), inner ); - mapper.bindElements( modelElement, outer ); - mapper.bindElements( modelElement, inner ); + const [ + outerBefore, + firstBefore, paraFooBefore, textFooBefore, paraBarBefore, textBarBefore, + secondBefore + ] = getNodes(); - for ( const slot of modelElement.getChildren() ) { - const viewSlot = createViewSlot( slot, { writer, mapper } ); + model.change( writer => { + const paragraph = writer.createElement( 'paragraph' ); + const text = writer.createText( 'baz' ); - writer.insert( writer.createPositionAt( inner, slot.index ), viewSlot ); - consumable.consume( slot, 'insert' ); - } + writer.insert( paragraph, modelRoot.getChild( 0 ), 1 ); + writer.insert( text, paragraph, 0 ); + } ); - return outer; - }, - triggerBy: { - attributes: [ 'classForMain', 'classForWrap', 'attributeToElement' ] - // Contrary to the previous test - do not act on child changes. - // children: [ 'slot' ] - } - } ); - downcastHelpers.elementToElement( { - model: 'slot', - view: createViewSlot - } ); + const [ + outerAfter, + firstAfter, paraFooAfter, textFooAfter, /* insertedPara */, /* insertedText */, + secondAfter, paraBarAfter, textBarAfter + ] = getNodes(); + + expectResult( + '
' + + '

foo

baz

' + + '

bar

' + + '
' + ); - model.schema.register( 'slot', { - allowIn: 'complex' - } ); + expect( outerAfter, 'outer' ).to.not.equal( outerBefore ); + expect( firstAfter, 'inner first' ).to.not.equal( firstBefore ); + expect( secondAfter, 'inner second' ).to.not.equal( secondBefore ); + expect( paraFooAfter, 'para foo' ).to.equal( paraFooBefore ); + expect( textFooAfter, 'text foo' ).to.equal( textFooBefore ); + expect( paraBarAfter, 'para bar' ).to.equal( paraBarBefore ); + expect( textBarAfter, 'text bar' ).to.equal( textBarBefore ); + expect( spy.called ).to.be.true; + } ); + + it( 'should convert on adding a child (at the end)', () => { + setModelData( model, + '' + + 'foo' + + 'bar' + + '' + ); - model.schema.register( 'paragraph', { - inheritAllFrom: '$block', - allowIn: 'slot' - } ); - downcastHelpers.elementToElement( { model: 'paragraph', view: 'p' } ); + const spy = sinon.spy(); + + controller.downcastDispatcher.on( 'insert:complex', ( evt, data ) => { + expect( data ).to.have.property( 'reconversion' ).to.be.true; + spy(); } ); - it( 'should convert on insert', () => { - model.change( writer => { - writer.insertElement( 'complex', modelRoot, 0 ); - } ); + const [ + outerBefore, + firstBefore, paraFooBefore, textFooBefore, paraBarBefore, textBarBefore, + secondBefore + ] = getNodes(); + + model.change( writer => { + const paragraph = writer.createElement( 'paragraph' ); + const text = writer.createText( 'baz' ); - expectResult( '
' ); + writer.insert( paragraph, modelRoot.getChild( 0 ), 'end' ); + writer.insert( text, paragraph, 0 ); } ); - it( 'should convert on attribute set (main element)', () => { - setModelData( model, '' ); + const [ + outerAfter, + firstAfter, paraFooAfter, textFooAfter, paraBarAfter, textBarAfter, + secondAfter, /* insertedPara */, /* insertedText */ + ] = getNodes(); + + expectResult( + '
' + + '

foo

bar

' + + '

baz

' + + '
' + ); + + expect( outerAfter, 'outer' ).to.not.equal( outerBefore ); + expect( firstAfter, 'inner first' ).to.not.equal( firstBefore ); + expect( secondAfter, 'inner second' ).to.not.equal( secondBefore ); + expect( paraFooAfter, 'para foo' ).to.equal( paraFooBefore ); + expect( textFooAfter, 'text foo' ).to.equal( textFooBefore ); + expect( paraBarAfter, 'para bar' ).to.equal( paraBarBefore ); + expect( textBarAfter, 'text bar' ).to.equal( textBarBefore ); + expect( spy.called ).to.be.true; + } ); + + it( 'should convert on removing a child', () => { + setModelData( model, + '' + + 'foo' + + 'bar' + + 'baz' + + 'abc' + + '' + ); - model.change( writer => { - writer.setAttribute( 'classForMain', true, modelRoot.getChild( 0 ) ); - } ); + const spy = sinon.spy(); - expectResult( '
' ); + controller.downcastDispatcher.on( 'insert:complex', ( evt, data ) => { + expect( data ).to.have.property( 'reconversion' ).to.be.true; + spy(); } ); - it( 'should convert on attribute set (other element)', () => { - setModelData( model, '' ); - - model.change( writer => { - writer.setAttribute( 'classForWrap', true, modelRoot.getChild( 0 ) ); - } ); + const [ + outerBefore, + firstBefore, paraFooBefore, textFooBefore, /* removedPara */, /* removedText */, + secondBefore, paraBazBefore, textBazBefore, paraAbcBefore, textAbcBefore + ] = getNodes(); - expectResult( '
' ); + model.change( writer => { + writer.remove( modelRoot.getNodeByPath( [ 0, 1 ] ) ); } ); - it( 'should convert on attribute set (insert new view element)', () => { - setModelData( model, '' ); + const [ + outerAfter, + firstAfter, paraFooAfter, textFooAfter, paraBazAfter, textBazAfter, + secondAfter, paraAbcAfter, textAbcAfter + ] = getNodes(); + + expectResult( + '
' + + '

foo

baz

' + + '

abc

' + + '
' + ); - model.change( writer => { - writer.setAttribute( 'attributeToElement', true, modelRoot.getChild( 0 ) ); - } ); + expect( outerAfter, 'outer' ).to.not.equal( outerBefore ); + expect( firstAfter, 'inner first' ).to.not.equal( firstBefore ); + expect( secondAfter, 'inner second' ).to.not.equal( secondBefore ); + expect( paraFooAfter, 'para foo' ).to.equal( paraFooBefore ); + expect( textFooAfter, 'text foo' ).to.equal( textFooBefore ); + expect( paraBazAfter, 'para baz' ).to.equal( paraBazBefore ); + expect( textBazAfter, 'text baz' ).to.equal( textBazBefore ); + expect( paraAbcAfter, 'para abc' ).to.equal( paraAbcBefore ); + expect( textAbcAfter, 'text abc' ).to.equal( textAbcBefore ); + expect( spy.called ).to.be.true; + } ); + } ); - expectResult( '
' ); - } ); + it( 'should throw an exception if invalid slot mode or filter was provided', () => { + model.schema.register( 'simple', { + allowIn: '$root' + } ); - it( 'should convert element with slots', () => { - setModelData( model, - '' + - 'foo' + - 'bar' + - '' ); + downcastHelpers.elementToStructure( { + model: { + name: 'simple', + children: true + }, + view: ( modelElement, { writer, slotFor } ) => { + const element = writer.createContainerElement( 'div' ); - expectResult( - '
' + - '
' + - '

foo

' + - '

bar

' + - '
' + - '
' - ); - } ); + writer.insert( writer.createPositionAt( element, 0 ), slotFor( 'foo' ) ); - it( 'should not convert element on adding slot', () => { - setModelData( model, - '' + - 'foo' + - 'bar' + - '' ); + return element; + } + } ); - model.change( writer => { - const slot = writer.createElement( 'slot' ); - const paragraph = writer.createElement( 'paragraph' ); - writer.insertText( 'baz', paragraph, 0 ); - writer.insert( paragraph, slot, 0 ); - writer.insert( slot, modelRoot.getChild( 0 ), 'end' ); - } ); + model.schema.register( 'paragraph', { + inheritAllFrom: '$block', + allowIn: 'simple' + } ); - expectResult( - '
' + - '
' + - '

foo

' + - '

bar

' + - '

baz

' + - '
' + - '
' - ); + downcastHelpers.elementToElement( { + model: 'paragraph', + view: 'p' + } ); + + expectToThrowCKEditorError( () => { + model.change( writer => { + const simple = writer.createElement( 'simple' ); + const paragraph = writer.createElement( 'paragraph' ); + + writer.insert( paragraph, simple, 0 ); + writer.insert( simple, modelRoot, 0 ); } ); + }, /^conversion-slot-mode-unknown/, controller.downcastDispatcher, { modeOrFilter: 'foo' } ); + } ); - it( 'should not convert element on removing slot', () => { - setModelData( model, - '' + - 'foo' + - 'bar' + - '' ); + it( 'should throw an exception if slot filter results overlap', () => { + model.schema.register( 'complex', { + allowIn: '$root' + } ); - model.change( writer => { - writer.remove( modelRoot.getChild( 0 ).getChild( 0 ) ); - } ); + downcastHelpers.elementToStructure( { + model: { + name: 'complex', + children: true + }, + view: ( modelElement, { writer, slotFor } ) => { + const outer = writer.createContainerElement( 'div' ); + const inner1 = writer.createContainerElement( 'div', { class: 'inner-first' } ); + const inner2 = writer.createContainerElement( 'div', { class: 'inner-second' } ); - expectResult( - '
' + - '
' + - '

bar

' + - '
' + - '
' - ); - } ); + writer.insert( writer.createPositionAt( outer, 'end' ), inner1 ); + writer.insert( writer.createPositionAt( outer, 'end' ), inner2 ); - it( 'should convert element on a trigger and block atomic converters (remove + attribute)', () => { - setModelData( model, - '' + - 'foo' + - 'bar' + - '' ); + writer.insert( writer.createPositionAt( inner1, 0 ), slotFor( element => element.index <= 1 ) ); + writer.insert( writer.createPositionAt( inner2, 0 ), slotFor( element => element.index >= 1 ) ); - model.change( writer => { - writer.remove( modelRoot.getChild( 0 ).getChild( 0 ) ); - writer.setAttribute( 'classForMain', true, modelRoot.getChild( 0 ) ); - } ); + return outer; + } + } ); - expectResult( - '
' + - '
' + - '

bar

' + - '
' + - '
' - ); - } ); + model.schema.register( 'paragraph', { + inheritAllFrom: '$block', + allowIn: 'complex' + } ); - it( 'should convert element on a trigger and block atomic converters (insert + attribute)', () => { - setModelData( model, - '' + - 'foo' + - 'bar' + - '' ); + downcastHelpers.elementToElement( { + model: 'paragraph', + view: 'p' + } ); - model.change( writer => { - writer.insert( modelRoot.getChild( 0 ).getChild( 0 ) ); - writer.setAttribute( 'classForMain', true, modelRoot.getChild( 0 ) ); - } ); + expectToThrowCKEditorError( () => { + model.change( writer => { + const complex = writer.createElement( 'complex' ); - expectResult( - '
' + - '
' + - '

foo

' + - '

bar

' + - '

baz

' + - '
' + - '
' - ); + writer.insertElement( 'paragraph', complex, 0 ); + writer.insertElement( 'paragraph', complex, 0 ); + writer.insertElement( 'paragraph', complex, 0 ); + writer.insert( complex, modelRoot, 0 ); } ); - } ); + }, /^conversion-slot-filter-overlap/, controller.downcastDispatcher ); + } ); - function getViewAttributes( modelElement ) { - const toStyle = modelElement.hasAttribute( 'toStyle' ) && { style: modelElement.getAttribute( 'toStyle' ) }; - const toClass = modelElement.hasAttribute( 'toClass' ) && { class: 'is-classy' }; + it( 'should throw an exception if slot filter not include all children', () => { + model.schema.register( 'complex', { + allowIn: '$root' + } ); - return { - ...toStyle, - ...toClass - }; - } + downcastHelpers.elementToStructure( { + model: { + name: 'complex', + children: true + }, + view: ( modelElement, { writer, slotFor } ) => { + const outer = writer.createContainerElement( 'div' ); + const inner1 = writer.createContainerElement( 'div', { class: 'inner-first' } ); + const inner2 = writer.createContainerElement( 'div', { class: 'inner-second' } ); - function insertBazSlot( writer, modelRoot ) { - const slot = writer.createElement( 'slot' ); - const paragraph = writer.createElement( 'paragraph' ); - writer.insertText( 'baz', paragraph, 0 ); - writer.insert( paragraph, slot, 0 ); - writer.insert( slot, modelRoot.getChild( 0 ), 'end' ); - } + writer.insert( writer.createPositionAt( outer, 'end' ), inner1 ); + writer.insert( writer.createPositionAt( outer, 'end' ), inner2 ); - function* getNodes() { - const main = viewRoot.getChild( 0 ); - yield main; + writer.insert( writer.createPositionAt( inner1, 0 ), slotFor( element => element.index < 1 ) ); + writer.insert( writer.createPositionAt( inner2, 0 ), slotFor( element => element.index > 1 ) ); - for ( const { item } of controller.view.createRangeIn( main ) ) { - if ( item.is( 'textProxy' ) ) { - // TreeWalker always create a new instance of a TextProxy so use referenced textNode. - yield item.textNode; - } else { - yield item; - } + return outer; } - } + } ); + + model.schema.register( 'paragraph', { + inheritAllFrom: '$block', + allowIn: 'complex' + } ); + + downcastHelpers.elementToElement( { + model: 'paragraph', + view: 'p' + } ); + + expectToThrowCKEditorError( () => { + model.change( writer => { + const complex = writer.createElement( 'complex' ); + + writer.insertElement( 'paragraph', complex, 0 ); + writer.insertElement( 'paragraph', complex, 0 ); + writer.insertElement( 'paragraph', complex, 0 ); + writer.insert( complex, modelRoot, 0 ); + } ); + }, /^conversion-slot-filter-incomplete/, controller.downcastDispatcher ); } ); } ); @@ -1848,7 +2080,7 @@ describe( 'DowncastHelpers', () => { writer.insertText( 'Foo', { test: '2' }, modelRoot.getChild( 0 ), 0 ); writer.insertText( 'Bar', { test: '3' }, modelRoot.getChild( 1 ), 0 ); } ); - }, /^conversion-attribute-to-attribute-on-text/ ); + }, /^conversion-attribute-to-attribute-on-text/, controller.downcastDispatcher ); } ); it( 'should convert attribute insert/change/remove on a model node', () => { @@ -3177,6 +3409,30 @@ describe( 'DowncastHelpers', () => { function expectResult( string ) { expect( stringifyView( viewRoot, null, { ignoreRoot: true } ) ).to.equal( string ); } + + function getViewAttributes( modelElement ) { + const toStyle = modelElement.hasAttribute( 'toStyle' ) && { style: modelElement.getAttribute( 'toStyle' ) }; + const toClass = modelElement.hasAttribute( 'toClass' ) && { class: 'is-classy' }; + + return { + ...toStyle, + ...toClass + }; + } + + function* getNodes( childIndex = 0 ) { + const main = viewRoot.getChild( childIndex ); + yield main; + + for ( const { item } of controller.view.createRangeIn( main ) ) { + if ( item.is( 'textProxy' ) ) { + // TreeWalker always create a new instance of a TextProxy so use referenced textNode. + yield item.textNode; + } else { + yield item; + } + } + } } ); describe( 'downcast converters', () => { @@ -3699,8 +3955,9 @@ describe( 'downcast selection converters', () => { // Convert model to view. view.change( writer => { - dispatcher.convertInsert( model.createRangeIn( modelRoot ), writer ); - dispatcher.convertMarkerAdd( marker.name, marker.getRange(), writer ); + const markers = [ [ marker.name, marker.getRange() ] ]; + + dispatcher.convert( model.createRangeIn( modelRoot ), markers, writer ); dispatcher.convertSelection( docSelection, model.markers, writer ); } ); @@ -3725,8 +3982,9 @@ describe( 'downcast selection converters', () => { // Convert model to view. view.change( writer => { - dispatcher.convertInsert( model.createRangeIn( modelRoot ), writer ); - dispatcher.convertMarkerAdd( marker.name, marker.getRange(), writer ); + const markers = [ [ marker.name, marker.getRange() ] ]; + + dispatcher.convert( model.createRangeIn( modelRoot ), markers, writer ); dispatcher.convertSelection( docSelection, model.markers, writer ); } ); @@ -3754,8 +4012,9 @@ describe( 'downcast selection converters', () => { // Convert model to view. view.change( writer => { - dispatcher.convertInsert( model.createRangeIn( modelRoot ), writer ); - dispatcher.convertMarkerAdd( marker.name, marker.getRange(), writer ); + const markers = [ [ marker.name, marker.getRange() ] ]; + + dispatcher.convert( model.createRangeIn( modelRoot ), markers, writer ); dispatcher.convertSelection( docSelection, model.markers, writer ); } ); @@ -3783,8 +4042,9 @@ describe( 'downcast selection converters', () => { // Convert model to view. view.change( writer => { - dispatcher.convertInsert( model.createRangeIn( modelRoot ), writer ); - dispatcher.convertMarkerAdd( marker.name, marker.getRange(), writer ); + const markers = [ [ marker.name, marker.getRange() ] ]; + + dispatcher.convert( model.createRangeIn( modelRoot ), markers, writer ); dispatcher.convertSelection( docSelection, model.markers, writer ); } ); @@ -3829,7 +4089,7 @@ describe( 'downcast selection converters', () => { // Convert model to view. view.change( writer => { - dispatcher.convertInsert( model.createRangeIn( modelRoot ), writer ); + dispatcher.convert( model.createRangeIn( modelRoot ), [], writer ); // Add ui element to view. const uiElement = new ViewUIElement( viewDocument, 'span' ); @@ -3854,7 +4114,7 @@ describe( 'downcast selection converters', () => { // Convert model to view. view.change( writer => { - dispatcher.convertInsert( model.createRangeIn( modelRoot ), writer ); + dispatcher.convert( model.createRangeIn( modelRoot ), [], writer ); // Add ui element to view. const uiElement = new ViewUIElement( viewDocument, 'span' ); @@ -4135,7 +4395,7 @@ describe( 'downcast selection converters', () => { // Convert model to view. view.change( writer => { - dispatcher.convertInsert( model.createRangeIn( modelRoot ), writer ); + dispatcher.convert( model.createRangeIn( modelRoot ), model.markers, writer ); dispatcher.convertSelection( docSelection, model.markers, writer ); } ); diff --git a/packages/ckeditor5-engine/tests/conversion/mapper.js b/packages/ckeditor5-engine/tests/conversion/mapper.js index 24d60314b87..dfa208ed542 100644 --- a/packages/ckeditor5-engine/tests/conversion/mapper.js +++ b/packages/ckeditor5-engine/tests/conversion/mapper.js @@ -17,6 +17,7 @@ import ViewUIElement from '../../src/view/uielement'; import ViewText from '../../src/view/text'; import ViewPosition from '../../src/view/position'; import ViewRange from '../../src/view/range'; +import ViewDocumentFragment from '../../src/view/documentfragment'; import { StylesProcessor } from '../../src/view/stylesmap'; describe( 'Mapper', () => { @@ -145,6 +146,60 @@ describe( 'Mapper', () => { expect( mapper.toModelElement( viewA ) ).to.be.undefined; expect( mapper.toViewElement( modelA ) ).to.equal( viewB ); } ); + + it( 'should allow deferred unbinding', () => { + const viewA = new ViewElement( viewDocument, 'a' ); + const modelA = new ModelElement( 'a' ); + + const mapper = new Mapper(); + mapper.bindElements( modelA, viewA ); + + expect( mapper.toModelElement( viewA ) ).to.equal( modelA ); + expect( mapper.toViewElement( modelA ) ).to.equal( viewA ); + + mapper.unbindViewElement( viewA, { defer: true } ); + + expect( mapper.toModelElement( viewA ) ).to.equal( modelA ); + expect( mapper.toViewElement( modelA ) ).to.equal( viewA ); + + mapper.flushDeferredBindings(); + + expect( mapper.toModelElement( viewA ) ).to.be.undefined; + expect( mapper.toViewElement( modelA ) ).to.be.undefined; + } ); + + it( 'should not unbind if element was reused after deferred unbinding', () => { + const viewA = new ViewElement( viewDocument, 'a' ); + const viewFragmentA = new ViewDocumentFragment( viewDocument, [ viewA ] ); + const viewFragmentB = new ViewDocumentFragment( viewDocument ); + + const modelA = new ModelElement( 'a' ); + + const mapper = new Mapper(); + mapper.bindElements( modelA, viewA ); + + expect( mapper.toModelElement( viewA ) ).to.equal( modelA ); + expect( mapper.toViewElement( modelA ) ).to.equal( viewA ); + expect( viewA.root ).to.equal( viewFragmentA ); + + mapper.unbindViewElement( viewA, { defer: true } ); + + expect( mapper.toModelElement( viewA ) ).to.equal( modelA ); + expect( mapper.toViewElement( modelA ) ).to.equal( viewA ); + expect( viewA.root ).to.equal( viewFragmentA ); + + viewFragmentB._appendChild( viewA ); + + expect( mapper.toModelElement( viewA ) ).to.equal( modelA ); + expect( mapper.toViewElement( modelA ) ).to.equal( viewA ); + expect( viewA.root ).to.equal( viewFragmentB ); + + mapper.flushDeferredBindings(); + + expect( mapper.toModelElement( viewA ) ).to.equal( modelA ); + expect( mapper.toViewElement( modelA ) ).to.equal( viewA ); + expect( viewA.root ).to.equal( viewFragmentB ); + } ); } ); describe( 'Standard mapping', () => { diff --git a/packages/ckeditor5-engine/tests/conversion/modelconsumable.js b/packages/ckeditor5-engine/tests/conversion/modelconsumable.js index 6ec4aeafa60..31974e927cb 100644 --- a/packages/ckeditor5-engine/tests/conversion/modelconsumable.js +++ b/packages/ckeditor5-engine/tests/conversion/modelconsumable.js @@ -52,6 +52,13 @@ describe( 'ModelConsumable', () => { expect( modelConsumable.test( modelElement, 'foo:xxx' ) ).to.be.null; } ); + it( 'should normalize type name for inserts', () => { + modelConsumable.add( modelElement, 'insert:foo' ); + + expect( modelConsumable.test( modelElement, 'insert:foo' ) ).to.be.true; + expect( modelConsumable.test( modelElement, 'insert' ) ).to.be.true; + } ); + it( 'should not normalize type name for markers', () => { modelConsumable.add( modelElement, 'addMarker:foo:bar:baz:abc' ); modelConsumable.add( modelElement, 'removeMarker:foo:bar:baz:abc' ); @@ -114,6 +121,16 @@ describe( 'ModelConsumable', () => { expect( modelConsumable.test( modelElement, 'foo:bar' ) ).to.be.false; } ); + it( 'should normalize type name for inserts', () => { + modelConsumable.add( modelElement, 'insert' ); + const result = modelConsumable.consume( modelElement, 'insert:foo' ); + + expect( result ).to.be.true; + + expect( modelConsumable.test( modelElement, 'insert:foo' ) ).to.be.false; + expect( modelConsumable.test( modelElement, 'insert' ) ).to.be.false; + } ); + it( 'should not normalize type names for markers', () => { modelConsumable.add( modelElement, 'addMarker:foo:bar:baz' ); modelConsumable.add( modelElement, 'removeMarker:foo:bar:baz' ); diff --git a/packages/ckeditor5-engine/tests/manual/slotconversion.html b/packages/ckeditor5-engine/tests/manual/slotconversion.html index 7ecc38a4691..fd504d810fd 100644 --- a/packages/ckeditor5-engine/tests/manual/slotconversion.html +++ b/packages/ckeditor5-engine/tests/manual/slotconversion.html @@ -16,6 +16,12 @@ background: hsl(0, 0%, 60%); } + .box-footer { + border: 1px solid hsl(0, 0%, 80%); + background: hsl(0, 0%, 40%); + color: hsl(0, 0%, 80%); + } + .box-content-field { padding: .5em; background: hsl(0, 0%, 100%); @@ -39,6 +45,11 @@ +
+ + +
+
diff --git a/packages/ckeditor5-engine/tests/manual/slotconversion.js b/packages/ckeditor5-engine/tests/manual/slotconversion.js index 895398f331b..cb40144f5ec 100644 --- a/packages/ckeditor5-engine/tests/manual/slotconversion.js +++ b/packages/ckeditor5-engine/tests/manual/slotconversion.js @@ -3,7 +3,7 @@ * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license */ -/* globals console, window, document */ +/* globals window, document */ import ClassicEditor from '@ckeditor/ckeditor5-editor-classic/src/classiceditor'; import ArticlePluginSet from '@ckeditor/ckeditor5-core/tests/_utils/articlepluginset'; @@ -41,7 +41,7 @@ function mapMeta( editor ) { } function getChildren( editor, viewElement ) { - return [ ...( editor.editing.view.createRangeIn( viewElement ) ) ] + return Array.from( editor.editing.view.createRangeIn( viewElement ) ) .filter( ( { type } ) => type === 'elementStart' ) .map( ( { item } ) => item ); } @@ -73,84 +73,67 @@ function getBoxUpcastConverter( editor ) { for ( const field of fields ) { const boxField = writer.createElement( 'boxField' ); - conversionApi.safeInsert( boxField, writer.createPositionAt( box, field.index ) ); + conversionApi.safeInsert( boxField, writer.createPositionAt( box, 'end' ) ); conversionApi.convertChildren( field, boxField ); } conversionApi.consumable.consume( viewElement, { name: true } ); - elements.map( element => { - conversionApi.consumable.consume( element, { name: true } ); - } ); + elements.forEach( element => conversionApi.consumable.consume( element, { name: true } ) ); conversionApi.updateConversionResult( box, data ); } ); } -function downcastBox( modelElement, conversionApi ) { - const { writer } = conversionApi; +function getBoxDowncastCreator( multiSlot ) { + return ( modelElement, conversionApi ) => { + const { writer, slotFor } = conversionApi; - const viewBox = writer.createContainerElement( 'div', { class: 'box' } ); - conversionApi.mapper.bindElements( modelElement, viewBox ); + const viewBox = writer.createContainerElement( 'div', { class: 'box' } ); + const contentWrap = writer.createContainerElement( 'div', { class: 'box-content' } ); - const contentWrap = writer.createContainerElement( 'div', { class: 'box-content' } ); - writer.insert( writer.createPositionAt( viewBox, 0 ), contentWrap ); + writer.insert( writer.createPositionAt( viewBox, 0 ), contentWrap ); - for ( const [ meta, metaValue ] of Object.entries( modelElement.getAttribute( 'meta' ) ) ) { - if ( meta === 'header' ) { - const header = writer.createRawElement( 'div', { - class: 'box-meta box-meta-header' - }, domElement => { - domElement.innerHTML = `

${ metaValue.title }

`; - } ); + for ( const [ meta, metaValue ] of Object.entries( modelElement.getAttribute( 'meta' ) ) ) { + if ( meta === 'header' ) { + const header = writer.createRawElement( 'div', { + class: 'box-meta box-meta-header' + }, domElement => { + domElement.innerHTML = `

${ metaValue.title }

`; + } ); + + writer.insert( writer.createPositionBefore( contentWrap ), header ); + } - writer.insert( writer.createPositionBefore( contentWrap ), header ); + if ( meta === 'author' ) { + const author = writer.createRawElement( 'div', { + class: 'box-meta box-meta-author' + }, domElement => { + domElement.innerHTML = `${ metaValue.name }`; + } ); + + writer.insert( writer.createPositionAfter( contentWrap ), author ); + } } - if ( meta === 'author' ) { - const author = writer.createRawElement( 'div', { - class: 'box-meta box-meta-author' - }, domElement => { - domElement.innerHTML = `${ metaValue.name }`; + if ( !multiSlot ) { + writer.insert( writer.createPositionAt( contentWrap, 0 ), slotFor( 'children' ) ); + } else { + writer.insert( writer.createPositionAt( contentWrap, 0 ), slotFor( element => element.index < 2 ) ); + + const contentWrap2 = writer.createContainerElement( 'div', { class: 'box-content' } ); + + writer.insert( writer.createPositionAt( viewBox, 'end' ), contentWrap2 ); + writer.insert( writer.createPositionAt( contentWrap2, 0 ), slotFor( element => element.index >= 2 ) ); + + const footer = writer.createRawElement( 'div', { class: 'box-footer' }, domElement => { + domElement.innerHTML = 'Footer'; } ); - writer.insert( writer.createPositionAfter( contentWrap ), author ); + writer.insert( writer.createPositionAfter( contentWrap2 ), footer ); } - } - for ( const field of modelElement.getChildren() ) { - const viewField = writer.createContainerElement( 'div', { class: 'box-content-field' } ); - - writer.insert( writer.createPositionAt( contentWrap, field.index ), viewField ); - conversionApi.mapper.bindElements( field, viewField ); - conversionApi.consumable.consume( field, 'insert' ); - - // Might be simplified to: - // - // writer.defineSlot( field, viewField, field.index ); - // - // but would require a converter: - // - // editor.conversion.for( 'downcast' ).elementToElement( { // .slotToElement()? - // model: 'viewField', - // view: { name: 'div', class: 'box-content-field' } - // } ); - } - - // At this point we're inserting whole "component". Equivalent to (JSX-like notation): - // - // "rendered" view Mapping/source - // - // <-- top-level box - // ... box[meta.header] - // - // ... <-- this is "slot" boxField - // ... many - // ... <-- this is "slot" boxField - // - // ... box[meta.author] - // - - return viewBox; + return viewBox; + }; } function addButton( editor, uiName, label, callback ) { @@ -188,7 +171,7 @@ function Box( editor ) { allowIn: '$root', isObject: true, isSelectable: true, - allowAttributes: [ 'infoBoxMeta' ] + allowAttributes: [ 'meta' ] } ); editor.model.schema.register( 'boxField', { @@ -199,13 +182,18 @@ function Box( editor ) { editor.conversion.for( 'upcast' ).add( getBoxUpcastConverter( editor ) ); - editor.conversion.for( 'downcast' ).elementToElement( { - model: 'box', - view: downcastBox, - triggerBy: { + editor.conversion.for( 'downcast' ).elementToStructure( { + model: { + name: 'box', attributes: [ 'meta' ], - children: [ 'boxField' ] - } + children: true + }, + view: getBoxDowncastCreator( editor.config.get( 'box.multiSlot' ) ) + } ); + + editor.conversion.for( 'downcast' ).elementToElement( { + model: 'boxField', + view: { name: 'div', classes: 'box-content-field' } } ); addBoxMetaButton( editor, 'boxTitle', 'Box title', () => ( { @@ -247,8 +235,8 @@ function AddRenderCount( editor ) { }, { priority: 'lowest' } ) ); } -ClassicEditor - .create( document.querySelector( '#editor' ), { +async function createEditor( multiSlot ) { + const editor = await ClassicEditor.create( document.querySelector( '#editor' ), { plugins: [ ArticlePluginSet, Box, AddRenderCount ], toolbar: [ 'heading', @@ -282,11 +270,23 @@ ClassicEditor 'tableRow', 'mergeTableCells' ] + }, + box: { multiSlot } + } ); + + window.editor = editor; +} + +for ( const option of document.querySelectorAll( 'input[name=box-mode]' ) ) { + option.addEventListener( 'change', async event => { + if ( window.editor ) { + await window.editor.destroy(); } - } ) - .then( editor => { - window.editor = editor; - } ) - .catch( err => { - console.error( err.stack ); + + await createEditor( event.target.value == 'multi' ); } ); + + if ( option.checked ) { + createEditor( option.value == 'multi' ); + } +} diff --git a/packages/ckeditor5-engine/tests/manual/slotconversion.md b/packages/ckeditor5-engine/tests/manual/slotconversion.md index bab4d7202b1..a8ac8f99231 100644 --- a/packages/ckeditor5-engine/tests/manual/slotconversion.md +++ b/packages/ckeditor5-engine/tests/manual/slotconversion.md @@ -1,12 +1,14 @@ # Slot conversion -The editor should be loaded with a "box" element that contains multiple "slots" in which user can edit content. +The editor should be loaded with a "box" element that contains multiple fields in which user can edit content. -An additional converter adds `"data-insert-count"` attribute to view elements to show when it was rendered. It is displayed with a CSS at the top-right corner of rendered element. If a view element was not re-rendered this attribute should not change. *Note*: it only acts on "insert" changes so it can omit attribute-to-element changes or insertions not passed through dispatcher. +An additional converter adds `"data-insert-count"` attribute to view elements to show when it was rendered. It is displayed with a CSS in the top-right corner of rendered element. If a view element was not re-rendered this attribute should not change. *Note*: it only acts on "insert" changes, so it can omit attribute-to-element changes or insertions not passed through dispatcher. Observe which view elements are re-rendered when using UI-buttons: * `Box title` - updates title attribute which triggers re-rendering of a "box". * `Box author` - updates author attribute which triggers re-rendering of a "box". -* `+` - adds "slot" to box" which triggers re-rendering of a "box". -* `-` - removes "slot" from box" which triggers re-rendering of a "box". +* `+` - adds field to box which triggers re-rendering of a "box". +* `-` - removes field from box which triggers re-rendering of a "box". + +There is a switch above the editor to load single slot version of the plugin (where all fields are in a single wrapper), and a multi-slot version (where first 2 fields are in one wrapper and the rest in the other wrapper). diff --git a/packages/ckeditor5-find-and-replace/tests/findcommand.js b/packages/ckeditor5-find-and-replace/tests/findcommand.js index 8aceb7ce7c1..7d426588db8 100644 --- a/packages/ckeditor5-find-and-replace/tests/findcommand.js +++ b/packages/ckeditor5-find-and-replace/tests/findcommand.js @@ -71,7 +71,7 @@ describe( 'FindCommand', () => { const markers = getSimplifiedMarkersFromResults( results ); expect( stringify( model.document.getRoot(), null, markers ) ).to.equal( - 'Foo bar baz. Bam bar bom.' + 'Foo bar baz. Bam bar bom.' ); } ); @@ -335,7 +335,7 @@ describe( 'FindCommand', () => { ); expect( stringify( multiRootModel.document.getRoot( 'second' ), null, [ markerSecond ] ) ).to.equal( - 'Foo bar baz' + 'Foo bar baz' ); } ); @@ -368,9 +368,13 @@ describe( 'FindCommand', () => { * random and unique. */ function getSimplifiedMarkersFromResults( results ) { + let letter = 'X'; + return results.map( item => { // Replace markers id to a predefined value, as originally these are unique random ids. - item.marker.name = 'X'; + item.marker.name = letter; + + letter = String.fromCharCode( letter.charCodeAt( 0 ) + 1 ); return item.marker; } ); diff --git a/packages/ckeditor5-find-and-replace/tests/replacecommand.js b/packages/ckeditor5-find-and-replace/tests/replacecommand.js index 0699b4249f2..fa759a9e64c 100644 --- a/packages/ckeditor5-find-and-replace/tests/replacecommand.js +++ b/packages/ckeditor5-find-and-replace/tests/replacecommand.js @@ -114,7 +114,7 @@ describe( 'ReplaceCommand', () => { } } - expect( getData( editor.model, { convertMarkers: true } ) ).to.equal( + expect( getData( editor.model, { convertMarkers: true, withoutSelection: true } ) ).to.equal( 'bar ' + 'foo' + ' ' + diff --git a/packages/ckeditor5-heading/src/title.js b/packages/ckeditor5-heading/src/title.js index 9f176c7a8d4..e93b926689e 100644 --- a/packages/ckeditor5-heading/src/title.js +++ b/packages/ckeditor5-heading/src/title.js @@ -153,27 +153,24 @@ export default class Title extends Plugin { const rootRange = model.createRangeIn( root ); const viewDocumentFragment = viewWriter.createDocumentFragment(); - data.downcastDispatcher.conversionApi.options = options; - - // Convert the entire root to view. - data.mapper.clearBindings(); - data.mapper.bindElements( root, viewDocumentFragment ); - data.downcastDispatcher.convertInsert( rootRange, viewWriter ); - - // Convert all markers that intersects with body. + // Find all markers that intersects with body. const bodyStartPosition = model.createPositionAfter( root.getChild( 0 ) ); const bodyRange = model.createRange( bodyStartPosition, model.createPositionAt( root, 'end' ) ); + const markers = new Map(); + for ( const marker of model.markers ) { const intersection = bodyRange.getIntersection( marker.getRange() ); if ( intersection ) { - data.downcastDispatcher.convertMarkerAdd( marker.name, intersection, viewWriter ); + markers.set( marker.name, intersection ); } } - // Clean `conversionApi`. - delete data.downcastDispatcher.conversionApi.options; + // Convert the entire root to view. + data.mapper.clearBindings(); + data.mapper.bindElements( root, viewDocumentFragment ); + data.downcastDispatcher.convert( rootRange, markers, viewWriter, options ); // Remove title element from view. viewWriter.remove( viewWriter.createRangeOn( viewDocumentFragment.getChild( 0 ) ) ); diff --git a/packages/ckeditor5-html-embed/src/htmlembedediting.js b/packages/ckeditor5-html-embed/src/htmlembedediting.js index 6c4a421eaec..675a15c91a4 100644 --- a/packages/ckeditor5-html-embed/src/htmlembedediting.js +++ b/packages/ckeditor5-html-embed/src/htmlembedediting.js @@ -118,11 +118,8 @@ export default class HtmlEmbedEditing extends Plugin { } } ); - editor.conversion.for( 'editingDowncast' ).elementToElement( { - triggerBy: { - attributes: [ 'value' ] - }, - model: 'rawHtml', + editor.conversion.for( 'editingDowncast' ).elementToStructure( { + model: { name: 'rawHtml', attributes: [ 'value' ] }, view: ( modelElement, { writer } ) => { let domContentWrapper, state, props; diff --git a/packages/ckeditor5-table/tests/converters/downcast.js b/packages/ckeditor5-table/tests/converters/downcast.js index e3454b5f6fc..db215889e78 100644 --- a/packages/ckeditor5-table/tests/converters/downcast.js +++ b/packages/ckeditor5-table/tests/converters/downcast.js @@ -473,6 +473,8 @@ describe( 'downcast converters', () => { model.change( writer => { const row = writer.createElement( 'tableRow' ); + writer.setAttribute( 'headingRows', 3, table ); + writer.insert( row, table, 1 ); writer.insertElement( 'tableCell', row, 'end' );