diff --git a/packages/ckeditor5-engine/package.json b/packages/ckeditor5-engine/package.json index ec23bb3bf41..b776300dfb6 100644 --- a/packages/ckeditor5-engine/package.json +++ b/packages/ckeditor5-engine/package.json @@ -38,6 +38,7 @@ "@ckeditor/ckeditor5-table": "^23.0.0", "@ckeditor/ckeditor5-theme-lark": "^23.0.0", "@ckeditor/ckeditor5-typing": "^23.0.0", + "@ckeditor/ckeditor5-ui": "^23.0.0", "@ckeditor/ckeditor5-undo": "^23.0.0", "@ckeditor/ckeditor5-widget": "^23.0.0" }, diff --git a/packages/ckeditor5-engine/src/conversion/downcastdispatcher.js b/packages/ckeditor5-engine/src/conversion/downcastdispatcher.js index 12eebfff4d1..4f0b30c127a 100644 --- a/packages/ckeditor5-engine/src/conversion/downcastdispatcher.js +++ b/packages/ckeditor5-engine/src/conversion/downcastdispatcher.js @@ -9,6 +9,8 @@ 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'; @@ -121,6 +123,14 @@ export default class DowncastDispatcher { * @member {module:engine/conversion/downcastdispatcher~DowncastConversionApi} */ this.conversionApi = Object.assign( { dispatcher: this }, conversionApi ); + + /** + * Maps conversion event names that will trigger element reconversion for given element name. + * + * @type {Map} + * @private + */ + this._reconversionEventsMapping = new Map(); } /** @@ -136,14 +146,18 @@ export default class DowncastDispatcher { this.convertMarkerRemove( change.name, change.range, writer ); } + const changes = this._mapChangesWithAutomaticReconversion( differ ); + // Convert changes that happened on model tree. - for ( const entry of differ.getChanges() ) { - if ( entry.type == 'insert' ) { + for ( const entry of changes ) { + if ( entry.type === 'insert' ) { this.convertInsert( Range._createFromPositionAndShift( entry.position, entry.length ), writer ); - } else if ( entry.type == 'remove' ) { + } else if ( entry.type === 'remove' ) { this.convertRemove( entry.position, entry.length, entry.name, writer ); + } else if ( entry.type === 'reconvert' ) { + this.reconvertElement( entry.element, writer ); } else { - // entry.type == 'attribute'. + // Defaults to 'attribute' change. this.convertAttribute( entry.range, entry.attributeKey, entry.attributeOldValue, entry.attributeNewValue, writer ); } } @@ -179,26 +193,8 @@ export default class DowncastDispatcher { this.conversionApi.consumable = this._createInsertConsumable( range ); // Fire a separate insert event for each node and text fragment contained 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 - }; - - this._testAndFire( 'insert', data ); - - // 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. - // If we would not add this part, attributes on inserted nodes would not be converted. - for ( const key of item.getAttributeKeys() ) { - data.attributeKey = key; - data.attributeOldValue = null; - data.attributeNewValue = item.getAttribute( key ); - - this._testAndFire( `attribute:${ key }`, data ); - } + for ( const data of Array.from( range ).map( walkerValueToEventData ) ) { + this._convertInsertWithAttributes( data ); } this._clearConversionApi(); @@ -256,6 +252,71 @@ export default class DowncastDispatcher { this._clearConversionApi(); } + /** + * Starts a reconversion of an element. It will: + * + * * Fire a {@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 a `triggerBy` configuration for + * {@link module:engine/conversion/downcasthelpers~DowncastHelpers#elementToElement `elementToElement()`} conversion helper. + * + * @fires insert + * @fires attribute + * @param {module:engine/model/element~Element} element The element to be reconverted. + * @param {module:engine/view/downcastwriter~DowncastWriter} writer The view writer that should be used to modify the view document. + */ + 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 ); + + // Iterate over children of reconverted element in order to... + for ( const value of Range._createIn( element ) ) { + const { item } = value; + + 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 ) ); + } + } + + this._clearConversionApi(); + } + /** * Starts model selection conversion. * @@ -393,6 +454,25 @@ export default class DowncastDispatcher { this._clearConversionApi(); } + /** + * Maps model element "insert" reconversion for given event names. The event names must be fully specified: + * + * * For "attribute" change event it should include main element name, ie: `'attribute:attributeName:elementName'`. + * * For child nodes change events, those should use child event name as well, ie: + * * For adding a node: `'insert:childElementName'`. + * * For removing a node: `'remove:childElementName'`. + * + * **Note**: This method should not be used directly. A reconversion is defined by `triggerBy` configuration of the `elementToElement()` + * conversion helper. + * + * @protected + * @param {String} modelName Main model element name for which events will trigger reconversion. + * @param {String} eventName Name of an event that would trigger conversion for given model element. + */ + _mapReconversionTriggerEvent( modelName, eventName ) { + this._reconversionEventsMapping.set( eventName, modelName ); + } + /** * Creates {@link module:engine/conversion/modelconsumable~ModelConsumable} with values to consume from given range, * assuming that the range has just been inserted to the model. @@ -474,9 +554,7 @@ export default class DowncastDispatcher { return; } - const name = data.item.name || '$text'; - - this.fire( type + ':' + name, data, this.conversionApi ); + this.fire( getEventName( type, data ), data, this.conversionApi ); } /** @@ -489,6 +567,126 @@ export default class DowncastDispatcher { delete this.conversionApi.consumable; } + /** + * Internal method for converting element insert. It will fire events for the inserted element and events for its attributes. + * + * @private + * @fires insert + * @fires attribute + * @param {Object} data Event data. + */ + _convertInsertWithAttributes( data ) { + this._testAndFire( 'insert', data ); + + // 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. + // If we would not add this part, attributes on inserted nodes would not be converted. + for ( const key of data.item.getAttributeKeys() ) { + data.attributeKey = key; + data.attributeOldValue = null; + data.attributeNewValue = data.item.getAttribute( key ); + + this._testAndFire( `attribute:${ key }`, data ); + } + } + + /** + * Returns differ changes together with added "reconvert" type changes for {@link #reconvertElement}. Those are defined by + * a `triggerBy` configuration for + * {@link module:engine/conversion/downcasthelpers~DowncastHelpers#elementToElement `elementToElement()`} conversion helper. + * + * This method will remove every mapped insert or remove change with a single "reconvert" changes. + * + * For instance: Having a `triggerBy` configuration defined for `` 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 + */ + _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 resulting change should trigger element reconversion. + * + * Those are defined by a `triggerBy` configuration for + * {@link module:engine/conversion/downcasthelpers~DowncastHelpers#elementToElement `elementToElement()`} conversion helper. + * + * @private + * @param {String} eventName Event name to check. + * @param {String} elementName Element name to check. + * @returns {Boolean} + */ + _isReconvertTriggerEvent( eventName, elementName ) { + return this._reconversionEventsMapping.get( eventName ) === elementName; + } + /** * Fired for inserted nodes. * @@ -636,6 +834,33 @@ function shouldMarkerChangeBeConverted( modelPosition, marker, mapper ) { return !hasCustomHandling; } +function getEventName( type, data ) { + const name = data.item.name || '$text'; + + return `${ type }:${ name }`; +} + +function walkerValueToEventData( value ) { + const item = value.item; + const itemRange = Range._createFromPositionAndShift( value.previousPosition, value.length ); + + return { + item, + range: itemRange + }; +} + +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} diff --git a/packages/ckeditor5-engine/src/conversion/downcasthelpers.js b/packages/ckeditor5-engine/src/conversion/downcasthelpers.js index a00e5033253..cc14c3a6a54 100644 --- a/packages/ckeditor5-engine/src/conversion/downcasthelpers.js +++ b/packages/ckeditor5-engine/src/conversion/downcasthelpers.js @@ -59,6 +59,19 @@ export default class DowncastHelpers extends ConversionHelpers { * } * } ); * + * The element-to-element conversion supports a reconversion mechanism. This is helpful in conversion to complex view structures where + * multiple atomic element-to-element and attribute-to-attribute or attribute-to-element could be used. By specifying `triggerBy` + * events you can trigger reconverting model to a full view tree structures at once. + * + * editor.conversion.for( 'downcast' ).elementToElement( { + * model: 'complex', + * view: ( modelElement, conversionApi ) => createComplexViewFromModel( modelElement, conversionApi ), + * triggerBy: { + * attributes: [ 'foo', 'bar' ], + * children: [ 'slot' ] + * } + * } ); + * * See {@link module:engine/conversion/conversion~Conversion#for `conversion.for()`} to learn how to add a converter * to the conversion process. * @@ -71,6 +84,9 @@ export default class DowncastHelpers extends ConversionHelpers { * @param {module:engine/view/elementdefinition~ElementDefinition|Function} config.view A view element definition or a function * that takes the model element and {@link module:engine/conversion/downcastdispatcher~DowncastConversionApi downcast conversion API} * as parameters and returns a view container element. + * @param {Object} [config.triggerBy] Re-conversion triggers. At least one trigger must be defined. + * @param {Array.} config.triggerBy.attributes Name of element's attributes which change will trigger element reconversion. + * @param {Array.} config.triggerBy.children Name of direct children that adding or removing will trigger element reconversion. * @returns {module:engine/conversion/downcasthelpers~DowncastHelpers} */ elementToElement( config ) { @@ -1334,13 +1350,14 @@ function removeHighlight( highlightDescriptor ) { // Model element to view element conversion helper. // -// See {@link ~DowncastHelpers#elementToElement `.elementToElement()` downcast helper} for examples. +// See {@link ~DowncastHelpers#elementToElement `.elementToElement()` downcast helper} for examples and config params description. // // @param {Object} config Conversion configuration. -// @param {String} config.model The name of the model element to convert. -// @param {module:engine/view/elementdefinition~ElementDefinition|Function} config.view A view element definition or a function -// that takes the model element and {@link module:engine/view/downcastwriter~DowncastWriter view downcast writer} -// as parameters and returns a view container element. +// @param {String} config.model +// @param {module:engine/view/elementdefinition~ElementDefinition|Function} config.view +// @param {Object} [config.triggerBy] +// @param {Array.} [config.triggerBy.attributes] +// @param {Array.} [config.triggerBy.children] // @returns {Function} Conversion helper. function downcastElementToElement( config ) { config = cloneDeep( config ); @@ -1349,6 +1366,21 @@ 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 }` ); + } + } + + if ( config.triggerBy.children ) { + for ( const childName of config.triggerBy.children ) { + dispatcher._mapReconversionTriggerEvent( config.model, `insert:${ childName }` ); + dispatcher._mapReconversionTriggerEvent( config.model, `remove:${ childName }` ); + } + } + } }; } diff --git a/packages/ckeditor5-engine/tests/conversion/downcastdispatcher.js b/packages/ckeditor5-engine/tests/conversion/downcastdispatcher.js index 23d98dd4fda..7595a6691fe 100644 --- a/packages/ckeditor5-engine/tests/conversion/downcastdispatcher.js +++ b/packages/ckeditor5-engine/tests/conversion/downcastdispatcher.js @@ -75,6 +75,7 @@ describe( 'DowncastDispatcher', () => { it( 'should call convertAttribute for attribute change', () => { sinon.stub( dispatcher, 'convertAttribute' ); + sinon.stub( dispatcher, '_mapChangesWithAutomaticReconversion' ).callsFake( differ => differ.getChanges() ); const position = model.createPositionFromPath( root, [ 0 ] ); const range = ModelRange._createFromPositionAndShift( position, 1 ); @@ -94,6 +95,7 @@ describe( 'DowncastDispatcher', () => { sinon.stub( dispatcher, 'convertInsert' ); sinon.stub( dispatcher, 'convertRemove' ); sinon.stub( dispatcher, 'convertAttribute' ); + sinon.stub( dispatcher, '_mapChangesWithAutomaticReconversion' ).callsFake( differ => differ.getChanges() ); const position = model.createPositionFromPath( root, [ 0 ] ); const range = ModelRange._createFromPositionAndShift( position, 1 ); diff --git a/packages/ckeditor5-engine/tests/conversion/downcasthelpers.js b/packages/ckeditor5-engine/tests/conversion/downcasthelpers.js index 2eed767080e..9f7d22939ea 100644 --- a/packages/ckeditor5-engine/tests/conversion/downcasthelpers.js +++ b/packages/ckeditor5-engine/tests/conversion/downcasthelpers.js @@ -113,6 +113,1147 @@ 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' ] + } + } ); + } ); + + it( 'should convert on insert', () => { + model.change( writer => { + writer.insertElement( 'simpleBlock', modelRoot, 0 ); + } ); + + expectResult( '
' ); + } ); + + it( 'should convert on attribute set', () => { + setModelData( model, '' ); + + const [ viewBefore ] = getNodes(); + + model.change( writer => { + writer.setAttribute( 'toStyle', 'display:block', modelRoot.getChild( 0 ) ); + } ); + + const [ viewAfter ] = getNodes(); + + expectResult( '
' ); + expect( viewAfter ).to.not.equal( viewBefore ); + } ); + + it( 'should convert on attribute change', () => { + setModelData( model, '' ); + + const [ viewBefore ] = getNodes(); + + model.change( writer => { + writer.setAttribute( 'toStyle', 'display:inline', modelRoot.getChild( 0 ) ); + } ); + + const [ viewAfter ] = getNodes(); + + expectResult( '
' ); + + expect( viewAfter ).to.not.equal( viewBefore ); + } ); + + it( 'should convert on attribute remove', () => { + setModelData( model, '' ); + + model.change( writer => { + writer.removeAttribute( 'toStyle', modelRoot.getChild( 0 ) ); + } ); + + expectResult( '
' ); + } ); + + it( 'should convert on one attribute add and other remove', () => { + setModelData( model, '' ); + + model.change( writer => { + writer.removeAttribute( 'toStyle', modelRoot.getChild( 0 ) ); + writer.setAttribute( 'toClass', true, modelRoot.getChild( 0 ) ); + } ); + + expectResult( '
' ); + } ); + + it( 'should do nothing if non-triggerBy attribute has changed', () => { + setModelData( model, '' ); + + 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' ] + } + } ); + + model.schema.register( 'paragraph', { + inheritAllFrom: '$block', + allowIn: 'simpleBlock' + } ); + downcastHelpers.elementToElement( { model: 'paragraph', view: 'p' } ); + } ); + + it( 'should convert on insert', () => { + 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

' ); + } ); + + it( 'should convert on attribute set', () => { + setModelData( model, 'foo' ); + + const [ viewBefore, paraBefore, textBefore ] = getNodes(); + + model.change( writer => { + writer.setAttribute( 'toStyle', 'display:block', 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 change', () => { + setModelData( model, 'foo' ); + + const [ viewBefore, paraBefore, textBefore ] = getNodes(); + + 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' ); + + model.change( writer => { + writer.removeAttribute( 'toStyle', modelRoot.getChild( 0 ) ); + } ); + + expectResult( '

foo

' ); + } ); + + it( 'should convert on one attribute add and other remove', () => { + setModelData( model, 'foo' ); + + model.change( writer => { + writer.removeAttribute( 'toStyle', modelRoot.getChild( 0 ) ); + writer.setAttribute( 'toClass', true, modelRoot.getChild( 0 ) ); + } ); + + expectResult( '

foo

' ); + } ); + + it( 'should do nothing if non-triggerBy attribute has changed', () => { + setModelData( model, 'foo' ); + + 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 ); + // TODO - is text always re-converted? + 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' ] + } + } ); + + model.schema.register( 'paragraph', { + inheritAllFrom: '$block', + allowIn: 'simpleBlock' + } ); + downcastHelpers.elementToElement( { model: 'paragraph', view: 'p' } ); + } ); + + it( 'should convert on insert', () => { + 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

' ); + } ); + + it( 'should convert on adding a child (at the beginning)', () => { + setModelData( model, 'foo' ); + + const [ viewBefore, paraBefore, textBefore ] = getNodes(); + + model.change( writer => { + const paragraph = writer.createElement( 'paragraph' ); + const text = writer.createText( 'bar' ); + + writer.insert( paragraph, modelRoot.getChild( 0 ), 0 ); + writer.insert( text, paragraph, 0 ); + } ); + + const [ viewAfter, /* insertedPara */, /* insertedText */, paraAfter, textAfter ] = getNodes(); + + expectResult( '

bar

foo

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

foo

baz

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 ); + } ); + + it( 'should convert on adding a child (at the end)', () => { + setModelData( model, 'foo' ); + + 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 ); + } ); + + it( 'should convert on removing a child', () => { + setModelData( model, + 'foobar' ); + + const [ viewBefore, paraBefore, textBefore ] = getNodes(); + + model.change( writer => { + writer.remove( modelRoot.getNodeByPath( [ 0, 1 ] ) ); + } ); + + 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 ); + } ); + } ); + + 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 ) ); + + writer.insert( writer.createPositionAt( outer, 0 ), inner ); + + return outer; + }, + triggerBy: { + attributes: [ 'toStyle', 'toClass' ] + } + } ); + } ); + + it( 'should convert on insert', () => { + model.change( writer => { + writer.insertElement( 'complex', modelRoot, 0 ); + } ); + + expectResult( '
' ); + } ); + + it( 'should convert on attribute set', () => { + setModelData( model, '' ); + + const [ outerDivBefore, innerDivBefore ] = getNodes(); + + model.change( writer => { + writer.setAttribute( 'toStyle', 'display:block', modelRoot.getChild( 0 ) ); + } ); + + const [ outerDivAfter, innerDivAfter ] = getNodes(); + + expectResult( '
' ); + expect( outerDivAfter, 'outer div' ).to.not.equal( outerDivBefore ); + expect( innerDivAfter, 'inner div' ).to.not.equal( innerDivBefore ); + } ); + + it( 'should convert on attribute change', () => { + setModelData( model, '' ); + + model.change( writer => { + writer.setAttribute( 'toStyle', 'display:inline', modelRoot.getChild( 0 ) ); + } ); + + expectResult( '
' ); + } ); + + it( 'should convert on attribute remove', () => { + setModelData( model, '' ); + + model.change( writer => { + writer.removeAttribute( 'toStyle', modelRoot.getChild( 0 ) ); + } ); + + expectResult( '
' ); + } ); + + it( 'should convert on one attribute add and other remove', () => { + setModelData( model, '' ); + + model.change( writer => { + writer.removeAttribute( 'toStyle', modelRoot.getChild( 0 ) ); + writer.setAttribute( 'toClass', true, modelRoot.getChild( 0 ) ); + } ); + + expectResult( '
' ); + } ); + + it( 'should do nothing if non-triggerBy attribute has changed', () => { + setModelData( model, '' ); + + const [ outerDivBefore, innerDivBefore ] = getNodes(); + + model.change( writer => { + writer.setAttribute( 'notTriggered', true, modelRoot.getChild( 0 ) ); + } ); + + const [ outerDivAfter, innerDivAfter ] = getNodes(); + + expectResult( '
' ); + + expect( outerDivAfter, 'outer div' ).to.equal( outerDivBefore ); + expect( innerDivAfter, 'inner div' ).to.equal( innerDivBefore ); + } ); + } ); + + 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 ) ); + + writer.insert( writer.createPositionAt( outer, 0 ), inner ); + mapper.bindElements( modelElement, outer ); // Need for nested mapping + mapper.bindElements( modelElement, inner ); + + return outer; + }, + triggerBy: { + attributes: [ 'toStyle', 'toClass' ] + } + } ); + + model.schema.register( 'paragraph', { + inheritAllFrom: '$block', + allowIn: 'complex' + } ); + downcastHelpers.elementToElement( { model: 'paragraph', view: 'p' } ); + } ); + + it( 'should convert on insert', () => { + model.change( writer => { + writer.insertElement( 'complex', modelRoot, 0 ); + } ); + + expectResult( '' ); + } ); + + it( 'should convert on attribute set', () => { + setModelData( model, '' ); + + model.change( writer => { + writer.setAttribute( 'toStyle', 'display:block', modelRoot.getChild( 0 ) ); + } ); + + expectResult( '' ); + } ); + + it( 'should convert on attribute remove', () => { + setModelData( model, '' ); + + model.change( writer => { + writer.removeAttribute( 'toStyle', modelRoot.getChild( 0 ) ); + } ); + + expectResult( '' ); + } ); + + it( 'should convert on one attribute add and other remove', () => { + setModelData( model, '' ); + + model.change( writer => { + writer.removeAttribute( 'toStyle', modelRoot.getChild( 0 ) ); + writer.setAttribute( 'toClass', true, modelRoot.getChild( 0 ) ); + } ); + + expectResult( '' ); + } ); + + it( 'should do nothing if non-triggerBy attribute has changed', () => { + setModelData( model, '' ); + + model.change( writer => { + writer.setAttribute( 'notTriggered', true, modelRoot.getChild( 0 ) ); + } ); + + expectResult( '' ); + } ); + + describe( 'memoization', () => { + it( 'should create new element on re-converting element', () => { + setModelData( model, '' ); + + const [ outerBefore, innerBefore ] = getNodes(); + + model.change( writer => { + writer.setAttribute( 'toStyle', 'display:block', modelRoot.getChild( 0 ) ); + } ); + + const [ outerAfter, innerAfter ] = getNodes(); + + expect( outerAfter, 'outer' ).to.not.equal( outerBefore ); + expect( innerAfter, 'inner' ).to.not.equal( innerBefore ); + } ); + + // 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( '

Foo bar baz

' ); + const renderedViewView = viewRoot.getChild( 0 ).getChild( 0 ); + + model.change( writer => { + writer.setAttribute( 'toStyle', 'display:block', modelRoot.getChild( 0 ) ); + } ); + + const viewAfterReRender = viewRoot.getChild( 0 ).getChild( 0 ); + + expect( viewAfterReRender ).to.equal( renderedViewView ); + } ); + } ); + } ); + + 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 ); + } + + writer.insert( writer.createPositionAt( outer, 'end' ), inner ); + mapper.bindElements( modelElement, inner ); + + for ( const slot of modelElement.getChildren() ) { + const viewSlot = writer.createContainerElement( 'div', { class: 'slot' } ); + + writer.insert( writer.createPositionAt( inner, slot.index ), viewSlot ); + mapper.bindElements( slot, viewSlot ); + } + + return outer; + }, + triggerBy: { + attributes: [ 'classForMain', 'classForWrap', 'attributeToElement' ], + children: [ 'slot' ] + } + } ); + + model.schema.register( 'slot', { + allowIn: 'complex' + } ); + + model.schema.register( 'paragraph', { + inheritAllFrom: '$block', + allowIn: 'slot' + } ); + downcastHelpers.elementToElement( { model: 'paragraph', view: 'p' } ); + } ); + + it( 'should convert on insert', () => { + model.change( writer => { + writer.insertElement( 'complex', modelRoot, 0 ); + } ); + + expectResult( '
' ); + } ); + + it( 'should convert on attribute set (main element)', () => { + setModelData( model, '' ); + + model.change( writer => { + writer.setAttribute( 'classForMain', true, modelRoot.getChild( 0 ) ); + } ); + + expectResult( '
' ); + } ); + + it( 'should convert on attribute set (other element)', () => { + setModelData( model, '' ); + + model.change( writer => { + writer.setAttribute( 'classForWrap', true, modelRoot.getChild( 0 ) ); + } ); + + expectResult( '
' ); + } ); + + it( 'should convert on attribute set (insert new view element)', () => { + setModelData( model, '' ); + + model.change( writer => { + writer.setAttribute( 'attributeToElement', true, modelRoot.getChild( 0 ) ); + } ); + + expectResult( '
' ); + } ); + + it( 'should convert element with slots', () => { + setModelData( model, + '' + + 'foo' + + 'bar' + + '' ); + + expectResult( + '
' + + '
' + + '

foo

' + + '

bar

' + + '
' + + '
' + ); + } ); + + it( 'should convert element on adding slot', () => { + setModelData( model, + '' + + 'foo' + + 'bar' + + '' ); + + model.change( writer => { + insertBazSlot( writer, modelRoot ); + } ); + + expectResult( + '
' + + '
' + + '

foo

' + + '

bar

' + + '

baz

' + + '
' + + '
' + ); + } ); + + it( 'should convert element on removing slot', () => { + setModelData( model, + '' + + 'foo' + + 'bar' + + '' ); + + model.change( writer => { + writer.remove( modelRoot.getChild( 0 ).getChild( 0 ) ); + } ); + + expectResult( + '
' + + '
' + + '

bar

' + + '
' + + '
' + ); + } ); + + 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 ); + } ); + + expectResult( + '
' + + '
' + + '

bar

' + + '

baz

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

bar

' + + '
' + + '
' + ); + } ); + + it( 'should convert element on multiple triggers (insert + attribute)', () => { + setModelData( model, + '' + + 'foo' + + 'bar' + + '' ); + + model.change( writer => { + insertBazSlot( writer, modelRoot ); + writer.setAttribute( 'classForMain', true, modelRoot.getChild( 0 ) ); + } ); + + expectResult( + '
' + + '
' + + '

foo

' + + '

bar

' + + '

baz

' + + '
' + + '
' + ); + } ); + + 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' + } + } ); + + setModelData( model, + '' + + 'foo' + + 'bar' + + '' + ); + const otherView = viewRoot.getChild( 0 ); + + model.change( writer => { + insertBazSlot( writer, modelRoot ); + } ); + + expectResult( + '
' + + '

foo

' + + '

bar

' + + '

baz

' + + '
' + ); + const otherViewAfter = viewRoot.getChild( 0 ); + + expect( otherView, 'the view should not be refreshed' ).to.equal( otherViewAfter ); + } ); + + describe( 'memoization', () => { + it( 'should create new element on re-converting element', () => { + setModelData( model, '' + + 'foo' + + 'bar' + + '' + ); + + const [ complexView ] = getNodes(); + + model.change( writer => { + writer.setAttribute( 'classForMain', true, modelRoot.getChild( 0 ) ); + } ); + + const [ viewAfterReRender ] = getNodes(); + + expect( viewAfterReRender, 'the view should be refreshed' ).to.not.equal( complexView ); + } ); + + 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 ); + } ); + + 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; + } ); + } ); + } ); + + // 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' ] + } ); + + function createViewSlot( slot, { writer, mapper } ) { + const viewSlot = writer.createContainerElement( 'div', { class: 'slot' } ); + + mapper.bindElements( slot, viewSlot ); + + return viewSlot; + } + + 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' : '' }` + } ); + + if ( attributeToElement ) { + const optional = writer.createEmptyElement( 'div', { class: 'optional' } ); + writer.insert( writer.createPositionAt( outer, 0 ), optional ); + } + + writer.insert( writer.createPositionAt( outer, 'end' ), inner ); + mapper.bindElements( modelElement, outer ); + mapper.bindElements( modelElement, inner ); + + for ( const slot of modelElement.getChildren() ) { + const viewSlot = createViewSlot( slot, { writer, mapper } ); + + writer.insert( writer.createPositionAt( inner, slot.index ), viewSlot ); + consumable.consume( slot, 'insert' ); + } + + 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 + } ); + + model.schema.register( 'slot', { + allowIn: 'complex' + } ); + + model.schema.register( 'paragraph', { + inheritAllFrom: '$block', + allowIn: 'slot' + } ); + downcastHelpers.elementToElement( { model: 'paragraph', view: 'p' } ); + } ); + + it( 'should convert on insert', () => { + model.change( writer => { + writer.insertElement( 'complex', modelRoot, 0 ); + } ); + + expectResult( '
' ); + } ); + + it( 'should convert on attribute set (main element)', () => { + setModelData( model, '' ); + + model.change( writer => { + writer.setAttribute( 'classForMain', true, modelRoot.getChild( 0 ) ); + } ); + + expectResult( '
' ); + } ); + + it( 'should convert on attribute set (other element)', () => { + setModelData( model, '' ); + + model.change( writer => { + writer.setAttribute( 'classForWrap', true, modelRoot.getChild( 0 ) ); + } ); + + expectResult( '
' ); + } ); + + it( 'should convert on attribute set (insert new view element)', () => { + setModelData( model, '' ); + + model.change( writer => { + writer.setAttribute( 'attributeToElement', true, modelRoot.getChild( 0 ) ); + } ); + + expectResult( '
' ); + } ); + + it( 'should convert element with slots', () => { + setModelData( model, + '' + + 'foo' + + 'bar' + + '' ); + + expectResult( + '
' + + '
' + + '

foo

' + + '

bar

' + + '
' + + '
' + ); + } ); + + it( 'should not convert element on adding slot', () => { + setModelData( model, + '' + + 'foo' + + 'bar' + + '' ); + + 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' ); + } ); + + expectResult( + '
' + + '
' + + '

foo

' + + '

bar

' + + '

baz

' + + '
' + + '
' + ); + } ); + + it( 'should not convert element on removing slot', () => { + setModelData( model, + '' + + 'foo' + + 'bar' + + '' ); + + model.change( writer => { + writer.remove( modelRoot.getChild( 0 ).getChild( 0 ) ); + } ); + + expectResult( + '
' + + '
' + + '

bar

' + + '
' + + '
' + ); + } ); + + it( 'should convert element on a trigger and block atomic converters (remove + attribute)', () => { + setModelData( model, + '' + + 'foo' + + 'bar' + + '' ); + + model.change( writer => { + writer.remove( modelRoot.getChild( 0 ).getChild( 0 ) ); + writer.setAttribute( 'classForMain', true, modelRoot.getChild( 0 ) ); + } ); + + expectResult( + '
' + + '
' + + '

bar

' + + '
' + + '
' + ); + } ); + + it( 'should convert element on a trigger and block atomic converters (insert + attribute)', () => { + setModelData( model, + '' + + 'foo' + + 'bar' + + '' ); + + model.change( writer => { + writer.insert( modelRoot.getChild( 0 ).getChild( 0 ) ); + writer.setAttribute( 'classForMain', true, modelRoot.getChild( 0 ) ); + } ); + + expectResult( + '
' + + '
' + + '

foo

' + + '

bar

' + + '

baz

' + + '
' + + '
' + ); + } ); + } ); + + function getViewAttributes( modelElement ) { + const toStyle = modelElement.hasAttribute( 'toStyle' ) && { style: modelElement.getAttribute( 'toStyle' ) }; + const toClass = modelElement.hasAttribute( 'toClass' ) && { class: 'is-classy' }; + + return { + ...toStyle, + ...toClass + }; + } + + 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' ); + } + + function* getNodes() { + const main = viewRoot.getChild( 0 ); + 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( 'attributeToElement()', () => { @@ -1046,10 +2187,10 @@ describe( 'DowncastHelpers', () => { expectResult( '

' + - 'Foo' + - '' + - '' + - 'bar' + + 'Foo' + + '' + + '' + + 'bar' + '

' ); @@ -1237,7 +2378,7 @@ describe( 'DowncastHelpers', () => { expectResult( '

' + - 'Foo' + + 'Foo' + '

' ); @@ -1266,7 +2407,7 @@ describe( 'DowncastHelpers', () => { expectResult( '

' + - 'Foo' + + 'Foo' + '

' ); diff --git a/packages/ckeditor5-engine/tests/manual/slotconversion.html b/packages/ckeditor5-engine/tests/manual/slotconversion.html new file mode 100644 index 00000000000..7ecc38a4691 --- /dev/null +++ b/packages/ckeditor5-engine/tests/manual/slotconversion.html @@ -0,0 +1,65 @@ + + + + +
+
+
+
+
+
+

I'm a title

+
+
+
+

Foo

+
+
    +
  • Bar
  • +
+
+

Baz

+
+
+ @john +
+
+
+
diff --git a/packages/ckeditor5-engine/tests/manual/slotconversion.js b/packages/ckeditor5-engine/tests/manual/slotconversion.js new file mode 100644 index 00000000000..dc52b4cbf94 --- /dev/null +++ b/packages/ckeditor5-engine/tests/manual/slotconversion.js @@ -0,0 +1,292 @@ +/** + * @license Copyright (c) 2003-2020, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license + */ + +/* globals console, window, document */ + +import ClassicEditor from '@ckeditor/ckeditor5-editor-classic/src/classiceditor'; +import ArticlePluginSet from '@ckeditor/ckeditor5-core/tests/_utils/articlepluginset'; +import ButtonView from '@ckeditor/ckeditor5-ui/src/button/buttonview'; + +const byClassName = className => element => element.hasClass( className ); + +const getRandom = () => parseInt( Math.random() * 1000 ); + +function mapMeta( editor ) { + return metaElement => { + if ( metaElement.hasClass( 'box-meta-header' ) ) { + const title = getChildren( editor, metaElement ) + .filter( byClassName( 'box-meta-header-title' ) ) + .pop().getChild( 0 ).getChild( 0 ).data; + + return { + header: { + title + } + }; + } + + if ( metaElement.hasClass( 'box-meta-author' ) ) { + const link = metaElement.getChild( 0 ); + + return { + author: { + name: link.getChild( 0 ).data, + website: link.getAttribute( 'href' ) + } + }; + } + }; +} + +function getChildren( editor, viewElement ) { + return [ ...( editor.editing.view.createRangeIn( viewElement ) ) ] + .filter( ( { type } ) => type === 'elementStart' ) + .map( ( { item } ) => item ); +} + +function getBoxUpcastConverter( editor ) { + return dispatcher => dispatcher.on( 'element:div', ( event, data, conversionApi ) => { + const viewElement = data.viewItem; + const writer = conversionApi.writer; + + if ( !viewElement.hasClass( 'box' ) ) { + return; + } + + const box = writer.createElement( 'box' ); + + if ( !conversionApi.safeInsert( box, data.modelCursor ) ) { + return; + } + + const elements = getChildren( editor, viewElement ); + + const fields = elements.filter( byClassName( 'box-content-field' ) ); + const metaElements = elements.filter( byClassName( 'box-meta' ) ); + + const meta = metaElements.map( mapMeta( editor ) ).reduce( ( prev, current ) => Object.assign( prev, current ), {} ); + + writer.setAttribute( 'meta', meta, box ); + + for ( const field of fields ) { + const boxField = writer.createElement( 'boxField' ); + + conversionApi.safeInsert( boxField, writer.createPositionAt( box, field.index ) ); + conversionApi.convertChildren( field, boxField ); + } + + conversionApi.consumable.consume( viewElement, { name: true } ); + elements.map( element => { + conversionApi.consumable.consume( element, { name: true } ); + } ); + + conversionApi.updateConversionResult( box, data ); + } ); +} + +function downcastBox( modelElement, conversionApi ) { + const { writer } = conversionApi; + + const viewBox = writer.createContainerElement( 'div', { class: 'box' } ); + conversionApi.mapper.bindElements( modelElement, viewBox ); + + const contentWrap = writer.createContainerElement( 'div', { class: 'box-content' } ); + 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 }

`; + } ); + + 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 ); + } + } + + 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; +} + +function addButton( editor, uiName, label, callback ) { + editor.ui.componentFactory.add( uiName, locale => { + const view = new ButtonView( locale ); + + view.set( { label, withText: true } ); + + view.listenTo( view, 'execute', () => { + const parent = editor.model.document.selection.getFirstPosition().parent; + const boxField = parent.findAncestor( 'boxField' ); + + if ( !boxField ) { + return; + } + + editor.model.change( writer => callback( writer, boxField.findAncestor( 'box' ), boxField ) ); + } ); + + return view; + } ); +} + +function addBoxMetaButton( editor, uiName, label, updateWith ) { + addButton( editor, uiName, label, ( writer, box ) => { + writer.setAttribute( 'meta', { + ...box.getAttribute( 'meta' ), + ...updateWith() + }, box ); + } ); +} + +function Box( editor ) { + editor.model.schema.register( 'box', { + allowIn: '$root', + isObject: true, + isSelectable: true, + allowAttributes: [ 'infoBoxMeta' ] + } ); + + editor.model.schema.register( 'boxField', { + allowContentOf: '$root', + allowIn: 'box', + isLimit: true + } ); + + editor.conversion.for( 'upcast' ).add( getBoxUpcastConverter( editor ) ); + + editor.conversion.for( 'downcast' ).elementToElement( { + model: 'box', + view: downcastBox, + triggerBy: { + attributes: [ 'meta' ], + children: [ 'boxField' ] + } + } ); + + addBoxMetaButton( editor, 'boxTitle', 'Box title', () => ( { + header: { title: `Random title no. ${ getRandom() }.` } + } ) ); + + addBoxMetaButton( editor, 'boxAuthor', 'Box author', () => ( { + author: { + website: `www.example.com/${ getRandom() }`, + name: `Random author no. ${ getRandom() }` + } + } ) ); + + addButton( editor, 'addBoxField', '+', ( writer, box, boxField ) => { + const newBoxField = writer.createElement( 'boxField' ); + writer.insert( newBoxField, box, boxField.index ); + writer.insert( writer.createElement( 'paragraph' ), newBoxField, 0 ); + } ); + + addButton( editor, 'removeBoxField', '-', ( writer, box, boxField ) => { + writer.remove( boxField ); + } ); +} + +function AddRenderCount( editor ) { + let insertCount = 0; + + const nextInsert = () => insertCount++; + + editor.conversion.for( 'downcast' ).add( dispatcher => dispatcher.on( 'insert', ( event, data, conversionApi ) => { + const view = conversionApi.mapper.toViewElement( data.item ); + + if ( view ) { + const insertCount = nextInsert(); + + conversionApi.writer.setAttribute( 'data-insert-count', `${ insertCount }`, view ); + conversionApi.writer.setAttribute( 'title', `Insertion counter: ${ insertCount }`, view ); + } + }, { priority: 'lowest' } ) ); +} + +ClassicEditor + .create( document.querySelector( '#editor' ), { + plugins: [ ArticlePluginSet, Box, AddRenderCount ], + toolbar: [ + 'heading', + '|', + 'boxTitle', + 'boxAuthor', + 'addBoxField', + 'removeBoxField', + '|', + 'bold', + 'italic', + 'link', + 'bulletedList', + 'numberedList', + '|', + 'outdent', + 'indent', + '|', + 'blockQuote', + 'insertTable', + 'mediaEmbed', + 'undo', + 'redo' + ], + image: { + toolbar: [ 'imageStyle:full', 'imageStyle:side', '|', 'imageTextAlternative' ] + }, + table: { + contentToolbar: [ + 'tableColumn', + 'tableRow', + 'mergeTableCells' + ] + } + } ) + .then( editor => { + window.editor = editor; + } ) + .catch( err => { + console.error( err.stack ); + } ); diff --git a/packages/ckeditor5-engine/tests/manual/slotconversion.md b/packages/ckeditor5-engine/tests/manual/slotconversion.md new file mode 100644 index 00000000000..bab4d7202b1 --- /dev/null +++ b/packages/ckeditor5-engine/tests/manual/slotconversion.md @@ -0,0 +1,12 @@ +# Slot conversion + +The editor should be loaded with a "box" element that contains multiple "slots" 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. + +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". diff --git a/packages/ckeditor5-engine/tests/model/differ.js b/packages/ckeditor5-engine/tests/model/differ.js index 072041f9c0b..e6121b66bd9 100644 --- a/packages/ckeditor5-engine/tests/model/differ.js +++ b/packages/ckeditor5-engine/tests/model/differ.js @@ -1677,7 +1677,7 @@ describe( 'Differ', () => { } ); describe( 'other cases', () => { - // #1309. + // See https://github.com/ckeditor/ckeditor5/issues/4284. it( 'multiple inserts and removes in one element', () => { model.change( () => { insert( new Text( 'x' ), new Position( root, [ 0, 2 ] ) ); @@ -1691,7 +1691,7 @@ describe( 'Differ', () => { } ); } ); - // ckeditor5#733. + // See https://github.com/ckeditor/ckeditor5/issues/733. it( 'proper filtering of changes in removed elements', () => { // Before fix there was a buggy scenario described in ckeditor5#733. // There was this structure: `foo[

te]xt

` diff --git a/packages/ckeditor5-image/tests/imageupload/imageuploadprogress.js b/packages/ckeditor5-image/tests/imageupload/imageuploadprogress.js index 9f1b0d90966..7e479f869a1 100644 --- a/packages/ckeditor5-image/tests/imageupload/imageuploadprogress.js +++ b/packages/ckeditor5-image/tests/imageupload/imageuploadprogress.js @@ -104,6 +104,7 @@ describe( 'ImageUploadProgress', () => { } ); // See https://github.com/ckeditor/ckeditor5/issues/1985. + // Might be obsolete after changes in table refreshing (now it refreshes siblings of an image and not its parent). it( 'should work if image parent is refreshed by the differ', function( done ) { model.schema.register( 'outerBlock', { allowWhere: '$block', @@ -125,7 +126,7 @@ describe( 'ImageUploadProgress', () => { if ( change.type == 'insert' && change.name == 'image' ) { doc.differ.refreshItem( change.position.parent ); - return true; + return false; // Refreshing item should not trigger calling post-fixer again. } } } ); @@ -137,10 +138,14 @@ describe( 'ImageUploadProgress', () => { model.document.once( 'change', () => { try { expect( getViewData( view ) ).to.equal( - '[
' + - `` + - '
' + - '
]
' + '' + + '' + + '[
' + + `` + + '
' + + '
]' + + '
' + + '
' ); done(); diff --git a/packages/ckeditor5-table/src/converters/downcast.js b/packages/ckeditor5-table/src/converters/downcast.js index 64e6ed07983..12a535452e5 100644 --- a/packages/ckeditor5-table/src/converters/downcast.js +++ b/packages/ckeditor5-table/src/converters/downcast.js @@ -8,7 +8,7 @@ */ import TableWalker from './../tablewalker'; -import { toWidget, toWidgetEditable, setHighlightHandling } from '@ckeditor/ckeditor5-widget/src/utils'; +import { setHighlightHandling, toWidget, toWidgetEditable } from '@ckeditor/ckeditor5-widget/src/utils'; /** * Model table element to view table element conversion helper. @@ -235,6 +235,54 @@ export function downcastRemoveRow() { }, { priority: 'higher' } ); } +/** + * Overrides paragraph inside table cell conversion. + * + * This converter: + * * should be used to override default paragraph conversion in the editing view. + * * It will only convert placed directly inside . + * * For a single paragraph without attributes it returns `` to simulate data table. + * * For all other cases it returns `

` element. + * + * @param {module:engine/model/element~Element} modelElement + * @param {module:engine/conversion/downcastdispatcher~DowncastConversionApi} conversionApi + * @returns {module:engine/view/containerelement~ContainerElement|undefined} + */ +export function convertParagraphInTableCell( modelElement, conversionApi ) { + const { writer } = conversionApi; + + if ( !modelElement.parent.is( 'element', 'tableCell' ) ) { + return; + } + + if ( isSingleParagraphWithoutAttributes( modelElement ) ) { + // Use display:inline-block to force Chrome/Safari to limit text mutations to this element. + // See #6062. + return writer.createContainerElement( 'span', { style: 'display:inline-block' } ); + } else { + return writer.createContainerElement( 'p' ); + } +} + +/** + * Checks if given model `` is an only child of a parent (``) and if it has any attribute set. + * + * The paragraph should be converted in the editing view to: + * + * * If returned `true` - to a `` + * * If returned `false` - to a `

` + * + * @param {module:engine/model/element~Element} modelElement + * @returns {Boolean} + */ +export function isSingleParagraphWithoutAttributes( modelElement ) { + const tableCell = modelElement.parent; + + const isSingleParagraph = tableCell.childCount === 1; + + return isSingleParagraph && !hasAnyAttribute( modelElement ); +} + // Converts a given {@link module:engine/view/element~Element} to a table widget: // * Adds a {@link module:engine/view/element~Element#_setCustomProperty custom property} allowing to recognize the table widget element. // * Calls the {@link module:widget/utils~toWidget} function with the proper element's label creator. @@ -327,27 +375,15 @@ function createViewTableCellElement( tableSlot, tableAttributes, insertPosition, conversionApi.writer.insert( insertPosition, cellElement ); - if ( isSingleParagraph && !hasAnyAttribute( firstChild ) ) { + conversionApi.mapper.bindElements( tableCell, cellElement ); + + // Additional requirement for data pipeline to have backward compatible data tables. + if ( !asWidget && !hasAnyAttribute( firstChild ) && isSingleParagraph ) { const innerParagraph = tableCell.getChild( 0 ); - const paragraphInsertPosition = conversionApi.writer.createPositionAt( cellElement, 'end' ); conversionApi.consumable.consume( innerParagraph, 'insert' ); - if ( asWidget ) { - // Use display:inline-block to force Chrome/Safari to limit text mutations to this element. - // See #6062. - const fakeParagraph = conversionApi.writer.createContainerElement( 'span', { style: 'display:inline-block' } ); - - conversionApi.mapper.bindElements( innerParagraph, fakeParagraph ); - conversionApi.writer.insert( paragraphInsertPosition, fakeParagraph ); - - conversionApi.mapper.bindElements( tableCell, cellElement ); - } else { - conversionApi.mapper.bindElements( tableCell, cellElement ); - conversionApi.mapper.bindElements( innerParagraph, cellElement ); - } - } else { - conversionApi.mapper.bindElements( tableCell, cellElement ); + conversionApi.mapper.bindElements( innerParagraph, cellElement ); } } diff --git a/packages/ckeditor5-table/src/converters/table-cell-refresh-post-fixer.js b/packages/ckeditor5-table/src/converters/table-cell-refresh-post-fixer.js index 10648030a67..ed6085afdeb 100644 --- a/packages/ckeditor5-table/src/converters/table-cell-refresh-post-fixer.js +++ b/packages/ckeditor5-table/src/converters/table-cell-refresh-post-fixer.js @@ -7,6 +7,8 @@ * @module table/converters/table-cell-refresh-post-fixer */ +import { isSingleParagraphWithoutAttributes } from './downcast'; + /** * Injects a table cell post-fixer into the model which marks the table cell in the differ to have it re-rendered. * @@ -17,87 +19,57 @@ * re-rendered so it changes from `` to `

`. The easiest way to do it is to re-render the entire table cell. * * @param {module:engine/model/model~Model} model + * @param {module:engine/conversion/mapper~Mapper} mapper */ -export default function injectTableCellRefreshPostFixer( model ) { - model.document.registerPostFixer( () => tableCellRefreshPostFixer( model ) ); +export default function injectTableCellRefreshPostFixer( model, mapper ) { + model.document.registerPostFixer( () => tableCellRefreshPostFixer( model.document.differ, mapper ) ); } -function tableCellRefreshPostFixer( model ) { - const differ = model.document.differ; - - // Stores cells to be refreshed so the table cell will be refreshed once for multiple changes. - const cellsToRefresh = new Set(); +function tableCellRefreshPostFixer( differ, mapper ) { + // Stores cells to be refreshed, so the table cell will be refreshed once for multiple changes. - // Counting the paragraph inserts to verify if it increased to more than one paragraph in the current differ. - let insertCount = 0; + // 1. Gather all changes inside table cell. + const cellsToCheck = new Set(); for ( const change of differ.getChanges() ) { - const parent = change.type == 'insert' || change.type == 'remove' ? change.position.parent : change.range.start.parent; - - if ( !parent.is( 'element', 'tableCell' ) ) { - continue; - } - - if ( change.type == 'insert' ) { - insertCount++; - } + const parent = change.type == 'attribute' ? change.range.start.parent : change.position.parent; - if ( checkRefresh( parent, change.type, insertCount ) ) { - cellsToRefresh.add( parent ); + if ( parent.is( 'element', 'tableCell' ) ) { + cellsToCheck.add( parent ); } } - if ( cellsToRefresh.size ) { - // @if CK_DEBUG_TABLE // console.log( `Post-fixing table: refreshing cells (${ cellsToRefresh.size }).` ); + // @if CK_DEBUG_TABLE // console.log( `Post-fixing table: Checking table cell to refresh (${ cellsToCheck.size }).` ); + // @if CK_DEBUG_TABLE // let paragraphsRefreshed = 0; - for ( const tableCell of cellsToRefresh.values() ) { - differ.refreshItem( tableCell ); + for ( const tableCell of cellsToCheck.values() ) { + for ( const paragraph of [ ...tableCell.getChildren() ].filter( child => shouldRefresh( child, mapper ) ) ) { + // @if CK_DEBUG_TABLE // console.log( `Post-fixing table: refreshing paragraph in table cell (${++paragraphsRefreshed}).` ); + differ.refreshItem( paragraph ); } - - return true; } + // Always return false to prevent the refresh post-fixer from re-running on the same set of changes and going into an infinite loop. + // This "post-fixer" does not change the model structure so there shouldn't be need to run other post-fixers again. + // See https://github.com/ckeditor/ckeditor5/issues/1936 & https://github.com/ckeditor/ckeditor5/issues/8200. return false; } -// Checks if the model table cell requires refreshing to be re-rendered to a proper state in the view. -// -// This method detects changes that will require renaming `` to `

` (or vice versa) in the view. -// -// This method is a simple heuristic that checks only a single change and will sometimes give a false positive result when multiple changes -// will result in a state that does not require renaming in the view (but will be seen as requiring a refresh). +// Check if given model element needs refreshing. // -// For instance: A `` should be renamed to `

` when adding an attribute to a ``. -// But adding one attribute and removing another one will result in a false positive: the check for an added attribute will see one -// attribute on a paragraph and will falsely qualify such change as adding an attribute to a paragraph without any attribute. -// -// @param {module:engine/model/element~Element} tableCell The table cell to check. -// @param {String} type Type of change. -// @param {Number} insertCount The number of inserts in differ. -function checkRefresh( tableCell, type, insertCount ) { - const hasInnerParagraph = Array.from( tableCell.getChildren() ).some( child => child.is( 'element', 'paragraph' ) ); - - // If there is no paragraph in table cell then the view doesn't require refreshing. - // - // Why? What we really want to achieve is to make all the old paragraphs (which weren't added in this batch) to be - // converted once again, so that the paragraph-in-table-cell converter can correctly create a `

` or a `` element. - // If there are no paragraphs in the table cell, we don't care. - if ( !hasInnerParagraph ) { +// @param {module:engine/model/element~Element} modelElement +// @param {module:engine/conversion/mapper~Mapper} mapper +// @returns {Boolean} +function shouldRefresh( child, mapper ) { + if ( !child.is( 'element', 'paragraph' ) ) { return false; } - // For attribute change we only refresh if there is a single paragraph as in this case we may want to change existing `` to `

`. - if ( type == 'attribute' ) { - const attributesCount = Array.from( tableCell.getChild( 0 ).getAttributeKeys() ).length; + const viewElement = mapper.toViewElement( child ); - return tableCell.childCount === 1 && attributesCount < 2; + if ( !viewElement ) { + return false; } - // For other changes (insert, remove) the `` to `

` change is needed when: - // - // - another element is added to a single paragraph (childCount becomes >= 2) - // - another element is removed and a single paragraph is left (childCount == 1) - // - // Change is not needed if there were multiple blocks before change. - return tableCell.childCount <= ( type == 'insert' ? insertCount + 1 : 1 ); + return isSingleParagraphWithoutAttributes( child ) !== viewElement.is( 'element', 'span' ); } diff --git a/packages/ckeditor5-table/src/converters/table-heading-rows-refresh-post-fixer.js b/packages/ckeditor5-table/src/converters/table-heading-rows-refresh-post-fixer.js index 8094d4e7c5f..ecfd8fb80c6 100644 --- a/packages/ckeditor5-table/src/converters/table-heading-rows-refresh-post-fixer.js +++ b/packages/ckeditor5-table/src/converters/table-heading-rows-refresh-post-fixer.js @@ -44,6 +44,7 @@ function tableHeadingRowsRefreshPostFixer( model ) { // @if CK_DEBUG_TABLE // console.log( `Post-fixing table: refreshing heading rows (${ tablesToRefresh.size }).` ); for ( const table of tablesToRefresh.values() ) { + // Should be handled by a `triggerBy` configuration. See: https://github.com/ckeditor/ckeditor5/issues/8138. differ.refreshItem( table ); } diff --git a/packages/ckeditor5-table/src/tableediting.js b/packages/ckeditor5-table/src/tableediting.js index d2b123076e3..2bd087d2a1c 100644 --- a/packages/ckeditor5-table/src/tableediting.js +++ b/packages/ckeditor5-table/src/tableediting.js @@ -11,6 +11,7 @@ import Plugin from '@ckeditor/ckeditor5-core/src/plugin'; import upcastTable, { skipEmptyTableRow } from './converters/upcasttable'; import { + convertParagraphInTableCell, downcastInsertCell, downcastInsertRow, downcastInsertTable, @@ -109,6 +110,13 @@ export default class TableEditing extends Plugin { conversion.for( 'editingDowncast' ).add( downcastInsertCell() ); + // Duplicates code - needed to properly refresh paragraph inside table cell. + editor.conversion.for( 'editingDowncast' ).elementToElement( { + model: 'paragraph', + view: convertParagraphInTableCell, + converterPriority: 'high' + } ); + // Table attributes conversion. conversion.attributeToAttribute( { model: 'colspan', view: 'colspan' } ); conversion.attributeToAttribute( { model: 'rowspan', view: 'rowspan' } ); @@ -144,7 +152,7 @@ export default class TableEditing extends Plugin { injectTableHeadingRowsRefreshPostFixer( model ); injectTableLayoutPostFixer( model ); - injectTableCellRefreshPostFixer( model ); + injectTableCellRefreshPostFixer( model, editor.editing.mapper ); injectTableCellParagraphPostFixer( model ); } diff --git a/packages/ckeditor5-table/tests/converters/downcast.js b/packages/ckeditor5-table/tests/converters/downcast.js index 0243a76076b..1563ceb0d9a 100644 --- a/packages/ckeditor5-table/tests/converters/downcast.js +++ b/packages/ckeditor5-table/tests/converters/downcast.js @@ -1085,7 +1085,7 @@ describe( 'downcast converters', () => { ); } ); - it( 'should react to removed row form the end of a body rows (no heading rows)', () => { + it( 'should react to removed row from the end of a body rows (no heading rows)', () => { setModelData( model, modelTable( [ [ '00[]', '01' ], [ '10', '11' ] @@ -1149,7 +1149,7 @@ describe( 'downcast converters', () => { ); } ); - it( 'should react to removed row form the end of a heading rows (no body rows)', () => { + it( 'should react to removed row from the end of a heading rows (no body rows)', () => { setModelData( model, modelTable( [ [ '00[]', '01' ], [ '10', '11' ] @@ -1182,7 +1182,7 @@ describe( 'downcast converters', () => { ); } ); - it( 'should react to removed row form the end of a heading rows (first cell in body has colspan)', () => { + it( 'should react to removed row from the end of a heading rows (first cell in body has colspan)', () => { setModelData( model, modelTable( [ [ '00[]', '01', '02', '03' ], [ { rowspan: 2, colspan: 2, contents: '10' }, '12', '13' ], diff --git a/packages/ckeditor5-table/tests/converters/table-cell-paragraph-post-fixer.js b/packages/ckeditor5-table/tests/converters/table-cell-paragraph-post-fixer.js index bf7d83b32e9..840328b392e 100644 --- a/packages/ckeditor5-table/tests/converters/table-cell-paragraph-post-fixer.js +++ b/packages/ckeditor5-table/tests/converters/table-cell-paragraph-post-fixer.js @@ -122,7 +122,7 @@ describe( 'Table cell paragraph post-fixer', () => { ); } ); - it( 'should wrap in paragraph $text nodes placed directly in tableCell (on table cell modification) ', () => { + it( 'should wrap in a paragraph $text nodes placed directly in tableCell (on table cell modification) ', () => { setModelData( model, '' + '' + diff --git a/packages/ckeditor5-table/tests/converters/table-cell-refresh-post-fixer.js b/packages/ckeditor5-table/tests/converters/table-cell-refresh-post-fixer.js index 55d2d719577..471dd0fca49 100644 --- a/packages/ckeditor5-table/tests/converters/table-cell-refresh-post-fixer.js +++ b/packages/ckeditor5-table/tests/converters/table-cell-refresh-post-fixer.js @@ -16,7 +16,7 @@ import TableEditing from '../../src/tableediting'; import { viewTable } from '../_utils/utils'; describe( 'Table cell refresh post-fixer', () => { - let editor, model, doc, root, view, refreshItemSpy, element; + let editor, model, doc, root, view, element; testUtils.createSinonSandbox(); @@ -40,8 +40,6 @@ describe( 'Table cell refresh post-fixer', () => { model.schema.extend( '$block', { allowAttributes: [ 'foo', 'bar' ] } ); editor.conversion.attributeToAttribute( { model: 'foo', view: 'foo' } ); editor.conversion.attributeToAttribute( { model: 'bar', view: 'bar' } ); - - refreshItemSpy = sinon.spy( model.document.differ, 'refreshItem' ); } ); } ); @@ -50,10 +48,15 @@ describe( 'Table cell refresh post-fixer', () => { return editor.destroy(); } ); - it( 'should rename to

when adding element to the same table cell', () => { + function getViewForParagraph( table ) { + return editor.editing.mapper.toViewElement( table.getNodeByPath( [ 0, 0, 0 ] ) ); + } + + it( 'should rename to

when adding element to the same table cell (append)', () => { editor.setData( viewTable( [ [ '

00

' ] ] ) ); const table = root.getChild( 0 ); + const previousView = getViewForParagraph( table ); model.change( writer => { const nodeByPath = table.getNodeByPath( [ 0, 0, 0 ] ); @@ -66,13 +69,33 @@ describe( 'Table cell refresh post-fixer', () => { assertEqualMarkup( getViewData( view, { withoutSelection: true } ), viewTable( [ [ '

00

' ] ], { asWidget: true } ) ); - sinon.assert.calledOnce( refreshItemSpy ); + expect( getViewForParagraph( table ) ).to.not.equal( previousView ); + } ); + + it( 'should rename to

when adding element to the same table cell (prepend)', () => { + editor.setData( viewTable( [ [ '

00

' ] ] ) ); + + const table = root.getChild( 0 ); + const previousView = getViewForParagraph( table ); + + model.change( writer => { + const nodeByPath = table.getNodeByPath( [ 0, 0, 0 ] ); + const paragraph = writer.createElement( 'paragraph' ); + + writer.insert( paragraph, nodeByPath, 'before' ); + } ); + + assertEqualMarkup( getViewData( view, { withoutSelection: true } ), viewTable( [ + [ '

00

' ] + ], { asWidget: true } ) ); + expect( getViewForParagraph( table ) ).to.not.equal( previousView ); } ); it( 'should rename to

when adding more elements to the same table cell', () => { editor.setData( viewTable( [ [ '

00

' ] ] ) ); const table = root.getChild( 0 ); + const previousView = getViewForParagraph( table ); model.change( writer => { const nodeByPath = table.getNodeByPath( [ 0, 0, 0 ] ); @@ -87,13 +110,14 @@ describe( 'Table cell refresh post-fixer', () => { assertEqualMarkup( getViewData( view, { withoutSelection: true } ), viewTable( [ [ '

00

' ] ], { asWidget: true } ) ); - sinon.assert.calledOnce( refreshItemSpy ); + expect( getViewForParagraph( table ) ).to.not.equal( previousView ); } ); it( 'should rename to

on adding other block element to the same table cell', () => { editor.setData( viewTable( [ [ '

00

' ] ] ) ); const table = root.getChild( 0 ); + const previousView = getViewForParagraph( table ); model.change( writer => { const nodeByPath = table.getNodeByPath( [ 0, 0, 0 ] ); @@ -106,13 +130,14 @@ describe( 'Table cell refresh post-fixer', () => { assertEqualMarkup( getViewData( view, { withoutSelection: true } ), viewTable( [ [ '

00

' ] ], { asWidget: true } ) ); - sinon.assert.calledOnce( refreshItemSpy ); + expect( getViewForParagraph( table ) ).to.not.equal( previousView ); } ); it( 'should rename to

on adding multiple other block elements to the same table cell', () => { editor.setData( viewTable( [ [ '

00

' ] ] ) ); const table = root.getChild( 0 ); + const previousView = getViewForParagraph( table ); model.change( writer => { const nodeByPath = table.getNodeByPath( [ 0, 0, 0 ] ); @@ -127,13 +152,34 @@ describe( 'Table cell refresh post-fixer', () => { assertEqualMarkup( getViewData( view, { withoutSelection: true } ), viewTable( [ [ '

00

' ] ], { asWidget: true } ) ); - sinon.assert.calledOnce( refreshItemSpy ); + expect( getViewForParagraph( table ) ).to.not.equal( previousView ); + } ); + + it( 'should not rename to

when adding and removing ', () => { + editor.setData( '

00

' ); + + const table = root.getChild( 0 ); + const previousView = getViewForParagraph( table ); + + model.change( writer => { + const nodeByPath = table.getNodeByPath( [ 0, 0, 0 ] ); + const paragraph = writer.createElement( 'paragraph' ); + + writer.insert( paragraph, nodeByPath, 'after' ); + writer.remove( table.getNodeByPath( [ 0, 0, 1 ] ) ); + } ); + + assertEqualMarkup( getViewData( view, { withoutSelection: true } ), viewTable( [ + [ '00' ] + ], { asWidget: true } ) ); + expect( getViewForParagraph( table ) ).to.equal( previousView ); } ); it( 'should properly rename the same element on consecutive changes', () => { editor.setData( viewTable( [ [ '

00

' ] ] ) ); const table = root.getChild( 0 ); + const previousView = getViewForParagraph( table ); model.change( writer => { const nodeByPath = table.getNodeByPath( [ 0, 0, 0 ] ); @@ -146,7 +192,6 @@ describe( 'Table cell refresh post-fixer', () => { assertEqualMarkup( getViewData( view, { withoutSelection: true } ), viewTable( [ [ '

00

' ] ], { asWidget: true } ) ); - sinon.assert.calledOnce( refreshItemSpy ); model.change( writer => { writer.remove( table.getNodeByPath( [ 0, 0, 1 ] ) ); @@ -155,13 +200,14 @@ describe( 'Table cell refresh post-fixer', () => { assertEqualMarkup( getViewData( view, { withoutSelection: true } ), viewTable( [ [ '00' ] ], { asWidget: true } ) ); - sinon.assert.calledTwice( refreshItemSpy ); + expect( getViewForParagraph( table ) ).to.not.equal( previousView ); } ); it( 'should rename to

when setting attribute on ', () => { editor.setData( '

00

' ); const table = root.getChild( 0 ); + const previousView = getViewForParagraph( table ); model.change( writer => { writer.setAttribute( 'foo', 'bar', table.getNodeByPath( [ 0, 0, 0 ] ) ); @@ -170,13 +216,14 @@ describe( 'Table cell refresh post-fixer', () => { assertEqualMarkup( getViewData( view, { withoutSelection: true } ), viewTable( [ [ '

00

' ] ], { asWidget: true } ) ); - sinon.assert.calledOnce( refreshItemSpy ); + expect( getViewForParagraph( table ) ).to.not.equal( previousView ); } ); it( 'should rename

to when removing one of two paragraphs inside table cell', () => { editor.setData( viewTable( [ [ '

00

foo

' ] ] ) ); const table = root.getChild( 0 ); + const previousView = getViewForParagraph( table ); model.change( writer => { writer.remove( table.getNodeByPath( [ 0, 0, 1 ] ) ); @@ -185,13 +232,14 @@ describe( 'Table cell refresh post-fixer', () => { assertEqualMarkup( getViewData( view, { withoutSelection: true } ), viewTable( [ [ '00' ] ], { asWidget: true } ) ); - sinon.assert.calledOnce( refreshItemSpy ); + expect( getViewForParagraph( table ) ).to.not.equal( previousView ); } ); it( 'should rename

to when removing all but one paragraph inside table cell', () => { editor.setData( viewTable( [ [ '

00

foo

bar

' ] ] ) ); const table = root.getChild( 0 ); + const previousView = getViewForParagraph( table ); model.change( writer => { writer.remove( table.getNodeByPath( [ 0, 0, 1 ] ) ); @@ -201,13 +249,14 @@ describe( 'Table cell refresh post-fixer', () => { assertEqualMarkup( getViewData( view, { withoutSelection: true } ), viewTable( [ [ '00' ] ], { asWidget: true } ) ); - sinon.assert.calledOnce( refreshItemSpy ); + expect( getViewForParagraph( table ) ).to.not.equal( previousView ); } ); it( 'should rename

to when removing attribute from ', () => { editor.setData( '

00

' ); const table = root.getChild( 0 ); + const previousView = getViewForParagraph( table ); model.change( writer => { writer.removeAttribute( 'foo', table.getNodeByPath( [ 0, 0, 0 ] ) ); @@ -216,13 +265,14 @@ describe( 'Table cell refresh post-fixer', () => { assertEqualMarkup( getViewData( view, { withoutSelection: true } ), viewTable( [ [ '00' ] ], { asWidget: true } ) ); - sinon.assert.calledOnce( refreshItemSpy ); + expect( getViewForParagraph( table ) ).to.not.equal( previousView ); } ); it( 'should keep

in the view when attribute value is changed', () => { editor.setData( viewTable( [ [ '

00

' ] ] ) ); const table = root.getChild( 0 ); + const previousView = getViewForParagraph( table ); model.change( writer => { writer.setAttribute( 'foo', 'baz', table.getNodeByPath( [ 0, 0, 0 ] ) ); @@ -231,14 +281,14 @@ describe( 'Table cell refresh post-fixer', () => { assertEqualMarkup( getViewData( view, { withoutSelection: true } ), viewTable( [ [ '

00

' ] ], { asWidget: true } ) ); - // False positive: should not be called. - sinon.assert.calledOnce( refreshItemSpy ); + expect( getViewForParagraph( table ) ).to.equal( previousView ); } ); it( 'should keep

in the view when adding another attribute to a with other attributes', () => { editor.setData( viewTable( [ [ '

00

' ] ] ) ); const table = root.getChild( 0 ); + const previousView = getViewForParagraph( table ); model.change( writer => { writer.setAttribute( 'bar', 'bar', table.getNodeByPath( [ 0, 0, 0 ] ) ); @@ -247,15 +297,14 @@ describe( 'Table cell refresh post-fixer', () => { assertEqualMarkup( getViewData( view, { withoutSelection: true } ), viewTable( [ [ '

00

' ] ], { asWidget: true } ) ); - - // False positive - sinon.assert.notCalled( refreshItemSpy ); + expect( getViewForParagraph( table ) ).to.equal( previousView ); } ); it( 'should keep

in the view when adding another attribute to a and removing attribute that is already set', () => { editor.setData( viewTable( [ [ '

00

' ] ] ) ); const table = root.getChild( 0 ); + const previousView = getViewForParagraph( table ); model.change( writer => { writer.setAttribute( 'bar', 'bar', table.getNodeByPath( [ 0, 0, 0 ] ) ); @@ -265,14 +314,14 @@ describe( 'Table cell refresh post-fixer', () => { assertEqualMarkup( getViewData( view, { withoutSelection: true } ), viewTable( [ [ '

00

' ] ], { asWidget: true } ) ); - // False positive: should not be called. - sinon.assert.calledOnce( refreshItemSpy ); + expect( getViewForParagraph( table ) ).to.equal( previousView ); } ); it( 'should keep

in the view when attribute value is changed (table cell with multiple blocks)', () => { editor.setData( viewTable( [ [ '

00

00

' ] ] ) ); const table = root.getChild( 0 ); + const previousView = getViewForParagraph( table ); model.change( writer => { writer.setAttribute( 'foo', 'baz', table.getNodeByPath( [ 0, 0, 0 ] ) ); @@ -281,13 +330,14 @@ describe( 'Table cell refresh post-fixer', () => { assertEqualMarkup( getViewData( view, { withoutSelection: true } ), viewTable( [ [ '

00

00

' ] ], { asWidget: true } ) ); - sinon.assert.notCalled( refreshItemSpy ); + expect( getViewForParagraph( table ) ).to.equal( previousView ); } ); it( 'should do nothing on rename to other block', () => { editor.setData( viewTable( [ [ '

00

' ] ] ) ); const table = root.getChild( 0 ); + const previousView = getViewForParagraph( table ); model.change( writer => { writer.rename( table.getNodeByPath( [ 0, 0, 0 ] ), 'block' ); @@ -296,13 +346,14 @@ describe( 'Table cell refresh post-fixer', () => { assertEqualMarkup( getViewData( view, { withoutSelection: true } ), viewTable( [ [ '
00
' ] ], { asWidget: true } ) ); - sinon.assert.notCalled( refreshItemSpy ); + expect( getViewForParagraph( table ) ).to.not.equal( previousView ); } ); it( 'should do nothing on adding to existing paragraphs', () => { editor.setData( viewTable( [ [ '

a

b

' ] ] ) ); const table = root.getChild( 0 ); + const previousView = getViewForParagraph( table ); model.change( writer => { writer.insertElement( 'paragraph', table.getNodeByPath( [ 0, 0, 1 ] ), 'after' ); @@ -311,13 +362,14 @@ describe( 'Table cell refresh post-fixer', () => { assertEqualMarkup( getViewData( view, { withoutSelection: true } ), viewTable( [ [ '

a

b

' ] ], { asWidget: true } ) ); - sinon.assert.notCalled( refreshItemSpy ); + expect( getViewForParagraph( table ) ).to.equal( previousView ); } ); it( 'should do nothing when setting attribute on block item other then ', () => { editor.setData( viewTable( [ [ '
foo
' ] ] ) ); const table = root.getChild( 0 ); + const previousView = getViewForParagraph( table ); model.change( writer => { writer.setAttribute( 'foo', 'bar', table.getNodeByPath( [ 0, 0, 0 ] ) ); @@ -326,13 +378,14 @@ describe( 'Table cell refresh post-fixer', () => { assertEqualMarkup( getViewData( view, { withoutSelection: true } ), viewTable( [ [ '
foo
' ] ], { asWidget: true } ) ); - sinon.assert.notCalled( refreshItemSpy ); + expect( getViewForParagraph( table ) ).to.equal( previousView ); } ); it( 'should rename

in to when removing (table cell with 2 paragraphs)', () => { editor.setData( viewTable( [ [ '

00

00

' ] ] ) ); const table = root.getChild( 0 ); + const previousView = getViewForParagraph( table ); model.change( writer => { writer.remove( writer.createRangeOn( table.getNodeByPath( [ 0, 0, 1 ] ) ) ); @@ -341,7 +394,7 @@ describe( 'Table cell refresh post-fixer', () => { assertEqualMarkup( getViewData( view, { withoutSelection: true } ), viewTable( [ [ '00' ] ], { asWidget: true } ) ); - sinon.assert.calledOnce( refreshItemSpy ); + expect( getViewForParagraph( table ) ).to.not.equal( previousView ); } ); it( 'should update view selection after deleting content', () => {