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

BrunoQuaresma · web-flow · commit 060033e4efcc · 2024-03-08T07:38:40.000-03:00
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 #7914
diff --git a/site/.storybook/preview.jsx b/site/.storybook/preview.jsx
@@ -69,6 +69,13 @@ export const parameters = {
         },
         type: "tablet",
       },
+      terminal: {
+        name: "Terminal",
+        styles: {
+          height: "400",
+          width: "400",
+        },
+      },
     },
   },
 };
diff --git a/site/src/@types/storybook.d.ts b/site/src/@types/storybook.d.ts
@@ -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;
diff --git a/site/src/pages/TerminalPage/TerminalPage.stories.tsx b/site/src/pages/TerminalPage/TerminalPage.stories.tsx
@@ -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")],
+  },
+};
diff --git a/site/src/pages/TerminalPage/TerminalPage.tsx b/site/src/pages/TerminalPage/TerminalPage.tsx
@@ -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%",
