Kaynağa Gözat

Merge branch 'master' into t/134

Szymon Kupś 8 yıl önce
ebeveyn
işleme
4aa100562b

+ 0 - 7
packages/ckeditor5-image/src/imagetextalternative.js

@@ -129,13 +129,6 @@ export default class ImageTextAlternative extends Plugin {
 			}
 		}, { priority: 'low' } );
 
-		// Hide the form when the editor is blurred.
-		this.listenTo( editor.ui.focusTracker, 'change:isFocused', ( evt, name, is ) => {
-			if ( !is ) {
-				this._hideForm();
-			}
-		}, { priority: 'low' } );
-
 		// Close on click outside of balloon panel element.
 		clickOutsideHandler( {
 			emitter: this._form,

+ 56 - 2
packages/ckeditor5-image/src/imagetextalternative/ui/textalternativeformview.js

@@ -8,12 +8,17 @@
  */
 
 import View from '@ckeditor/ckeditor5-ui/src/view';
-import ButtonView from '@ckeditor/ckeditor5-ui/src/button/buttonview';
 import Template from '@ckeditor/ckeditor5-ui/src/template';
+import ViewCollection from '@ckeditor/ckeditor5-ui/src/viewcollection';
+
+import ButtonView from '@ckeditor/ckeditor5-ui/src/button/buttonview';
 import LabeledInputView from '@ckeditor/ckeditor5-ui/src/labeledinput/labeledinputview';
 import InputTextView from '@ckeditor/ckeditor5-ui/src/inputtext/inputtextview';
+
 import submitHandler from '@ckeditor/ckeditor5-ui/src/bindings/submithandler';
 import KeystrokeHandler from '@ckeditor/ckeditor5-utils/src/keystrokehandler';
+import FocusTracker from '@ckeditor/ckeditor5-utils/src/focustracker';
+import FocusCycler from '@ckeditor/ckeditor5-ui/src/focuscycler';
 
 /**
  * The TextAlternativeFormView class.
@@ -30,6 +35,14 @@ export default class TextAlternativeFormView extends View {
 		const t = this.locale.t;
 
 		/**
+		 * Tracks information about DOM focus in the form.
+		 *
+		 * @readonly
+		 * @member {module:utils/focustracker~FocusTracker}
+		 */
+		this.focusTracker = new FocusTracker();
+
+		/**
 		 * An instance of the {@link module:utils/keystrokehandler~KeystrokeHandler}.
 		 *
 		 * @readonly
@@ -59,6 +72,35 @@ export default class TextAlternativeFormView extends View {
 		 */
 		this.cancelButtonView = this._createButton( t( 'Cancel' ), 'cancel' );
 
+		/**
+		 * A collection of views which can be focused in the form.
+		 *
+		 * @readonly
+		 * @protected
+		 * @member {module:ui/viewcollection~ViewCollection}
+		 */
+		this._focusables = new ViewCollection();
+
+		/**
+		 * Helps cycling over {@link #_focusables} in the form.
+		 *
+		 * @readonly
+		 * @protected
+		 * @member {module:ui/focuscycler~FocusCycler}
+		 */
+		this._focusCycler = new FocusCycler( {
+			focusables: this._focusables,
+			focusTracker: this.focusTracker,
+			keystrokeHandler: this.keystrokes,
+			actions: {
+				// Navigate form fields backwards using the Shift + Tab keystroke.
+				focusPrevious: 'shift + tab',
+
+				// Navigate form fields forwards using the Tab key.
+				focusNext: 'tab'
+			}
+		} );
+
 		Template.extend( this.saveButtonView.template, {
 			attributes: {
 				class: [
@@ -73,7 +115,10 @@ export default class TextAlternativeFormView extends View {
 			attributes: {
 				class: [
 					'cke-text-alternative-form',
-				]
+				],
+
+				// https://github.com/ckeditor/ckeditor5-image/issues/40
+				tabindex: '-1'
 			},
 
 			children: [
@@ -98,6 +143,15 @@ export default class TextAlternativeFormView extends View {
 		submitHandler( {
 			view: this
 		} );
+
+		[ this.labeledInput, this.saveButtonView, this.cancelButtonView ]
+			.forEach( v => {
+				// Register the view as focusable.
+				this._focusables.add( v );
+
+				// Register the view in the focus tracker.
+				this.focusTracker.add( v.element );
+			} );
 	}
 
 	/**

+ 0 - 14
packages/ckeditor5-image/tests/imagetextalternative.js

@@ -186,20 +186,6 @@ describe( 'ImageTextAlternative', () => {
 			} );
 		} );
 
-		describe( 'integration with the editor focus', () => {
-			it( 'should hide the toolbar when the editor loses focus and the image is selected', () => {
-				editor.ui.focusTracker.isFocused = true;
-
-				setData( doc, '[<image src=""></image>]' );
-				button.fire( 'execute' );
-
-				const spy = sinon.spy( balloon, 'remove' );
-
-				editor.ui.focusTracker.isFocused = false;
-				sinon.assert.calledWithExactly( spy, form );
-			} );
-		} );
-
 		describe( 'close listeners', () => {
 			describe( 'keyboard', () => {
 				it( 'should close upon Esc key press and focus the editing view', () => {

+ 81 - 0
packages/ckeditor5-image/tests/imagetextalternative/ui/textalternativeformview.js

@@ -5,9 +5,16 @@
 
 /* global Event */
 
+import { keyCodes } from '@ckeditor/ckeditor5-utils/src/keyboard';
 import TextAlternativeFormView from '../../../src/imagetextalternative/ui/textalternativeformview';
 import View from '@ckeditor/ckeditor5-ui/src/view';
 import KeystrokeHandler from '@ckeditor/ckeditor5-utils/src/keystrokehandler';
+import FocusTracker from '@ckeditor/ckeditor5-utils/src/focustracker';
+import FocusCycler from '@ckeditor/ckeditor5-ui/src/focuscycler';
+import ViewCollection from '@ckeditor/ckeditor5-ui/src/viewcollection';
+import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils';
+
+testUtils.createSinonSandbox();
 
 describe( 'TextAlternativeFormView', () => {
 	let view;
@@ -21,6 +28,11 @@ describe( 'TextAlternativeFormView', () => {
 	describe( 'constructor()', () => {
 		it( 'should create element from template', () => {
 			expect( view.element.classList.contains( 'cke-text-alternative-form' ) ).to.be.true;
+			expect( view.element.getAttribute( 'tabindex' ) ).to.equal( '-1' );
+		} );
+
+		it( 'should create #focusTracker instance', () => {
+			expect( view.focusTracker ).to.be.instanceOf( FocusTracker );
 		} );
 
 		it( 'should create #keystrokes instance', () => {
@@ -40,6 +52,75 @@ describe( 'TextAlternativeFormView', () => {
 
 			sinon.assert.calledOnce( spy );
 		} );
+
+		describe( 'focus cycling and management', () => {
+			it( 'should create #_focusCycler instance', () => {
+				expect( view._focusCycler ).to.be.instanceOf( FocusCycler );
+			} );
+
+			it( 'should create #_focusables view collection', () => {
+				expect( view._focusables ).to.be.instanceOf( ViewCollection );
+			} );
+
+			it( 'should register child views in #_focusables', () => {
+				expect( view._focusables.map( f => f ) ).to.have.members( [
+					view.labeledInput,
+					view.saveButtonView,
+					view.cancelButtonView
+				] );
+			} );
+
+			it( 'should register child views\' #element in #focusTracker', () => {
+				const spy = testUtils.sinon.spy( FocusTracker.prototype, 'add' );
+
+				view = new TextAlternativeFormView( { t: () => {} } );
+
+				sinon.assert.calledWithExactly( spy.getCall( 0 ), view.labeledInput.element );
+				sinon.assert.calledWithExactly( spy.getCall( 1 ), view.saveButtonView.element );
+				sinon.assert.calledWithExactly( spy.getCall( 2 ), view.cancelButtonView.element );
+			} );
+
+			describe( 'activates keyboard navigation in the form', () => {
+				it( 'so "tab" focuses the next focusable item', () => {
+					const keyEvtData = {
+						keyCode: keyCodes.tab,
+						preventDefault: sinon.spy(),
+						stopPropagation: sinon.spy()
+					};
+
+					// Mock the url input is focused.
+					view.focusTracker.isFocused = true;
+					view.focusTracker.focusedElement = view.labeledInput.element;
+
+					const spy = sinon.spy( view.saveButtonView, 'focus' );
+
+					view.keystrokes.press( keyEvtData );
+					sinon.assert.calledOnce( keyEvtData.preventDefault );
+					sinon.assert.calledOnce( keyEvtData.stopPropagation );
+					sinon.assert.calledOnce( spy );
+				} );
+
+				it( 'so "shift + tab" focuses the previous focusable item', () => {
+					const keyEvtData = {
+						keyCode: keyCodes.tab,
+						shiftKey: true,
+						preventDefault: sinon.spy(),
+						stopPropagation: sinon.spy()
+					};
+
+					// Mock the cancel button is focused.
+					view.focusTracker.isFocused = true;
+					view.focusTracker.focusedElement = view.cancelButtonView.element;
+
+					const spy = sinon.spy( view.saveButtonView, 'focus' );
+
+					view.keystrokes.press( keyEvtData );
+					sinon.assert.calledOnce( keyEvtData.preventDefault );
+					sinon.assert.calledOnce( keyEvtData.stopPropagation );
+					sinon.assert.calledOnce( spy );
+				} );
+			} );
+		} );
 	} );
 
 	describe( 'init()', () => {

+ 5 - 0
packages/ckeditor5-image/theme/imagetextalternative/theme.scss

@@ -8,6 +8,11 @@
 		padding: ck-spacing( 'large' );
 		overflow: hidden;
 
+		&:focus {
+			// https://github.com/ckeditor/ckeditor5-image/issues/40
+			outline: none;
+		}
+
 		.ck-label {
 			margin-bottom: ck-spacing( 'small' );
 		}