Merge pull request #50 from ckeditor/t/48

Other: Code refactoring in editor-classic to share API with editor-inline. Closes #48.

Author: Reinmar

src/classiceditorui.js

@@ -9,9 +9,12 @@
 
 import ComponentFactory from '@ckeditor/ckeditor5-ui/src/componentfactory';
 import FocusTracker from '@ckeditor/ckeditor5-utils/src/focustracker';
+import enableToolbarKeyboardFocus from '@ckeditor/ckeditor5-ui/src/toolbar/enabletoolbarkeyboardfocus';
 
 /**
  * The classic editor UI class.
+ *
+ * @implements module:core/editor/editorui~EditorUI
  */
 export default class ClassicEditorUI {
 	/**
@@ -22,34 +25,22 @@ export default class ClassicEditorUI {
 	 */
 	constructor( editor, view ) {
 		/**
-		 * Editor that the UI belongs to.
-		 *
-		 * @readonly
-		 * @member {module:core/editor/editor~Editor}
+		 * @inheritDoc
 		 */
 		this.editor = editor;
 
 		/**
-		 * View of the ui.
-		 *
-		 * @readonly
-		 * @member {module:ui/editorui/editoruiview~EditorUIView}
+		 * @inheritDoc
 		 */
 		this.view = view;
 
 		/**
-		 * Instance of the {@link module:ui/componentfactory~ComponentFactory}.
-		 *
-		 * @readonly
-		 * @member {module:ui/componentfactory~ComponentFactory}
+		 * @inheritDoc
 		 */
 		this.componentFactory = new ComponentFactory( editor );
 
 		/**
-		 * Keeps information about editor focus.
-		 *
-		 * @readonly
-		 * @member {module:utils/focustracker~FocusTracker}
+		 * @inheritDoc
 		 */
 		this.focusTracker = new FocusTracker();
 
@@ -79,36 +70,14 @@ export default class ClassicEditorUI {
 
 		return this.view.init()
 			.then( () => {
-				const toolbarConfig = editor.config.get( 'toolbar' );
-				const promises = [];
-
-				if ( toolbarConfig ) {
-					promises.push( this.view.toolbar.fillFromConfig( toolbarConfig, this.componentFactory ) );
-				}
-
-				return Promise.all( promises );
+				return this.view.toolbar.fillFromConfig( editor.config.get( 'toolbar' ), this.componentFactory );
 			} )
 			.then( () => {
-				const toolbarFocusTracker = this.view.toolbar.focusTracker;
-
-				// Because toolbar items can get focus, the overall state of
-				// the toolbar must also be tracked.
-				this.focusTracker.add( this.view.toolbar.element );
-
-				// Focus the toolbar on the keystroke, if not already focused.
-				editor.keystrokes.set( 'Alt+F10', ( data, cancel ) => {
-					if ( this.focusTracker.isFocused && !toolbarFocusTracker.isFocused ) {
-						this.view.toolbar.focus();
-						cancel();
-					}
-				} );
-
-				// Blur the toolbar and bring the focus back to editable on the keystroke.
-				this.view.toolbar.keystrokes.set( 'Esc', ( data, cancel ) => {
-					if ( toolbarFocusTracker.isFocused ) {
-						editor.editing.view.focus();
-						cancel();
-					}
+				enableToolbarKeyboardFocus( {
+					origin: editor.editing.view,
+					originFocusTracker: this.focusTracker,
+					originKeystrokeHandler: editor.keystrokes,
+					toolbar: this.view.toolbar
 				} );
 			} );
 	}

tests/classiceditorui.js

@@ -13,7 +13,7 @@ import ClassicEditorUIView from '../src/classiceditoruiview';
 import ClassicTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/classictesteditor';
 
 import FocusTracker from '@ckeditor/ckeditor5-utils/src/focustracker';
-import { keyCodes } from '@ckeditor/ckeditor5-utils/src/keyboard';
+import * as enableToolbarKeyboardFocus from '@ckeditor/ckeditor5-ui/src/toolbar/enabletoolbarkeyboardfocus';
 
 import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils';
 import utils from '@ckeditor/ckeditor5-utils/tests/_utils/utils';
@@ -139,69 +139,22 @@ describe( 'ClassicEditorUI', () => {
 		} );
 
 		it( 'fills view.toolbar#items with editor config', () => {
+			const spy = testUtils.sinon.spy( view.toolbar, 'fillFromConfig' );
+
 			return ui.init().then( () => {
-				expect( view.toolbar.items ).to.have.length( 2 );
-				expect( view.toolbar.items.get( 0 ).name ).to.equal( 'foo' );
-				expect( view.toolbar.items.get( 1 ).name ).to.equal( 'bar' );
+				sinon.assert.calledWithExactly( spy, editor.config.get( 'toolbar' ), ui.componentFactory );
 			} );
 		} );
 
-		describe( 'activates keyboard navigation for the toolbar', () => {
-			it( 'Alt+F10: focus the first focusable toolbar item', () => {
-				return ui.init().then( () => {
-					const spy = sinon.spy( view.toolbar, 'focus' );
-					const toolbarFocusTracker = view.toolbar.focusTracker;
-					const keyEvtData = {
-						keyCode: keyCodes.f10,
-						altKey: true,
-						preventDefault: sinon.spy(),
-						stopPropagation: sinon.spy()
-					};
-
-					toolbarFocusTracker.isFocused = false;
-					ui.focusTracker.isFocused = false;
-
-					editor.keystrokes.press( keyEvtData );
-					sinon.assert.notCalled( spy );
-
-					toolbarFocusTracker.isFocused = true;
-					ui.focusTracker.isFocused = true;
-
-					editor.keystrokes.press( keyEvtData );
-					sinon.assert.notCalled( spy );
-
-					toolbarFocusTracker.isFocused = false;
-					ui.focusTracker.isFocused = true;
-
-					editor.keystrokes.press( keyEvtData );
-					sinon.assert.calledOnce( spy );
-
-					sinon.assert.calledOnce( keyEvtData.preventDefault );
-					sinon.assert.calledOnce( keyEvtData.stopPropagation );
-				} );
-			} );
-
-			it( 'esc: re–foucus editable when toolbar is focused', () => {
-				return ui.init().then( () => {
-					const spy = sinon.spy( editor.editing.view, 'focus' );
-					const toolbarFocusTracker = view.toolbar.focusTracker;
-					const keyEvtData = { keyCode: keyCodes.esc,
-						preventDefault: sinon.spy(),
-						stopPropagation: sinon.spy()
-					};
-
-					toolbarFocusTracker.isFocused = false;
-
-					ui.view.toolbar.keystrokes.press( keyEvtData );
-					sinon.assert.notCalled( spy );
+		it( 'initializes keyboard navigation between view#toolbar and view#editable', () => {
+			const spy = testUtils.sinon.spy( enableToolbarKeyboardFocus, 'default' );
 
-					toolbarFocusTracker.isFocused = true;
-
-					ui.view.toolbar.keystrokes.press( keyEvtData );
-
-					sinon.assert.calledOnce( spy );
-					sinon.assert.calledOnce( keyEvtData.preventDefault );
-					sinon.assert.calledOnce( keyEvtData.stopPropagation );
+			return ui.init().then( () => {
+				sinon.assert.calledWithExactly( spy, {
+					origin: editor.editing.view,
+					originFocusTracker: ui.focusTracker,
+					originKeystrokeHandler: editor.keystrokes,
+					toolbar: view.toolbar
 				} );
 			} );
 		} );

tests/manual/classic.html

@@ -8,6 +8,16 @@ <h2>Hello world!</h2>
 	<p>This is an editor instance.</p>
 </div>
 
-<div style="height: 1500px; width: 1500px; background: #eee; outline: 2px dashed #ddd; margin: 1em 0; padding: 1em;">
-	* I'm a dummy div to enable scrolling in this test. *
-</div>
+<style>
+	body {
+		width: 10000px;
+		height: 10000px;
+	}
+
+	.ck-editor {
+		margin-top: 200px;
+		margin-left: 100px;
+		margin-bottom: 200px;
+		width: 450px;
+	}
+</style>