Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

T/200 #206

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

T/200 #206

Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions tests/_helpers/keyEvent.js
Original file line number Diff line number Diff line change
@@ -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() {
}
};
}
}
} );
3 changes: 2 additions & 1 deletion tests/manual/a11ychecker.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
- 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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, put the last TC "- 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." into a dedicated manual test case.

Create a .md file with tc tag, and target AC version (1.0.1).

56 changes: 53 additions & 3 deletions tests/ui/Viewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need undo plugin?

/* 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: '<p>foo</p>'
},
inline: {
name: 'editor2',
creator: 'inline',
startupData: '<p>foo</p>'
}
};

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() ) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to reset the focus in this test suite? I don't think it's explicitly focused in any of these tests. With that cleanupAC might be removed and we can simply call this.editors.classic._.a11ychecker.close() and same for inline.

// @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
Expand All @@ -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 );
} );
} )();
26 changes: 3 additions & 23 deletions tests/ui/ViewerController.focus.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down Expand Up @@ -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' );
},
Expand All @@ -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' );
Expand Down Expand Up @@ -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 );
} );
} )();
22 changes: 22 additions & 0 deletions ui/Viewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This 3 lines of code:

this.blur();
this.hide();
evt.data.preventDefault();

are used in line 88, 100, 122, maybe we could extract it to the separated method, e.g. hidePanel/closePanel ?

}
}, this );
} );

// Hide the panel once the closing X is clicked.
this.panel.addShowListener( function() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You've added another click listener here, it's not needed.

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();
Expand Down