fix(FN-2500): improve terminal modal sizing and input behavior
- Widen the desktop terminal modal to improve usability and align with UX review feedback - Stabilize terminal input lifecycle handling to avoid focus and interaction regressions - Add regression coverage for terminal modal input behavior and mobile keyboard layout scenarios - Include a changeset documenting the terminal modal desktop width and input fixes
This commit is contained in:
5
.changeset/fix-terminal-modal-desktop-width-input.md
Normal file
5
.changeset/fix-terminal-modal-desktop-width-input.md
Normal file
@@ -0,0 +1,5 @@
|
|||||||
|
---
|
||||||
|
"@runfusion/fusion": patch
|
||||||
|
---
|
||||||
|
|
||||||
|
Fix the dashboard terminal modal desktop width contract so large displays use a broad viewport-based layout, and harden terminal input lifecycle handling so xterm keyboard input continues forwarding reliably after rerenders.
|
||||||
@@ -153,13 +153,24 @@ describe("terminal mobile keyboard layout CSS contract", () => {
|
|||||||
return match?.[1] ?? "";
|
return match?.[1] ?? "";
|
||||||
}
|
}
|
||||||
|
|
||||||
it("has min-height: 80vh on desktop", () => {
|
it("uses viewport-based width with desktop side margins", () => {
|
||||||
const ruleBody = findDesktopTerminalModalRule();
|
const ruleBody = findDesktopTerminalModalRule();
|
||||||
expect(ruleBody).toContain("min-height: 80vh");
|
expect(ruleBody).toContain(
|
||||||
|
"width: min(1800px, calc(100vw - (var(--space-xl) * 2)))",
|
||||||
|
);
|
||||||
|
expect(ruleBody).toContain(
|
||||||
|
"max-width: calc(100vw - (var(--space-xl) * 2))",
|
||||||
|
);
|
||||||
});
|
});
|
||||||
|
|
||||||
it("has max-height: 85vh on desktop", () => {
|
it("does not cap desktop width to the old narrow 1600px max", () => {
|
||||||
const ruleBody = findDesktopTerminalModalRule();
|
const ruleBody = findDesktopTerminalModalRule();
|
||||||
|
expect(ruleBody).not.toContain("max-width: 1600px");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("keeps desktop height constraints", () => {
|
||||||
|
const ruleBody = findDesktopTerminalModalRule();
|
||||||
|
expect(ruleBody).toContain("min-height: 80vh");
|
||||||
expect(ruleBody).toContain("max-height: 85vh");
|
expect(ruleBody).toContain("max-height: 85vh");
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -7,8 +7,8 @@
|
|||||||
}
|
}
|
||||||
|
|
||||||
.modal.terminal-modal {
|
.modal.terminal-modal {
|
||||||
width: 90vw;
|
width: min(1800px, calc(100vw - (var(--space-xl) * 2)));
|
||||||
max-width: 1600px;
|
max-width: calc(100vw - (var(--space-xl) * 2));
|
||||||
min-height: 80vh;
|
min-height: 80vh;
|
||||||
max-height: 85vh;
|
max-height: 85vh;
|
||||||
background: var(--card);
|
background: var(--card);
|
||||||
@@ -409,7 +409,7 @@
|
|||||||
}
|
}
|
||||||
|
|
||||||
.terminal-output-error {
|
.terminal-output-error {
|
||||||
background: rgba(239, 68, 68, 0.1);
|
background: color-mix(in srgb, var(--color-error) 10%, transparent);
|
||||||
border-left: 3px solid var(--failed);
|
border-left: 3px solid var(--failed);
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -678,7 +678,7 @@
|
|||||||
|
|
||||||
.terminal-error {
|
.terminal-error {
|
||||||
padding: var(--space-md) var(--space-lg);
|
padding: var(--space-md) var(--space-lg);
|
||||||
background: rgba(179, 38, 38, 0.1);
|
background: color-mix(in srgb, var(--color-error) 10%, transparent);
|
||||||
border-bottom: 1px solid var(--border);
|
border-bottom: 1px solid var(--border);
|
||||||
color: var(--error);
|
color: var(--error);
|
||||||
font-size: 13px;
|
font-size: 13px;
|
||||||
@@ -762,7 +762,7 @@
|
|||||||
justify-content: flex-end;
|
justify-content: flex-end;
|
||||||
border-top: 1px solid var(--border);
|
border-top: 1px solid var(--border);
|
||||||
padding: 0 4px;
|
padding: 0 4px;
|
||||||
min-height: 32px;
|
min-height: 36px;
|
||||||
}
|
}
|
||||||
|
|
||||||
/* Hide text labels on action buttons to save space */
|
/* Hide text labels on action buttons to save space */
|
||||||
@@ -774,8 +774,8 @@
|
|||||||
.terminal-restart-btn,
|
.terminal-restart-btn,
|
||||||
.terminal-clear-btn {
|
.terminal-clear-btn {
|
||||||
padding: 8px;
|
padding: 8px;
|
||||||
min-height: 32px;
|
min-height: 36px;
|
||||||
min-width: 32px;
|
min-width: 36px;
|
||||||
justify-content: center;
|
justify-content: center;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -305,12 +305,15 @@ export function TerminalModal({ isOpen, onClose, initialCommand, projectId }: Te
|
|||||||
// without needing resize as a dependency (avoids ordering issues).
|
// without needing resize as a dependency (avoids ordering issues).
|
||||||
resizeRef.current = resize;
|
resizeRef.current = resize;
|
||||||
|
|
||||||
// Initialize xterm.js when session is ready
|
// Initialize xterm.js when session is ready.
|
||||||
// Depends on `isReady`, `activeTab`, and xtermReady to properly reinitialize on tab switch
|
// Keying this effect by active session id (not full activeTab object) avoids
|
||||||
|
// tearing down xterm lifecycle wiring during unrelated tab metadata updates
|
||||||
|
// such as title changes.
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
if (!isOpen || !isReady || !activeTab) return;
|
if (!isOpen || !isReady) return;
|
||||||
|
|
||||||
const currentSessionId = activeTab.sessionId;
|
const currentSessionId = activeTab?.sessionId;
|
||||||
|
if (!currentSessionId) return;
|
||||||
|
|
||||||
// Detect project switch: if projectId changed, invalidate xterm even if sessionId is the same.
|
// Detect project switch: if projectId changed, invalidate xterm even if sessionId is the same.
|
||||||
// This ensures xterm content from the previous project is not displayed in the new project.
|
// This ensures xterm content from the previous project is not displayed in the new project.
|
||||||
@@ -335,7 +338,7 @@ export function TerminalModal({ isOpen, onClose, initialCommand, projectId }: Te
|
|||||||
}
|
}
|
||||||
|
|
||||||
let mounted = true;
|
let mounted = true;
|
||||||
let watchdogTimer: ReturnType<typeof setTimeout>;
|
let watchdogTimer: ReturnType<typeof setTimeout> | undefined;
|
||||||
|
|
||||||
const initTerminal = async () => {
|
const initTerminal = async () => {
|
||||||
// Dynamically import xterm modules with watchdog timeout
|
// Dynamically import xterm modules with watchdog timeout
|
||||||
@@ -414,7 +417,9 @@ export function TerminalModal({ isOpen, onClose, initialCommand, projectId }: Te
|
|||||||
terminal.open(terminalRef.current);
|
terminal.open(terminalRef.current);
|
||||||
|
|
||||||
// Clear watchdog — imports and open() succeeded within deadline
|
// Clear watchdog — imports and open() succeeded within deadline
|
||||||
clearTimeout(watchdogTimer);
|
if (watchdogTimer) {
|
||||||
|
clearTimeout(watchdogTimer);
|
||||||
|
}
|
||||||
|
|
||||||
// Ensure xterm's textarea receives focus for keyboard input.
|
// Ensure xterm's textarea receives focus for keyboard input.
|
||||||
// xterm.js creates a hidden textarea that captures keyboard events.
|
// xterm.js creates a hidden textarea that captures keyboard events.
|
||||||
@@ -470,54 +475,73 @@ export function TerminalModal({ isOpen, onClose, initialCommand, projectId }: Te
|
|||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
// Signal that xterm is ready so the subscription effect re-runs
|
// Signal that xterm is ready so lifecycle effects can subscribe.
|
||||||
setXtermReady(true);
|
setXtermReady(true);
|
||||||
// Clear any prior xterm init error
|
// Clear any prior xterm init error
|
||||||
setXtermInitError(null);
|
setXtermInitError(null);
|
||||||
|
|
||||||
// Handle data from terminal (user input)
|
|
||||||
const dataHandler = terminal.onData((data) => {
|
|
||||||
sendInput(data);
|
|
||||||
});
|
|
||||||
|
|
||||||
// Handle resize
|
|
||||||
const resizeHandler = () => {
|
|
||||||
if (fitAddonRef.current && xtermRef.current) {
|
|
||||||
try {
|
|
||||||
(fitAddonRef.current as InstanceType<typeof FitAddon>).fit();
|
|
||||||
const { cols, rows } = xtermRef.current;
|
|
||||||
resize(cols, rows);
|
|
||||||
} catch {
|
|
||||||
// Ignore fit errors
|
|
||||||
}
|
|
||||||
}
|
|
||||||
};
|
|
||||||
|
|
||||||
window.addEventListener("resize", resizeHandler);
|
|
||||||
|
|
||||||
return () => {
|
|
||||||
dataHandler.dispose();
|
|
||||||
window.removeEventListener("resize", resizeHandler);
|
|
||||||
};
|
|
||||||
} catch (err) {
|
} catch (err) {
|
||||||
clearTimeout(watchdogTimer);
|
if (watchdogTimer) {
|
||||||
|
clearTimeout(watchdogTimer);
|
||||||
|
}
|
||||||
if (!mounted) return;
|
if (!mounted) return;
|
||||||
const message = err instanceof Error ? err.message : "xterm initialization failed";
|
const message = err instanceof Error ? err.message : "xterm initialization failed";
|
||||||
setXtermInitError(message);
|
setXtermInitError(message);
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
const cleanupPromise = initTerminal();
|
void initTerminal();
|
||||||
|
|
||||||
return () => {
|
return () => {
|
||||||
mounted = false;
|
mounted = false;
|
||||||
clearTimeout(watchdogTimer);
|
if (watchdogTimer) {
|
||||||
cleanupPromise.then((cleanup) => cleanup?.());
|
clearTimeout(watchdogTimer);
|
||||||
|
}
|
||||||
|
|
||||||
// Don't dispose xterm here - it should persist across tab switches
|
// Don't dispose xterm here - it should persist across tab switches
|
||||||
// Only dispose when the modal is fully closed
|
// Only dispose when the modal is fully closed
|
||||||
};
|
};
|
||||||
}, [fitAndResizeForSession, isOpen, isReady, activeTab, activeTab?.sessionId, sendInput, resize]);
|
}, [fitAndResizeForSession, isOpen, isReady, activeTab?.sessionId, projectId]);
|
||||||
|
|
||||||
|
// Keep user input forwarding and resize publishing attached to the current
|
||||||
|
// xterm instance/session. This prevents unrelated rerenders (tab title or
|
||||||
|
// status updates) from silently dropping onData -> sendInput wiring.
|
||||||
|
useEffect(() => {
|
||||||
|
if (!isOpen || !xtermReady || !activeTab?.sessionId || !xtermRef.current) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
const expectedSessionId = activeTab.sessionId;
|
||||||
|
const terminal = xtermRef.current;
|
||||||
|
|
||||||
|
const dataHandler = terminal.onData((data) => {
|
||||||
|
if (xtermInitializedRef.current !== expectedSessionId) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
sendInput(data);
|
||||||
|
});
|
||||||
|
|
||||||
|
const resizeHandler = () => {
|
||||||
|
if (xtermInitializedRef.current !== expectedSessionId) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
if (fitAddonRef.current && xtermRef.current) {
|
||||||
|
try {
|
||||||
|
(fitAddonRef.current as InstanceType<typeof FitAddon>).fit();
|
||||||
|
const { cols, rows } = xtermRef.current;
|
||||||
|
resize(cols, rows);
|
||||||
|
} catch {
|
||||||
|
// Ignore fit errors
|
||||||
|
}
|
||||||
|
}
|
||||||
|
};
|
||||||
|
|
||||||
|
window.addEventListener("resize", resizeHandler);
|
||||||
|
|
||||||
|
return () => {
|
||||||
|
dataHandler.dispose();
|
||||||
|
window.removeEventListener("resize", resizeHandler);
|
||||||
|
};
|
||||||
|
}, [isOpen, xtermReady, activeTab?.sessionId, sendInput, resize]);
|
||||||
|
|
||||||
// Cleanup xterm when modal closes
|
// Cleanup xterm when modal closes
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
|
|||||||
@@ -198,7 +198,7 @@ describe("AgentsView", () => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
// Ensure the single-load path still powers dependent UI sections.
|
// Ensure the single-load path still powers dependent UI sections.
|
||||||
expect(screen.getByText("Active Agents (1)")).toBeTruthy();
|
expect(await screen.findByText("Active Agents (1)")).toBeTruthy();
|
||||||
});
|
});
|
||||||
|
|
||||||
it("renders token stats derived from the currently displayed agents", async () => {
|
it("renders token stats derived from the currently displayed agents", async () => {
|
||||||
|
|||||||
@@ -950,11 +950,11 @@ describe("MissionManager", () => {
|
|||||||
|
|
||||||
fireEvent.click(screen.getByTestId("mission-activity-load-more"));
|
fireEvent.click(screen.getByTestId("mission-activity-load-more"));
|
||||||
|
|
||||||
await waitFor(() => {
|
await screen.findByText("Mission event 65", undefined, { timeout: 10_000 });
|
||||||
expect(screen.getByText("Mission event 65")).toBeDefined();
|
|
||||||
}, { timeout: 5000 });
|
|
||||||
|
|
||||||
expect(screen.queryByTestId("mission-activity-load-more")).toBeNull();
|
await waitFor(() => {
|
||||||
|
expect(screen.queryByTestId("mission-activity-load-more")).toBeNull();
|
||||||
|
});
|
||||||
}, 15000);
|
}, 15000);
|
||||||
|
|
||||||
it("auto-scrolls to latest mission activity on initial load", async () => {
|
it("auto-scrolls to latest mission activity on initial load", async () => {
|
||||||
|
|||||||
@@ -3600,6 +3600,7 @@ describe("TerminalModal — xterm focus initialization (FN-1602)", () => {
|
|||||||
|
|
||||||
beforeEach(() => {
|
beforeEach(() => {
|
||||||
vi.clearAllMocks();
|
vi.clearAllMocks();
|
||||||
|
mockTerminalInstance.onData.mockImplementation(() => ({ dispose: vi.fn() }));
|
||||||
mockCreateTerminalSession.mockResolvedValue({
|
mockCreateTerminalSession.mockResolvedValue({
|
||||||
sessionId: "test-session-123",
|
sessionId: "test-session-123",
|
||||||
shell: "/bin/bash",
|
shell: "/bin/bash",
|
||||||
@@ -3687,6 +3688,105 @@ describe("TerminalModal — xterm focus initialization (FN-1602)", () => {
|
|||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("forwards xterm onData input to sendInput", async () => {
|
||||||
|
let terminalInputCallback: ((data: string) => void) | null = null;
|
||||||
|
mockTerminalInstance.onData.mockImplementation((cb: (data: string) => void) => {
|
||||||
|
terminalInputCallback = cb;
|
||||||
|
return { dispose: vi.fn() };
|
||||||
|
});
|
||||||
|
|
||||||
|
render(<TerminalModal isOpen={true} onClose={mockOnClose} />);
|
||||||
|
|
||||||
|
await waitFor(() => {
|
||||||
|
expect(terminalInputCallback).not.toBeNull();
|
||||||
|
});
|
||||||
|
|
||||||
|
act(() => {
|
||||||
|
terminalInputCallback?.("echo hello\r");
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(mockSendInput).toHaveBeenCalledWith("echo hello\r");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("keeps terminal input forwarding active after active-tab title rerenders", async () => {
|
||||||
|
let terminalInputCallback: ((data: string) => void) | null = null;
|
||||||
|
const disposeInputHandler = vi.fn();
|
||||||
|
|
||||||
|
mockTerminalInstance.onData.mockImplementation((cb: (data: string) => void) => {
|
||||||
|
terminalInputCallback = cb;
|
||||||
|
return { dispose: disposeInputHandler };
|
||||||
|
});
|
||||||
|
|
||||||
|
const { rerender } = render(
|
||||||
|
<TerminalModal isOpen={true} onClose={mockOnClose} />,
|
||||||
|
);
|
||||||
|
|
||||||
|
await waitFor(() => {
|
||||||
|
expect(terminalInputCallback).not.toBeNull();
|
||||||
|
});
|
||||||
|
|
||||||
|
// Simulate tab metadata update (title change) that should not tear down
|
||||||
|
// input forwarding for the same session.
|
||||||
|
const renamedTab = {
|
||||||
|
...defaultTab,
|
||||||
|
title: "bash (connected)",
|
||||||
|
isActive: true,
|
||||||
|
};
|
||||||
|
|
||||||
|
mockUseTerminalSessions.mockReturnValue({
|
||||||
|
...defaultSessionState,
|
||||||
|
tabs: [renamedTab],
|
||||||
|
activeTab: renamedTab,
|
||||||
|
});
|
||||||
|
|
||||||
|
rerender(<TerminalModal isOpen={true} onClose={mockOnClose} />);
|
||||||
|
|
||||||
|
expect(disposeInputHandler).not.toHaveBeenCalled();
|
||||||
|
expect(mockTerminalInstance.onData).toHaveBeenCalledTimes(1);
|
||||||
|
|
||||||
|
act(() => {
|
||||||
|
terminalInputCallback?.("pwd\r");
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(mockSendInput).toHaveBeenCalledWith("pwd\r");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("keeps terminal input forwarding active after connection status transitions", async () => {
|
||||||
|
let terminalInputCallback: ((data: string) => void) | null = null;
|
||||||
|
const disposeInputHandler = vi.fn();
|
||||||
|
|
||||||
|
mockTerminalInstance.onData.mockImplementation((cb: (data: string) => void) => {
|
||||||
|
terminalInputCallback = cb;
|
||||||
|
return { dispose: disposeInputHandler };
|
||||||
|
});
|
||||||
|
|
||||||
|
mockUseTerminal.mockReturnValue(
|
||||||
|
createMockTerminalState({ connectionStatus: "disconnected" }),
|
||||||
|
);
|
||||||
|
|
||||||
|
const { rerender } = render(
|
||||||
|
<TerminalModal isOpen={true} onClose={mockOnClose} />,
|
||||||
|
);
|
||||||
|
|
||||||
|
await waitFor(() => {
|
||||||
|
expect(terminalInputCallback).not.toBeNull();
|
||||||
|
});
|
||||||
|
|
||||||
|
mockUseTerminal.mockReturnValue(
|
||||||
|
createMockTerminalState({ connectionStatus: "connected" }),
|
||||||
|
);
|
||||||
|
rerender(<TerminalModal isOpen={true} onClose={mockOnClose} />);
|
||||||
|
|
||||||
|
expect(disposeInputHandler).not.toHaveBeenCalled();
|
||||||
|
expect(mockTerminalInstance.onData).toHaveBeenCalledTimes(1);
|
||||||
|
|
||||||
|
act(() => {
|
||||||
|
terminalInputCallback?.("ls\r");
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(mockSendInput).toHaveBeenCalledWith("ls\r");
|
||||||
|
});
|
||||||
|
|
||||||
it("focuses xterm helper textarea on user pointer gesture", async () => {
|
it("focuses xterm helper textarea on user pointer gesture", async () => {
|
||||||
render(<TerminalModal isOpen={true} onClose={mockOnClose} />);
|
render(<TerminalModal isOpen={true} onClose={mockOnClose} />);
|
||||||
|
|
||||||
|
|||||||
@@ -743,7 +743,7 @@ describe("POST /api/projects route handler", () => {
|
|||||||
} finally {
|
} finally {
|
||||||
rmSync(tempRoot, { recursive: true, force: true });
|
rmSync(tempRoot, { recursive: true, force: true });
|
||||||
}
|
}
|
||||||
});
|
}, 15_000);
|
||||||
|
|
||||||
it("returns clone failure and skips registration when git clone fails", async () => {
|
it("returns clone failure and skips registration when git clone fails", async () => {
|
||||||
const store = new MockStoreForRoutes();
|
const store = new MockStoreForRoutes();
|
||||||
|
|||||||
@@ -297,5 +297,5 @@ describe("diff-base parity between dashboard and merger", () => {
|
|||||||
} finally {
|
} finally {
|
||||||
rmSync(repoDir, { recursive: true, force: true });
|
rmSync(repoDir, { recursive: true, force: true });
|
||||||
}
|
}
|
||||||
});
|
}, 15_000);
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -10057,7 +10057,7 @@ describe("Saturated-slot regression: heartbeat wake routes", () => {
|
|||||||
} finally {
|
} finally {
|
||||||
rmSync(tempDir, { recursive: true, force: true });
|
rmSync(tempDir, { recursive: true, force: true });
|
||||||
}
|
}
|
||||||
});
|
}, 15_000);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user