Accept individual modifier keys as valid keybindings (#637)

* extend keystrokes to mod keys * fix doubled keys * remove comments * remove console-log * Removed whitespace from keystrokes * Run prettier * Updated normalizeKeystroke to account for empty string * Check for empty string in key * Check for empty string * Checking for whitespace * Trim whitespace * Added check for keys * refactor add space for puley modifier key * Changed tests to reflect mod keys creating events * Changed tests to match key bindings * Revert test changes * remove added white space * Removed ctrl key from test to check functionality * Modified test * delay modifier key only execution * Update packages/commands/src/index.ts Co-authored-by: Nicolas Brichet <32258950+brichet@users.noreply.github.com> * re-write * removed redundantcode * Ran Prettier to fix failing test * Update packages/commands/src/index.ts Co-authored-by: Nicolas Brichet <32258950+brichet@users.noreply.github.com> * Refactored func and test * modifier key bindings trigger on key holds * reverted changed test * undo reverted test changes * Ran Prettier * Starts the timer for mod keys only before updating the keystrokes list * Update packages/commands/src/index.ts Co-authored-by: Nicolas Brichet <32258950+brichet@users.noreply.github.com> * added new variable with a shorter delay * ran yarn run api and committing changes * ran linter * refactor variable to camelcase * refactor variable to camelcase * updated api * Clear modifier timeout if an exact match is found --------- Co-authored-by: m158261 <141649264+m158261@users.noreply.github.com> Co-authored-by: EC2 Default User <m158261@users.noreply.github.com> Co-authored-by: Nicolas Brichet <32258950+brichet@users.noreply.github.com> Co-authored-by: Nicolas Brichet <nicolas.brichet@quantstack.net>
jupyterlab · Apr 4, 2024 · ededf7f · ededf7f
1 parent 4af2eca
commit ededf7f
Show file tree

Hide file tree

Showing 5 changed files with 87 additions and 13 deletions.
diff --git a/packages/application/src/index.ts b/packages/application/src/index.ts
@@ -575,6 +575,9 @@ export class Application<T extends Widget = Widget> {
       case 'keydown':
         this.evtKeydown(event as KeyboardEvent);
         break;
+      case 'keyup':
+        this.evtKeyup(event as KeyboardEvent);
+        break;
       case 'contextmenu':
         this.evtContextMenu(event as PointerEvent);
         break;
@@ -610,6 +613,7 @@ export class Application<T extends Widget = Widget> {
   protected addEventListeners(): void {
     document.addEventListener('contextmenu', this);
     document.addEventListener('keydown', this, !this._bubblingKeydown);
+    document.addEventListener('keyup', this, !this._bubblingKeydown);
     window.addEventListener('resize', this);
   }
 
@@ -626,6 +630,19 @@ export class Application<T extends Widget = Widget> {
     this.commands.processKeydownEvent(event);
   }
 
+  /**
+   * A method invoked on a document `'keyup'` event.
+   *
+   * #### Notes
+   * The default implementation of this method invokes the key up
+   * processing method of the application command registry.
+   *
+   * A subclass may reimplement this method as needed.
+   */
+  protected evtKeyup(event: KeyboardEvent): void {
+    this.commands.processKeyupEvent(event);
+  }
+
   /**
    * A method invoked on a document `'contextmenu'` event.
    *

diff --git a/packages/commands/src/index.ts b/packages/commands/src/index.ts
@@ -511,13 +511,8 @@ export class CommandRegistry {
    * events will not invoke commands.
    */
   processKeydownEvent(event: KeyboardEvent): void {
-    // Bail immediately if playing back keystrokes
-    // or if the event has been processed
-    if (
-      event.defaultPrevented ||
-      this._replaying ||
-      CommandRegistry.isModifierKeyPressed(event)
-    ) {
+    // Bail immediately if playing back keystrokes.
+    if (event.defaultPrevented || this._replaying) {
       return;
     }
 
@@ -532,6 +527,26 @@ export class CommandRegistry {
       return;
     }
 
+    // Check that only mod key(s) have been pressed.
+    if (CommandRegistry.isModifierKeyPressed(event)) {
+      // Find the exact match for the modifier keys.
+      let { exact } = Private.matchKeyBinding(
+        this._keyBindings,
+        [keystroke],
+        event
+      );
+      if (exact) {
+        // If the mod keys match an exact shortcut, start a dedicated timer.
+        event.preventDefault();
+        event.stopPropagation();
+        this._startModifierTimer(exact);
+      } else {
+        // Otherwise stop potential existing timer.
+        this._clearModifierTimer();
+      }
+      return;
+    }
+
     // Add the keystroke to the current key sequence.
     this._keystrokes.push(keystroke);
 
@@ -599,6 +614,36 @@ export class CommandRegistry {
     this._holdKeyBindingPromises.set(event, permission);
   }
 
+  /**
+   * Process a ``keyup`` event to clear the timer on the modifier, if it exists.
+   *
+   * @param event - The event object for a `'keydown'` event.
+   */
+  processKeyupEvent(event: KeyboardEvent): void {
+    this._clearModifierTimer();
+  }
+
+  /**
+   * Start or restart the timeout on the modifier keys.
+   *
+   * This timeout will end only if the keys are hold.
+   */
+  private _startModifierTimer(exact: CommandRegistry.IKeyBinding): void {
+    this._clearModifierTimer();
+    this._timerModifierID = window.setTimeout(() => {
+      this._executeKeyBinding(exact);
+    }, Private.modifierkeyTimeOut);
+  }
+
+  /**
+   * Clear the timeout on modifier keys.
+   */
+  private _clearModifierTimer(): void {
+    if (this._timerModifierID !== 0) {
+      clearTimeout(this._timerModifierID);
+      this._timerModifierID = 0;
+    }
+  }
   /**
    * Start or restart the pending timeout.
    */
@@ -688,6 +733,7 @@ export class CommandRegistry {
    */
   private _clearPendingState(): void {
     this._clearTimer();
+    this._clearModifierTimer();
     this._exactKeyMatch = null;
     this._keystrokes.length = 0;
     this._keydownEvents.length = 0;
@@ -707,6 +753,7 @@ export class CommandRegistry {
   }
 
   private _timerID = 0;
+  private _timerModifierID = 0;
   private _replaying = false;
   private _keystrokes: string[] = [];
   private _keydownEvents: KeyboardEvent[] = [];
@@ -1245,6 +1292,9 @@ export namespace CommandRegistry {
     if (parts.cmd && Platform.IS_MAC) {
       mods += 'Cmd ';
     }
+    if (!parts.key) {
+      return mods.trim();
+    }
     return mods + parts.key;
   }
 
@@ -1328,9 +1378,6 @@ export namespace CommandRegistry {
   export function keystrokeForKeydownEvent(event: KeyboardEvent): string {
     let layout = getKeyboardLayout();
     let key = layout.keyForKeydownEvent(event);
-    if (!key || layout.isModifierKey(key)) {
-      return '';
-    }
     let mods = [];
     if (event.ctrlKey) {
       mods.push('Ctrl');
@@ -1344,7 +1391,10 @@ export namespace CommandRegistry {
     if (event.metaKey && Platform.IS_MAC) {
       mods.push('Cmd');
     }
-    mods.push(key);
+    if (!layout.isModifierKey(key)) {
+      mods.push(key);
+    }
+    // for purely modifier key strings
     return mods.join(' ');
   }
 }
@@ -1363,6 +1413,11 @@ namespace Private {
    */
   export const KEYBINDING_HOLD_TIMEOUT = 1000;
 
+  /**
+   * The timeout in ms for triggering a modifer key binding.
+   */
+  export const modifierkeyTimeOut = 500;
+
   /**
    * A convenience type alias for a command func.
    */

diff --git a/packages/commands/tests/src/index.spec.ts b/packages/commands/tests/src/index.spec.ts
@@ -1508,11 +1508,11 @@ describe('@lumino/commands', () => {
         expect(keystroke).to.equal('');
       });
 
-      it('should return nothing for keys that are marked as modifier in keyboard layout', () => {
+      it('should return keys that are marked as modifier in keyboard layout', () => {
         let keystroke = CommandRegistry.keystrokeForKeydownEvent(
           new KeyboardEvent('keydown', { keyCode: 17, ctrlKey: true })
         );
-        expect(keystroke).to.equal('');
+        expect(keystroke).to.equal('Ctrl');
       });
     });
   });

diff --git a/review/api/application.api.md b/review/api/application.api.md
@@ -24,6 +24,7 @@ export class Application<T extends Widget = Widget> {
     deregisterPlugin(id: string, force?: boolean): void;
     protected evtContextMenu(event: PointerEvent): void;
     protected evtKeydown(event: KeyboardEvent): void;
+    protected evtKeyup(event: KeyboardEvent): void;
     protected evtResize(event: Event): void;
     getPluginDescription(id: string): string;
     handleEvent(event: Event): void;

diff --git a/review/api/commands.api.md b/review/api/commands.api.md
@@ -37,6 +37,7 @@ export class CommandRegistry {
     mnemonic(id: string, args?: ReadonlyPartialJSONObject): number;
     notifyCommandChanged(id?: string): void;
     processKeydownEvent(event: KeyboardEvent): void;
+    processKeyupEvent(event: KeyboardEvent): void;
     usage(id: string, args?: ReadonlyPartialJSONObject): string;
 }