diff --git a/site/src/modules/resources/AgentLogs/AgentLogLine.tsx b/site/src/modules/resources/AgentLogs/AgentLogLine.tsx index 46f6e24784e..91c9f078932 100644 --- a/site/src/modules/resources/AgentLogs/AgentLogLine.tsx +++ b/site/src/modules/resources/AgentLogs/AgentLogLine.tsx @@ -7,7 +7,7 @@ export const AGENT_LOG_LINE_HEIGHT = 20; interface AgentLogLineProps { line: Line; - style: React.CSSProperties; + style?: React.CSSProperties; sourceIcon: ReactNode; } @@ -28,7 +28,7 @@ export const AgentLogLine: FC = ({ }, [line.time]); return ( - + {sourceIcon} {timestamp} diff --git a/site/src/modules/resources/AgentLogs/AgentLogs.test.tsx b/site/src/modules/resources/AgentLogs/AgentLogs.test.tsx new file mode 100644 index 00000000000..d59f716f4ad --- /dev/null +++ b/site/src/modules/resources/AgentLogs/AgentLogs.test.tsx @@ -0,0 +1,86 @@ +import { render, waitFor } from "@testing-library/react"; +import type { Line } from "#/components/Logs/LogLine"; +import { AGENT_LOG_LINE_HEIGHT } from "./AgentLogLine"; +import { AgentLogs } from "./AgentLogs"; +import { MockSources } from "./mocks"; + +// A real log row renders taller than the AGENT_LOG_LINE_HEIGHT estimate (e.g. +// when output wraps). We simulate that by reporting a fixed measured height +// that is larger than the estimate. +const MEASURED_ROW_HEIGHT = 40; +const ROW_COUNT = 5; + +const makeLogs = (count: number): Line[] => + Array.from({ length: count }, (_, i) => ({ + id: i + 1, + level: "info", + output: `log line ${i + 1}`, + sourceId: MockSources[0].id, + time: "2024-03-14T11:31:04.090715Z", + })); + +const getInnerSizer = (container: HTMLElement): HTMLElement => { + const outer = container.querySelector( + 'div[style*="overflow: auto"]', + ); + const inner = outer?.firstElementChild as HTMLElement | null; + if (!inner) { + throw new Error("react-window inner sizing element not found"); + } + return inner; +}; + +describe("AgentLogs virtualized height", () => { + let boundingRect: ReturnType; + + beforeAll(() => { + // jsdom performs no layout, so make every row report a height that is + // taller than the fixed estimate. The list must measure and use this + // height rather than assuming AGENT_LOG_LINE_HEIGHT per row. + boundingRect = vi + .spyOn(HTMLElement.prototype, "getBoundingClientRect") + .mockReturnValue({ + height: MEASURED_ROW_HEIGHT, + width: 600, + top: 0, + left: 0, + right: 600, + bottom: MEASURED_ROW_HEIGHT, + x: 0, + y: 0, + toJSON: () => ({}), + } as DOMRect); + }); + + afterAll(() => { + boundingRect.mockRestore(); + }); + + it("sizes the scroll area from measured row heights, not the fixed estimate", async () => { + const { container } = render( + , + ); + + // The virtualized height must account for the real, taller rows. + // react-window's fixed itemSize would produce ROW_COUNT * + // AGENT_LOG_LINE_HEIGHT, which is short of the real content and is what + // left the last lines unreachable (coder/coder#25692). + await waitFor(() => { + const totalHeight = Number.parseFloat( + getInnerSizer(container).style.height, + ); + expect(totalHeight).toBe(ROW_COUNT * MEASURED_ROW_HEIGHT); + }); + expect( + Number.parseFloat(getInnerSizer(container).style.height), + ).toBeGreaterThan(ROW_COUNT * AGENT_LOG_LINE_HEIGHT); + }); +}); diff --git a/site/src/modules/resources/AgentLogs/AgentLogs.tsx b/site/src/modules/resources/AgentLogs/AgentLogs.tsx index 83f43545c35..0f57a422fd4 100644 --- a/site/src/modules/resources/AgentLogs/AgentLogs.tsx +++ b/site/src/modules/resources/AgentLogs/AgentLogs.tsx @@ -1,5 +1,14 @@ -import type { JSX } from "react"; -import { FixedSizeList as List } from "react-window"; +import { + type CSSProperties, + type FC, + type JSX, + type ReactNode, + type Ref, + useCallback, + useLayoutEffect, + useRef, +} from "react"; +import { VariableSizeList as List } from "react-window"; import type { WorkspaceAgentLogSource } from "#/api/typesGenerated"; import { Badge } from "#/components/Badge/Badge"; import { ExternalImage } from "#/components/ExternalImage/ExternalImage"; @@ -24,32 +33,95 @@ const fallbackLog: WorkspaceAgentLogSource = { }; type AgentLogsProps = Omit< - React.ComponentPropsWithRef, + React.ComponentPropsWithoutRef, "children" | "itemSize" | "itemCount" | "itemKey" > & { logs: readonly Line[]; sources: readonly WorkspaceAgentLogSource[]; overflowed: boolean; showSourceIcons?: boolean; + ref?: Ref; }; -export const AgentLogs: React.FC = ({ +export const AgentLogs: FC = ({ logs, sources, overflowed, className, showSourceIcons = true, + ref, ...listProps }) => { const logSourceById = Object.fromEntries(sources.map((s) => [s.id, s])); const getLogSource = (id: string) => logSourceById[id] || fallbackLog; + const listRef = useRef(null); + const mergeListRef = useCallback( + (instance: List | null) => { + listRef.current = instance; + if (typeof ref === "function") { + ref(instance); + } else if (ref) { + ref.current = instance; + } + }, + [ref], + ); + + // A log line's real height depends on its content (long lines wrap, so a + // single log entry can span multiple visual rows). A fixed itemSize makes + // react-window compute the total scroll height as itemCount * itemSize, + // which drifts further from reality with every taller-than-expected row. + // With enough rows the estimated height falls short of the real content + // and the list can no longer scroll to the last line (coder/coder#25692). + // Measuring each row and feeding the true height back keeps the total + // height accurate regardless of how many rows there are. + const sizeMapRef = useRef>(new Map()); + const pendingResetIndexRef = useRef(null); + // resetAfterIndex tells react-window to recompute offsets from the first + // row whose height changed. The list ref is assigned after the child rows + // commit, so on the initial mount we can't reset from within a row's + // measurement effect; instead we record the lowest changed index and flush + // it from a parent layout effect once the ref is available. + const flushPendingReset = useCallback(() => { + const index = pendingResetIndexRef.current; + if (index !== null && listRef.current) { + pendingResetIndexRef.current = null; + listRef.current.resetAfterIndex(index); + } + }, []); + const setRowHeight = useCallback( + (index: number, height: number) => { + if (sizeMapRef.current.get(index) === height) { + return; + } + sizeMapRef.current.set(index, height); + pendingResetIndexRef.current = + pendingResetIndexRef.current === null + ? index + : Math.min(pendingResetIndexRef.current, index); + flushPendingReset(); + }, + [flushPendingReset], + ); + const getRowHeight = useCallback( + (index: number) => sizeMapRef.current.get(index) ?? AGENT_LOG_LINE_HEIGHT, + [], + ); + + // Flush measurements taken before the list ref existed (initial mount). + useLayoutEffect(() => { + flushPendingReset(); + }); + return (
logs[index]?.id || index} // We need the div selector to be able to apply the padding // top from startupLogs @@ -125,26 +197,31 @@ export const AgentLogs: React.FC = ({ } return ( - - {icon} - - {logSource.display_name} - {assignedIcon && ( - -
- No icon specified! - - )} - - - ) : null - } - /> + onMeasure={setRowHeight} + > + + {icon} + + {logSource.display_name} + {assignedIcon && ( + +
+ No icon specified! + + )} + + + ) : null + } + /> + ); }} @@ -180,6 +257,50 @@ export const AgentLogs: React.FC = ({ ); }; +interface MeasuredLogRowProps { + index: number; + // react-window's positioning style for the row (absolute top/left/width). + style: CSSProperties; + onMeasure: (index: number, height: number) => void; + children: ReactNode; +} + +// Wraps a log line and reports its rendered height back to the virtualized +// list. The height is left to the content (`height: auto`) so wrapped or +// multi-line output is measured accurately instead of being assumed to be a +// single fixed-height row. +const MeasuredLogRow: FC = ({ + index, + style, + onMeasure, + children, +}) => { + const rowRef = useRef(null); + + useLayoutEffect(() => { + const el = rowRef.current; + if (!el) { + return; + } + const report = () => { + const height = el.getBoundingClientRect().height; + if (height > 0) { + onMeasure(index, height); + } + }; + report(); + const observer = new ResizeObserver(report); + observer.observe(el); + return () => observer.disconnect(); + }, [index, onMeasure]); + + return ( +
+ {children} +
+ ); +}; + // These colors were picked at random. Feel free to add more, adjust, or change! // Users will not depend on these colors. const scriptDisplayColors: readonly string[] = [ diff --git a/site/src/modules/resources/AgentRow.tsx b/site/src/modules/resources/AgentRow.tsx index 12a8a2e5662..0a354d34640 100644 --- a/site/src/modules/resources/AgentRow.tsx +++ b/site/src/modules/resources/AgentRow.tsx @@ -17,7 +17,7 @@ import { } from "react"; import { Link as RouterLink } from "react-router"; import AutoSizer from "react-virtualized-auto-sizer"; -import type { FixedSizeList as List, ListOnScrollProps } from "react-window"; +import type { VariableSizeList as List, ListOnScrollProps } from "react-window"; import type { AgentScriptTiming, Template, diff --git a/site/src/pages/TaskPage/TaskPage.tsx b/site/src/pages/TaskPage/TaskPage.tsx index 1c15ec4db2f..c3142ab8f15 100644 --- a/site/src/pages/TaskPage/TaskPage.tsx +++ b/site/src/pages/TaskPage/TaskPage.tsx @@ -18,7 +18,7 @@ import { import { useMutation, useQuery, useQueryClient } from "react-query"; import { Panel, PanelGroup, PanelResizeHandle } from "react-resizable-panels"; import { Link as RouterLink, useParams } from "react-router"; -import type { FixedSizeList } from "react-window"; +import type { VariableSizeList } from "react-window"; import { toast } from "sonner"; import { API } from "#/api/api"; import { getErrorDetail, getErrorMessage, isApiError } from "#/api/errors"; @@ -830,7 +830,7 @@ type TaskStartingAgentProps = { const TaskStartingAgent: FC = ({ task, agent }) => { const logs = useAgentLogs({ agentId: agent.id }); - const listRef = useRef(null); + const listRef = useRef(null); const queryClient = useQueryClient(); const pauseMutation = useMutation({ ...pauseTask(task, queryClient),