From 98ea0c02535e27593720797610d746d57ca7f5ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=CE=A3rebe=20-=20Romain=20GERARD?= Date: Mon, 13 Jul 2026 10:23:33 +0200 Subject: [PATCH] fix(shell): Take a bigger margin for terminal size 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 --- .../lib/cluster-terminal/cluster-terminal.tsx | 22 ++-- .../lib/service-terminal/service-terminal.tsx | 20 ++-- libs/shared/util-hooks/src/index.ts | 1 + .../use-terminal-dimensions.spec.tsx | 94 ++++++++++++++++ .../use-terminal-dimensions.ts | 101 ++++++++++++++++++ 5 files changed, 223 insertions(+), 15 deletions(-) create mode 100644 libs/shared/util-hooks/src/lib/use-terminal-dimensions/use-terminal-dimensions.spec.tsx create mode 100644 libs/shared/util-hooks/src/lib/use-terminal-dimensions/use-terminal-dimensions.ts diff --git a/libs/domains/clusters/feature/src/lib/cluster-terminal/cluster-terminal.tsx b/libs/domains/clusters/feature/src/lib/cluster-terminal/cluster-terminal.tsx index e4f3a02581a..7ff67119793 100644 --- a/libs/domains/clusters/feature/src/lib/cluster-terminal/cluster-terminal.tsx +++ b/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 (
-
+
{isTerminalLoading ? (
diff --git a/libs/domains/services/feature/src/lib/service-terminal/service-terminal.tsx b/libs/domains/services/feature/src/lib/service-terminal/service-terminal.tsx index 513f5ebb386..26034b24740 100644 --- a/libs/domains/services/feature/src/lib/service-terminal/service-terminal.tsx +++ b/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({
-
+
{terminalLaunchError ? (

Request ID: {requestId}

diff --git a/libs/shared/util-hooks/src/index.ts b/libs/shared/util-hooks/src/index.ts index c3daf7a75de..8131105e9ba 100644 --- a/libs/shared/util-hooks/src/index.ts +++ b/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' diff --git a/libs/shared/util-hooks/src/lib/use-terminal-dimensions/use-terminal-dimensions.spec.tsx b/libs/shared/util-hooks/src/lib/use-terminal-dimensions/use-terminal-dimensions.spec.tsx new file mode 100644 index 00000000000..ba1787247bd --- /dev/null +++ b/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[0]) { + const { ref, cols, rows } = useTerminalDimensions(props) + return ( +
+ {cols} + {rows} +
+ ) +} + +function readDimensions() { + return { + cols: Number(screen.getByTestId('cols').textContent), + rows: Number(screen.getByTestId('rows').textContent), + } +} + +function renderDimensions(props: Parameters[0]) { + const { rerender } = render() + return { ...readDimensions(), rerender: () => rerender() } +} + +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 }) + }) +}) diff --git a/libs/shared/util-hooks/src/lib/use-terminal-dimensions/use-terminal-dimensions.ts b/libs/shared/util-hooks/src/lib/use-terminal-dimensions/use-terminal-dimensions.ts new file mode 100644 index 00000000000..5c1794a1d4e --- /dev/null +++ b/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(null) + const [dimensions, setDimensions] = useState({ + 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