From a2974a3e6d791a38d7f61d14fc54e13ea6c2a647 Mon Sep 17 00:00:00 2001 From: Evan Simkowitz Date: Thu, 2 Jan 2025 11:38:07 -0500 Subject: [PATCH] Fix escape getting eaten by global event handler (#1668) The terminal keydown handler was set to filter out all key bindings that have a registered global handler, regardless of whether they actually propagated or not. This allowed the global handlers to still work despite the terminal input having precedence, but it also meant that global key bindings that were invalid for the current context would still get eaten and not sent to stdin. Now, the terminal keydown handler will directly call the global handlers so we can actually see whether or not the global key binding is valid. If the global handler is valid, it'll be processed immediately and stdin won't receive the input. If it's not handled, we'll let xterm pass it to stdin. Because anything xterm doesn't handle gets sent to the globally-registered version of the handler, we need to make sure we don't do extra work to process an input we've already checked. We'll store the last-handled keydown event as a static variable so we can dedupe later calls for the same event to prevent doing double work. --- frontend/app/store/keymodel.ts | 8 ++++++++ frontend/app/view/term/term.tsx | 12 ++++++------ frontend/util/keyutil.ts | 4 +--- 3 files changed, 15 insertions(+), 9 deletions(-) diff --git a/frontend/app/store/keymodel.ts b/frontend/app/store/keymodel.ts index 8ae6d3c3e..f9f6d988a 100644 --- a/frontend/app/store/keymodel.ts +++ b/frontend/app/store/keymodel.ts @@ -184,7 +184,14 @@ async function handleCmdN() { await createBlock(termBlockDef); } +let lastHandledEvent: KeyboardEvent | null = null; + function appHandleKeyDown(waveEvent: WaveKeyboardEvent): boolean { + const nativeEvent = (waveEvent as any).nativeEvent; + if (lastHandledEvent != null && nativeEvent != null && lastHandledEvent === nativeEvent) { + return false; + } + lastHandledEvent = nativeEvent; const handled = handleGlobalWaveKeyboardEvents(waveEvent); if (handled) { return true; @@ -366,6 +373,7 @@ function handleGlobalWaveKeyboardEvents(waveEvent: WaveKeyboardEvent): boolean { return handler(waveEvent); } } + return false; } export { diff --git a/frontend/app/view/term/term.tsx b/frontend/app/view/term/term.tsx index 963bdd209..5aaaf475a 100644 --- a/frontend/app/view/term/term.tsx +++ b/frontend/app/view/term/term.tsx @@ -4,7 +4,7 @@ import { Block, SubBlock } from "@/app/block/block"; import { BlockNodeModel } from "@/app/block/blocktypes"; import { Search, useSearch } from "@/app/element/search"; -import { getAllGlobalKeyBindings } from "@/app/store/keymodel"; +import { appHandleKeyDown } from "@/app/store/keymodel"; import { waveEventSubscribe } from "@/app/store/wps"; import { RpcApi } from "@/app/store/wshclientapi"; import { makeFeBlockRouteId } from "@/app/store/wshrouter"; @@ -434,11 +434,11 @@ class TermViewModel implements ViewModel { this.forceRestartController(); return false; } - const globalKeys = getAllGlobalKeyBindings(); - for (const key of globalKeys) { - if (keyutil.checkKeyPressed(waveEvent, key)) { - return false; - } + const appHandled = appHandleKeyDown(waveEvent); + if (appHandled) { + event.preventDefault(); + event.stopPropagation(); + return false; } return true; } diff --git a/frontend/util/keyutil.ts b/frontend/util/keyutil.ts index f560cc902..841d12d46 100644 --- a/frontend/util/keyutil.ts +++ b/frontend/util/keyutil.ts @@ -78,9 +78,6 @@ function parseKeyDescription(keyDescription: string): KeyPressDecl { rtn.mods.Option = true; } rtn.mods.Meta = true; - } else if (key == "Esc") { - rtn.key = "Escape"; - rtn.keyType = KeyTypeKey; } else { let { key: parsedKey, type: keyType } = parseKey(key); rtn.key = parsedKey; @@ -213,6 +210,7 @@ function adaptFromReactOrNativeKeyEvent(event: React.KeyboardEvent | KeyboardEve rtn.code = event.code; rtn.key = event.key; rtn.location = event.location; + (rtn as any).nativeEvent = event; if (event.type == "keydown" || event.type == "keyup" || event.type == "keypress") { rtn.type = event.type; } else {