From 78ec07ba52e827487d831895051b02202ec2c36b Mon Sep 17 00:00:00 2001 From: Asher Date: Thu, 14 Sep 2023 09:20:07 -0800 Subject: [PATCH 1/5] Listen to web terminal keydown on capture Instead of bubbling. I think maybe what happens here is that xterm is capturing key presses and preventing the event from bubbling? So setting the listener on the capture phase instead works around this. Probably would also work to dipsose the terminal. --- site/src/pages/TerminalPage/TerminalPage.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/site/src/pages/TerminalPage/TerminalPage.tsx b/site/src/pages/TerminalPage/TerminalPage.tsx index e8e4e4ab25c6f..5fc210bd091c4 100644 --- a/site/src/pages/TerminalPage/TerminalPage.tsx +++ b/site/src/pages/TerminalPage/TerminalPage.tsx @@ -479,10 +479,10 @@ const useReloading = (isDisconnected: boolean) => { window.location.reload(); }; - document.addEventListener("keydown", keyDownHandler); + document.addEventListener("keydown", keyDownHandler, true); return () => { - document.removeEventListener("keydown", keyDownHandler); + document.removeEventListener("keydown", keyDownHandler, true); }; }, [isDisconnected]); From 46f26154ee01edfcc0a0bbd77bb2a1d259f6b2eb Mon Sep 17 00:00:00 2001 From: Asher Date: Thu, 14 Sep 2023 09:21:58 -0800 Subject: [PATCH 2/5] Prevent issuing terminal reload when already reloading I am not sure this actually causes any issues, but might as well. --- site/src/pages/TerminalPage/TerminalPage.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/site/src/pages/TerminalPage/TerminalPage.tsx b/site/src/pages/TerminalPage/TerminalPage.tsx index 5fc210bd091c4..f7683585aa972 100644 --- a/site/src/pages/TerminalPage/TerminalPage.tsx +++ b/site/src/pages/TerminalPage/TerminalPage.tsx @@ -470,7 +470,7 @@ const useReloading = (isDisconnected: boolean) => { // Retry connection on key press when it is disconnected useEffect(() => { - if (!isDisconnected) { + if (!isDisconnected || status === "reloading") { return; } @@ -484,7 +484,7 @@ const useReloading = (isDisconnected: boolean) => { return () => { document.removeEventListener("keydown", keyDownHandler, true); }; - }, [isDisconnected]); + }, [status, isDisconnected]); return { status, From 85ffce8216066c2245ec89307aea8f777445c167 Mon Sep 17 00:00:00 2001 From: Asher Date: Thu, 14 Sep 2023 10:40:14 -0800 Subject: [PATCH 3/5] Ignore modifier keys for reconnecting terminal --- site/src/pages/TerminalPage/TerminalPage.tsx | 47 ++++++++++++++++++-- 1 file changed, 44 insertions(+), 3 deletions(-) diff --git a/site/src/pages/TerminalPage/TerminalPage.tsx b/site/src/pages/TerminalPage/TerminalPage.tsx index f7683585aa972..8fa29f6246e9d 100644 --- a/site/src/pages/TerminalPage/TerminalPage.tsx +++ b/site/src/pages/TerminalPage/TerminalPage.tsx @@ -474,15 +474,56 @@ const useReloading = (isDisconnected: boolean) => { return; } - const keyDownHandler = () => { - setStatus("reloading"); - window.location.reload(); + // Keep track of modifier keys since we want to avoid reconnecting while + // modifiers are held. This covers cases where the terminal unexpectedly + // tries to reconnect like when pressing ctrl+w, ctrl+r, and so on. This + // will not work if you pressed a modifier before the disconnect and are + // still holding it; if we need to account for that we will need to listen + // for modifier keys while connected as well. + const modifierKeyState: Record = { + Alt: false, + AltGraph: false, + CapsLock: false, + Control: false, + Fn: false, + FnLock: false, + Meta: false, + NumLock: false, + ScrollLock: false, + Shift: false, + Symbol: false, + SymbolLock: false, + }; + + const isModifier = (event: KeyboardEvent): boolean => { + return typeof modifierKeyState[event.key] !== "undefined"; + }; + + const isModified = (): boolean => { + return Object.values(modifierKeyState).includes(true); + }; + + const keyDownHandler = (event: KeyboardEvent) => { + if (isModifier(event)) { + modifierKeyState[event.key] = true; + } else if (!isModified()) { + setStatus("reloading"); + window.location.reload(); + } + }; + + const keyUpHandler = (event: KeyboardEvent) => { + if (isModifier(event)) { + modifierKeyState[event.key] = false; + } }; document.addEventListener("keydown", keyDownHandler, true); + document.addEventListener("keyup", keyUpHandler, true); return () => { document.removeEventListener("keydown", keyDownHandler, true); + document.removeEventListener("keyup", keyUpHandler, true); }; }, [status, isDisconnected]); From 88db8cc412c09889a558d18009cbb60efe4d3b6f Mon Sep 17 00:00:00 2001 From: Asher Date: Fri, 15 Sep 2023 13:39:37 -0800 Subject: [PATCH 4/5] Swap undefined check for `in` Co-authored-by: Kayla Washburn --- site/src/pages/TerminalPage/TerminalPage.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/site/src/pages/TerminalPage/TerminalPage.tsx b/site/src/pages/TerminalPage/TerminalPage.tsx index 8fa29f6246e9d..1e8235b7825cd 100644 --- a/site/src/pages/TerminalPage/TerminalPage.tsx +++ b/site/src/pages/TerminalPage/TerminalPage.tsx @@ -496,7 +496,7 @@ const useReloading = (isDisconnected: boolean) => { }; const isModifier = (event: KeyboardEvent): boolean => { - return typeof modifierKeyState[event.key] !== "undefined"; + return event.key in modifierKeyState; }; const isModified = (): boolean => { From 9cab1434ad40b802524a1dfbccea1f274287ce43 Mon Sep 17 00:00:00 2001 From: Asher Date: Fri, 15 Sep 2023 14:26:58 -0800 Subject: [PATCH 5/5] Use modifier properties on keydown event Instead of manually tracking modifiers with keydown/keyup, as this can miss keys like in the case of ctrl+tab, leaving ctrl in a permanent "on" state. The other modifiers do not appear to be used in conjunction with other keys, except possibly Fn? So this should be more or less the same functionality. --- site/src/pages/TerminalPage/TerminalPage.tsx | 64 ++++++++------------ 1 file changed, 25 insertions(+), 39 deletions(-) diff --git a/site/src/pages/TerminalPage/TerminalPage.tsx b/site/src/pages/TerminalPage/TerminalPage.tsx index 1e8235b7825cd..0cc206cd51493 100644 --- a/site/src/pages/TerminalPage/TerminalPage.tsx +++ b/site/src/pages/TerminalPage/TerminalPage.tsx @@ -474,56 +474,42 @@ const useReloading = (isDisconnected: boolean) => { return; } - // Keep track of modifier keys since we want to avoid reconnecting while - // modifiers are held. This covers cases where the terminal unexpectedly - // tries to reconnect like when pressing ctrl+w, ctrl+r, and so on. This - // will not work if you pressed a modifier before the disconnect and are - // still holding it; if we need to account for that we will need to listen - // for modifier keys while connected as well. - const modifierKeyState: Record = { - Alt: false, - AltGraph: false, - CapsLock: false, - Control: false, - Fn: false, - FnLock: false, - Meta: false, - NumLock: false, - ScrollLock: false, - Shift: false, - Symbol: false, - SymbolLock: false, - }; - - const isModifier = (event: KeyboardEvent): boolean => { - return event.key in modifierKeyState; - }; - - const isModified = (): boolean => { - return Object.values(modifierKeyState).includes(true); - }; + // Modifier keys should not trigger a reload. + const ignoredKeys = [ + "Alt", + "AltGraph", + "CapsLock", + "Control", + "Fn", + "FnLock", + "Meta", + "NumLock", + "ScrollLock", + "Shift", + "Symbol", + "SymbolLock", + ]; const keyDownHandler = (event: KeyboardEvent) => { - if (isModifier(event)) { - modifierKeyState[event.key] = true; - } else if (!isModified()) { + // In addition to ignored keys, avoid reloading while modifiers are held + // to cover cases where the terminal unexpectedly tries to reconnect like + // when pressing ctrl+w, ctrl+r, and so on. + if ( + !ignoredKeys.includes(event.key) && + !event.altKey && + !event.ctrlKey && + !event.metaKey && + !event.shiftKey + ) { setStatus("reloading"); window.location.reload(); } }; - const keyUpHandler = (event: KeyboardEvent) => { - if (isModifier(event)) { - modifierKeyState[event.key] = false; - } - }; - document.addEventListener("keydown", keyDownHandler, true); - document.addEventListener("keyup", keyUpHandler, true); return () => { document.removeEventListener("keydown", keyDownHandler, true); - document.removeEventListener("keyup", keyUpHandler, true); }; }, [status, isDisconnected]);