fix(search): Escape restores terminal focus after Cmd+F#283
Closed
KwaminaWhyte wants to merge 2 commits into
Closed
fix(search): Escape restores terminal focus after Cmd+F#283KwaminaWhyte wants to merge 2 commits into
KwaminaWhyte wants to merge 2 commits into
Conversation
During Chinese/Korean/Japanese input the browser fires keydown events with event.isComposing=true while the user is mid-composition. xterm's attachCustomKeyEventHandler was forwarding those events straight to the PTY, causing two problems: - Chinese IME (issue crynta#158): pressing Enter to commit a candidate also submitted the current shell command. - Korean Hangul (issue crynta#147): inline-composed jamo were duplicated or swallowed because raw keydown and the compositionend string both reached the shell. Fix: return false from the custom key handler whenever isComposing is true or keyCode === 229 (the synthetic 'Process' code Chromium reports while any key is handled inside an IME session). Returning false tells xterm to skip its own keydown processing; the final composed string still arrives via xterm's compositionend path and is written to the PTY exactly once. Note: the upstream refactor moved terminal creation from useTerminalSession.ts into rendererPool.ts (createSlot). The fix is applied there, in the single place where all xterm Terminal instances are constructed. Closes crynta#158 Closes crynta#147
searchTarget.focus() for a terminal pane was calling terminalRefs.current.get(activeId) where activeId is the *tab* ID. terminalRefs is keyed by *leaf* ID, so the lookup always returned undefined and focus was never restored. Fix: use activeLeafId (already in scope) as the key, matching how every other callsite in App.tsx addresses terminalRefs (see lines 508, 525, 774, 784). Also add activeLeafId to the useMemo dependency array so the closure always captures the current leaf. Result: pressing Escape in the search bar now closes it and hands focus back to the active terminal pane, as described in issue crynta#195. Closes crynta#195
Owner
|
Tracked under #207 which landed the same fix. Closing as duplicate, thanks for the report. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
One-line bug fix so that pressing Escape in the search bar hands focus back to the active terminal pane.
Also adds
activeLeafIdto theuseMemodependency array (it was missing, so the closure would capture a stale leaf ID on pane switches).Why
Closes #195.
searchTarget.focus()for a terminal pane calledterminalRefs.current.get(activeId), whereactiveIdis the tab ID. ButterminalRefsis keyed by leaf ID — so the lookup always returnedundefined, andfocus()was never called. Every other callsite inApp.tsxthat addressesterminalRefsalready usesactiveLeafId(lines 508, 525, 774, 784).The
SearchInlinecomponent already had the correct Escape handler and calledrestoreTargetFocus()— it was just silently no-oping because the handle was never found.How it was tested
tsc --noEmit— zero new errors (3 pre-existing missing-dep errors unchanged)terminalRefs.current.get(activeId)returnsundefinedsince the map holds leaf IDs as keysactiveLeafIdis derived fromactiveTerminalTab?.activeLeafId(line 112) and is the correct key