Improve the keybinding input guard using proposed lumino features (#1…

…5927)

* Fix saving shortcut not preventing default

* Run `Command + Shift + c` immediately too

* Command + S for saving on Mac

* Use lumino's `holdKeyBindingExecution` and disable `preventDefault`

on inline completer shortcuts

* Fix compilation by adding type argument

* Upgrade lumino packages to v2024.3.25

* Short-circuit on `keyup` too, disconnect handlers when done

* Wait for command mode confirmation in search test

galata/test/jupyterlab/notebook-search.test.ts

@@ -334,6 +334,7 @@ test.describe('Notebook Search', () => {
 
     // Switch back to command mode
     await page.keyboard.press('Escape');
+    await page.getByText(`Mode: Command`, { exact: true }).waitFor();
 
     // Select two cells below
     await page.keyboard.press('Shift+ArrowDown');

packages/application/src/lab.ts

@@ -199,6 +199,103 @@ export class JupyterLab extends JupyterFrontEnd<ILabShell> {
     });
   }
 
+  /**
+   * Override keydown handling to prevent command shortcuts from preventing user input.
+   *
+   * This introduces a slight delay to the command invocation, but no delay to user input.
+   */
+  protected evtKeydown(keyDownEvent: KeyboardEvent): void {
+    const permissionToExecute = new PromiseDelegate<boolean>();
+
+    // Hold the execution of any keybinding until we know if this event would cause text insertion
+    this.commands.holdKeyBindingExecution(
+      keyDownEvent,
+      permissionToExecute.promise
+    );
+
+    // Process the key immediately to invoke the prevent default handlers as needed
+    this.commands.processKeydownEvent(keyDownEvent);
+
+    // If we do not know the target, we cannot check if input would be inserted
+    // as there is no target to attach the `beforeinput` event listener; in that
+    // case we just permit execution immediately (this may happen for programmatic
+    // uses of keydown)
+    const target = keyDownEvent.target;
+    if (!target) {
+      return permissionToExecute.resolve(true);
+    }
+
+    let onBeforeInput: ((event: InputEvent) => void) | null = null;
+    let onBeforeKeyUp: ((event: KeyboardEvent) => void) | null = null;
+
+    const disconnectListeners = () => {
+      if (onBeforeInput) {
+        target.removeEventListener('beforeinput', onBeforeInput);
+      }
+      if (onBeforeKeyUp) {
+        target.removeEventListener('keyup', onBeforeKeyUp);
+      }
+    };
+
+    // Permit the execution conditionally, depending on whether the event would lead to text insertion
+    const causesInputPromise = Promise.race([
+      new Promise(resolve => {
+        onBeforeInput = (inputEvent: InputEvent) => {
+          switch (inputEvent.inputType) {
+            case 'historyUndo':
+            case 'historyRedo': {
+              if (
+                inputEvent.target instanceof Element &&
+                inputEvent.target.closest('[data-jp-undoer]')
+              ) {
+                // Allow to use custom undo/redo bindings on `jpUndoer`s
+                inputEvent.preventDefault();
+                disconnectListeners();
+                return resolve(false);
+              }
+              break;
+            }
+            case 'insertLineBreak': {
+              if (
+                inputEvent.target instanceof Element &&
+                inputEvent.target.closest('.jp-Cell')
+              ) {
+                // Allow to override the default action of Shift + Enter on cells as this is used for cell execution
+                inputEvent.preventDefault();
+                disconnectListeners();
+                return resolve(false);
+              }
+              break;
+            }
+          }
+          disconnectListeners();
+          return resolve(true);
+        };
+        target.addEventListener('beforeinput', onBeforeInput, { once: true });
+      }),
+      new Promise(resolve => {
+        onBeforeKeyUp = (keyUpEvent: KeyboardEvent) => {
+          if (keyUpEvent.code === keyDownEvent.code) {
+            disconnectListeners();
+            return resolve(false);
+          }
+        };
+        target.addEventListener('keyup', onBeforeKeyUp, { once: true });
+      }),
+      new Promise(resolve => {
+        setTimeout(() => {
+          disconnectListeners();
+          return resolve(false);
+        }, Private.INPUT_GUARD_TIMEOUT);
+      })
+    ]);
+    causesInputPromise
+      .then(willCauseInput => {
+        permissionToExecute.resolve(!willCauseInput);
+      })
+      .catch(console.warn);
+  }
+
   private _info: JupyterLab.IInfo = JupyterLab.defaultInfo;
   private _paths: JupyterFrontEnd.IPaths;
   private _allPluginsActivated = new PromiseDelegate<void>();
@@ -374,3 +471,18 @@ export namespace JupyterLab {
       | JupyterFrontEndPlugin<any, any, any>[];
   }
 }
+
+/**
+ * A namespace for module-private functionality.
+ */
+namespace Private {
+  /**
+   * The delay for invoking a command introduced by user input guard.
+   * Decreasing this value may lead to commands incorrectly triggering
+   * on user input. Increasing this value will lead to longer delay for
+   * command invocation. Note that user input is never delayed.
+   *
+   * The value represents the number in milliseconds.
+   */
+  export const INPUT_GUARD_TIMEOUT = 10;
+}

packages/completer-extension/schema/inline-completer.json

@@ -8,12 +8,14 @@
     {
       "command": "inline-completer:next",
       "keys": ["Alt ]"],
-      "selector": ".jp-mod-completer-enabled"
+      "selector": ".jp-mod-completer-enabled",
+      "preventDefault": false
     },
     {
       "command": "inline-completer:previous",
       "keys": ["Alt ["],
-      "selector": ".jp-mod-completer-enabled"
+      "selector": ".jp-mod-completer-enabled",
+      "preventDefault": false
     },
     {
       "command": "inline-completer:accept",
@@ -23,7 +25,8 @@
     {
       "command": "inline-completer:invoke",
       "keys": ["Alt \\"],
-      "selector": ".jp-mod-completer-enabled"
+      "selector": ".jp-mod-completer-enabled",
+      "preventDefault": false
     }
   ],
   "properties": {

packages/shortcuts-extension/schema/shortcuts.json

@@ -64,7 +64,8 @@
           "items": { "type": "string" },
           "type": "array"
         },
-        "selector": { "type": "string" }
+        "selector": { "type": "string" },
+        "preventDefault": { "type": "boolean" }
       },
       "required": ["command", "keys", "selector"],
       "type": "object"