diff --git a/packages/commands/src/index.ts b/packages/commands/src/index.ts index 6a5f37b61..81aad73ac 100644 --- a/packages/commands/src/index.ts +++ b/packages/commands/src/index.ts @@ -497,7 +497,7 @@ export class CommandRegistry { */ processKeydownEvent(event: KeyboardEvent): void { // Bail immediately if playing back keystrokes. - if (this._replaying) { + if (this._replaying || CommandRegistry.isModifierKeyPressed(event)) { return; } @@ -1201,6 +1201,19 @@ export namespace CommandRegistry { return mods + parts.key; } + /** + * Check if `'keydown'` event is caused by pressing a modifier key that should be ignored. + * + * @param event - The event object for a `'keydown'` event. + * + * @returns `true` if modifier key was pressed, `false` otherwise. + */ + export function isModifierKeyPressed(event: KeyboardEvent): boolean { + let layout = getKeyboardLayout(); + let key = layout.keyForKeydownEvent(event); + return layout.isModifierKey(key); + } + /** * Create a normalized keystroke for a `'keydown'` event. * @@ -1210,8 +1223,9 @@ export namespace CommandRegistry { * does not represent a valid keystroke for the given layout. */ export function keystrokeForKeydownEvent(event: KeyboardEvent): string { - let key = getKeyboardLayout().keyForKeydownEvent(event); - if (!key) { + let layout = getKeyboardLayout(); + let key = layout.keyForKeydownEvent(event); + if (!key || layout.isModifierKey(key)) { return ''; } let mods = ''; diff --git a/packages/commands/tests/src/index.spec.ts b/packages/commands/tests/src/index.spec.ts index 0430d5cc4..fd8bd8235 100644 --- a/packages/commands/tests/src/index.spec.ts +++ b/packages/commands/tests/src/index.spec.ts @@ -1007,6 +1007,55 @@ describe('@lumino/commands', () => { expect(called).to.equal(false); document.body.removeEventListener('keydown', keydown); }); + + it('should ignore modifier keys pressed in the middle of key sequence', () => { + let count = 0; + registry.addCommand('test', { + execute: () => { + count++; + } + }); + registry.addKeyBinding({ + keys: ['Ctrl K', 'Ctrl L'], + selector: `#${elem.id}`, + command: 'test' + }); + let eventK = generate('keydown', { keyCode: 75, ctrlKey: true }); + let eventCtrl = generate('keydown', { keyCode: 17, ctrlKey: true }); + let eventL = generate('keydown', { keyCode: 76, ctrlKey: true }); + elem.dispatchEvent(eventK); + expect(count).to.equal(0); + elem.dispatchEvent(eventCtrl); // user presses Ctrl again - this should not break the sequence + expect(count).to.equal(0); + elem.dispatchEvent(eventL); + expect(count).to.equal(1); + }); + + it('should process key sequences that use different modifier keys', () => { + let count = 0; + registry.addCommand('test', { + execute: () => { + count++; + } + }); + registry.addKeyBinding({ + keys: ['Shift K', 'Ctrl L'], + selector: `#${elem.id}`, + command: 'test' + }); + let eventShift = generate('keydown', { keyCode: 16, shiftlKey: true }); + let eventK = generate('keydown', { keyCode: 75, shiftKey: true }); + let eventCtrl = generate('keydown', { keyCode: 17, ctrlKey: true }); + let eventL = generate('keydown', { keyCode: 76, ctrlKey: true }); + elem.dispatchEvent(eventShift); + expect(count).to.equal(0); + elem.dispatchEvent(eventK); + expect(count).to.equal(0); + elem.dispatchEvent(eventCtrl); + expect(count).to.equal(0); + elem.dispatchEvent(eventL); + expect(count).to.equal(1); + }); }); describe('.parseKeystroke()', () => { @@ -1085,6 +1134,14 @@ describe('@lumino/commands', () => { ); expect(keystroke).to.equal(''); }); + + it('should return nothing for keys that are marked as modifier in keyboard layout', () => { + let event = generate('keydown', { keyCode: 17, ctrlKey: true }); + let keystroke = CommandRegistry.keystrokeForKeydownEvent( + event as KeyboardEvent + ); + expect(keystroke).to.equal(''); + }); }); }); }); diff --git a/packages/keyboard/src/index.ts b/packages/keyboard/src/index.ts index 1b8071e08..3e3cd5d01 100644 --- a/packages/keyboard/src/index.ts +++ b/packages/keyboard/src/index.ts @@ -39,6 +39,22 @@ export interface IKeyboardLayout { */ isValidKey(key: string): boolean; + /** + * Test whether the given key is a modifier key. + * + * @param key - The user provided key. + * + * @returns `true` if the key is a modifier key, `false` otherwise. + * + * #### Notes + * This is necessary so that we don't process modifier keys pressed + * in the middle of the key sequence. + * E.g. "Shift C Ctrl P" is actually 4 keydown events: + * "Shift", "Shift P", "Ctrl", "Ctrl P", + * and events for "Shift" and "Ctrl" should be ignored. + */ + isModifierKey(key: string): boolean; + /** * Get the key for a `'keydown'` event. * @@ -93,11 +109,18 @@ export class KeycodeLayout implements IKeyboardLayout { * @param name - The human readable name for the layout. * * @param codes - A mapping of keycode to key value. + * + * @param modifierKeys - Array of modifier key names */ - constructor(name: string, codes: KeycodeLayout.CodeMap) { + constructor( + name: string, + codes: KeycodeLayout.CodeMap, + modifierKeys: string[] = [] + ) { this.name = name; this._codes = codes; this._keys = KeycodeLayout.extractKeys(codes); + this._modifierKeys = KeycodeLayout.convertToKeySet(modifierKeys); } /** @@ -125,6 +148,17 @@ export class KeycodeLayout implements IKeyboardLayout { return key in this._keys; } + /** + * Test whether the given key is a modifier key. + * + * @param key - The user provided key. + * + * @returns `true` if the key is a modifier key, `false` otherwise. + */ + isModifierKey(key: string): boolean { + return key in this._modifierKeys; + } + /** * Get the key for a `'keydown'` event. * @@ -139,6 +173,7 @@ export class KeycodeLayout implements IKeyboardLayout { private _keys: KeycodeLayout.KeySet; private _codes: KeycodeLayout.CodeMap; + private _modifierKeys: KeycodeLayout.KeySet; } /** @@ -169,6 +204,21 @@ export namespace KeycodeLayout { } return keys as KeySet; } + + /** + * Convert array of keys to a key set. + * + * @param keys - The array that needs to be converted + * + * @returns A set of the keys in the array. + */ + export function convertToKeySet(keys: string[]): KeySet { + let keySet = Object(null); + for (let i = 0, n = keys.length; i < n; ++i) { + keySet[keys[i]] = true; + } + return keySet; + } } /** @@ -192,102 +242,111 @@ export namespace KeycodeLayout { * * Other combinations may also work, but are untested. */ -export const EN_US: IKeyboardLayout = new KeycodeLayout('en-us', { - 8: 'Backspace', - 9: 'Tab', - 13: 'Enter', - 19: 'Pause', - 27: 'Escape', - 32: 'Space', - 33: 'PageUp', - 34: 'PageDown', - 35: 'End', - 36: 'Home', - 37: 'ArrowLeft', - 38: 'ArrowUp', - 39: 'ArrowRight', - 40: 'ArrowDown', - 45: 'Insert', - 46: 'Delete', - 48: '0', - 49: '1', - 50: '2', - 51: '3', - 52: '4', - 53: '5', - 54: '6', - 55: '7', - 56: '8', - 57: '9', - 59: ';', // firefox - 61: '=', // firefox - 65: 'A', - 66: 'B', - 67: 'C', - 68: 'D', - 69: 'E', - 70: 'F', - 71: 'G', - 72: 'H', - 73: 'I', - 74: 'J', - 75: 'K', - 76: 'L', - 77: 'M', - 78: 'N', - 79: 'O', - 80: 'P', - 81: 'Q', - 82: 'R', - 83: 'S', - 84: 'T', - 85: 'U', - 86: 'V', - 87: 'W', - 88: 'X', - 89: 'Y', - 90: 'Z', - 93: 'ContextMenu', - 96: '0', // numpad - 97: '1', // numpad - 98: '2', // numpad - 99: '3', // numpad - 100: '4', // numpad - 101: '5', // numpad - 102: '6', // numpad - 103: '7', // numpad - 104: '8', // numpad - 105: '9', // numpad - 106: '*', // numpad - 107: '+', // numpad - 109: '-', // numpad - 110: '.', // numpad - 111: '/', // numpad - 112: 'F1', - 113: 'F2', - 114: 'F3', - 115: 'F4', - 116: 'F5', - 117: 'F6', - 118: 'F7', - 119: 'F8', - 120: 'F9', - 121: 'F10', - 122: 'F11', - 123: 'F12', - 173: '-', // firefox - 186: ';', // non-firefox - 187: '=', // non-firefox - 188: ',', - 189: '-', // non-firefox - 190: '.', - 191: '/', - 192: '`', - 219: '[', - 220: '\\', - 221: ']', - 222: "'" -}); +export const EN_US: IKeyboardLayout = new KeycodeLayout( + 'en-us', + { + 8: 'Backspace', + 9: 'Tab', + 13: 'Enter', + 16: 'Shift', + 17: 'Ctrl', + 18: 'Alt', + 19: 'Pause', + 27: 'Escape', + 32: 'Space', + 33: 'PageUp', + 34: 'PageDown', + 35: 'End', + 36: 'Home', + 37: 'ArrowLeft', + 38: 'ArrowUp', + 39: 'ArrowRight', + 40: 'ArrowDown', + 45: 'Insert', + 46: 'Delete', + 48: '0', + 49: '1', + 50: '2', + 51: '3', + 52: '4', + 53: '5', + 54: '6', + 55: '7', + 56: '8', + 57: '9', + 59: ';', // firefox + 61: '=', // firefox + 65: 'A', + 66: 'B', + 67: 'C', + 68: 'D', + 69: 'E', + 70: 'F', + 71: 'G', + 72: 'H', + 73: 'I', + 74: 'J', + 75: 'K', + 76: 'L', + 77: 'M', + 78: 'N', + 79: 'O', + 80: 'P', + 81: 'Q', + 82: 'R', + 83: 'S', + 84: 'T', + 85: 'U', + 86: 'V', + 87: 'W', + 88: 'X', + 89: 'Y', + 90: 'Z', + 91: 'Meta', // non-firefox + 93: 'ContextMenu', + 96: '0', // numpad + 97: '1', // numpad + 98: '2', // numpad + 99: '3', // numpad + 100: '4', // numpad + 101: '5', // numpad + 102: '6', // numpad + 103: '7', // numpad + 104: '8', // numpad + 105: '9', // numpad + 106: '*', // numpad + 107: '+', // numpad + 109: '-', // numpad + 110: '.', // numpad + 111: '/', // numpad + 112: 'F1', + 113: 'F2', + 114: 'F3', + 115: 'F4', + 116: 'F5', + 117: 'F6', + 118: 'F7', + 119: 'F8', + 120: 'F9', + 121: 'F10', + 122: 'F11', + 123: 'F12', + 173: '-', // firefox + 186: ';', // non-firefox + 187: '=', // non-firefox + 188: ',', + 189: '-', // non-firefox + 190: '.', + 191: '/', + 192: '`', + 219: '[', + 220: '\\', + 221: ']', + 222: "'", + 224: 'Meta' // firefox + }, + ['Shift', 'Ctrl', 'Alt', 'Meta'] // modifier keys +); /** * The namespace for the module implementation details. diff --git a/packages/keyboard/tests/src/index.spec.ts b/packages/keyboard/tests/src/index.spec.ts index d84a6a545..46bf88f7e 100644 --- a/packages/keyboard/tests/src/index.spec.ts +++ b/packages/keyboard/tests/src/index.spec.ts @@ -65,6 +65,24 @@ describe('@lumino/keyboard', () => { expect(layout.isValidKey('F')).to.equal(true); expect(layout.isValidKey('A')).to.equal(false); }); + + it('should treat modifier keys as valid', () => { + let layout = new KeycodeLayout('foo', { 100: 'F', 101: 'A' }, ['A']); + expect(layout.isValidKey('A')).to.equal(true); + }); + }); + + describe('#isModifierKey()', () => { + it('should test whether the key is modifier for the layout', () => { + let layout = new KeycodeLayout('foo', { 100: 'F', 101: 'A' }, ['A']); + expect(layout.isModifierKey('F')).to.equal(false); + expect(layout.isModifierKey('A')).to.equal(true); + }); + + it('should return false for keys that are not in the layout', () => { + let layout = new KeycodeLayout('foo', { 100: 'F', 101: 'A' }, ['A']); + expect(layout.isModifierKey('B')).to.equal(false); + }); }); describe('#keyForKeydownEvent()', () => { @@ -90,6 +108,14 @@ describe('@lumino/keyboard', () => { expect(KeycodeLayout.extractKeys(keys)).to.deep.equal(goal); }); }); + + describe('.convertToKeySet()', () => { + it('should convert key array to key set', () => { + let keys: string[] = ['F', 'G', 'H']; + let goal: KeycodeLayout.KeySet = { F: true, G: true, H: true }; + expect(KeycodeLayout.convertToKeySet(keys)).to.deep.equal(goal); + }); + }); }); describe('EN_US', () => { @@ -103,5 +129,19 @@ describe('@lumino/keyboard', () => { expect(EN_US.isValidKey('0')).to.equal(true); expect(EN_US.isValidKey('a')).to.equal(false); }); + + it('should have modifier keys', () => { + expect(EN_US.isValidKey('Shift')).to.equal(true); + expect(EN_US.isValidKey('Ctrl')).to.equal(true); + expect(EN_US.isValidKey('Alt')).to.equal(true); + expect(EN_US.isValidKey('Meta')).to.equal(true); + }); + + it('should correctly detect modifier keys', () => { + expect(EN_US.isModifierKey('Shift')).to.equal(true); + expect(EN_US.isModifierKey('Ctrl')).to.equal(true); + expect(EN_US.isModifierKey('Alt')).to.equal(true); + expect(EN_US.isModifierKey('Meta')).to.equal(true); + }); }); });