Skip to content

Commit

Permalink
Merge pull request #8099 from ckeditor/i/7956
Browse files Browse the repository at this point in the history 
Feature (engine): Introduce automatic model-to-view reconversion by defining `triggerBy` option for `elementToElement()` conversion helper. Closes #7956.

Other (table): Table cell contents refreshing for the editing view will now make fewer view updates.
  • Loading branch information
@niegowski
niegowski committed Oct 13, 2020
2 parents 1ba5671 + d193903 commit a7c9973
Show file tree
Hide file tree
Showing 17 changed files with 1,998 additions and 153 deletions.
1 change: 1 addition & 0 deletions packages/ckeditor5-engine/package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
},
Expand Down
279 changes: 252 additions & 27 deletions packages/ckeditor5-engine/src/conversion/downcastdispatcher.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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<String, String>}
* @private
*/
this._reconversionEventsMapping = new Map();
}

/**
Expand All @@ -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 );
}
}
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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.
*
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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 );
}

/**
Expand All @@ -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 `<complex>` 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.<Object>} 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.
*
Expand Down Expand Up @@ -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}
Expand Down
Loading

0 comments on commit a7c9973

Please sign in to comment.