From ebee8e699c57b5b5a5d4c2c52d82d7c597d52209 Mon Sep 17 00:00:00 2001 From: Marek Lewandowski Date: Thu, 2 Jun 2016 10:52:27 +0200 Subject: [PATCH 1/4] Fixed #200 - Pressing space in close button will close the panel. --- ui/Viewer.js | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/ui/Viewer.js b/ui/Viewer.js index c51a13a4d..7b0c8d7d4 100644 --- a/ui/Viewer.js +++ b/ui/Viewer.js @@ -91,6 +91,28 @@ define( [ }, this ); } ); + this.panel.addShowListener( function() { + return this.parts.close.on( 'keydown', function( evt ) { + var keystroke = evt.data.getKeystroke(); + + // Hide the panel on space key press in close button. + if ( keystroke == 32 ) { + this.blur(); + this.hide(); + evt.data.preventDefault(); + } + }, this ); + } ); + + // Hide the panel once the closing X is clicked. + this.panel.addShowListener( function() { + return this.parts.close.on( 'click', function( evt ) { + this.blur(); + this.hide(); + evt.data.preventDefault(); + }, this ); + } ); + this.panel.addShowListener( function() { return this.parts.panel.on( 'keydown', function( evt ) { var keystroke = evt.data.getKeystroke(); From 6d37156ab07c0934e374559429a72e039e7703eb Mon Sep 17 00:00:00 2001 From: Tadeusz Piskozub Date: Thu, 2 Jun 2016 11:57:19 +0200 Subject: [PATCH 2/4] Tests: move getKeyEvent to separate module. --- tests/_helpers/keyEvent.js | 25 +++++++++++++++++++++++++ tests/ui/ViewerController.focus.js | 26 +++----------------------- 2 files changed, 28 insertions(+), 23 deletions(-) create mode 100644 tests/_helpers/keyEvent.js diff --git a/tests/_helpers/keyEvent.js b/tests/_helpers/keyEvent.js new file mode 100644 index 000000000..7bd52e8c3 --- /dev/null +++ b/tests/_helpers/keyEvent.js @@ -0,0 +1,25 @@ +define( function() { + 'use strict'; + + return { + // Returns a key event mockup. + // @param {Number} keystroke Keystroke with modifiers. eg. CKEDITOR.SHIFT + 9 // shift + tab + getKeyEvent: function( keystroke ) { + // This fancy construction will remove modifier bits. + var key = keystroke & ~( CKEDITOR.CTRL | CKEDITOR.ALT | CKEDITOR.SHIFT ); + + return { + getKey: function() { + return key; + }, + getKeystroke: function() { + return keystroke; + }, + stopPropagation: function() { + }, + preventDefault: function() { + } + }; + } + } +} ); diff --git a/tests/ui/ViewerController.focus.js b/tests/ui/ViewerController.focus.js index 0532bb86d..6d510809d 100644 --- a/tests/ui/ViewerController.focus.js +++ b/tests/ui/ViewerController.focus.js @@ -12,7 +12,7 @@ // Note that we have an extra (unused) requirement for 'EngineMock' and 'Controller' classes. // That way it will force them to be available for the editor, and we have sure that a11ychecker // plugin will be ready synchronously. - bender.require( [ 'testSuite', 'EngineMock' ], function( testSuite, EngineMock ) { + bender.require( [ 'testSuite', 'EngineMock', 'helpers/keyEvent' ], function( testSuite, EngineMock, keyEvent ) { testSuite.useEngine( EngineMock ); bender.editors = { @@ -136,7 +136,7 @@ // Will focus the last button. initialFocusElem.focus(); - initialFocusElem.fire( 'keydown', getKeyEvent( 9 ) ); + initialFocusElem.fire( 'keydown', keyEvent.getKeyEvent( 9 ) ); assert.areSame( expectedFocusElem, CKEDITOR.document.getActive(), 'Invalid element focused' ); }, @@ -156,7 +156,7 @@ window.setTimeout( function() { resume( function() { - initialFocusElem.fire( 'keydown', getKeyEvent( CKEDITOR.SHIFT + 9 ) ); + initialFocusElem.fire( 'keydown', keyEvent.getKeyEvent( CKEDITOR.SHIFT + 9 ) ); var activeElement = CKEDITOR.document.getActive(); assert.areSame( expectedFocusElem, activeElement, 'Invalid element focused' ); @@ -208,26 +208,6 @@ return viewer.form.parts.ignoreButton; } - // Returns a key event mockup. - // @param {Number} keystroke Keystroke with modifiers. eg. CKEDITOR.SHIFT + 9 // shift + tab - function getKeyEvent( keystroke ) { - // This fancy construction will remove modifier bits. - var key = keystroke & ~( CKEDITOR.CTRL | CKEDITOR.ALT | CKEDITOR.SHIFT ); - - return { - getKey: function() { - return key; - }, - getKeystroke: function() { - return keystroke; - }, - stopPropagation: function() { - }, - preventDefault: function() { - } - }; - } - testSuite.testEditors( bender.editors, tests ); } ); } )(); From 9466b9b5079dafcb217da144dc3311cba720006d Mon Sep 17 00:00:00 2001 From: Tadeusz Piskozub Date: Thu, 2 Jun 2016 11:57:37 +0200 Subject: [PATCH 3/4] Tests: add unit test. --- tests/ui/Viewer.js | 56 +++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 53 insertions(+), 3 deletions(-) diff --git a/tests/ui/Viewer.js b/tests/ui/Viewer.js index c12fbe6c9..7350c6cff 100644 --- a/tests/ui/Viewer.js +++ b/tests/ui/Viewer.js @@ -2,14 +2,47 @@ * @license Copyright (c) 2014-2016, CKSource - Frederico Knabben. All rights reserved. * For licensing, see LICENSE.md or http://ckeditor.com/license */ +/* bender-ckeditor-plugins: a11ychecker,toolbar,undo */ /* bender-tags: a11ychecker,unit */ ( function() { 'use strict'; - bender.require( [ 'ui/Viewer', 'ui/ViewerDescription', 'mocking' ], function( Viewer, ViewerDescription, mocking ) { + bender.require( [ 'ui/Viewer', 'ui/ViewerDescription', 'mocking', 'EngineMock', 'testSuite', 'helpers/keyEvent' ], function( Viewer, ViewerDescription, mocking, EngineMock, testSuite, keyEvent ) { + testSuite.useEngine( EngineMock ); + + bender.editors = { + classic: { + name: 'editor1', + startupData: '

foo

' + }, + inline: { + name: 'editor2', + creator: 'inline', + startupData: '

foo

' + } + }; + + var tests = { + tearDown: function() { + // For each test a11ychecker needs to be closed. + // Note that we have 2 editor instances, but only 1 can be enabled at + // a time. + function cleanupAC( editor ) { + var a11ychecker = editor._.a11ychecker; + + if ( a11ychecker.issues && a11ychecker.issues.getFocused() ) { + // @todo: it might be worth to investigate what's causing wrong selection if we won't call resetFocus(). + a11ychecker.issues.resetFocus(); + } + + a11ychecker.close(); + } + + cleanupAC( this.editors.classic ); + cleanupAC( this.editors.inline ); + }, - bender.test( { 'test Viewer.setupDescription': function() { var mock = { setupDescription: Viewer.prototype.setupDescription @@ -31,7 +64,24 @@ // Ensure that append method was called for elem in panel.parts. assert.areSame( 1, appendMock.callCount, 'panel.parts.content.append call count' ); mocking.assert.alwaysCalledWith( appendMock, mock.description.parts.wrapper ); + }, + 'test Viewer close with hotkey': function( editor ) { + var a11ychecker = editor._.a11ychecker, + viewer = a11ychecker.viewerController.viewer, + closeButton = viewer.panel.parts.close; + + viewer.panel.blur = mocking.spy(); + viewer.panel.hide = mocking.spy(); + + a11ychecker.exec(); + + closeButton.fire( 'keydown', keyEvent.getKeyEvent( 32 ) ); + + assert.isTrue( viewer.panel.blur.called ); + assert.isTrue( viewer.panel.hide.called ); } - } ); + }; + + testSuite.testEditors( bender.editors, tests ); } ); } )(); From b9782e9db89bfbed16772b8abcebb0424239462e Mon Sep 17 00:00:00 2001 From: Tadeusz Piskozub Date: Thu, 2 Jun 2016 13:15:50 +0200 Subject: [PATCH 4/4] Tests: add space-closing-AC case to manual test. --- tests/manual/a11ychecker.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/manual/a11ychecker.md b/tests/manual/a11ychecker.md index ecd9dadcd..e43881615 100644 --- a/tests/manual/a11ychecker.md +++ b/tests/manual/a11ychecker.md @@ -13,4 +13,5 @@ Some hints: - Instead of using "Quick fix", click on the issue source element to edit it manually. A popup should appear in the right-bottom corner of the window. - Ignore an issue, close the Accessibility Checker dialog and run AC again to find, if the issue is still ignored. -- Ensure that `esc` key closes the Accessiblity Checker balloon and puts selection on the issue source element. \ No newline at end of file +- Ensure that `esc` key closes the Accessiblity Checker balloon and puts selection on the issue source element. +- Open Accessiblity Checker, press `Shift+Tab` twice. The close button should be focused. Ensure that in this state pressing the `space` key closes Accessiblity Checker.