Jelajahi Sumber

Merge pull request #516 from ckeditor/t/512

Fix for focus toggling in editable
Szymon Kupś 9 tahun lalu
induk
melakukan
ad0ea81bf8

+ 19 - 17
packages/ckeditor5-engine/src/view/document.js

@@ -86,17 +86,6 @@ export default class Document {
 		 */
 		this.set( 'isFocused', false );
 
-		/**
-		 * {@link engine.view.EditableElement EditableElement} which is containing selection. It can be `null` if
-		 * there is no selection found.
-		 *
-		 * @readonly
-		 * @observable
-		 * @member {engine.view.EditableElement|null} engine.view.Document#selectedEditable
-		 */
-		this.set( 'selectedEditable', this.selection.getEditableElement() );
-		this.selection.on( 'change', () => this.selectedEditable = this.selection.getEditableElement() );
-
 		/**
 		 * Instance of the {@link engine.view.Document#renderer renderer}.
 		 *
@@ -115,6 +104,14 @@ export default class Document {
 		this._observers = new Map();
 
 		injectQuirksHandling( this );
+
+		// Listens `render` event on default priority.
+		// This way we can attach other listeners before or after rendering execution.
+		this.on( 'render', () => {
+			this.disableObservers();
+			this.renderer.render();
+			this.enableObservers();
+		} );
 	}
 
 	/**
@@ -248,13 +245,11 @@ export default class Document {
 	/**
 	 * Renders all changes. In order to avoid triggering the observers (e.g. mutations) all observers are disabled
 	 * before rendering and re-enabled after that.
+	 *
+	 * @fires engine.view.Document#render
 	 */
 	render() {
-		this.disableObservers();
-
-		this.renderer.render();
-
-		this.enableObservers();
+		this.fire( 'render' );
 	}
 
 	/**
@@ -263,7 +258,7 @@ export default class Document {
 	 */
 	focus() {
 		if ( !this.isFocused ) {
-			const editable = this.selectedEditable;
+			const editable = this.selection.getEditableElement();
 
 			if ( editable ) {
 				this.domConverter.focus( editable );
@@ -313,3 +308,10 @@ mix( Document, ObservableMixin );
  *
  * @typedef {String} engine.view.ChangeType
  */
+
+/**
+ * Fired when {@link engine.view.Document#render render} method is called. Actual rendering is executed as a listener to
+ * this event with default priority. This way other listeners can be used to run code before or after rendering.
+ *
+ * @event engine.view.Document.render
+ */

+ 10 - 6
packages/ckeditor5-engine/src/view/editableelement.js

@@ -43,20 +43,24 @@ export default class EditableElement extends ContainerElement {
 		/**
 		 * Whether the editable is focused.
 		 *
-		 * This property updates when {@link engine.view.Document#isFocused} or {@link engine.view.Document#selectedEditable}
-		 * change.
+		 * This property updates when {@link engine.view.Document#isFocused document.isFocused} is changed and after each
+		 * {@link engine.view.Document#render render} method call.
 		 *
 		 * @readonly
 		 * @observable
-		 * @member {Boolean} engine.view.RootEditableElement#isFocused
+		 * @member {Boolean} engine.view.EditableElement#isFocused
 		 */
 		this.bind( 'isFocused' ).to(
 			document,
 			'isFocused',
-			document,
-			'selectedEditable',
-			( isFocused, selectedEditable ) => isFocused && selectedEditable == this
+			( isFocused ) => isFocused && document.selection.getEditableElement() == this
 		);
+
+		// Update focus state after each rendering. Selection might be moved to different editable before rendering,
+		// but this does not mean that editable has focus - it will be placed there after rendering.
+		this.listenTo( document, 'render', () => {
+			this.isFocused = document.isFocused && document.selection.getEditableElement() == this;
+		}, null, 11 );
 	}
 
 	/**

+ 3 - 1
packages/ckeditor5-engine/src/view/observer/focusobserver.js

@@ -28,7 +28,9 @@ export default class FocusObserver extends DomEventObserver {
 		} );
 
 		document.on( 'blur', ( evt, data ) => {
-			if ( document.selectedEditable === null || document.selectedEditable === data.target ) {
+			const selectedEditable = document.selection.getEditableElement();
+
+			if ( selectedEditable === null || selectedEditable === data.target ) {
 				document.isFocused = false;
 			}
 		} );

+ 0 - 1
packages/ckeditor5-engine/tests/editingcontroller.js

@@ -192,7 +192,6 @@ describe( 'EditingController', () => {
 				} );
 			} );
 
-			editing.view.selectedEditable = viewRoot;
 			editing.view.isFocused = true;
 
 			const domSelection = document.getSelection();

+ 2 - 1
packages/ckeditor5-engine/tests/view/_utils/createdocumentmock.js

@@ -8,6 +8,7 @@
 'use strict';
 
 import ObservableMixin from '/ckeditor5/utils/observablemixin.js';
+import Selection from '/ckeditor5/engine/view/selection.js';
 
 /**
  * Creates {@link engine.view.Document view Document} mock.
@@ -17,7 +18,7 @@ import ObservableMixin from '/ckeditor5/utils/observablemixin.js';
 export default function createDocumentMock() {
 	const doc = Object.create( ObservableMixin );
 	doc.set( 'isFocused', false );
-	doc.set( 'selectedEditable', null );
+	doc.selection = new Selection();
 
 	return doc;
 }

+ 0 - 14
packages/ckeditor5-engine/tests/view/document/document.js

@@ -60,7 +60,6 @@ describe( 'Document', () => {
 			expect( viewDocument ).to.have.property( 'renderer' ).that.is.instanceOf( Renderer );
 			expect( viewDocument ).to.have.property( 'writer' ).that.is.instanceOf( Writer );
 			expect( viewDocument ).to.have.property( 'domConverter' ).that.is.instanceOf( DomConverter );
-			expect( viewDocument ).to.have.property( 'selectedEditable' ).that.is.null;
 			expect( viewDocument ).to.have.property( 'isFocused' ).that.is.false;
 		} );
 	} );
@@ -355,19 +354,6 @@ describe( 'Document', () => {
 		} );
 	} );
 
-	describe( 'selectedEditable', () => {
-		it( 'should change when selection is moved to different editable', () => {
-			const viewDocument = new Document();
-
-			expect( viewDocument.selectedEditable ).to.equal( null );
-
-			const viewRoot = viewDocument.createRoot( 'div' );
-			viewDocument.selection.addRange( ViewRange.createFromParentsAndOffsets( viewRoot, 0, viewRoot, 0 ) );
-
-			expect( viewDocument.selectedEditable ).to.equal( viewRoot );
-		} );
-	} );
-
 	describe( 'focus', () => {
 		let viewDocument, domEditable, viewEditable;
 

+ 1 - 2
packages/ckeditor5-engine/tests/view/document/jumpoverinlinefiller.js

@@ -24,11 +24,10 @@ describe( 'Document', () => {
 
 		viewDocument.addObserver( KeyObserver );
 
-		const viewRoot = viewDocument.createRoot( document.getElementById( 'editor' ) );
+		viewDocument.createRoot( document.getElementById( 'editor' ) );
 
 		document.getSelection().removeAllRanges();
 
-		viewDocument.selectedEditable = viewRoot;
 		viewDocument.isFocused = true;
 	} );
 

+ 11 - 10
packages/ckeditor5-engine/tests/view/editableelement.js

@@ -9,6 +9,7 @@
 
 import createDocumentMock from '/tests/engine/view/_utils/createdocumentmock.js';
 import RootEditableElement from '/ckeditor5/engine/view/rooteditableelement.js';
+import Range from '/ckeditor5/engine/view/range.js';
 
 describe( 'EditableElement', () => {
 	describe( 'isFocused', () => {
@@ -37,26 +38,26 @@ describe( 'EditableElement', () => {
 			expect( isFocusedSpy.calledOnce ).to.be.true;
 		} );
 
-		it( 'should change isFocused when selectedEditable changes', () => {
-			docMock.selectedEditable = viewMain;
+		it( 'should change isFocused on document render event', () => {
+			const rangeMain = Range.createFromParentsAndOffsets( viewMain, 0, viewMain, 0 );
+			const rangeHeader = Range.createFromParentsAndOffsets( viewHeader, 0, viewHeader, 0 );
+			docMock.selection.addRange( rangeMain );
 			docMock.isFocused = true;
 
 			expect( viewMain.isFocused ).to.be.true;
 			expect( viewHeader.isFocused ).to.be.false;
 
-			docMock.selectedEditable = viewHeader;
+			docMock.selection.setRanges( [ rangeHeader ] );
+			docMock.fire( 'render' );
 
 			expect( viewMain.isFocused ).to.be.false;
 			expect( viewHeader.isFocused ).to.be.true;
-
-			docMock.selectedEditable = null;
-
-			expect( viewMain.isFocused ).to.be.false;
-			expect( viewHeader.isFocused ).to.be.false;
 		} );
 
 		it( 'should change isFocused when document.isFocus changes', () => {
-			docMock.selectedEditable = viewMain;
+			const rangeMain = Range.createFromParentsAndOffsets( viewMain, 0, viewMain, 0 );
+			const rangeHeader = Range.createFromParentsAndOffsets( viewHeader, 0, viewHeader, 0 );
+			docMock.selection.addRange( rangeMain );
 			docMock.isFocused = true;
 
 			expect( viewMain.isFocused ).to.be.true;
@@ -67,7 +68,7 @@ describe( 'EditableElement', () => {
 			expect( viewMain.isFocused ).to.be.false;
 			expect( viewHeader.isFocused ).to.be.false;
 
-			docMock.selectedEditable = viewHeader;
+			docMock.selection.setRanges( [ rangeHeader ] );
 
 			expect( viewMain.isFocused ).to.be.false;
 			expect( viewHeader.isFocused ).to.be.false;

+ 1 - 2
packages/ckeditor5-engine/tests/view/manual/x-index.js

@@ -13,12 +13,11 @@ import MutationObserver from '/ckeditor5/engine/view/observer/mutationobserver.j
 import { setData } from '/tests/engine/_utils/view.js';
 
 const viewDocument = new Document();
-const viewRoot = viewDocument.createRoot( document.getElementById( 'editor' ) );
+viewDocument.createRoot( document.getElementById( 'editor' ) );
 
 viewDocument.addObserver( MutationObserver );
 viewDocument.addObserver( SelectionObserver );
 
-viewDocument.selectedEditable = viewRoot;
 viewDocument.isFocused = true;
 
 setData( viewDocument,

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

@@ -47,7 +47,6 @@ describe( 'SelectionObserver', () => {
 		viewDocument.selection.removeAllRanges();
 		document.getSelection().removeAllRanges();
 
-		viewDocument.selectedEditable = viewRoot;
 		viewDocument.isFocused = true;
 
 		viewDocument.getObserver( SelectionObserver ).enable();

+ 0 - 1
packages/ckeditor5-engine/tests/view/utils-tests/createdocumentmock.js

@@ -23,7 +23,6 @@ describe( 'createDocumentMock', () => {
 			done();
 		} );
 
-		docMock.selectedEditable = rootMock;
 		docMock.isFocused = true;
 	} );
 } );