jupyterlab/lumino

New keybinding attribute to indicate shortcuts which prevent default actions

Closed this issue · 2 comments

Problem

  • Currently lumino shortcuts can prevent user from typing characters on certain keyboards (because it always calls preventDefault() and stopPropagation() on matched keyboard events)
  • JupyterLab 4.1.3+ solves it by delaying command execution until it knows if a character would have been typed (jupyterlab/jupyterlab#15790)
  • However, some shortcuts need to be executed immediately to prevent default browser action
  • Currently there is no way to distinguish the situation in which the current keyboard event would invoke a shortcut which needs such immediately handling

Proposed Solution

  1. Add new attribute preventDefault: bool = true to IKeyBindingOptions

  2. Add new public method on commands to check if the default action on the event should be prevented, say:

        willPreventDefault(event) {
            // Bail immediately if playing back keystrokes.
            if (this._replaying || CommandRegistry.isModifierKeyPressed(event)) {
                return;
            }
            // Get the normalized keystroke for the event.
            let keystroke = CommandRegistry.keystrokeForKeydownEvent(event);
            // If the keystroke is not valid for the keyboard layout, replay
            // any suppressed events and clear the pending state.
            if (!keystroke) {
                this._replayKeydownEvents();
                this._clearPendingState();
                return;
            }
            // Add the keystroke to the current key sequence.
            const keystrokes = [*this._keystrokes, keystroke];
            // Find the exact and partial matches for the key sequence.
            let { exact } = Private.matchKeyBinding(this._keyBindings, keystrokes, event);
    
            if (exact.preventDefault || partial.preventDefault) {
              return true;
            }
            return false;
        }
  3. Change processKeydownEvent to only preventDefault when needed, this is change:

    // Stop propagation of the event. If there is only a partial match,
    // the event will be replayed if a final exact match never occurs.
    event.preventDefault();
    event.stopPropagation();

    to:

    if (exact.preventDefault || partial.preventDefault) {
       event.preventDefault(); 
       event.stopPropagation(); 
    }
  4. In JupyterLab:
    a) mark all keybindings as preventDefault=false by default, and flick the few ones which should prevent default manually to true
    b) rework the evtKeydown handler to call willPreventDefault(event) and depending on the result decide whether to process the event immediately or not

  5. In a future major lumino version, consider changing preventDefault default to false, and adopting the refined evtKeydown implementation from JupyterLab

Additional context

Here are some more thoughts:

  • willPreventDefault is a bit single-use case method which needs to be called in a very specific way (after previous processKeydownEvent but before next processKeydownEvent). I don't like it from the API design perspective.
  • the script on the lab side is wrong with respect to chord because it just skips call to processKeydownEvent if it find that user input arises from a new key event; this can still result in a command being dispatched if it was a partial match; we need a better solution on lumino level

Currently thinking about adding some kind of a hook in _executeKeyBinding but don't see a clean API for it yet.