Skip to content
This repository has been archived by the owner on Jun 26, 2020. It is now read-only.

Commit

Permalink
Merge pull request #181 from ckeditor/t/ckeditor5/447
Browse files Browse the repository at this point in the history
Feature: `KeystrokeHandler` should support priorities and proper keystroke cancelling. Closes #180.
  • Loading branch information
Reinmar authored Aug 21, 2017
2 parents 8c131a9 + a5018b6 commit 14af24c
Show file tree
Hide file tree
Showing 2 changed files with 110 additions and 89 deletions.
76 changes: 39 additions & 37 deletions src/keystrokehandler.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ import { getCode, parseKeystroke } from './keyboard';
*
* handler.listenTo( emitter );
*
* handler.set( 'ctrl + a', ( keyEventData, cancel ) => {
* console.log( 'ctrl + a has been pressed' );
* handler.set( 'Ctrl+A', ( keyEvtData, cancel ) => {
* console.log( 'Ctrl+A has been pressed' );
* cancel();
* } );
*/
Expand All @@ -36,14 +36,6 @@ export default class KeystrokeHandler {
* @member {module:utils/dom/emittermixin~Emitter}
*/
this._listener = Object.create( DomEmitterMixin );

/**
* Map of the defined keystrokes. Keystroke codes are the keys.
*
* @private
* @member {Map}
*/
this._keystrokes = new Map();
}

/**
Expand All @@ -52,8 +44,17 @@ export default class KeystrokeHandler {
* @param {module:utils/emittermixin~Emitter} emitter
*/
listenTo( emitter ) {
this._listener.listenTo( emitter, 'keydown', ( evt, data ) => {
this.press( data );
// The #_listener works here as a kind of dispatcher. It groups the events coming from the same
// keystroke so the listeners can be attached to them with different priorities.
//
// E.g. all the keystrokes with the `keyCode` of 42 coming from the `emitter` are propagated
// as a `_keydown:42` event by the `_listener`. If there's a callback created by the `set`
// method for this 42 keystroke, it listens to the `_listener#_keydown:42` event only and interacts
// only with other listeners of this particular event, thus making it possible to prioritize
// the listeners and safely cancel execution, when needed. Instead of duplicating the Emitter logic,
// the KeystrokeHandler re–uses it to do its job.
this._listener.listenTo( emitter, 'keydown', ( evt, keyEvtData ) => {
this._listener.fire( '_keydown:' + getCode( keyEvtData ), keyEvtData );
} );
}

Expand All @@ -65,47 +66,48 @@ export default class KeystrokeHandler {
* @param {Function} callback A function called with the
* {@link module:engine/view/observer/keyobserver~KeyEventData key event data} object and
* a helper to both `preventDefault` and `stopPropagation` of the event.
* @param {Object} [options={}] Additional options.
* @param {module:utils/priorities~PriorityString|Number} [options.priority='normal'] The priority of the keystroke
* callback. The higher the priority value the sooner the callback will be executed. Keystrokes having the same priority
* are called in the order they were added.
*/
set( keystroke, callback ) {
set( keystroke, callback, options = {} ) {
const keyCode = parseKeystroke( keystroke );
const callbacks = this._keystrokes.get( keyCode );
const priority = options.priority;

// Execute the passed callback on KeystrokeHandler#_keydown.
// TODO: https://github.com/ckeditor/ckeditor5-utils/issues/144
this._listener.listenTo( this._listener, '_keydown:' + keyCode, ( evt, keyEvtData ) => {
callback( keyEvtData, () => {
// Stop the event in the DOM: no listener in the web page
// will be triggered by this event.
keyEvtData.preventDefault();
keyEvtData.stopPropagation();

if ( callbacks ) {
callbacks.push( callback );
} else {
this._keystrokes.set( keyCode, [ callback ] );
}
// Stop the event in the KeystrokeHandler: no more callbacks
// will be executed for this keystroke.
evt.stop();
} );

// Mark this keystroke as handled by the callback. See: #press.
evt.return = true;
}, { priority } );
}

/**
* Triggers a keystroke handler for a specified key combination, if such a keystroke was {@link #set defined}.
*
* @param {module:engine/view/observer/keyobserver~KeyEventData} keyEventData Key event data.
* @param {module:engine/view/observer/keyobserver~KeyEventData} keyEvtData Key event data.
* @returns {Boolean} Whether the keystroke was handled.
*/
press( keyEventData ) {
const keyCode = getCode( keyEventData );
const callbacks = this._keystrokes.get( keyCode );

if ( !callbacks ) {
return false;
}

for ( const callback of callbacks ) {
callback( keyEventData, () => {
keyEventData.preventDefault();
keyEventData.stopPropagation();
} );
}

return true;
press( keyEvtData ) {
return !!this._listener.fire( '_keydown:' + getCode( keyEvtData ), keyEvtData );
}

/**
* Destroys the keystroke handler.
*/
destroy() {
this._keystrokes = new Map();
this._listener.stopListening();
}
}
123 changes: 71 additions & 52 deletions tests/keystrokehandler.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,31 +19,14 @@ describe( 'KeystrokeHandler', () => {

describe( 'listenTo()', () => {
it( 'activates the listening on the emitter', () => {
emitter = Object.create( EmitterMixin );
keystrokes = new KeystrokeHandler();

const spy = sinon.spy( keystrokes, 'press' );
const keyEvtData = { keyCode: 1 };

emitter.fire( 'keydown', keyEvtData );

expect( spy.notCalled ).to.be.true;

keystrokes.listenTo( emitter );
emitter.fire( 'keydown', keyEvtData );

sinon.assert.calledOnce( spy );
sinon.assert.calledWithExactly( spy, keyEvtData );
} );

it( 'triggers #press on #keydown', () => {
const spy = sinon.spy( keystrokes, 'press' );
const keyEvtData = { keyCode: 1 };
const spy = sinon.spy();
const keyEvtData = getCtrlA();

keystrokes.set( 'Ctrl+A', spy );
emitter.fire( 'keydown', keyEvtData );

sinon.assert.calledOnce( spy );
sinon.assert.calledWithExactly( spy, keyEvtData );
sinon.assert.calledWithExactly( spy, keyEvtData, sinon.match.func );
} );
} );

Expand All @@ -52,7 +35,7 @@ describe( 'KeystrokeHandler', () => {
const spy = sinon.spy();
const keyEvtData = getCtrlA();

keystrokes.set( 'ctrl + A', spy );
keystrokes.set( 'Ctrl+A', spy );

const wasHandled = keystrokes.press( keyEvtData );

Expand All @@ -61,31 +44,6 @@ describe( 'KeystrokeHandler', () => {
expect( wasHandled ).to.be.true;
} );

it( 'provides a callback which both preventDefault and stopPropagation', done => {
const keyEvtData = getCtrlA();

Object.assign( keyEvtData, {
preventDefault: sinon.spy(),
stopPropagation: sinon.spy()
} );

keystrokes.set( 'ctrl + A', ( data, cancel ) => {
expect( data ).to.equal( keyEvtData );

sinon.assert.notCalled( keyEvtData.preventDefault );
sinon.assert.notCalled( keyEvtData.stopPropagation );

cancel();

sinon.assert.calledOnce( keyEvtData.preventDefault );
sinon.assert.calledOnce( keyEvtData.stopPropagation );

done();
} );

emitter.fire( 'keydown', keyEvtData );
} );

it( 'returns false when no handler', () => {
const keyEvtData = getCtrlA();

Expand All @@ -99,7 +57,7 @@ describe( 'KeystrokeHandler', () => {
it( 'handles array format', () => {
const spy = sinon.spy();

keystrokes.set( [ 'ctrl', 'A' ], spy );
keystrokes.set( [ 'Ctrl', 'A' ], spy );

expect( keystrokes.press( getCtrlA() ) ).to.be.true;
} );
Expand All @@ -108,14 +66,70 @@ describe( 'KeystrokeHandler', () => {
const spy1 = sinon.spy();
const spy2 = sinon.spy();

keystrokes.set( [ 'ctrl', 'A' ], spy1 );
keystrokes.set( [ 'ctrl', 'A' ], spy2 );
keystrokes.set( [ 'Ctrl', 'A' ], spy1 );
keystrokes.set( [ 'Ctrl', 'A' ], spy2 );

keystrokes.press( getCtrlA() );

sinon.assert.calledOnce( spy1 );
sinon.assert.calledOnce( spy2 );
} );

it( 'supports priorities', () => {
const spy1 = sinon.spy();
const spy2 = sinon.spy();
const spy3 = sinon.spy();
const spy4 = sinon.spy();

keystrokes.set( [ 'Ctrl', 'A' ], spy1 );
keystrokes.set( [ 'Ctrl', 'A' ], spy2, { priority: 'high' } );
keystrokes.set( [ 'Ctrl', 'A' ], spy3, { priority: 'low' } );
keystrokes.set( [ 'Ctrl', 'A' ], spy4 );

keystrokes.press( getCtrlA() );

sinon.assert.callOrder( spy2, spy1, spy4, spy3 );
} );

it( 'provides a callback which causes preventDefault and stopPropagation in the DOM', done => {
const keyEvtData = getCtrlA();

keystrokes.set( 'Ctrl+A', ( data, cancel ) => {
expect( data ).to.equal( keyEvtData );

sinon.assert.notCalled( keyEvtData.preventDefault );
sinon.assert.notCalled( keyEvtData.stopPropagation );

cancel();

sinon.assert.calledOnce( keyEvtData.preventDefault );
sinon.assert.calledOnce( keyEvtData.stopPropagation );

done();
} );

emitter.fire( 'keydown', keyEvtData );
} );

it( 'provides a callback which stops the event and remaining callbacks in the keystroke handler', () => {
const spy1 = sinon.spy();
const spy2 = sinon.spy();
const spy3 = sinon.spy();
const spy4 = sinon.spy();

keystrokes.set( [ 'Ctrl', 'A' ], spy1 );
keystrokes.set( [ 'Ctrl', 'A' ], spy2, { priority: 'high' } );
keystrokes.set( [ 'Ctrl', 'A' ], spy3, { priority: 'low' } );
keystrokes.set( [ 'Ctrl', 'A' ], ( keyEvtData, cancel ) => {
spy4();
cancel();
} );

keystrokes.press( getCtrlA() );

sinon.assert.callOrder( spy2, spy1, spy4 );
sinon.assert.notCalled( spy3 );
} );
} );

describe( 'destroy()', () => {
Expand All @@ -133,7 +147,7 @@ describe( 'KeystrokeHandler', () => {
const spy = sinon.spy();
const keystrokeHandler = keystrokes;

keystrokeHandler.set( 'ctrl + A', spy );
keystrokeHandler.set( 'Ctrl+A', spy );

keystrokeHandler.destroy();

Expand All @@ -146,5 +160,10 @@ describe( 'KeystrokeHandler', () => {
} );

function getCtrlA() {
return { keyCode: keyCodes.a, ctrlKey: true };
return {
keyCode: keyCodes.a,
ctrlKey: true,
preventDefault: sinon.spy(),
stopPropagation: sinon.spy()
};
}

0 comments on commit 14af24c

Please sign in to comment.