Преглед изворни кода

Delaying render call in FocusObserver.

Szymon Kupś пре 9 година
родитељ
комит
bc6b2151f7

+ 23 - 6
packages/ckeditor5-engine/src/view/observer/focusobserver.js

@@ -8,7 +8,10 @@
  */
 
 import DomEventObserver from './domeventobserver';
-import SelectionObserver from './selectionobserver';
+import global from '@ckeditor/ckeditor5-utils/src/dom/global';
+
+const setTimeout = global.window.setTimeout;
+const clearTimeout = global.window.clearTimeout;
 
 /**
  * {@link module:engine/view/document~Document#event:focus Focus}
@@ -27,8 +30,6 @@ export default class FocusObserver extends DomEventObserver {
 		this.domEventType = [ 'focus', 'blur' ];
 		this.useCapture = true;
 
-		this.selectionObserver = document.getObserver( SelectionObserver );
-
 		document.on( 'focus', () => {
 			document.isFocused = true;
 
@@ -36,9 +37,7 @@ export default class FocusObserver extends DomEventObserver {
 			// We need to wait until `SelectionObserver` handle the event and then render. Otherwise rendering will
 			// overwrite new DOM selection with selection from the view.
 			// See https://github.com/ckeditor/ckeditor5-engine/issues/795 for more details.
-			this.selectionObserver.once( 'selectionChangeHandling', () => {
-				document.render();
-			}, { priority: 'low' } );
+			this._renderTimeoutId = setTimeout( () => document.render(), 0 );
 		} );
 
 		document.on( 'blur', ( evt, data ) => {
@@ -51,11 +50,29 @@ export default class FocusObserver extends DomEventObserver {
 				document.render();
 			}
 		} );
+
+		/**
+		 * Identifier of the timeout currently used by focus listener to delay rendering execution.
+		 *
+		 * @private
+		 * @member {Number} #_renderTimeoutId
+		 */
 	}
 
 	onDomEvent( domEvent ) {
 		this.fire( domEvent.type, domEvent );
 	}
+
+	/**
+	 * @inheritDoc
+	 */
+	destroy() {
+		if ( this._renderTimeoutId ) {
+			clearTimeout( this._renderTimeoutId );
+		}
+
+		super.destroy();
+	}
 }
 
 /**

+ 1 - 14
packages/ckeditor5-engine/src/view/observer/selectionobserver.js

@@ -85,10 +85,6 @@ export default class SelectionObserver extends Observer {
 
 		this._clearInfiniteLoopInterval = setInterval( () => this._clearInfiniteLoop(), 2000 );
 
-		this.on( 'selectionChangeHandling', ( evt, data ) => {
-			this._handleSelectionChange( data.domDocument );
-		} );
-
 		/**
 		 * Private property to store the last selection, to check if the code does not enter infinite loop.
 		 *
@@ -123,7 +119,7 @@ export default class SelectionObserver extends Observer {
 		}
 
 		this.listenTo( domDocument, 'selectionchange', () => {
-			this.fire( 'selectionChangeHandling', { domDocument } );
+			this._handleSelectionChange( domDocument );
 		} );
 
 		this._documents.add( domDocument );
@@ -274,12 +270,3 @@ export default class SelectionObserver extends Observer {
  * @param {module:engine/view/selection~Selection} data.newSelection New View selection which is converted DOM selection.
  * @param {Selection} data.domSelection Native DOM selection.
  */
-
-/**
- * Fired when selection change is being handled. It can be used to execute code before or after actual `selectionchange`
- * event handling.
- *
- * @event module:engine/view/observer/selectionobserver~SelectionObserver#event:selectionChangeHandling
- * @param {Object} data
- * @param {Document} domDocument DOM document on which `selectionchange` event was fired.
- */

+ 5 - 0
packages/ckeditor5-engine/tests/manual/nestededitable.md

@@ -3,3 +3,8 @@
 * Put selection inside `foo bar baz` nested editable. Main editable and nested one should be focused (blue outline should be visible).
 * Change selection inside nested editable and see if `Model contents` change accordingly.
 * Click outside the editor. Outline from main editable and nested editable should be removed.
+* Check following scenario:
+  * put selection inside nested editable: `foo bar baz{}`,
+  * click outside the editor (outlines should be removed),
+  * put selection at exact same place as before: `foo bar baz{}`,
+  * both editables should be focused (blue outline should be visible).

+ 16 - 10
packages/ckeditor5-engine/tests/view/observer/focusobserver.js

@@ -4,11 +4,12 @@
  */
 
 import FocusObserver from '../../../src/view/observer/focusobserver';
-import SelectionObserver from '../../../src/view/observer/selectionobserver';
 import ViewDocument from '../../../src/view/document';
 import ViewRange from '../../../src/view/range';
 import global from '@ckeditor/ckeditor5-utils/src/dom/global';
 
+const setTimeout = global.window.setTimeout;
+
 describe( 'FocusObserver', () => {
 	let viewDocument, observer;
 
@@ -116,24 +117,29 @@ describe( 'FocusObserver', () => {
 			expect( viewDocument.isFocused ).to.be.true;
 		} );
 
-		it( 'should call render after selectionChange event is handled', () => {
+		it( 'should delay rendering to the next iteration of event loop', ( done ) => {
 			const renderSpy = sinon.spy( viewDocument, 'render' );
-			const selectionObserver = viewDocument.getObserver( SelectionObserver );
 
 			observer.onDomEvent( { type: 'focus', target: domMain } );
 			sinon.assert.notCalled( renderSpy );
-			selectionObserver.fire( 'selectionChangeHandling', { domDocument: global.document } );
-			sinon.assert.called( renderSpy );
+
+			setTimeout( () => {
+				sinon.assert.called( renderSpy );
+				done();
+			}, 0 );
 		} );
 
-		it( 'should call render only after first selectionChange after focus', () => {
+		it( 'should not call render if destroyed', ( done ) => {
 			const renderSpy = sinon.spy( viewDocument, 'render' );
-			const selectionObserver = viewDocument.getObserver( SelectionObserver );
 
 			observer.onDomEvent( { type: 'focus', target: domMain } );
-			selectionObserver.fire( 'selectionChangeHandling', { domDocument: global.document } );
-			selectionObserver.fire( 'selectionChangeHandling', { domDocument: global.document } );
-			sinon.assert.calledOnce( renderSpy );
+			sinon.assert.notCalled( renderSpy );
+			observer.destroy();
+
+			setTimeout( () => {
+				sinon.assert.notCalled( renderSpy );
+				done();
+			}, 0 );
 		} );
 	} );
 } );

+ 0 - 15
packages/ckeditor5-engine/tests/view/observer/selectionobserver.js

@@ -299,21 +299,6 @@ describe( 'SelectionObserver', () => {
 			}, 110 );
 		}, 100 );
 	} );
-
-	it( 'should fire selectionChangeHandling on standard priority', ( done ) => {
-		let spy1 = sinon.spy();
-		let spy2 = sinon.spy();
-
-		selectionObserver.on( 'selectionChangeHandling', spy1, { priority: 'high' } );
-		viewDocument.on( 'selectionChange', spy2 );
-
-		selectionObserver.on( 'selectionChangeHandling', () => {
-			sinon.assert.callOrder( spy1, spy2 );
-			done();
-		}, { priority: 'low' } );
-
-		changeDomSelection();
-	} );
 } );
 
 function changeDomSelection() {