Ignore keydown events for modifier keys when accumulating key sequence

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 = '';

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('');
+      });
     });
   });
 });