From 38ca822e7f03f03c39d8181ed3148f3da8d11a43 Mon Sep 17 00:00:00 2001 From: Clint Berry Date: Tue, 28 Jul 2026 19:03:30 +0000 Subject: [PATCH 1/2] feat(terminal): focus on tab select and show a start-up indicator MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- src/components/terminal/TerminalView.test.tsx | 68 ++++++++++++++++++- src/components/terminal/TerminalView.tsx | 54 +++++++++++++-- 2 files changed, 116 insertions(+), 6 deletions(-) diff --git a/src/components/terminal/TerminalView.test.tsx b/src/components/terminal/TerminalView.test.tsx index 0783d09..6986f7e 100644 --- a/src/components/terminal/TerminalView.test.tsx +++ b/src/components/terminal/TerminalView.test.tsx @@ -1,5 +1,5 @@ import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; -import { render, act } from "@testing-library/react"; +import { render, act, screen } from "@testing-library/react"; // Declared through vi.hoisted so the vi.mock factories below — which are // hoisted to the top of the module — can reach them. @@ -15,6 +15,7 @@ const { FakeTerminal, FakeWebSocket } = vi.hoisted(() => { rows = 24; written: string[] = []; disposed = false; + focusCount = 0; private queue: Array<{ data: string; cb?: () => void }> = []; private dataHandler: ((d: string) => void) | undefined; @@ -25,6 +26,9 @@ const { FakeTerminal, FakeWebSocket } = vi.hoisted(() => { loadAddon() {} open() {} + focus() { + this.focusCount++; + } dispose() { this.disposed = true; } @@ -298,6 +302,68 @@ describe("TerminalView replay gate", () => { }); }); + it("focuses the terminal on mount so selecting the tab lands the cursor", () => { + const { term } = setup(); + + expect(term.focusCount).toBe(1); + }); + + it("shows a loading indicator while the terminal is still blank", () => { + vi.useFakeTimers(); + setup(); + + // Nothing yet — the indicator is delayed to avoid flickering on a fast + // attach, so it must not be up immediately. + expect(screen.queryByRole("status")).not.toBeInTheDocument(); + + act(() => { + vi.advanceTimersByTime(250); + }); + + expect(screen.getByRole("status")).toHaveTextContent("Starting terminal"); + }); + + it("does not flash the indicator when output arrives promptly", () => { + vi.useFakeTimers(); + const { ws } = setup(); + + deliver(ws, 0x02, "previous scrollback"); + act(() => { + vi.advanceTimersByTime(5000); + }); + + expect(screen.queryByRole("status")).not.toBeInTheDocument(); + }); + + it("hides the loading indicator once the first output arrives", () => { + vi.useFakeTimers(); + const { ws } = setup(); + + act(() => { + vi.advanceTimersByTime(250); + }); + expect(screen.getByRole("status")).toBeInTheDocument(); + + deliver(ws, 0x00, "user@container:~$ "); + + expect(screen.queryByRole("status")).not.toBeInTheDocument(); + }); + + it("keeps the indicator up when only the replay boundary has arrived", () => { + // `devpod ssh` spawns fast but connects slowly, so 0x03 lands on a blank + // screen. Treating it as ready would drop the spinner too early. + vi.useFakeTimers(); + const { term, ws } = setup(); + + deliver(ws, 0x03); + act(() => term.flush()); + act(() => { + vi.advanceTimersByTime(250); + }); + + expect(screen.getByRole("status")).toBeInTheDocument(); + }); + it("clears the fallback timer on unmount", () => { vi.useFakeTimers(); const { unmount } = render(); diff --git a/src/components/terminal/TerminalView.tsx b/src/components/terminal/TerminalView.tsx index 7d3d4c7..defa200 100644 --- a/src/components/terminal/TerminalView.tsx +++ b/src/components/terminal/TerminalView.tsx @@ -1,4 +1,4 @@ -import { useEffect, useRef } from "react"; +import { useEffect, useRef, useState } from "react"; import { Loader2, AlertCircle } from "lucide-react"; import { Terminal } from "@xterm/xterm"; import { FitAddon } from "@xterm/addon-fit"; @@ -6,10 +6,22 @@ import { WebLinksAddon } from "@xterm/addon-web-links"; import "@xterm/xterm/css/xterm.css"; import { useSessionStore } from "@/stores/session-store"; +// How long the terminal may stay blank before we show a spinner. Attaching to +// an existing PTY paints from the replay buffer almost immediately, and a +// spinner that appears for 20ms reads as a flicker rather than as feedback. +const SPINNER_DELAY_MS = 250; + export function TerminalView() { const { activeSessionId, sessions } = useSessionStore(); const terminalRef = useRef(null); + // `devpod ssh` spawns immediately but takes seconds to establish the + // connection into the container, so a first attach sits on a black + // rectangle with no explanation. First output is the honest ready signal: + // the replay boundary arrives long before the shell has drawn anything. + const [hasOutput, setHasOutput] = useState(false); + const [spinnerVisible, setSpinnerVisible] = useState(false); + const session = sessions.find((s) => s.id === activeSessionId); useEffect(() => { @@ -18,6 +30,14 @@ export function TerminalView() { let disposed = false; let ws: WebSocket | null = null; + // Switching sessions re-runs this effect without remounting, so the + // previous session's ready state must not carry over. + setHasOutput(false); + setSpinnerVisible(false); + const spinnerTimer = setTimeout(() => { + if (!disposed) setSpinnerVisible(true); + }, SPINNER_DELAY_MS); + // The server replays recent PTY output to every newly-attached client so // the terminal isn't blank on reconnect. That buffer can contain terminal // *queries* the remote shell emitted earlier (OSC 11 background-colour, @@ -69,6 +89,11 @@ export function TerminalView() { term.loadAddon(webLinksAddon); term.open(terminalRef.current); fitAddon.fit(); + // The tab was just selected, so the terminal is what the user came for — + // put the cursor in it rather than making them click first. This effect + // re-runs on every mount, and CenterPanel remounts TerminalView on tab + // switch, so selecting the tab focuses the terminal. + term.focus(); // Send keystrokes to WebSocket with 0x00 prefix term.onData((data) => { @@ -125,6 +150,12 @@ export function TerminalView() { const data = new Uint8Array(event.data as ArrayBuffer); if (data.length < 1) return; + // Any payload-bearing frame means the terminal has something to show. + if ((data[0] === 0x00 || data[0] === 0x02) && data.length > 1) { + clearTimeout(spinnerTimer); + setHasOutput(true); + } + switch (data[0]) { case 0x02: // Replayed output renders exactly like live output — the frame type @@ -153,6 +184,7 @@ export function TerminalView() { return () => { disposed = true; clearTimeout(unmuteTimer); + clearTimeout(spinnerTimer); resizeObserver.disconnect(); if (ws) { ws.onmessage = null; @@ -198,10 +230,22 @@ export function TerminalView() { ); } + // The terminal container must stay mounted while connecting — xterm has + // already attached to it — so the indicator overlays rather than replaces it. return ( -
+
+
+ {!hasOutput && spinnerVisible && ( +
+
+ +

Starting terminal...

+
+
+ )} +
); } From 0dce5e2153d7b4708cb5bc3b40e02e4d23abb28c Mon Sep 17 00:00:00 2001 From: Clint Berry Date: Tue, 28 Jul 2026 19:04:25 +0000 Subject: [PATCH 2/2] test(terminal): make the anti-flicker delay assertion meaningful 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. --- src/components/terminal/TerminalView.test.tsx | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/components/terminal/TerminalView.test.tsx b/src/components/terminal/TerminalView.test.tsx index 6986f7e..35257f3 100644 --- a/src/components/terminal/TerminalView.test.tsx +++ b/src/components/terminal/TerminalView.test.tsx @@ -324,9 +324,17 @@ describe("TerminalView replay gate", () => { }); it("does not flash the indicator when output arrives promptly", () => { + // Re-attaching to a running PTY paints from the replay buffer within a + // few milliseconds. The indicator must never appear at all in that + // window — showing and immediately hiding it reads as a glitch. vi.useFakeTimers(); const { ws } = setup(); + act(() => { + vi.advanceTimersByTime(20); + }); + expect(screen.queryByRole("status")).not.toBeInTheDocument(); + deliver(ws, 0x02, "previous scrollback"); act(() => { vi.advanceTimersByTime(5000);