fix(site): fix terminal size when displaying alerts (#12444)

Before - The terminal size does not fit the available space so the bottom is hidden.

https://github.com/coder/coder/assets/3165839/d08470b9-9fc6-476c-a551-8a3e13fc25bf

After - The terminal adjusts when there are alert changes.

https://github.com/coder/coder/assets/3165839/8cc32bfb-056f-47cb-97f2-3bb18c5fe906

Unfortunately, I don't think there is a sane way to automate tests for this but open to suggestions.

Close https://github.com/coder/coder/issues/7914
This commit is contained in:
Bruno Quaresma 2024-03-08 07:38:40 -03:00 committed by GitHub
parent d2a5b31b2b
commit 060033e4ef
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 104 additions and 15 deletions

View File

@ -69,6 +69,13 @@ export const parameters = {
},
type: "tablet",
},
terminal: {
name: "Terminal",
styles: {
height: "400",
width: "400",
},
},
},
},
};

View File

@ -3,7 +3,9 @@ import type { QueryKey } from "react-query";
import type { Experiments, FeatureName } from "api/typesGenerated";
declare module "@storybook/react" {
type WebSocketEvent = { event: "message"; data: string } | { event: "error" };
type WebSocketEvent =
| { event: "message"; data: string }
| { event: "error" | "close" };
interface Parameters {
features?: FeatureName[];
experiments?: Experiments;

View File

@ -142,3 +142,27 @@ export const ConnectionError: Story = {
queries: [...meta.parameters.queries, createWorkspaceWithAgent("ready")],
},
};
// Check if the terminal is not getting hide when the bottom message is shown
// together with the error message
export const BottomMessage: Story = {
decorators: [withWebSocket],
parameters: {
...meta.parameters,
// Forcing smaller viewport to make it easier to identify the issue
viewport: {
defaultViewport: "terminal",
},
webSocket: [
{
event: "message",
// This outputs text in the bottom left and right corners of the terminal.
data: "\x1b[1000BLEFT\x1b[1000C\x1b[4DRIGHT",
},
{
event: "close",
},
],
queries: [...meta.parameters.queries, createWorkspaceWithAgent("ready")],
},
};

View File

@ -13,6 +13,7 @@ import { WebLinksAddon } from "xterm-addon-web-links";
import { WebglAddon } from "xterm-addon-webgl";
import { deploymentConfig } from "api/queries/deployment";
import { workspaceByOwnerAndName } from "api/queries/workspaces";
import type { WorkspaceAgent } from "api/typesGenerated";
import { useProxy } from "contexts/ProxyContext";
import { ThemeOverride } from "contexts/ThemeProvider";
import themes from "theme";
@ -34,6 +35,8 @@ export const Language = {
websocketErrorMessagePrefix: "WebSocket failed: ",
};
type TerminalState = "connected" | "disconnected" | "initializing";
const TerminalPage: FC = () => {
// Maybe one day we'll support a light themed terminal, but terminal coloring
// is notably a pain because of assumptions certain programs might make about your
@ -45,9 +48,8 @@ const TerminalPage: FC = () => {
const username = params.username.replace("@", "");
const xtermRef = useRef<HTMLDivElement>(null);
const [terminal, setTerminal] = useState<XTerm.Terminal | null>(null);
const [terminalState, setTerminalState] = useState<
"connected" | "disconnected" | "initializing"
>("initializing");
const [terminalState, setTerminalState] =
useState<TerminalState>("initializing");
const [searchParams] = useSearchParams();
const isDebugging = searchParams.has("debug");
// The reconnection token is a unique token that identifies
@ -67,12 +69,6 @@ const TerminalPage: FC = () => {
const selectedProxy = proxy.proxy;
const latency = selectedProxy ? proxyLatencies[selectedProxy.id] : undefined;
const lifecycleState = workspaceAgent?.lifecycle_state;
const prevLifecycleState = useRef(lifecycleState);
useEffect(() => {
prevLifecycleState.current = lifecycleState;
}, [lifecycleState]);
const config = useQuery(deploymentConfig());
const renderer = config.data?.config.web_terminal_renderer;
@ -95,6 +91,7 @@ const TerminalPage: FC = () => {
}, [handleWebLink]);
// Create the terminal!
const fitAddonRef = useRef<FitAddon>();
useEffect(() => {
if (!xtermRef.current || config.isLoading) {
return;
@ -115,6 +112,7 @@ const TerminalPage: FC = () => {
terminal.loadAddon(new CanvasAddon());
}
const fitAddon = new FitAddon();
fitAddonRef.current = fitAddon;
terminal.loadAddon(fitAddon);
terminal.loadAddon(new Unicode11Addon());
terminal.unicode.activeVersion = "11";
@ -303,11 +301,13 @@ const TerminalPage: FC = () => {
</title>
</Helmet>
<div css={{ display: "flex", flexDirection: "column", height: "100vh" }}>
{lifecycleState === "start_error" && <ErrorScriptAlert />}
{lifecycleState === "starting" && <LoadingScriptsAlert />}
{lifecycleState === "ready" &&
prevLifecycleState.current === "starting" && <LoadedScriptsAlert />}
{terminalState === "disconnected" && <DisconnectedAlert />}
<TerminalAlerts
agent={workspaceAgent}
state={terminalState}
onAlertChange={() => {
fitAddonRef.current?.fit();
}}
/>
<div css={styles.terminal} ref={xtermRef} data-testid="terminal" />
</div>
@ -328,6 +328,62 @@ const TerminalPage: FC = () => {
);
};
type TerminalAlertsProps = {
agent: WorkspaceAgent | undefined;
state: TerminalState;
onAlertChange: () => void;
};
const TerminalAlerts = ({
agent,
state,
onAlertChange,
}: TerminalAlertsProps) => {
const lifecycleState = agent?.lifecycle_state;
const prevLifecycleState = useRef(lifecycleState);
useEffect(() => {
prevLifecycleState.current = lifecycleState;
}, [lifecycleState]);
// We want to observe the children of the wrapper to detect when the alert
// changes. So the terminal page can resize itself.
//
// Would it be possible to just always call fit() when this component
// re-renders instead of using an observer?
//
// This is a good question and the why this does not work is that the .fit()
// needs to run after the render so in this case, I just think the mutation
// observer is more reliable. I could use some hacky setTimeout inside of
// useEffect to do that, I guess, but I don't think it would be any better.
const wrapperRef = useRef<HTMLDivElement>(null);
useEffect(() => {
if (!wrapperRef.current) {
return;
}
const observer = new MutationObserver(onAlertChange);
observer.observe(wrapperRef.current, { childList: true });
return () => {
observer.disconnect();
};
}, [onAlertChange]);
return (
<div ref={wrapperRef}>
{state === "disconnected" ? (
<DisconnectedAlert />
) : lifecycleState === "start_error" ? (
<ErrorScriptAlert />
) : lifecycleState === "starting" ? (
<LoadingScriptsAlert />
) : lifecycleState === "ready" &&
prevLifecycleState.current === "starting" ? (
<LoadedScriptsAlert />
) : null}
</div>
);
};
const styles = {
terminal: (theme) => ({
width: "100%",