From 0b2044e9af989c8fba6dcf2571f9e2917bc5ff3b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Tue, 4 Aug 2020 13:20:33 +0200 Subject: [PATCH 1/8] Introduced additional conversion options for downcast conversion. --- .../src/editor/utils/dataapimixin.js | 1 + .../src/controller/datacontroller.js | 17 +-- .../src/conversion/downcastdispatcher.js | 20 +++- .../tests/controller/datacontroller.js | 108 ++++++++++++++++++ .../tests/conversion/downcastdispatcher.js | 41 +++++++ 5 files changed, 176 insertions(+), 11 deletions(-) diff --git a/packages/ckeditor5-core/src/editor/utils/dataapimixin.js b/packages/ckeditor5-core/src/editor/utils/dataapimixin.js index 057d8bb8883..f9f76d1dcf2 100644 --- a/packages/ckeditor5-core/src/editor/utils/dataapimixin.js +++ b/packages/ckeditor5-core/src/editor/utils/dataapimixin.js @@ -76,5 +76,6 @@ export default DataApiMixin; * @param {String} [options.trim='empty'] Whether returned data should be trimmed. This option is set to `'empty'` by default, * which means that whenever editor content is considered empty, an empty string is returned. To turn off trimming * use `'none'`. In such cases exact content will be returned (for example `'

 

'` for an empty editor). + * @param {Object} [options.conversionOptions] Additional, custom configuration passed to the conversion process. * @returns {String} Output data. */ diff --git a/packages/ckeditor5-engine/src/controller/datacontroller.js b/packages/ckeditor5-engine/src/controller/datacontroller.js index e779e3da0c6..7f7329be185 100644 --- a/packages/ckeditor5-engine/src/controller/datacontroller.js +++ b/packages/ckeditor5-engine/src/controller/datacontroller.js @@ -151,10 +151,11 @@ export default class DataController { * @param {String} [options.trim='empty'] Whether returned data should be trimmed. This option is set to `empty` by default, * which means whenever editor content is considered empty, an empty string will be returned. To turn off trimming completely * use `'none'`. In such cases exact content will be returned (for example `

 

` for an empty editor). + * @param {Object} [options.conversionOptions] Additional, custom configuration passed to the conversion process. * @returns {String} Output data. */ get( options ) { - const { rootName = 'main', trim = 'empty' } = options || {}; + const { rootName = 'main', trim = 'empty', conversionOptions } = options || {}; if ( !this._checkIfRootsExists( [ rootName ] ) ) { /** @@ -177,7 +178,7 @@ export default class DataController { return ''; } - return this.stringify( root ); + return this.stringify( root, conversionOptions ); } /** @@ -187,11 +188,12 @@ export default class DataController { * * @param {module:engine/model/element~Element|module:engine/model/documentfragment~DocumentFragment} modelElementOrFragment * Element whose content will be stringified. + * @param {Object} [conversionOptions] Additional, custom configuration passed to the conversion process. * @returns {String} Output data. */ - stringify( modelElementOrFragment ) { + stringify( modelElementOrFragment, conversionOptions ) { // Model -> view. - const viewDocumentFragment = this.toView( modelElementOrFragment ); + const viewDocumentFragment = this.toView( modelElementOrFragment, conversionOptions ); // View -> data. return this.processor.toData( viewDocumentFragment ); @@ -205,9 +207,10 @@ export default class DataController { * * @param {module:engine/model/element~Element|module:engine/model/documentfragment~DocumentFragment} modelElementOrFragment * Element or document fragment whose content will be converted. + * @param {Object} [conversionOptions] Additional, custom configuration passed to the conversion process. * @returns {module:engine/view/documentfragment~DocumentFragment} Output view DocumentFragment. */ - toView( modelElementOrFragment ) { + toView( modelElementOrFragment, conversionOptions ) { const viewDocument = this.viewDocument; const viewWriter = this._viewWriter; @@ -221,7 +224,7 @@ export default class DataController { this.mapper.bindElements( modelElementOrFragment, viewDocumentFragment ); // We have no view controller and rendering to DOM in DataController so view.change() block is not used here. - this.downcastDispatcher.convertInsert( modelRange, viewWriter ); + this.downcastDispatcher.convertInsert( modelRange, viewWriter, conversionOptions ); if ( !modelElementOrFragment.is( 'documentFragment' ) ) { // Then, if a document element is converted, convert markers. @@ -229,7 +232,7 @@ export default class DataController { const markers = _getMarkersRelativeToElement( modelElementOrFragment ); for ( const [ name, range ] of markers ) { - this.downcastDispatcher.convertMarkerAdd( name, range, viewWriter ); + this.downcastDispatcher.convertMarkerAdd( name, range, viewWriter, conversionOptions ); } } diff --git a/packages/ckeditor5-engine/src/conversion/downcastdispatcher.js b/packages/ckeditor5-engine/src/conversion/downcastdispatcher.js index aa2ab403506..80ff4e9befb 100644 --- a/packages/ckeditor5-engine/src/conversion/downcastdispatcher.js +++ b/packages/ckeditor5-engine/src/conversion/downcastdispatcher.js @@ -11,7 +11,6 @@ import Consumable from './modelconsumable'; import Range from '../model/range'; import EmitterMixin from '@ckeditor/ckeditor5-utils/src/emittermixin'; import mix from '@ckeditor/ckeditor5-utils/src/mix'; -import { extend } from 'lodash-es'; /** * Downcast dispatcher is a central point of downcasting (conversion from the model to the view), which is a process of reacting to changes @@ -115,7 +114,7 @@ export default class DowncastDispatcher { * * @member {module:engine/conversion/downcastdispatcher~DowncastConversionApi} */ - this.conversionApi = extend( { dispatcher: this }, conversionApi ); + this.conversionApi = Object.assign( { dispatcher: this }, conversionApi ); } /** @@ -166,9 +165,12 @@ export default class DowncastDispatcher { * @fires attribute * @param {module:engine/model/range~Range} range The inserted range. * @param {module:engine/view/downcastwriter~DowncastWriter} writer The view writer that should be used to modify the view document. + * @param {Object} [options={}] Additional, custom configuration passed to the converters through + * {module:engine/conversion/downcastdispatcher~DowncastConversionApi conversion api}. Used only with `dataDowncast` group. */ - convertInsert( range, writer ) { + convertInsert( range, writer, options = {} ) { this.conversionApi.writer = writer; + this.conversionApi.options = options; // Create a list of things that can be consumed, consisting of nodes and their attributes. this.conversionApi.consumable = this._createInsertConsumable( range ); @@ -319,14 +321,17 @@ export default class DowncastDispatcher { * @param {String} markerName Marker name. * @param {module:engine/model/range~Range} markerRange Marker range. * @param {module:engine/view/downcastwriter~DowncastWriter} writer View writer that should be used to modify view document. + * @param {Object} [options={}] Additional, custom configuration passed to the converters through + * {module:engine/conversion/downcastdispatcher~DowncastConversionApi conversion api}. Used only with `dataDowncast` group. */ - convertMarkerAdd( markerName, markerRange, writer ) { + convertMarkerAdd( markerName, markerRange, writer, options = {} ) { // Do not convert if range is in graveyard or not in the document (e.g. in DocumentFragment). if ( !markerRange.root.document || markerRange.root.rootName == '$graveyard' ) { return; } this.conversionApi.writer = writer; + this.conversionApi.options = options; // In markers' case, event name == consumable name. const eventName = 'addMarker:' + markerName; @@ -482,6 +487,7 @@ export default class DowncastDispatcher { _clearConversionApi() { delete this.conversionApi.writer; delete this.conversionApi.consumable; + delete this.conversionApi.options; } /** @@ -669,3 +675,9 @@ function shouldMarkerChangeBeConverted( modelPosition, marker, mapper ) { * * @member {module:engine/view/downcastwriter~DowncastWriter} #writer */ + +/** + * An object with an additional, custom configuration used during conversion process. Available only for `dataDowncast` group. + * + * @member {Object} #options + */ diff --git a/packages/ckeditor5-engine/tests/controller/datacontroller.js b/packages/ckeditor5-engine/tests/controller/datacontroller.js index e21c1bf2967..a41c16235b5 100644 --- a/packages/ckeditor5-engine/tests/controller/datacontroller.js +++ b/packages/ckeditor5-engine/tests/controller/datacontroller.js @@ -454,6 +454,82 @@ describe( 'DataController', () => { data.get( { rootName: 'nonexistent' } ); }, /datacontroller-get-non-existent-root:/ ); } ); + + it( 'should allow to provide additional conversion options - insert conversion', () => { + schema.register( 'paragraph', { inheritAllFrom: '$block' } ); + + data.downcastDispatcher.on( 'insert:paragraph', ( evt, data, conversionApi ) => { + conversionApi.consumable.consume( data.item, 'insert' ); + + const viewPosition = conversionApi.mapper.toViewPosition( data.range.start ); + const viewElement = conversionApi.writer.createContainerElement( 'p', { + attribute: conversionApi.options.attributeValue + } ); + + conversionApi.mapper.bindElements( data.item, viewElement ); + conversionApi.writer.insert( viewPosition, viewElement ); + }, { priority: 'high' } ); + + setData( model, 'foo' ); + + expect( data.get( { conversionOptions: { attributeValue: 'foo' } } ) ).to.equal( '

foo

' ); + expect( data.get( { conversionOptions: { attributeValue: 'bar' } } ) ).to.equal( '

foo

' ); + } ); + + it( 'should allow to provide additional conversion options - attribute conversion', () => { + schema.register( 'paragraph', { inheritAllFrom: '$block', allowAttributes: [ 'foo' ] } ); + downcastHelpers.elementToElement( { model: 'paragraph', view: 'p' } ); + + data.downcastDispatcher.on( 'attribute:foo', ( evt, data, conversionApi ) => { + if ( data.attributeNewValue === conversionApi.options.skipAttribute ) { + return; + } + + const viewRange = conversionApi.mapper.toViewRange( data.range ); + const viewElement = conversionApi.writer.createAttributeElement( data.attributeNewValue ); + + conversionApi.writer.wrap( viewRange, viewElement ); + } ); + + setData( model, 'f<$text foo="a">oob<$text foo="b">ar' ); + + expect( data.get() ).to.equal( '

foobar

' ); + expect( data.get( { conversionOptions: { skipAttribute: 'a' } } ) ).to.equal( '

foobar

' ); + expect( data.get( { conversionOptions: { skipAttribute: 'b' } } ) ).to.equal( '

foobar

' ); + } ); + + it( 'should allow to provide additional conversion options - addMarker conversion', () => { + schema.register( 'paragraph', { inheritAllFrom: '$block' } ); + downcastHelpers.elementToElement( { model: 'paragraph', view: 'p' } ); + + data.downcastDispatcher.on( 'addMarker', ( evt, data, conversionApi ) => { + if ( conversionApi.options.skipMarker ) { + return; + } + + const viewElement = conversionApi.writer.createAttributeElement( 'marker' ); + const viewRange = conversionApi.mapper.toViewRange( data.markerRange ); + + conversionApi.writer.wrap( viewRange, viewElement ); + } ); + + setData( model, 'foo' ); + + const root = model.document.getRoot(); + + model.change( writer => { + const start = writer.createPositionFromPath( root, [ 0, 1 ] ); + const end = writer.createPositionFromPath( root, [ 0, 2 ] ); + + writer.addMarker( 'marker', { + range: writer.createRange( start, end ), + usingOperation: false + } ); + } ); + + expect( data.get( { conversionOptions: { skipMarker: false } } ) ).to.equal( '

foo

' ); + expect( data.get( { conversionOptions: { skipMarker: true } } ) ).to.equal( '

foo

' ); + } ); } ); describe( 'stringify()', () => { @@ -478,6 +554,22 @@ describe( 'DataController', () => { expect( data.stringify( modelDocumentFragment ) ).to.equal( '

foo

bar

' ); } ); + + it( 'should allow to provide additional options to the conversion process', () => { + const spy = sinon.spy(); + + data.downcastDispatcher.on( 'insert:paragraph', ( evt, data, conversionApi ) => { + spy( conversionApi.options ); + }, { priority: 'high' } ); + + const modelDocumentFragment = parseModel( 'foobar', schema ); + + data.stringify( modelDocumentFragment ); + expect( spy.lastCall.args[ 0 ] ).to.deep.equal( {} ); + + data.stringify( modelDocumentFragment, { foo: 'bar' } ); + expect( spy.lastCall.args[ 0 ] ).to.deep.equal( { foo: 'bar' } ); + } ); } ); describe( 'toView()', () => { @@ -590,6 +682,22 @@ describe( 'DataController', () => { expect( mappedViewRange.end.nodeBefore ).to.equal( firstViewElement ); expect( mappedViewRange.end.nodeAfter ).to.equal( viewDocumentFragment.getChild( 1 ) ); } ); + + it( 'should allow to provide additional options to the conversion process', () => { + const spy = sinon.spy(); + + data.downcastDispatcher.on( 'insert:paragraph', ( evt, data, conversionApi ) => { + spy( conversionApi.options ); + }, { priority: 'high' } ); + + const modelDocumentFragment = parseModel( 'foobar', schema ); + + data.toView( modelDocumentFragment ); + expect( spy.lastCall.args[ 0 ] ).to.deep.equal( {} ); + + data.toView( modelDocumentFragment, { foo: 'bar' } ); + expect( spy.lastCall.args[ 0 ] ).to.deep.equal( { foo: 'bar' } ); + } ); } ); describe( 'destroy()', () => { diff --git a/packages/ckeditor5-engine/tests/conversion/downcastdispatcher.js b/packages/ckeditor5-engine/tests/conversion/downcastdispatcher.js index 23d98dd4fda..d2e9f90f3e8 100644 --- a/packages/ckeditor5-engine/tests/conversion/downcastdispatcher.js +++ b/packages/ckeditor5-engine/tests/conversion/downcastdispatcher.js @@ -257,6 +257,31 @@ describe( 'DowncastDispatcher', () => { expect( dispatcher.fire.calledWith( 'attribute:bold:image' ) ).to.be.false; expect( dispatcher.fire.calledWith( 'insert:caption' ) ).to.be.false; } ); + + it( 'should allow to provide an additional options', () => { + root._appendChild( [ + new ModelText( 'foo', { bold: true } ) + ] ); + + const range = model.createRangeIn( root ); + const conversionOptions = { foo: 'bar' }; + const spy = sinon.spy(); + + dispatcher.on( 'insert', ( evt, data, conversionApi ) => { + expect( conversionApi.options ).to.equal( conversionOptions ); + spy(); + } ); + + dispatcher.on( 'attribute', ( evt, data, conversionApi ) => { + expect( conversionApi.options ).to.equal( conversionOptions ); + spy(); + } ); + + dispatcher.convertInsert( range, {}, conversionOptions ); + + // To be sure all assertions was checked. + sinon.assert.calledTwice( spy ); + } ); } ); describe( 'convertRemove', () => { @@ -630,6 +655,22 @@ describe( 'DowncastDispatcher', () => { // Called once for each item, twice total. expect( highAddMarkerSpy.calledTwice ).to.be.true; } ); + + it( 'should allow to provide an additional options', () => { + const range = model.createRange( model.createPositionAt( element, 2 ), model.createPositionAt( element, 2 ) ); + const conversionOptions = { foo: 'bar' }; + const spy = sinon.spy(); + + dispatcher.on( 'addMarker:name', ( evt, data, conversionApi ) => { + expect( conversionApi.options ).to.equal( conversionOptions ); + spy(); + } ); + + dispatcher.convertMarkerAdd( 'name', range, {}, conversionOptions ); + + // To be sure all assertions was checked. + sinon.assert.called( spy ); + } ); } ); describe( 'convertMarkerRemove', () => { From 5b8a7b19ce10fed886a80b66d944a1f52372b788 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Fri, 7 Aug 2020 12:10:44 +0200 Subject: [PATCH 2/8] Made getData options flat. --- .../src/controller/datacontroller.js | 27 +++++++------- .../tests/controller/datacontroller.js | 35 ++++++++++++++----- 2 files changed, 41 insertions(+), 21 deletions(-) diff --git a/packages/ckeditor5-engine/src/controller/datacontroller.js b/packages/ckeditor5-engine/src/controller/datacontroller.js index 7f7329be185..4d507672d0f 100644 --- a/packages/ckeditor5-engine/src/controller/datacontroller.js +++ b/packages/ckeditor5-engine/src/controller/datacontroller.js @@ -146,16 +146,19 @@ export default class DataController { * Returns the model's data converted by downcast dispatchers attached to {@link #downcastDispatcher} and * formatted by the {@link #processor data processor}. * - * @param {Object} [options] + * @param {Object} [options] Additional configuration for the retrieved data. `DataController` provides two optional + * properties: `root` and `trim`. Other properties of this object are specified by various editor features. * @param {String} [options.rootName='main'] Root name. * @param {String} [options.trim='empty'] Whether returned data should be trimmed. This option is set to `empty` by default, * which means whenever editor content is considered empty, an empty string will be returned. To turn off trimming completely * use `'none'`. In such cases exact content will be returned (for example `

 

` for an empty editor). - * @param {Object} [options.conversionOptions] Additional, custom configuration passed to the conversion process. * @returns {String} Output data. */ - get( options ) { - const { rootName = 'main', trim = 'empty', conversionOptions } = options || {}; + get( options = {} ) { + const { rootName = 'main', trim = 'empty' } = options; + + delete options.rootName; + delete options.trim; if ( !this._checkIfRootsExists( [ rootName ] ) ) { /** @@ -178,7 +181,7 @@ export default class DataController { return ''; } - return this.stringify( root, conversionOptions ); + return this.stringify( root, options ); } /** @@ -188,12 +191,12 @@ export default class DataController { * * @param {module:engine/model/element~Element|module:engine/model/documentfragment~DocumentFragment} modelElementOrFragment * Element whose content will be stringified. - * @param {Object} [conversionOptions] Additional, custom configuration passed to the conversion process. + * @param {Object} [options] Additional configuration passed to the conversion process. * @returns {String} Output data. */ - stringify( modelElementOrFragment, conversionOptions ) { + stringify( modelElementOrFragment, options ) { // Model -> view. - const viewDocumentFragment = this.toView( modelElementOrFragment, conversionOptions ); + const viewDocumentFragment = this.toView( modelElementOrFragment, options ); // View -> data. return this.processor.toData( viewDocumentFragment ); @@ -207,10 +210,10 @@ export default class DataController { * * @param {module:engine/model/element~Element|module:engine/model/documentfragment~DocumentFragment} modelElementOrFragment * Element or document fragment whose content will be converted. - * @param {Object} [conversionOptions] Additional, custom configuration passed to the conversion process. + * @param {Object} [options] Additional configuration passed to the conversion process. * @returns {module:engine/view/documentfragment~DocumentFragment} Output view DocumentFragment. */ - toView( modelElementOrFragment, conversionOptions ) { + toView( modelElementOrFragment, options ) { const viewDocument = this.viewDocument; const viewWriter = this._viewWriter; @@ -224,7 +227,7 @@ export default class DataController { this.mapper.bindElements( modelElementOrFragment, viewDocumentFragment ); // We have no view controller and rendering to DOM in DataController so view.change() block is not used here. - this.downcastDispatcher.convertInsert( modelRange, viewWriter, conversionOptions ); + this.downcastDispatcher.convertInsert( modelRange, viewWriter, options ); if ( !modelElementOrFragment.is( 'documentFragment' ) ) { // Then, if a document element is converted, convert markers. @@ -232,7 +235,7 @@ export default class DataController { const markers = _getMarkersRelativeToElement( modelElementOrFragment ); for ( const [ name, range ] of markers ) { - this.downcastDispatcher.convertMarkerAdd( name, range, viewWriter, conversionOptions ); + this.downcastDispatcher.convertMarkerAdd( name, range, viewWriter, options ); } } diff --git a/packages/ckeditor5-engine/tests/controller/datacontroller.js b/packages/ckeditor5-engine/tests/controller/datacontroller.js index a41c16235b5..bb577009f29 100644 --- a/packages/ckeditor5-engine/tests/controller/datacontroller.js +++ b/packages/ckeditor5-engine/tests/controller/datacontroller.js @@ -455,7 +455,7 @@ describe( 'DataController', () => { }, /datacontroller-get-non-existent-root:/ ); } ); - it( 'should allow to provide additional conversion options - insert conversion', () => { + it( 'should allow to provide additional options for retrieving data - insert conversion', () => { schema.register( 'paragraph', { inheritAllFrom: '$block' } ); data.downcastDispatcher.on( 'insert:paragraph', ( evt, data, conversionApi ) => { @@ -472,11 +472,11 @@ describe( 'DataController', () => { setData( model, 'foo' ); - expect( data.get( { conversionOptions: { attributeValue: 'foo' } } ) ).to.equal( '

foo

' ); - expect( data.get( { conversionOptions: { attributeValue: 'bar' } } ) ).to.equal( '

foo

' ); + expect( data.get( { attributeValue: 'foo' } ) ).to.equal( '

foo

' ); + expect( data.get( { attributeValue: 'bar' } ) ).to.equal( '

foo

' ); } ); - it( 'should allow to provide additional conversion options - attribute conversion', () => { + it( 'should allow to provide additional options for retrieving data - attribute conversion', () => { schema.register( 'paragraph', { inheritAllFrom: '$block', allowAttributes: [ 'foo' ] } ); downcastHelpers.elementToElement( { model: 'paragraph', view: 'p' } ); @@ -494,11 +494,11 @@ describe( 'DataController', () => { setData( model, 'f<$text foo="a">oob<$text foo="b">ar' ); expect( data.get() ).to.equal( '

foobar

' ); - expect( data.get( { conversionOptions: { skipAttribute: 'a' } } ) ).to.equal( '

foobar

' ); - expect( data.get( { conversionOptions: { skipAttribute: 'b' } } ) ).to.equal( '

foobar

' ); + expect( data.get( { skipAttribute: 'a' } ) ).to.equal( '

foobar

' ); + expect( data.get( { skipAttribute: 'b' } ) ).to.equal( '

foobar

' ); } ); - it( 'should allow to provide additional conversion options - addMarker conversion', () => { + it( 'should allow to provide additional options for retrieving data - addMarker conversion', () => { schema.register( 'paragraph', { inheritAllFrom: '$block' } ); downcastHelpers.elementToElement( { model: 'paragraph', view: 'p' } ); @@ -527,8 +527,25 @@ describe( 'DataController', () => { } ); } ); - expect( data.get( { conversionOptions: { skipMarker: false } } ) ).to.equal( '

foo

' ); - expect( data.get( { conversionOptions: { skipMarker: true } } ) ).to.equal( '

foo

' ); + expect( data.get( { skipMarker: false } ) ).to.equal( '

foo

' ); + expect( data.get( { skipMarker: true } ) ).to.equal( '

foo

' ); + } ); + + it( 'should provide additional options for retrieving data without basic rootName and trim properties', () => { + downcastHelpers.elementToElement( { model: 'paragraph', view: 'p' } ); + schema.register( 'paragraph', { inheritAllFrom: '$block' } ); + + const spy = sinon.spy(); + + data.downcastDispatcher.on( 'insert:paragraph', ( evt, data, conversionApi ) => { + spy( conversionApi.options ); + }, { priority: 'high' } ); + + setData( model, 'foo' ); + + data.get( { rootName: 'main', trim: 'empty', foo: 'bar' } ); + + expect( spy.lastCall.args[ 0 ] ).to.deep.equal( { foo: 'bar' } ); } ); } ); From ef301bb2cf88318d1019c59f52d30ee8732f52cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Fri, 7 Aug 2020 14:31:32 +0200 Subject: [PATCH 3/8] Do not remove basic options from DataController#get config. --- .../src/controller/datacontroller.js | 3 --- .../tests/controller/datacontroller.js | 17 ----------------- 2 files changed, 20 deletions(-) diff --git a/packages/ckeditor5-engine/src/controller/datacontroller.js b/packages/ckeditor5-engine/src/controller/datacontroller.js index 4d507672d0f..4e31cf46d06 100644 --- a/packages/ckeditor5-engine/src/controller/datacontroller.js +++ b/packages/ckeditor5-engine/src/controller/datacontroller.js @@ -157,9 +157,6 @@ export default class DataController { get( options = {} ) { const { rootName = 'main', trim = 'empty' } = options; - delete options.rootName; - delete options.trim; - if ( !this._checkIfRootsExists( [ rootName ] ) ) { /** * Cannot get data from a non-existing root. This error is thrown when {@link #get DataController#get() method} diff --git a/packages/ckeditor5-engine/tests/controller/datacontroller.js b/packages/ckeditor5-engine/tests/controller/datacontroller.js index bb577009f29..022c3ec67ea 100644 --- a/packages/ckeditor5-engine/tests/controller/datacontroller.js +++ b/packages/ckeditor5-engine/tests/controller/datacontroller.js @@ -530,23 +530,6 @@ describe( 'DataController', () => { expect( data.get( { skipMarker: false } ) ).to.equal( '

foo

' ); expect( data.get( { skipMarker: true } ) ).to.equal( '

foo

' ); } ); - - it( 'should provide additional options for retrieving data without basic rootName and trim properties', () => { - downcastHelpers.elementToElement( { model: 'paragraph', view: 'p' } ); - schema.register( 'paragraph', { inheritAllFrom: '$block' } ); - - const spy = sinon.spy(); - - data.downcastDispatcher.on( 'insert:paragraph', ( evt, data, conversionApi ) => { - spy( conversionApi.options ); - }, { priority: 'high' } ); - - setData( model, 'foo' ); - - data.get( { rootName: 'main', trim: 'empty', foo: 'bar' } ); - - expect( spy.lastCall.args[ 0 ] ).to.deep.equal( { foo: 'bar' } ); - } ); } ); describe( 'stringify()', () => { From 719a13c12b5319135308a317d2034c070d946523 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Mon, 10 Aug 2020 10:02:57 +0200 Subject: [PATCH 4/8] Changed the way of passing options to the downcast conversion process. --- .../src/controller/datacontroller.js | 13 ++++-- .../src/conversion/downcastdispatcher.js | 9 +--- .../tests/controller/datacontroller.js | 32 +++++++++++---- .../tests/conversion/downcastdispatcher.js | 41 ------------------- 4 files changed, 36 insertions(+), 59 deletions(-) diff --git a/packages/ckeditor5-engine/src/controller/datacontroller.js b/packages/ckeditor5-engine/src/controller/datacontroller.js index 4e31cf46d06..7fe511c33e3 100644 --- a/packages/ckeditor5-engine/src/controller/datacontroller.js +++ b/packages/ckeditor5-engine/src/controller/datacontroller.js @@ -207,7 +207,8 @@ export default class DataController { * * @param {module:engine/model/element~Element|module:engine/model/documentfragment~DocumentFragment} modelElementOrFragment * Element or document fragment whose content will be converted. - * @param {Object} [options] Additional configuration passed to the conversion process. + * @param {Object} [options] Additional configuration that will be available through + * {@link module:engine/conversion/downcastdispatcher~DowncastConversionApi#options} during the conversion process. * @returns {module:engine/view/documentfragment~DocumentFragment} Output view DocumentFragment. */ toView( modelElementOrFragment, options ) { @@ -223,8 +224,11 @@ export default class DataController { this.mapper.bindElements( modelElementOrFragment, viewDocumentFragment ); + // Make additional options available during conversion process through conversionSpi. + this.downcastDispatcher.conversionApi.options = options; + // We have no view controller and rendering to DOM in DataController so view.change() block is not used here. - this.downcastDispatcher.convertInsert( modelRange, viewWriter, options ); + this.downcastDispatcher.convertInsert( modelRange, viewWriter ); if ( !modelElementOrFragment.is( 'documentFragment' ) ) { // Then, if a document element is converted, convert markers. @@ -232,10 +236,13 @@ export default class DataController { const markers = _getMarkersRelativeToElement( modelElementOrFragment ); for ( const [ name, range ] of markers ) { - this.downcastDispatcher.convertMarkerAdd( name, range, viewWriter, options ); + this.downcastDispatcher.convertMarkerAdd( name, range, viewWriter ); } } + // Remove options from conversionApi. + delete this.downcastDispatcher.conversionApi.options; + return viewDocumentFragment; } diff --git a/packages/ckeditor5-engine/src/conversion/downcastdispatcher.js b/packages/ckeditor5-engine/src/conversion/downcastdispatcher.js index 80ff4e9befb..6e785932d89 100644 --- a/packages/ckeditor5-engine/src/conversion/downcastdispatcher.js +++ b/packages/ckeditor5-engine/src/conversion/downcastdispatcher.js @@ -165,12 +165,10 @@ export default class DowncastDispatcher { * @fires attribute * @param {module:engine/model/range~Range} range The inserted range. * @param {module:engine/view/downcastwriter~DowncastWriter} writer The view writer that should be used to modify the view document. - * @param {Object} [options={}] Additional, custom configuration passed to the converters through * {module:engine/conversion/downcastdispatcher~DowncastConversionApi conversion api}. Used only with `dataDowncast` group. */ - convertInsert( range, writer, options = {} ) { + convertInsert( range, writer ) { this.conversionApi.writer = writer; - this.conversionApi.options = options; // Create a list of things that can be consumed, consisting of nodes and their attributes. this.conversionApi.consumable = this._createInsertConsumable( range ); @@ -321,17 +319,15 @@ export default class DowncastDispatcher { * @param {String} markerName Marker name. * @param {module:engine/model/range~Range} markerRange Marker range. * @param {module:engine/view/downcastwriter~DowncastWriter} writer View writer that should be used to modify view document. - * @param {Object} [options={}] Additional, custom configuration passed to the converters through * {module:engine/conversion/downcastdispatcher~DowncastConversionApi conversion api}. Used only with `dataDowncast` group. */ - convertMarkerAdd( markerName, markerRange, writer, options = {} ) { + convertMarkerAdd( markerName, markerRange, writer ) { // Do not convert if range is in graveyard or not in the document (e.g. in DocumentFragment). if ( !markerRange.root.document || markerRange.root.rootName == '$graveyard' ) { return; } this.conversionApi.writer = writer; - this.conversionApi.options = options; // In markers' case, event name == consumable name. const eventName = 'addMarker:' + markerName; @@ -487,7 +483,6 @@ export default class DowncastDispatcher { _clearConversionApi() { delete this.conversionApi.writer; delete this.conversionApi.consumable; - delete this.conversionApi.options; } /** diff --git a/packages/ckeditor5-engine/tests/controller/datacontroller.js b/packages/ckeditor5-engine/tests/controller/datacontroller.js index 022c3ec67ea..d5e84e6cc26 100644 --- a/packages/ckeditor5-engine/tests/controller/datacontroller.js +++ b/packages/ckeditor5-engine/tests/controller/datacontroller.js @@ -564,11 +564,13 @@ describe( 'DataController', () => { const modelDocumentFragment = parseModel( 'foobar', schema ); + const options = { foo: 'bar' }; + data.stringify( modelDocumentFragment ); - expect( spy.lastCall.args[ 0 ] ).to.deep.equal( {} ); + expect( spy.lastCall.args[ 0 ] ).to.not.equal( options ); - data.stringify( modelDocumentFragment, { foo: 'bar' } ); - expect( spy.lastCall.args[ 0 ] ).to.deep.equal( { foo: 'bar' } ); + data.stringify( modelDocumentFragment, options ); + expect( spy.lastCall.args[ 0 ] ).to.equal( options ); } ); } ); @@ -684,19 +686,33 @@ describe( 'DataController', () => { } ); it( 'should allow to provide additional options to the conversion process', () => { + const root = model.document.getRoot(); const spy = sinon.spy(); data.downcastDispatcher.on( 'insert:paragraph', ( evt, data, conversionApi ) => { spy( conversionApi.options ); }, { priority: 'high' } ); - const modelDocumentFragment = parseModel( 'foobar', schema ); + data.downcastDispatcher.on( 'addMarker:marker', ( evt, data, conversionApi ) => { + spy( conversionApi.options ); + }, { priority: 'high' } ); + + setData( model, 'foo' ); + + model.change( writer => { + writer.addMarker( 'marker', { + range: model.createRange( model.createPositionFromPath( root, [ 0, 1 ] ) ), + usingOperation: false + } ); + } ); + + const options = { foo: 'bar' }; - data.toView( modelDocumentFragment ); - expect( spy.lastCall.args[ 0 ] ).to.deep.equal( {} ); + data.toView( root, options ); - data.toView( modelDocumentFragment, { foo: 'bar' } ); - expect( spy.lastCall.args[ 0 ] ).to.deep.equal( { foo: 'bar' } ); + sinon.assert.calledTwice( spy ); + expect( spy.firstCall.args[ 0 ] ).to.equal( options ); + expect( spy.lastCall.args[ 0 ] ).to.equal( options ); } ); } ); diff --git a/packages/ckeditor5-engine/tests/conversion/downcastdispatcher.js b/packages/ckeditor5-engine/tests/conversion/downcastdispatcher.js index d2e9f90f3e8..23d98dd4fda 100644 --- a/packages/ckeditor5-engine/tests/conversion/downcastdispatcher.js +++ b/packages/ckeditor5-engine/tests/conversion/downcastdispatcher.js @@ -257,31 +257,6 @@ describe( 'DowncastDispatcher', () => { expect( dispatcher.fire.calledWith( 'attribute:bold:image' ) ).to.be.false; expect( dispatcher.fire.calledWith( 'insert:caption' ) ).to.be.false; } ); - - it( 'should allow to provide an additional options', () => { - root._appendChild( [ - new ModelText( 'foo', { bold: true } ) - ] ); - - const range = model.createRangeIn( root ); - const conversionOptions = { foo: 'bar' }; - const spy = sinon.spy(); - - dispatcher.on( 'insert', ( evt, data, conversionApi ) => { - expect( conversionApi.options ).to.equal( conversionOptions ); - spy(); - } ); - - dispatcher.on( 'attribute', ( evt, data, conversionApi ) => { - expect( conversionApi.options ).to.equal( conversionOptions ); - spy(); - } ); - - dispatcher.convertInsert( range, {}, conversionOptions ); - - // To be sure all assertions was checked. - sinon.assert.calledTwice( spy ); - } ); } ); describe( 'convertRemove', () => { @@ -655,22 +630,6 @@ describe( 'DowncastDispatcher', () => { // Called once for each item, twice total. expect( highAddMarkerSpy.calledTwice ).to.be.true; } ); - - it( 'should allow to provide an additional options', () => { - const range = model.createRange( model.createPositionAt( element, 2 ), model.createPositionAt( element, 2 ) ); - const conversionOptions = { foo: 'bar' }; - const spy = sinon.spy(); - - dispatcher.on( 'addMarker:name', ( evt, data, conversionApi ) => { - expect( conversionApi.options ).to.equal( conversionOptions ); - spy(); - } ); - - dispatcher.convertMarkerAdd( 'name', range, {}, conversionOptions ); - - // To be sure all assertions was checked. - sinon.assert.called( spy ); - } ); } ); describe( 'convertMarkerRemove', () => { From 134ff463985497ac2d70b072f1e5d9bd16898ae5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Mon, 10 Aug 2020 13:58:43 +0200 Subject: [PATCH 5/8] Imporved docs. --- packages/ckeditor5-core/src/editor/utils/dataapimixin.js | 4 ++-- packages/ckeditor5-engine/src/controller/datacontroller.js | 6 +++--- .../ckeditor5-engine/src/conversion/downcastdispatcher.js | 2 -- 3 files changed, 5 insertions(+), 7 deletions(-) diff --git a/packages/ckeditor5-core/src/editor/utils/dataapimixin.js b/packages/ckeditor5-core/src/editor/utils/dataapimixin.js index f9f76d1dcf2..f1254603576 100644 --- a/packages/ckeditor5-core/src/editor/utils/dataapimixin.js +++ b/packages/ckeditor5-core/src/editor/utils/dataapimixin.js @@ -71,11 +71,11 @@ export default DataApiMixin; * the right format for you. * * @method #getData - * @param {Object} [options] + * @param {Object} [options] Additional configuration for the retrieved data. By default two optional + * properties: `rootName` and `trim` are provided. Other properties of this object are specified by various editor features. * @param {String} [options.rootName='main'] Root name. * @param {String} [options.trim='empty'] Whether returned data should be trimmed. This option is set to `'empty'` by default, * which means that whenever editor content is considered empty, an empty string is returned. To turn off trimming * use `'none'`. In such cases exact content will be returned (for example `'

 

'` for an empty editor). - * @param {Object} [options.conversionOptions] Additional, custom configuration passed to the conversion process. * @returns {String} Output data. */ diff --git a/packages/ckeditor5-engine/src/controller/datacontroller.js b/packages/ckeditor5-engine/src/controller/datacontroller.js index 7fe511c33e3..8fe921f7a0a 100644 --- a/packages/ckeditor5-engine/src/controller/datacontroller.js +++ b/packages/ckeditor5-engine/src/controller/datacontroller.js @@ -147,7 +147,7 @@ export default class DataController { * formatted by the {@link #processor data processor}. * * @param {Object} [options] Additional configuration for the retrieved data. `DataController` provides two optional - * properties: `root` and `trim`. Other properties of this object are specified by various editor features. + * properties: `rootName` and `trim`. Other properties of this object are specified by various editor features. * @param {String} [options.rootName='main'] Root name. * @param {String} [options.trim='empty'] Whether returned data should be trimmed. This option is set to `empty` by default, * which means whenever editor content is considered empty, an empty string will be returned. To turn off trimming completely @@ -224,7 +224,7 @@ export default class DataController { this.mapper.bindElements( modelElementOrFragment, viewDocumentFragment ); - // Make additional options available during conversion process through conversionSpi. + // Make additional options available during conversion process through `conversionApi`. this.downcastDispatcher.conversionApi.options = options; // We have no view controller and rendering to DOM in DataController so view.change() block is not used here. @@ -240,7 +240,7 @@ export default class DataController { } } - // Remove options from conversionApi. + // Clean `conversionApi`. delete this.downcastDispatcher.conversionApi.options; return viewDocumentFragment; diff --git a/packages/ckeditor5-engine/src/conversion/downcastdispatcher.js b/packages/ckeditor5-engine/src/conversion/downcastdispatcher.js index 6e785932d89..0a5a340eb26 100644 --- a/packages/ckeditor5-engine/src/conversion/downcastdispatcher.js +++ b/packages/ckeditor5-engine/src/conversion/downcastdispatcher.js @@ -165,7 +165,6 @@ export default class DowncastDispatcher { * @fires attribute * @param {module:engine/model/range~Range} range The inserted range. * @param {module:engine/view/downcastwriter~DowncastWriter} writer The view writer that should be used to modify the view document. - * {module:engine/conversion/downcastdispatcher~DowncastConversionApi conversion api}. Used only with `dataDowncast` group. */ convertInsert( range, writer ) { this.conversionApi.writer = writer; @@ -319,7 +318,6 @@ export default class DowncastDispatcher { * @param {String} markerName Marker name. * @param {module:engine/model/range~Range} markerRange Marker range. * @param {module:engine/view/downcastwriter~DowncastWriter} writer View writer that should be used to modify view document. - * {module:engine/conversion/downcastdispatcher~DowncastConversionApi conversion api}. Used only with `dataDowncast` group. */ convertMarkerAdd( markerName, markerRange, writer ) { // Do not convert if range is in graveyard or not in the document (e.g. in DocumentFragment). From 5c08d2f1b308e693dd45bc5ec81169480fa925d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Mon, 10 Aug 2020 14:19:33 +0200 Subject: [PATCH 6/8] More docs improvements. --- packages/ckeditor5-core/src/editor/utils/dataapimixin.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/ckeditor5-core/src/editor/utils/dataapimixin.js b/packages/ckeditor5-core/src/editor/utils/dataapimixin.js index f1254603576..057d8bb8883 100644 --- a/packages/ckeditor5-core/src/editor/utils/dataapimixin.js +++ b/packages/ckeditor5-core/src/editor/utils/dataapimixin.js @@ -71,8 +71,7 @@ export default DataApiMixin; * the right format for you. * * @method #getData - * @param {Object} [options] Additional configuration for the retrieved data. By default two optional - * properties: `rootName` and `trim` are provided. Other properties of this object are specified by various editor features. + * @param {Object} [options] * @param {String} [options.rootName='main'] Root name. * @param {String} [options.trim='empty'] Whether returned data should be trimmed. This option is set to `'empty'` by default, * which means that whenever editor content is considered empty, an empty string is returned. To turn off trimming From d0998347c4697428003320309373a3819187dfa5 Mon Sep 17 00:00:00 2001 From: Szymon Cofalik Date: Mon, 10 Aug 2020 14:28:11 +0200 Subject: [PATCH 7/8] Update packages/ckeditor5-engine/src/conversion/downcastdispatcher.js --- packages/ckeditor5-engine/src/conversion/downcastdispatcher.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/ckeditor5-engine/src/conversion/downcastdispatcher.js b/packages/ckeditor5-engine/src/conversion/downcastdispatcher.js index 0a5a340eb26..1fdc18a22f4 100644 --- a/packages/ckeditor5-engine/src/conversion/downcastdispatcher.js +++ b/packages/ckeditor5-engine/src/conversion/downcastdispatcher.js @@ -670,7 +670,7 @@ function shouldMarkerChangeBeConverted( modelPosition, marker, mapper ) { */ /** - * An object with an additional, custom configuration used during conversion process. Available only for `dataDowncast` group. + * An object with an additional configuration which can be used during conversion process. Available only for data downcast conversion. * * @member {Object} #options */ From 27453f608940b2dec2ac6b420b0aacfd6d586d12 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Wr=C3=B3bel?= Date: Mon, 10 Aug 2020 14:51:10 +0200 Subject: [PATCH 8/8] More and more docs. --- packages/ckeditor5-core/src/editor/utils/dataapimixin.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/ckeditor5-core/src/editor/utils/dataapimixin.js b/packages/ckeditor5-core/src/editor/utils/dataapimixin.js index 057d8bb8883..be18fb1d6fd 100644 --- a/packages/ckeditor5-core/src/editor/utils/dataapimixin.js +++ b/packages/ckeditor5-core/src/editor/utils/dataapimixin.js @@ -71,7 +71,8 @@ export default DataApiMixin; * the right format for you. * * @method #getData - * @param {Object} [options] + * @param {Object} [options] Additional configuration for the retrieved data. + * Editor features may introduce more configuration options that can be set through this parameter. * @param {String} [options.rootName='main'] Root name. * @param {String} [options.trim='empty'] Whether returned data should be trimmed. This option is set to `'empty'` by default, * which means that whenever editor content is considered empty, an empty string is returned. To turn off trimming