fix(shell): Take a bigger margin for terminal size (#2811)

When openning the shell websocket, the frontend compute the width and height of the terminal to forward it to the backend.
Today, the width and height are too big causing line wrapping

    Use the shell container size instead of the global one
    increase the cell height to avoid line wrapping

Author: erebe

libs/domains/clusters/feature/src/lib/cluster-terminal/cluster-terminal.tsx

@@ -9,7 +9,7 @@ import { memo, useCallback, useEffect, useMemo, useState } from 'react'
 import { XTerm } from 'react-xtermjs'
 import { match } from 'ts-pattern'
 import { LoaderSpinner, toast } from '@qovery/shared/ui'
-import { useTerminalReadiness } from '@qovery/shared/util-hooks'
+import { useTerminalDimensions, useTerminalReadiness } from '@qovery/shared/util-hooks'
 import { QOVERY_WS } from '@qovery/shared/util-node-env'
 import { useReactQueryWsSubscription } from '@qovery/state/util-queries'
 
@@ -76,12 +76,18 @@ export function ClusterTerminal({ organizationId, clusterId, cloudProvider }: Cl
     [detachWebSocket]
   )
 
-  // Necesssary to calculate the number of rows and columns (tty) for the terminal
-  // https://github.com/xtermjs/xterm.js/issues/1412#issuecomment-724421101
-  // 14 is the font height for better k9s compatibility
-  const rows = Math.ceil(document.body.clientHeight / 14)
-  // 9 is the font width for better k9s compatibility, with a minimum width of 80 columns
-  const cols = Math.max(80, Math.ceil(document.body.clientWidth / 9))
+  // Measure the actual terminal container (not document.body) so the TTY size sent to the
+  // backend matches what is rendered. https://github.com/xtermjs/xterm.js/issues/1412#issuecomment-724421101
+  // 9x17 is the font cell size for better k9s compatibility, with a minimum width of 80 columns.
+  const {
+    ref: terminalContainerRef,
+    cols,
+    rows,
+  } = useTerminalDimensions({
+    cellWidth: 9,
+    cellHeight: 17,
+    minCols: 80,
+  })
 
   useReactQueryWsSubscription({
     url: QOVERY_WS + '/shell/debug',
@@ -104,7 +110,7 @@ export function ClusterTerminal({ organizationId, clusterId, cloudProvider }: Cl
 
   return (
     <div className="flex h-full min-h-0 w-full flex-1 flex-col overflow-hidden rounded-none border-0 bg-background">
-      <div className="relative h-full flex-1 border-neutral bg-background px-4 py-2">
+      <div ref={terminalContainerRef} className="relative h-full flex-1 border-neutral bg-background px-4 py-2">
         {isTerminalLoading ? (
           <div className="flex h-40 items-start justify-center p-5">
             <LoaderSpinner />

libs/domains/services/feature/src/lib/service-terminal/service-terminal.tsx

@@ -19,7 +19,7 @@ import {
   toast,
   useModal,
 } from '@qovery/shared/ui'
-import { useTerminalReadiness } from '@qovery/shared/util-hooks'
+import { useTerminalDimensions, useTerminalReadiness } from '@qovery/shared/util-hooks'
 import { QOVERY_WS } from '@qovery/shared/util-node-env'
 import { useReactQueryWsSubscription } from '@qovery/state/util-queries'
 import { useRunningStatus, useService } from '../..'
@@ -236,11 +236,17 @@ export function ServiceTerminal({
     [runningStatuses?.state]
   )
 
-  // Necesssary to calculate the number of rows and columns (tty) for the terminal
-  // https://github.com/xtermjs/xterm.js/issues/1412#issuecomment-724421101
-  // 16 is the font height
-  const rows = Math.ceil(document.body.clientHeight / 16)
-  const cols = Math.ceil(document.body.clientWidth / 8)
+  // Measure the actual terminal container (not document.body) so the TTY size sent to the
+  // backend matches what is rendered. https://github.com/xtermjs/xterm.js/issues/1412#issuecomment-724421101
+  // 9x17 is the font cell size.
+  const {
+    ref: terminalContainerRef,
+    cols,
+    rows,
+  } = useTerminalDimensions({
+    cellWidth: 9,
+    cellHeight: 17,
+  })
 
   useReactQueryWsSubscription({
     url: QOVERY_WS + (ephemeralConfig ? '/shell/ephemeral' : '/shell/exec'),
@@ -321,7 +327,7 @@ export function ServiceTerminal({
         </ExternalLink>
       </div>
       <div className="flex h-full flex-1 flex-col bg-background px-3 pt-5">
-        <div className="relative min-h-0 flex-1">
+        <div ref={terminalContainerRef} className="relative min-h-0 flex-1">
           {terminalLaunchError ? (
             <EmptyState icon="terminal" title="Unable to launch CLI" description={terminalUnavailableDescription}>
               <p className="text-xs text-neutral-subtle">Request ID: {requestId}</p>

libs/shared/util-hooks/src/index.ts

@@ -9,6 +9,7 @@ export * from './lib/use-local-storage/use-local-storage'
 export * from './lib/use-support-chat/use-support-chat'
 export * from './lib/use-pod-color/use-pod-color'
 export * from './lib/use-terminal-readiness/use-terminal-readiness'
+export * from './lib/use-terminal-dimensions/use-terminal-dimensions'
 export * from './lib/use-interval-tick/use-interval-tick'
 export * from './lib/use-utm-params/use-utm-params'
 export * from './lib/use-os/use-os'

libs/shared/util-hooks/src/lib/use-terminal-dimensions/use-terminal-dimensions.spec.tsx

@@ -0,0 +1,94 @@
+import { render, screen } from '@qovery/shared/util-tests'
+import { useTerminalDimensions } from './use-terminal-dimensions'
+
+// jsdom does not lay out elements, so we stub the geometry the hook reads. The
+// returned box lets a test mutate the size after mount to simulate a resize.
+function mockElementBox(width: number, height: number, padding = 0) {
+  const box = { width, height }
+  jest.spyOn(HTMLElement.prototype, 'clientWidth', 'get').mockImplementation(() => box.width)
+  jest.spyOn(HTMLElement.prototype, 'clientHeight', 'get').mockImplementation(() => box.height)
+  jest.spyOn(window, 'getComputedStyle').mockReturnValue({
+    paddingLeft: `${padding}px`,
+    paddingRight: `${padding}px`,
+    paddingTop: `${padding}px`,
+    paddingBottom: `${padding}px`,
+  } as CSSStyleDeclaration)
+  return box
+}
+
+function TestComponent(props: Parameters<typeof useTerminalDimensions>[0]) {
+  const { ref, cols, rows } = useTerminalDimensions(props)
+  return (
+    <div ref={ref} data-testid="terminal-container">
+      <span data-testid="cols">{cols}</span>
+      <span data-testid="rows">{rows}</span>
+    </div>
+  )
+}
+
+function readDimensions() {
+  return {
+    cols: Number(screen.getByTestId('cols').textContent),
+    rows: Number(screen.getByTestId('rows').textContent),
+  }
+}
+
+function renderDimensions(props: Parameters<typeof useTerminalDimensions>[0]) {
+  const { rerender } = render(<TestComponent {...props} />)
+  return { ...readDimensions(), rerender: () => rerender(<TestComponent {...props} />) }
+}
+
+describe('useTerminalDimensions', () => {
+  afterEach(() => {
+    jest.restoreAllMocks()
+  })
+
+  it('computes cols and rows from the measured container box', () => {
+    mockElementBox(800, 400)
+
+    // 800 / 8 = 100 cols, 400 / 16 = 25 rows
+    const { cols, rows } = renderDimensions({ cellWidth: 8, cellHeight: 16 })
+    expect({ cols, rows }).toEqual({ cols: 100, rows: 25 })
+  })
+
+  it('subtracts container padding before dividing by the cell size', () => {
+    mockElementBox(800, 400, 20)
+
+    // (800 - 40) / 8 = 95 cols, (400 - 40) / 16 = 22.5 -> 22 rows
+    const { cols, rows } = renderDimensions({ cellWidth: 8, cellHeight: 16 })
+    expect({ cols, rows }).toEqual({ cols: 95, rows: 22 })
+  })
+
+  it('respects the configured minimum columns and rows', () => {
+    mockElementBox(100, 8)
+
+    // 100 / 9 = 11 cols but min is 80; 8 / 14 = 0 rows but min is 1
+    const { cols, rows } = renderDimensions({ cellWidth: 9, cellHeight: 14, minCols: 80, minRows: 1 })
+    expect({ cols, rows }).toEqual({ cols: 80, rows: 1 })
+  })
+
+  it('does not re-measure after the initial capture, so a resize keeps the socket dimensions stable', () => {
+    // The returned cols/rows feed the websocket connect params; the subscription
+    // reconnects when they change. Latching on capture is what keeps a running
+    // shell alive across window/panel resizes.
+    const box = mockElementBox(800, 400)
+
+    const { cols, rows, rerender } = renderDimensions({ cellWidth: 8, cellHeight: 16 })
+    expect({ cols, rows }).toEqual({ cols: 100, rows: 25 })
+
+    // Simulate a resize + re-render: the reported dimensions must not change.
+    box.width = 400
+    box.height = 200
+    rerender()
+
+    expect(readDimensions()).toEqual({ cols: 100, rows: 25 })
+  })
+
+  it('keeps the min defaults when the container is not laid out yet (0x0)', () => {
+    // A 0x0 box means layout has not resolved; we must not latch a bogus size.
+    mockElementBox(0, 0)
+
+    const { cols, rows } = renderDimensions({ cellWidth: 8, cellHeight: 16, minCols: 80, minRows: 1 })
+    expect({ cols, rows }).toEqual({ cols: 80, rows: 1 })
+  })
+})

libs/shared/util-hooks/src/lib/use-terminal-dimensions/use-terminal-dimensions.ts

@@ -0,0 +1,101 @@
+import { useLayoutEffect, useRef, useState } from 'react'
+
+export interface UseTerminalDimensionsOptions {
+  /** Width of a single character cell in pixels (matches the terminal font). */
+  cellWidth: number
+  /** Height of a single character cell in pixels (matches the terminal font). */
+  cellHeight: number
+  /** Minimum number of columns, useful to keep TUIs (e.g. k9s) usable on narrow layouts. */
+  minCols?: number
+  /** Minimum number of rows. */
+  minRows?: number
+}
+
+export interface TerminalDimensions {
+  cols: number
+  rows: number
+}
+
+function computeDimensions(
+  element: HTMLElement,
+  { cellWidth, cellHeight, minCols = 2, minRows = 1 }: UseTerminalDimensionsOptions
+): TerminalDimensions {
+  const styles = window.getComputedStyle(element)
+  const paddingX = parseFloat(styles.paddingLeft) + parseFloat(styles.paddingRight)
+  const paddingY = parseFloat(styles.paddingTop) + parseFloat(styles.paddingBottom)
+  const availableWidth = Math.max(0, element.clientWidth - paddingX)
+  const availableHeight = Math.max(0, element.clientHeight - paddingY)
+
+  return {
+    cols: Math.max(minCols, Math.floor(availableWidth / cellWidth)),
+    rows: Math.max(minRows, Math.floor(availableHeight / cellHeight)),
+  }
+}
+
+/**
+ * Measures the terminal container element to derive the TTY `cols`/`rows` that
+ * actually fit, instead of guessing from `document.body`. The value is sent to
+ * the backend at websocket connect time as `tty_width`/`tty_height`.
+ *
+ * The dimensions are latched from the first non-zero measurement and then stay
+ * stable for the lifetime of the mount. This is intentional: the returned
+ * `cols`/`rows` feed the websocket connect params, and the subscription hook
+ * reconnects whenever those params change. If we kept re-measuring on every
+ * resize we would tear down the live socket — and the user's running shell —
+ * every time the window or a panel crossed a cell boundary. So we measure once
+ * the layout has settled and let subsequent resizes be handled client-side by
+ * the xterm fit addon. A genuinely new session (remount) re-measures.
+ *
+ * Returns a `ref` to attach to the element wrapping the terminal, and the
+ * captured dimensions.
+ */
+export function useTerminalDimensions(options: UseTerminalDimensionsOptions) {
+  const ref = useRef<HTMLDivElement | null>(null)
+  const [dimensions, setDimensions] = useState<TerminalDimensions>({
+    cols: options.minCols ?? 2,
+    rows: options.minRows ?? 1,
+  })
+  // Keep the latest options without re-subscribing the observer on every render.
+  const optionsRef = useRef(options)
+  optionsRef.current = options
+
+  useLayoutEffect(() => {
+    const element = ref.current
+    if (!element) {
+      return
+    }
+
+    // Latch the first measurement that reflects a laid-out container. The box can
+    // start at 0x0 before flex/layout resolves, which would otherwise freeze a
+    // bogus (min) size for the whole session.
+    const capture = () => {
+      if (element.clientWidth === 0 && element.clientHeight === 0) {
+        return false
+      }
+      setDimensions(computeDimensions(element, optionsRef.current))
+      return true
+    }
+
+    if (capture()) {
+      return
+    }
+
+    // Not laid out yet: wait for the first resize that gives it a real size,
+    // capture it, then stop observing so later resizes don't churn the socket.
+    if (typeof ResizeObserver === 'undefined') {
+      return
+    }
+    const resizeObserver = new ResizeObserver(() => {
+      if (capture()) {
+        resizeObserver.disconnect()
+      }
+    })
+    resizeObserver.observe(element)
+
+    return () => resizeObserver.disconnect()
+  }, [])
+
+  return { ref, ...dimensions }
+}
+
+export default useTerminalDimensions