8
0
Просмотр исходного кода

Changed logic of focusing view document. DOM element focusing moved to engine.view.DOMConverter.

Szymon Kupś 9 лет назад
Родитель
Сommit
ead3764867

+ 18 - 4
packages/ckeditor5-engine/src/view/document.js

@@ -11,6 +11,7 @@ import Writer from './writer.js';
 import DomConverter from './domconverter.js';
 import RootEditableElement from './rooteditableelement.js';
 import { injectQuirksHandling } from './filler.js';
+import log from '../../utils/log.js';
 
 import mix from '../../utils/mix.js';
 import ObservableMixin from '../../utils/observablemixin.js';
@@ -258,12 +259,25 @@ export default class Document {
 	/**
 	 * Focuses document. It will focus {@link engine.view.EditableElement EditableElement} that is currently having
 	 * selection inside.
-	 *
-	 * @fires engine.view.Document#focus
 	 */
 	focus() {
-		this.fire( 'focus' );
-		this.render();
+		if ( !this.isFocused ) {
+			const editable = this.selectedEditable;
+
+			if ( editable ) {
+				this.domConverter.focus( editable );
+				this.render();
+			} else {
+				/**
+				 * Before focusing view document, selection should be placed inside one of the view's editables.
+				 * Normally its selection will be converted from model document (which have default selection), but
+				 * when using view document on its own, we need to manually place selection before focusing it.
+				 *
+				 * @error view-focus-no-selection
+				 */
+				log.warn( 'view-focus-no-selection: There is no selection in any editable to focus.' );
+			}
+		}
 	}
 
 	/**

+ 13 - 0
packages/ckeditor5-engine/src/view/domconverter.js

@@ -606,4 +606,17 @@ export default class DomConverter {
 
 		return null;
 	}
+
+	/**
+	 * Focuses DOM editable that is corresponding to provided {@link engine.view.EditableElement EditableElement}.
+	 *
+	 * @param {engine.view.EditableElement} viewEditable
+	 */
+	focus( viewEditable ) {
+		const domEditable = this.getCorrespondingDomElement( viewEditable );
+
+		if ( domEditable && domEditable.ownerDocument.activeElement !== domEditable ) {
+			domEditable.focus();
+		}
+	}
 }

+ 1 - 5
packages/ckeditor5-engine/src/view/renderer.js

@@ -455,11 +455,7 @@ export default class Renderer {
 			const editable = this.selection.getEditableElement();
 
 			if ( editable ) {
-				const domElement = this.domConverter.getCorrespondingDomElement( editable );
-
-				if ( domElement && domElement.ownerDocument.activeElement !== domElement ) {
-					domElement.focus();
-				}
+				this.domConverter.focus( editable );
 			}
 		}
 	}

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

@@ -13,8 +13,11 @@ import Renderer from '/ckeditor5/engine/view/renderer.js';
 import ViewRange from '/ckeditor5/engine/view/range.js';
 import Writer from '/ckeditor5/engine/view/writer.js';
 import DomConverter from '/ckeditor5/engine/view/domconverter.js';
-
+import testUtils from '/tests/ckeditor5/_utils/utils.js';
 import count from '/ckeditor5/utils/count.js';
+import log from '/ckeditor5/utils/log.js';
+
+testUtils.createSinonSandbox();
 
 describe( 'Document', () => {
 	let ObserverMock, ObserverMockGlobalCount, instantiated, enabled;
@@ -364,4 +367,58 @@ describe( 'Document', () => {
 			expect( viewDocument.selectedEditable ).to.equal( viewRoot );
 		} );
 	} );
+
+	describe( 'focus', () => {
+		let viewDocument, domEditable, viewEditable;
+
+		beforeEach( () => {
+			viewDocument = new Document();
+			domEditable = document.createElement( 'div' );
+			domEditable.setAttribute( 'contenteditable', 'true' );
+			document.body.appendChild( domEditable );
+			viewEditable = viewDocument.createRoot( domEditable );
+			viewDocument.selection.addRange( ViewRange.createFromParentsAndOffsets( viewEditable, 0, viewEditable, 0 ) );
+		} );
+
+		afterEach( () => {
+			document.body.removeChild( domEditable );
+		} );
+
+		it( 'should focus editable with selection', () => {
+			const converterFocusSpy = testUtils.sinon.spy( viewDocument.domConverter, 'focus' );
+			const renderSpy = testUtils.sinon.spy( viewDocument, 'render' );
+
+			viewDocument.focus();
+
+			expect( converterFocusSpy.calledOnce ).to.be.true;
+			expect( renderSpy.calledOnce ).to.be.true;
+			expect( document.activeElement ).to.equal( domEditable );
+			const domSelection = document.getSelection();
+			expect( domSelection.rangeCount ).to.equal( 1 );
+			const domRange = domSelection.getRangeAt( 0 );
+			expect( domRange.startContainer ).to.equal( domEditable );
+			expect( domRange.startOffset ).to.equal( 0 );
+			expect( domRange.collapsed ).to.be.true;
+		} );
+
+		it( 'should not focus if document is already focused', () => {
+			const converterFocusSpy = testUtils.sinon.spy( viewDocument.domConverter, 'focus' );
+			const renderSpy = testUtils.sinon.spy( viewDocument, 'render' );
+			viewDocument.isFocused = true;
+
+			viewDocument.focus();
+
+			expect( converterFocusSpy.called ).to.be.false;
+			expect( renderSpy.called ).to.be.false;
+		} );
+
+		it( 'should log warning when no selection', () => {
+			const logSpy = testUtils.sinon.stub( log, 'warn' );
+			viewDocument.selection.removeAllRanges();
+
+			viewDocument.focus();
+			expect( logSpy.calledOnce ).to.be.true;
+			expect( logSpy.args[ 0 ][ 0 ] ).to.match( /^view-focus-no-selection/ );
+		} );
+	} );
 } );

+ 39 - 2
packages/ckeditor5-engine/tests/view/domconverter/domconverter.js

@@ -8,18 +8,22 @@
 'use strict';
 
 import DomConverter from '/ckeditor5/engine/view/domconverter.js';
+import ViewEditable from '/ckeditor5/engine/view/editableelement.js';
+import ViewDocument from '/ckeditor5/engine/view/document.js';
 import { BR_FILLER, NBSP_FILLER } from '/ckeditor5/engine/view/filler.js';
+import testUtils from '/tests/ckeditor5/_utils/utils.js';
+
+testUtils.createSinonSandbox();
 
 describe( 'DomConverter', () => {
 	let converter;
 
-	before( () => {
+	beforeEach( () => {
 		converter = new DomConverter();
 	} );
 
 	describe( 'constructor', () => {
 		it( 'should create converter with BR block filler by default', () => {
-			converter = new DomConverter();
 			expect( converter.blockFiller ).to.equal( BR_FILLER );
 		} );
 
@@ -28,4 +32,37 @@ describe( 'DomConverter', () => {
 			expect( converter.blockFiller ).to.equal( NBSP_FILLER );
 		} );
 	} );
+
+	describe( 'focus', () => {
+		let viewEditable, domEditable;
+
+		beforeEach( () => {
+			viewEditable = new ViewEditable( new ViewDocument(), 'div' );
+			domEditable = document.createElement( 'div' );
+			converter.bindElements( domEditable, viewEditable );
+			domEditable.setAttribute( 'contenteditable', 'true' );
+			document.body.appendChild( domEditable );
+		} );
+
+		afterEach( () => {
+			document.body.removeChild( domEditable );
+		} );
+
+		it( 'should call focus on corresponding DOM editable', () => {
+			const focusSpy = testUtils.sinon.spy( domEditable, 'focus' );
+
+			converter.focus( viewEditable );
+
+			expect( focusSpy.calledOnce ).to.be.true;
+		} );
+
+		it( 'should not focus already focused editable', () => {
+			const focusSpy = testUtils.sinon.spy( domEditable, 'focus' );
+
+			converter.focus( viewEditable );
+			converter.focus( viewEditable );
+
+			expect( focusSpy.calledOnce ).to.be.true;
+		} );
+	} );
 } );