Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 75 additions & 1 deletion src/components/terminal/TerminalView.test.tsx
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -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;
Expand All @@ -25,6 +26,9 @@ const { FakeTerminal, FakeWebSocket } = vi.hoisted(() => {

loadAddon() {}
open() {}
focus() {
this.focusCount++;
}
dispose() {
this.disposed = true;
}
Expand Down Expand Up @@ -298,6 +302,76 @@ 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", () => {
// 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);
});

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(<TerminalView />);
Expand Down
54 changes: 49 additions & 5 deletions src/components/terminal/TerminalView.tsx
Original file line number Diff line number Diff line change
@@ -1,15 +1,27 @@
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";
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<HTMLDivElement>(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(() => {
Expand All @@ -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,
Expand Down Expand Up @@ -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) => {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -153,6 +184,7 @@ export function TerminalView() {
return () => {
disposed = true;
clearTimeout(unmuteTimer);
clearTimeout(spinnerTimer);
resizeObserver.disconnect();
if (ws) {
ws.onmessage = null;
Expand Down Expand Up @@ -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 (
<div
ref={terminalRef}
className="h-full w-full bg-[#0d1117] p-3"
/>
<div className="relative h-full w-full bg-[#0d1117]">
<div ref={terminalRef} className="h-full w-full p-3" />
{!hasOutput && spinnerVisible && (
<div
role="status"
className="absolute inset-0 flex items-center justify-center bg-[#0d1117]"
>
<div className="text-center">
<Loader2 className="mx-auto h-8 w-8 text-accent animate-spin mb-3" />
<p className="text-sm text-foreground-muted">Starting terminal...</p>
</div>
</div>
)}
</div>
);
}
Loading