Merge pull request #1255 from ckeditor/t/1254

Other: UIElement custom `render()` method can be now provided without using inheritance. Closes #1254.

BREAKING CHANGE: Introduced new method of creating custom UIElements.

Author: Piotr Jasiun

src/view/uielement.js

@@ -68,11 +68,31 @@ export default class UIElement extends Element {
 	/**
 	 * Renders this {@link module:engine/view/uielement~UIElement} to DOM. This method is called by
 	 * {@link module:engine/view/domconverter~DomConverter}.
+	 * Do not use inheritance to create custom rendering method, replace `render()` method instead:
+	 *
+	 *		const myUIElement = new UIElement( 'span' );
+	 *		myUIElement.render = function( domDocument ) {
+	 *			const domElement = this.toDomElement( domDocument );
+	 *			domElement.innerHTML = '<b>this is ui element</b>';
+	 *
+	 *			return domElement;
+	 *		};
 	 *
 	 * @param {Document} domDocument
 	 * @return {HTMLElement}
 	 */
 	render( domDocument ) {
+		return this.toDomElement( domDocument );
+	}
+
+	/**
+	 * Creates DOM element based on this view UIElement.
+	 * Note that each time this method is called new DOM element is created.
+	 *
+	 * @param {Document} domDocument
+	 * @returns {HTMLElement}
+	 */
+	toDomElement( domDocument ) {
 		const domElement = domDocument.createElement( this.name );
 
 		for ( const key of this.getAttributeKeys() ) {

tests/view/document/jumpoveruielement.js

@@ -20,13 +20,17 @@ import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils';
 describe( 'Document', () => {
 	let viewDocument, domRoot, domSelection, viewRoot, foo, bar, ui, ui2;
 
-	class MyUIElement extends UIElement {
-		render( domDocument ) {
-			const element = super.render( domDocument );
-			element.innerText = this.contents;
+	function createUIElement( name, contents ) {
+		const element = new UIElement( name );
 
-			return element;
-		}
+		element.render = function( domDocument ) {
+			const domElement = this.toDomElement( domDocument );
+			domElement.innerText = contents;
+
+			return domElement;
+		};
+
+		return element;
 	}
 
 	beforeEach( () => {
@@ -46,11 +50,8 @@ describe( 'Document', () => {
 
 		foo = new ViewText( 'foo' );
 		bar = new ViewText( 'bar' );
-		ui = new MyUIElement( 'span' );
-		ui.contents = 'xxx';
-
-		ui2 = new MyUIElement( 'span' );
-		ui2.contents = 'yyy';
+		ui = createUIElement( 'span', 'xxx' );
+		ui2 = createUIElement( 'span', 'yyy' );
 	} );
 
 	afterEach( () => {

tests/view/domconverter/uielement.js

@@ -12,13 +12,17 @@ import DomConverter from '../../../src/view/domconverter';
 describe( 'DOMConverter UIElement integration', () => {
 	let converter;
 
-	class MyUIElement extends ViewUIElement {
-		render( domDocument ) {
-			const root = super.render( domDocument );
+	function createUIElement( name ) {
+		const element = new ViewUIElement( name );
+
+		element.render = function( domDocument ) {
+			const root = this.toDomElement( domDocument );
 			root.innerHTML = '<p><span>foo</span> bar</p>';
 
 			return root;
-		}
+		};
+
+		return element;
 	}
 
 	beforeEach( () => {
@@ -34,15 +38,15 @@ describe( 'DOMConverter UIElement integration', () => {
 		} );
 
 		it( 'should create DOM structure from UIElement', () => {
-			const myElement = new MyUIElement( 'div' );
+			const myElement = createUIElement( 'div' );
 			const domElement = converter.viewToDom( myElement, document );
 
 			expect( domElement ).to.be.instanceOf( HTMLElement );
 			expect( domElement.innerHTML ).to.equal( '<p><span>foo</span> bar</p>' );
 		} );
 
 		it( 'should create DOM structure that all is mapped to single UIElement', () => {
-			const myElement = new MyUIElement( 'div' );
+			const myElement = createUIElement( 'div' );
 			const domElement = converter.viewToDom( myElement, document, { bind: true } );
 			const domParagraph = domElement.childNodes[ 0 ];
 
@@ -54,14 +58,14 @@ describe( 'DOMConverter UIElement integration', () => {
 
 	describe( 'domToView()', () => {
 		it( 'should return UIElement itself', () => {
-			const uiElement = new MyUIElement( 'div' );
+			const uiElement = createUIElement( '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 uiElement = createUIElement( 'div' );
 			const domElement = converter.viewToDom( uiElement, document, { bind: true } );
 
 			const domParagraph = domElement.childNodes[ 0 ];
@@ -76,7 +80,7 @@ describe( 'DOMConverter UIElement integration', () => {
 
 	describe( 'domPositionToView()', () => {
 		it( 'should convert position inside UIElement to position before it', () => {
-			const uiElement = new MyUIElement( 'h1' );
+			const uiElement = createUIElement( 'h1' );
 			const container = new ViewContainer( 'div', null, [ new ViewContainer( 'div' ), uiElement ] );
 			const domContainer = converter.viewToDom( container, document, { bind: true } );
 
@@ -87,7 +91,7 @@ describe( 'DOMConverter UIElement integration', () => {
 		} );
 
 		it( 'should convert position inside UIElement children to position before UIElement', () => {
-			const uiElement = new MyUIElement( 'h1' );
+			const uiElement = createUIElement( 'h1' );
 			const container = new ViewContainer( 'div', null, [ new ViewContainer( 'div' ), uiElement ] );
 			const domContainer = converter.viewToDom( container, document, { bind: true } );
 
@@ -100,7 +104,7 @@ describe( 'DOMConverter UIElement integration', () => {
 
 	describe( 'mapDomToView()', () => {
 		it( 'should return UIElement for DOM elements inside', () => {
-			const myElement = new MyUIElement( 'div' );
+			const myElement = createUIElement( 'div' );
 			const domElement = converter.viewToDom( myElement, document, { bind: true } );
 
 			expect( converter.mapDomToView( domElement ) ).to.equal( myElement );
@@ -115,7 +119,7 @@ describe( 'DOMConverter UIElement integration', () => {
 
 	describe( 'findCorrespondingViewText()', () => {
 		it( 'should return UIElement for DOM text inside', () => {
-			const myElement = new MyUIElement( 'div' );
+			const myElement = createUIElement( 'div' );
 			const domElement = converter.viewToDom( myElement, document, { bind: true } );
 
 			const domText = domElement.querySelector( 'span' ).childNodes[ 0 ];
@@ -125,7 +129,7 @@ describe( 'DOMConverter UIElement integration', () => {
 
 	describe( 'getParentUIElement()', () => {
 		it( 'should return UIElement for DOM children', () => {
-			const uiElement = new MyUIElement( 'div' );
+			const uiElement = createUIElement( 'div' );
 			const domElement = converter.viewToDom( uiElement, document, { bind: true } );
 
 			const domParagraph = domElement.childNodes[ 0 ];
@@ -136,7 +140,7 @@ describe( 'DOMConverter UIElement integration', () => {
 		} );
 
 		it( 'should return null for element itself', () => {
-			const uiElement = new MyUIElement( 'div' );
+			const uiElement = createUIElement( 'div' );
 			const domElement = converter.viewToDom( uiElement, document, { bind: true } );
 
 			expect( converter.getParentUIElement( domElement ) ).to.be.null;

tests/view/manual/uielement.js

@@ -17,26 +17,32 @@ import UIElement from '../../../src/view/uielement';
 import Position from '../../../src/view/position';
 import writer from '../../../src/view/writer';
 
-class EndingUIElement extends UIElement {
-	render( domDocument ) {
-		const root = super.render( domDocument );
+function createEndingUIElement() {
+	const element = new UIElement( 'span' );
 
+	element.render = function( domDocument ) {
+		const root = this.toDomElement( domDocument );
 		root.classList.add( 'ui-element' );
 		root.innerHTML = 'END OF PARAGRAPH';
 
 		return root;
-	}
+	};
+
+	return element;
 }
 
-class MiddleUIElement extends UIElement {
-	render( domDocument ) {
-		const root = super.render( domDocument );
+function createMiddleUIElement() {
+	const element = new UIElement( 'span' );
 
+	element.render = function( domDocument ) {
+		const root = this.toDomElement( domDocument );
 		root.classList.add( 'ui-element' );
 		root.innerHTML = 'X';
 
 		return root;
-	}
+	};
+
+	return element;
 }
 
 class UIElementTestPlugin extends Plugin {
@@ -47,7 +53,7 @@ class UIElementTestPlugin extends Plugin {
 		// 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 EndingUIElement( 'span' ) );
+			viewP.appendChildren( createEndingUIElement() );
 		}, { priority: 'lowest' } );
 	}
 }
@@ -65,10 +71,10 @@ ClassicEditor
 		const viewText1 = viewRoot.getChild( 0 ).getChild( 0 );
 		const viewText2 = viewRoot.getChild( 1 ).getChild( 0 );
 
-		writer.insert( new Position( viewText1, 20 ), new MiddleUIElement( 'span' ) );
-		writer.insert( new Position( viewText1, 20 ), new MiddleUIElement( 'span' ) );
-		writer.insert( new Position( viewText2, 0 ), new MiddleUIElement( 'span' ) );
-		writer.insert( new Position( viewText2, 6 ), new MiddleUIElement( 'span' ) );
+		writer.insert( new Position( viewText1, 20 ), createMiddleUIElement() );
+		writer.insert( new Position( viewText1, 20 ), createMiddleUIElement() );
+		writer.insert( new Position( viewText2, 0 ), createMiddleUIElement() );
+		writer.insert( new Position( viewText2, 6 ), createMiddleUIElement() );
 
 		editor.editing.view.render();
 	} )

tests/view/observer/domeventobserver.js

@@ -163,20 +163,24 @@ describe( 'DomEventObserver', () => {
 	describe( 'integration with UIElement', () => {
 		let domRoot, domEvent, evtSpy, uiElement;
 
-		class MyUIElement extends UIElement {
-			render( domDocument ) {
-				const root = super.render( domDocument );
+		function createUIElement( name ) {
+			const element = new UIElement( name );
+
+			element.render = function( domDocument ) {
+				const root = this.toDomElement( domDocument );
 				root.innerHTML = '<span>foo bar</span>';
 
 				return root;
-			}
+			};
+
+			return element;
 		}
 
 		beforeEach( () => {
 			domRoot = document.createElement( 'div' );
 			const viewRoot = createViewRoot( viewDocument );
 			viewDocument.attachDomRoot( domRoot );
-			uiElement = new MyUIElement( 'p' );
+			uiElement = createUIElement( 'p' );
 			viewRoot.appendChildren( uiElement );
 			viewDocument.render();
 

tests/view/observer/mutationobserver.js

@@ -420,17 +420,21 @@ describe( 'MutationObserver', () => {
 	} );
 
 	describe( 'UIElement integration', () => {
-		class MyUIElement extends UIElement {
-			render( domDocument ) {
-				const root = super.render( domDocument );
+		function createUIElement( name ) {
+			const element = new UIElement( name );
+
+			element.render = function( domDocument ) {
+				const root = this.toDomElement( domDocument );
 				root.innerHTML = 'foo bar';
 
 				return root;
-			}
+			};
+
+			return element;
 		}
 
 		beforeEach( () => {
-			const uiElement = new MyUIElement( 'div' );
+			const uiElement = createUIElement( 'div' );
 			viewRoot.appendChildren( uiElement );
 
 			viewDocument.render();

tests/view/uielement.js

@@ -122,5 +122,27 @@ describe( 'UIElement', () => {
 				expect( domElement.getAttribute( key ) ).to.equal( uiElement.getAttribute( key ) );
 			}
 		} );
+
+		it( 'should allow to change render() method', () => {
+			uiElement.render = function( domDocument ) {
+				return domDocument.createElement( 'b' );
+			};
+
+			expect( uiElement.render( document ).tagName.toLowerCase() ).to.equal( 'b' );
+		} );
+
+		it( 'should allow to add new elements inside', () => {
+			uiElement.render = function( domDocument ) {
+				const element = this.toDomElement( domDocument );
+				const text = domDocument.createTextNode( 'foo bar' );
+				element.appendChild( text );
+
+				return element;
+			};
+
+			const rendered = uiElement.render( document );
+			expect( rendered.tagName.toLowerCase() ).to.equal( 'span' );
+			expect( rendered.textContent ).to.equal( 'foo bar' );
+		} );
 	} );
 } );