Przeglądaj źródła

Fixed tests for placeholders and view.

Maciej Bukowski 7 lat temu
rodzic
commit
9ff563fbce

+ 0 - 3
packages/ckeditor5-engine/src/view/placeholder.js

@@ -38,9 +38,6 @@ export function attachPlaceholder( view, element, placeholderText, checkFunction
 		placeholderText,
 		checkFunction
 	} );
-
-	// Update view right away.
-	view.render( { force: true } );
 }
 
 /**

+ 3 - 7
packages/ckeditor5-engine/src/view/view.js

@@ -176,12 +176,8 @@ export default class View {
 			this._hasChangedSinceTheLastRendering = false;
 		} );
 
-		// Listen to the selection changes.
-		this.listenTo( this.document, 'selectionChange', () => {
-			this._hasChangedSinceTheLastRendering = true;
-		} );
-
-		this.listenTo( this.document, 'selectionChange', () => {
+		// Listen to the document selection changes directly.
+		this.listenTo( this.document.selection, 'change', () => {
 			this._hasChangedSinceTheLastRendering = true;
 		} );
 	}
@@ -420,7 +416,7 @@ export default class View {
 			this._hasChangedSinceTheLastRendering = true;
 		}
 
-		this.change( () => {} );
+		this.change( () => { } );
 	}
 
 	/**

+ 12 - 0
packages/ckeditor5-engine/tests/view/placeholder.js

@@ -25,6 +25,7 @@ describe( 'placeholder', () => {
 			const element = viewRoot.getChild( 0 );
 
 			attachPlaceholder( view, element, 'foo bar baz' );
+			view.render( { force: true } );
 
 			expect( element.getAttribute( 'data-placeholder' ) ).to.equal( 'foo bar baz' );
 			expect( element.hasClass( 'ck-placeholder' ) ).to.be.true;
@@ -35,6 +36,7 @@ describe( 'placeholder', () => {
 			const element = viewRoot.getChild( 0 );
 
 			attachPlaceholder( view, element, 'foo bar baz' );
+			view.render( { force: true } );
 
 			expect( element.getAttribute( 'data-placeholder' ) ).to.equal( 'foo bar baz' );
 			expect( element.hasClass( 'ck-placeholder' ) ).to.be.false;
@@ -45,6 +47,7 @@ describe( 'placeholder', () => {
 			const element = viewRoot.getChild( 0 );
 
 			attachPlaceholder( view, element, 'foo bar baz' );
+			view.render( { force: true } );
 
 			expect( element.getAttribute( 'data-placeholder' ) ).to.equal( 'foo bar baz' );
 			expect( element.hasClass( 'ck-placeholder' ) ).to.be.true;
@@ -55,6 +58,7 @@ describe( 'placeholder', () => {
 			const element = viewRoot.getChild( 0 );
 
 			attachPlaceholder( view, element, 'foo bar baz' );
+			view.render( { force: true } );
 
 			expect( element.getAttribute( 'data-placeholder' ) ).to.equal( 'foo bar baz' );
 			expect( element.hasClass( 'ck-placeholder' ) ).to.be.false;
@@ -66,6 +70,7 @@ describe( 'placeholder', () => {
 			viewDocument.isFocused = false;
 
 			attachPlaceholder( view, element, 'foo bar baz' );
+			view.render( { force: true } );
 
 			expect( element.getAttribute( 'data-placeholder' ) ).to.equal( 'foo bar baz' );
 			expect( element.hasClass( 'ck-placeholder' ) ).to.be.true;
@@ -78,6 +83,7 @@ describe( 'placeholder', () => {
 			const spy = sinon.spy( () => result );
 
 			attachPlaceholder( view, element, 'foo bar baz', spy );
+			view.render( { force: true } );
 
 			sinon.assert.called( spy );
 			expect( element.getAttribute( 'data-placeholder' ) ).to.equal( 'foo bar baz' );
@@ -93,6 +99,7 @@ describe( 'placeholder', () => {
 			const element = viewRoot.getChild( 0 );
 
 			attachPlaceholder( view, element, 'foo bar baz' );
+			view.render( { force: true } );
 
 			expect( element.getAttribute( 'data-placeholder' ) ).to.equal( 'foo bar baz' );
 			expect( element.hasClass( 'ck-placeholder' ) ).to.be.true;
@@ -110,6 +117,7 @@ describe( 'placeholder', () => {
 
 			attachPlaceholder( view, element, 'foo bar baz' );
 			attachPlaceholder( view, element, 'new text' );
+			view.render( { force: true } );
 
 			expect( element.getAttribute( 'data-placeholder' ) ).to.equal( 'new text' );
 			expect( element.hasClass( 'ck-placeholder' ) ).to.be.true;
@@ -138,6 +146,8 @@ describe( 'placeholder', () => {
 
 			attachPlaceholder( view, element, 'first placeholder' );
 			attachPlaceholder( secondView, secondElement, 'second placeholder' );
+			view.render( { force: true } );
+			secondView.render( { force: true } );
 
 			expect( element.getAttribute( 'data-placeholder' ) ).to.equal( 'first placeholder' );
 			expect( element.hasClass( 'ck-placeholder' ) ).to.be.true;
@@ -166,6 +176,7 @@ describe( 'placeholder', () => {
 			const element = viewRoot.getChild( 0 );
 
 			attachPlaceholder( view, element, 'foo bar baz' );
+			view.render( { force: true } );
 
 			view.change( writer => {
 				writer.setSelection( ViewRange._createIn( element ) );
@@ -186,6 +197,7 @@ describe( 'placeholder', () => {
 			const element = viewRoot.getChild( 0 );
 
 			attachPlaceholder( view, element, 'foo bar baz' );
+			view.render( { force: true } );
 
 			expect( element.getAttribute( 'data-placeholder' ) ).to.equal( 'foo bar baz' );
 			expect( element.hasClass( 'ck-placeholder' ) ).to.be.true;

+ 64 - 21
packages/ckeditor5-engine/tests/view/view/view.js

@@ -204,7 +204,7 @@ describe( 'view', () => {
 
 		it( 'should be disabled and re-enabled on render', () => {
 			const observerMock = view.addObserver( ObserverMock );
-			view.render();
+			view.render( { force: true } );
 
 			sinon.assert.calledOnce( observerMock.disable );
 			sinon.assert.calledOnce( view._renderer.render );
@@ -382,23 +382,49 @@ describe( 'view', () => {
 			const observerMock = view.addObserver( ObserverMock );
 			const renderStub = sinon.stub( view._renderer, 'render' );
 
-			view.render();
+			view.render( { force: true } );
 
 			sinon.assert.callOrder( observerMock.disable, renderStub, observerMock.enable );
 		} );
 
-		it( 'should fire view.document.layoutChanged event on render', () => {
-			const spy = sinon.spy();
+		it( 'should not fire `render` and `layoutChanged` without the `force` flag if there were no changes', () => {
+			const renderSpy = sinon.spy();
+			const layoutChangedSpy = sinon.spy();
 
-			view.document.on( 'layoutChanged', spy );
+			view.on( 'render', renderSpy );
+			view.document.on( 'layoutChanged', layoutChangedSpy );
 
 			view.render();
 
-			sinon.assert.calledOnce( spy );
+			sinon.assert.notCalled( renderSpy );
+			sinon.assert.notCalled( layoutChangedSpy );
+		} );
+
+		it( 'should fire `render` and `layoutChanged` if there were no changes but the `force` flag was set', () => {
+			const renderSpy = sinon.spy();
+			const layoutChangedSpy = sinon.spy();
+
+			view.on( 'render', renderSpy );
+			view.document.on( 'layoutChanged', layoutChangedSpy );
+
+			view.render( { force: true } );
+
+			sinon.assert.calledOnce( renderSpy );
+			sinon.assert.calledOnce( layoutChangedSpy );
+		} )
 
+		it( 'should fire `render` and `layoutChanged` if there is some buffered change', () => {
+			const renderSpy = sinon.spy();
+			const layoutChangedSpy = sinon.spy();
+
+			view.on( 'render', renderSpy );
+			view.document.on( 'layoutChanged', layoutChangedSpy );
+
+			view.document.selection._setTo( null );
 			view.render();
 
-			sinon.assert.calledTwice( spy );
+			sinon.assert.calledOnce( renderSpy );
+			sinon.assert.calledOnce( layoutChangedSpy );
 		} );
 	} );
 
@@ -414,7 +440,7 @@ describe( 'view', () => {
 
 			createViewRoot( viewDocument, 'div', 'main' );
 			view.attachDomRoot( domDiv );
-			view.render();
+			view.render( { force: true } );
 
 			expect( domDiv.childNodes.length ).to.equal( 1 );
 			expect( isBlockFiller( domDiv.childNodes[ 0 ], BR_FILLER ) ).to.be.true;
@@ -513,7 +539,9 @@ describe( 'view', () => {
 
 			view.on( 'render', eventSpy );
 
-			view.change( () => {} );
+			view.change( writer => {
+				writer.setSelection( null );
+			} );
 
 			sinon.assert.callOrder( renderSpy, eventSpy );
 		} );
@@ -521,16 +549,24 @@ describe( 'view', () => {
 		it( 'should fire render event once for nested change blocks', () => {
 			const renderSpy = sinon.spy( view._renderer, 'render' );
 			const eventSpy = sinon.spy();
+			const viewEditable = createViewRoot( viewDocument, 'div', 'main' );
 
 			view.on( 'render', eventSpy );
 
-			view.change( () => {
-				view.change( () => {} );
-				view.change( () => {
-					view.change( () => {} );
-					view.change( () => {} );
+			view.change( writer => {
+				writer.setSelection( null );
+				view.change( writer => {
+					writer.setSelection( viewEditable, 0 );
+				} );
+				view.change( writer => {
+					writer.setSelection( null );
+					view.change( writer => {
+						writer.setSelection( viewEditable, 0 );
+					} );
+				} );
+				view.change( writer => {
+					writer.setSelection( null );
 				} );
-				view.change( () => {} );
 			} );
 
 			sinon.assert.calledOnce( renderSpy );
@@ -545,11 +581,12 @@ describe( 'view', () => {
 			view.on( 'render', eventSpy );
 
 			view.change( () => {
-				view.render();
-				view.change( () => {
-					view.render();
+				view.render( { force: true } );
+				view.change( writer => {
+					writer.setSelection( null );
+					view.render( { force: true } );
 				} );
-				view.render();
+				view.render( { force: true } );
 			} );
 
 			sinon.assert.calledOnce( renderSpy );
@@ -567,7 +604,10 @@ describe( 'view', () => {
 			viewDocument.registerPostFixer( postFixer2 );
 			view.on( 'render', eventSpy );
 
-			view.change( changeSpy );
+			view.change( writer => {
+				changeSpy();
+				writer.setSelection( null );
+			} );
 
 			sinon.assert.calledOnce( postFixer1 );
 			sinon.assert.calledOnce( postFixer2 );
@@ -598,7 +638,10 @@ describe( 'view', () => {
 			} );
 			view.on( 'render', eventSpy );
 
-			view.change( changeSpy );
+			view.change( writer => {
+				changeSpy();
+				writer.setSelection( null );
+			} );
 
 			sinon.assert.calledOnce( postFixer1 );
 			sinon.assert.calledOnce( postFixer2 );