Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Declarative reconversion using triggerBy #8099

Merged
merged 119 commits into from
Oct 13, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
119 commits
Select commit Hold shift + click to select a range
0b1e50e
PoC: Create complex re-render with slot conversion example.
jodator Aug 11, 2020
f467fce
PoC: Document box view output in JSX-ish notation.
jodator Aug 12, 2020
7d85e32
PoC: Initial 'refresh' downcast action.
jodator Aug 12, 2020
7be4b06
PoC: Refresh conversion - retain "slot" children.
jodator Aug 12, 2020
9bd831f
PoC: Add render id to the view element itself.
jodator Aug 13, 2020
aa55061
PoC: Implement triggerBy handling.
jodator Aug 13, 2020
120af23
Merge branch 'master' into i/7956
jodator Aug 27, 2020
a8f35ad
Revert POC code.
jodator Aug 31, 2020
dbc1ed6
Fix tests.
jodator Aug 31, 2020
1a4245b
Add base scenarios for re-converting using triggerBy.
jodator Aug 31, 2020
cfe4658
Make triggerBy test example to use a more complex view.
jodator Aug 31, 2020
5f77aae
Refactor tests to simple and complex scenarios.
jodator Sep 1, 2020
2c95c02
Add tests for complex view structure without children.
jodator Sep 1, 2020
89e6afa
Unify test cases names.
jodator Sep 1, 2020
dff6f2f
Add tests for slot conversion.
jodator Sep 1, 2020
703ac9e
Update test cases for slot conversion.
jodator Sep 1, 2020
a30c086
Add more cases for memoization in downcast converter.
jodator Sep 1, 2020
c2609ac
Fix case of re-converting main element on child add.
jodator Sep 1, 2020
28bd1cf
Add test for removing a slot element.
jodator Sep 1, 2020
16d18d4
Typo fix.
jodator Sep 1, 2020
213cb9d
Merge branch 'master' into i/7956-reconversion-trigger-by
jodator Sep 1, 2020
ffc29a6
Move poc code to downcastDispatcher.convertChanges().
jodator Sep 3, 2020
7e7e4b7
Remove redundant slice and move it to return statements.
jodator Sep 3, 2020
bd192b5
Inline poc code into DowncastDispatcher.convertChanges().
jodator Sep 3, 2020
9164e3c
Refactor downcasthelpers tests.
jodator Sep 3, 2020
2b9bd51
Use re-render whole element with triggerBy in table downcast conversion.
jodator Sep 3, 2020
5da1570
Add test cases for various slot conversion scenarios.
jodator Sep 4, 2020
93dedac
Update links to issues.
jodator Sep 4, 2020
5b3a700
Handle child removal on parent refresh in the differ.
jodator Sep 4, 2020
9fa12db
Handle attribute change on refreshed node.
jodator Sep 7, 2020
5507aab
Update the differ tests for "refresh" changes handling.
jodator Sep 7, 2020
1013f0e
Merge branch 'master' into i/7956-reconversion-trigger-by
jodator Sep 9, 2020
a8ab011
Handle remove when converting refresh change.
jodator Sep 9, 2020
db3b0cc
Merge branch 'master' into i/7956-reconversion-trigger-by
jodator Sep 14, 2020
e3b8c4e
Handle item refresh in downcast dispatcher.
jodator Sep 14, 2020
1b65b1a
Trigger conversion on adding a slot with new content.
jodator Sep 14, 2020
db8c17a
Fix test cases for slot reconversion.
jodator Sep 14, 2020
63f4b30
Update slot conversion tests formatting.
jodator Sep 14, 2020
36f8d1e
Skip tests that should be covered by #1589.
jodator Sep 14, 2020
74a777a
Remove debug code.
jodator Sep 14, 2020
64d11de
Update documentation of the downcast dispatcher and downcast helpers.
jodator Sep 14, 2020
df4c370
Skip hanging tests.
jodator Sep 15, 2020
5a00cbb
Do not relay on particular API in table cell refresh post-fixer tests.
jodator Sep 15, 2020
62a3fb3
Rewrite table cell refresh post-fixer to use new refresh mechanism on…
jodator Sep 15, 2020
b7a2c84
WiP: Update differ & downcast dispatcher cooperation on refresh change.
jodator Sep 15, 2020
ece9999
Bring back attribute hindering in differ as "refresh" change should b…
jodator Sep 16, 2020
8125eee
Adjust table downcast conversion to anticipate external converter for…
jodator Sep 16, 2020
ab6468a
Move view refreshing logic from downcast helpers to the dispatcher.
jodator Sep 16, 2020
c3e7471
Code style in tests.
jodator Sep 16, 2020
a081176
Remove old Differ.refreshItem with poc implementation.
jodator Sep 16, 2020
544a1c9
Remove refreshItem checks for table cell refresh post-fixer tests.
jodator Sep 16, 2020
11eb06d
Update differ code and tests for coverage.
jodator Sep 16, 2020
fb1fd1b
Update differ code and tests for coverage.
jodator Sep 16, 2020
8dc9244
Skip table tests causing infinite differ.getChanges() loop.
jodator Sep 16, 2020
7eefb61
Merge branch 'master' into i/7956-reconversion-trigger-by
jodator Sep 16, 2020
000d50b
Bring back table clipboard tests formatting.
jodator Sep 16, 2020
ef19033
Typo fix.
jodator Sep 17, 2020
4a9beb2
Skip main event.
jodator Sep 17, 2020
d11e778
Bring back table re-insert to previous implementation of differ.refre…
jodator Sep 17, 2020
09b5baa
Bring back tests for Differ.reInsertItem (old refreshItem).
jodator Sep 17, 2020
ac56842
Refactor DowncastDispatcher.convertChanges().
jodator Sep 22, 2020
45076c9
Refactor DowncastDispatcher.convertRefresh().
jodator Sep 22, 2020
cf6e4fc
Refactor internal method names.
jodator Sep 22, 2020
c02fe65
Remove useless checks in reconvert element handling.
jodator Sep 22, 2020
a36d954
Refactor DowncastDispatcher._reconvertElement() API.
jodator Sep 22, 2020
6c707ff
Differ refresh should not be converter on newly inserted items.
jodator Sep 22, 2020
3e9c9eb
Refresh change should be forfeit also when inside range insert (multi…
jodator Sep 23, 2020
adfba56
Consequent element refresh should be handled as one change.
jodator Sep 23, 2020
60740f2
Add tests for multiple non-overlapping refresh changes.
jodator Sep 23, 2020
225ea0e
Minor leftover fixes.
jodator Sep 24, 2020
8c29821
Bring back table cell paragraph post-fixer tests.
jodator Sep 24, 2020
8ba1791
Add info about follow-ups to the code & comments cleanup.
jodator Sep 24, 2020
e7f161c
Update API of slot binding.
jodator Sep 24, 2020
1c24874
Revert article manual tests and create slot conversion manual test in…
jodator Sep 24, 2020
e49cb63
Merge branch 'master' into i/7956-reconversion-trigger-by
jodator Sep 24, 2020
7b0dafc
Add ui as a dev dependencies for the engine feature.
jodator Sep 24, 2020
270359f
Update table-cell-refresh-post-fixer comments and internal code.
jodator Sep 24, 2020
c3b15ce
Merge branch 'master' into i/7956-reconversion-trigger-by
jodator Sep 28, 2020
93dbbf9
Move paragraph converter from table editing to conversion/downcast.
jodator Sep 28, 2020
76a48bd
Minor fixes in manual test.
niegowski Sep 30, 2020
3d5bea5
Apply suggestions from code review
jodator Oct 1, 2020
a671ed5
Merge branch 'master' into i/7956-reconversion-trigger-by
jodator Oct 1, 2020
3079ff7
Update reconversion documentation for DowncastDispatcher.
jodator Oct 1, 2020
9421571
Remove duplicated code from DowncastDispatcher.
jodator Oct 1, 2020
7e50748
Add early return for text attribute change in automatic element refre…
jodator Oct 1, 2020
d9e6049
Updates docs in DowncastHelpers.
jodator Oct 1, 2020
9f01122
Remove helper method from a global scope in downcasthelpers.js.
jodator Oct 1, 2020
1f67593
Explain why slot-to-view mapping is needed in mapper.
jodator Oct 1, 2020
61fd0d2
Remove intermediate array.
jodator Oct 1, 2020
b5b227d
Update the automatic element refreshing in DowncastDispatcher.
jodator Oct 1, 2020
8428d9e
Update information about number of "x" actions for each change in the…
jodator Oct 1, 2020
f6b1327
Update packages/ckeditor5-table/src/converters/table-cell-refresh-pos…
jodator Oct 1, 2020
a8846e6
Add memoization check to table-cell-refresh-post-fixer tests.
jodator Oct 1, 2020
7d825c3
Refactor, again, how table cell refresh post-fixer check which paragr…
jodator Oct 1, 2020
c123cf3
Refactor, again, how table cell refresh post-fixer check which paragr…
jodator Oct 1, 2020
a8d8208
Remove caching of already refreshed items.
jodator Oct 1, 2020
eba356c
Refactored check for refresh trigger event mapping.
jodator Oct 2, 2020
a143f8f
Add jsdoc comment.
jodator Oct 2, 2020
64ae08b
Update comment.
jodator Oct 2, 2020
85bec0b
Add child consuming for slot conversion manual test.
jodator Oct 2, 2020
83da738
Update comments.
jodator Oct 2, 2020
3c82a08
Apply suggestions from code review
jodator Oct 8, 2020
0f59a7a
Change how the downcast dispatcher handles triggerBy attribute for re…
jodator Oct 8, 2020
8081dce
Revert differ changes.
jodator Oct 8, 2020
7e359d8
Add more tests for the re-conversion scenarios.
jodator Oct 8, 2020
467ebcc
Fix view elements memoization.
jodator Oct 9, 2020
9704c97
Remove slot binding after changes in memoization done by DowncastDisp…
jodator Oct 12, 2020
4abbf4a
Change triggerBy configuration API.
jodator Oct 12, 2020
39592eb
Rename conversion refresh to reconversion.
jodator Oct 12, 2020
7c85c64
Refactor DowncastDispatcher#_mapChangesWithAutomaticReconversion().
jodator Oct 12, 2020
019066e
Add comments to DowncastDispatcher#reconvertElement().
jodator Oct 12, 2020
dc85a1e
Merge branch 'master' into i/7956-reconversion-trigger-by
jodator Oct 12, 2020
e8921b6
Rename DowncastDispatcher#mapReconversionTriggerEvent to _mapReconver…
jodator Oct 12, 2020
2a5e54a
Update jsdocs of the reconvertElement methods.
jodator Oct 12, 2020
e31c77a
Update logic for ignoring text changes when mapping events to reconve…
jodator Oct 13, 2020
b8c5703
Remove Array.isArray() check for configuration option.
jodator Oct 13, 2020
73c2fb8
Fix manual test.
jodator Oct 13, 2020
857343b
Apply suggestions from code review
jodator Oct 13, 2020
d193903
Update DowncastDispatcher#_mapChangesWithAutomaticReconversion() docs.
jodator Oct 13, 2020
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/ckeditor5-engine/package.json
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
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