From 2638c1b8870703cb6c84068c8eec8cb69250185a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szymon=20Kup=C5=9B?= Date: Mon, 5 Jun 2017 13:55:14 +0200 Subject: [PATCH 01/15] Small doc fix in editing controller. --- src/controller/editingcontroller.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/controller/editingcontroller.js b/src/controller/editingcontroller.js index bf5d1438d..a9df9be2c 100644 --- a/src/controller/editingcontroller.js +++ b/src/controller/editingcontroller.js @@ -85,7 +85,7 @@ export default class EditingController { * stop listening on {@link #destroy}. * * @private - * @member {utils.EmitterMixin} #_listenter + * @member {utils.EmitterMixin} #_listener */ this._listener = Object.create( EmitterMixin ); From ce382235a931562dbcbb0bc4a8f887f48047780d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szymon=20Kup=C5=9B?= Date: Tue, 6 Jun 2017 14:16:38 +0200 Subject: [PATCH 02/15] DOMConverter is using UIElement.render method. --- src/view/domconverter.js | 8 ++++++++ src/view/uielement.js | 10 ++++++++++ tests/view/domconverter/view-to-dom.js | 25 +++++++++++++++++++++++++ 3 files changed, 43 insertions(+) diff --git a/src/view/domconverter.js b/src/view/domconverter.js index d8c7355ea..a6da5f9bd 100644 --- a/src/view/domconverter.js +++ b/src/view/domconverter.js @@ -199,6 +199,14 @@ export default class DomConverter { if ( options.bind ) { this.bindDocumentFragments( domElement, viewNode ); } + } else if ( viewNode.is( 'uiElement' ) ) { + // UIElement has it's own render() method. + // https://github.com/ckeditor/ckeditor5-engine/issues/799 + domElement = viewNode.render( domDocument ); + + if ( options.bind ) { + this.bindElements( domElement, viewNode ); + } } else { // Create DOM element. domElement = domDocument.createElement( viewNode.name ); diff --git a/src/view/uielement.js b/src/view/uielement.js index eece7ea1b..a1d0006a7 100644 --- a/src/view/uielement.js +++ b/src/view/uielement.js @@ -63,6 +63,16 @@ export default class UIElement extends Element { throw new CKEditorError( 'view-uielement-cannot-add: Cannot add child nodes to UIElement instance.' ); } } + + render( domDocument ) { + const domElement = domDocument.createElement( this.name ); + + for ( const key of this.getAttributeKeys() ) { + domElement.setAttribute( key, this.getAttribute( key ) ); + } + + return domElement; + } } // Returns `null` because block filler is not needed for UIElements. diff --git a/tests/view/domconverter/view-to-dom.js b/tests/view/domconverter/view-to-dom.js index d392e4ee6..81470f73f 100644 --- a/tests/view/domconverter/view-to-dom.js +++ b/tests/view/domconverter/view-to-dom.js @@ -8,6 +8,7 @@ import ViewText from '../../../src/view/text'; import ViewElement from '../../../src/view/element'; import ViewPosition from '../../../src/view/position'; +import ViewUIElement from '../../../src/view/uielement'; import ViewContainerElement from '../../../src/view/containerelement'; import ViewAttributeElement from '../../../src/view/attributeelement'; import DomConverter from '../../../src/view/domconverter'; @@ -179,6 +180,30 @@ describe( 'DomConverter', () => { expect( domTextNode.data ).to.equal( 'foo' ); } ); + it( 'should create DOM element from UIElement', () => { + const uiElement = new ViewUIElement( 'div' ); + const domElement = converter.viewToDom( uiElement, document ); + + expect( domElement ).to.be.instanceOf( HTMLElement ); + } ); + + it( 'should create DOM structure from UIElement', () => { + class MyUIElement extends ViewUIElement { + render( domDocument ) { + const root = super.render( domDocument ); + root.innerHTML = 'foo bar baz'; + + return root; + } + } + + const myElement = new MyUIElement( 'div' ); + const domElement = converter.viewToDom( myElement, document ); + + expect( domElement ).to.be.instanceOf( HTMLElement ); + expect( domElement.innerHTML ).to.equal( 'foo bar baz' ); + } ); + describe( 'it should convert spaces to  ', () => { it( 'at the beginning of each container element', () => { const viewDiv = new ViewContainerElement( 'div', null, [ From b87e4cbd708435526efad88e69d8544909bafda5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szymon=20Kup=C5=9B?= Date: Tue, 6 Jun 2017 23:38:31 +0200 Subject: [PATCH 03/15] DomConverter.domToView is returning null for items rendered from UIElement. --- src/view/domconverter.js | 39 ++++++++++++++++++-------- tests/view/domconverter/dom-to-view.js | 25 +++++++++++++++++ 2 files changed, 52 insertions(+), 12 deletions(-) diff --git a/src/view/domconverter.js b/src/view/domconverter.js index a6da5f9bd..4754cb530 100644 --- a/src/view/domconverter.js +++ b/src/view/domconverter.js @@ -195,32 +195,24 @@ export default class DomConverter { if ( viewNode.is( 'documentFragment' ) ) { // Create DOM document fragment. domElement = domDocument.createDocumentFragment(); - - if ( options.bind ) { - this.bindDocumentFragments( domElement, viewNode ); - } } else if ( viewNode.is( 'uiElement' ) ) { // UIElement has it's own render() method. // https://github.com/ckeditor/ckeditor5-engine/issues/799 domElement = viewNode.render( domDocument ); - - if ( options.bind ) { - this.bindElements( domElement, viewNode ); - } } else { // Create DOM element. domElement = domDocument.createElement( viewNode.name ); - if ( options.bind ) { - this.bindElements( domElement, viewNode ); - } - // Copy element's attributes. for ( const key of viewNode.getAttributeKeys() ) { domElement.setAttribute( key, viewNode.getAttribute( key ) ); } } + if ( options.bind ) { + this.bindDocumentFragments( domElement, viewNode ); + } + if ( options.withChildren || options.withChildren === undefined ) { for ( const child of this.viewChildrenToDom( viewNode, domDocument, options ) ) { domElement.appendChild( child ); @@ -361,6 +353,11 @@ export default class DomConverter { return null; } + // When node is inside UIElement it should not be converted to view. + if ( isInsideUIElement( domNode, this._domToViewMapping ) ) { + return null; + } + if ( this.isText( domNode ) ) { if ( isInlineFiller( domNode ) ) { return null; @@ -1097,3 +1094,21 @@ function forEachDomNodeAncestor( node, callback ) { node = node.parentNode; } } + +function isInsideUIElement( domNode, domToViewMapping ) { + const ancestors = getAncestors( domNode ); + + // Remove domNode from the list. + ancestors.pop(); + + while ( ancestors.length ) { + const domNode = ancestors.pop(); + const viewNode = domToViewMapping.get( domNode ); + + if ( viewNode && viewNode.is( 'uiElement' ) ) { + return true; + } + } + + return false; +} diff --git a/tests/view/domconverter/dom-to-view.js b/tests/view/domconverter/dom-to-view.js index a8c67fd4b..69c683242 100644 --- a/tests/view/domconverter/dom-to-view.js +++ b/tests/view/domconverter/dom-to-view.js @@ -8,6 +8,7 @@ import ViewElement from '../../../src/view/element'; import ViewRange from '../../../src/view/range'; import ViewSelection from '../../../src/view/selection'; +import ViewUIElement from '../../../src/view/uielement'; import DomConverter from '../../../src/view/domconverter'; import ViewDocumentFragment from '../../../src/view/documentfragment'; import { INLINE_FILLER, INLINE_FILLER_LENGTH, NBSP_FILLER } from '../../../src/view/filler'; @@ -175,6 +176,30 @@ describe( 'DomConverter', () => { expect( converter.domToView( comment ) ).to.be.null; } ); + it( 'should return null for nodes inside UIElement', () => { + class MyUIElement extends ViewUIElement { + render( doc ) { + const root = super.render( doc ); + root.innerHTML = '

foo

bar'; + + return root; + } + } + + const uiElement = new MyUIElement( 'div' ); + const domElement = converter.viewToDom( uiElement, document, { bind: true, withChildren: true } ); + + const domParagraph = domElement.childNodes[ 0 ]; + const domSpan = domElement.childNodes[ 1 ]; + + expect( domParagraph.innerHTML ).to.equal( 'foo' ); + expect( domSpan.innerHTML ).to.equal( 'bar' ); + expect( converter.domToView( domParagraph ) ).to.be.null; + expect( converter.domToView( domSpan ) ).to.be.null; + expect( converter.domToView( domParagraph.childNodes[ 0 ] ) ).to.be.null; + expect( converter.domToView( domSpan.childNodes[ 0 ] ) ).to.be.null; + } ); + describe( 'it should clear whitespaces', () => { it( 'at the beginning of block element', () => { const domDiv = createElement( document, 'div', {}, [ From bb4219777f179beb7a4b00ccf945f39489924c6b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szymon=20Kup=C5=9B?= Date: Wed, 7 Jun 2017 00:16:02 +0200 Subject: [PATCH 04/15] Added tests checking if rendered UIElement children are not bound to any view elements. --- tests/view/domconverter/view-to-dom.js | 36 +++++++++++++++++--------- 1 file changed, 24 insertions(+), 12 deletions(-) diff --git a/tests/view/domconverter/view-to-dom.js b/tests/view/domconverter/view-to-dom.js index 81470f73f..c83a55ba8 100644 --- a/tests/view/domconverter/view-to-dom.js +++ b/tests/view/domconverter/view-to-dom.js @@ -180,14 +180,7 @@ describe( 'DomConverter', () => { expect( domTextNode.data ).to.equal( 'foo' ); } ); - it( 'should create DOM element from UIElement', () => { - const uiElement = new ViewUIElement( 'div' ); - const domElement = converter.viewToDom( uiElement, document ); - - expect( domElement ).to.be.instanceOf( HTMLElement ); - } ); - - it( 'should create DOM structure from UIElement', () => { + describe( 'UIElement', () => { class MyUIElement extends ViewUIElement { render( domDocument ) { const root = super.render( domDocument ); @@ -197,11 +190,30 @@ describe( 'DomConverter', () => { } } - const myElement = new MyUIElement( 'div' ); - const domElement = converter.viewToDom( myElement, document ); + it( 'should create DOM element from UIElement', () => { + const uiElement = new ViewUIElement( 'div' ); + const domElement = converter.viewToDom( uiElement, document ); + + expect( domElement ).to.be.instanceOf( HTMLElement ); + } ); + + it( 'should create DOM structure from UIElement', () => { + const myElement = new MyUIElement( 'div' ); + const domElement = converter.viewToDom( myElement, document ); + + expect( domElement ).to.be.instanceOf( HTMLElement ); + expect( domElement.innerHTML ).to.equal( 'foo bar baz' ); + } ); - expect( domElement ).to.be.instanceOf( HTMLElement ); - expect( domElement.innerHTML ).to.equal( 'foo bar baz' ); + it( 'should not bind rendered elements', () => { + const myElement = new MyUIElement( 'div' ); + const domElement = converter.viewToDom( myElement, document, { bind: true } ); + const domSpan = domElement.childNodes[ 0 ]; + + expect( converter.getCorrespondingView( domElement ) ).to.equal( myElement ); + expect( converter.getCorrespondingView( domSpan ) ).to.be.falsy; + expect( converter.getCorrespondingView( domSpan.childNodes[ 0 ] ) ).to.be.falsy; + } ); } ); describe( 'it should convert spaces to  ', () => { From 32062b74abf7b3935960bbf07070df4113d68e3f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szymon=20Kup=C5=9B?= Date: Wed, 7 Jun 2017 13:38:42 +0200 Subject: [PATCH 05/15] Observers will not fire DOM events from inside of UIElements. --- src/view/domconverter.js | 46 ++++++++++++++----------- src/view/observer/domeventobserver.js | 5 ++- tests/view/observer/domeventobserver.js | 41 ++++++++++++++++++++++ 3 files changed, 71 insertions(+), 21 deletions(-) diff --git a/src/view/domconverter.js b/src/view/domconverter.js index 4754cb530..8afaf16e6 100644 --- a/src/view/domconverter.js +++ b/src/view/domconverter.js @@ -353,8 +353,8 @@ export default class DomConverter { return null; } - // When node is inside UIElement it should not be converted to view. - if ( isInsideUIElement( domNode, this._domToViewMapping ) ) { + // When node is inside UIElement it has no representation in the view. + if ( this.isDomInsideUIlement( domNode ) ) { return null; } @@ -837,6 +837,30 @@ export default class DomConverter { return backward; } + /** + * Returns true if `domNode` is placed inside {@link module:engine/view/uielement~UIElement}/ + * + * @param {Node} domNode + * @return {Boolean} + */ + isDomInsideUIlement( domNode ) { + const ancestors = getAncestors( domNode ); + + // Remove domNode from the list. + ancestors.pop(); + + while ( ancestors.length ) { + const domNode = ancestors.pop(); + const viewNode = this._domToViewMapping.get( domNode ); + + if ( viewNode && viewNode.is( 'uiElement' ) ) { + return true; + } + } + + return false; + } + /** * Takes text data from given {@link module:engine/view/text~Text#data} and processes it so it is correctly displayed in DOM. * @@ -1094,21 +1118,3 @@ function forEachDomNodeAncestor( node, callback ) { node = node.parentNode; } } - -function isInsideUIElement( domNode, domToViewMapping ) { - const ancestors = getAncestors( domNode ); - - // Remove domNode from the list. - ancestors.pop(); - - while ( ancestors.length ) { - const domNode = ancestors.pop(); - const viewNode = domToViewMapping.get( domNode ); - - if ( viewNode && viewNode.is( 'uiElement' ) ) { - return true; - } - } - - return false; -} diff --git a/src/view/observer/domeventobserver.js b/src/view/observer/domeventobserver.js index 471804724..a09039b92 100644 --- a/src/view/observer/domeventobserver.js +++ b/src/view/observer/domeventobserver.js @@ -75,7 +75,10 @@ export default class DomEventObserver extends Observer { types.forEach( type => { this.listenTo( domElement, type, ( eventInfo, domEvent ) => { - if ( this.isEnabled ) { + const domConverter = this.document.domConverter; + + // Block all events if observer is disabled and events from inside of UIElement. + if ( this.isEnabled && !domConverter.isDomInsideUIlement( domEvent.target ) ) { this.onDomEvent( domEvent ); } }, { useCapture: this.useCapture } ); diff --git a/tests/view/observer/domeventobserver.js b/tests/view/observer/domeventobserver.js index 1617d36b7..3ed35c0eb 100644 --- a/tests/view/observer/domeventobserver.js +++ b/tests/view/observer/domeventobserver.js @@ -8,6 +8,7 @@ import DomEventObserver from '../../../src/view/observer/domeventobserver'; import Observer from '../../../src/view/observer/observer'; import ViewDocument from '../../../src/view/document'; +import UIElement from '../../../src/view/uielement'; class ClickObserver extends DomEventObserver { constructor( document ) { @@ -153,6 +154,46 @@ describe( 'DomEventObserver', () => { childDomElement.dispatchEvent( domEvent ); } ); + describe( 'integration with UIElement', () => { + let domRoot, domEvent, evtSpy; + + class MyUIElement extends UIElement { + render( domDocument ) { + const root = super.render( domDocument ); + root.innerHTML = 'foo bar'; + + return root; + } + } + + beforeEach( () => { + domRoot = document.createElement( 'div' ); + const viewRoot = viewDocument.createRoot( domRoot, 'root' ); + const uiElement = new MyUIElement( 'p' ); + viewRoot.appendChildren( uiElement ); + viewDocument.render(); + + domEvent = new MouseEvent( 'click', { bubbles: true } ); + evtSpy = sinon.spy(); + viewDocument.addObserver( ClickObserver ); + viewDocument.on( 'click', evtSpy ); + } ); + + it( 'should fire events from UIElement itself', () => { + const domUiElement = domRoot.childNodes[ 0 ]; + domUiElement.dispatchEvent( domEvent ); + + expect( evtSpy.calledOnce ).to.be.true; + } ); + + it( 'should block events from inside of UIElement', () => { + const domUiElementChild = domRoot.querySelector( 'span' ); + domUiElementChild.dispatchEvent( domEvent ); + + expect( evtSpy.calledOnce ).to.be.false; + } ); + } ); + describe( 'fire', () => { it( 'should do nothing if observer is disabled', () => { const testObserver = new ClickObserver( viewDocument ); From 879795fd5fc9e4933b9dcf3164718d4c6f5769d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szymon=20Kup=C5=9B?= Date: Wed, 7 Jun 2017 16:57:58 +0200 Subject: [PATCH 06/15] Moved more UIElement handling to DOM converter. --- src/view/domconverter.js | 25 +++++++++++++++++-------- src/view/observer/domeventobserver.js | 5 +---- tests/view/domconverter/dom-to-view.js | 12 +++++------- tests/view/observer/domeventobserver.js | 16 +++++++++++----- 4 files changed, 34 insertions(+), 24 deletions(-) diff --git a/src/view/domconverter.js b/src/view/domconverter.js index 8afaf16e6..1f726f527 100644 --- a/src/view/domconverter.js +++ b/src/view/domconverter.js @@ -354,8 +354,10 @@ export default class DomConverter { } // When node is inside UIElement it has no representation in the view. - if ( this.isDomInsideUIlement( domNode ) ) { - return null; + const uiElement = this.getParentUIElement( domNode, this._domToViewMapping ); + + if ( uiElement ) { + return uiElement; } if ( this.isText( domNode ) ) { @@ -577,7 +579,7 @@ export default class DomConverter { * @returns {module:engine/view/element~Element|null} Corresponding element or `null` if no element was bound. */ getCorrespondingViewElement( domElement ) { - return this._domToViewMapping.get( domElement ); + return this.getParentUIElement( domElement ) || this._domToViewMapping.get( domElement ); } /** @@ -615,6 +617,12 @@ export default class DomConverter { return null; } + const uiElement = this.getParentUIElement( domText ); + + if ( uiElement ) { + return uiElement; + } + const previousSibling = domText.previousSibling; // Try to use previous sibling to find the corresponding text node. @@ -838,12 +846,13 @@ export default class DomConverter { } /** - * Returns true if `domNode` is placed inside {@link module:engine/view/uielement~UIElement}/ + * Returns parent {@link module:engine/view/uielement~UIElement} for provided DOM node. Returns null if there is no + * parent UIElement. * * @param {Node} domNode - * @return {Boolean} + * @return {module:engine/view/uielement~UIElement|null} */ - isDomInsideUIlement( domNode ) { + getParentUIElement( domNode ) { const ancestors = getAncestors( domNode ); // Remove domNode from the list. @@ -854,11 +863,11 @@ export default class DomConverter { const viewNode = this._domToViewMapping.get( domNode ); if ( viewNode && viewNode.is( 'uiElement' ) ) { - return true; + return viewNode; } } - return false; + return null; } /** diff --git a/src/view/observer/domeventobserver.js b/src/view/observer/domeventobserver.js index a09039b92..471804724 100644 --- a/src/view/observer/domeventobserver.js +++ b/src/view/observer/domeventobserver.js @@ -75,10 +75,7 @@ export default class DomEventObserver extends Observer { types.forEach( type => { this.listenTo( domElement, type, ( eventInfo, domEvent ) => { - const domConverter = this.document.domConverter; - - // Block all events if observer is disabled and events from inside of UIElement. - if ( this.isEnabled && !domConverter.isDomInsideUIlement( domEvent.target ) ) { + if ( this.isEnabled ) { this.onDomEvent( domEvent ); } }, { useCapture: this.useCapture } ); diff --git a/tests/view/domconverter/dom-to-view.js b/tests/view/domconverter/dom-to-view.js index 69c683242..b78095d05 100644 --- a/tests/view/domconverter/dom-to-view.js +++ b/tests/view/domconverter/dom-to-view.js @@ -176,7 +176,7 @@ describe( 'DomConverter', () => { expect( converter.domToView( comment ) ).to.be.null; } ); - it( 'should return null for nodes inside UIElement', () => { + it( 'should return UIElement for nodes inside', () => { class MyUIElement extends ViewUIElement { render( doc ) { const root = super.render( doc ); @@ -192,12 +192,10 @@ describe( 'DomConverter', () => { const domParagraph = domElement.childNodes[ 0 ]; const domSpan = domElement.childNodes[ 1 ]; - expect( domParagraph.innerHTML ).to.equal( 'foo' ); - expect( domSpan.innerHTML ).to.equal( 'bar' ); - expect( converter.domToView( domParagraph ) ).to.be.null; - expect( converter.domToView( domSpan ) ).to.be.null; - expect( converter.domToView( domParagraph.childNodes[ 0 ] ) ).to.be.null; - expect( converter.domToView( domSpan.childNodes[ 0 ] ) ).to.be.null; + expect( converter.domToView( domParagraph ) ).to.equal( uiElement ); + expect( converter.domToView( domSpan ) ).to.be.equal( uiElement ); + expect( converter.domToView( domParagraph.childNodes[ 0 ] ) ).equal( uiElement ); + expect( converter.domToView( domSpan.childNodes[ 0 ] ) ).equal( uiElement ); } ); describe( 'it should clear whitespaces', () => { diff --git a/tests/view/observer/domeventobserver.js b/tests/view/observer/domeventobserver.js index 3ed35c0eb..c9c391d60 100644 --- a/tests/view/observer/domeventobserver.js +++ b/tests/view/observer/domeventobserver.js @@ -155,7 +155,7 @@ describe( 'DomEventObserver', () => { } ); describe( 'integration with UIElement', () => { - let domRoot, domEvent, evtSpy; + let domRoot, domEvent, evtSpy, uiElement; class MyUIElement extends UIElement { render( domDocument ) { @@ -169,7 +169,7 @@ describe( 'DomEventObserver', () => { beforeEach( () => { domRoot = document.createElement( 'div' ); const viewRoot = viewDocument.createRoot( domRoot, 'root' ); - const uiElement = new MyUIElement( 'p' ); + uiElement = new MyUIElement( 'p' ); viewRoot.appendChildren( uiElement ); viewDocument.render(); @@ -183,14 +183,20 @@ describe( 'DomEventObserver', () => { const domUiElement = domRoot.childNodes[ 0 ]; domUiElement.dispatchEvent( domEvent ); - expect( evtSpy.calledOnce ).to.be.true; + const data = evtSpy.args[ 0 ][ 1 ]; + + sinon.assert.calledOnce( evtSpy ); + expect( data.target ).to.equal( uiElement ); } ); - it( 'should block events from inside of UIElement', () => { + it( 'events from inside of UIElement should target UIElement', () => { const domUiElementChild = domRoot.querySelector( 'span' ); domUiElementChild.dispatchEvent( domEvent ); - expect( evtSpy.calledOnce ).to.be.false; + const data = evtSpy.args[ 0 ][ 1 ]; + + sinon.assert.calledOnce( evtSpy ); + expect( data.target ).to.equal( uiElement ); } ); } ); From a9906dc88e024511a181a7117f02bea06476a98e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szymon=20Kup=C5=9B?= Date: Thu, 8 Jun 2017 17:28:22 +0200 Subject: [PATCH 07/15] More tests and changes in DOM converter in order to handle UIElements. --- src/view/domconverter.js | 26 +++++-- tests/view/domconverter/dom-to-view.js | 23 ------ tests/view/domconverter/uielement.js | 100 +++++++++++++++++++++++++ tests/view/domconverter/view-to-dom.js | 37 --------- tests/view/manual/uielement.html | 1 + tests/view/manual/uielement.js | 54 +++++++++++++ tests/view/manual/uielement.md | 1 + 7 files changed, 177 insertions(+), 65 deletions(-) create mode 100644 tests/view/domconverter/uielement.js create mode 100644 tests/view/manual/uielement.html create mode 100644 tests/view/manual/uielement.js create mode 100644 tests/view/manual/uielement.md diff --git a/src/view/domconverter.js b/src/view/domconverter.js index 1f726f527..51f37ba20 100644 --- a/src/view/domconverter.js +++ b/src/view/domconverter.js @@ -195,24 +195,34 @@ export default class DomConverter { if ( viewNode.is( 'documentFragment' ) ) { // Create DOM document fragment. domElement = domDocument.createDocumentFragment(); + + if ( options.bind ) { + this.bindDocumentFragments( domElement, viewNode ); + } } else if ( viewNode.is( 'uiElement' ) ) { // UIElement has it's own render() method. // https://github.com/ckeditor/ckeditor5-engine/issues/799 domElement = viewNode.render( domDocument ); + + if ( options.bind ) { + this.bindElements( domElement, viewNode ); + } + + return domElement; } else { // Create DOM element. domElement = domDocument.createElement( viewNode.name ); + if ( options.bind ) { + this.bindElements( domElement, viewNode ); + } + // Copy element's attributes. for ( const key of viewNode.getAttributeKeys() ) { domElement.setAttribute( key, viewNode.getAttribute( key ) ); } } - if ( options.bind ) { - this.bindDocumentFragments( domElement, viewNode ); - } - if ( options.withChildren || options.withChildren === undefined ) { for ( const child of this.viewChildrenToDom( viewNode, domDocument, options ) ) { domElement.appendChild( child ); @@ -353,7 +363,7 @@ export default class DomConverter { return null; } - // When node is inside UIElement it has no representation in the view. + // When node is inside UIElement return that UIElement as it's view representation. const uiElement = this.getParentUIElement( domNode, this._domToViewMapping ); if ( uiElement ) { @@ -506,6 +516,12 @@ export default class DomConverter { return this.domPositionToView( domParent.parentNode, indexOf( domParent ) ); } + // If position inside UIElement - return position before. + const viewElement = this.getCorrespondingViewElement( domParent ); + if ( viewElement && viewElement.is( 'uiElement' ) ) { + return ViewPosition.createBefore( viewElement ); + } + if ( this.isText( domParent ) ) { if ( isInlineFiller( domParent ) ) { return this.domPositionToView( domParent.parentNode, indexOf( domParent ) ); diff --git a/tests/view/domconverter/dom-to-view.js b/tests/view/domconverter/dom-to-view.js index b78095d05..a8c67fd4b 100644 --- a/tests/view/domconverter/dom-to-view.js +++ b/tests/view/domconverter/dom-to-view.js @@ -8,7 +8,6 @@ import ViewElement from '../../../src/view/element'; import ViewRange from '../../../src/view/range'; import ViewSelection from '../../../src/view/selection'; -import ViewUIElement from '../../../src/view/uielement'; import DomConverter from '../../../src/view/domconverter'; import ViewDocumentFragment from '../../../src/view/documentfragment'; import { INLINE_FILLER, INLINE_FILLER_LENGTH, NBSP_FILLER } from '../../../src/view/filler'; @@ -176,28 +175,6 @@ describe( 'DomConverter', () => { expect( converter.domToView( comment ) ).to.be.null; } ); - it( 'should return UIElement for nodes inside', () => { - class MyUIElement extends ViewUIElement { - render( doc ) { - const root = super.render( doc ); - root.innerHTML = '

foo

bar'; - - return root; - } - } - - const uiElement = new MyUIElement( 'div' ); - const domElement = converter.viewToDom( uiElement, document, { bind: true, withChildren: true } ); - - const domParagraph = domElement.childNodes[ 0 ]; - const domSpan = domElement.childNodes[ 1 ]; - - expect( converter.domToView( domParagraph ) ).to.equal( uiElement ); - expect( converter.domToView( domSpan ) ).to.be.equal( uiElement ); - expect( converter.domToView( domParagraph.childNodes[ 0 ] ) ).equal( uiElement ); - expect( converter.domToView( domSpan.childNodes[ 0 ] ) ).equal( uiElement ); - } ); - describe( 'it should clear whitespaces', () => { it( 'at the beginning of block element', () => { const domDiv = createElement( document, 'div', {}, [ diff --git a/tests/view/domconverter/uielement.js b/tests/view/domconverter/uielement.js new file mode 100644 index 000000000..111cff1a5 --- /dev/null +++ b/tests/view/domconverter/uielement.js @@ -0,0 +1,100 @@ +/** + * @license Copyright (c) 2003-2017, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md. + */ + +/* globals document, HTMLElement */ + +import ViewUIElement from '../../../src/view/uielement'; +import ViewContainer from '../../../src/view/containerelement'; +import DomConverter from '../../../src/view/domconverter'; + +describe( 'DOMConverter UIElement integration', () => { + let converter; + + class MyUIElement extends ViewUIElement { + render( domDocument ) { + const root = super.render( domDocument ); + root.innerHTML = '

foo bar

'; + + return root; + } + } + + beforeEach( () => { + converter = new DomConverter(); + } ); + + describe( 'viewToDom', () => { + it( 'should create DOM element from UIElement', () => { + const uiElement = new ViewUIElement( 'div' ); + const domElement = converter.viewToDom( uiElement, document ); + + expect( domElement ).to.be.instanceOf( HTMLElement ); + } ); + + it( 'should create DOM structure from UIElement', () => { + const myElement = new MyUIElement( 'div' ); + const domElement = converter.viewToDom( myElement, document ); + + expect( domElement ).to.be.instanceOf( HTMLElement ); + expect( domElement.innerHTML ).to.equal( '

foo bar

' ); + } ); + + it( 'should not bind rendered elements', () => { + const myElement = new MyUIElement( 'div' ); + const domElement = converter.viewToDom( myElement, document, { bind: true } ); + const domSpan = domElement.childNodes[ 0 ]; + + expect( converter.getCorrespondingView( domElement ) ).to.equal( myElement ); + expect( converter.getCorrespondingView( domSpan ) ).to.be.falsy; + expect( converter.getCorrespondingView( domSpan.childNodes[ 0 ] ) ).to.be.falsy; + } ); + } ); + + describe( 'domToView', () => { + it( 'should return UIElement itself', () => { + const uiElement = new MyUIElement( 'div' ); + const domElement = converter.viewToDom( uiElement, document, { bind: true } ); + + expect( converter.domToView( domElement ) ).to.equal( uiElement ); + } ); + + it( 'should return UIElement for nodes inside', () => { + const uiElement = new MyUIElement( 'div' ); + const domElement = converter.viewToDom( uiElement, document, { bind: true } ); + + const domParagraph = domElement.childNodes[ 0 ]; + const domSpan = domParagraph.childNodes[ 0 ]; + + expect( converter.domToView( domParagraph ) ).to.equal( uiElement ); + expect( converter.domToView( domSpan ) ).to.be.equal( uiElement ); + expect( converter.domToView( domParagraph.childNodes[ 0 ] ) ).equal( uiElement ); + expect( converter.domToView( domSpan.childNodes[ 0 ] ) ).equal( uiElement ); + } ); + } ); + + describe( 'domPositionToView', () => { + it( 'should convert position inside UIElement to position before it', () => { + const uiElement = new MyUIElement( 'h1' ); + const container = new ViewContainer( 'div', null, [ new ViewContainer( 'div' ), uiElement ] ); + const domContainer = converter.viewToDom( container, document, { bind: true } ); + + const viewPosition = converter.domPositionToView( domContainer.childNodes[ 1 ], 0 ); + + expect( viewPosition.parent ).to.equal( container ); + expect( viewPosition.offset ).to.equal( 1 ); + } ); + + it( 'should convert position inside UIElement children to position before UIElement', () => { + const uiElement = new MyUIElement( 'h1' ); + const container = new ViewContainer( 'div', null, [ new ViewContainer( 'div' ), uiElement ] ); + const domContainer = converter.viewToDom( container, document, { bind: true } ); + + const viewPosition = converter.domPositionToView( domContainer.childNodes[ 1 ].childNodes[ 0 ], 1 ); + + expect( viewPosition.parent ).to.equal( container ); + expect( viewPosition.offset ).to.equal( 1 ); + } ); + } ); +} ); diff --git a/tests/view/domconverter/view-to-dom.js b/tests/view/domconverter/view-to-dom.js index c83a55ba8..d392e4ee6 100644 --- a/tests/view/domconverter/view-to-dom.js +++ b/tests/view/domconverter/view-to-dom.js @@ -8,7 +8,6 @@ import ViewText from '../../../src/view/text'; import ViewElement from '../../../src/view/element'; import ViewPosition from '../../../src/view/position'; -import ViewUIElement from '../../../src/view/uielement'; import ViewContainerElement from '../../../src/view/containerelement'; import ViewAttributeElement from '../../../src/view/attributeelement'; import DomConverter from '../../../src/view/domconverter'; @@ -180,42 +179,6 @@ describe( 'DomConverter', () => { expect( domTextNode.data ).to.equal( 'foo' ); } ); - describe( 'UIElement', () => { - class MyUIElement extends ViewUIElement { - render( domDocument ) { - const root = super.render( domDocument ); - root.innerHTML = 'foo bar baz'; - - return root; - } - } - - it( 'should create DOM element from UIElement', () => { - const uiElement = new ViewUIElement( 'div' ); - const domElement = converter.viewToDom( uiElement, document ); - - expect( domElement ).to.be.instanceOf( HTMLElement ); - } ); - - it( 'should create DOM structure from UIElement', () => { - const myElement = new MyUIElement( 'div' ); - const domElement = converter.viewToDom( myElement, document ); - - expect( domElement ).to.be.instanceOf( HTMLElement ); - expect( domElement.innerHTML ).to.equal( 'foo bar baz' ); - } ); - - it( 'should not bind rendered elements', () => { - const myElement = new MyUIElement( 'div' ); - const domElement = converter.viewToDom( myElement, document, { bind: true } ); - const domSpan = domElement.childNodes[ 0 ]; - - expect( converter.getCorrespondingView( domElement ) ).to.equal( myElement ); - expect( converter.getCorrespondingView( domSpan ) ).to.be.falsy; - expect( converter.getCorrespondingView( domSpan.childNodes[ 0 ] ) ).to.be.falsy; - } ); - } ); - describe( 'it should convert spaces to  ', () => { it( 'at the beginning of each container element', () => { const viewDiv = new ViewContainerElement( 'div', null, [ diff --git a/tests/view/manual/uielement.html b/tests/view/manual/uielement.html new file mode 100644 index 000000000..fb550a0b6 --- /dev/null +++ b/tests/view/manual/uielement.html @@ -0,0 +1 @@ +

This is editor instance. Text after UIElement

diff --git a/tests/view/manual/uielement.js b/tests/view/manual/uielement.js new file mode 100644 index 000000000..aaf2e44e2 --- /dev/null +++ b/tests/view/manual/uielement.js @@ -0,0 +1,54 @@ +/** + * @license Copyright (c) 2003-2017, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md. + */ + +/* global document */ + +import ClassicEditor from '@ckeditor/ckeditor5-editor-classic/src/classic'; +import Enter from '@ckeditor/ckeditor5-enter/src/enter'; +import Typing from '@ckeditor/ckeditor5-typing/src/typing'; +import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph'; +import Undo from '@ckeditor/ckeditor5-undo/src/undo'; +import Plugin from '@ckeditor/ckeditor5-core/src/plugin'; + +import buildModelConverter from '../../../src/conversion/buildmodelconverter'; +import buildViewConverter from '../../../src/conversion/buildviewconverter'; +import UIElement from '../../../src/view/uielement'; + +class MyUIElement extends UIElement { + render( domDocument ) { + const root = super.render( domDocument ); + + root.setAttribute( 'contenteditable', 'false' ); + root.style.backgroundColor = 'red'; + root.innerHTML = 'This is UIElement'; + + return root; + } +} + +class UIElementTestPlugin extends Plugin { + init() { + const editor = this.editor; + const document = editor.document; + const editing = editor.editing; + const data = editor.data; + const schema = document.schema; + + schema.registerItem( 'span', '$inline' ); + + buildModelConverter().for( data.modelToView, editing.modelToView ) + .fromElement( 'span' ) + .toElement( () => new MyUIElement( 'span' ) ); + + buildViewConverter().for( data.viewToModel ) + .fromElement( 'span' ) + .toElement( 'span' ); + } +} + +ClassicEditor.create( document.querySelector( '#editor' ), { + plugins: [ Enter, Typing, Paragraph, Undo, UIElementTestPlugin ], + toolbar: [ 'undo', 'redo' ] +} ); diff --git a/tests/view/manual/uielement.md b/tests/view/manual/uielement.md new file mode 100644 index 000000000..38f23c662 --- /dev/null +++ b/tests/view/manual/uielement.md @@ -0,0 +1 @@ +### UIElement handling From c3ea6f5774cfb4bf27cee0306e01a8a6fecf56c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szymon=20Kup=C5=9B?= Date: Thu, 8 Jun 2017 18:27:45 +0200 Subject: [PATCH 08/15] Preventing mutations from inside of UIElement. --- src/view/observer/mutationobserver.js | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/view/observer/mutationobserver.js b/src/view/observer/mutationobserver.js index b7734b743..b59161053 100644 --- a/src/view/observer/mutationobserver.js +++ b/src/view/observer/mutationobserver.js @@ -149,6 +149,11 @@ export default class MutationObserver extends Observer { if ( mutation.type === 'childList' ) { const element = domConverter.getCorrespondingViewElement( mutation.target ); + // Prevent mutation from UIElements. + if ( element && element.is( 'uiElement' ) ) { + continue; + } + if ( element && !this._isBogusBrMutation( mutation ) ) { mutatedElements.add( element ); } @@ -157,6 +162,13 @@ export default class MutationObserver extends Observer { // Handle `characterData` mutations later, when we have the full list of nodes which changed structure. for ( const mutation of domMutations ) { + const element = domConverter.getCorrespondingViewElement( mutation.target ); + + // Prevent mutation from UIElements. + if ( element && element.is( 'uiElement' ) ) { + continue; + } + if ( mutation.type === 'characterData' ) { const text = domConverter.getCorrespondingViewText( mutation.target ); From 0e7757d99577c967f1743d3ddf81acc1bbf0f411 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szymon=20Kup=C5=9B?= Date: Fri, 9 Jun 2017 11:16:28 +0200 Subject: [PATCH 09/15] Modification to UIElement manual test. --- tests/view/manual/close.png | Bin 0 -> 368 bytes tests/view/manual/uielement.html | 13 +++++- tests/view/manual/uielement.js | 75 ++++++++++++++++++++++++++++--- 3 files changed, 81 insertions(+), 7 deletions(-) create mode 100644 tests/view/manual/close.png diff --git a/tests/view/manual/close.png b/tests/view/manual/close.png new file mode 100644 index 0000000000000000000000000000000000000000..0a7fb2d7b8499caf7d4195013efd2481b9aed247 GIT binary patch literal 368 zcmV-$0gwKPP)BbT8?;C82Ic@l7TE-{cx2~Br{kls z(Q4nO!jBne{_}rdPyhC%X_{nNMv)qG!qF6yMifO!9LE%aZ)&U&j^@WC3`2^*j5-Jc zW`Z$U(Hw+9;QM}!Azj$@5dca-5Z*A~%$r!>GXu}_NZXu^dJmrc{;!oA?S?!<&Q zd^(+SmOnNe$Kjc<;q&>NkNrz`JVBKtos_LCW?s#;)pnKjF{ZP{&W znnnQNm&s&CmgPc{Bp4X-`BI9a5bV_tn&+iAJ|M4ahZaSVVE+Rdhra+tpm~jW#!b8c O0000

This is editor instance. Text after UIElement

+ +
This is editor instance
diff --git a/tests/view/manual/uielement.js b/tests/view/manual/uielement.js index aaf2e44e2..0e5e1f385 100644 --- a/tests/view/manual/uielement.js +++ b/tests/view/manual/uielement.js @@ -15,6 +15,59 @@ import Plugin from '@ckeditor/ckeditor5-core/src/plugin'; import buildModelConverter from '../../../src/conversion/buildmodelconverter'; import buildViewConverter from '../../../src/conversion/buildviewconverter'; import UIElement from '../../../src/view/uielement'; +import ContainerElement from '../../../src/view/containerelement'; + +class MyUIElement extends UIElement { + render( domDocument ) { + const root = super.render( domDocument ); + + root.setAttribute( 'contenteditable', 'false' ); + root.classList.add( 'ui-element' ); + root.innerHTML = 'UIElement with some structure inside '; + + return root; + } +} + +class UIElementTestPlugin extends Plugin { + init() { + const editor = this.editor; + const document = editor.document; + const editing = editor.editing; + const data = editor.data; + const schema = document.schema; + + schema.registerItem( 'figure', '$block' ); + + buildModelConverter().for( data.modelToView, editing.modelToView ) + .fromElement( 'figure' ) + .toElement( () => { + return new ContainerElement( 'figure', null, new MyUIElement( 'div' ) ); + } ); + + buildViewConverter().for( data.viewToModel ) + .fromElement( 'figure' ) + .toElement( 'figure' ); + } +} + +ClassicEditor.create( document.querySelector( '#editor' ), { + plugins: [ Enter, Typing, Paragraph, Undo, UIElementTestPlugin ], + toolbar: [ 'undo', 'redo' ] +} ); + +/* +import ClassicEditor from '@ckeditor/ckeditor5-editor-classic/src/classic'; +import Enter from '@ckeditor/ckeditor5-enter/src/enter'; +import Typing from '@ckeditor/ckeditor5-typing/src/typing'; +import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph'; +import Undo from '@ckeditor/ckeditor5-undo/src/undo'; +import Plugin from '@ckeditor/ckeditor5-core/src/plugin'; + +import buildModelConverter from '../../../src/conversion/buildmodelconverter'; +import buildViewConverter from '../../../src/conversion/buildviewconverter'; +import UIElement from '../../../src/view/uielement'; +import AttributeElement from '../../../src/view/attributeelement'; class MyUIElement extends UIElement { render( domDocument ) { @@ -22,7 +75,9 @@ class MyUIElement extends UIElement { root.setAttribute( 'contenteditable', 'false' ); root.style.backgroundColor = 'red'; - root.innerHTML = 'This is UIElement'; + root.style.display = 'inline-block'; + root.style.width = '5px'; + root.style.height = '5px'; return root; } @@ -36,15 +91,21 @@ class UIElementTestPlugin extends Plugin { const data = editor.data; const schema = document.schema; - schema.registerItem( 'span', '$inline' ); + // Allow bold attribute on all inline nodes. + editor.document.schema.allow( { name: '$inline', attributes: [ 'strong' ], inside: '$block' } ); + // Build converter from model to view for data and editing pipelines. buildModelConverter().for( data.modelToView, editing.modelToView ) - .fromElement( 'span' ) - .toElement( () => new MyUIElement( 'span' ) ); + .fromAttribute( 'strong' ) + .toElement( data => { + return new AttributeElement( 'strong', null, [ new MyUIElement( 'span' ) ] ); + // return new UIElement( 'strong' ); + } ); + // Build converter from view to model for data pipeline. buildViewConverter().for( data.viewToModel ) - .fromElement( 'span' ) - .toElement( 'span' ); + .fromElement( 'strong' ) + .toAttribute( 'strong', true ); } } @@ -52,3 +113,5 @@ ClassicEditor.create( document.querySelector( '#editor' ), { plugins: [ Enter, Typing, Paragraph, Undo, UIElementTestPlugin ], toolbar: [ 'undo', 'redo' ] } ); + + */ From 1d8f2b4c73d853d36dc240b30e8ef65d51f0432e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szymon=20Kup=C5=9B?= Date: Mon, 12 Jun 2017 10:11:42 +0200 Subject: [PATCH 10/15] UIElement.render() tests. --- src/view/uielement.js | 7 +++++++ tests/view/uielement.js | 24 ++++++++++++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/src/view/uielement.js b/src/view/uielement.js index a1d0006a7..8f0d293e3 100644 --- a/src/view/uielement.js +++ b/src/view/uielement.js @@ -64,6 +64,13 @@ export default class UIElement extends Element { } } + /** + * Renders this {@link module:engine/view/uielement~UIElement} to DOM. This method is called by + * {@link engine/view/domconverter~DomConverter}. + * + * @param {Document} domDocument + * @return {HTMLElement} + */ render( domDocument ) { const domElement = domDocument.createElement( this.name ); diff --git a/tests/view/uielement.js b/tests/view/uielement.js index 2b7f61d64..dcbcd162c 100644 --- a/tests/view/uielement.js +++ b/tests/view/uielement.js @@ -3,6 +3,8 @@ * For licensing, see LICENSE.md. */ +/* global document, HTMLElement */ + import UIElement from '../../src/view/uielement'; import Element from '../../src/view/element'; import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror'; @@ -99,4 +101,26 @@ describe( 'UIElement', () => { expect( uiElement.getFillerOffset() ).to.null; } ); } ); + + describe( 'render()', () => { + let domElement; + + beforeEach( () => { + domElement = uiElement.render( document ); + } ); + + it( 'should return DOM element', () => { + expect( domElement ).to.be.instanceOf( HTMLElement ); + } ); + + it( 'should use element name', () => { + expect( domElement.tagName.toLowerCase() ).to.equal( uiElement.name ); + } ); + + it( 'should render attributes', () => { + for ( const key of uiElement.getAttributeKeys() ) { + expect( domElement.getAttribute( key ) ).to.equal( uiElement.getAttribute( key ) ); + } + } ); + } ); } ); From 254cc80d28d03af125c0176252bf7c804e5fb6d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szymon=20Kup=C5=9B?= Date: Mon, 12 Jun 2017 12:03:19 +0200 Subject: [PATCH 11/15] More tests to changes made in DomConverter. Added tests to unbindDomElement method. --- src/view/domconverter.js | 2 +- tests/view/domconverter/binding.js | 50 ++++++++++++++++++++++++++++ tests/view/domconverter/uielement.js | 47 +++++++++++++++++++++++++- 3 files changed, 97 insertions(+), 2 deletions(-) diff --git a/src/view/domconverter.js b/src/view/domconverter.js index 51f37ba20..04fb9ec89 100644 --- a/src/view/domconverter.js +++ b/src/view/domconverter.js @@ -516,7 +516,7 @@ export default class DomConverter { return this.domPositionToView( domParent.parentNode, indexOf( domParent ) ); } - // If position inside UIElement - return position before. + // If position is somewhere inside UIElement - return position before that element. const viewElement = this.getCorrespondingViewElement( domParent ); if ( viewElement && viewElement.is( 'uiElement' ) ) { return ViewPosition.createBefore( viewElement ); diff --git a/tests/view/domconverter/binding.js b/tests/view/domconverter/binding.js index 7d12a567c..58ec4a443 100644 --- a/tests/view/domconverter/binding.js +++ b/tests/view/domconverter/binding.js @@ -365,4 +365,54 @@ describe( 'DomConverter', () => { expect( bindSelection.isEqual( selectionCopy ) ).to.be.true; } ); } ); + + describe( 'unbindDomElement', () => { + it( 'should unbind elements', () => { + const domElement = document.createElement( 'p' ); + const viewElement = new ViewElement( 'p' ); + + converter.bindElements( domElement, viewElement ); + + expect( converter.getCorrespondingView( domElement ) ).to.equal( viewElement ); + expect( converter.getCorrespondingDom( viewElement ) ).to.equal( domElement ); + + converter.unbindDomElement( domElement ); + + expect( converter.getCorrespondingView( domElement ) ).to.be.undefined; + expect( converter.getCorrespondingDom( viewElement ) ).to.be.undefined; + } ); + + it( 'should unbind element\'s child nodes', () => { + const domElement = document.createElement( 'p' ); + const domChild = document.createElement( 'span' ); + domElement.appendChild( domChild ); + + const viewElement = new ViewElement( 'p' ); + const viewChild = new ViewElement( 'span' ); + + converter.bindElements( domElement, viewElement ); + converter.bindElements( domChild, viewChild ); + + expect( converter.getCorrespondingView( domChild ) ).to.equal( viewChild ); + expect( converter.getCorrespondingDom( viewChild ) ).to.equal( domChild ); + + converter.unbindDomElement( domElement ); + + expect( converter.getCorrespondingView( domChild ) ).to.be.undefined; + expect( converter.getCorrespondingDom( viewChild ) ).to.be.undefined; + } ); + + it( 'should do nothing if there are no elements bind', () => { + const domElement = document.createElement( 'p' ); + const viewElement = new ViewElement( 'p' ); + + expect( converter.getCorrespondingView( domElement ) ).to.be.undefined; + expect( converter.getCorrespondingDom( viewElement ) ).to.be.undefined; + + converter.unbindDomElement( domElement ); + + expect( converter.getCorrespondingView( domElement ) ).to.be.undefined; + expect( converter.getCorrespondingDom( viewElement ) ).to.be.undefined; + } ); + } ); } ); diff --git a/tests/view/domconverter/uielement.js b/tests/view/domconverter/uielement.js index 111cff1a5..93d225bfe 100644 --- a/tests/view/domconverter/uielement.js +++ b/tests/view/domconverter/uielement.js @@ -41,7 +41,7 @@ describe( 'DOMConverter UIElement integration', () => { expect( domElement.innerHTML ).to.equal( '

foo bar

' ); } ); - it( 'should not bind rendered elements', () => { + it( 'should bind only UIElement not child elements', () => { const myElement = new MyUIElement( 'div' ); const domElement = converter.viewToDom( myElement, document, { bind: true } ); const domSpan = domElement.childNodes[ 0 ]; @@ -97,4 +97,49 @@ describe( 'DOMConverter UIElement integration', () => { expect( viewPosition.offset ).to.equal( 1 ); } ); } ); + + describe( 'getCorrespondingViewElement', () => { + it( 'should return UIElement for DOM elements inside', () => { + const myElement = new MyUIElement( 'div' ); + const domElement = converter.viewToDom( myElement, document, { bind: true } ); + + expect( converter.getCorrespondingViewElement( domElement ) ).to.equal( myElement ); + + const domParagraph = domElement.childNodes[ 0 ]; + expect( converter.getCorrespondingViewElement( domParagraph ) ).to.equal( myElement ); + + const domSpan = domParagraph.childNodes[ 0 ]; + expect( converter.getCorrespondingViewElement( domSpan ) ).to.equal( myElement ); + } ); + } ); + + describe( 'getCorrespondingViewText', () => { + it( 'should return UIElement for DOM text inside', () => { + const myElement = new MyUIElement( 'div' ); + const domElement = converter.viewToDom( myElement, document, { bind: true } ); + + const domText = domElement.querySelector( 'span' ).childNodes[ 0 ]; + expect( converter.getCorrespondingViewText( domText ) ).to.equal( myElement ); + } ); + } ); + + describe( 'getParentUIElement', () => { + it( 'should return UIElement for DOM children', () => { + const uiElement = new MyUIElement( 'div' ); + const domElement = converter.viewToDom( uiElement, document, { bind: true } ); + + const domParagraph = domElement.childNodes[ 0 ]; + const domSpan = domParagraph.childNodes[ 0 ]; + + expect( converter.getParentUIElement( domParagraph ) ).to.equal( uiElement ); + expect( converter.getParentUIElement( domSpan ) ).to.equal( uiElement ); + } ); + + it( 'should return null for element itself', () => { + const uiElement = new MyUIElement( 'div' ); + const domElement = converter.viewToDom( uiElement, document, { bind: true } ); + + expect( converter.getParentUIElement( domElement ) ).to.be.null; + } ); + } ); } ); From 7f66d7c2e35d075f6cba158e71df2d4b25c4b32c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szymon=20Kup=C5=9B?= Date: Mon, 12 Jun 2017 12:29:24 +0200 Subject: [PATCH 12/15] Updated DomConverter docs. --- src/view/domconverter.js | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/view/domconverter.js b/src/view/domconverter.js index 04fb9ec89..90235c2a6 100644 --- a/src/view/domconverter.js +++ b/src/view/domconverter.js @@ -349,6 +349,7 @@ export default class DomConverter { * Converts DOM to view. For all text nodes, not bound elements and document fragments new items will * be created. For bound elements and document fragments function will return corresponding items. For * {@link module:engine/view/filler fillers} `null` will be returned. + * For all DOM elements rendered by {@link module:engine/view/uielement~UIElement} that UIElement will be returned. * * @param {Node|DocumentFragment} domNode DOM node or document fragment to transform. * @param {Object} [options] Conversion options. @@ -505,6 +506,9 @@ export default class DomConverter { * If the position is inside a {@link module:engine/view/filler filler} which has no corresponding view node, * position of the filler will be converted and returned. * + * If the position is inside DOM element rendered by {@link module:engine/view/uielement~UIElement} + * that position will be converted to view position before that UIElement. + * * If structures are too different and it is not possible to find corresponding position then `null` will be returned. * * @param {Node} domParent DOM position parent. @@ -590,6 +594,7 @@ export default class DomConverter { /** * Gets corresponding view element. Returns element if an view element was * {@link module:engine/view/domconverter~DomConverter#bindElements bound} to the given DOM element or `null` otherwise. + * For all DOM elements rendered by {@link module:engine/view/uielement~UIElement} that UIElement will be returned. * * @param {HTMLElement} domElement DOM element. * @returns {module:engine/view/element~Element|null} Corresponding element or `null` if no element was bound. @@ -620,6 +625,8 @@ export default class DomConverter { * If this is a first child in the parent and the parent is a {@link module:engine/view/domconverter~DomConverter#bindElements bound} * element, it is used to find the corresponding text node. * + * For all text nodes rendered by {@link module:engine/view/uielement~UIElement} that UIElement will be returned. + * * Otherwise `null` is returned. * * Note that for the block or inline {@link module:engine/view/filler filler} this method returns `null`. @@ -633,6 +640,7 @@ export default class DomConverter { return null; } + // If DOM text was rendered by UIElement - return that element. const uiElement = this.getParentUIElement( domText ); if ( uiElement ) { From bd2fde6e16b60820dd5d6401abedb33c3d88c525 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szymon=20Kup=C5=9B?= Date: Mon, 12 Jun 2017 15:36:15 +0200 Subject: [PATCH 13/15] Added tests to MutationObserver that checks integration with UIElement. --- src/view/observer/mutationobserver.js | 4 +-- tests/view/observer/mutationobserver.js | 36 +++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 2 deletions(-) diff --git a/src/view/observer/mutationobserver.js b/src/view/observer/mutationobserver.js index b59161053..659d36c1f 100644 --- a/src/view/observer/mutationobserver.js +++ b/src/view/observer/mutationobserver.js @@ -149,7 +149,7 @@ export default class MutationObserver extends Observer { if ( mutation.type === 'childList' ) { const element = domConverter.getCorrespondingViewElement( mutation.target ); - // Prevent mutation from UIElements. + // Do not collect mutations from UIElements. if ( element && element.is( 'uiElement' ) ) { continue; } @@ -164,7 +164,7 @@ export default class MutationObserver extends Observer { for ( const mutation of domMutations ) { const element = domConverter.getCorrespondingViewElement( mutation.target ); - // Prevent mutation from UIElements. + // Do not collect mutations from UIElements. if ( element && element.is( 'uiElement' ) ) { continue; } diff --git a/tests/view/observer/mutationobserver.js b/tests/view/observer/mutationobserver.js index d5d33eb91..cec8c271d 100644 --- a/tests/view/observer/mutationobserver.js +++ b/tests/view/observer/mutationobserver.js @@ -7,6 +7,7 @@ import ViewDocument from '../../../src/view/document'; import MutationObserver from '../../../src/view/observer/mutationobserver'; +import UIElement from '../../../src/view/uielement'; import { parse } from '../../../src/dev-utils/view'; describe( 'MutationObserver', () => { @@ -346,6 +347,41 @@ describe( 'MutationObserver', () => { expect( lastMutations[ 0 ].oldChildren.length ).to.equal( 1 ); } ); + describe( 'UIElement integration', () => { + class MyUIElement extends UIElement { + render( domDocument ) { + const root = super.render( domDocument ); + root.innerHTML = 'foo bar'; + + return root; + } + } + + beforeEach( () => { + const uiElement = new MyUIElement( 'div' ); + viewRoot.appendChildren( uiElement ); + + viewDocument.render(); + } ); + + it( 'should not collect text mutations from UIElement', () => { + domEditor.childNodes[ 2 ].childNodes[ 0 ].data = 'foom'; + + mutationObserver.flush(); + + expect( lastMutations.length ).to.equal( 0 ); + } ); + + it( 'should not collect child mutations from UIElement', () => { + const span = document.createElement( 'span' ); + domEditor.childNodes[ 2 ].appendChild( span ); + + mutationObserver.flush(); + + expect( lastMutations.length ).to.equal( 0 ); + } ); + } ); + function expectDomEditorNotToChange() { expect( domEditor.childNodes.length ).to.equal( 2 ); expect( domEditor.childNodes[ 0 ].tagName ).to.equal( 'P' ); From 41911131e29cf7c9e5b922e6f26eebc8325e2d6f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szymon=20Kup=C5=9B?= Date: Tue, 13 Jun 2017 00:27:22 +0200 Subject: [PATCH 14/15] Updated UIElement manual test. --- tests/view/manual/close.png | Bin 368 -> 0 bytes tests/view/manual/uielement.html | 20 ++++++-- tests/view/manual/uielement.js | 85 +++---------------------------- tests/view/manual/uielement.md | 4 ++ 4 files changed, 27 insertions(+), 82 deletions(-) delete mode 100644 tests/view/manual/close.png diff --git a/tests/view/manual/close.png b/tests/view/manual/close.png deleted file mode 100644 index 0a7fb2d7b8499caf7d4195013efd2481b9aed247..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 368 zcmV-$0gwKPP)BbT8?;C82Ic@l7TE-{cx2~Br{kls z(Q4nO!jBne{_}rdPyhC%X_{nNMv)qG!qF6yMifO!9LE%aZ)&U&j^@WC3`2^*j5-Jc zW`Z$U(Hw+9;QM}!Azj$@5dca-5Z*A~%$r!>GXu}_NZXu^dJmrc{;!oA?S?!<&Q zd^(+SmOnNe$Kjc<;q&>NkNrz`JVBKtos_LCW?s#;)pnKjF{ZP{&W znnnQNm&s&CmgPc{Bp4X-`BI9a5bV_tn&+iAJ|M4ahZaSVVE+Rdhra+tpm~jW#!b8c O0000 + div#container { + width: 600px; + } + figure { margin-left: 0; margin-right: 0; } .ui-element { - font-size: 12px; - background-color: #dadada; + font-size: 8px; + border-top: 1px solid #dadada; + font-family: sans-serif; + } + + .ui-element span { + background-color: #7a7a7a; + color: white; + padding: 1px 5px; } -
This is editor instance
+
+

Lorem ipsum dolor sit amet, consectetur adipiscing elit. Nulla sapien diam, pretium at laoreet eget, commodo bibendum urna. Praesent in dolor leo. Vivamus sagittis ligula tempor laoreet tincidunt. Praesent hendrerit in tortor ac sollicitudin. Curabitur eleifend blandit ultrices. Aliquam euismod ut lectus blandit faucibus. Quisque ut sapien euismod, interdum ex ac, tristique neque. In a volutpat risus. Vivamus blandit est et sodales imperdiet. Duis et metus nisi. Proin viverra pellentesque velit vel egestas. Etiam sed ornare orci. Pellentesque id quam finibus, semper ante aliquam, rhoncus tortor. Vestibulum eget porta velit. Aenean id nisl scelerisque, pulvinar ante nec, eleifend sapien.

Lorem ipsum dolor sit amet, consectetur adipiscing elit. Nulla sapien diam, pretium at laoreet eget, commodo bibendum urna. Praesent in dolor leo. Vivamus sagittis ligula tempor laoreet tincidunt. Praesent hendrerit in tortor ac sollicitudin. Curabitur eleifend blandit ultrices. Aliquam euismod ut lectus blandit faucibus. Quisque ut sapien euismod, interdum ex ac, tristique neque. In a volutpat risus. Vivamus blandit est et sodales imperdiet. Duis et metus nisi. Proin viverra pellentesque velit vel egestas. Etiam sed ornare orci. Pellentesque id quam finibus, semper ante aliquam, rhoncus tortor. Vestibulum eget porta velit. Aenean id nisl scelerisque, pulvinar ante nec, eleifend sapien.

+
+ diff --git a/tests/view/manual/uielement.js b/tests/view/manual/uielement.js index 0e5e1f385..ec0a6f866 100644 --- a/tests/view/manual/uielement.js +++ b/tests/view/manual/uielement.js @@ -11,11 +11,7 @@ import Typing from '@ckeditor/ckeditor5-typing/src/typing'; import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph'; import Undo from '@ckeditor/ckeditor5-undo/src/undo'; import Plugin from '@ckeditor/ckeditor5-core/src/plugin'; - -import buildModelConverter from '../../../src/conversion/buildmodelconverter'; -import buildViewConverter from '../../../src/conversion/buildviewconverter'; import UIElement from '../../../src/view/uielement'; -import ContainerElement from '../../../src/view/containerelement'; class MyUIElement extends UIElement { render( domDocument ) { @@ -23,7 +19,7 @@ class MyUIElement extends UIElement { root.setAttribute( 'contenteditable', 'false' ); root.classList.add( 'ui-element' ); - root.innerHTML = 'UIElement with some structure inside '; + root.innerHTML = 'END OF PARAGRAPH'; return root; } @@ -32,22 +28,13 @@ class MyUIElement extends UIElement { class UIElementTestPlugin extends Plugin { init() { const editor = this.editor; - const document = editor.document; const editing = editor.editing; - const data = editor.data; - const schema = document.schema; - - schema.registerItem( 'figure', '$block' ); - - buildModelConverter().for( data.modelToView, editing.modelToView ) - .fromElement( 'figure' ) - .toElement( () => { - return new ContainerElement( 'figure', null, new MyUIElement( 'div' ) ); - } ); - buildViewConverter().for( data.viewToModel ) - .fromElement( 'figure' ) - .toElement( 'figure' ); + // Add some UIElement to each paragraph. + editing.modelToView.on( 'insert:paragraph', ( evt, data, consumable, conversionApi ) => { + const viewP = conversionApi.mapper.toViewElement( data.item ); + viewP.appendChildren( new MyUIElement( 'div' ) ); + }, { priority: 'lowest' } ); } } @@ -55,63 +42,3 @@ ClassicEditor.create( document.querySelector( '#editor' ), { plugins: [ Enter, Typing, Paragraph, Undo, UIElementTestPlugin ], toolbar: [ 'undo', 'redo' ] } ); - -/* -import ClassicEditor from '@ckeditor/ckeditor5-editor-classic/src/classic'; -import Enter from '@ckeditor/ckeditor5-enter/src/enter'; -import Typing from '@ckeditor/ckeditor5-typing/src/typing'; -import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph'; -import Undo from '@ckeditor/ckeditor5-undo/src/undo'; -import Plugin from '@ckeditor/ckeditor5-core/src/plugin'; - -import buildModelConverter from '../../../src/conversion/buildmodelconverter'; -import buildViewConverter from '../../../src/conversion/buildviewconverter'; -import UIElement from '../../../src/view/uielement'; -import AttributeElement from '../../../src/view/attributeelement'; - -class MyUIElement extends UIElement { - render( domDocument ) { - const root = super.render( domDocument ); - - root.setAttribute( 'contenteditable', 'false' ); - root.style.backgroundColor = 'red'; - root.style.display = 'inline-block'; - root.style.width = '5px'; - root.style.height = '5px'; - - return root; - } -} - -class UIElementTestPlugin extends Plugin { - init() { - const editor = this.editor; - const document = editor.document; - const editing = editor.editing; - const data = editor.data; - const schema = document.schema; - - // Allow bold attribute on all inline nodes. - editor.document.schema.allow( { name: '$inline', attributes: [ 'strong' ], inside: '$block' } ); - - // Build converter from model to view for data and editing pipelines. - buildModelConverter().for( data.modelToView, editing.modelToView ) - .fromAttribute( 'strong' ) - .toElement( data => { - return new AttributeElement( 'strong', null, [ new MyUIElement( 'span' ) ] ); - // return new UIElement( 'strong' ); - } ); - - // Build converter from view to model for data pipeline. - buildViewConverter().for( data.viewToModel ) - .fromElement( 'strong' ) - .toAttribute( 'strong', true ); - } -} - -ClassicEditor.create( document.querySelector( '#editor' ), { - plugins: [ Enter, Typing, Paragraph, Undo, UIElementTestPlugin ], - toolbar: [ 'undo', 'redo' ] -} ); - - */ diff --git a/tests/view/manual/uielement.md b/tests/view/manual/uielement.md index 38f23c662..a290c7803 100644 --- a/tests/view/manual/uielement.md +++ b/tests/view/manual/uielement.md @@ -1 +1,5 @@ ### UIElement handling + +1. Each paragraph should have UIElement at it's bottom showing "END OF PARAGRAPH". +1. UIElement should not block typing or prevent regular editor usage. +1. When paragraph is split or new paragraph is created - new UIElement should be created too. From e6e1eaa3bd1d94615f8bbce7ea74647098a9aa05 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Szymon=20Kup=C5=9B?= Date: Tue, 13 Jun 2017 11:31:03 +0200 Subject: [PATCH 15/15] Added UIElement handling to getFillerOffset methods in Container and Attribute elements. --- src/view/attributeelement.js | 14 +++++++++++--- src/view/containerelement.js | 2 +- tests/view/attributeelement.js | 15 +++++++++++++++ tests/view/containerelement.js | 4 ++++ tests/view/observer/domeventobserver.js | 2 +- 5 files changed, 32 insertions(+), 5 deletions(-) diff --git a/src/view/attributeelement.js b/src/view/attributeelement.js index 353db8596..360e8fd84 100644 --- a/src/view/attributeelement.js +++ b/src/view/attributeelement.js @@ -101,7 +101,7 @@ AttributeElement.DEFAULT_PRIORITY = DEFAULT_PRIORITY; // @returns {Number|null} Block filler offset or `null` if block filler is not needed. function getFillerOffset() { // foo does not need filler. - if ( this.childCount ) { + if ( nonUiChildrenCount( this ) ) { return null; } @@ -109,16 +109,24 @@ function getFillerOffset() { //

needs filler ->


while ( element && element.is( 'attributeElement' ) ) { - if ( element.childCount > 1 ) { + if ( nonUiChildrenCount( element ) > 1 ) { return null; } element = element.parent; } - if ( !element || element.childCount > 1 ) { + if ( !element || nonUiChildrenCount( element ) > 1 ) { return null; } return 0; } + +// Returns total count of children that are not {@link module:engine/view/uielement~UIElement UIElements}. +// +// @param {module:engine/view/element~Element} element +// @return {Number} +function nonUiChildrenCount( element ) { + return Array.from( element.getChildren() ).filter( element => !element.is( 'uiElement' ) ).length; +} diff --git a/src/view/containerelement.js b/src/view/containerelement.js index 757a3af0c..5bec26f3c 100644 --- a/src/view/containerelement.js +++ b/src/view/containerelement.js @@ -78,5 +78,5 @@ export default class ContainerElement extends Element { // // @returns {Number|null} Block filler offset or `null` if block filler is not needed. function getFillerOffset() { - return this.childCount === 0 ? 0 : null; + return Array.from( this.getChildren() ).some( element => !element.is( 'uiElement' ) ) ? null : 0; } diff --git a/tests/view/attributeelement.js b/tests/view/attributeelement.js index 54f46e42e..99cbdcadb 100644 --- a/tests/view/attributeelement.js +++ b/tests/view/attributeelement.js @@ -137,5 +137,20 @@ describe( 'AttributeElement', () => { expect( attribute.getFillerOffset() ).to.be.null; } ); + + it( 'should return position 0 if it is the only nested element in the container and has UIElement inside', () => { + const { selection } = parse( + '[]' ); + const attribute = selection.getFirstPosition().parent; + + expect( attribute.getFillerOffset() ).to.equals( 0 ); + } ); + + it( 'should return 0 if there is no parent container element and has UIElement inside', () => { + const { selection } = parse( '[]' ); + const attribute = selection.getFirstPosition().parent; + + expect( attribute.getFillerOffset() ).to.equal( 0 ); + } ); } ); } ); diff --git a/tests/view/containerelement.js b/tests/view/containerelement.js index 13717a477..926889847 100644 --- a/tests/view/containerelement.js +++ b/tests/view/containerelement.js @@ -52,6 +52,10 @@ describe( 'ContainerElement', () => { expect( parse( '' ).getFillerOffset() ).to.equals( 0 ); } ); + it( 'should return position 0 if element contains only ui elements', () => { + expect( parse( '' ).getFillerOffset() ).to.equals( 0 ); + } ); + it( 'should return null if element is not empty', () => { expect( parse( 'foo' ).getFillerOffset() ).to.be.null; } ); diff --git a/tests/view/observer/domeventobserver.js b/tests/view/observer/domeventobserver.js index c9c391d60..60112ce86 100644 --- a/tests/view/observer/domeventobserver.js +++ b/tests/view/observer/domeventobserver.js @@ -180,7 +180,7 @@ describe( 'DomEventObserver', () => { } ); it( 'should fire events from UIElement itself', () => { - const domUiElement = domRoot.childNodes[ 0 ]; + const domUiElement = domRoot.querySelector( 'p' ); domUiElement.dispatchEvent( domEvent ); const data = evtSpy.args[ 0 ][ 1 ];