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()
andstopPropagation()
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
-
Add new attribute
preventDefault: bool = true
toIKeyBindingOptions
-
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; }
-
Change
processKeydownEvent
to onlypreventDefault
when needed, this is change:
lumino/packages/commands/src/index.ts
Lines 553 to 557 in f101429
to:
if (exact.preventDefault || partial.preventDefault) { event.preventDefault(); event.stopPropagation(); }
-
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 theevtKeydown
handler to callwillPreventDefault(event)
and depending on the result decide whether to process the event immediately or not -
In a future major lumino version, consider changing
preventDefault
default tofalse
, and adopting the refinedevtKeydown
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 previousprocessKeydownEvent
but before nextprocessKeydownEvent
). 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.