jupyterlab / lumino

Lumino is a library for building interactive web applications
https://lumino.readthedocs.io/
Other
590 stars 127 forks source link

New keybinding attribute to indicate shortcuts which prevent default actions #688

Closed krassowski closed 2 months ago

krassowski commented 2 months ago

Problem

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: https://github.com/jupyterlab/lumino/blob/f1014291ac83e4524bbf57afb2cbdd55ea4ee546/packages/commands/src/index.ts#L553-L557

    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

krassowski commented 2 months ago

Here are some more thoughts:

krassowski commented 2 months ago

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