Bladeren bron

Use change event instead of direct marking elements to render.

Piotr Jasiun 10 jaren geleden
bovenliggende
commit
feda0159dd

+ 4 - 4
packages/ckeditor5-engine/src/treeview/element.js

@@ -66,13 +66,13 @@ export default class Element extends Node {
 	}
 
 	setAttr( key, value ) {
-		this.markToSync( 'ATTRIBUTES_NEED_UPDATE' );
+		this._fireChange( 'ATTRIBUTES', this );
 
 		this._attrs.set( key, value );
 	}
 
 	insertChildren( index, nodes ) {
-		this.markToSync( 'CHILDREN_NEED_UPDATE' );
+		this._fireChange( 'CHILDREN', this );
 
 		if ( !utils.isIterable( nodes ) ) {
 			nodes = [ nodes ];
@@ -87,13 +87,13 @@ export default class Element extends Node {
 	}
 
 	removeAttr( key ) {
-		this.markToSync( 'ATTRIBUTES_NEED_UPDATE' );
+		this._fireChange( 'ATTRIBUTES', this );
 
 		return this._attrs.delete( key );
 	}
 
 	removeChildren( index, number ) {
-		this.markToSync( 'CHILDREN_NEED_UPDATE' );
+		this._fireChange( 'CHILDREN', this );
 
 		for ( let i = index; i < index + number; i++ ) {
 			this._children[ i ].parent = null;

+ 8 - 8
packages/ckeditor5-engine/src/treeview/node.js

@@ -6,6 +6,8 @@
 'use strict';
 
 import CKEditorError from '../ckeditorerror.js';
+import EmitterMixin from '../emittermixin.js';
+import objectUtils from '../lib/lodash/object.js';
 
 export default class Node {
 	constructor() {
@@ -52,15 +54,13 @@ export default class Node {
 		}
 	}
 
-	markToSync( type ) {
-		const treeView = this.getTreeView();
+	_fireChange( type, node ) {
+		this.fire( 'change', type, node );
 
-		// If element is not attached to the Tree view it is a child of the detached subtree and will be rendered anyway with this
-		// subtree.
-		if ( !treeView || !treeView.renderer ) {
-			return;
+		if ( this.parent ) {
+			this.parent._fireChange( type, node );
 		}
-
-		treeView.renderer.markToSync( this, type );
 	}
 }
+
+objectUtils.extend( Node.prototype, EmitterMixin );

+ 4 - 4
packages/ckeditor5-engine/src/treeview/renderer.js

@@ -17,8 +17,8 @@ export default class Renderer {
 		this.markedTexts = new Set();
 	}
 
-	markToSync( node, type ) {
-		if ( type === 'TEXT_NEEDS_UPDATE' ) {
+	markToSync( type, node ) {
+		if ( type === 'TEXT' ) {
 			if ( this.converter.getCorespondingDom( node.parent ) ) {
 				this.markedTexts.add( node );
 			}
@@ -28,9 +28,9 @@ export default class Renderer {
 				return;
 			}
 
-			if ( type === 'ATTRIBUTES_NEED_UPDATE' ) {
+			if ( type === 'ATTRIBUTES' ) {
 				this.markedAttrs.add( node );
-			} else if ( type === 'CHILDREN_NEED_UPDATE' ) {
+			} else if ( type === 'CHILDREN' ) {
 				this.markedChildren.add( node );
 			} else {
 				/**

+ 1 - 1
packages/ckeditor5-engine/src/treeview/text.js

@@ -19,7 +19,7 @@ export default class Text extends Node {
 	}
 
 	setText( text ) {
-		this.markToSync( 'TEXT_NEEDS_UPDATE' );
+		this._fireChange( 'TEXT', this );
 
 		this._text = text;
 	}

+ 5 - 1
packages/ckeditor5-engine/src/treeview/treeview.js

@@ -31,7 +31,11 @@ export default class TreeView {
 		this.converter.bindElements( domRoot, this.viewRoot );
 
 		this.renderer = new Renderer( this.converter );
-		this.renderer.markToSync( this.viewRoot, 'CHILDREN_NEED_UPDATE' );
+		this.renderer.markToSync( 'CHILDREN', this.viewRoot );
+
+		this.viewRoot.on( 'change', ( evt, type, node ) => {
+			this.renderer.markToSync( type, node );
+		} );
 	}
 
 	addObserver( observer ) {

+ 0 - 86
packages/ckeditor5-engine/tests/treeview/marktosync.js

@@ -1,86 +0,0 @@
-/**
- * @license Copyright (c) 2003-2015, CKSource - Frederico Knabben. All rights reserved.
- * For licensing, see LICENSE.md.
- */
-
-/* bender-tags: treeview */
-
-'use strict';
-
-import ViewText from '/ckeditor5/core/treeview/text.js';
-import ViewElement from '/ckeditor5/core/treeview/element.js';
-import RootElement from '/ckeditor5/core/treeview/rootelement.js';
-
-describe( 'Node.markToSync', () => {
-	let root, text, img;
-	let markToSyncSpy;
-
-	before( () => {
-		markToSyncSpy = sinon.spy();
-	} );
-
-	beforeEach( () => {
-		text = new ViewText( 'foo' );
-		img = new ViewElement( 'img' );
-		img.setAttr( 'src', 'img.png' );
-
-		root = new RootElement( 'p', { renderer: { markToSync: markToSyncSpy } } );
-		root.appendChildren( [ text, img ] );
-
-		markToSyncSpy.reset();
-	} );
-
-	describe( 'setAttr', () => {
-		it( 'should mark attibutes to sync', () => {
-			img.setAttr( 'width', 100 );
-
-			sinon.assert.calledOnce( markToSyncSpy );
-			sinon.assert.calledWith( markToSyncSpy, img, 'ATTRIBUTES_NEED_UPDATE' );
-		} );
-	} );
-
-	describe( 'removeAttr', () => {
-		it( 'should mark attibutes to sync', () => {
-			img.removeAttr( 'src' );
-
-			sinon.assert.calledOnce( markToSyncSpy );
-			sinon.assert.calledWith( markToSyncSpy, img, 'ATTRIBUTES_NEED_UPDATE' );
-		} );
-	} );
-
-	describe( 'insertChildren', () => {
-		it( 'should mark children to sync', () => {
-			root.insertChildren( 1, new ViewElement( 'img' ) );
-
-			sinon.assert.calledOnce( markToSyncSpy );
-			sinon.assert.calledWith( markToSyncSpy, root, 'CHILDREN_NEED_UPDATE' );
-		} );
-	} );
-
-	describe( 'appendChildren', () => {
-		it( 'should mark children to sync', () => {
-			root.appendChildren( new ViewElement( 'img' ) );
-
-			sinon.assert.calledOnce( markToSyncSpy );
-			sinon.assert.calledWith( markToSyncSpy, root, 'CHILDREN_NEED_UPDATE' );
-		} );
-	} );
-
-	describe( 'removeChildren', () => {
-		it( 'should mark children to sync', () => {
-			root.removeChildren( 1, 1 );
-
-			sinon.assert.calledOnce( markToSyncSpy );
-			sinon.assert.calledWith( markToSyncSpy, root, 'CHILDREN_NEED_UPDATE' );
-		} );
-	} );
-
-	describe( 'removeChildren', () => {
-		it( 'should mark children to sync', () => {
-			text.setText( 'bar' );
-
-			sinon.assert.calledOnce( markToSyncSpy );
-			sinon.assert.calledWith( markToSyncSpy, text, 'TEXT_NEEDS_UPDATE' );
-		} );
-	} );
-} );

+ 98 - 0
packages/ckeditor5-engine/tests/treeview/node.js

@@ -104,4 +104,102 @@ describe( 'Node', () => {
 			expect( child.getTreeView() ).to.equal( tvMock );
 		} );
 	} );
+
+	describe( 'change event', () => {
+		let root, text, img;
+		let rootChangeSpy;
+
+		before( () => {
+			rootChangeSpy = sinon.spy();
+		} );
+
+		beforeEach( () => {
+			text = new Text( 'foo' );
+			img = new Element( 'img' );
+			img.setAttr( 'src', 'img.png' );
+
+			root = new RootElement( 'p', { renderer: { markToSync: rootChangeSpy } } );
+			root.appendChildren( [ text, img ] );
+
+			root.on( 'change', ( evt, type, node ) => {
+				rootChangeSpy( type, node );
+			} );
+
+			rootChangeSpy.reset();
+		} );
+
+		it( 'should be fired on the node', () => {
+			const imgChangeSpy = sinon.spy();
+
+			img.on( 'change', ( evt, type, node ) => {
+				imgChangeSpy( type, node );
+			} );
+
+			img.setAttr( 'width', 100 );
+
+			sinon.assert.calledOnce( imgChangeSpy );
+			sinon.assert.calledWith( imgChangeSpy, 'ATTRIBUTES', img );
+		} );
+
+		it( 'should be fired on the parent', () => {
+			img.setAttr( 'width', 100 );
+
+			sinon.assert.calledOnce( rootChangeSpy );
+			sinon.assert.calledWith( rootChangeSpy, 'ATTRIBUTES', img );
+		} );
+
+		describe( 'setAttr', () => {
+			it( 'should fire change event', () => {
+				img.setAttr( 'width', 100 );
+
+				sinon.assert.calledOnce( rootChangeSpy );
+				sinon.assert.calledWith( rootChangeSpy, 'ATTRIBUTES', img );
+			} );
+		} );
+
+		describe( 'removeAttr', () => {
+			it( 'should fire change event', () => {
+				img.removeAttr( 'src' );
+
+				sinon.assert.calledOnce( rootChangeSpy );
+				sinon.assert.calledWith( rootChangeSpy, 'ATTRIBUTES', img );
+			} );
+		} );
+
+		describe( 'insertChildren', () => {
+			it( 'should fire change event', () => {
+				root.insertChildren( 1, new Element( 'img' ) );
+
+				sinon.assert.calledOnce( rootChangeSpy );
+				sinon.assert.calledWith( rootChangeSpy, 'CHILDREN', root );
+			} );
+		} );
+
+		describe( 'appendChildren', () => {
+			it( 'should fire change event', () => {
+				root.appendChildren( new Element( 'img' ) );
+
+				sinon.assert.calledOnce( rootChangeSpy );
+				sinon.assert.calledWith( rootChangeSpy, 'CHILDREN', root );
+			} );
+		} );
+
+		describe( 'removeChildren', () => {
+			it( 'should fire change event', () => {
+				root.removeChildren( 1, 1 );
+
+				sinon.assert.calledOnce( rootChangeSpy );
+				sinon.assert.calledWith( rootChangeSpy, 'CHILDREN', root );
+			} );
+		} );
+
+		describe( 'removeChildren', () => {
+			it( 'should fire change event', () => {
+				text.setText( 'bar' );
+
+				sinon.assert.calledOnce( rootChangeSpy );
+				sinon.assert.calledWith( rootChangeSpy, 'TEXT', text );
+			} );
+		} );
+	} );
 } );

+ 22 - 22
packages/ckeditor5-engine/tests/treeview/renderer.js

@@ -39,7 +39,7 @@ describe( 'Renderer', () => {
 		it( 'should mark attributes which need update', () => {
 			viewNode.setAttr( 'class', 'foo' );
 
-			renderer.markToSync( viewNode, 'ATTRIBUTES_NEED_UPDATE' );
+			renderer.markToSync( 'ATTRIBUTES', viewNode );
 
 			expect( renderer.markedAttrs.has( viewNode ) ).to.be.true;
 		} );
@@ -47,7 +47,7 @@ describe( 'Renderer', () => {
 		it( 'should mark children which need update', () => {
 			viewNode.appendChildren( new ViewText( 'foo' ) );
 
-			renderer.markToSync( viewNode, 'CHILDREN_NEED_UPDATE' );
+			renderer.markToSync( 'CHILDREN', viewNode );
 
 			expect( renderer.markedChildren.has( viewNode ) ).to.be.true;
 		} );
@@ -58,7 +58,7 @@ describe( 'Renderer', () => {
 
 			viewNode.appendChildren( new ViewText( 'foo' ) );
 
-			renderer.markToSync( viewNode, 'CHILDREN_NEED_UPDATE' );
+			renderer.markToSync( 'CHILDREN', viewNode );
 
 			expect( renderer.markedTexts.has( viewNode ) ).to.be.false;
 		} );
@@ -68,7 +68,7 @@ describe( 'Renderer', () => {
 			viewNode.appendChildren( viewText );
 			viewText.setText( 'bar' );
 
-			renderer.markToSync( viewText, 'TEXT_NEEDS_UPDATE' );
+			renderer.markToSync( 'TEXT', viewText );
 
 			expect( renderer.markedTexts.has( viewText ) ).to.be.true;
 		} );
@@ -81,14 +81,14 @@ describe( 'Renderer', () => {
 			viewNode.appendChildren( viewText );
 			viewText.setText( 'bar' );
 
-			renderer.markToSync( viewText, 'TEXT_NEEDS_UPDATE' );
+			renderer.markToSync( 'TEXT', viewText );
 
 			expect( renderer.markedTexts.has( viewText ) ).to.be.false;
 		} );
 
 		it( 'should throw if the type is unknown', () => {
 			expect( () => {
-				renderer.markToSync( viewNode, 'UNKNOWN' );
+				renderer.markToSync( 'UNKNOWN', viewNode );
 			} ).to.throw( CKEditorError, /^renderer-unknown-type/ );
 		} );
 	} );
@@ -110,7 +110,7 @@ describe( 'Renderer', () => {
 		it( 'should update attributes', () => {
 			viewNode.setAttr( 'class', 'foo' );
 
-			renderer.markToSync( viewNode, 'ATTRIBUTES_NEED_UPDATE' );
+			renderer.markToSync( 'ATTRIBUTES', viewNode );
 			renderer.render();
 
 			expect( domNode.getAttribute( 'class' ) ).to.equal( 'foo' );
@@ -123,7 +123,7 @@ describe( 'Renderer', () => {
 			domNode.setAttribute( 'id', 'bar' );
 			domNode.setAttribute( 'class', 'bar' );
 
-			renderer.markToSync( viewNode, 'ATTRIBUTES_NEED_UPDATE' );
+			renderer.markToSync( 'ATTRIBUTES', viewNode );
 			renderer.render();
 
 			expect( domNode.getAttribute( 'class' ) ).to.equal( 'foo' );
@@ -135,7 +135,7 @@ describe( 'Renderer', () => {
 		it( 'should add children', () => {
 			viewNode.appendChildren( new ViewText( 'foo' ) );
 
-			renderer.markToSync( viewNode, 'CHILDREN_NEED_UPDATE' );
+			renderer.markToSync( 'CHILDREN', viewNode );
 			renderer.render();
 
 			expect( domNode.childNodes.length ).to.equal( 1 );
@@ -147,7 +147,7 @@ describe( 'Renderer', () => {
 		it( 'should remove children', () => {
 			viewNode.appendChildren( new ViewText( 'foo' ) );
 
-			renderer.markToSync( viewNode, 'CHILDREN_NEED_UPDATE' );
+			renderer.markToSync( 'CHILDREN', viewNode );
 			renderer.render();
 
 			expect( domNode.childNodes.length ).to.equal( 1 );
@@ -155,7 +155,7 @@ describe( 'Renderer', () => {
 
 			viewNode.removeChildren( 0, 1 );
 
-			renderer.markToSync( viewNode, 'CHILDREN_NEED_UPDATE' );
+			renderer.markToSync( 'CHILDREN', viewNode );
 			renderer.render();
 
 			expect( domNode.childNodes.length ).to.equal( 0 );
@@ -167,7 +167,7 @@ describe( 'Renderer', () => {
 			const viewText = new ViewText( 'foo' );
 			viewNode.appendChildren( viewText );
 
-			renderer.markToSync( viewNode, 'CHILDREN_NEED_UPDATE' );
+			renderer.markToSync( 'CHILDREN', viewNode );
 			renderer.render();
 
 			expect( domNode.childNodes.length ).to.equal( 1 );
@@ -175,7 +175,7 @@ describe( 'Renderer', () => {
 
 			viewText.setText( 'bar' );
 
-			renderer.markToSync( viewText, 'TEXT_NEEDS_UPDATE' );
+			renderer.markToSync( 'TEXT', viewText );
 			renderer.render();
 
 			expect( domNode.childNodes.length ).to.equal( 1 );
@@ -189,8 +189,8 @@ describe( 'Renderer', () => {
 			const viewText = new ViewText( 'foo' );
 			viewNode.appendChildren( [ viewImg, viewText ] );
 
-			renderer.markToSync( viewNode, 'CHILDREN_NEED_UPDATE' );
-			renderer.markToSync( viewText, 'TEXT_NEEDS_UPDATE' );
+			renderer.markToSync( 'CHILDREN', viewNode );
+			renderer.markToSync( 'TEXT', viewText );
 			renderer.render();
 
 			expect( domNode.childNodes.length ).to.equal( 2 );
@@ -202,13 +202,13 @@ describe( 'Renderer', () => {
 			const viewText = new ViewText( 'foo' );
 			viewNode.appendChildren( viewText );
 
-			renderer.markToSync( viewNode, 'CHILDREN_NEED_UPDATE' );
+			renderer.markToSync( 'CHILDREN', viewNode );
 			renderer.render();
 
 			// This should not be changed during the render.
 			const domText = domNode.childNodes[ 0 ];
 
-			renderer.markToSync( viewText, 'TEXT_NEEDS_UPDATE' );
+			renderer.markToSync( 'TEXT', viewText );
 			renderer.render();
 
 			expect( domNode.childNodes.length ).to.equal( 1 );
@@ -219,13 +219,13 @@ describe( 'Renderer', () => {
 			const viewText = new ViewText( 'foo' );
 			viewNode.appendChildren( viewText );
 
-			renderer.markToSync( viewNode, 'CHILDREN_NEED_UPDATE' );
+			renderer.markToSync( 'CHILDREN', viewNode );
 			renderer.render();
 
 			// This should not be changed during the render.
 			const domText = domNode.childNodes[ 0 ];
 
-			renderer.markToSync( viewNode, 'CHILDREN_NEED_UPDATE' );
+			renderer.markToSync( 'CHILDREN', viewNode );
 			renderer.render();
 
 			expect( domNode.childNodes.length ).to.equal( 1 );
@@ -242,7 +242,7 @@ describe( 'Renderer', () => {
 
 			converter.bindElements( domImg, viewImg );
 
-			renderer.markToSync( viewNode, 'CHILDREN_NEED_UPDATE' );
+			renderer.markToSync( 'CHILDREN', viewNode );
 			renderer.render();
 
 			expect( domNode.childNodes.length ).to.equal( 1 );
@@ -253,14 +253,14 @@ describe( 'Renderer', () => {
 			const viewImg = new ViewElement( 'img' );
 			viewNode.appendChildren( viewImg );
 
-			renderer.markToSync( viewNode, 'CHILDREN_NEED_UPDATE' );
+			renderer.markToSync( 'CHILDREN', viewNode );
 			renderer.render();
 
 			const viewP = new ViewElement( 'p' );
 			viewNode.removeChildren( 0, 1 );
 			viewNode.appendChildren( viewP );
 
-			renderer.markToSync( viewNode, 'CHILDREN_NEED_UPDATE' );
+			renderer.markToSync( 'CHILDREN', viewNode );
 			renderer.render();
 
 			expect( domNode.childNodes.length ).to.equal( 1 );