feat(terminal): focus on tab select and show a start-up indicator - #44
Merged
Conversation
Two papercuts on the Terminal tab. Selecting the tab left the cursor unfocused, so the first keystroke went nowhere and users had to click into the terminal first. CenterPanel remounts TerminalView on tab switch, so focusing after open() means selecting the tab now lands the cursor. First attach also sat on a black rectangle for several seconds with no explanation: pty.Start returns as soon as `devpod ssh` spawns, but the connection into the container takes far longer. Show a spinner until the first payload-bearing frame arrives. The readiness signal is first output, not the replay boundary — 0x03 lands almost immediately on a blank screen, so keying off it would drop the indicator while there is still nothing to see. The indicator is delayed 250ms so attaching to an existing PTY, which paints from the replay buffer in milliseconds, doesn't flicker.
The no-flash test delivered output before any timer processing, so it passed whether the delay was 250ms or 0. Advance a realistic attach latency first and assert the indicator is absent at that point, which fails if the delay is removed.
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
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.
Summary
Two papercuts on the Terminal tab:
Focus on tab select
CenterPanelrenders<TerminalView />conditionally on the active tab, so selecting the tab mounts the component. Callingterm.focus()afteropen()means that mount — i.e. the tab selection — lands the cursor in the terminal.Start-up indicator
A spinner overlays the terminal container while it has nothing to show. Two decisions worth calling out.
The ready signal is first output, not the replay boundary.
pty.Startreturns as soon asdevpod sshspawns, but establishing the connection into the container is the slow part. The0x03replay-complete frame therefore lands almost immediately on a still-blank screen — keying the indicator off it would hide it while there is genuinely nothing to see. It clears on the first payload-bearing0x00/0x02frame instead.The indicator is delayed 250ms. Re-attaching to a running PTY paints from the replay buffer within milliseconds. An undelayed spinner would flicker on every tab switch, which reads as a glitch rather than as feedback.
The indicator overlays the terminal container rather than replacing it — xterm has already attached to that node, so it has to stay mounted while connecting.
Test plan
84 tests pass (5 new);
tsc -bclean,eslint0 errors.New coverage in
TerminalView.test.tsx:0x03as ready.All three behaviours were mutation-tested: removing the focus call, treating
0x03as ready, and dropping the delay each fail a specific test. The delay assertion needed strengthening to earn that (second commit) — the original version delivered output before any timer processing, so it passed whether the delay was 250ms or 0.Post-Deploy Monitoring & Validation
No additional operational monitoring required — frontend-only, no backend, data, or auth impact.
Validate manually after deploy:
Known residuals / follow-ups
devpod sshdies during startup the PTY closes, but nothing signals that over the socket, so the spinner would persist indefinitely. The same failure previously showed as a permanently black screen, so this is not a regression — but a spinner implies progress in a way a black screen does not. Mitigations range from swapping the message after ~20s to having the server signal PTY exit; deliberately out of scope here.